Skip to content
Open
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 @@ -36,6 +36,7 @@
import com.optimizely.ab.OptimizelyRuntimeException;
import com.optimizely.ab.OptimizelyUserContext;
import com.optimizely.ab.config.Experiment;
import com.optimizely.ab.config.ExperimentCore;
import com.optimizely.ab.config.FeatureFlag;
import com.optimizely.ab.config.Holdout;
import com.optimizely.ab.config.ProjectConfig;
Expand Down Expand Up @@ -325,9 +326,10 @@ 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) {
// Evaluate global holdouts at flag level (before any rules are iterated)
List<Holdout> globalHoldouts = projectConfig.getGlobalHoldouts();
if (!globalHoldouts.isEmpty()) {
for (Holdout holdout : globalHoldouts) {
Comment thread
jaeopt marked this conversation as resolved.
DecisionResponse<Variation> holdoutDecision = getVariationForHoldout(holdout, user, projectConfig);
reasons.merge(holdoutDecision.getReasons());
if (holdoutDecision.getResult() != null) {
Expand Down Expand Up @@ -395,33 +397,22 @@ DecisionResponse<FeatureDecision> getVariationFromExperiment(@Nonnull ProjectCon
@Nullable UserProfileTracker userProfileTracker,
@Nonnull DecisionPath decisionPath) {
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
// Cache flagKey once to avoid multiple getKey() calls (important for mock-based tests)
String flagKey = featureFlag.getKey();
if (!featureFlag.getExperimentIds().isEmpty()) {
for (String experimentId : featureFlag.getExperimentIds()) {
Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId);

DecisionResponse<Variation> decisionVariation =
getVariationFromExperimentRule(projectConfig, featureFlag.getKey(), experiment, user, options, userProfileTracker, decisionPath);
DecisionResponse<FeatureDecision> decisionVariation =
getVariationFromExperimentRule(projectConfig, flagKey, experiment, user, options, userProfileTracker, decisionPath);
reasons.merge(decisionVariation.getReasons());
Variation variation = decisionVariation.getResult();
String cmabUuid = decisionVariation.getCmabUuid();
boolean error = decisionVariation.isError();
if (error) {
return new DecisionResponse(
new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST, cmabUuid),
reasons,
decisionVariation.isError(),
cmabUuid);
}
if (variation != null) {
return new DecisionResponse(
new FeatureDecision(experiment, variation, FeatureDecision.DecisionSource.FEATURE_TEST, cmabUuid),
reasons,
decisionVariation.isError(),
cmabUuid);
FeatureDecision featureDecision = decisionVariation.getResult();
if (decisionVariation.isError() || (featureDecision != null && featureDecision.variation != null)) {
return new DecisionResponse(featureDecision, reasons, decisionVariation.isError(), decisionVariation.getCmabUuid());
}
}
} else {
String message = reasons.addInfo("The feature flag \"%s\" is not used in any experiments.", featureFlag.getKey());
String message = reasons.addInfo("The feature flag \"%s\" is not used in any experiments.", flagKey);
logger.info(message);
}

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

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

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

AbstractMap.SimpleEntry<Variation, Boolean> response = decisionVariationResponse.getResult();
Variation variation = response.getKey();
AbstractMap.SimpleEntry<FeatureDecision, Boolean> response = decisionVariationResponse.getResult();
FeatureDecision featureDecision = response.getKey();
Boolean skipToEveryoneElse = response.getValue();
if (variation != null) {
Experiment rule = rollout.getExperiments().get(index);
FeatureDecision featureDecision = new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT);
if (featureDecision != null) {
return new DecisionResponse(featureDecision, reasons);
}

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

DecisionResponse<FeatureDecision> evaluateLocalHoldouts(@Nonnull ExperimentCore rule,
@Nonnull ProjectConfig projectConfig,
@Nonnull OptimizelyUserContext user) {
DecisionReasons reasons = DefaultDecisionReasons.newInstance();
List<Holdout> localHoldouts = projectConfig.getHoldoutsForRule(rule.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);
}
}
return new DecisionResponse<>(null, reasons);
}

public ConcurrentHashMap<String, ConcurrentHashMap<String, String>> getForcedVariationMapping() {
return forcedVariationMapping;
}
Expand Down Expand Up @@ -826,7 +833,7 @@ public DecisionResponse<Variation> getForcedVariation(@Nonnull Experiment experi
}


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

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

reasons.merge(forcedDecisionResponse.getReasons());

Variation variation = forcedDecisionResponse.getResult();
if (variation != null) {
return new DecisionResponse(variation, reasons);
return new DecisionResponse(
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.FEATURE_TEST),
reasons);
}

// Step 2: Check local holdouts
if (rule != null) {
DecisionResponse<FeatureDecision> holdoutResponse = evaluateLocalHoldouts(rule, projectConfig, user);
reasons.merge(holdoutResponse.getReasons());
if (holdoutResponse.getResult() != null) {
return new DecisionResponse<>(holdoutResponse.getResult(), reasons);
}
}
//regular decision

// Step 3: Regular rule decision
DecisionResponse<Variation> decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath);
reasons.merge(decisionResponse.getReasons());

