From 3a3a0457443ebe0fdb894e1882e347ef38848b3f Mon Sep 17 00:00:00 2001 From: pulak-opti <129880418+pulak-opti@users.noreply.github.com> Date: Wed, 30 Oct 2024 19:03:53 +0600 Subject: [PATCH] [FSSDK-10764] add support for batch UPS for decideAll & decideForKeys (#394) * add support for batch UPS for decideAll & decideForKeys * update decideForKeys logic Signed-off-by: pulak-opti * refactor code Signed-off-by: pulak-opti * fix bug Signed-off-by: pulak-opti * use deepcopy for userprofile Signed-off-by: pulak-opti * fix lint error Signed-off-by: pulak-opti * fix bug Signed-off-by: pulak-opti * fix Signed-off-by: pulak-opti * fix Signed-off-by: pulak-opti * fix * test Signed-off-by: pulak-opti * refactor code Signed-off-by: pulak-opti * fix Signed-off-by: pulak-opti * save profile everytime Signed-off-by: pulak-opti * update tests Signed-off-by: pulak-opti * remove deepcopy method Signed-off-by: pulak-opti * update unit tests Signed-off-by: pulak-opti * update unit tests Signed-off-by: pulak-opti * add unit tests --------- Signed-off-by: pulak-opti --- pkg/client/client.go | 38 ++++++-- pkg/client/factory.go | 4 + pkg/client/optimizely_user_context.go | 27 +++++- pkg/client/optimizely_user_context_test.go | 91 +++++++++++++++++++ pkg/decision/entities.go | 3 + pkg/decision/feature_experiment_service.go | 1 + pkg/decision/persisting_experiment_service.go | 23 ++++- 7 files changed, 171 insertions(+), 16 deletions(-) diff --git a/pkg/client/client.go b/pkg/client/client.go index 61fc8174..9010ad44 100644 --- a/pkg/client/client.go +++ b/pkg/client/client.go @@ -104,6 +104,7 @@ type OptimizelyClient struct { ctx context.Context ConfigManager config.ProjectConfigManager DecisionService decision.Service + UserProfileService decision.UserProfileService EventProcessor event.Processor OdpManager odp.Manager notificationCenter notification.Center @@ -130,7 +131,7 @@ func (o *OptimizelyClient) WithTraceContext(ctx context.Context) *OptimizelyClie return o } -func (o *OptimizelyClient) decide(userContext OptimizelyUserContext, key string, options *decide.Options) OptimizelyDecision { +func (o *OptimizelyClient) decide(userContext *OptimizelyUserContext, key string, options *decide.Options) OptimizelyDecision { var err error defer func() { if r := recover(); r != nil { @@ -153,16 +154,17 @@ func (o *OptimizelyClient) decide(userContext OptimizelyUserContext, key string, decisionContext := decision.FeatureDecisionContext{ ForcedDecisionService: userContext.forcedDecisionService, + UserProfile: userContext.userProfile, } projectConfig, err := o.getProjectConfig() if err != nil { - return NewErrorDecision(key, userContext, decide.GetDecideError(decide.SDKNotReady)) + return NewErrorDecision(key, *userContext, decide.GetDecideError(decide.SDKNotReady)) } decisionContext.ProjectConfig = projectConfig feature, err := projectConfig.GetFeatureByKey(key) if err != nil { - return NewErrorDecision(key, userContext, decide.GetDecideError(decide.FlagKeyInvalid, key)) + return NewErrorDecision(key, *userContext, decide.GetDecideError(decide.FlagKeyInvalid, key)) } decisionContext.Feature = &feature @@ -235,7 +237,7 @@ func (o *OptimizelyClient) decide(userContext OptimizelyUserContext, key string, } } - return NewOptimizelyDecision(variationKey, ruleKey, key, flagEnabled, optimizelyJSON, userContext, reasonsToReport) + return NewOptimizelyDecision(variationKey, ruleKey, key, flagEnabled, optimizelyJSON, *userContext, reasonsToReport) } func (o *OptimizelyClient) decideForKeys(userContext OptimizelyUserContext, keys []string, options *decide.Options) map[string]OptimizelyDecision { @@ -268,13 +270,30 @@ func (o *OptimizelyClient) decideForKeys(userContext OptimizelyUserContext, keys if len(keys) == 0 { return decisionMap } + allOptions := o.getAllOptions(options) - enabledFlagsOnly := o.getAllOptions(options).EnabledFlagsOnly - for _, key := range keys { - optimizelyDecision := o.decide(userContext, key, options) - if !enabledFlagsOnly || optimizelyDecision.Enabled { - decisionMap[key] = optimizelyDecision + var userProfile *decision.UserProfile + ignoreUserProfileSvc := o.UserProfileService == nil || allOptions.IgnoreUserProfileService + if !ignoreUserProfileSvc { + up := o.UserProfileService.Lookup(userContext.GetUserID()) + if up.ID == "" { + up = decision.UserProfile{ + ID: userContext.GetUserID(), + ExperimentBucketMap: map[decision.UserDecisionKey]string{}, + } } + userProfile = &up + userContext.userProfile = userProfile + } + + for _, key := range keys { + optimizelyDecision := o.decide(&userContext, key, options) + decisionMap[key] = optimizelyDecision + } + + if !ignoreUserProfileSvc && userProfile != nil && userProfile.HasUnsavedChange { + o.UserProfileService.Save(*userProfile) + userProfile.HasUnsavedChange = false } return decisionMap @@ -1076,6 +1095,7 @@ func (o *OptimizelyClient) getExperimentDecision(experimentKey string, userConte decisionContext = decision.ExperimentDecisionContext{ Experiment: &experiment, ProjectConfig: projectConfig, + UserProfile: nil, } options := &decide.Options{} diff --git a/pkg/client/factory.go b/pkg/client/factory.go index fd9d62ed..e7e8dd1d 100644 --- a/pkg/client/factory.go +++ b/pkg/client/factory.go @@ -145,6 +145,10 @@ func (f *OptimizelyFactory) Client(clientOptions ...OptionFunc) (*OptimizelyClie appClient.EventProcessor = event.NewBatchEventProcessor(eventProcessorOptions...) } + if f.userProfileService != nil { + appClient.UserProfileService = f.userProfileService + } + if f.decisionService != nil { appClient.DecisionService = f.decisionService } else { diff --git a/pkg/client/optimizely_user_context.go b/pkg/client/optimizely_user_context.go index 1b592247..e6254346 100644 --- a/pkg/client/optimizely_user_context.go +++ b/pkg/client/optimizely_user_context.go @@ -35,6 +35,7 @@ type OptimizelyUserContext struct { qualifiedSegments []string optimizely *OptimizelyClient forcedDecisionService *pkgDecision.ForcedDecisionService + userProfile *pkgDecision.UserProfile mutex *sync.RWMutex } @@ -130,21 +131,31 @@ func (o *OptimizelyUserContext) IsQualifiedFor(segment string) bool { func (o *OptimizelyUserContext) Decide(key string, options []decide.OptimizelyDecideOptions) OptimizelyDecision { // use a copy of the user context so that any changes to the original context are not reflected inside the decision userContextCopy := newOptimizelyUserContext(o.GetOptimizely(), o.GetUserID(), o.GetUserAttributes(), o.getForcedDecisionService(), o.GetQualifiedSegments()) - return o.optimizely.decide(userContextCopy, key, convertDecideOptions(options)) + decision, found := o.optimizely.decideForKeys(userContextCopy, []string{key}, convertDecideOptions(options))[key] + if !found { + return NewErrorDecision(key, *o, decide.GetDecideError(decide.SDKNotReady)) + } + return decision } // DecideAll returns a key-map of decision results for all active flag keys with options. func (o *OptimizelyUserContext) DecideAll(options []decide.OptimizelyDecideOptions) map[string]OptimizelyDecision { // use a copy of the user context so that any changes to the original context are not reflected inside the decision userContextCopy := newOptimizelyUserContext(o.GetOptimizely(), o.GetUserID(), o.GetUserAttributes(), o.getForcedDecisionService(), o.GetQualifiedSegments()) - return o.optimizely.decideAll(userContextCopy, convertDecideOptions(options)) + decideOptions := convertDecideOptions(options) + decisionMap := o.optimizely.decideAll(userContextCopy, decideOptions) + + return filteredDecision(decisionMap, o.optimizely.getAllOptions(decideOptions).EnabledFlagsOnly) } // DecideForKeys returns a key-map of decision results for multiple flag keys and options. func (o *OptimizelyUserContext) DecideForKeys(keys []string, options []decide.OptimizelyDecideOptions) map[string]OptimizelyDecision { // use a copy of the user context so that any changes to the original context are not reflected inside the decision userContextCopy := newOptimizelyUserContext(o.GetOptimizely(), o.GetUserID(), o.GetUserAttributes(), o.getForcedDecisionService(), o.GetQualifiedSegments()) - return o.optimizely.decideForKeys(userContextCopy, keys, convertDecideOptions(options)) + decideOptions := convertDecideOptions(options) + decisionMap := o.optimizely.decideForKeys(userContextCopy, keys, decideOptions) + + return filteredDecision(decisionMap, o.optimizely.getAllOptions(decideOptions).EnabledFlagsOnly) } // TrackEvent generates a conversion event with the given event key if it exists and queues it up to be sent to the Optimizely @@ -208,3 +219,13 @@ func copyQualifiedSegments(qualifiedSegments []string) (qualifiedSegmentsCopy [] copy(qualifiedSegmentsCopy, qualifiedSegments) return } + +func filteredDecision(decisionMap map[string]OptimizelyDecision, enabledFlagsOnly bool) map[string]OptimizelyDecision { + filteredDecision := make(map[string]OptimizelyDecision) + for key, decision := range decisionMap { + if !enabledFlagsOnly || decision.Enabled { + filteredDecision[key] = decision + } + } + return filteredDecision +} diff --git a/pkg/client/optimizely_user_context_test.go b/pkg/client/optimizely_user_context_test.go index e85fe676..f74b83cc 100644 --- a/pkg/client/optimizely_user_context_test.go +++ b/pkg/client/optimizely_user_context_test.go @@ -1221,6 +1221,97 @@ func (s *OptimizelyUserContextTestSuite) TestForcedDecision() { s.Error(err) } +func (s *OptimizelyUserContextTestSuite) TestDecideAllFlagsWithBatchUPS() { + userProfileService := new(MockUserProfileService) + var err error + s.OptimizelyClient, err = s.factory.Client( + WithEventProcessor(s.eventProcessor), + WithUserProfileService(userProfileService), + ) + s.Nil(err) + + savedUserProfile := decision.UserProfile{ + ID: s.userID, + } + userProfileService.On("Lookup", s.userID).Return(savedUserProfile) + userProfileService.On("Save", mock.Anything) + + user := s.OptimizelyClient.CreateUserContext(s.userID, nil) + decisions := user.DecideAll(nil) + s.Len(decisions, 3) + + userProfileService.AssertNumberOfCalls(s.T(), "Lookup", 1) + userProfileService.AssertNumberOfCalls(s.T(), "Save", 1) +} + +func (s *OptimizelyUserContextTestSuite) TestDecideForKeysWithBatchUPS() { + flagKey1 := "feature_1" + experimentID1 := "10390977673" + variationKey1 := "18257766532" + variationID1 := "variation_with_traffic" + flagKey2 := "feature_2" // embedding experiment: "exp_no_audience" + experimentID2 := "10420810910" + variationID2 := "10418510624" + variationKey2 := "variation_no_traffic" + userProfileService := new(MockUserProfileService) + var err error + s.OptimizelyClient, err = s.factory.Client( + WithEventProcessor(s.eventProcessor), + WithUserProfileService(userProfileService), + ) + s.Nil(err) + + savedUserProfile := decision.UserProfile{ + ID: s.userID, + ExperimentBucketMap: map[decision.UserDecisionKey]string{ + decision.NewUserDecisionKey(experimentID1): variationID1, + decision.NewUserDecisionKey(experimentID2): variationID2, + }, + } + userProfileService.On("Lookup", s.userID).Return(savedUserProfile) + userProfileService.On("Save", mock.Anything) + + user := s.OptimizelyClient.CreateUserContext(s.userID, nil) + decisions := user.DecideForKeys([]string{flagKey1, flagKey2}, nil) + s.Len(decisions, 2) + s.Equal(variationKey1, decisions[flagKey1].VariationKey) + s.Equal(variationKey2, decisions[flagKey2].VariationKey) + + userProfileService.AssertNumberOfCalls(s.T(), "Lookup", 1) + userProfileService.AssertNumberOfCalls(s.T(), "Save", 0) +} + +func (s *OptimizelyUserContextTestSuite) TestDecideWithBatchUPS() { + flagKey := "feature_2" // embedding experiment: "exp_no_audience" + experimentID := "10420810910" + variationID2 := "10418510624" + variationKey1 := "variation_no_traffic" + + userProfileService := new(MockUserProfileService) + s.OptimizelyClient, _ = s.factory.Client( + WithEventProcessor(s.eventProcessor), + WithUserProfileService(userProfileService), + ) + + decisionKey := decision.NewUserDecisionKey(experimentID) + savedUserProfile := decision.UserProfile{ + ID: s.userID, + ExperimentBucketMap: map[decision.UserDecisionKey]string{decisionKey: variationID2}, + } + userProfileService.On("Lookup", s.userID).Return(savedUserProfile) + userProfileService.On("Save", mock.Anything) + + client, err := s.factory.Client(WithUserProfileService(userProfileService)) + s.Nil(err) + user := client.CreateUserContext(s.userID, nil) + decision := user.Decide(flagKey, []decide.OptimizelyDecideOptions{decide.IncludeReasons}) + s.Len(decision.Reasons, 1) + + s.Equal(variationKey1, decision.VariationKey) + userProfileService.AssertCalled(s.T(), "Lookup", s.userID) + userProfileService.AssertNotCalled(s.T(), "Save", mock.Anything) +} + func TestOptimizelyUserContextTestSuite(t *testing.T) { suite.Run(t, new(OptimizelyUserContextTestSuite)) } diff --git a/pkg/decision/entities.go b/pkg/decision/entities.go index 63139329..bebfe5f4 100644 --- a/pkg/decision/entities.go +++ b/pkg/decision/entities.go @@ -27,6 +27,7 @@ import ( type ExperimentDecisionContext struct { Experiment *entities.Experiment ProjectConfig config.ProjectConfig + UserProfile *UserProfile } // FeatureDecisionContext contains the information needed to be able to make a decision for a given feature @@ -35,6 +36,7 @@ type FeatureDecisionContext struct { ProjectConfig config.ProjectConfig Variable entities.Variable ForcedDecisionService *ForcedDecisionService + UserProfile *UserProfile } // UnsafeFeatureDecisionInfo represents response for GetDetailedFeatureDecisionUnsafe api @@ -92,4 +94,5 @@ func NewUserDecisionKey(experimentID string) UserDecisionKey { type UserProfile struct { ID string ExperimentBucketMap map[UserDecisionKey]string + HasUnsavedChange bool } diff --git a/pkg/decision/feature_experiment_service.go b/pkg/decision/feature_experiment_service.go index e9f07ec6..f8c7132b 100644 --- a/pkg/decision/feature_experiment_service.go +++ b/pkg/decision/feature_experiment_service.go @@ -64,6 +64,7 @@ func (f FeatureExperimentService) GetDecision(decisionContext FeatureDecisionCon experimentDecisionContext := ExperimentDecisionContext{ Experiment: &experiment, ProjectConfig: decisionContext.ProjectConfig, + UserProfile: decisionContext.UserProfile, } experimentDecision, decisionReasons, err := f.compositeExperimentService.GetDecision(experimentDecisionContext, userContext, options) diff --git a/pkg/decision/persisting_experiment_service.go b/pkg/decision/persisting_experiment_service.go index b27f8414..72f7d589 100644 --- a/pkg/decision/persisting_experiment_service.go +++ b/pkg/decision/persisting_experiment_service.go @@ -52,6 +52,8 @@ func (p PersistingExperimentService) GetDecision(decisionContext ExperimentDecis return p.experimentBucketedService.GetDecision(decisionContext, userContext, options) } + isUserProfileNil := decisionContext.UserProfile == nil + var userProfile UserProfile var decisionReasons decide.DecisionReasons // check to see if there is a saved decision for the user @@ -66,7 +68,16 @@ func (p PersistingExperimentService) GetDecision(decisionContext ExperimentDecis if experimentDecision.Variation != nil { // save decision if a user profile service is provided userProfile.ID = userContext.ID - p.saveDecision(userProfile, decisionContext.Experiment, experimentDecision) + decisionKey := NewUserDecisionKey(decisionContext.Experiment.ID) + if isUserProfileNil { + p.saveDecision(userProfile, decisionKey, experimentDecision) + } else { + if decisionContext.UserProfile.ExperimentBucketMap == nil { + decisionContext.UserProfile.ExperimentBucketMap = make(map[UserDecisionKey]string) + } + decisionContext.UserProfile.ExperimentBucketMap[decisionKey] = experimentDecision.Variation.ID + decisionContext.UserProfile.HasUnsavedChange = true + } } return experimentDecision, reasons, err @@ -75,7 +86,12 @@ func (p PersistingExperimentService) GetDecision(decisionContext ExperimentDecis func (p PersistingExperimentService) getSavedDecision(decisionContext ExperimentDecisionContext, userContext entities.UserContext, options *decide.Options) (ExperimentDecision, UserProfile, decide.DecisionReasons) { reasons := decide.NewDecisionReasons(options) experimentDecision := ExperimentDecision{} - userProfile := p.userProfileService.Lookup(userContext.ID) + var userProfile UserProfile + if decisionContext.UserProfile == nil { + userProfile = p.userProfileService.Lookup(userContext.ID) + } else { + userProfile = *decisionContext.UserProfile + } // look up experiment decision from user profile decisionKey := NewUserDecisionKey(decisionContext.Experiment.ID) @@ -97,9 +113,8 @@ func (p PersistingExperimentService) getSavedDecision(decisionContext Experiment return experimentDecision, userProfile, reasons } -func (p PersistingExperimentService) saveDecision(userProfile UserProfile, experiment *entities.Experiment, decision ExperimentDecision) { +func (p PersistingExperimentService) saveDecision(userProfile UserProfile, decisionKey UserDecisionKey, decision ExperimentDecision) { if p.userProfileService != nil { - decisionKey := NewUserDecisionKey(experiment.ID) if userProfile.ExperimentBucketMap == nil { userProfile.ExperimentBucketMap = map[UserDecisionKey]string{} }