feat: WIP better way to handle loading & saving for UPS

mikechu-optimizely · mikechu-optimizely · commit 9f2f471f2458 · 2024-10-17T14:05:08.000-04:00
I need more time to fix tests
diff --git a/OptimizelySDK.Tests/OptimizelyUserContextTest.cs b/OptimizelySDK.Tests/OptimizelyUserContextTest.cs
@@ -540,6 +540,7 @@ public void DecideAllWithUpsShouldOnlyLookupSaveOnce()
                 { "122238", new Decision("122240") },
                 { "122241", new Decision("122242") },
                 { "122235", new Decision("122236") },
+                { "188880", new Decision("188881") },
             });
 
             _ = user.DecideAll();
diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs
@@ -43,23 +43,8 @@ public class DecisionService
         private IErrorHandler ErrorHandler;
         private UserProfileService UserProfileService;
         private ILogger Logger;
-        private UserProfile _userProfile;
-
-        private bool _decisionBatchInProgress;
-
-        public bool DecisionBatchInProgress
-        {
-            set
-            {
-                // Only save if the value is changing from true to false
-                if (_decisionBatchInProgress && !value)
-                {
-                    SaveToUserProfileService();
-                    _userProfile = null;
-                }
-                _decisionBatchInProgress = value;
-            }
-        }
+        private readonly DecisionUnitOfWork _decisionUnitOfWork;
+        private readonly UserProfileCache _userProfileCache;
 
         /// <summary>
         /// Associative array of user IDs to an associative array
@@ -89,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<string, Dictionary<string, string>>();
 #else
@@ -157,41 +144,17 @@ OptimizelyDecideOption[] options
             var ignoreUPS = Array.Exists(options,
                 option => option == OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE);
 
-
+            UserProfile userProfile = null;
             if (!ignoreUPS && UserProfileService != null)
             {
                 try
                 {
-                    if (_userProfile == null)
-                    {
-                        var userProfileMap = UserProfileService.Lookup(user.GetUserId());
-                        if (userProfileMap != null &&
-                            UserProfileUtil.IsValidUserProfileMap(userProfileMap))
-                        {
-                            _userProfile = UserProfileUtil.ConvertMapToUserProfile(userProfileMap);
-                        }
-                        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."));
-                        }
-                    }
-
-                    if (_userProfile != null)
+                    userProfile = _userProfileCache.GetUserProfile(userId);
+                    decisionVariationResult = GetStoredVariation(experiment, userProfile, config);
+                    reasons += decisionVariationResult.DecisionReasons;
+                    if (decisionVariationResult.ResultObject != null)
                     {
-                        decisionVariationResult =
-                            GetStoredVariation(experiment, _userProfile, config);
-                        reasons += decisionVariationResult.DecisionReasons;
-                        if (decisionVariationResult.ResultObject != null)
-                        {
-                            return decisionVariationResult.SetReasons(reasons);
-                        }
+                        return decisionVariationResult.SetReasons(reasons);
                     }
                 }
                 catch (Exception exception)
@@ -221,7 +184,7 @@ OptimizelyDecideOption[] options
                 {
                     if (UserProfileService != null && !ignoreUPS)
                     {
-                        var bucketerUserProfile = _userProfile ??
+                        var bucketerUserProfile = userProfile ??
                                                   new UserProfile(userId,
                                                       new Dictionary<string, Decision>());
                         SaveVariation(experiment, decisionVariationResult.ResultObject,
@@ -491,60 +454,55 @@ UserProfile userProfile
                 return;
             }
 
-            if (_userProfile == null)
-            {
-                _userProfile = userProfile;
-            }
+            var decision = new Decision(variation.Id);
+            _decisionUnitOfWork.AddDecision(userProfile.UserId, experiment.Id, decision);
 
-            Decision decision;
-            if (_userProfile.ExperimentBucketMap.ContainsKey(experiment.Id))
-            {
-                decision = _userProfile.ExperimentBucketMap[experiment.Id];
-                decision.VariationId = variation.Id;
-            }
-            else
-            {
-                decision = new Decision(variation.Id);
-            }
-
-            _userProfile.ExperimentBucketMap[experiment.Id] = decision;
+            var cachedProfile = _userProfileCache.GetUserProfile(userProfile.UserId);
+            cachedProfile.ExperimentBucketMap[experiment.Id] = decision;
+        }
+        
+        public void AddDecisionToUnitOfWork(string userId, string experimentId, Decision decision)
+        {
+            _decisionUnitOfWork.AddDecision(userId, experimentId, decision);
+        }
 
-            if (!_decisionBatchInProgress)
+        /// <summary>
+        /// Commits the decisions to the user profile service.
+        /// </summary>
+        public void CommitDecisionsToUserProfileService()
+        {
+            if (UserProfileService == null || !_decisionUnitOfWork.HasDecisions)
             {
-                SaveToUserProfileService(experiment, variation);
+                return;
             }
-        }
 
-        private void SaveToUserProfileService(Experiment experiment = null,
-            Variation variation = null
-        )
-        {
-            var hasExperimentDetails = experiment != null && variation != null &&
-                                       !string.IsNullOrEmpty(_userProfile.UserId);
-            try
+            var decisions = _decisionUnitOfWork.GetDecisions();
+            foreach (var userDecisions in decisions)
             {
-                if (_userProfile == null)
+                var userId = userDecisions.Key;
+                var experimentDecisions = userDecisions.Value;
+
+                var userProfile = _userProfileCache.GetUserProfile(userId);
+                foreach (var experimentDecision in experimentDecisions)
                 {
-                    return;
+                    userProfile.ExperimentBucketMap[experimentDecision.Key] =
+                        experimentDecision.Value;
                 }
 
-                UserProfileService.Save(_userProfile.ToMap());
-
-                Logger.Log(LogLevel.INFO,
-                    hasExperimentDetails ?
-                        $"Saved variation \"{variation.Id}\" of experiment \"{experiment.Id}\" for user \"{_userProfile.UserId}\"." :
-                        "Saved user profile after batch decision.");
+                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));
+                }
             }
