Skip to content

Commit 0369d44

Browse files
authored
[AI-FSSDK] [FSSDK-12369] Add local holdouts support (#628)
1 parent 5752354 commit 0369d44

13 files changed

Lines changed: 828 additions & 118 deletions

File tree

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

Lines changed: 75 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import com.optimizely.ab.OptimizelyRuntimeException;
3737
import com.optimizely.ab.OptimizelyUserContext;
3838
import com.optimizely.ab.config.Experiment;
39+
import com.optimizely.ab.config.ExperimentCore;
3940
import com.optimizely.ab.config.FeatureFlag;
4041
import com.optimizely.ab.config.Holdout;
4142
import com.optimizely.ab.config.ProjectConfig;
@@ -325,9 +326,10 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
325326
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
326327
reasons.merge(upsReasons);
327328

328-
List<Holdout> holdouts = projectConfig.getHoldoutForFlag(featureFlag.getId());
329-
if (!holdouts.isEmpty()) {
330-
for (Holdout holdout : holdouts) {
329+
// Evaluate global holdouts at flag level (before any rules are iterated)
330+
List<Holdout> globalHoldouts = projectConfig.getGlobalHoldouts();
331+
if (!globalHoldouts.isEmpty()) {
332+
for (Holdout holdout : globalHoldouts) {
331333
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
332334
reasons.merge(holdoutDecision.getReasons());
333335
if (holdoutDecision.getResult() != null) {
@@ -395,33 +397,22 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
395397
@Nullable UserProfileTracker userProfileTracker,
396398
@Nonnull DecisionPath decisionPath) {
397399
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
400+
// Cache flagKey once to avoid multiple getKey() calls (important for mock-based tests)
401+
String flagKey = featureFlag.getKey();
398402
if (!featureFlag.getExperimentIds().isEmpty()) {
399403
for (String experimentId : featureFlag.getExperimentIds()) {
400404
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);
401405

402-
DecisionResponse<Variation> decisionVariation =
403-
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
406+
DecisionResponse<FeatureDecision> decisionVariation =
407+
getVariationFromExperimentRule(projectConfig, flagKey, experiment, user, options, userProfileTracker, decisionPath);
404408
reasons.merge(decisionVariation.getReasons());
405-
Variation variation = decisionVariation.getResult();
406-
String cmabUuid = decisionVariation.getCmabUuid();
407-
boolean error = decisionVariation.isError();
408-
if (error) {
409-
return new DecisionResponse(
410-
new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST, cmabUuid),
411-
reasons,
412-
decisionVariation.isError(),
413-
cmabUuid);
414-
}
415-
if (variation != null) {
416-
return new DecisionResponse(
417-
new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST, cmabUuid),
418-
reasons,
419-
decisionVariation.isError(),
420-
cmabUuid);
409+
FeatureDecision featureDecision = decisionVariation.getResult();
410+
if (decisionVariation.isError() || (featureDecision != null && featureDecision.variation != null)) {
411+
return new DecisionResponse(featureDecision, reasons, decisionVariation.isError(), decisionVariation.getCmabUuid());
421412
}
422413
}
423414
} else {
424-
String message = reasons.addInfo("The feature flag \"%s\" is not used in any experiments.", featureFlag.getKey());
415+
String message = reasons.addInfo("The feature flag \"%s\" is not used in any experiments.", flagKey);
425416
logger.info(message);
426417
}
427418

@@ -468,6 +459,7 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
468459

469460
int index = 0;
470461
while (index < rolloutRulesLength) {
462+
Experiment rolloutRule = rollout.getExperiments().get(index);
471463

472464
DecisionResponse<AbstractMap.SimpleEntry> decisionVariationResponse = getVariationFromDeliveryRule(
473465
projectConfig,
@@ -478,12 +470,10 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
478470
);
479471
reasons.merge(decisionVariationResponse.getReasons());
480472

481-
AbstractMap.SimpleEntry<Variation, Boolean> response = decisionVariationResponse.getResult();
482-
Variation variation = response.getKey();
473+
AbstractMap.SimpleEntry<FeatureDecision, Boolean> response = decisionVariationResponse.getResult();
474+
FeatureDecision featureDecision = response.getKey();
483475
Boolean skipToEveryoneElse = response.getValue();
484-
if (variation != null) {
485-
Experiment rule = rollout.getExperiments().get(index);
486-
FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT);
476+
if (featureDecision != null) {
487477
return new DecisionResponse(featureDecision, reasons);
488478
}
489479

@@ -714,6 +704,23 @@ public DecisionResponse<Variation> validatedForcedDecision(@Nonnull OptimizelyDe
714704
return new DecisionResponse<>(null, reasons);
715705
}
716706

707+
DecisionResponse<FeatureDecision> evaluateLocalHoldouts(@Nonnull ExperimentCore rule,
708+
@Nonnull ProjectConfig projectConfig,
709+
@Nonnull OptimizelyUserContext user) {
710+
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
711+
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.getId());
712+
for (Holdout holdout : localHoldouts) {
713+
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
714+
reasons.merge(holdoutDecision.getReasons());
715+
if (holdoutDecision.getResult() != null) {
716+
return new DecisionResponse<>(
717+
new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT),
718+
reasons);
719+
}
720+
}
721+
return new DecisionResponse<>(null, reasons);
722+
}
723+
717724
public ConcurrentHashMap<String, ConcurrentHashMap<String, String>> getForcedVariationMapping() {
718725
return forcedVariationMapping;
719726
}
@@ -826,7 +833,7 @@ public DecisionResponse<Variation> getForcedVariation(@Nonnull Experiment experi
826833
}
827834

828835

829-
private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull ProjectConfig projectConfig,
836+
private DecisionResponse<FeatureDecision> getVariationFromExperimentRule(@Nonnull ProjectConfig projectConfig,
830837
@Nonnull String flagKey,
831838
@Nonnull Experiment rule,
832839
@Nonnull OptimizelyUserContext user,
@@ -836,23 +843,37 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
836843
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
837844

838845
String ruleKey = rule != null ? rule.getKey() : null;
839-
// Check Forced-Decision
846+
// Step 1: Check Forced-Decision
840847
OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, ruleKey);
841848
DecisionResponse<Variation> forcedDecisionResponse = validatedForcedDecision(optimizelyDecisionContext, projectConfig, user);
842849

843850
reasons.merge(forcedDecisionResponse.getReasons());
844851

845852
Variation variation = forcedDecisionResponse.getResult();
846853
if (variation != null) {
847-
return new DecisionResponse(variation, reasons);
854+
return new DecisionResponse(
855+
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.FEATURE_TEST),
856+
reasons);
857+
}
858+
859+
// Step 2: Check local holdouts
860+
if (rule != null) {
861+
DecisionResponse<FeatureDecision> holdoutResponse = evaluateLocalHoldouts(rule, projectConfig, user);
862+
reasons.merge(holdoutResponse.getReasons());
863+
if (holdoutResponse.getResult() != null) {
864+
return new DecisionResponse<>(holdoutResponse.getResult(), reasons);
865+
}
848866
}
849-
//regular decision
867+
868+
// Step 3: Regular rule decision
850869
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
851870
reasons.merge(decisionResponse.getReasons());
852871

853872
variation = decisionResponse.getResult();
854873

855-
return new DecisionResponse<>(variation, reasons, decisionResponse.isError(), decisionResponse.getCmabUuid());
874+
return new DecisionResponse<>(
875+
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.FEATURE_TEST, decisionResponse.getCmabUuid()),
876+
reasons, decisionResponse.isError(), decisionResponse.getCmabUuid());
856877
}
857878

