Skip to content

Commit a80c0d3

Browse files
FarhanAnjum-optiFarhanAnjum-opti
committed
update: enhance CMAB handling in bucketing and decision services, add backward compatibility for mobile apps
1 parent 6fc6446 commit a80c0d3

File tree

4 files changed

+70
-74
lines changed

4 files changed

+70
-74
lines changed

‎core-api/src/main/java/com/optimizely/ab/Optimizely.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,7 @@ Map<String, OptimizelyDecision> decideAll(@Nonnull OptimizelyUserContext user,
15121512
/**
15131513
* Returns a decision result ({@link OptimizelyDecision}) for a given flag key and a user context,
15141514
* skipping CMAB logic and using only traditional A/B testing.
1515+
* This will be called by mobile apps which will use non-blocking legacy ab-tests only (for backward compatibility with android-sdk)
15151516
*
15161517
* @param user An OptimizelyUserContext associated with this OptimizelyClient.
15171518
* @param key A flag key for which a decision will be made.
@@ -1534,7 +1535,8 @@ OptimizelyDecision decideSync(@Nonnull OptimizelyUserContext user,
15341535

15351536
/**
15361537
* Returns decision results for multiple flag keys, skipping CMAB logic and using only traditional A/B testing.
1537-
*
1538+
* This will be called by mobile apps which will use non-blocking legacy ab-tests only (for backward compatibility with android-sdk)
1539+
*
15381540
* @param user An OptimizelyUserContext associated with this OptimizelyClient.
15391541
* @param keys A list of flag keys for which decisions will be made.
15401542
* @param options A list of options for decision-making.
@@ -1548,7 +1550,8 @@ Map<String, OptimizelyDecision> decideForKeysSync(@Nonnull OptimizelyUserContext
15481550

15491551
/**
15501552
* Returns decision results for all active flag keys, skipping CMAB logic and using only traditional A/B testing.
1551-
*
1553+
* This will be called by mobile apps which will use non-blocking legacy ab-tests only (for backward compatibility with android-sdk)
1554+
*
15521555
* @param user An OptimizelyUserContext associated with this OptimizelyClient.
15531556
* @param options A list of options for decision-making.
15541557
* @return All decision results mapped by flag keys, using traditional A/B testing logic only.

‎core-api/src/main/java/com/optimizely/ab/bucketing/Bucketer.java‎

Lines changed: 33 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -183,6 +183,24 @@ private DecisionResponse<String> bucketToEntityForCmab(@Nonnull Experiment exper
183183
public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
184184
@Nonnull String bucketingId,
185185
@Nonnull ProjectConfig projectConfig) {
186+
187+
return bucket(experiment, bucketingId, projectConfig, false);
188+
}
189+
190+
/**
191+
* Assign a {@link Variation} of an {@link Experiment} to a user based on hashed value from murmurhash3.
192+
*
193+
* @param experiment The Experiment in which the user is to be bucketed.
194+
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
195+
* @param projectConfig The current projectConfig
196+
* @param useCmab boolean flag to decide whether to handle cmab experiments.
197+
* @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons
198+
*/
199+
@Nonnull
200+
public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
201+
@Nonnull String bucketingId,
202+
@Nonnull ProjectConfig projectConfig,
203+
@Nonnull boolean useCmab) {
186204
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
187205

188206
// ---------- Bucket User ----------
@@ -215,57 +233,26 @@ public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
215233
}
216234
}
217235

