[FSSDK-11177] review update 1

Author: junaed-optimizely

OptimizelySDK.Tests/CmabTests/DecisionServiceCmabTest.cs

@@ -138,8 +138,8 @@ public void TestGetVariationWithCmabExperimentZeroTrafficAllocation()
             var result = _decisionService.GetVariation(experiment, userContext, mockConfig.Object);
 
             Assert.IsNotNull(result);
-            Assert.IsNull(result.ResultObject, "No variation should be returned with 0 traffic");
-            Assert.IsNull(result.ResultObject?.CmabUuid);
+            Assert.IsNull(result.ResultObject.Variation, "No variation should be returned with 0 traffic");
+            Assert.IsNull(result.ResultObject.CmabUuid);
 
             var reasons = result.DecisionReasons.ToReport(true);
             var expectedMessage =
@@ -183,8 +183,8 @@ public void TestGetVariationWithCmabExperimentServiceError()
             var result = _decisionService.GetVariation(experiment, userContext, mockConfig.Object);
 
             Assert.IsNotNull(result);
-            Assert.IsNull(result.ResultObject, "Should return null on error");
-            // CmabUuid is now in VariationDecisionResult, not DecisionReasons
+            Assert.IsNull(result.ResultObject.Variation, "Should return null on error");
+            Assert.IsTrue(result.ResultObject.CmabError);
 
             var reasonsList = result.DecisionReasons.ToReport(true);
             Assert.IsTrue(reasonsList.Exists(reason =>
@@ -232,8 +232,7 @@ public void TestGetVariationWithCmabExperimentUnknownVariationId()
             var result = _decisionService.GetVariation(experiment, userContext, mockConfig.Object);
 
             Assert.IsNotNull(result);
-            Assert.IsNull(result.ResultObject);
-            // CmabUuid is now in VariationDecisionResult, not DecisionReasons
+            Assert.IsNull(result.ResultObject.Variation, "Should return null on error");
 
             var reasons = result.DecisionReasons.ToReport(true);
             var expectedMessage =

OptimizelySDK.Tests/OptimizelyFactoryTest.cs

@@ -271,7 +271,7 @@ public void SetCmabCacheConfigStoresCacheSizeAndTtl()
             Assert.IsNotNull(config);
             Assert.AreEqual(cacheSize, config.CacheSize);
             Assert.AreEqual(cacheTtl, config.CacheTtl);
-            Assert.IsNull(config.CustomCache);
+            Assert.IsNull(config.Cache);
         }
 
         [Test]
@@ -284,7 +284,7 @@ public void SetCmabCustomCacheStoresCustomCacheInstance()
             var config = GetCurrentCmabConfiguration();
 
             Assert.IsNotNull(config);
-            Assert.AreSame(customCache, config.CustomCache);
+            Assert.AreSame(customCache, config.Cache);
             Assert.IsNull(config.CacheSize);
             Assert.IsNull(config.CacheTtl);
         }

OptimizelySDK/Bucketing/DecisionService.cs

@@ -227,34 +227,14 @@ public virtual Result<VariationDecisionResult> GetVariation(Experiment experimen
                 var bucketingId = GetBucketingId(userId, user.GetAttributes()).ResultObject;
 
 #if USE_CMAB
-                // Check if this is a CMAB experiment
                 if (experiment.Cmab != null)
                 {
                     var cmabDecisionResult =
                         GetDecisionForCmabExperiment(experiment, user, config, bucketingId, options);
                     reasons += cmabDecisionResult.DecisionReasons;
 
-                    var cmabResult = cmabDecisionResult.ResultObject;
-                    if (cmabResult != null)
-                    {
-                        variation = cmabResult.Variation;
-
-                        // For CMAB experiments, we don't save to user profile
-                        // Return VariationDecisionResult with CMAB UUID
-                        if (variation != null)
-                        {
-                            return Result<VariationDecisionResult>.NewResult(cmabResult, reasons);
-                        }
-
-                        // If cmabResult.CmabError is true, it means there was an error fetching
-                        // Return null variation but log that it was an error, not just no bucketing
-                        if (cmabResult.CmabError)
-                        {
-                            return Result<VariationDecisionResult>.NullResult(reasons);
-                        }
-                    }
-
-                    return Result<VariationDecisionResult>.NullResult(reasons);
+                    return Result<VariationDecisionResult>.NewResult(
+                        cmabDecisionResult.ResultObject, reasons);
                 }
 #endif
 
@@ -312,7 +292,7 @@ OptimizelyDecideOption[] options
             var reasons = new DecisionReasons();
             var userId = user.GetUserId();
 
-            // Create dummy traffic allocation for CMAB
+            // dummy traffic allocation for CMAB
             var cmabTrafficAllocation = new List<TrafficAllocation>
             {
                 new TrafficAllocation
@@ -322,7 +302,6 @@ OptimizelyDecideOption[] options
                 },
             };
 
-            // Check if user is in CMAB traffic allocation
             var bucketResult = Bucketer.BucketToEntityId(config, experiment, bucketingId, userId,
                 cmabTrafficAllocation);
             reasons += bucketResult.DecisionReasons;