-            catch (Exception exception)
-            {
-                Logger.Log(LogLevel.ERROR,
-                    hasExperimentDetails ?
-                        $"Failed to save variation \"{variation.Id}\" of experiment \"{experiment.Id}\" for user \"{_userProfile.UserId}\"." :
-                        "Failed to save user profile after batch decision.");
 
-                ErrorHandler.HandleError(
-                    new Exceptions.OptimizelyRuntimeException(exception.Message));
-            }
+            _decisionUnitOfWork.ClearDecisions();
         }
 
         /// <summary>
diff --git a/OptimizelySDK/Bucketing/DecisionUnitOfWork.cs b/OptimizelySDK/Bucketing/DecisionUnitOfWork.cs
@@ -0,0 +1,48 @@
+﻿/*
+ * 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<string, Dictionary<string, Decision>> _decisions =
+            new Dictionary<string, Dictionary<string, Decision>>();
+
+        public void AddDecision(string userId, string experimentId, Decision decision)
+        {
+            if (!_decisions.ContainsKey(userId))
+            {
+                _decisions[userId] = new Dictionary<string, Decision>();
+            }
+
+            _decisions[userId][experimentId] = decision;
+        }
+
+        public Dictionary<string, Dictionary<string, Decision>> 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
@@ -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<string, UserProfile> _cache =
+            new Dictionary<string, UserProfile>();
+
+        private readonly UserProfileService _userProfileService;
+        private readonly ILogger _logger;
+
+        public UserProfileCache(UserProfileService userProfileService, ILogger logger)
+        {
+            _userProfileService = userProfileService;
+            _logger = logger;
+        }
+
+        public UserProfile GetUserProfile(string userId)
+        {
+            if (_cache.TryGetValue(userId, out var userProfile))
+            {
+                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<string, Decision>());
+
+            return _cache[userId];
+        }
+    }
+}
diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs
@@ -861,12 +861,15 @@ private OptimizelyUserContext CreateUserContextCopy(string userId,
         /// <li>If the SDK finds an error, it’ll return a decision with <b>null</b> for <b>variationKey</b>. The decision will include an error message in <b>reasons</b>.
         /// </ul>
         /// </summary>
+        /// <param name="user">User context.</param>
         /// <param name="key">A flag key for which a decision will be made.</param>
         /// <param name="options">A list of options for decision-making.</param>
+        /// <param name="commitImmediately">Whether to commit decisions to the user profile service (if in use) immediately.</param>
         /// <returns>A decision result.</returns>
         internal OptimizelyDecision Decide(OptimizelyUserContext user,
             string key,
-            OptimizelyDecideOption[] options
+            OptimizelyDecideOption[] options,
+            bool commitImmediately = true
         )
         {
             var config = ProjectConfigManager?.GetConfig();
@@ -929,6 +932,8 @@ OptimizelyDecideOption[] options
             if (decision?.Variation != null)
             {
                 featureEnabled = decision.Variation.FeatureEnabled.GetValueOrDefault();
+                DecisionService.AddDecisionToUnitOfWork(userId, decision.Experiment?.Id,
+                    new Decision(decision.Variation.Id));
             }
 
             if (featureEnabled)
@@ -1004,14 +1009,21 @@ OptimizelyDecideOption[] options
                 DecisionNotificationTypes.FLAG, userId,
                 userAttributes ?? new UserAttributes(), decisionInfo);
 
-            return new OptimizelyDecision(
+            var result = new OptimizelyDecision(
                 variationKey,
                 featureEnabled,
                 optimizelyJSON,
                 ruleKey,
                 key,
                 user,
                 reasonsToReport);
+
+            if (commitImmediately)
+            {
+                DecisionService.CommitDecisionsToUserProfileService();
+            }
+
+            return result;
         }
 
         internal Dictionary<string, OptimizelyDecision> DecideAll(OptimizelyUserContext user,
@@ -1056,18 +1068,17 @@ OptimizelyDecideOption[] options
 
             var allOptions = GetAllOptions(options);
 
-            DecisionService.DecisionBatchInProgress = true;
             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)
                 {
                     decisionDictionary.Add(key, decision);
                 }
             }
-
-            DecisionService.DecisionBatchInProgress = false;
+            
+            DecisionService.CommitDecisionsToUserProfileService();
 
             return decisionDictionary;
         }
diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj
@@ -75,6 +75,8 @@
     <Compile Include="AudienceConditions\OrCondition.cs"/>
     <Compile Include="Bucketing\Bucketer.cs"/>
     <Compile Include="Bucketing\Decision.cs"/>
+    <Compile Include="Bucketing\DecisionUnitOfWork.cs" />
+    <Compile Include="Bucketing\UserProfileCache.cs" />
     <Compile Include="ClientConfigHandler.cs"/>
     <Compile Include="Entity\Attribute.cs"/>
     <Compile Include="Entity\Audience.cs"/>
