diff --git a/.github/workflows/csharp.yml b/.github/workflows/csharp.yml index 4303e0e2..25a3fb48 100644 --- a/.github/workflows/csharp.yml +++ b/.github/workflows/csharp.yml @@ -7,9 +7,20 @@ on: branches: [ master ] jobs: + failOnDraftPullRequest: + name: Fail If Draft Pull Request + if: github.event.pull_request.draft == true + runs-on: ubuntu-latest + steps: + - name: Fails Draft pull request. + run: | + echo "Draft pull requests should not trigger CI. Please mark the pull request as Ready for review to continue." + exit 1 + lintCodebase: + name: Lint Codebase + needs: [ failOnDraftPullRequest ] runs-on: ubuntu-latest - name: Lint Codebase steps: - name: Checkout code uses: actions/checkout@v3 diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index 633847ae..94c47a38 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -700,7 +700,7 @@ public void TestGetVariationForFeatureExperimentGivenNonMutexGroupAndUserNotBuck DecisionServiceMock. Setup(ds => ds.GetVariation(multiVariateExp, OptimizelyUserContextMock.Object, - ProjectConfig, null)). + ProjectConfig, It.IsAny(), It.IsAny())). Returns(null); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("multi_variate_feature"); @@ -736,7 +736,7 @@ public void TestGetVariationForFeatureExperimentGivenNonMutexGroupAndUserIsBucke DecisionServiceMock.Setup(ds => ds.GetVariation( ProjectConfig.GetExperimentFromKey("test_experiment_multivariate"), OptimizelyUserContextMock.Object, ProjectConfig, - It.IsAny())). + It.IsAny(), It.IsAny())). Returns(variation); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("multi_variate_feature"); @@ -771,7 +771,7 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserIsBucketed DecisionServiceMock. Setup(ds => ds.GetVariation(ProjectConfig.GetExperimentFromKey("group_experiment_1"), - OptimizelyUserContextMock.Object, ProjectConfig)). + OptimizelyUserContextMock.Object, ProjectConfig, It.IsAny(), It.IsAny())). Returns(variation); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("boolean_feature"); @@ -795,7 +795,7 @@ public void TestGetVariationForFeatureExperimentGivenMutexGroupAndUserNotBuckete DecisionServiceMock. Setup(ds => ds.GetVariation(It.IsAny(), It.IsAny(), ProjectConfig, - It.IsAny())). + It.IsAny(), It.IsAny())). Returns(Result.NullResult(null)); var featureFlag = ProjectConfig.GetFeatureFlagFromKey("boolean_feature"); @@ -1311,7 +1311,7 @@ public void TestGetVariationForFeatureWhenTheUserIsBuckedtedInBothExperimentAndR DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, OptimizelyUserContextMock.Object, ProjectConfig, - It.IsAny())). + It.IsAny(), It.IsAny())). Returns(variation); var actualDecision = DecisionServiceMock.Object.GetVariationForFeatureExperiment( featureFlag, OptimizelyUserContextMock.Object, userAttributes, ProjectConfig, diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index 5dab3aec..f2d85fe9 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017-2023, Optimizely + * Copyright 2017-2024, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -1394,19 +1394,19 @@ public void TestForcedVariationPreceedsUserProfile() LoggerMock.Verify( l => l.Log(LogLevel.INFO, "No previously activated variation of experiment \"etag1\" for user \"testUser3\" found in user profile."), - Times.Exactly(2)); + Times.Once); LoggerMock.Verify( l => l.Log(LogLevel.DEBUG, "Assigned bucket [4969] to user [testUser3] with bucketing ID [testUser3]."), - Times.Exactly(2)); + Times.Once); LoggerMock.Verify( l => l.Log(LogLevel.INFO, "User [testUser3] is in variation [vtag2] of experiment [etag1]."), - Times.Exactly(2)); + Times.Once); LoggerMock.Verify( l => l.Log(LogLevel.INFO, "Saved variation \"277\" of experiment \"223\" for user \"testUser3\"."), - Times.Exactly(2)); + Times.Once); LoggerMock.Verify( l => l.Log(LogLevel.DEBUG, "Set variation \"276\" for experiment \"223\" and user \"testUser3\" in the forced variation map."), @@ -3395,8 +3395,9 @@ public void TestActivateListener(UserAttributes userAttributes) mockUserContext.Setup(ouc => ouc.GetUserId()).Returns(TestUserId); DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, - It.IsAny(), It.IsAny())) - .Returns(variation); + It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())). + Returns(variation); DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, It.IsAny(), It.IsAny())) .Returns(decision); @@ -3509,9 +3510,10 @@ public void TestTrackListener(UserAttributes userAttributes, EventTags eventTags ErrorHandlerMock.Object, LoggerMock.Object); mockUserContext.Setup(ouc => ouc.GetUserId()).Returns(TestUserId); - DecisionServiceMock - .Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config)) - .Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, + It.IsAny(), Config, It.IsAny(), + It.IsAny())). + Returns(variation); // Adding notification listeners. var notificationType = NotificationCenter.NotificationType.Track; @@ -3565,9 +3567,10 @@ public void TestActivateSendsDecisionNotificationWithActualVariationKey() It.IsAny(), It.IsAny(), It.IsAny>())); - DecisionServiceMock - .Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config)) - .Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, + It.IsAny(), Config, It.IsAny(), + It.IsAny())). + Returns(variation); var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("ProjectConfigManager", ConfigManager); @@ -3622,9 +3625,10 @@ public void It.IsAny(), It.IsAny(), It.IsAny>())); - DecisionServiceMock - .Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config)) - .Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, + It.IsAny(), Config, It.IsAny(), + It.IsAny())). + Returns(variation); var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("ProjectConfigManager", ConfigManager); @@ -3666,8 +3670,9 @@ public void TestActivateSendsDecisionNotificationWithNullVariationKey() DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, It.IsAny(), - It.IsAny(), null)) - .Returns(Result.NullResult(null)); + It.IsAny(), It.IsAny(), + It.IsAny())). + Returns(Result.NullResult(null)); optStronglyTyped.NotificationCenter.AddNotification( NotificationCenter.NotificationType.Decision, @@ -3727,9 +3732,10 @@ public void TestGetVariationSendsDecisionNotificationWithActualVariationKey() ErrorHandlerMock.Object, LoggerMock.Object); mockUserContext.Setup(ouc => ouc.GetUserId()).Returns(TestUserId); - DecisionServiceMock - .Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config)) - .Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, + It.IsAny(), Config, It.IsAny(), + It.IsAny())). + Returns(variation); optStronglyTyped.NotificationCenter.AddNotification( NotificationCenter.NotificationType.Decision, @@ -3788,9 +3794,10 @@ public void ErrorHandlerMock.Object, LoggerMock.Object); mockUserContext.Setup(ouc => ouc.GetUserId()).Returns(TestUserId); - DecisionServiceMock - .Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config)) - .Returns(variation); + DecisionServiceMock. + Setup(ds => ds.GetVariation(experiment, It.IsAny(), Config + , It.IsAny(), It.IsAny())). + Returns(variation); optStronglyTyped.NotificationCenter.AddNotification( NotificationCenter.NotificationType.Decision, @@ -3828,8 +3835,9 @@ public void TestGetVariationSendsDecisionNotificationWithNullVariationKey() It.IsAny(), It.IsAny>())); DecisionServiceMock.Setup(ds => ds.GetVariation(It.IsAny(), - It.IsAny(), It.IsAny())) - .Returns(Result.NullResult(null)); + It.IsAny(), It.IsAny() + , It.IsAny(), It.IsAny())). + Returns(Result.NullResult(null)); //DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, TestUserId, Config, null)).Returns(Result.NullResult(null)); optStronglyTyped.NotificationCenter.AddNotification( @@ -3875,8 +3883,9 @@ public void It.IsAny(), It.IsAny>())); DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, - It.IsAny(), ConfigManager.GetConfig(), null)) - .Returns(variation); + It.IsAny(), ConfigManager.GetConfig() + , It.IsAny(), It.IsAny())). + Returns(variation); var optly = Helper.CreatePrivateOptimizely(); var optStronglyTyped = optly.GetObject() as Optimizely; @@ -3936,8 +3945,9 @@ public void It.IsAny(), It.IsAny>())); DecisionServiceMock.Setup(ds => - ds.GetVariation(experiment, It.IsAny(), Config, null)) - .Returns(variation); + ds.GetVariation(experiment, It.IsAny(), Config + , It.IsAny(), It.IsAny())). + Returns(variation); var optly = Helper.CreatePrivateOptimizely(); var optStronglyTyped = optly.GetObject() as Optimizely; @@ -5322,7 +5332,8 @@ public void TestGetAllFeatureVariablesReturnsNullScenarios() LoggerMock.Verify( log => log.Log(LogLevel.ERROR, - "Optimizely instance is not valid, failing getAllFeatureVariableValues call. type"), + "Optimizely instance is not valid, failing getAllFeatureVariableValues call. type") + , Times.Once); } diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs index 49fd91a4..c4558839 100644 --- a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs +++ b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs @@ -1,5 +1,5 @@ /* - * Copyright 2020-2021, 2022-2023 Optimizely and contributors + * Copyright 2020-2024 Optimizely and contributors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,7 +16,7 @@ using System; using System.Collections.Generic; -using System.Threading; +using System.Linq; using Castle.Core.Internal; using Moq; using NUnit.Framework; @@ -62,6 +62,22 @@ public void SetUp() LoggerMock.Object, ErrorHandlerMock.Object); } + private Mock MakeUserProfileServiceMock() + { + var projectConfig = DatafileProjectConfig.Create(TestData.Datafile, LoggerMock.Object, + ErrorHandlerMock.Object); + var experiment = projectConfig.Experiments[8]; + var variation = experiment.Variations[0]; + var decision = new Decision(variation.Id); + var userProfile = new UserProfile(UserID, new Dictionary + { + { experiment.Id, decision }, + }); + var userProfileServiceMock = new Mock(); + userProfileServiceMock.Setup(up => up.Lookup(UserID)).Returns(userProfile.ToMap()); + return userProfileServiceMock; + } + [Test] public void OptimizelyUserContextWithAttributes() { @@ -193,7 +209,7 @@ public void SetAttributeToOverrideAttribute() Assert.AreEqual(user.GetAttributes()["k1"], true); } - #region decide + #region Decide [Test] public void TestDecide() @@ -409,9 +425,117 @@ public void DecideWhenConfigIsNull() Assert.IsTrue(TestData.CompareObjects(decision, decisionExpected)); } - #endregion decide + [Test] + public void SeparateDecideShouldHaveSameNumberOfUpsSaveOnlyOneLookup() + { + var experimentFlagKeys = new[] { "double_single_variable_feature", "integer_single_variable_feature" }; + // var rolloutFlagKey = "boolean_single_variable_feature"; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfileExperiment1 = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + var expectedUserProfileExperiment2 = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + { "122241", new Decision("122242") }, + }); + // var expectedUserProfileRollout = new UserProfile(UserID, new Dictionary + // { + // // expected decisions for rollout + // }); + + user.Decide(experimentFlagKeys[0]); + user.Decide(experimentFlagKeys[1]); + // user.Decide(rolloutFlagKey); // TODO: Test UPS with rollout? + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Exactly(2)); + Assert.AreEqual(saveArgsCollector[0], expectedUserProfileExperiment1.ToMap()); + Assert.AreEqual(saveArgsCollector[1], expectedUserProfileExperiment2.ToMap()); + // Assert.AreEqual(saveArgsCollector[2], expectedUserProfileRollout.ToMap()); + } + + [Test] + public void DecideWithUpsShouldOnlyLookupSaveOnce() + { + var flagKeyFromTestDataJson = "double_single_variable_feature"; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + + user.Decide(flagKeyFromTestDataJson); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } + + #endregion Decide - #region decideAll + #region DecideForKeys + + [Test] + public void DecideForKeysWithUpsShouldOnlyLookupSaveOnceWithMultipleFlags() + { + var flagKeys = new[] { "double_single_variable_feature", "boolean_feature" }; + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var userContext = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + }); + + userContext.DecideForKeys(flagKeys); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), + Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } [Test] public void DecideForKeysWithOneFlag() @@ -443,6 +567,42 @@ public void DecideForKeysWithOneFlag() Assert.IsTrue(TestData.CompareObjects(decision, expDecision)); } + #endregion DecideForKeys + + #region DecideAll + + [Test] + public void DecideAllWithUpsShouldOnlyLookupSaveOnce() + { + var userProfileServiceMock = MakeUserProfileServiceMock(); + var saveArgsCollector = new List>(); + userProfileServiceMock.Setup(up => up.Save(Capture.In(saveArgsCollector))); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, + LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var user = optimizely.CreateUserContext(UserID); + var expectedUserProfile = new UserProfile(UserID, new Dictionary + { + { "224", new Decision("280") }, + { "122238", new Decision("122240") }, + { "122241", new Decision("122242") }, + { "122235", new Decision("122236") }, + { "188880", new Decision("188881") }, + }); + + user.DecideAll(); + + LoggerMock.Verify( + l => l.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."), + Times.Never); + LoggerMock.Verify( + l => l.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."), + Times.Never); + userProfileServiceMock.Verify(l => l.Lookup(UserID), Times.Once); + userProfileServiceMock.Verify(l => l.Save(It.IsAny>()), Times.Once); + Assert.AreEqual(saveArgsCollector.First(), expectedUserProfile.ToMap()); + } + [Test] public void DecideAllTwoFlag() { diff --git a/OptimizelySDK.sln.DotSettings b/OptimizelySDK.sln.DotSettings index 3ccf7ffc..8ee6e5a4 100644 --- a/OptimizelySDK.sln.DotSettings +++ b/OptimizelySDK.sln.DotSettings @@ -43,10 +43,15 @@ <Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /> <Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /> <Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /> + <Policy><Descriptor Staticness="Static" AccessRightKinds="Private" Description="Static readonly fields (private)"><ElementKinds><Kind Name="READONLY_FIELD" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="aaBb" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Enum members"><ElementKinds><Kind Name="ENUM_MEMBER" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AaBb"><ExtraRule Prefix="" Suffix="" Style="AaBb" /></Policy></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Local constants"><ElementKinds><Kind Name="LOCAL_CONSTANT" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="" Suffix="" Style="AA_BB" /></Policy> + <Policy><Descriptor Staticness="Any" AccessRightKinds="Any" Description="Interfaces"><ElementKinds><Kind Name="INTERFACE" /></ElementKinds></Descriptor><Policy Inspect="True" Prefix="I" Suffix="" Style="AaBb" /></Policy> True True True True + True True True True diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index e6088d7e..60830f3f 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -1,5 +1,5 @@ /* -* Copyright 2017-2022, Optimizely +* Copyright 2017-2022, 2024 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -43,6 +43,8 @@ public class DecisionService private IErrorHandler ErrorHandler; private UserProfileService UserProfileService; private ILogger Logger; + private readonly DecisionUnitOfWork _decisionUnitOfWork; + private readonly UserProfileCache _userProfileCache; /// /// Associative array of user IDs to an associative array @@ -60,10 +62,10 @@ public class DecisionService /// /// Initialize a decision service for the Optimizely client. /// - /// Base bucketer to allocate new users to an experiment. - /// The error handler of the Optimizely client. - /// - /// < param name= "logger" > UserProfileService implementation for storing user info. + /// Base bucketer to allocate new users to an experiment. + /// The error handler of the Optimizely client. + /// The injected implementation providing control over the bucketing. + /// < param name="logger"> UserProfileService implementation for storing user info. public DecisionService(Bucketer bucketer, IErrorHandler errorHandler, UserProfileService userProfileService, ILogger logger ) @@ -72,6 +74,8 @@ public DecisionService(Bucketer bucketer, IErrorHandler errorHandler, ErrorHandler = errorHandler; UserProfileService = userProfileService; Logger = logger; + _decisionUnitOfWork = new DecisionUnitOfWork(); + _userProfileCache = new UserProfileCache(userProfileService, logger); #if NET35 ForcedVariationMap = new Dictionary>(); #else @@ -84,32 +88,23 @@ public DecisionService(Bucketer bucketer, IErrorHandler errorHandler, /// /// Get a Variation of an Experiment for a user to be allocated into. /// - /// The Experiment the user will be bucketed into. - /// Optimizely user context. - /// Project config. - /// The Variation the user is allocated into. - public virtual Result GetVariation(Experiment experiment, - OptimizelyUserContext user, - ProjectConfig config - ) - { - return GetVariation(experiment, user, config, new OptimizelyDecideOption[] { }); - } - - /// - /// Get a Variation of an Experiment for a user to be allocated into. - /// - /// The Experiment the user will be bucketed into. - /// optimizely user context. - /// Project Config. - /// An array of decision options. + /// The Experiment the user will be bucketed into. + /// optimizely user context. + /// Project Config. + /// An array of decision options. + /// Whether to force a lookup of the user profile when UPS is enabled. /// The Variation the user is allocated into. public virtual Result GetVariation(Experiment experiment, OptimizelyUserContext user, ProjectConfig config, - OptimizelyDecideOption[] options + OptimizelyDecideOption[] options = null, + bool forceUserProfileLookup = false ) { + if (options == null) + { + options = new OptimizelyDecideOption[] { }; + } var reasons = new DecisionReasons(); var userId = user.GetUserId(); if (!ExperimentUtils.IsExperimentActive(experiment, Logger)) @@ -145,29 +140,12 @@ OptimizelyDecideOption[] options { try { - var userProfileMap = UserProfileService.Lookup(user.GetUserId()); - if (userProfileMap != null && - UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + userProfile = _userProfileCache.GetUserProfile(userId, forceUserProfileLookup); + decisionVariationResult = GetStoredVariation(experiment, userProfile, config); + reasons += decisionVariationResult.DecisionReasons; + if (decisionVariationResult.ResultObject != null) { - userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); - decisionVariationResult = - GetStoredVariation(experiment, userProfile, config); - reasons += decisionVariationResult.DecisionReasons; - if (decisionVariationResult.ResultObject != null) - { - return decisionVariationResult.SetReasons(reasons); - } - } - else if (userProfileMap == null) - { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - "We were unable to get a user profile map from the UserProfileService.")); - } - else - { - Logger.Log(LogLevel.ERROR, - reasons.AddInfo("The UserProfileService returned an invalid map.")); + return decisionVariationResult.SetReasons(reasons); } } catch (Exception exception) @@ -454,9 +432,9 @@ ProjectConfig config /// /// Save a { @link Variation } of an { @link Experiment } for a user in the {@link UserProfileService}. /// - /// The experiment the user was buck - /// The Variation to save. - /// instance of the user information. + /// The experiment the user was buck + /// The Variation to save. + /// Instance of the user information. public void SaveVariation(Experiment experiment, Variation variation, UserProfile userProfile ) @@ -467,32 +445,63 @@ UserProfile userProfile return; } - Decision decision; - if (userProfile.ExperimentBucketMap.ContainsKey(experiment.Id)) + var decision = new Decision(variation.Id); + _decisionUnitOfWork.AddDecision(userProfile.UserId, experiment.Id, decision); + + var cachedProfile = _userProfileCache.GetUserProfile(userProfile.UserId); + cachedProfile.ExperimentBucketMap[experiment.Id] = decision; + } + + public void AddDecisionToUnitOfWork(string userId, string experimentId, Decision decision) + { + if (UserProfileService == null) { - decision = userProfile.ExperimentBucketMap[experiment.Id]; - decision.VariationId = variation.Id; + return; } - else + + if (!string.IsNullOrEmpty(experimentId)) { - decision = new Decision(variation.Id); + _decisionUnitOfWork.AddDecision(userId, experimentId, decision); } + } - userProfile.ExperimentBucketMap[experiment.Id] = decision; - - try + /// + /// Commits the decisions to the user profile service. + /// + public void CommitDecisionsToUserProfileService() + { + if (UserProfileService == null || !_decisionUnitOfWork.HasDecisions) { - UserProfileService.Save(userProfile.ToMap()); - Logger.Log(LogLevel.INFO, - $"Saved variation \"{variation.Id}\" of experiment \"{experiment.Id}\" for user \"{userProfile.UserId}\"."); + return; } - catch (Exception exception) + + var decisions = _decisionUnitOfWork.GetDecisions(); + foreach (var userDecisions in decisions) { - Logger.Log(LogLevel.ERROR, - $"Failed to save variation \"{variation.Id}\" of experiment \"{experiment.Id}\" for user \"{userProfile.UserId}\"."); - ErrorHandler.HandleError( - new Exceptions.OptimizelyRuntimeException(exception.Message)); + var userId = userDecisions.Key; + var experimentDecisions = userDecisions.Value; + + var userProfile = _userProfileCache.GetUserProfile(userId); + foreach (var experimentDecision in experimentDecisions) + { + userProfile.ExperimentBucketMap[experimentDecision.Key] = + experimentDecision.Value; + } + + try + { + UserProfileService.Save(userProfile.ToMap()); + Logger.Log(LogLevel.INFO, $"Saved decisions for user \"{userId}\"."); + } + catch (Exception exception) + { + Logger.Log(LogLevel.ERROR, $"Failed to save decisions for user \"{userId}\"."); + ErrorHandler.HandleError( + new Exceptions.OptimizelyRuntimeException(exception.Message)); + } } + + _decisionUnitOfWork.ClearDecisions(); } /// diff --git a/OptimizelySDK/Bucketing/DecisionUnitOfWork.cs b/OptimizelySDK/Bucketing/DecisionUnitOfWork.cs new file mode 100644 index 00000000..2c4aee7e --- /dev/null +++ b/OptimizelySDK/Bucketing/DecisionUnitOfWork.cs @@ -0,0 +1,53 @@ +/* + * Copyright 2024 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; + +namespace OptimizelySDK.Bucketing +{ + public class DecisionUnitOfWork + { + private readonly Dictionary> _decisions = + new Dictionary>(); + + public void AddDecision(string userId, string experimentId, Decision decision) + { + if (string.IsNullOrEmpty(userId) || string.IsNullOrEmpty(experimentId)) + { + return; + } + + if (!_decisions.ContainsKey(userId)) + { + _decisions[userId] = new Dictionary(); + } + + _decisions[userId][experimentId] = decision; + } + + public Dictionary> GetDecisions() + { + return _decisions; + } + + public bool HasDecisions => _decisions.Count > 0; + + public void ClearDecisions() + { + _decisions.Clear(); + } + } +} diff --git a/OptimizelySDK/Bucketing/UserProfileCache.cs b/OptimizelySDK/Bucketing/UserProfileCache.cs new file mode 100644 index 00000000..d34833e2 --- /dev/null +++ b/OptimizelySDK/Bucketing/UserProfileCache.cs @@ -0,0 +1,64 @@ +/* + * Copyright 2024 Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using System.Collections.Generic; +using OptimizelySDK.Logger; + +namespace OptimizelySDK.Bucketing +{ + public class UserProfileCache + { + private readonly Dictionary _cache = + new Dictionary(); + + private readonly UserProfileService _userProfileService; + private readonly ILogger _logger; + + public UserProfileCache(UserProfileService userProfileService, ILogger logger) + { + _userProfileService = userProfileService; + _logger = logger; + } + + public UserProfile GetUserProfile(string userId, bool forceLookup = false) + { + if (_cache.TryGetValue(userId, out var userProfile) && !forceLookup) + { + return _cache[userId]; + } + + var userProfileMap = _userProfileService.Lookup(userId); + if (userProfileMap != null && UserProfileUtil.IsValidUserProfileMap(userProfileMap)) + { + userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap); + } + else if (userProfileMap == null) + { + _logger.Log(LogLevel.INFO, + "We were unable to get a user profile map from the UserProfileService."); + } + else + { + _logger.Log(LogLevel.ERROR, "The UserProfileService returned an invalid map."); + } + + _cache[userId] = userProfile ?? + new UserProfile(userId, new Dictionary()); + + return _cache[userId]; + } + } +} diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 9da300f8..19e2a417 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017-2023, Optimizely + * Copyright 2017-2024, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use file except in compliance with the License. @@ -310,7 +310,8 @@ public Variation Activate(string experimentKey, string userId, return null; } - var variation = GetVariation(experimentKey, userId, config, userAttributes); + var variation = GetVariation(experimentKey, userId, config, userAttributes, + forceUserProfileLookup: true); if (variation == null || variation.Key == null) { @@ -404,7 +405,8 @@ public Variation GetVariation(string experimentKey, string userId, ) { var config = ProjectConfigManager?.GetConfig(); - return GetVariation(experimentKey, userId, config, userAttributes); + return GetVariation(experimentKey, userId, config, userAttributes, + forceUserProfileLookup: true); } /// @@ -414,9 +416,10 @@ public Variation GetVariation(string experimentKey, string userId, /// ID for the user /// ProjectConfig to be used for variation /// Attributes for the users + /// Whether to force a lookup of the user profile when UPS is enabled. /// null|Variation Representing variation private Variation GetVariation(string experimentKey, string userId, ProjectConfig config, - UserAttributes userAttributes = null + UserAttributes userAttributes = null, bool forceUserProfileLookup = false ) { if (config == null) @@ -442,8 +445,10 @@ private Variation GetVariation(string experimentKey, string userId, ProjectConfi userAttributes = userAttributes ?? new UserAttributes(); var userContext = CreateUserContextCopy(userId, userAttributes); - var variation = DecisionService.GetVariation(experiment, userContext, config) - ?.ResultObject; + var variation = DecisionService. + GetVariation(experiment, userContext, config, + forceUserProfileLookup: forceUserProfileLookup)?. + ResultObject; var decisionInfo = new Dictionary { { "experimentKey", experimentKey }, { "variationKey", variation?.Key }, @@ -861,12 +866,15 @@ private OptimizelyUserContext CreateUserContextCopy(string userId, ///
  • If the SDK finds an error, it’ll return a decision with null for variationKey. The decision will include an error message in reasons. /// ///
    • + /// User context. /// A flag key for which a decision will be made. /// A list of options for decision-making. + /// Whether to commit decisions to the user profile service (if in use) immediately. /// A decision result. internal OptimizelyDecision Decide(OptimizelyUserContext user, string key, - OptimizelyDecideOption[] options + OptimizelyDecideOption[] options, + bool commitImmediately = true ) { var config = ProjectConfigManager?.GetConfig(); @@ -924,6 +932,12 @@ OptimizelyDecideOption[] options decision = flagDecisionResult.ResultObject; } + if (!options.Contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE)) + { + DecisionService.AddDecisionToUnitOfWork(userId, decision.Experiment?.Id, + new Decision(decision.Variation?.Id)); + } + var featureEnabled = false; if (decision?.Variation != null) @@ -1004,7 +1018,7 @@ OptimizelyDecideOption[] options DecisionNotificationTypes.FLAG, userId, userAttributes ?? new UserAttributes(), decisionInfo); - return new OptimizelyDecision( + var result = new OptimizelyDecision( variationKey, featureEnabled, optimizelyJSON, @@ -1012,6 +1026,13 @@ OptimizelyDecideOption[] options key, user, reasonsToReport); + + if (commitImmediately) + { + DecisionService.CommitDecisionsToUserProfileService(); + } + + return result; } internal Dictionary DecideAll(OptimizelyUserContext user, @@ -1058,7 +1079,7 @@ OptimizelyDecideOption[] options foreach (var key in keys) { - var decision = Decide(user, key, options); + var decision = Decide(user, key, options, false); if (!allOptions.Contains(OptimizelyDecideOption.ENABLED_FLAGS_ONLY) || decision.Enabled) { @@ -1066,6 +1087,8 @@ OptimizelyDecideOption[] options } } + DecisionService.CommitDecisionsToUserProfileService(); + return decisionDictionary; } diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 1812a2ad..024eb3f7 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -75,6 +75,8 @@ + +