858879
/**
@@ -872,8 +893,8 @@ private boolean validateUserId(String userId) {
872893
* @param rules The experiments belonging to a rollout
873894
* @param ruleIndex The index of the rule
874895
* @param user The OptimizelyUserContext
875-
* @return Returns a DecisionResponse Object containing a AbstractMap.SimpleEntry<Variation, Boolean>
876-
* where the Variation is the result and the Boolean is the skipToEveryoneElse.
896+
* @return Returns a DecisionResponse Object containing a AbstractMap.SimpleEntry<FeatureDecision, Boolean>
897+
* where the FeatureDecision is the result and the Boolean is the skipToEveryoneElse.
877898
*/
878899
DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull ProjectConfig projectConfig,
879900
@Nonnull String flagKey,
@@ -883,20 +904,30 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
883904
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
884905

885906
Boolean skipToEveryoneElse = false;
886-
AbstractMap.SimpleEntry<Variation, Boolean> variationToSkipToEveryoneElsePair;
887-
// Check forced-decisions first
907+
AbstractMap.SimpleEntry<FeatureDecision, Boolean> resultPair;
888908
Experiment rule = rules.get(ruleIndex);
909+
910+
// Step 1: Check Forced-Decision
889911
OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, rule.getKey());
890912
DecisionResponse<Variation> forcedDecisionResponse = validatedForcedDecision(optimizelyDecisionContext, projectConfig, user);
891913
reasons.merge(forcedDecisionResponse.getReasons());
892914

