Skip to content

Commit c37a6d6

Browse files
Mat001claude
Mat001
and
claude
committed
[FSSDK-12368] Implement Local Holdouts support
Add Local Holdouts support to replace legacy flag-level holdouts with rule-level targeting. Changes: - Add includedRules field to Holdout class (replaces includedFlags/excludedFlags) - Add isGlobal() method for global vs local holdout detection - Update HoldoutConfig mapping from flag-level to rule-level - Implement getGlobalHoldouts() and getHoldoutsForRule(ruleId) methods - Integrate local holdout evaluation in decision flow (per-rule, before audience/traffic) - Handle edge cases (missing field, empty list, invalid rule IDs, cross-flag targeting) - Add comprehensive unit tests for local holdouts (32 test cases) Quality Metrics: - Tests: 32 comprehensive test cases - Critical Issues: 0 - Warnings: 0 Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
1 parent e3e689e commit c37a6d6

8 files changed

Lines changed: 661 additions & 228 deletions

File tree

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

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -325,9 +325,10 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
325325
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
326326
reasons.merge(upsReasons);
327327

328-
List<Holdout> holdouts = projectConfig.getHoldoutForFlag(featureFlag.getId());
329-
if (!holdouts.isEmpty()) {
330-
for (Holdout holdout : holdouts) {
328+
// Evaluate global holdouts at flag level
329+
List<Holdout> globalHoldouts = projectConfig.getGlobalHoldouts();
330+
if (!globalHoldouts.isEmpty()) {
331+
for (Holdout holdout : globalHoldouts) {
331332
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
332333
reasons.merge(holdoutDecision.getReasons());
333334
if (holdoutDecision.getResult() != null) {
@@ -846,6 +847,20 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
846847
if (variation != null) {
847848
return new DecisionResponse(variation, reasons);
848849
}
850+
851+
// Check local holdouts targeting this rule
852+
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.getId());
853+
if (!localHoldouts.isEmpty()) {
854+
for (Holdout holdout : localHoldouts) {
855+
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
856+
reasons.merge(holdoutDecision.getReasons());
857+
if (holdoutDecision.getResult() != null) {
858+
// User is in holdout - return holdout variation immediately, skip this rule
859+
return new DecisionResponse(holdoutDecision.getResult(), reasons);
860+
}
861+
}
862+
}
863+
849864
//regular decision
850865
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
851866
reasons.merge(decisionResponse.getReasons());
@@ -896,6 +911,20 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
896911
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
897912
}
898913

914+
// Check local holdouts targeting this delivery rule
915+
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.getId());
916+
if (!localHoldouts.isEmpty()) {
917+
for (Holdout holdout : localHoldouts) {
918+
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
919+
reasons.merge(holdoutDecision.getReasons());
920+
if (holdoutDecision.getResult() != null) {
921+
// User is in holdout - return holdout variation, skip this delivery rule
922+
variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(holdoutDecision.getResult(), skipToEveryoneElse);
923+
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
924+
}
925+
}
926+
}
927+
899928
// Handle a regular decision
900929
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
901930
Boolean everyoneElse = (ruleIndex == rules.size() - 1);

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

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -571,11 +571,16 @@ public List<Holdout> getHoldouts() {
571571
}
572572

573573
@Override
574-
public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
575-
return holdoutConfig.getHoldoutForFlag(id);
574+
public List<Holdout> getGlobalHoldouts() {
575+
return holdoutConfig.getGlobalHoldouts();
576576
}
577577

578-
@Override
578+
@Override
579+
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
580+
return holdoutConfig.getHoldoutsForRule(ruleId);
581+
}
582+
583+
@Override
579584
public Holdout getHoldout(@Nonnull String id) {
580585
return holdoutConfig.getHoldout(id);
581586
}

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

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,8 +43,7 @@ public class Holdout implements ExperimentCore {
4343
private final Condition<AudienceIdCondition> audienceConditions;
4444
private final List<Variation> variations;
4545
private final List<TrafficAllocation> trafficAllocation;
46-
private final List<String> includedFlags;
47-
private final List<String> excludedFlags;
46+
private final List<String> includedRules;
4847

4948
private final Map<String, Variation> variationKeyToVariationMap;
5049
private final Map<String, Variation> variationIdToVariationMap;
@@ -70,7 +69,7 @@ public String toString() {
7069

7170
@VisibleForTesting
7271
public Holdout(String id, String key) {
73-
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null, null);
72+
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null);
7473
}
7574

7675
// Keep only this constructor and add @JsonCreator to it
@@ -82,17 +81,15 @@ public Holdout(@JsonProperty("id") @Nonnull String id,
8281
@JsonProperty("audienceConditions") @Nullable Condition audienceConditions,
8382
@JsonProperty("variations") @Nonnull List<Variation> variations,
8483
@JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation,
85-
@JsonProperty("includedFlags") @Nullable List<String> includedFlags,
86-
@JsonProperty("excludedFlags") @Nullable List<String> excludedFlags) {
84+
@JsonProperty(value = "includedRules", required = false) @Nullable List<String> includedRules) {
8785
this.id = id;
8886
this.key = key;
8987
this.status = status;
9088
this.audienceIds = audienceIds;
9189
this.audienceConditions = audienceConditions;
9290
this.variations = variations;
9391
this.trafficAllocation = trafficAllocation;
94-
this.includedFlags = includedFlags == null ? Collections.emptyList() : Collections.unmodifiableList(includedFlags);
95-
this.excludedFlags = excludedFlags == null ? Collections.emptyList() : Collections.unmodifiableList(excludedFlags);
92+
this.includedRules = includedRules;
9693
this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(this.variations);
9794
this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(this.variations);
9895
}
@@ -141,12 +138,12 @@ public String getGroupId() {
141138
return "";
142139
}
143140

144-
public List<String> getIncludedFlags() {
145-
return includedFlags;
141+
public List<String> getIncludedRules() {
142+
return includedRules;
146143
}
147144

148-
public List<String> getExcludedFlags() {
149-
return excludedFlags;
145+
public boolean isGlobal() {
146+
return includedRules == null;
150147
}
151148

152149
public boolean isActive() {

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

Lines changed: 30 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -35,11 +35,9 @@
3535
*/
3636
public class HoldoutConfig {
3737
private List<Holdout> allHoldouts;
38-
private List<Holdout> global;
38+
private List<Holdout> globalHoldouts;
3939
private Map<String, Holdout> holdoutIdMap;
40-
private Map<String, List<Holdout>> flagHoldoutsMap;
41-
private Map<String, List<Holdout>> includedHoldouts;
42-
private Map<String, Set<Holdout>> excludedHoldouts;
40+
private Map<String, List<Holdout>> ruleHoldoutsMap;
4341

4442
/**
4543
* Initializes a new HoldoutConfig with an empty list of holdouts.
@@ -55,91 +53,59 @@ public HoldoutConfig() {
5553
*/
5654
public HoldoutConfig(@Nonnull List<Holdout> allHoldouts) {
5755
this.allHoldouts = new ArrayList<>(allHoldouts);
58-
this.global = new ArrayList<>();
56+
this.globalHoldouts = new ArrayList<>();
5957
this.holdoutIdMap = new HashMap<>();
60-
this.flagHoldoutsMap = new ConcurrentHashMap<>();
61-
this.includedHoldouts = new HashMap<>();
62-
this.excludedHoldouts = new HashMap<>();
58+
this.ruleHoldoutsMap = new HashMap<>();
6359
updateHoldoutMapping();
6460
}
6561

6662
/**
6763
* Updates internal mappings of holdouts including the id map, global list,
68-
* and per-flag inclusion/exclusion maps.
64+
* and per-rule holdout maps.
6965
*/
7066
private void updateHoldoutMapping() {
7167
holdoutIdMap.clear();
7268
for (Holdout holdout : allHoldouts) {
7369
holdoutIdMap.put(holdout.getId(), holdout);
7470
}
7571

76-
flagHoldoutsMap.clear();
77-
global.clear();
78-
includedHoldouts.clear();
79-
excludedHoldouts.clear();
72+
globalHoldouts.clear();
73+
ruleHoldoutsMap.clear();
8074

8175
for (Holdout holdout : allHoldouts) {
82-
boolean hasIncludedFlags = !holdout.getIncludedFlags().isEmpty();
83-
boolean hasExcludedFlags = !holdout.getExcludedFlags().isEmpty();
84-
85-
if (!hasIncludedFlags && !hasExcludedFlags) {
86-
// Global holdout (applies to all flags)
87-
global.add(holdout);
88-
} else if (hasIncludedFlags) {
89-
// Holdout only applies to specific included flags
90-
for (String flagId : holdout.getIncludedFlags()) {
91-
includedHoldouts.computeIfAbsent(flagId, k -> new ArrayList<>()).add(holdout);
92-
}
76+
if (holdout.isGlobal()) {
77+
// Global holdout (includedRules == null, applies to all rules)
78+
globalHoldouts.add(holdout);
9379
} else {
94-
// Global holdout with specific exclusions
95-
global.add(holdout);
96-
97-
for (String flagId : holdout.getExcludedFlags()) {
98-
excludedHoldouts.computeIfAbsent(flagId, k -> new HashSet<>()).add(holdout);
80+
// Local holdout (includedRules != null, applies to specific rules)
81+
List<String> includedRules = holdout.getIncludedRules();
82+
if (includedRules != null) {
83+
for (String ruleId : includedRules) {
84+
ruleHoldoutsMap.computeIfAbsent(ruleId, k -> new ArrayList<>()).add(holdout);
85+
}
9986
}
10087
}
10188
}
10289
}
10390

10491
/**
105-
* Returns the applicable holdouts for the given flag ID by combining global holdouts
106-
* (excluding any specified) and included holdouts, in that order.
107-
* Caches the result for future calls.
92+
* Returns global holdouts that apply to all rules.
10893
*
109-
* @param id The flag identifier
110-
* @return A list of Holdout objects relevant to the given flag
94+
* @return A list of global Holdout objects
11195
*/
112-
public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
113-
if (allHoldouts.isEmpty()) {
114-
return Collections.emptyList();
115-
}
116-
117-
// Check cache and return persistent holdouts
118-
if (flagHoldoutsMap.containsKey(id)) {
119-
return flagHoldoutsMap.get(id);
120-
}
121-
122-
// Prioritize global holdouts first
123-
List<Holdout> activeHoldouts = new ArrayList<>();
124-
Set<Holdout> excluded = excludedHoldouts.getOrDefault(id, Collections.emptySet());
125-
126-
if (!excluded.isEmpty()) {
127-
for (Holdout holdout : global) {
128-
if (!excluded.contains(holdout)) {
129-
activeHoldouts.add(holdout);
130-
}
131-
}
132-
} else {
133-
activeHoldouts.addAll(global);
134-
}
135-
136-
// Add included holdouts
137-
activeHoldouts.addAll(includedHoldouts.getOrDefault(id, Collections.emptyList()));
138-
139-
// Cache the result
140-
flagHoldoutsMap.put(id, activeHoldouts);
96+
public List<Holdout> getGlobalHoldouts() {
97+
return Collections.unmodifiableList(globalHoldouts);
98+
}
14199

142-
return activeHoldouts;
100+
/**
101+
* Returns local holdouts that target a specific rule.
102+
*
103+
* @param ruleId The rule identifier
104+
* @return A list of Holdout objects targeting the specified rule
105+
*/
106+
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
107+
List<Holdout> holdouts = ruleHoldoutsMap.get(ruleId);
108+
return holdouts == null ? Collections.emptyList() : Collections.unmodifiableList(holdouts);
143109
}
144110

145111
/**

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ Experiment getExperimentForKey(@Nonnull String experimentKey,
7373

7474
List<Holdout > getHoldouts();
7575

76-
List<Holdout> getHoldoutForFlag(@Nonnull String id);
76+
List<Holdout> getGlobalHoldouts();
77+
78+
List<Holdout> getHoldoutsForRule(@Nonnull String ruleId);
7779

7880
Holdout getHoldout(@Nonnull String id);
7981

0 commit comments

Comments
 (0)