Skip to content

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jaeopt wants to merge 1 commit into
base: master
Choose a base branch
from
Open

[AI-FSSDK] [FSSDK-12760] add localHoldouts to datafile for backward compatibility #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
70 changes: 66 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,10 @@ public DatafileProjectConfig(String accountId, String projectId, String version,
);
}

// v4 constructor
// v4 constructor (legacy — single holdouts list). For backward compatibility with callers
// that pre-date the 'localHoldouts' section split, entries are auto-partitioned by their
// entity-level includedRules field: null -> global section, non-null -> local section.
// Prefer the 21-arg constructor when you have explicit datafile sections.
public DatafileProjectConfig(String accountId,
boolean anonymizeIP,
boolean sendFlagDecisions,
Expand All @@ -157,6 +160,63 @@ public DatafileProjectConfig(String accountId,
List<Group> groups,
List<Rollout> rollouts,
List<Integration> integrations) {
this(accountId, anonymizeIP, sendFlagDecisions, botFiltering, region, projectId, revision,
sdkKey, environmentKey, version, attributes, audiences, typedAudiences, events,
experiments,
partitionLegacyHoldoutsByScope(holdouts, true),
partitionLegacyHoldoutsByScope(holdouts, false),
featureFlags, groups, rollouts, integrations);
}

// Helper for the legacy v4 constructor: splits a mixed holdouts list by entity-level scope
// so callers that haven't migrated to the section-aware constructor still get the right
// global/local routing. {@code wantGlobal=true} returns entries with {@code includedRules == null};
// {@code wantGlobal=false} returns entries with {@code includedRules != null}.
private static List<Holdout> partitionLegacyHoldoutsByScope(List<Holdout> holdouts, boolean wantGlobal) {
if (holdouts == null || holdouts.isEmpty()) {
return Collections.emptyList();
}
List<Holdout> partition = new ArrayList<Holdout>();
for (Holdout holdout : holdouts) {
if (holdout.isGlobal() == wantGlobal) {
partition.add(holdout);
}
}
return partition;
}

// v4 constructor with separate localHoldouts section.
//
// The two top-level datafile sections drive holdout scoping (Gen 3+):
// - 'holdouts' -> all entries are global holdouts (every flag).
// Any 'includedRules' on these entries is IGNORED (HoldoutConfig
// strips it at parse time so section membership is the sole signal).
// - 'localHoldouts' -> all entries are local holdouts (rule-scoped via includedRules).
// Entries missing/empty includedRules are logged and skipped.
//
// Backward compatibility: older datafiles without a 'localHoldouts' section continue to
// work unchanged. Pass {@code null} or an empty list for {@code localHoldouts}.
public DatafileProjectConfig(String accountId,
boolean anonymizeIP,
boolean sendFlagDecisions,
Boolean botFiltering,
String region,
String projectId,
String revision,
String sdkKey,
String environmentKey,
String version,
List<Attribute> attributes,
List<Audience> audiences,
List<Audience> typedAudiences,
List<EventType> events,
List<Experiment> experiments,
List<Holdout> holdouts,
List<Holdout> localHoldouts,
List<FeatureFlag> featureFlags,
List<Group> groups,
List<Rollout> rollouts,
List<Integration> integrations) {
this.accountId = accountId;
this.projectId = projectId;
this.version = version;
Expand Down Expand Up @@ -200,10 +260,12 @@ public DatafileProjectConfig(String accountId,

this.experiments = Collections.unmodifiableList(allExperiments);

if (holdouts == null) {
List<Holdout> globalHoldoutsSection = holdouts != null ? holdouts : Collections.<Holdout>emptyList();
List<Holdout> localHoldoutsSection = localHoldouts != null ? localHoldouts : Collections.<Holdout>emptyList();
if (globalHoldoutsSection.isEmpty() && localHoldoutsSection.isEmpty()) {
this.holdoutConfig = new HoldoutConfig();
} else {
this.holdoutConfig = new HoldoutConfig(holdouts);
} else {
this.holdoutConfig = new HoldoutConfig(globalHoldoutsSection, localHoldoutsSection);
}

String publicKeyForODP = "";
Expand Down
9 changes: 5 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/config/Holdout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,8 @@ public class Holdout implements ExperimentCore {
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.
* Per-rule targeting for local holdouts. Scope comes from the datafile section, not
* this field; HoldoutConfig strips it on entries from the global 'holdouts' section.
*/
@Nullable
private final List<String> includedRules;
Expand Down Expand Up @@ -183,7 +182,9 @@ public List<String> getIncludedRules() {

/**
* Returns true if this holdout is global (applies to all rules across all flags).
* A holdout is global when includedRules is null.
* Scope is determined by the datafile section ('holdouts' vs 'localHoldouts');
* HoldoutConfig strips 'includedRules' from global-section entries, so this stays
* consistent with section membership.
*
* @return true if this is a global holdout, false if it is a local holdout
*/
Expand Down
152 changes: 129 additions & 23 deletions core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,63 +27,148 @@
import javax.annotation.Nonnull;
import javax.annotation.Nullable;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* HoldoutConfig manages collections of Holdout objects, distinguishing between global holdouts
* (which apply to all rules) and local holdouts (which target specific rule IDs).
* HoldoutConfig manages collections of Holdout objects partitioned by datafile section.
*
* <p>Two top-level datafile sections drive holdout scoping (Gen 3+):
* <ul>
* <li>{@code holdouts} — every entry is a global holdout (applied to every flag).
* Any {@code includedRules} field on these entries is IGNORED
* and stripped at parse time; section membership alone determines
* scope.</li>
* <li>{@code localHoldouts} — every entry is a local holdout (rule-scoped via
* {@code includedRules}). Entries missing or with empty
* {@code includedRules} are invalid and skipped with an error log.</li>
* </ul>
*
* <p>Backward compatibility: older datafiles that only emit the {@code holdouts} section
* continue to work unchanged — every entry is treated as global, matching pre-localHoldouts
* behavior. The {@code localHoldouts} key is simply absent and parsed as an empty list.
*/
public class HoldoutConfig {
private static final Logger logger = LoggerFactory.getLogger(HoldoutConfig.class);

private List<Holdout> allHoldouts;
private Map<String, Holdout> holdoutIdMap;

/** Global holdouts: holdouts where includedRules == null. Evaluated at flag level. */
/** Global holdouts: entries from the datafile 'holdouts' section. Evaluated at flag level. */
private List<Holdout> globalHoldouts;

/** Rule-level map: ruleId -> list of local holdouts targeting that rule. */
private Map<String, List<Holdout>> ruleHoldoutsMap;

/**
* Initializes a new HoldoutConfig with an empty list of holdouts.
* Initializes a new HoldoutConfig with no holdouts.
*/
public HoldoutConfig() {
this(Collections.emptyList());
this(Collections.<Holdout>emptyList(), Collections.<Holdout>emptyList());
}

/**
* Initializes a new HoldoutConfig with the specified holdouts.
* Backward-compatible constructor: treats every entry as if it came from the global
* 'holdouts' section. Any {@code includedRules} field on these entries is preserved
* (legacy classification is by entity-level {@code includedRules}, used only by callers
* who pre-date the section split).
*
* @param allHoldouts The list of holdouts to manage
* @deprecated Prefer {@link #HoldoutConfig(List, List)} so global vs. local scope is
* driven by datafile section membership.
*/
@Deprecated
public HoldoutConfig(@Nonnull List<Holdout> allHoldouts) {
this.allHoldouts = new ArrayList<>(allHoldouts);
this.holdoutIdMap = new HashMap<>();
this.globalHoldouts = new ArrayList<>();
this.ruleHoldoutsMap = new HashMap<>();
updateHoldoutMapping();
updateLegacyHoldoutMapping();
}

/**
* Initializes a new HoldoutConfig from the two top-level datafile sections.
*
* <p>Entries in {@code globalHoldoutsFromSection} are treated as global regardless of
* any {@code includedRules} field they may carry; that field is stripped so section
* membership is the sole signal for scope.
*
* <p>Entries in {@code localHoldoutsFromSection} must carry a non-empty
* {@code includedRules} list. Invalid entries (null or empty {@code includedRules})
* are logged at ERROR and excluded from evaluation — they do NOT fall back to
* global application (the partition between sections is hard).
*
* @param globalHoldoutsFromSection Entries from the datafile 'holdouts' section
* @param localHoldoutsFromSection Entries from the datafile 'localHoldouts' section
*/
public HoldoutConfig(@Nonnull List<Holdout> globalHoldoutsFromSection,
@Nonnull List<Holdout> localHoldoutsFromSection) {
this.allHoldouts = new ArrayList<>();
this.holdoutIdMap = new HashMap<>();
this.globalHoldouts = new ArrayList<>();
this.ruleHoldoutsMap = new HashMap<>();
updateHoldoutMapping(globalHoldoutsFromSection, localHoldoutsFromSection);
}

/**
* Updates internal mappings:
* - holdoutIdMap: id -> Holdout
* - globalHoldouts: holdouts where includedRules == null
* - ruleHoldoutsMap: ruleId -> list of holdouts that include that rule
* Section-aware mapping: enforces that scope comes from the datafile section, not the
* {@code includedRules} field. Stale {@code includedRules} values on global-section
* entries are stripped; invalid local-section entries are logged and skipped.
*/
private void updateHoldoutMapping() {
holdoutIdMap.clear();
globalHoldouts.clear();
ruleHoldoutsMap.clear();
private void updateHoldoutMapping(@Nonnull List<Holdout> globalHoldoutsFromSection,
@Nonnull List<Holdout> localHoldoutsFromSection) {
// Process global holdouts: section membership is the sole signal for scope.
// Strip any stale 'includedRules' so the entity is unambiguously global (isGlobal -> true),
// even if the datafile incorrectly includes one.
for (Holdout holdout : globalHoldoutsFromSection) {
Holdout sanitized = holdout.isGlobal() ? holdout : stripIncludedRules(holdout);

allHoldouts.add(sanitized);
holdoutIdMap.put(sanitized.getId(), sanitized);
globalHoldouts.add(sanitized);
}

// Process local holdouts: every entry must carry an 'includedRules' field.
// Entries with null/missing includedRules are invalid per spec — log an error and
// exclude them from evaluation (do NOT fall back to global application).
// An empty includedRules list is valid but inert: the entity is tracked in the id
// map but is not registered under any rule (matches Python reference semantics).
for (Holdout holdout : localHoldoutsFromSection) {
List<String> includedRules = holdout.getIncludedRules();
if (includedRules == null) {
logger.error(
"Local holdout \"{}\" is missing required 'includedRules' field and will be excluded from evaluation.",
holdout.getKey() != null ? holdout.getKey() : holdout.getId());
continue;
}

allHoldouts.add(holdout);
holdoutIdMap.put(holdout.getId(), holdout);
for (String ruleId : includedRules) {
if (!ruleHoldoutsMap.containsKey(ruleId)) {
ruleHoldoutsMap.put(ruleId, new ArrayList<Holdout>());
}
ruleHoldoutsMap.get(ruleId).add(holdout);
}
}
}

/**
* Legacy mapping used by the deprecated single-list constructor. Classifies each entry
* by its entity-level {@code includedRules} (null -> global, non-null -> local).
* Preserved unchanged for callers that have not migrated to section-aware construction.
*/
private void updateLegacyHoldoutMapping() {
for (Holdout holdout : allHoldouts) {
holdoutIdMap.put(holdout.getId(), holdout);

if (holdout.isGlobal()) {
// includedRules == null: global holdout — applies to all rules
globalHoldouts.add(holdout);
} else {
// includedRules != null: local holdout — add to each targeted rule
List<String> includedRules = holdout.getIncludedRules();
for (String ruleId : includedRules) {
if (!ruleHoldoutsMap.containsKey(ruleId)) {
ruleHoldoutsMap.put(ruleId, new ArrayList<>());
ruleHoldoutsMap.put(ruleId, new ArrayList<Holdout>());
}
ruleHoldoutsMap.get(ruleId).add(holdout);
}
Expand All @@ -92,8 +177,28 @@ private void updateHoldoutMapping() {
}

/**
* Returns all global holdouts (holdouts where includedRules == null).
* Returns a copy of the given holdout with {@code includedRules} forced to null, so the
* entity is unambiguously classified as global. Used only when a stale {@code includedRules}
* appears on an entry coming from the global 'holdouts' section.
*/
private static Holdout stripIncludedRules(Holdout holdout) {
return new Holdout(
holdout.getId(),
holdout.getKey(),
holdout.getStatus(),
holdout.getAudienceIds(),
holdout.getAudienceConditions(),
holdout.getVariations(),
holdout.getTrafficAllocation(),
null
);
}

/**
* Returns all global holdouts (entries from the datafile 'holdouts' section).
* These are evaluated at the flag level, before any rules are evaluated.
* Section membership in 'holdouts' is the sole signal for global scope — any
* 'includedRules' field on these entries is ignored.
*
* @return An unmodifiable list of global holdouts
*/
Expand All @@ -102,16 +207,17 @@ public List<Holdout> getGlobalHoldouts() {
}

/**
* Returns local holdouts targeting a specific rule ID.
* These are evaluated per-rule, after the forced decision check and before regular rule evaluation.
* Returns local holdouts targeting a specific rule ID. Local holdouts come from the
* datafile 'localHoldouts' section and are scoped per-rule via 'includedRules'.
* Evaluated per-rule, after the forced decision check and before regular rule evaluation.
*
* @param ruleId The rule identifier to look up
* @return An unmodifiable list of local holdouts targeting that rule, or empty list if none
*/
@Nonnull
public List<Holdout> getHoldoutsForRule(@Nonnull String ruleId) {
List<Holdout> holdouts = ruleHoldoutsMap.get(ruleId);
return holdouts != null ? Collections.unmodifiableList(holdouts) : Collections.emptyList();
return holdouts != null ? Collections.unmodifiableList(holdouts) : Collections.<Holdout>emptyList();
}

/**
Expand Down Expand Up @@ -140,7 +246,7 @@ public Holdout getHoldout(@Nonnull String id) {
}

/**
* Returns all holdouts managed by this config.
* Returns all holdouts managed by this config (both global and local sections, in that order).
*
* @return An unmodifiable list of all holdouts
*/
Expand Down
10 changes: 10 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,15 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa
holdouts = Collections.emptyList();
}

// Parse optional 'localHoldouts' top-level section (Gen 3+ datafile shape).
// Absent in legacy datafiles — handled by passing an empty list to the constructor.
List<Holdout> localHoldouts;
if (jsonObject.has("localHoldouts")) {
localHoldouts = context.deserialize(jsonObject.get("localHoldouts").getAsJsonArray(), holdoutsType);
} else {
localHoldouts = Collections.emptyList();
}

List<Attribute> attributes;
attributes = context.deserialize(jsonObject.get("attributes"), attributesType);

Expand Down Expand Up @@ -143,6 +152,7 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa
events,
experiments,
holdouts,
localHoldouts,
featureFlags,
groups,
rollouts,
Expand Down
10 changes: 10 additions & 0 deletions core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,15 @@ public DatafileProjectConfig deserialize(JsonParser parser, DeserializationConte
holdouts = Collections.emptyList();
}

// Parse optional 'localHoldouts' top-level section (Gen 3+ datafile shape).
// Absent in legacy datafiles — handled by passing an empty list to the constructor.
List<Holdout> localHoldouts;
if (node.has("localHoldouts")) {
localHoldouts = JacksonHelpers.arrayNodeToList(node.get("localHoldouts"), Holdout.class, codec);
} else {
localHoldouts = Collections.emptyList();
}

List<Audience> audiences = Collections.emptyList();
if (node.has("audiences")) {
audiences = JacksonHelpers.arrayNodeToList(node.get("audiences"), Audience.class, codec);
Expand Down Expand Up @@ -117,6 +126,7 @@ public DatafileProjectConfig deserialize(JsonParser parser, DeserializationConte
events,
experiments,
holdouts,
localHoldouts,
featureFlags,
groups,
rollouts,
Expand Down
Loading
Loading