218-
DecisionResponse<Variation> decisionResponse = bucketToVariation(experiment, bucketingId);
219-
reasons.merge(decisionResponse.getReasons());
220-
return new DecisionResponse<>(decisionResponse.getResult(), reasons);
221-
}
222-
223-
/**
224-
* Assign a user to CMAB traffic for an experiment based on hashed value from murmurhash3.
225-
* This method handles CMAB (Contextual Multi-Armed Bandit) traffic allocation.
226-
*
227-
* @param experiment The CMAB Experiment in which the user is to be bucketed.
228-
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
229-
* @param projectConfig The current projectConfig
230-
* @return A {@link DecisionResponse} including the entity ID ("$" if bucketed to CMAB, null otherwise) and decision reasons
231-
*/
232-
@Nonnull
233-
public DecisionResponse<String> bucketForCmab(@Nonnull Experiment experiment,
234-
@Nonnull String bucketingId,
235-
@Nonnull ProjectConfig projectConfig) {
236-
237-
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
238-
239-
// ---------- Handle Group Logic (same as regular bucket method) ----------
240-
String groupId = experiment.getGroupId();
241-
if (!groupId.isEmpty()) {
242-
Group experimentGroup = projectConfig.getGroupIdMapping().get(groupId);
243-
244-
if (experimentGroup.getPolicy().equals(Group.RANDOM_POLICY)) {
245-
Experiment bucketedExperiment = bucketToExperiment(experimentGroup, bucketingId, projectConfig);
246-
if (bucketedExperiment == null) {
247-
String message = reasons.addInfo("User with bucketingId \"%s\" is not in any experiment of group %s.", bucketingId, experimentGroup.getId());
248-
logger.info(message);
249-
return new DecisionResponse<>(null, reasons);
250-
}
251-
252-
if (!bucketedExperiment.getId().equals(experiment.getId())) {
253-
String message = reasons.addInfo("User with bucketingId \"%s\" is not in experiment \"%s\" of group %s.", bucketingId, experiment.getKey(),
254-
experimentGroup.getId());
255-
logger.info(message);
236+
if (useCmab){
237+
if (experiment instanceof Experiment) {
238+
DecisionResponse<String> decisionResponse = bucketToEntityForCmab((Experiment) experiment, bucketingId);
239+
reasons.merge(decisionResponse.getReasons());
240+
String entityId = decisionResponse.getResult();
241+
if (entityId==null){
256242
return new DecisionResponse<>(null, reasons);
257243
}
258-
259-
String message = reasons.addInfo("User with bucketingId \"%s\" is in experiment \"%s\" of group %s.", bucketingId, experiment.getKey(),
260-
experimentGroup.getId());
244+
Variation variation = new Variation(entityId, entityId); //return dummy variation for cmab
245+
return new DecisionResponse<>(variation, reasons);
246+
} else {
247+
String message = reasons.addInfo("ExperimentCore instance is not of type Experiment, cannot perform CMAB bucketing.");
261248
logger.info(message);
249+
return new DecisionResponse<>(null, reasons);
262250
}
251+
} else {
252+
DecisionResponse<Variation> decisionResponse = bucketToVariation(experiment, bucketingId);
253+
reasons.merge(decisionResponse.getReasons());
254+
return new DecisionResponse<>(decisionResponse.getResult(), reasons);
263255
}
264-
265-
// ---------- Use CMAB-aware bucketToEntity ----------
266-
DecisionResponse<String> decisionResponse = bucketToEntityForCmab(experiment, bucketingId);
267-
reasons.merge(decisionResponse.getReasons());
268-
return new DecisionResponse<>(decisionResponse.getResult(), reasons);
269256
}
270257

271258
//======== Helper methods ========//

‎core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -941,10 +941,12 @@ private DecisionResponse<CmabDecision> getDecisionForCmabExperiment(@Nonnull Pro
941941
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
942942

943943
// Check if user is in CMAB traffic allocation
944-
DecisionResponse<String> bucketResponse = bucketer.bucketForCmab(experiment, bucketingId, projectConfig);
944+
DecisionResponse<Variation> bucketResponse = bucketer.bucket(experiment, bucketingId, projectConfig, true);
945+
// DecisionResponse<String> bucketResponse = bucketer.bucketForCmab(experiment, bucketingId, projectConfig);
945946
reasons.merge(bucketResponse.getReasons());
946947

947-
String bucketedEntityId = bucketResponse.getResult();
948+
Variation bucketedVariation = bucketResponse.getResult();
949+
String bucketedEntityId = bucketedVariation != null ? bucketedVariation.getId() : null;
948950

949951
if (bucketedEntityId == null) {
950952
String message = String.format("User \"%s\" not in CMAB experiment \"%s\" due to traffic allocation.",

‎core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java‎

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1527,7 +1527,7 @@ public void getVariationCmabExperimentServiceError() {
15271527
cmabExperiment.getKey(),
15281528
cmabExperiment.getStatus(),
15291529
cmabExperiment.getLayerId(),
1530-
cmabExperiment.getAudienceIds(),
1530+
cmabExperiment.getAudienceIds(),
15311531
cmabExperiment.getAudienceConditions(),
15321532
cmabExperiment.getVariations(),
15331533
Collections.emptyMap(), // No whitelisting
@@ -1574,65 +1574,69 @@ public void getVariationCmabExperimentServiceError() {
15741574
*/
15751575
@Test
15761576
public void getVariationCmabExperimentServiceSuccess() {
1577-
// Create a CMAB experiment
1578-
Experiment cmabExperiment = createMockCmabExperiment();
1579-
Variation expectedVariation = cmabExperiment.getVariations().get(1); // Use second variation
1577+
// Use an existing experiment from v4ProjectConfig and modify it to be CMAB
1578+
Experiment baseExperiment = v4ProjectConfig.getExperiments().get(0);
1579+
Variation expectedVariation = baseExperiment.getVariations().get(0);
15801580

15811581
// Create mock Cmab object
15821582
Cmab mockCmab = mock(Cmab.class);
1583-
when(mockCmab.getTrafficAllocation()).thenReturn(4000);
1583+
when(mockCmab.getTrafficAllocation()).thenReturn(10000); // 100% allocation
15841584

1585-
// Create experiment with CMAB config (no whitelisting, no forced variations)
1586-
Experiment experiment = new Experiment(
1587-
cmabExperiment.getId(),
1588-
cmabExperiment.getKey(),
1589-
cmabExperiment.getStatus(),
1590-
cmabExperiment.getLayerId(),
1591-
cmabExperiment.getAudienceIds(),
1592-
cmabExperiment.getAudienceConditions(),
1593-
cmabExperiment.getVariations(),
1585+
// Create CMAB experiment using existing experiment structure
1586+
Experiment cmabExperiment = new Experiment(
1587+
baseExperiment.getId(),
1588+
baseExperiment.getKey(),
1589+
baseExperiment.getStatus(),
1590+
baseExperiment.getLayerId(),
1591+
baseExperiment.getAudienceIds(),
1592+
baseExperiment.getAudienceConditions(),
1593+
baseExperiment.getVariations(),
15941594
Collections.emptyMap(), // No whitelisting
1595-
cmabExperiment.getTrafficAllocation(),
1595+
baseExperiment.getTrafficAllocation(),
15961596
mockCmab // This makes it a CMAB experiment
15971597
);
15981598

1599+
// Mock bucketer to return a variation (user is in CMAB traffic)
1600+
Variation bucketedVariation = new Variation("$", "$");
15991601
Bucketer mockBucketer = mock(Bucketer.class);
1600-
when(mockBucketer.bucketForCmab(any(Experiment.class), anyString(), any(ProjectConfig.class)))
1601-
.thenReturn(DecisionResponse.responseNoReasons("$"));
1602+
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), eq(true)))
1603+
.thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
1604+
16021605
DecisionService decisionServiceWithMockCmabService = new DecisionService(
16031606
mockBucketer,
16041607
mockErrorHandler,
16051608
null,
16061609
mockCmabService
16071610
);
16081611

1609-
// Mock CmabService.getDecision to return a valid decision
1612+
// Mock CmabService.getDecision to return the expected variation ID
16101613
CmabDecision mockCmabDecision = mock(CmabDecision.class);
16111614
when(mockCmabDecision.getVariationId()).thenReturn(expectedVariation.getId());
16121615
when(mockCmabService.getDecision(any(), any(), any(), any()))
16131616
.thenReturn(mockCmabDecision);
16141617

16151618
// Call getVariation
16161619
DecisionResponse<Variation> result = decisionServiceWithMockCmabService.getVariation(
1617-
experiment,
1620+
cmabExperiment,
16181621
optimizely.createUserContext(genericUserId, Collections.emptyMap()),
16191622
v4ProjectConfig
16201623
);
16211624

16221625
// Verify that CMAB service decision is returned
1626+
assertNotNull("Result should not be null", result.getResult());
16231627
assertEquals(expectedVariation, result.getResult());
16241628

16251629
// Verify that the result is not an error
16261630
assertFalse(result.isError());
16271631

1628-
// Assert that CmabService.getDecision was called exactly once
1632+
// Verify CmabService.getDecision was called
16291633
verify(mockCmabService, times(1)).getDecision(any(), any(), any(), any());
16301634

16311635
// Verify that the correct parameters were passed to CMAB service
16321636
verify(mockCmabService).getDecision(
16331637
eq(v4ProjectConfig),
16341638
any(OptimizelyUserContext.class),
1635-
eq(experiment.getId()),
1639+
eq(cmabExperiment.getId()),
16361640
any(List.class)
16371641
);
16381642
}
@@ -1648,7 +1652,7 @@ public void getVariationCmabExperimentUserNotInTrafficAllocation() {
16481652

16491653
// Create mock Cmab object
16501654
Cmab mockCmab = mock(Cmab.class);
1651-
when(mockCmab.getTrafficAllocation()).thenReturn(5000); // 50% traffic allocation
1655+
when(mockCmab.getTrafficAllocation()).thenReturn(5000);
16521656

16531657
// Create experiment with CMAB config (no whitelisting, no forced variations)
16541658
Experiment experiment = new Experiment(
@@ -1666,7 +1670,7 @@ public void getVariationCmabExperimentUserNotInTrafficAllocation() {
16661670

16671671
// Mock bucketer to return null for CMAB allocation (user not in CMAB traffic)
16681672
Bucketer mockBucketer = mock(Bucketer.class);
1669-
when(mockBucketer.bucketForCmab(any(Experiment.class), anyString(), any(ProjectConfig.class)))
1673+
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), eq(true)))
16701674
.thenReturn(DecisionResponse.nullNoReasons());
16711675

16721676
DecisionService decisionServiceWithMockCmabService = new DecisionService(
@@ -1693,7 +1697,7 @@ public void getVariationCmabExperimentUserNotInTrafficAllocation() {
16931697
verify(mockCmabService, never()).getDecision(any(), any(), any(), any());
16941698

16951699
// Verify that bucketer was called for CMAB allocation
1696-
verify(mockBucketer, times(1)).bucketForCmab(any(Experiment.class), anyString(), any(ProjectConfig.class));
1700+
verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), eq(true));
16971701
}
16981702

16991703
private Experiment createMockCmabExperiment() {

0 commit comments

Comments
 (0)