diff --git a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java index 28ac62789..e7497d3f1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/DatafileProjectConfig.java @@ -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, @@ -157,6 +160,63 @@ public DatafileProjectConfig(String accountId, List groups, List rollouts, List 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 partitionLegacyHoldoutsByScope(List holdouts, boolean wantGlobal) { + if (holdouts == null || holdouts.isEmpty()) { + return Collections.emptyList(); + } + List partition = new ArrayList(); + 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 attributes, + List audiences, + List typedAudiences, + List events, + List experiments, + List holdouts, + List localHoldouts, + List featureFlags, + List groups, + List rollouts, + List integrations) { this.accountId = accountId; this.projectId = projectId; this.version = version; @@ -200,10 +260,12 @@ public DatafileProjectConfig(String accountId, this.experiments = Collections.unmodifiableList(allExperiments); - if (holdouts == null) { + List globalHoldoutsSection = holdouts != null ? holdouts : Collections.emptyList(); + List localHoldoutsSection = localHoldouts != null ? localHoldouts : Collections.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 = ""; diff --git a/core-api/src/main/java/com/optimizely/ab/config/Holdout.java b/core-api/src/main/java/com/optimizely/ab/config/Holdout.java index 85c530ad8..1680ae941 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/Holdout.java +++ b/core-api/src/main/java/com/optimizely/ab/config/Holdout.java @@ -45,9 +45,8 @@ public class Holdout implements ExperimentCore { private final List 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 includedRules; @@ -183,7 +182,9 @@ public List 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 */ diff --git a/core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java b/core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java index ebd5e6a60..f79f4052e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java @@ -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. + * + *

Two top-level datafile sections drive holdout scoping (Gen 3+): + *

    + *
  • {@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.
    • + *
  • {@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.
    • + *
+ * + *

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 allHoldouts; private Map 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 globalHoldouts; /** Rule-level map: ruleId -> list of local holdouts targeting that rule. */ private Map> 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.emptyList(), Collections.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 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. + * + *

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. + * + *

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 globalHoldoutsFromSection, + @Nonnull List 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 globalHoldoutsFromSection, + @Nonnull List 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 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()); + } + 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 includedRules = holdout.getIncludedRules(); for (String ruleId : includedRules) { if (!ruleHoldoutsMap.containsKey(ruleId)) { - ruleHoldoutsMap.put(ruleId, new ArrayList<>()); + ruleHoldoutsMap.put(ruleId, new ArrayList()); } ruleHoldoutsMap.get(ruleId).add(holdout); } @@ -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 */ @@ -102,8 +207,9 @@ public List 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 @@ -111,7 +217,7 @@ public List getGlobalHoldouts() { @Nonnull public List getHoldoutsForRule(@Nonnull String ruleId) { List holdouts = ruleHoldoutsMap.get(ruleId); - return holdouts != null ? Collections.unmodifiableList(holdouts) : Collections.emptyList(); + return holdouts != null ? Collections.unmodifiableList(holdouts) : Collections.emptyList(); } /** @@ -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 */ diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java index 12ad20808..a70b073bb 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileGsonDeserializer.java @@ -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 localHoldouts; + if (jsonObject.has("localHoldouts")) { + localHoldouts = context.deserialize(jsonObject.get("localHoldouts").getAsJsonArray(), holdoutsType); + } else { + localHoldouts = Collections.emptyList(); + } + List attributes; attributes = context.deserialize(jsonObject.get("attributes"), attributesType); @@ -143,6 +152,7 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa events, experiments, holdouts, + localHoldouts, featureFlags, groups, rollouts, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java index 5c94a444f..0155c285e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/DatafileJacksonDeserializer.java @@ -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 localHoldouts; + if (node.has("localHoldouts")) { + localHoldouts = JacksonHelpers.arrayNodeToList(node.get("localHoldouts"), Holdout.class, codec); + } else { + localHoldouts = Collections.emptyList(); + } + List audiences = Collections.emptyList(); if (node.has("audiences")) { audiences = JacksonHelpers.arrayNodeToList(node.get("audiences"), Audience.class, codec); @@ -117,6 +126,7 @@ public DatafileProjectConfig deserialize(JsonParser parser, DeserializationConte events, experiments, holdouts, + localHoldouts, featureFlags, groups, rollouts, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 6b99f53b7..5a546acf8 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -55,6 +55,15 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse 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 localHoldouts; + if (rootObject.has("localHoldouts")) { + localHoldouts = parseHoldouts(rootObject.getJSONArray("localHoldouts")); + } else { + localHoldouts = Collections.emptyList(); + } + List attributes; attributes = parseAttributes(rootObject.getJSONArray("attributes")); @@ -122,6 +131,7 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse events, experiments, holdouts, + localHoldouts, featureFlags, groups, rollouts, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index d30978186..435abca28 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -64,6 +64,15 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse 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 localHoldouts; + if (rootObject.containsKey("localHoldouts")) { + localHoldouts = parseHoldouts((JSONArray) rootObject.get("localHoldouts")); + } else { + localHoldouts = Collections.emptyList(); + } + List attributes; attributes = parseAttributes((JSONArray) rootObject.get("attributes")); @@ -125,6 +134,7 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse events, experiments, holdouts, + localHoldouts, featureFlags, groups, rollouts, diff --git a/core-api/src/test/java/com/optimizely/ab/config/HoldoutConfigTest.java b/core-api/src/test/java/com/optimizely/ab/config/HoldoutConfigTest.java index 1d4f7f4dd..5fc51164c 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/HoldoutConfigTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/HoldoutConfigTest.java @@ -29,6 +29,7 @@ import org.junit.Before; import org.junit.Test; +@SuppressWarnings("deprecation") public class HoldoutConfigTest { private Holdout globalHoldout1; @@ -263,8 +264,177 @@ public void testGetAllHoldoutsIsUnmodifiable() { } } - // Helper for assertNotNull (avoids import of static from junit 4.x) + // ----------------------------------------------------------------------- + // Section-aware constructor (FSSDK-12760): localHoldouts datafile section + // ----------------------------------------------------------------------- + + @Test + public void testSectionConstructorPartitionsByDatafileSection() { + // Entries are routed by section membership, not by includedRules. + HoldoutConfig config = new HoldoutConfig( + Arrays.asList(globalHoldout1, globalHoldout2), + Arrays.asList(localHoldoutRuleA, localHoldoutRuleB) + ); + + List globals = config.getGlobalHoldouts(); + assertEquals(2, globals.size()); + assertTrue(globals.contains(globalHoldout1)); + assertTrue(globals.contains(globalHoldout2)); + + // Local entries are registered under their includedRules. + List forRuleA = config.getHoldoutsForRule("ruleA"); + assertEquals(2, forRuleA.size()); + assertTrue(forRuleA.contains(localHoldoutRuleA)); + assertTrue(forRuleA.contains(localHoldoutRuleB)); + } + + @Test + public void testGlobalSectionEntriesIgnoreIncludedRules() { + // A holdout placed in the global 'holdouts' section must be treated as global + // and have its includedRules stripped — even if the datafile incorrectly carries one. + Holdout strayGlobalWithRules = new Holdout( + "stray", "stray_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Arrays.asList("ruleA") + ); + + HoldoutConfig config = new HoldoutConfig( + Arrays.asList(strayGlobalWithRules), + Collections.emptyList() + ); + + // The entity-level isGlobal must report true after stripping + List globals = config.getGlobalHoldouts(); + assertEquals(1, globals.size()); + Holdout sanitized = globals.get(0); + assertEquals("stray", sanitized.getId()); + assertTrue("includedRules should be stripped on global-section entries", sanitized.isGlobal()); + assertNull(sanitized.getIncludedRules()); + + // And the stray rule ID must NOT be registered as a local rule + assertTrue(config.getHoldoutsForRule("ruleA").isEmpty()); + + // holdoutIdMap returns the sanitized copy (verifies that callers see the stripped form) + Holdout viaIdMap = config.getHoldout("stray"); + assertNotNull("Stray global must be retrievable by ID", viaIdMap); + assertTrue(viaIdMap.isGlobal()); + assertNull(viaIdMap.getIncludedRules()); + } + + @Test + public void testLocalSectionEntryWithoutIncludedRulesIsExcluded() { + // Entry placed in the localHoldouts section but missing includedRules is invalid. + Holdout invalidLocal = new Holdout( + "invalid", "invalid_local", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + null // missing includedRules + ); + + HoldoutConfig config = new HoldoutConfig( + Collections.emptyList(), + Arrays.asList(invalidLocal) + ); + + // Excluded from every map: must NOT fall back to global application + assertTrue("Invalid local must not be promoted to global", config.getGlobalHoldouts().isEmpty()); + assertTrue("Invalid local must not be registered for any rule", + config.getHoldoutsForRule("ruleA").isEmpty()); + assertNull("Invalid local must not be retrievable by ID", config.getHoldout("invalid")); + } + + @Test + public void testLocalSectionEntryWithEmptyIncludedRulesIsValidButInert() { + // includedRules == [] is valid but inert: the entity is tracked in the id map + // (no error logged) but is not registered under any rule. Matches the spec's + // entity-level semantics where [] != null. + HoldoutConfig config = new HoldoutConfig( + Collections.emptyList(), + Arrays.asList(localHoldoutEmpty) + ); + + // Not promoted to global + assertTrue(config.getGlobalHoldouts().isEmpty()); + // Not registered under any rule (no rule IDs to target) + assertTrue(config.getHoldoutsForRule("ruleA").isEmpty()); + // But the entity is tracked in the id map + Holdout stored = config.getHoldout("local_holdout_empty"); + assertNotNull("Empty-includedRules local must still be retrievable by ID", stored); + assertFalse("Empty-includedRules local must not report isGlobal", stored.isGlobal()); + } + + @Test + public void testBothSectionsPresentPartitionCorrectly() { + // When both sections are non-empty, scope is enforced strictly by section. + HoldoutConfig config = new HoldoutConfig( + Arrays.asList(globalHoldout1), + Arrays.asList(localHoldoutRuleA) + ); + + List globals = config.getGlobalHoldouts(); + assertEquals(1, globals.size()); + assertSame(globalHoldout1, globals.get(0)); + + List forRuleA = config.getHoldoutsForRule("ruleA"); + assertEquals(1, forRuleA.size()); + assertSame(localHoldoutRuleA, forRuleA.get(0)); + + // ID map covers both + assertNotNull("Global must be in id map", config.getHoldout("holdout1")); + assertNotNull("Local must be in id map", config.getHoldout("local_holdout_a")); + } + + @Test + public void testBackwardCompatNoLocalSection() { + // Older datafiles emit only the 'holdouts' section. Passing an empty list for + // localHoldoutsSection must match the legacy behavior exactly. + HoldoutConfig config = new HoldoutConfig( + Arrays.asList(globalHoldout1, globalHoldout2), + Collections.emptyList() + ); + + assertEquals(2, config.getGlobalHoldouts().size()); + assertTrue(config.getHoldoutsForRule("any_rule").isEmpty()); + assertEquals(2, config.getAllHoldouts().size()); + } + + @Test + public void testEmptyBothSections() { + HoldoutConfig config = new HoldoutConfig( + Collections.emptyList(), + Collections.emptyList() + ); + + assertTrue(config.getAllHoldouts().isEmpty()); + assertTrue(config.getGlobalHoldouts().isEmpty()); + assertTrue(config.getHoldoutsForRule("any_rule").isEmpty()); + assertNull(config.getHoldout("any_id")); + } + + @Test + public void testSectionConstructorIdMapTracksBothSections() { + HoldoutConfig config = new HoldoutConfig( + Arrays.asList(globalHoldout1), + Arrays.asList(localHoldoutRuleA) + ); + + assertSame(globalHoldout1, config.getHoldout("holdout1")); + assertSame(localHoldoutRuleA, config.getHoldout("local_holdout_a")); + } + + // Helper for assertNotNull/assertNull (avoids import of static from junit 4.x) private static void assertNotNull(String message, Object obj) { assertTrue(message, obj != null); } + + private static void assertNotNull(Object obj) { + assertTrue("Expected non-null", obj != null); + } } diff --git a/core-api/src/test/resources/config/holdouts-project-config.json b/core-api/src/test/resources/config/holdouts-project-config.json index 9bf3dfe43..b319eb130 100644 --- a/core-api/src/test/resources/config/holdouts-project-config.json +++ b/core-api/src/test/resources/config/holdouts-project-config.json @@ -532,7 +532,9 @@ ], "audienceIds": ["3468206643", "3468206644", "3468206646", "3468206645"], "audienceConditions" : ["or", "3468206643", "3468206644", "3468206646", "3468206645"] - }, + } + ], + "localHoldouts": [ { "id": "10075323430", "key": "local_holdout_for_basic_experiment",