893915
Variation variation = forcedDecisionResponse.getResult();
894916
if (variation != null) {
895-
variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(variation, false);
896-
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
917+
resultPair = new AbstractMap.SimpleEntry<>(
918+
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT), false);
919+
return new DecisionResponse(resultPair, reasons);
920+
}
921+
922+
// Step 2: Check local holdouts
923+
DecisionResponse<FeatureDecision> holdoutResponse = evaluateLocalHoldouts(rule, projectConfig, user);
924+
reasons.merge(holdoutResponse.getReasons());
925+
if (holdoutResponse.getResult() != null) {
926+
resultPair = new AbstractMap.SimpleEntry<>(holdoutResponse.getResult(), false);
927+
return new DecisionResponse(resultPair, reasons);
897928
}
898929

899-
// Handle a regular decision
930+
// Step 3: Regular rule decision
900931
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
901932
Boolean everyoneElse = (ruleIndex == rules.size() - 1);
902933
String loggingKey = everyoneElse ? "Everyone Else" : String.valueOf(ruleIndex + 1);
@@ -938,8 +969,11 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
938969
reasons.addInfo(message);
939970
logger.debug(message);
940971
}
941-
variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(bucketedVariation, skipToEveryoneElse);
942-
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
972+
FeatureDecision featureDecision = bucketedVariation != null
973+
? new FeatureDecision(rule, bucketedVariation, FeatureDecision.DecisionSource.ROLLOUT)
974+
: null;
975+
resultPair = new AbstractMap.SimpleEntry<>(featureDecision, skipToEveryoneElse);
976+
return new DecisionResponse(resultPair, reasons);
943977
}
944978