@@ -341,7 +320,6 @@ OptimizelyDecideOption[] options
             {
                 var cmabDecision = CmabService.GetDecision(config, user, experiment.Id, options);
 
-                // Get the variation from the project config
                 var variation = config.GetVariationFromIdByExperimentId(experiment.Id,
                     cmabDecision.VariationId);
 

OptimizelySDK/Cmab/CmabConfig.cs

@@ -25,44 +25,55 @@ namespace OptimizelySDK.Cmab
     public class CmabConfig
     {
         /// <summary>
-        ///     Initializes a new instance of the CmabConfig class with default cache settings.
+        ///     Gets or sets the maximum number of entries in the CMAB cache.
+        ///     If null, the default value (1000) will be used.
         /// </summary>
-        /// <param name="cacheSize">Maximum number of entries in the cache. Default is 1000.</param>
-        /// <param name="cacheTtl">Time-to-live for cache entries. Default is 30 minutes.</param>
-        public CmabConfig(int? cacheSize = null, TimeSpan? cacheTtl = null)
-        {
-            CacheSize = cacheSize;
-            CacheTtl = cacheTtl;
-            CustomCache = null;
-        }
+        public int? CacheSize { get; private set; }
 
         /// <summary>
-        ///     Initializes a new instance of the CmabConfig class with a custom cache implementation.
+        ///     Gets or sets the time-to-live for CMAB cache entries.
+        ///     If null, the default value (30 minutes) will be used.
         /// </summary>
-        /// <param name="customCache">Custom cache implementation for CMAB decisions.</param>
-        public CmabConfig(ICacheWithRemove<CmabCacheEntry> customCache)
-        {
-            CustomCache = customCache ?? throw new ArgumentNullException(nameof(customCache));
-            CacheSize = null;
-            CacheTtl = null;
-        }
+        public TimeSpan? CacheTtl { get; private set; }
 
         /// <summary>
-        ///     Gets the maximum number of entries in the CMAB cache.
-        ///     If null, the default value (1000) will be used.
+        ///     Gets or sets the custom cache implementation for CMAB decisions.
+        ///     If provided, CacheSize and CacheTtl will be ignored.
         /// </summary>
-        public int? CacheSize { get; }
+        public ICacheWithRemove<CmabCacheEntry> Cache { get; private set; }
 
         /// <summary>
-        ///     Gets the time-to-live for CMAB cache entries.
-        ///     If null, the default value (30 minutes) will be used.
+        ///     Sets the maximum number of entries in the CMAB cache.
         /// </summary>
-        public TimeSpan? CacheTtl { get; }
+        /// <param name="cacheSize">Maximum number of entries in the cache.</param>
+        /// <returns>This CmabConfig instance for method chaining.</returns>
+        public CmabConfig SetCacheSize(int cacheSize)
+        {
+            CacheSize = cacheSize;
+            return this;
+        }
 
         /// <summary>
-        ///     Gets the custom cache implementation for CMAB decisions.
-        ///     If provided, CacheSize and CacheTtl will be ignored.
+        ///     Sets the time-to-live for CMAB cache entries.
         /// </summary>
-        public ICacheWithRemove<CmabCacheEntry> CustomCache { get; }
+        /// <param name="cacheTtl">Time-to-live for cache entries.</param>
+        /// <returns>This CmabConfig instance for method chaining.</returns>
+        public CmabConfig SetCacheTtl(TimeSpan cacheTtl)
+        {
+            CacheTtl = cacheTtl;
+            return this;
+        }
+
+        /// <summary>
+        ///     Sets a custom cache implementation for CMAB decisions.
+        ///     When set, CacheSize and CacheTtl will be ignored.
+        /// </summary>
+        /// <param name="cache">Custom cache implementation for CMAB decisions.</param>
+        /// <returns>This CmabConfig instance for method chaining.</returns>
+        public CmabConfig SetCache(ICacheWithRemove<CmabCacheEntry> cache)
+        {
+            Cache = cache ?? throw new ArgumentNullException(nameof(cache));
+            return this;
+        }
     }
 }

OptimizelySDK/Optimizely.cs

@@ -291,9 +291,9 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null,
                 var config = cmabConfig ?? new CmabConfig();
                 ICacheWithRemove<CmabCacheEntry> cache;
 
-                if (config.CustomCache != null)
+                if (config.Cache != null)
                 {
-                    cache = config.CustomCache;
+                    cache = config.Cache;
                 }
                 else
                 {

OptimizelySDK/OptimizelyFactory.cs

@@ -96,7 +96,9 @@ public static void SetLogger(ILogger logger)
         /// <param name="cacheTtl">Time-to-live for CMAB cache entries.</param>
         public static void SetCmabCacheConfig(int cacheSize, TimeSpan cacheTtl)
         {
-            CmabConfiguration = new CmabConfig(cacheSize, cacheTtl);
+            CmabConfiguration = new CmabConfig()
+                .SetCacheSize(cacheSize)
+                .SetCacheTtl(cacheTtl);
         }
 
         /// <summary>
@@ -105,7 +107,8 @@ public static void SetCmabCacheConfig(int cacheSize, TimeSpan cacheTtl)
         /// <param name="customCache">Custom cache implementation.</param>
         public static void SetCmabCustomCache(ICacheWithRemove<CmabCacheEntry> customCache)
         {
-            CmabConfiguration = new CmabConfig(customCache);
+            CmabConfiguration = new CmabConfig()
+                .SetCache(customCache);
         }
 #endif