Skip to content

Commit 89771bc

Browse files
FarhanAnjum-optiFarhanAnjum-opti
committed
update: refactor decision-making logic to use DecisionPath enum for clarity and maintainability
1 parent b2dcf9e commit 89771bc

File tree

5 files changed

+35
-31
lines changed

5 files changed

+35
-31
lines changed

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

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,7 @@
1616
package com.optimizely.ab;
1717

1818
import com.optimizely.ab.annotations.VisibleForTesting;
19-
import com.optimizely.ab.bucketing.Bucketer;
20-
import com.optimizely.ab.bucketing.DecisionService;
21-
import com.optimizely.ab.bucketing.FeatureDecision;
22-
import com.optimizely.ab.bucketing.UserProfileService;
19+
import com.optimizely.ab.bucketing.*;
2320
import com.optimizely.ab.cmab.service.CmabCacheValue;
2421
import com.optimizely.ab.cmab.service.CmabService;
2522
import com.optimizely.ab.cmab.service.CmabServiceOptions;
@@ -1409,7 +1406,7 @@ private Map<String, OptimizelyDecision> decideForKeys(@Nonnull OptimizelyUserCon
14091406
@Nonnull List<String> keys,
14101407
@Nonnull List<OptimizelyDecideOption> options,
14111408
boolean ignoreDefaultOptions,
1412-
boolean useCmab) {
1409+
DecisionPath decisionPath) {
14131410
Map<String, OptimizelyDecision> decisionMap = new HashMap<>();
14141411

14151412
ProjectConfig projectConfig = getProjectConfig();
@@ -1454,7 +1451,7 @@ private Map<String, OptimizelyDecision> decideForKeys(@Nonnull OptimizelyUserCon
14541451
}
14551452

14561453
List<DecisionResponse<FeatureDecision>> decisionList =
1457-
decisionService.getVariationsForFeatureList(flagsWithoutForcedDecision, user, projectConfig, allOptions, useCmab);
1454+
decisionService.getVariationsForFeatureList(flagsWithoutForcedDecision, user, projectConfig, allOptions, decisionPath);
14581455

14591456
for (int i = 0; i < flagsWithoutForcedDecision.size(); i++) {
14601457
DecisionResponse<FeatureDecision> decision = decisionList.get(i);
@@ -1497,7 +1494,7 @@ private Map<String, OptimizelyDecision> decideForKeys(@Nonnull OptimizelyUserCon
14971494
@Nonnull List<String> keys,
14981495
@Nonnull List<OptimizelyDecideOption> options,
14991496
boolean ignoreDefaultOptions) {
1500-
return decideForKeys(user, keys, options, ignoreDefaultOptions, true);
1497+
return decideForKeys(user, keys, options, ignoreDefaultOptions, DecisionPath.WITH_CMAB);
15011498
}
15021499

15031500
Map<String, OptimizelyDecision> decideAll(@Nonnull OptimizelyUserContext user,
@@ -1585,7 +1582,7 @@ private Map<String, OptimizelyDecision> decideForKeysSync(@Nonnull OptimizelyUse
15851582
@Nonnull List<String> keys,
15861583
@Nonnull List<OptimizelyDecideOption> options,
15871584
boolean ignoreDefaultOptions) {
1588-
return decideForKeys(user, keys, options, ignoreDefaultOptions, false);
1585+
return decideForKeys(user, keys, options, ignoreDefaultOptions, DecisionPath.WITHOUT_CMAB);
15891586
}
15901587

15911588

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
package com.optimizely.ab.bucketing;
2+
3+
public enum DecisionPath {
4+
WITH_CMAB, // Use CMAB logic
5+
WITHOUT_CMAB // Skip CMAB logic (traditional A/B testing)
6+
}

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

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public DecisionService(@Nonnull Bucketer bucketer,
106106
* @param options An array of decision options
107107
* @param userProfileTracker tracker for reading and updating user profile of the user
108108
* @param reasons Decision reasons
109-
* @param useCmab Boolean flag to determine if cmab service is to be used
109+
* @param decisionPath An enum of paths for decision-making logic
110110
* @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons
111111
*/
112112
@Nonnull
@@ -116,7 +116,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
116116
@Nonnull List<OptimizelyDecideOption> options,
117117
@Nullable UserProfileTracker userProfileTracker,
118118
@Nullable DecisionReasons reasons,
119-
@Nonnull boolean useCmab) {
119+
@Nonnull DecisionPath decisionPath) {
120120
if (reasons == null) {
121121
reasons = DefaultDecisionReasons.newInstance();
122122
}
@@ -158,7 +158,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
158158
if (decisionMeetAudience.getResult()) {
159159
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
160160
String cmabUUID = null;
161-
if (useCmab && isCmabExperiment(experiment)) {
161+
if (decisionPath == DecisionPath.WITH_CMAB && isCmabExperiment(experiment)) {
162162
DecisionResponse<CmabDecision> cmabDecision = getDecisionForCmabExperiment(projectConfig, experiment, user, bucketingId, options);
163163
reasons.merge(cmabDecision.getReasons());
164164

@@ -202,15 +202,15 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
202202
* @param user The current OptimizelyUserContext
203203
* @param projectConfig The current projectConfig
204204
* @param options An array of decision options
205-
* @param useCmab Boolean to check if cmab service is to be used.
205+
* @param decisionPath An enum of paths for decision-making logic
206206
* @return A {@link DecisionResponse} including the {@link Variation} that user is bucketed into (or null) and the decision reasons
207207
*/
208208
@Nonnull
209209
public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
210210
@Nonnull OptimizelyUserContext user,
211211
@Nonnull ProjectConfig projectConfig,
212212
@Nonnull List<OptimizelyDecideOption> options,
213-
@Nonnull boolean useCmab) {
213+
@Nonnull DecisionPath decisionPath) {
214214
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
215215

216216
// fetch the user profile map from the user profile service
@@ -222,7 +222,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
222222
userProfileTracker.loadUserProfile(reasons, errorHandler);
223223
}
224224

225-
DecisionResponse<Variation> response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons, useCmab);
225+
DecisionResponse<Variation> response = getVariation(experiment, user, projectConfig, options, userProfileTracker, reasons, decisionPath);
226226

227227
if(userProfileService != null && !ignoreUPS) {
228228
userProfileTracker.saveUserProfile(errorHandler);
@@ -234,7 +234,7 @@ public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
234234
public DecisionResponse<Variation> getVariation(@Nonnull Experiment experiment,
235235
@Nonnull OptimizelyUserContext user,
236236
@Nonnull ProjectConfig projectConfig) {
237-
return getVariation(experiment, user, projectConfig, Collections.emptyList(), true);
237+
return getVariation(experiment, user, projectConfig, Collections.emptyList(), DecisionPath.WITH_CMAB);
238238
}
239239

240240
/**
@@ -268,7 +268,7 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
268268
@Nonnull OptimizelyUserContext user,
269269
@Nonnull ProjectConfig projectConfig,
270270
@Nonnull List<OptimizelyDecideOption> options) {
271-
return getVariationsForFeatureList(featureFlags, user, projectConfig, options, true);
271+
return getVariationsForFeatureList(featureFlags, user, projectConfig, options, DecisionPath.WITH_CMAB);
272272
}
273273

274274
/**
@@ -278,15 +278,15 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
278278
* @param user The current OptimizelyuserContext
279279
* @param projectConfig The current projectConfig
280280
* @param options An array of decision options
281-
* @param useCmab Boolean field that determines whether to use cmab service
281+
* @param decisionPath An enum of paths for decision-making logic
282282
* @return A {@link DecisionResponse} including a {@link FeatureDecision} and the decision reasons
283283
*/
284284
@Nonnull
285285
public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Nonnull List<FeatureFlag> featureFlags,
286286
@Nonnull OptimizelyUserContext user,
287287
@Nonnull ProjectConfig projectConfig,
288288
@Nonnull List<OptimizelyDecideOption> options,
289-
@Nonnull boolean useCmab) {
289+
@Nonnull DecisionPath decisionPath) {
290290
DecisionReasons upsReasons = DefaultDecisionReasons.newInstance();
291291

292292
boolean ignoreUPS = options.contains(OptimizelyDecideOption.IGNORE_USER_PROFILE_SERVICE);
@@ -315,7 +315,7 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
315315
}
316316
}
317317

318-
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker, useCmab);
318+
DecisionResponse<FeatureDecision> decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker, decisionPath);
319319
reasons.merge(decisionVariationResponse.getReasons());
320320

321321
FeatureDecision decision = decisionVariationResponse.getResult();
@@ -371,14 +371,14 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
371371
@Nonnull OptimizelyUserContext user,
372372
@Nonnull List<OptimizelyDecideOption> options,
373373
@Nullable UserProfileTracker userProfileTracker,
374-
@Nonnull boolean useCmab) {
374+
@Nonnull DecisionPath decisionPath) {
375375
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
376376
if (!featureFlag.getExperimentIds().isEmpty()) {
377377
for (String experimentId : featureFlag.getExperimentIds()) {
378378
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
379379

380380
DecisionResponse<Variation> decisionVariation =
381-
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, useCmab);
381+
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
382382
reasons.merge(decisionVariation.getReasons());
383383
Variation variation = decisionVariation.getResult();
384384
String cmabUUID = decisionVariation.getCmabUUID();
@@ -810,7 +810,7 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
810810
@Nonnull OptimizelyUserContext user,
811811
@Nonnull List<OptimizelyDecideOption> options,
812812
@Nullable UserProfileTracker userProfileTracker,
813-
@Nonnull boolean useCmab) {
813+
@Nonnull DecisionPath decisionPath) {
814814
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
815815

816816
String ruleKey = rule != null ? rule.getKey() : null;
@@ -825,7 +825,7 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
825825
return new DecisionResponse(variation, reasons);
826826
}
827827
//regular decision
828-
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, useCmab);
828+
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
829829
reasons.merge(decisionResponse.getReasons());
830830

831831
variation = decisionResponse.getResult();

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@
6363
import com.google.gson.JsonElement;
6464
import com.google.gson.JsonParser;
6565
import com.optimizely.ab.bucketing.Bucketer;
66+
import com.optimizely.ab.bucketing.DecisionPath;
6667
import com.optimizely.ab.bucketing.DecisionService;
6768
import com.optimizely.ab.bucketing.FeatureDecision;
6869
import com.optimizely.ab.config.Attribute;
@@ -5117,7 +5118,7 @@ public void testDecideReturnsErrorDecisionWhenDecisionServiceFails() throws Exce
51175118
any(OptimizelyUserContext.class),
51185119
any(ProjectConfig.class),
51195120
any(List.class),
5120-
eq(true)
5121+
eq(DecisionPath.WITH_CMAB)
51215122
)).thenReturn(Arrays.asList(errorDecisionResponse));
51225123

