[AI-FSSDK] [FSSDK-12368] Remove legacy flag-level holdout fields (#604)

Author: Mat001

core-api/src/main/java/com/optimizely/ab/config/Holdout.java

@@ -43,8 +43,6 @@ public class Holdout implements ExperimentCore {
     private final Condition<AudienceIdCondition> audienceConditions;
     private final List<Variation> variations;
     private final List<TrafficAllocation> trafficAllocation;
-    private final List<String> includedFlags;
-    private final List<String> excludedFlags;
 
     private final Map<String, Variation> variationKeyToVariationMap;
     private final Map<String, Variation> variationIdToVariationMap;
@@ -70,7 +68,7 @@ public String toString() {
 
     @VisibleForTesting
     public Holdout(String id, String key) {
-        this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList(), null, null);
+        this(id, key, "Running", Collections.emptyList(), null, Collections.emptyList(), Collections.emptyList());
     }
 
     // Keep only this constructor and add @JsonCreator to it
@@ -81,18 +79,14 @@ public Holdout(@JsonProperty("id") @Nonnull String id,
             @JsonProperty("audienceIds") @Nonnull List<String> audienceIds,
             @JsonProperty("audienceConditions") @Nullable Condition audienceConditions,
             @JsonProperty("variations") @Nonnull List<Variation> variations,
-            @JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation,
-            @JsonProperty("includedFlags") @Nullable List<String> includedFlags,
-            @JsonProperty("excludedFlags") @Nullable List<String> excludedFlags) {
+            @JsonProperty("trafficAllocation") @Nonnull List<TrafficAllocation> trafficAllocation) {
         this.id = id;
         this.key = key;
         this.status = status;
         this.audienceIds = audienceIds;
         this.audienceConditions = audienceConditions;
         this.variations = variations;
         this.trafficAllocation = trafficAllocation;
-        this.includedFlags = includedFlags == null ? Collections.emptyList() : Collections.unmodifiableList(includedFlags);
-        this.excludedFlags = excludedFlags == null ? Collections.emptyList() : Collections.unmodifiableList(excludedFlags);
         this.variationKeyToVariationMap = ProjectConfigUtils.generateNameMapping(this.variations);
         this.variationIdToVariationMap = ProjectConfigUtils.generateIdMapping(this.variations);
     }
@@ -141,14 +135,6 @@ public String getGroupId() {
         return "";
     }
 
-    public List<String> getIncludedFlags() {
-        return includedFlags;
-    }
-
-    public List<String> getExcludedFlags() {
-        return excludedFlags;
-    }
-
     public boolean isActive() {
         return status.equals(Holdout.HoldoutStatus.RUNNING.toString());
     }

core-api/src/main/java/com/optimizely/ab/config/HoldoutConfig.java

@@ -21,25 +21,19 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
 
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 /**
- * HoldoutConfig manages collections of Holdout objects and their relationships to flags.
+ * HoldoutConfig manages collections of Holdout objects.
+ * All holdouts are global and apply to all flags.
  */
 public class HoldoutConfig {
     private List<Holdout> allHoldouts;
-    private List<Holdout> global;
     private Map<String, Holdout> holdoutIdMap;
-    private Map<String, List<Holdout>> flagHoldoutsMap;
-    private Map<String, List<Holdout>> includedHoldouts;
-    private Map<String, Set<Holdout>> excludedHoldouts;
 
     /**
      * Initializes a new HoldoutConfig with an empty list of holdouts.
@@ -55,91 +49,29 @@ public HoldoutConfig() {
      */
     public HoldoutConfig(@Nonnull List<Holdout> allHoldouts) {
         this.allHoldouts = new ArrayList<>(allHoldouts);
-        this.global = new ArrayList<>();
         this.holdoutIdMap = new HashMap<>();
-        this.flagHoldoutsMap = new ConcurrentHashMap<>();
-        this.includedHoldouts = new HashMap<>();
-        this.excludedHoldouts = new HashMap<>();
         updateHoldoutMapping();
     }
 
     /**
-     * Updates internal mappings of holdouts including the id map, global list,
-     * and per-flag inclusion/exclusion maps.
+     * Updates internal mapping of holdout IDs to holdout objects.
      */
     private void updateHoldoutMapping() {
         holdoutIdMap.clear();
         for (Holdout holdout : allHoldouts) {
             holdoutIdMap.put(holdout.getId(), holdout);
         }
-
-        flagHoldoutsMap.clear();
-        global.clear();
-        includedHoldouts.clear();
-        excludedHoldouts.clear();
-
-        for (Holdout holdout : allHoldouts) {
-            boolean hasIncludedFlags = !holdout.getIncludedFlags().isEmpty();
-            boolean hasExcludedFlags = !holdout.getExcludedFlags().isEmpty();
-
-            if (!hasIncludedFlags && !hasExcludedFlags) {
-                // Global holdout (applies to all flags)
-                global.add(holdout);
-            } else if (hasIncludedFlags) {
-                // Holdout only applies to specific included flags
-                for (String flagId : holdout.getIncludedFlags()) {
-                    includedHoldouts.computeIfAbsent(flagId, k -> new ArrayList<>()).add(holdout);
-                }
-            } else {
-                // Global holdout with specific exclusions
-                global.add(holdout);
-
-                for (String flagId : holdout.getExcludedFlags()) {
-                    excludedHoldouts.computeIfAbsent(flagId, k -> new HashSet<>()).add(holdout);
-                }
-            }
-        }
     }
 
     /**
-     * Returns the applicable holdouts for the given flag ID by combining global holdouts
-     * (excluding any specified) and included holdouts, in that order.
-     * Caches the result for future calls.
+     * Returns all holdouts for the given flag ID.
+     * Since all holdouts are now global, this returns all holdouts.
      *
      * @param id The flag identifier
-     * @return A list of Holdout objects relevant to the given flag
+     * @return A list of all Holdout objects
      */
     public List<Holdout> getHoldoutForFlag(@Nonnull String id) {
-        if (allHoldouts.isEmpty()) {
-            return Collections.emptyList();
-        }
-
-        // Check cache and return persistent holdouts
-        if (flagHoldoutsMap.containsKey(id)) {
-            return flagHoldoutsMap.get(id);
-        }
-
-        // Prioritize global holdouts first
-        List<Holdout> activeHoldouts = new ArrayList<>();
-        Set<Holdout> excluded = excludedHoldouts.getOrDefault(id, Collections.emptySet()); 
-
-        if (!excluded.isEmpty()) {
-            for (Holdout holdout : global) {
-                if (!excluded.contains(holdout)) {
-                    activeHoldouts.add(holdout);
-                }
-            }
-        } else {
-            activeHoldouts.addAll(global);
-        }
-
-        // Add included holdouts
-        activeHoldouts.addAll(includedHoldouts.getOrDefault(id, Collections.emptyList()));
-
-        // Cache the result
-        flagHoldoutsMap.put(id, activeHoldouts);
-
-        return activeHoldouts;
+        return Collections.unmodifiableList(allHoldouts);
     }
 
     /**

core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java

@@ -202,23 +202,7 @@ static Holdout parseHoldout(JsonObject holdoutJson, JsonDeserializationContext c
         List<TrafficAllocation> trafficAllocations =
             parseTrafficAllocation(holdoutJson.getAsJsonArray("trafficAllocation"));
 
-        List<String> includedFlags = new ArrayList<>();
-        if (holdoutJson.has("includedFlags")) {
-            JsonArray includedIdsJson = holdoutJson.getAsJsonArray("includedFlags");
-            for (JsonElement hoIdObj : includedIdsJson) {
-                includedFlags.add(hoIdObj.getAsString());
-            }
-        }
-
-        List<String> excludedFlags = new ArrayList<>();
-        if (holdoutJson.has("excludedFlags")) {
-            JsonArray excludedIdsJson = holdoutJson.getAsJsonArray("excludedFlags");
-            for (JsonElement hoIdObj : excludedIdsJson) {
-                excludedFlags.add(hoIdObj.getAsString());
-            }
-        }
-
-        return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations, includedFlags, excludedFlags);
+        return new Holdout(id, key, status, audienceIds, conditions, variations, trafficAllocations);
     }
 
     static FeatureFlag parseFeatureFlag(JsonObject featureFlagJson, JsonDeserializationContext context) {

core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java

@@ -218,34 +218,8 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
             List<TrafficAllocation> trafficAllocations =
                 parseTrafficAllocation(holdoutObject.getJSONArray("trafficAllocation"));
 
-            List<String> includedFlags;
-            if (holdoutObject.has("includedFlags")) {
-                JSONArray includedIdsJson = holdoutObject.getJSONArray("includedFlags");
-                includedFlags = new ArrayList<>(includedIdsJson.length());
-
-                for (int j = 0; j < includedIdsJson.length(); j++) {
-                    Object idObj = includedIdsJson.get(j);
-                    includedFlags.add((String) idObj);
-                }
-            } else {
-                includedFlags = Collections.emptyList();
-            }
-
-            List<String> excludedFlags;
-            if (holdoutObject.has("excludedFlags")) {
-                JSONArray excludedIdsJson = holdoutObject.getJSONArray("excludedFlags");
-                excludedFlags = new ArrayList<>(excludedIdsJson.length());
-
-                for (int j = 0; j < excludedIdsJson.length(); j++) {
-                    Object idObj = excludedIdsJson.get(j);
-                    excludedFlags.add((String) idObj);
-                }
-            } else {
-                excludedFlags = Collections.emptyList();
-            }
-
             holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
-                trafficAllocations, includedFlags, excludedFlags));
+                trafficAllocations));
         }
 
         return holdouts;

core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java

@@ -237,22 +237,8 @@ private List<Holdout> parseHoldouts(JSONArray holdoutJson) {
             List<TrafficAllocation> trafficAllocations =
                 parseTrafficAllocation((JSONArray) hoObject.get("trafficAllocation"));
 
-            List<String> includedFlags;
-            if (hoObject.containsKey("includedFlags")) {
-                includedFlags = new ArrayList<String>((JSONArray) hoObject.get("includedFlags"));
-            } else {
-                includedFlags = Collections.emptyList();
-            }
-
-            List<String> excludedFlags;
-            if (hoObject.containsKey("excludedFlags")) {
-                excludedFlags = new ArrayList<String>((JSONArray) hoObject.get("excludedFlags"));
-            } else {
-                excludedFlags = Collections.emptyList();
-            }
-
             holdouts.add(new Holdout(id, key, status, audienceIds, conditions, variations,
-                trafficAllocations, includedFlags, excludedFlags));
+                trafficAllocations));
         }
 
         return holdouts;

core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java

@@ -2199,69 +2199,4 @@ public void decide_for_keys_with_holdout() throws Exception {
         logbackVerifier.expectMessage(Level.INFO, expectedReason);
     }
 
-    @Test
-    public void decide_all_with_holdout() throws Exception {
-
-        Optimizely optWithHoldout = createOptimizelyWithHoldouts();
-        String userId = "user123";
-        Map<String, Object> attrs = new HashMap<>();
-        // ppid120000 buckets user into holdout_included_flags
-        attrs.put("$opt_bucketing_id", "ppid120000");
-        OptimizelyUserContext user = optWithHoldout.createUserContext(userId, attrs);
-
-        // All flag keys present in holdouts-project-config.json
-        List<String> allFlagKeys = Arrays.asList(
-            "boolean_feature",
-            "double_single_variable_feature",
-            "integer_single_variable_feature",
-            "boolean_single_variable_feature",
-            "string_single_variable_feature",
-            "multi_variate_feature",
-            "multi_variate_future_feature",
-            "mutex_group_feature"
-        );
-
-        // Flags INCLUDED in holdout_included_flags (only these should be holdout decisions)
-        List<String> includedInHoldout = Arrays.asList(
-            "boolean_feature",
-            "double_single_variable_feature",
-            "integer_single_variable_feature"
-        );
-
-        Map<String, OptimizelyDecision> decisions = user.decideAll(Arrays.asList(
-            OptimizelyDecideOption.INCLUDE_REASONS,
-            OptimizelyDecideOption.DISABLE_DECISION_EVENT
-        ));
-        assertEquals(allFlagKeys.size(), decisions.size());
-
-        String holdoutExperimentId = "1007543323427"; // holdout_included_flags id
-        String variationId = "$opt_dummy_variation_id";
-        String variationKey = "ho_off_key";
-        String expectedReason = "User (" + userId + ") is in variation (" + variationKey + ") of holdout (holdout_included_flags).";
-
-        int holdoutCount = 0;
-        for (String flagKey : allFlagKeys) {
-            OptimizelyDecision d = decisions.get(flagKey);
-            assertNotNull("Missing decision for flag " + flagKey, d);
-            if (includedInHoldout.contains(flagKey)) {
-                // Should be holdout decision
-                assertEquals(variationKey, d.getVariationKey());
-                assertFalse(d.getEnabled());
-                assertTrue("Expected holdout reason for flag " + flagKey, d.getReasons().contains(expectedReason));
-                DecisionMetadata metadata = new DecisionMetadata.Builder()
-                    .setFlagKey(flagKey)
-                    .setRuleKey("holdout_included_flags")
-                    .setRuleType("holdout")
-                    .setVariationKey(variationKey)
-                    .setEnabled(false)
-                    .build();
-                holdoutCount++;
-            } else {
-                // Should NOT be a holdout decision
-                assertFalse("Non-included flag should not have holdout reason: " + flagKey, d.getReasons().contains(expectedReason));
-            }
-        }
-        assertEquals("Expected exactly the included flags to be in holdout", includedInHoldout.size(), holdoutCount);
-        logbackVerifier.expectMessage(Level.INFO, expectedReason);
-    }
 }