Skip to content

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

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
Mat001 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c76e0e9
[FSSDK-12368] Remove legacy flag-level holdout fields
Mat001 Apr 21, 2026
8dece35
fix tests
jaeopt Apr 21, 2026
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
18 changes: 2 additions & 16 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 @@ -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;
Expand All @@ -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
Expand All @@ -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);
}
Expand Down Expand Up @@ -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());
}
Expand Down
82 changes: 7 additions & 75 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 @@ -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.
Expand All @@ -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);
}

/**
Expand Down
18 changes: 1 addition & 17 deletions core-api/src/main/java/com/optimizely/ab/config/parser/GsonHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
28 changes: 1 addition & 27 deletions core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
16 changes: 1 addition & 15 deletions core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
65 changes: 0 additions & 65 deletions core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Loading
Loading