Skip to content

Commit b5e691b

Browse files
FarhanAnjum-optiFarhanAnjum-opti
committed
Add DecisionPath parameter to bucketing methods for CMAB support
1 parent 41e491a commit b5e691b

File tree

4 files changed

+42
-25
lines changed

4 files changed

+42
-25
lines changed

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

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,8 @@ private Experiment bucketToExperiment(@Nonnull Group group,
9898

9999
@Nonnull
100100
private DecisionResponse<Variation> bucketToVariation(@Nonnull ExperimentCore experiment,
101-
@Nonnull String bucketingId) {
101+
@Nonnull String bucketingId,
102+
@Nonnull DecisionPath decisionPath) {
102103
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
103104

104105
// "salt" the bucket id using the experiment id
@@ -112,7 +113,8 @@ private DecisionResponse<Variation> bucketToVariation(@Nonnull ExperimentCore ex
112113
int bucketValue = generateBucketValue(hashCode);
113114
logger.debug("Assigned bucket {} to user with bucketingId \"{}\" when bucketing to a variation.", bucketValue, bucketingId);
114115

115-
if (experiment instanceof Experiment && ((Experiment) experiment).getCmab() != null) {
116+
// Only apply CMAB traffic allocation logic if decision path is WITH_CMAB
117+
if (decisionPath == DecisionPath.WITH_CMAB && experiment instanceof Experiment && ((Experiment) experiment).getCmab() != null) {
116118
// Override traffic allocations for CMAB experiments
117119
String message = reasons.addInfo("Using CMAB traffic allocation for experiment \"%s\"", experimentKey);
118120
logger.info(message);
@@ -122,7 +124,7 @@ private DecisionResponse<Variation> bucketToVariation(@Nonnull ExperimentCore ex
122124
}
123125

124126
String bucketedVariationId = bucketToEntity(bucketValue, trafficAllocations);
125-
if ("$".equals(bucketedVariationId)) {
127+
if (decisionPath == DecisionPath.WITH_CMAB && "$".equals(bucketedVariationId)) {
126128
// for cmab experiments
127129
String message = reasons.addInfo("User with bucketingId \"%s\" is bucketed into CMAB for experiment \"%s\"", bucketingId, experimentKey);
128130
logger.info(message);
@@ -150,12 +152,14 @@ else if (bucketedVariationId != null) {
150152
* @param experiment The Experiment in which the user is to be bucketed.
151153
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
152154
* @param projectConfig The current projectConfig
155+
* @param decisionPath enum for decision making logic
153156
* @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons
154157
*/
155158
@Nonnull
156159
public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
157160
@Nonnull String bucketingId,
158-
@Nonnull ProjectConfig projectConfig) {
161+
@Nonnull ProjectConfig projectConfig,
162+
@Nonnull DecisionPath decisionPath) {
159163
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
160164

161165
// ---------- Bucket User ----------
@@ -170,8 +174,6 @@ public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
170174
String message = reasons.addInfo("User with bucketingId \"%s\" is not in any experiment of group %s.", bucketingId, experimentGroup.getId());
171175
logger.info(message);
172176
return new DecisionResponse(null, reasons);
173-
} else {
174-
175177
}
176178
// if the experiment a user is bucketed in within a group isn't the same as the experiment provided,
177179
// don't perform further bucketing within the experiment
@@ -188,11 +190,26 @@ public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
188190
}
189191
}
190192

191-
DecisionResponse<Variation> decisionResponse = bucketToVariation(experiment, bucketingId);
193+
DecisionResponse<Variation> decisionResponse = bucketToVariation(experiment, bucketingId, decisionPath);
192194
reasons.merge(decisionResponse.getReasons());
193195
return new DecisionResponse<>(decisionResponse.getResult(), reasons);
194196
}
195197

198+
/**
199+
* Assign a {@link Variation} of an {@link Experiment} to a user based on hashed value from murmurhash3.
200+
*
201+
* @param experiment The Experiment in which the user is to be bucketed.
202+
* @param bucketingId string A customer-assigned value used to create the key for the murmur hash.
203+
* @param projectConfig The current projectConfig
204+
* @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons
205+
*/
206+
@Nonnull
207+
public DecisionResponse<Variation> bucket(@Nonnull ExperimentCore experiment,
208+
@Nonnull String bucketingId,
209+
@Nonnull ProjectConfig projectConfig) {
210+
return bucket(experiment, bucketingId, projectConfig, DecisionPath.WITHOUT_CMAB);
211+
}
212+
196213
//======== Helper methods ========//
197214

198215
/**

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
170170
if (decisionMeetAudience.getResult()) {
171171
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
172172
String cmabUUID = null;
173-
decisionVariation = bucketer.bucket(experiment, bucketingId, projectConfig);
173+
decisionVariation = bucketer.bucket(experiment, bucketingId, projectConfig, decisionPath);
174174
if (decisionPath == DecisionPath.WITH_CMAB && isCmabExperiment(experiment) && decisionVariation.getResult() != null) {
175175
// group-allocation and traffic-allocation checking passed for cmab
176176
// we need server decision overruling local bucketing for cmab

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ public void activateForNullVariation() throws Exception {
504504
Experiment activatedExperiment = validProjectConfig.getExperiments().get(0);
505505
Map<String, String> testUserAttributes = Collections.singletonMap("browser_type", "chromey");
506506

507-
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.nullNoReasons());
507+
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.nullNoReasons());
508508

509509
logbackVerifier.expectMessage(Level.INFO, "Not activating user \"userId\" for experiment \"" +
510510
activatedExperiment.getKey() + "\".");
@@ -1381,7 +1381,7 @@ public void getVariation() throws Exception {
13811381

13821382
Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build();
13831383

1384-
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
1384+
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
13851385

13861386
Map<String, String> testUserAttributes = new HashMap<>();
13871387
testUserAttributes.put("browser_type", "chrome");
@@ -1391,7 +1391,7 @@ public void getVariation() throws Exception {
13911391
testUserAttributes);
13921392

13931393
// verify that the bucketing algorithm was called correctly
1394-
verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig));
1394+
verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(validProjectConfig), any(DecisionPath.class));
13951395
assertThat(actualVariation, is(bucketedVariation));
13961396

13971397
// verify that we didn't attempt to dispatch an event
@@ -1412,13 +1412,13 @@ public void getVariationWithExperimentKey() throws Exception {
14121412
.withConfig(noAudienceProjectConfig)
14131413
.build();
14141414

1415-
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
1415+
when(mockBucketer.bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
14161416

14171417
// activate the experiment
14181418
Variation actualVariation = optimizely.getVariation(activatedExperiment.getKey(), testUserId);
14191419

14201420
// verify that the bucketing algorithm was called correctly
1421-
verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig));
1421+
verify(mockBucketer).bucket(eq(activatedExperiment), eq(testUserId), eq(noAudienceProjectConfig), any(DecisionPath.class));
14221422
assertThat(actualVariation, is(bucketedVariation));
14231423

14241424
// verify that we didn't attempt to dispatch an event
@@ -1473,7 +1473,7 @@ public void getVariationWithAudiences() throws Exception {
14731473
Experiment experiment = validProjectConfig.getExperiments().get(0);
14741474
Variation bucketedVariation = experiment.getVariations().get(0);
14751475

1476-
when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
1476+
when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(validProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
14771477

14781478
Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build();
14791479

@@ -1482,7 +1482,7 @@ public void getVariationWithAudiences() throws Exception {
14821482

14831483
Variation actualVariation = optimizely.getVariation(experiment.getKey(), testUserId, testUserAttributes);
14841484

1485-
verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(validProjectConfig));
1485+
verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(validProjectConfig), any(DecisionPath.class));
14861486
assertThat(actualVariation, is(bucketedVariation));
14871487
}
14881488

@@ -1523,7 +1523,7 @@ public void getVariationNoAudiences() throws Exception {
15231523
Experiment experiment = noAudienceProjectConfig.getExperiments().get(0);
15241524
Variation bucketedVariation = experiment.getVariations().get(0);
15251525

1526-
when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
1526+
when(mockBucketer.bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
15271527

15281528
Optimizely optimizely = optimizelyBuilder
15291529
.withConfig(noAudienceProjectConfig)
@@ -1532,7 +1532,7 @@ public void getVariationNoAudiences() throws Exception {
15321532

15331533
Variation actualVariation = optimizely.getVariation(experiment.getKey(), testUserId);
15341534

1535-
verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig));
1535+
verify(mockBucketer).bucket(eq(experiment), eq(testUserId), eq(noAudienceProjectConfig), any(DecisionPath.class));
15361536
assertThat(actualVariation, is(bucketedVariation));
15371537
}
15381538

@@ -1590,7 +1590,7 @@ public void getVariationForGroupExperimentWithMatchingAttributes() throws Except
15901590
attributes.put("browser_type", "chrome");
15911591
}
15921592

1593-
when(mockBucketer.bucket(eq(experiment), eq("user"), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation));
1593+
when(mockBucketer.bucket(eq(experiment), eq("user"), eq(validProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(variation));
15941594

15951595
Optimizely optimizely = optimizelyBuilder.withBucketing(mockBucketer).build();
15961596

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,7 +1041,7 @@ public void getVariationSavesBucketedVariationIntoUserProfile() throws Exception
10411041
Collections.singletonMap(experiment.getId(), decision));
10421042

10431043
Bucketer mockBucketer = mock(Bucketer.class);
1044-
when(mockBucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation));
1044+
when(mockBucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(variation));
10451045

10461046
DecisionService decisionService = new DecisionService(mockBucketer, mockErrorHandler, userProfileService, mockCmabService);
10471047

@@ -1107,7 +1107,7 @@ public void getVariationSavesANewUserProfile() throws Exception {
11071107
UserProfileService userProfileService = mock(UserProfileService.class);
11081108
DecisionService decisionService = new DecisionService(bucketer, mockErrorHandler, userProfileService, mockCmabService);
11091109

1110-
when(bucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(variation));
1110+
when(bucketer.bucket(eq(experiment), eq(userProfileId), eq(noAudienceProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(variation));
11111111
when(userProfileService.lookup(userProfileId)).thenReturn(null);
11121112

11131113
assertEquals(variation, decisionService.getVariation(experiment, optimizely.createUserContext(userProfileId, Collections.emptyMap()), noAudienceProjectConfig).getResult());
@@ -1121,7 +1121,7 @@ public void getVariationBucketingId() throws Exception {
11211121
Experiment experiment = validProjectConfig.getExperiments().get(0);
11221122
Variation expectedVariation = experiment.getVariations().get(0);
11231123

1124-
when(bucketer.bucket(eq(experiment), eq("bucketId"), eq(validProjectConfig))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation));
1124+
when(bucketer.bucket(eq(experiment), eq("bucketId"), eq(validProjectConfig), any(DecisionPath.class))).thenReturn(DecisionResponse.responseNoReasons(expectedVariation));
11251125

11261126
Map<String, Object> attr = new HashMap();
11271127
attr.put(ControlAttribute.BUCKETING_ATTRIBUTE.toString(), "bucketId");
@@ -1538,7 +1538,7 @@ public void getVariationCmabExperimentServiceError() {
15381538
// Bucketer bucketer = new Bucketer();
15391539
Bucketer mockBucketer = mock(Bucketer.class);
15401540
Variation bucketedVariation = new Variation("$", "$");
1541-
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)))
1541+
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), any(DecisionPath.class)))
15421542
.thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
15431543
DecisionService decisionServiceWithMockCmabService = new DecisionService(
15441544
mockBucketer,
@@ -1603,7 +1603,7 @@ public void getVariationCmabExperimentServiceSuccess() {
16031603
// Mock bucketer to return a variation (user is in CMAB traffic)
16041604
Variation bucketedVariation = new Variation("$", "$");
16051605
Bucketer mockBucketer = mock(Bucketer.class);
1606-
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)))
1606+
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), eq(DecisionPath.WITH_CMAB)))
16071607
.thenReturn(DecisionResponse.responseNoReasons(bucketedVariation));
16081608

16091609
DecisionService decisionServiceWithMockCmabService = new DecisionService(
@@ -1674,7 +1674,7 @@ public void getVariationCmabExperimentUserNotInTrafficAllocation() {
16741674

16751675
// Mock bucketer to return null for CMAB allocation (user not in CMAB traffic)
16761676
Bucketer mockBucketer = mock(Bucketer.class);
1677-
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class)))
1677+
when(mockBucketer.bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), any(DecisionPath.class)))
16781678
.thenReturn(DecisionResponse.nullNoReasons());
16791679

16801680
DecisionService decisionServiceWithMockCmabService = new DecisionService(
@@ -1701,7 +1701,7 @@ public void getVariationCmabExperimentUserNotInTrafficAllocation() {
17011701
verify(mockCmabService, never()).getDecision(any(), any(), any(), any());
17021702

17031703
// Verify that bucketer was called for CMAB allocation
1704-
verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class));
1704+
verify(mockBucketer, times(1)).bucket(any(Experiment.class), anyString(), any(ProjectConfig.class), any(DecisionPath.class));
17051705
}
17061706

17071707
private Experiment createMockCmabExperiment() {

0 commit comments

Comments
 (0)