Skip to content

Commit eb6877b

Browse files
junaed-optimizelyjunaed-optimizely
committed
[FSSDK-11177] review update 3
1 parent c9d3d17 commit eb6877b

File tree

7 files changed

+40
-64
lines changed

7 files changed

+40
-64
lines changed

‎OptimizelySDK.Tests/CmabTests/OptimizelyUserContextCmabTest.cs‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ public void SetUp()
8888
}
8989

9090
/// <summary>
91-
/// Verifies Decide returns decision with CMAB UUID populated
91+
/// Verifies Decide returns decision for CMAB experiment
9292
/// </summary>
9393
[Test]
9494
public void TestDecideWithCmabExperimentReturnsDecision()
@@ -101,7 +101,6 @@ public void TestDecideWithCmabExperimentReturnsDecision()
101101
Assert.IsTrue(decision.Enabled, "Feature flag should be enabled for CMAB variation.");
102102
Assert.AreEqual(TEST_FEATURE_KEY, decision.FlagKey);
103103
Assert.AreEqual(TEST_EXPERIMENT_KEY, decision.RuleKey);
104-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
105104
Assert.IsTrue(decision.Reasons == null || decision.Reasons.Length == 0);
106105

107106
Assert.AreEqual(1, _cmabService.CallCount);
@@ -123,7 +122,6 @@ public void TestDecideWithCmabExperimentVerifyImpressionEvent()
123122
var decision = userContext.Decide(TEST_FEATURE_KEY);
124123

125124
Assert.IsNotNull(decision);
126-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
127125
_eventDispatcherMock.Verify(d => d.DispatchEvent(It.IsAny<LogEvent>()), Times.Once);
128126
Assert.IsNotNull(impressionEvent, "Impression event should be dispatched.");
129127

@@ -182,7 +180,6 @@ public void TestDecideWithCmabExperimentDisableDecisionEvent()
182180
new[] { OptimizelyDecideOption.DISABLE_DECISION_EVENT });
183181

184182
Assert.IsNotNull(decision);
185-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
186183
_eventDispatcherMock.Verify(d => d.DispatchEvent(It.IsAny<LogEvent>()), Times.Never,
187184
"No impression event should be sent with DISABLE_DECISION_EVENT");
188185
Assert.AreEqual(1, _cmabService.CallCount);
@@ -210,10 +207,8 @@ public void TestDecideForKeysMixedCmabAndNonCmab()
210207

211208
Assert.IsNotNull(cmabDecision);
212209
Assert.AreEqual(VARIATION_A_KEY, cmabDecision.VariationKey);
213-
Assert.AreEqual(TEST_CMAB_UUID, cmabDecision.CmabUuid);
214210

215211
Assert.IsNotNull(nonCmabDecision);
216-
Assert.IsNull(nonCmabDecision.CmabUuid);
217212
Assert.AreEqual(1, _cmabService.CallCount);
218213
}
219214

@@ -231,7 +226,6 @@ public void TestDecideAllIncludesCmabExperiments()
231226
Assert.IsTrue(decisions.TryGetValue(TEST_FEATURE_KEY, out var cmabDecision));
232227
Assert.IsNotNull(cmabDecision);
233228
Assert.AreEqual(VARIATION_A_KEY, cmabDecision.VariationKey);
234-
Assert.AreEqual(TEST_CMAB_UUID, cmabDecision.CmabUuid);
235229
Assert.GreaterOrEqual(_cmabService.CallCount, 1);
236230
}
237231

@@ -335,7 +329,6 @@ public void TestDecideWithCmabExperimentUserProfileService()
335329

336330
Assert.IsNotNull(decision);
337331
Assert.AreEqual(VARIATION_A_KEY, decision.VariationKey);
338-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
339332
userProfileServiceMock.Verify(ups => ups.Save(It.IsAny<Dictionary<string, object>>()),
340333
Times.Never);
341334
Assert.AreEqual(1, cmabService.CallCount);
@@ -369,7 +362,6 @@ public void TestDecideWithCmabExperimentIgnoreUserProfileService()
369362

370363
Assert.IsNotNull(decision);
371364
Assert.AreEqual(VARIATION_A_KEY, decision.VariationKey);
372-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
373365

374366
userProfileServiceMock.Verify(ups => ups.Lookup(It.IsAny<string>()), Times.Never);
375367
Assert.AreEqual(1, cmabService.CallCount);
@@ -394,7 +386,6 @@ public void TestDecideWithCmabExperimentIncludeReasons()
394386
TEST_EXPERIMENT_KEY);
395387
Assert.IsTrue(decision.Reasons.Any(r => r.Contains(expectedMessage)),
396388
"Decision reasons should include CMAB fetch success message.");
397-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
398389
Assert.AreEqual(1, _cmabService.CallCount);
399390
}
400391

