Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -325,15 +325,14 @@ public List<DecisionResponse<FeatureDecision>> getVariationsForFeatureList(@Non
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
reasons.merge(upsReasons);

List<Holdout> holdouts = projectConfig.getHoldoutForFlag(featureFlag.getId());
if (!holdouts.isEmpty()) {
for (Holdout holdout : holdouts) {
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
reasons.merge(holdoutDecision.getReasons());
if (holdoutDecision.getResult() != null) {
decisions.add(new DecisionResponse<>(new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT), reasons));
continue flagLoop;
}
// Evaluate global holdouts at the flag level (before any per-rule logic)
List<Holdout> globalHoldouts = projectConfig.getHoldoutConfig().getGlobalHoldouts();
for (Holdout holdout : globalHoldouts) {
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
reasons.merge(holdoutDecision.getReasons());
if (holdoutDecision.getResult() != null) {
decisions.add(new DecisionResponse<>(new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT), reasons));
continue flagLoop;
}
}

Expand Down Expand Up @@ -399,6 +398,20 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
for (String experimentId : featureFlag.getExperimentIds()) {
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);

// Check local holdouts targeting this experiment rule before regular evaluation (FSSDK-12369)
if (experiment != null) {
List<Holdout> localHoldouts = projectConfig.getHoldoutConfig().getHoldoutsForRule(experiment.getId());
for (Holdout holdout : localHoldouts) {
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
reasons.merge(holdoutDecision.getReasons());
if (holdoutDecision.getResult() != null) {
return new DecisionResponse<>(
new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT),
reasons);
}
}
}

DecisionResponse<Variation> decisionVariation =
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
reasons.merge(decisionVariation.getReasons());
Expand Down Expand Up @@ -468,6 +481,19 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu

int index = 0;
while (index < rolloutRulesLength) {
Experiment rule = rollout.getExperiments().get(index);

// Check local holdouts targeting this delivery rule before regular evaluation (FSSDK-12369)
List<Holdout> localHoldoutsForRule = projectConfig.getHoldoutConfig().getHoldoutsForRule(rule.getId());
boolean localHoldoutHit = false;
for (Holdout holdout : localHoldoutsForRule) {
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
reasons.merge(holdoutDecision.getReasons());
if (holdoutDecision.getResult() != null) {
FeatureDecision holdoutFeatureDecision = new FeatureDecision(holdout, holdoutDecision.getResult(), FeatureDecision.DecisionSource.HOLDOUT);
return new DecisionResponse(holdoutFeatureDecision, reasons);
}
}

DecisionResponse<AbstractMap.SimpleEntry> decisionVariationResponse = getVariationFromDeliveryRule(
projectConfig,
Expand All @@ -482,7 +508,6 @@ DecisionResponse<FeatureDecision> getVariationForFeatureInRollout(@Nonnull Featu
Variation variation = response.getKey();
Boolean skipToEveryoneElse = response.getValue();
if (variation != null) {
Experiment rule = rollout.getExperiments().get(index);
FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT);
return new DecisionResponse(featureDecision, reasons);
}
Expand Down Expand Up @@ -846,6 +871,7 @@ private DecisionResponse<Variation> getVariationFromExperimentRule(@Nonnull Proj
if (variation != null) {
return new DecisionResponse(variation, reasons);
}

//regular decision
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
reasons.merge(decisionResponse.getReasons());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,11 +575,16 @@ public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
return holdoutConfig.getHoldoutForFlag(id);
}

@Override
@Override
public Holdout getHoldout(@Nonnull String id) {
return holdoutConfig.getHoldout(id);
}

@Override
public HoldoutConfig getHoldoutConfig() {
return holdoutConfig;
}

@Override
public Set<String> getAllSegments() {
return this.allSegments;
Expand Down
52 changes: 48 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/config/Holdout.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,20 @@ public class Holdout implements ExperimentCore {
private final String id;
private final String key;
private final String status;

private final List<String> audienceIds;
private final Condition<AudienceIdCondition> audienceConditions;
private final List<Variation> variations;
private final List<TrafficAllocation> trafficAllocation;

/**
* Optional list of rule IDs this holdout targets.
* null = global holdout (applies to all rules across all flags)
* non-null = local holdout (applies only to the specified rule IDs)
*/
@Nullable
private final List<String> includedRules;

private final Map<String, Variation> variationKeyToVariationMap;
private final Map<String, Variation> variationIdToVariationMap;
// Not necessary for HO
Expand All @@ -68,25 +76,39 @@ public String toString() {

@VisibleForTesting
public Holdout(String id, String key) {
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList());
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null);
}

/**
* Convenience constructor for tests that do not need includedRules (maintains backward compatibility).
*/
public Holdout(@Nonnull String id,
@Nonnull String key,
@Nonnull String status,
@Nonnull List<String> audienceIds,
@Nullable Condition audienceConditions,
@Nonnull List<Variation> variations,
@Nonnull List<TrafficAllocation> trafficAllocation) {
this(id, key, status, audienceIds, audienceConditions, variations, trafficAllocation, null);
}

// Keep only this constructor and add @JsonCreator to it
@JsonCreator
public Holdout(@JsonProperty("id") @Nonnull String id,
@JsonProperty("key") @Nonnull String key,
@JsonProperty("status") @Nonnull String status,
@JsonProperty("audienceIds") @Nonnull List<String> audienceIds,
@JsonProperty("audienceConditions") @Nullable Condition audienceConditions,
@JsonProperty("variations") @Nonnull List<Variation> variations,
@JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation) {
@JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation,
@JsonProperty("includedRules") @Nullable List<String> includedRules) {
this.id = id;
this.key = key;
this.status = status;
this.audienceIds = audienceIds;
this.audienceConditions = audienceConditions;
this.variations = variations;
this.trafficAllocation = trafficAllocation;
this.includedRules = includedRules;
this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(this.variations);
this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(this.variations);
}
Expand Down Expand Up @@ -135,6 +157,27 @@ public String getGroupId() {
return "";
}

/**
* Returns the list of rule IDs this holdout is scoped to, or null if this is a global holdout.
*
* @return null for a global holdout; a (possibly empty) list of rule IDs for a local holdout
*/
@Nullable
public List<String> getIncludedRules() {
return includedRules;
}

/**
* Returns {@code true} if this is a global holdout (applies to all rules across all flags).
* A holdout is global when {@code includedRules} is {@code null}.
* An empty list is NOT the same as global — it is a local holdout that targets no rules.
*
* @return true if global, false if local
*/
public boolean isGlobal() {
return includedRules == null;
}

public boolean isActive() {
return status.equals(Holdout.HoldoutStatus.RUNNING.toString());
}
Expand All @@ -154,6 +197,7 @@ public String toString() {
+ ", variations=" + variations
+ ", variationKeyToVariationMap=" + variationKeyToVariationMap
+ ", trafficAllocation=" + trafficAllocation
+ ", includedRules=" + includedRules
+ '}';
}
}
69 changes: 62 additions & 7 deletions core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,22 @@
import javax.annotation.Nullable;

/**
* HoldoutConfig manages collections of Holdout objects.
* All holdouts are global and apply to all flags.
* HoldoutConfig manages collections of Holdout objects and provides access
* by global classification and by rule ID for local holdouts.
*/
public class HoldoutConfig {
private List<Holdout> allHoldouts;
private Map<String, Holdout> holdoutIdMap;

/** All holdouts whose {@code includedRules} is null (global holdouts). */
private List<Holdout> globalHoldouts;

/**
* Map from rule ID to the list of local holdouts targeting that rule.
* A holdout is local when its {@code includedRules} is non-null.
*/
private Map<String, List<Holdout>> ruleHoldoutsMap;

/**
* Initializes a new HoldoutConfig with an empty list of holdouts.
*/
Expand All @@ -50,28 +59,73 @@ public HoldoutConfig() {
public HoldoutConfig(@Nonnull List<Holdout> allHoldouts) {
this.allHoldouts = new ArrayList<>(allHoldouts);
this.holdoutIdMap = new HashMap<>();
this.globalHoldouts = new ArrayList<>();
this.ruleHoldoutsMap = new HashMap<>();
updateHoldoutMapping();
}

/**
* Updates internal mapping of holdout IDs to holdout objects.
* Rebuilds all internal mappings from the current {@code allHoldouts} list.
*
* <p>Global holdouts (includedRules == null) are collected into {@code globalHoldouts}.
* Local holdouts (includedRules != null) are indexed by each rule ID they target
* into {@code ruleHoldoutsMap}.
*/
private void updateHoldoutMapping() {
holdoutIdMap.clear();
globalHoldouts.clear();
ruleHoldoutsMap.clear();

for (Holdout holdout : allHoldouts) {
holdoutIdMap.put(holdout.getId(), holdout);

if (holdout.isGlobal()) {
globalHoldouts.add(holdout);
} else {
List<String> rules = holdout.getIncludedRules();
// rules is non-null here because isGlobal() returned false
for (String ruleId : rules) {
ruleHoldoutsMap.computeIfAbsent(ruleId, k -> new ArrayList<>()).add(holdout);
}
}
}
}

/**
* Returns all global holdouts (those with {@code includedRules == null}).
* Global holdouts apply to every rule in every flag.
*
* @return An unmodifiable list of global holdouts
*/
@Nonnull
public List<Holdout> getGlobalHoldouts() {
return Collections.unmodifiableList(globalHoldouts);
}

/**
* Returns all local holdouts that target the given rule ID.
* If no local holdouts are registered for this rule, an empty list is returned.
*
* @param ruleId The experiment or delivery rule ID to look up
* @return An unmodifiable list of holdouts targeting the specified rule
*/
@Nonnull
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
List<Holdout> result = ruleHoldoutsMap.get(ruleId);
return result != null ? Collections.unmodifiableList(result) : Collections.emptyList();
}

/**
* Returns all holdouts for the given flag ID.
* Since all holdouts are now global, this returns all holdouts.
* For backward compatibility this returns all global holdouts.
*
* @param id The flag identifier
* @return A list of all Holdout objects
* @param id The flag identifier (unused — retained for API compatibility)
* @return An unmodifiable list of global Holdout objects
* @deprecated Prefer {@link #getGlobalHoldouts()} for new code.
*/
@Deprecated
public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
return Collections.unmodifiableList(allHoldouts);
return getGlobalHoldouts();
}

/**
Expand All @@ -90,6 +144,7 @@ public Holdout getHoldout(@Nonnull String id) {
*
* @return An unmodifiable list of all holdouts
*/
@Nonnull
public List<Holdout> getAllHoldouts() {
return Collections.unmodifiableList(allHoldouts);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ Experiment getExperimentForKey(@Nonnull String experimentKey,

Holdout getHoldout(@Nonnull String id);

HoldoutConfig getHoldoutConfig();

Set<String> getAllSegments();

List<Experiment> getExperimentsForEventKey(String eventKey);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,17 @@ static Holdout parseHoldout(JsonObject holdoutJson, JsonDeserializationContext c
List<TrafficAllocation> trafficAllocations =
parseTrafficAllocation(holdoutJson.getAsJsonArray("trafficAllocation"));

return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations);
// Parse optional includedRules field (null = global holdout, non-null = local holdout)
List<String> includedRules = null;
if (holdoutJson.has("includedRules") && !holdoutJson.get("includedRules").isJsonNull()) {
JsonArray includedRulesJson = holdoutJson.getAsJsonArray("includedRules");
includedRules = new ArrayList<>(includedRulesJson.size());
for (JsonElement ruleIdElement : includedRulesJson) {
includedRules.add(ruleIdElement.getAsString());
}
}

return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedRules);
}

static FeatureFlag parseFeatureFlag(JsonObject featureFlagJson, JsonDeserializationContext context) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,18 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
List<TrafficAllocation> trafficAllocations =
parseTrafficAllocation(holdoutObject.getJSONArray("trafficAllocation"));

// Parse optional includedRules field (null = global holdout, non-null = local holdout)
List<String> includedRules = null;
if (holdoutObject.has("includedRules") && !holdoutObject.isNull("includedRules")) {
JSONArray includedRulesJson = holdoutObject.getJSONArray("includedRules");
includedRules = new ArrayList<>(includedRulesJson.length());
for (int j = 0; j < includedRulesJson.length(); j++) {
includedRules.add(includedRulesJson.getString(j));
}
}

holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
trafficAllocations));
trafficAllocations, includedRules));
}

return holdouts;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,18 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
List<TrafficAllocation> trafficAllocations =
parseTrafficAllocation((JSONArray) hoObject.get("trafficAllocation"));

// Parse optional includedRules field (null = global holdout, non-null = local holdout)
List<String> includedRules = null;
if (hoObject.containsKey("includedRules") && hoObject.get("includedRules") != null) {
JSONArray includedRulesJson = (JSONArray) hoObject.get("includedRules");
includedRules = new ArrayList<String>(includedRulesJson.size());
for (Object ruleIdObj : includedRulesJson) {
includedRules.add((String) ruleIdObj);
}
}

holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
trafficAllocations));
trafficAllocations, includedRules));
}

return holdouts;
Expand Down
Loading
Loading