variation = decisionResponse.getResult();

return new DecisionResponse<>(variation, reasons, decisionResponse.isError(), decisionResponse.getCmabUuid());
return new DecisionResponse<>(
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.FEATURE_TEST, decisionResponse.getCmabUuid()),
reasons, decisionResponse.isError(), decisionResponse.getCmabUuid());
}

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

Boolean skipToEveryoneElse = false;
AbstractMap.SimpleEntry<Variation, Boolean> variationToSkipToEveryoneElsePair;
// Check forced-decisions first
AbstractMap.SimpleEntry<FeatureDecision, Boolean> resultPair;
Experiment rule = rules.get(ruleIndex);

// Step 1: Check Forced-Decision
OptimizelyDecisionContext optimizelyDecisionContext = new OptimizelyDecisionContext(flagKey, rule.getKey());
DecisionResponse<Variation> forcedDecisionResponse = validatedForcedDecision(optimizelyDecisionContext, projectConfig, user);
reasons.merge(forcedDecisionResponse.getReasons());

Variation variation = forcedDecisionResponse.getResult();
if (variation != null) {
variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(variation, false);
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
resultPair = new AbstractMap.SimpleEntry<>(
new FeatureDecision(rule, variation, FeatureDecision.DecisionSource.ROLLOUT), false);
return new DecisionResponse(resultPair, reasons);
}

// Step 2: Check local holdouts
DecisionResponse<FeatureDecision> holdoutResponse = evaluateLocalHoldouts(rule, projectConfig, user);
reasons.merge(holdoutResponse.getReasons());
if (holdoutResponse.getResult() != null) {
resultPair = new AbstractMap.SimpleEntry<>(holdoutResponse.getResult(), false);
return new DecisionResponse(resultPair, reasons);
}

// Handle a regular decision
// Step 3: Regular rule decision
String bucketingId = getBucketingId(user.getUserId(), user.getAttributes());
Boolean everyoneElse = (ruleIndex == rules.size() - 1);
String loggingKey = everyoneElse ? "Everyone Else" : String.valueOf(ruleIndex + 1);
Expand Down Expand Up @@ -938,8 +969,11 @@ DecisionResponse<AbstractMap.SimpleEntry> getVariationFromDeliveryRule(@Nonnull
reasons.addInfo(message);
logger.debug(message);
}
variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(bucketedVariation, skipToEveryoneElse);
return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons);
FeatureDecision featureDecision = bucketedVariation != null
? new FeatureDecision(rule, bucketedVariation, FeatureDecision.DecisionSource.ROLLOUT)
: null;
resultPair = new AbstractMap.SimpleEntry<>(featureDecision, skipToEveryoneElse);
return new DecisionResponse(resultPair, reasons);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,17 @@ public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
return holdoutConfig.getHoldoutForFlag(id);
}

@Override
@Override
public List<Holdout> getGlobalHoldouts() {
return holdoutConfig.getGlobalHoldouts();
}

@Override
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
return holdoutConfig.getHoldoutsForRule(ruleId);
}

@Override
public Holdout getHoldout(@Nonnull String id) {
return holdoutConfig.getHoldout(id);
}
Expand Down
63 changes: 56 additions & 7 deletions core-api/src/main/java/com/optimizely/ab/config/Holdout.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/**
*
* Copyright 2016-2019, 2021, Optimizely and contributors
* Copyright 2016-2019, 2021, 2026, Optimizely and contributors
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down 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. When null, the holdout is global
* (applies to all rules across all flags). When non-null (even empty), it is a local
* holdout that only applies 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,45 @@ public String toString() {

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

// Keep only this constructor and add @JsonCreator to it
this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null);
}

/**
* Constructor without includedRules (backward-compatible — treated as global holdout).
*/
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);
}

/**
* Full constructor including optional includedRules field (used by parsers).
*
* @param includedRules null = global holdout (applies to all rules); non-null list = local holdout
* targeting only those rule IDs (empty list = local holdout with no matching rules)
*/
@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 @@ -143,6 +171,26 @@ public boolean isRunning() {
return status.equals(Holdout.HoldoutStatus.RUNNING.toString());
}

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

/**
* Returns true if this holdout is global (applies to all rules across all flags).
* A holdout is global when includedRules is null.
*
* @return true if this is a global holdout, false if it is a local holdout
*/
public boolean isGlobal() {
return includedRules == null;
}

@Override
public String toString() {
return "Holdout {"
Expand All @@ -154,6 +202,7 @@ public String toString() {
+ ", variations=" + variations
+ ", variationKeyToVariationMap=" + variationKeyToVariationMap
+ ", trafficAllocation=" + trafficAllocation
+ ", includedRules=" + includedRules
+ '}';
}
}
Loading
Loading