diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java index c68ea4575..2c35f813a 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/DecisionService.java @@ -325,9 +325,10 @@ public List> getVariationsForFeatureList(@Non DecisionReasons reasons = DefaultDecisionReasons.newInstance(); reasons.merge(upsReasons); - List holdouts = projectConfig.getHoldoutForFlag(featureFlag.getId()); - if (!holdouts.isEmpty()) { - for (Holdout holdout : holdouts) { + // Check global holdouts first (apply to all rules) + List globalHoldouts = projectConfig.getGlobalHoldouts(); + if (!globalHoldouts.isEmpty()) { + for (Holdout holdout : globalHoldouts) { DecisionResponse holdoutDecision = getVariationForHoldout(holdout, user, projectConfig); reasons.merge(holdoutDecision.getReasons()); if (holdoutDecision.getResult() != null) { @@ -846,6 +847,20 @@ private DecisionResponse getVariationFromExperimentRule(@Nonnull Proj if (variation != null) { return new DecisionResponse(variation, reasons); } + + // Check local holdouts targeting this specific rule + List localHoldouts = projectConfig.getHoldoutsForRule(rule.getId()); + if (!localHoldouts.isEmpty()) { + for (Holdout holdout : localHoldouts) { + DecisionResponse holdoutDecision = getVariationForHoldout(holdout, user, projectConfig); + reasons.merge(holdoutDecision.getReasons()); + if (holdoutDecision.getResult() != null) { + // User is in local holdout - return holdout variation and skip this rule + return new DecisionResponse(holdoutDecision.getResult(), reasons); + } + } + } + //regular decision DecisionResponse decisionResponse = getVariation(rule, user, projectConfig, options, userProfileTracker, null, decisionPath); reasons.merge(decisionResponse.getReasons()); @@ -896,6 +911,20 @@ DecisionResponse getVariationFromDeliveryRule(@Nonnull return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons); } + // Check local holdouts targeting this delivery rule + List localHoldouts = projectConfig.getHoldoutsForRule(rule.getId()); + if (!localHoldouts.isEmpty()) { + for (Holdout holdout : localHoldouts) { + DecisionResponse holdoutDecision = getVariationForHoldout(holdout, user, projectConfig); + reasons.merge(holdoutDecision.getReasons()); + if (holdoutDecision.getResult() != null) { + // User is in local holdout - return holdout variation and skip this delivery rule + variationToSkipToEveryoneElsePair = new AbstractMap.SimpleEntry<>(holdoutDecision.getResult(), skipToEveryoneElse); + return new DecisionResponse(variationToSkipToEveryoneElsePair, reasons); + } + } + } + // Handle a regular decision String bucketingId = getBucketingId(user.getUserId(), user.getAttributes()); Boolean everyoneElse = (ruleIndex == rules.size() - 1); 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 0a892b286..28ac62789 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 @@ -575,7 +575,17 @@ public List getHoldoutForFlag(@Nonnull String id) { return holdoutConfig.getHoldoutForFlag(id); } - @Override + @Override + public List getGlobalHoldouts() { + return holdoutConfig.getGlobalHoldouts(); + } + + @Override + public List getHoldoutsForRule(@Nonnull String ruleId) { + return holdoutConfig.getHoldoutsForRule(ruleId); + } + + @Override public Holdout getHoldout(@Nonnull String id) { return holdoutConfig.getHoldout(id); } 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 8144b0d7d..fd341b15a 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 @@ -43,6 +43,7 @@ public class Holdout implements ExperimentCore { private final Condition audienceConditions; private final List variations; private final List trafficAllocation; + private final List includedRules; private final Map variationKeyToVariationMap; private final Map variationIdToVariationMap; @@ -68,7 +69,7 @@ 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); } // Keep only this constructor and add @JsonCreator to it @@ -79,7 +80,8 @@ public Holdout(@JsonProperty("id") @Nonnull String id, @JsonProperty("audienceIds") @Nonnull List audienceIds, @JsonProperty("audienceConditions") @Nullable Condition audienceConditions, @JsonProperty("variations") @Nonnull List variations, - @JsonProperty("trafficAllocation") @Nonnull List trafficAllocation) { + @JsonProperty("trafficAllocation") @Nonnull List trafficAllocation, + @JsonProperty("includedRules") @Nullable List includedRules) { this.id = id; this.key = key; this.status = status; @@ -87,6 +89,7 @@ public Holdout(@JsonProperty("id") @Nonnull String id, this.audienceConditions = audienceConditions; this.variations = variations; this.trafficAllocation = trafficAllocation; + this.includedRules = includedRules; this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(this.variations); this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(this.variations); } @@ -131,6 +134,15 @@ public List getTrafficAllocation() { return trafficAllocation; } + @Nullable + public List getIncludedRules() { + return includedRules; + } + + public boolean isGlobal() { + return includedRules == null; + } + public String getGroupId() { return ""; } @@ -154,6 +166,7 @@ public String toString() { + ", variations=" + variations + ", variationKeyToVariationMap=" + variationKeyToVariationMap + ", trafficAllocation=" + trafficAllocation + + ", includedRules=" + includedRules + '}'; } } 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 77b9ba30f..4953dee4a 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 @@ -29,11 +29,13 @@ /** * HoldoutConfig manages collections of Holdout objects. - * All holdouts are global and apply to all flags. + * Supports both global holdouts (apply to all rules) and local holdouts (apply to specific rules). */ public class HoldoutConfig { private List allHoldouts; private Map holdoutIdMap; + private List globalHoldouts; + private Map> ruleHoldoutsMap; /** * Initializes a new HoldoutConfig with an empty list of holdouts. @@ -50,28 +52,72 @@ public HoldoutConfig() { public HoldoutConfig(@Nonnull List 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. + * Updates internal mappings of holdout IDs and rule-level holdouts. + * Separates global holdouts (includedRules == null) from local holdouts (includedRules != null). */ private void updateHoldoutMapping() { holdoutIdMap.clear(); + globalHoldouts.clear(); + ruleHoldoutsMap.clear(); + for (Holdout holdout : allHoldouts) { holdoutIdMap.put(holdout.getId(), holdout); + + if (holdout.isGlobal()) { + // Global holdout: applies to all rules + globalHoldouts.add(holdout); + } else { + // Local holdout: applies to specific rules + List includedRules = holdout.getIncludedRules(); + if (includedRules != null) { + for (String ruleId : includedRules) { + ruleHoldoutsMap.computeIfAbsent(ruleId, k -> new ArrayList<>()).add(holdout); + } + } + } } } + /** + * Returns all global holdouts (those that apply to all rules). + * + * @return An unmodifiable list of global holdouts + */ + @Nonnull + public List getGlobalHoldouts() { + return Collections.unmodifiableList(globalHoldouts); + } + + /** + * Returns local holdouts that target a specific rule. + * + * @param ruleId The rule identifier + * @return A list of holdouts targeting this rule, or an empty list if none + */ + @Nonnull + public List getHoldoutsForRule(@Nonnull String ruleId) { + List holdouts = ruleHoldoutsMap.get(ruleId); + return holdouts != null ? Collections.unmodifiableList(holdouts) : Collections.emptyList(); + } + /** * Returns all holdouts for the given flag ID. - * Since all holdouts are now global, this returns all holdouts. + * Since all holdouts are now global, this returns all global holdouts. + * This method is deprecated; use getGlobalHoldouts() instead. * * @param id The flag identifier - * @return A list of all Holdout objects + * @return A list of all global Holdout objects + * @deprecated Use {@link #getGlobalHoldouts()} instead */ + @Deprecated public List getHoldoutForFlag(@Nonnull String id) { - return Collections.unmodifiableList(allHoldouts); + return getGlobalHoldouts(); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java index 1872061dd..caf67f38c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java +++ b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java @@ -75,6 +75,10 @@ Experiment getExperimentForKey(@Nonnull String experimentKey, List getHoldoutForFlag(@Nonnull String id); + List getGlobalHoldouts(); + + List getHoldoutsForRule(@Nonnull String ruleId); + Holdout getHoldout(@Nonnull String id); Set getAllSegments(); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java index 8bd82dc0f..b78854a53 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java @@ -202,7 +202,17 @@ static Holdout parseHoldout(JsonObject holdoutJson, JsonDeserializationContext c List trafficAllocations = parseTrafficAllocation(holdoutJson.getAsJsonArray("trafficAllocation")); - return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations); + // parse includedRules (optional field for local holdouts, null for global holdouts) + List includedRules = null; + if (holdoutJson.has("includedRules") && !holdoutJson.get("includedRules").isJsonNull()) { + JsonArray includedRulesJson = holdoutJson.getAsJsonArray("includedRules"); + includedRules = new ArrayList<>(includedRulesJson.size()); + for (JsonElement ruleIdObj : includedRulesJson) { + includedRules.add(ruleIdObj.getAsString()); + } + } + + return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedRules); } static FeatureFlag parseFeatureFlag(JsonObject featureFlagJson, JsonDeserializationContext context) { 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 b361031e2..1bf594a05 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 @@ -218,8 +218,18 @@ private List parseHoldouts(JSONArray holdoutJson) { List trafficAllocations = parseTrafficAllocation(holdoutObject.getJSONArray("trafficAllocation")); + // parse includedRules (optional field for local holdouts, null for global holdouts) + List 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; 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 8491f1e3e..57ecd4005 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 @@ -237,8 +237,18 @@ private List parseHoldouts(JSONArray holdoutJson) { List trafficAllocations = parseTrafficAllocation((JSONArray) hoObject.get("trafficAllocation")); + // parse includedRules (optional field for local holdouts, null for global holdouts) + List includedRules = null; + if (hoObject.containsKey("includedRules") && hoObject.get("includedRules") != null) { + JSONArray includedRulesJson = (JSONArray) hoObject.get("includedRules"); + includedRules = new ArrayList(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; 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 c0ddf7c71..82200f471 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2016-2019, 2021, Optimizely and contributors + * Copyright 2016-2019, 2021, 2025, 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. @@ -37,7 +37,7 @@ public class HoldoutConfigTest { @Before public void setUp() { - // All holdouts are now global (apply to all flags) + // Global holdouts (no includedRules) holdout1 = new Holdout("holdout1", "first_holdout"); holdout2 = new Holdout("holdout2", "second_holdout"); holdout3 = new Holdout("holdout3", "third_holdout"); @@ -46,7 +46,7 @@ public void setUp() { @Test public void testEmptyConstructor() { HoldoutConfig config = new HoldoutConfig(); - + assertTrue(config.getAllHoldouts().isEmpty()); assertTrue(config.getHoldoutForFlag("any_flag").isEmpty()); assertNull(config.getHoldout("any_id")); @@ -55,7 +55,7 @@ public void testEmptyConstructor() { @Test public void testConstructorWithEmptyList() { HoldoutConfig config = new HoldoutConfig(Collections.emptyList()); - + assertTrue(config.getAllHoldouts().isEmpty()); assertTrue(config.getHoldoutForFlag("any_flag").isEmpty()); assertNull(config.getHoldout("any_id")); @@ -81,7 +81,7 @@ public void testGetHoldout() { } @Test - public void testGetHoldoutForFlagReturnsAllHoldouts() { + public void testGetHoldoutForFlagReturnsAllGlobalHoldouts() { List holdouts = Arrays.asList(holdout1, holdout2, holdout3); HoldoutConfig config = new HoldoutConfig(holdouts); @@ -94,11 +94,8 @@ public void testGetHoldoutForFlagReturnsAllHoldouts() { List flag2Holdouts = config.getHoldoutForFlag("flag2"); assertEquals(3, flag2Holdouts.size()); - assertTrue(flag2Holdouts.contains(holdout1)); - assertTrue(flag2Holdouts.contains(holdout2)); - assertTrue(flag2Holdouts.contains(holdout3)); - // Any flag should return all holdouts + // Any flag should return all global holdouts List anyFlagHoldouts = config.getHoldoutForFlag("any_flag"); assertEquals(3, anyFlagHoldouts.size()); } @@ -126,4 +123,217 @@ public void testEmptyFlagHoldouts() { assertTrue(flagHoldouts.isEmpty()); } -} \ No newline at end of file + // --- Local holdout tests --- + + @Test + public void testGetGlobalHoldoutsReturnsOnlyGlobalHoldouts() { + // Global holdout has null includedRules + Holdout globalHoldout = new Holdout("global_1", "global_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + null); // null = global + + // Local holdout has non-null includedRules + Holdout localHoldout = new Holdout("local_1", "local_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Arrays.asList("rule_123")); // non-null = local + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(globalHoldout, localHoldout)); + + List globalHoldouts = config.getGlobalHoldouts(); + assertEquals(1, globalHoldouts.size()); + assertEquals("global_1", globalHoldouts.get(0).getId()); + } + + @Test + public void testGetHoldoutsForRuleReturnsLocalHoldoutsForRule() { + Holdout localHoldout = new Holdout("local_1", "local_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Arrays.asList("rule_123", "rule_456")); + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(localHoldout)); + + List rule123Holdouts = config.getHoldoutsForRule("rule_123"); + assertEquals(1, rule123Holdouts.size()); + assertEquals("local_1", rule123Holdouts.get(0).getId()); + + List rule456Holdouts = config.getHoldoutsForRule("rule_456"); + assertEquals(1, rule456Holdouts.size()); + + // Other rules should return empty + List otherRuleHoldouts = config.getHoldoutsForRule("rule_999"); + assertTrue(otherRuleHoldouts.isEmpty()); + } + + @Test + public void testLocalHoldoutDoesNotAppearInGlobalHoldouts() { + Holdout localHoldout = new Holdout("local_1", "local_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Arrays.asList("rule_123")); + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(localHoldout)); + + // Local holdout should not appear in global holdouts + assertTrue(config.getGlobalHoldouts().isEmpty()); + + // Local holdout should appear via getHoldoutsForRule + List ruleHoldouts = config.getHoldoutsForRule("rule_123"); + assertEquals(1, ruleHoldouts.size()); + } + + @Test + public void testGlobalHoldoutDoesNotAppearInRuleHoldouts() { + Holdout globalHoldout = new Holdout("global_1", "global_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + null); // null = global + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(globalHoldout)); + + // Global holdout should not appear in rule holdouts + assertTrue(config.getHoldoutsForRule("rule_123").isEmpty()); + + // Global holdout should appear in getGlobalHoldouts + assertEquals(1, config.getGlobalHoldouts().size()); + } + + @Test + public void testEmptyIncludedRulesIsLocalNotGlobal() { + // Empty list (not null) means local holdout with no rules + Holdout emptyRulesHoldout = new Holdout("empty_rules", "empty_rules_holdout", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Collections.emptyList()); // empty list = local holdout with no rules + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(emptyRulesHoldout)); + + // Empty-rules holdout is NOT global + assertTrue(config.getGlobalHoldouts().isEmpty()); + + // Empty-rules holdout doesn't match any rule + assertTrue(config.getHoldoutsForRule("any_rule").isEmpty()); + } + + @Test + public void testMixedGlobalAndLocalHoldouts() { + Holdout globalHoldout = new Holdout("global_h", "global", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + null); + + Holdout localHoldout = new Holdout("local_h", "local", + "Running", + Collections.emptyList(), + null, + Collections.emptyList(), + Collections.emptyList(), + Arrays.asList("rule_abc")); + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(globalHoldout, localHoldout)); + + // Global holdouts + List globals = config.getGlobalHoldouts(); + assertEquals(1, globals.size()); + assertEquals("global_h", globals.get(0).getId()); + + // Local holdouts by rule + List ruleHoldouts = config.getHoldoutsForRule("rule_abc"); + assertEquals(1, ruleHoldouts.size()); + assertEquals("local_h", ruleHoldouts.get(0).getId()); + + // Deprecated getHoldoutForFlag returns only global + List flagHoldouts = config.getHoldoutForFlag("any_flag"); + assertEquals(1, flagHoldouts.size()); + assertEquals("global_h", flagHoldouts.get(0).getId()); + } + + @Test + public void testBackwardCompatibilityOldDatafileNullIncludedRules() { + // Old datafile holdout — no includedRules field, defaults to null = global + Holdout oldHoldout = new Holdout("old_holdout", "old"); + assertTrue(oldHoldout.isGlobal()); + assertNull(oldHoldout.getIncludedRules()); + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(oldHoldout)); + + // Should behave as global holdout + assertEquals(1, config.getGlobalHoldouts().size()); + assertTrue(config.getHoldoutsForRule("any_rule").isEmpty()); + } + + @Test + public void testIsGlobalReturnsTrueWhenIncludedRulesIsNull() { + Holdout globalHoldout = new Holdout("g1", "global", + "Running", + Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyList(), + null); + assertTrue(globalHoldout.isGlobal()); + assertNull(globalHoldout.getIncludedRules()); + } + + @Test + public void testIsGlobalReturnsFalseWhenIncludedRulesIsNonNull() { + Holdout localHoldout = new Holdout("l1", "local", + "Running", + Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyList(), + Arrays.asList("rule_1")); + assertFalse(localHoldout.isGlobal()); + assertEquals(Arrays.asList("rule_1"), localHoldout.getIncludedRules()); + } + + @Test + public void testGetHoldoutsForRuleReturnsEmptyForUnknownRule() { + HoldoutConfig config = new HoldoutConfig(Arrays.asList(holdout1, holdout2)); + assertTrue(config.getHoldoutsForRule("unknown_rule").isEmpty()); + } + + @Test + public void testMultipleLocalHoldoutsForSameRule() { + Holdout local1 = new Holdout("local_1", "local_holdout_1", + "Running", + Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyList(), + Arrays.asList("rule_abc")); + + Holdout local2 = new Holdout("local_2", "local_holdout_2", + "Running", + Collections.emptyList(), null, + Collections.emptyList(), Collections.emptyList(), + Arrays.asList("rule_abc", "rule_xyz")); + + HoldoutConfig config = new HoldoutConfig(Arrays.asList(local1, local2)); + + List ruleAbcHoldouts = config.getHoldoutsForRule("rule_abc"); + assertEquals(2, ruleAbcHoldouts.size()); + + List ruleXyzHoldouts = config.getHoldoutsForRule("rule_xyz"); + assertEquals(1, ruleXyzHoldouts.size()); + assertEquals("local_2", ruleXyzHoldouts.get(0).getId()); + } +}