51235124

‎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
@@ -361,7 +361,7 @@ public void getVariationForFeatureReturnsNullWhenItGetsNoVariationsForExperiment
361361
anyObject(),
362362
anyObject(),
363363
any(DecisionReasons.class),
364-
anyBoolean()
364+
any(DecisionPath.class)
365365
);
366366
// do not bucket to any rollouts
367367
doReturn(DecisionResponse.responseNoReasons(new FeatureDecision(null, null, null))).when(decisionService).getVariationForFeatureInRollout(
@@ -401,15 +401,15 @@ public void getVariationForFeatureReturnsVariationReturnedFromGetVariation() {
401401
any(OptimizelyUserContext.class),
402402
any(ProjectConfig.class),
403403
anyObject(),
404-
anyBoolean()
404+
any(DecisionPath.class)
405405
);
406406

407407
doReturn(DecisionResponse.responseNoReasons(ValidProjectConfigV4.VARIATION_MUTEX_GROUP_EXP_2_VAR_1)).when(decisionService).getVariation(
408408
eq(ValidProjectConfigV4.EXPERIMENT_MUTEX_GROUP_EXPERIMENT_2),
409409
any(OptimizelyUserContext.class),
410410
any(ProjectConfig.class),
411411
anyObject(),
412-
anyBoolean()
412+
any(DecisionPath.class)
413413
);
414414

415415
FeatureDecision featureDecision = decisionService.getVariationForFeature(
@@ -450,7 +450,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout()
450450
anyObject(),
451451
anyObject(),
452452
any(DecisionReasons.class),
453-
anyBoolean()
453+
any(DecisionPath.class)
454454
);
455455

456456
// return variation for rollout
@@ -485,7 +485,7 @@ public void getVariationForFeatureReturnsVariationFromExperimentBeforeRollout()
485485
anyObject(),
486486
anyObject(),
487487
any(DecisionReasons.class),
488-
anyBoolean()
488+
any(DecisionPath.class)
489489
);
490490
}
491491

@@ -513,7 +513,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails
513513
anyObject(),
514514
anyObject(),
515515
any(DecisionReasons.class),
516-
anyBoolean()
516+
any(DecisionPath.class)
517517
);
518518

519519
// return variation for rollout
@@ -548,7 +548,7 @@ public void getVariationForFeatureReturnsVariationFromRolloutWhenExperimentFails
548548
anyObject(),
549549
anyObject(),
550550
any(DecisionReasons.class),
551-
anyBoolean()
551+
any(DecisionPath.class)
552552
);
553553

554554
logbackVerifier.expectMessage(

0 commit comments

Comments
 (0)