945979
/**

core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -575,7 +575,17 @@ public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
575575
return holdoutConfig.getHoldoutForFlag(id);
576576
}
577577

578-
@Override
578+
@Override
579+
public List<Holdout> getGlobalHoldouts() {
580+
return holdoutConfig.getGlobalHoldouts();
581+
}
582+
583+
@Override
584+
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
585+
return holdoutConfig.getHoldoutsForRule(ruleId);
586+
}
587+
588+
@Override
579589
public Holdout getHoldout(@Nonnull String id) {
580590
return holdoutConfig.getHoldout(id);
581591
}

core-api/src/main/java/com/optimizely/ab/config/Holdout.java

Lines changed: 56 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
22
*
3-
* Copyright 2016-2019, 2021, Optimizely and contributors
3+
* Copyright 2016-2019, 2021, 2026, Optimizely and contributors
44
*
55
* Licensed under the Apache License, Version 2.0 (the "License");
66
* you may not use this file except in compliance with the License.
@@ -38,12 +38,20 @@ public class Holdout implements ExperimentCore {
3838
private final String id;
3939
private final String key;
4040
private final String status;
41-
41+
4242
private final List<String> audienceIds;
4343
private final Condition<AudienceIdCondition> audienceConditions;
4444
private final List<Variation> variations;
4545
private final List<TrafficAllocation> trafficAllocation;
4646

47+
/**
48+
* Optional list of rule IDs this holdout targets. When null, the holdout is global
49+
* (applies to all rules across all flags). When non-null (even empty), it is a local
50+
* holdout that only applies to the specified rule IDs.
51+
*/
52+
@Nullable
53+
private final List<String> includedRules;
54+
4755
private final Map<String, Variation> variationKeyToVariationMap;
4856
private final Map<String, Variation> variationIdToVariationMap;
4957
// Not necessary for HO
@@ -68,25 +76,45 @@ public String toString() {
6876

6977
@VisibleForTesting
7078
public Holdout(String id, String key) {
71-
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList());
72-
}
73-
74-
// Keep only this constructor and add @JsonCreator to it
79+
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null);
80+
}
81+
82+
/**
83+
* Constructor without includedRules (backward-compatible — treated as global holdout).
84+
*/
85+
public Holdout(@Nonnull String id,
86+
@Nonnull String key,
87+
@Nonnull String status,
88+
@Nonnull List<String> audienceIds,
89+
@Nullable Condition audienceConditions,
90+
@Nonnull List<Variation> variations,
91+
@Nonnull List<TrafficAllocation> trafficAllocation) {
92+
this(id, key, status, audienceIds, audienceConditions, variations, trafficAllocation, null);
93+
}
94+
95+
/**
96+
* Full constructor including optional includedRules field (used by parsers).
97+
*
98+
* @param includedRules null = global holdout (applies to all rules); non-null list = local holdout
99+
* targeting only those rule IDs (empty list = local holdout with no matching rules)
100+
*/
75101
@JsonCreator
76102
public Holdout(@JsonProperty("id") @Nonnull String id,
77103
@JsonProperty("key") @Nonnull String key,
78104
@JsonProperty("status") @Nonnull String status,
79105
@JsonProperty("audienceIds") @Nonnull List<String> audienceIds,
80106
@JsonProperty("audienceConditions") @Nullable Condition audienceConditions,
81107
@JsonProperty("variations") @Nonnull List<Variation> variations,
82-
@JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation) {
108+
@JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation,
109+
@JsonProperty("includedRules") @Nullable List<String> includedRules) {
83110
this.id = id;
84111
this.key = key;
85112
this.status = status;
86113
this.audienceIds = audienceIds;
87114
this.audienceConditions = audienceConditions;
88115
this.variations = variations;
89116
this.trafficAllocation = trafficAllocation;
117+
this.includedRules = includedRules;
90118
this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(this.variations);
91119
this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(this.variations);
92120
}
@@ -143,6 +171,26 @@ public boolean isRunning() {
143171
return status.equals(Holdout.HoldoutStatus.RUNNING.toString());
144172
}
145173

174+
/**
175+
* Returns the list of rule IDs this holdout targets, or null if this is a global holdout.
176+
*
177+
* @return null for global holdouts; a (possibly empty) list of rule IDs for local holdouts
178+
*/
179+
@Nullable
180+
public List<String> getIncludedRules() {
181+
return includedRules;
182+
}
183+
184+
/**
185+
* Returns true if this holdout is global (applies to all rules across all flags).
186+
* A holdout is global when includedRules is null.
187+
*
188+
* @return true if this is a global holdout, false if it is a local holdout
189+
*/
190+
public boolean isGlobal() {
191+
return includedRules == null;
192+
}
193+
146194
@Override
147195
public String toString() {
148196
return "Holdout {"
@@ -154,6 +202,7 @@ public String toString() {
154202
+ ", variations=" + variations
155203
+ ", variationKeyToVariationMap=" + variationKeyToVariationMap
156204
+ ", trafficAllocation=" + trafficAllocation
205+
+ ", includedRules=" + includedRules
157206
+ '}';
158207
}
159208
}

0 commit comments

Comments
 (0)