@@ -413,7 +404,6 @@ public void TestDecideWithCmabErrorReturnsErrorDecision()
413404

414405
Assert.IsNotNull(decision);
415406
Assert.IsNull(decision.VariationKey);
416-
Assert.IsNull(decision.CmabUuid);
417407
Assert.IsTrue(decision.Reasons.Any(r => r.Contains(
418408
string.Format(CmabConstants.CMAB_FETCH_FAILED, TEST_EXPERIMENT_KEY))));
419409
Assert.AreEqual(1, _cmabService.CallCount);
@@ -459,7 +449,6 @@ public void TestDecideWithCmabExperimentDecisionNotification()
459449
Assert.IsNotNull(decision);
460450
Assert.AreEqual(TEST_FEATURE_KEY, decision.FlagKey);
461451
Assert.AreEqual(VARIATION_A_KEY, decision.VariationKey);
462-
Assert.AreEqual(TEST_CMAB_UUID, decision.CmabUuid);
463452
_notificationCallbackMock.Verify(nc => nc.TestDecisionCallback(
464453
DecisionNotificationTypes.FLAG,
465454
TEST_USER_ID,

‎OptimizelySDK.Tests/OptimizelyFactoryTest.cs‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,15 @@ public void TestGetFeatureVariableJSONEmptyDatafileTest()
259259
}
260260

261261
[Test]
262-
public void SetCmabCacheConfigStoresCacheSizeAndTtl()
262+
public void SetCmabConfigStoresCacheSizeAndTtl()
263263
{
264264
const int cacheSize = 1234;
265265
var cacheTtl = TimeSpan.FromSeconds(45);
266266

267-
OptimizelyFactory.SetCmabCacheConfig(cacheSize, cacheTtl);
267+
var cmabConfig = new CmabConfig()
268+
.SetCacheSize(cacheSize)
269+
.SetCacheTtl(cacheTtl);
270+
OptimizelyFactory.SetCmabConfig(cmabConfig);
268271

269272
var config = GetCurrentCmabConfiguration();
270273

@@ -275,11 +278,13 @@ public void SetCmabCacheConfigStoresCacheSizeAndTtl()
275278
}
276279

277280
[Test]
278-
public void SetCmabCustomCacheStoresCustomCacheInstance()
281+
public void SetCmabConfigStoresCustomCacheInstance()
279282
{
280283
var customCache = new LruCache<CmabCacheEntry>(maxSize: 10, itemTimeout: TimeSpan.FromMinutes(2));
281284

282-
OptimizelyFactory.SetCmabCustomCache(customCache);
285+
var cmabConfig = new CmabConfig()
286+
.SetCache(customCache);
287+
OptimizelyFactory.SetCmabConfig(cmabConfig);
283288

284289
var config = GetCurrentCmabConfiguration();
285290

@@ -294,7 +299,10 @@ public void NewDefaultInstanceUsesConfiguredCmabCache()
294299
{
295300
const int cacheSize = 7;
296301
var cacheTtl = TimeSpan.FromSeconds(30);
297-
OptimizelyFactory.SetCmabCacheConfig(cacheSize, cacheTtl);
302+
var cmabConfig = new CmabConfig()
303+
.SetCacheSize(cacheSize)
304+
.SetCacheTtl(cacheTtl);
305+
OptimizelyFactory.SetCmabConfig(cmabConfig);
298306

299307
var logger = new NoOpLogger();
300308
var errorHandler = new NoOpErrorHandler();

‎OptimizelySDK/Cmab/CmabConfig.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
*/
1616

1717
using System;
18+
using OptimizelySDK.Cmab;
1819
using OptimizelySDK.Utils;
1920

20-
namespace OptimizelySDK.Cmab
21+
namespace OptimizelySDK
2122
{
2223
/// <summary>
2324
/// Configuration options for CMAB (Contextual Multi-Armed Bandit) functionality.

‎OptimizelySDK/Cmab/CmabConstants.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,8 @@ internal static class CmabConstants
4242

4343
public static readonly TimeSpan MAX_TIMEOUT = TimeSpan.FromSeconds(10);
4444
public static readonly TimeSpan DEFAULT_CACHE_TTL = TimeSpan.FromMinutes(10);
45+
46+
public const int CMAB_MAX_RETRIES = 1;
47+
public static readonly TimeSpan CMAB_INITIAL_BACKOFF = TimeSpan.FromMilliseconds(100);
4548
}
4649
}

‎OptimizelySDK/Optimizely.cs‎

Lines changed: 16 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -286,27 +286,25 @@ private void InitializeComponents(IEventDispatcher eventDispatcher = null,
286286
NotificationCenter = notificationCenter ?? new NotificationCenter(Logger);
287287

288288
#if USE_CMAB
289-
if (cmabService == null)
290-
{
291-
var config = cmabConfig ?? new CmabConfig();
292-
ICacheWithRemove<CmabCacheEntry> cache;
289+
var config = cmabConfig ?? new CmabConfig();
290+
ICacheWithRemove<CmabCacheEntry> cache;
293291

294-
if (config.Cache != null)
295-
{
296-
cache = config.Cache;
297-
}
298-
else
299-
{
300-
var cacheSize = config.CacheSize ?? CmabConstants.DEFAULT_CACHE_SIZE;
301-
var cacheTtl = config.CacheTtl ?? CmabConstants.DEFAULT_CACHE_TTL;
302-
cache = new LruCache<CmabCacheEntry>(cacheSize, cacheTtl, Logger);
303-
}
292+
if (config.Cache != null)
293+
{
294+
cache = config.Cache;
295+
}
296+
else
297+
{
298+
var cacheSize = config.CacheSize ?? CmabConstants.DEFAULT_CACHE_SIZE;
299+
var cacheTtl = config.CacheTtl ?? CmabConstants.DEFAULT_CACHE_TTL;
300+
cache = new LruCache<CmabCacheEntry>(cacheSize, cacheTtl, Logger);
301+
}
304302

305-
var cmabRetryConfig = new CmabRetryConfig(1, TimeSpan.FromMilliseconds(100));
306-
var cmabClient = new DefaultCmabClient(null, cmabRetryConfig, Logger);
303+
var cmabRetryConfig = new CmabRetryConfig(CmabConstants.CMAB_MAX_RETRIES,
304+
CmabConstants.CMAB_INITIAL_BACKOFF);
305+
var cmabClient = new DefaultCmabClient(null, cmabRetryConfig, Logger);
307306

308-
cmabService = new DefaultCmabService(cache, cmabClient, Logger);
309-
}
307+
cmabService = new DefaultCmabService(cache, cmabClient, Logger);
310308

311309
DecisionService =
312310
new DecisionService(Bucketer, ErrorHandler, userProfileService, Logger,
@@ -1172,9 +1170,6 @@ ProjectConfig projectConfig
11721170
flagKey,
11731171
user,
11741172
reasonsToReport
1175-
#if USE_CMAB
1176-
, flagDecision.CmabUuid
1177-
#endif
11781173
);
11791174
}
11801175

‎OptimizelySDK/OptimizelyDecisions/OptimizelyDecision.cs‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,13 @@ public class OptimizelyDecision
6060
/// </summary>
6161
public string[] Reasons { get; private set; }
6262

63-
/// <summary>
64-
/// CMAB UUID associated with the decision for contextual multi-armed bandit experiments.
65-
/// </summary>
66-
public string CmabUuid { get; private set; }
67-
6863
public OptimizelyDecision(string variationKey,
6964
bool enabled,
7065
OptimizelyJSON variables,
7166
string ruleKey,
7267
string flagKey,
7368
OptimizelyUserContext userContext,
74-
string[] reasons,
75-
string cmabUuid = null
69+
string[] reasons
7670
)
7771
{
7872
VariationKey = variationKey;
@@ -82,7 +76,6 @@ public OptimizelyDecision(string variationKey,
8276
FlagKey = flagKey;
8377
UserContext = userContext;
8478
Reasons = reasons;
85-
CmabUuid = cmabUuid;
8679
}
8780

8881
/// <summary>

‎OptimizelySDK/OptimizelyFactory.cs‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -90,25 +90,12 @@ public static void SetLogger(ILogger logger)
9090

9191
#if USE_CMAB
9292
/// <summary>
93-
/// Sets the CMAB cache configuration with custom size and time-to-live.
93+
/// Sets the CMAB (Contextual Multi-Armed Bandit) configuration.
9494
/// </summary>
95-
/// <param name="cacheSize">Maximum number of entries in the CMAB cache.</param>
96-
/// <param name="cacheTtl">Time-to-live for CMAB cache entries.</param>
97-
public static void SetCmabCacheConfig(int cacheSize, TimeSpan cacheTtl)
95+
/// <param name="config">CMAB configuration with cache settings.</param>
96+
public static void SetCmabConfig(CmabConfig config)
9897
{
99-
CmabConfiguration = new CmabConfig()
100-
.SetCacheSize(cacheSize)
101-
.SetCacheTtl(cacheTtl);
102-
}
103-
104-
/// <summary>
105-
/// Sets a custom cache implementation for CMAB.
106-
/// </summary>
107-
/// <param name="customCache">Custom cache implementation.</param>
108-
public static void SetCmabCustomCache(ICacheWithRemove<CmabCacheEntry> customCache)
109-
{
110-
CmabConfiguration = new CmabConfig()
111-
.SetCache(customCache);
98+
CmabConfiguration = config;
11299
}
113100
#endif
114101

0 commit comments

Comments
 (0)