From 912a491c6fac09f7412c1aa2b7cd4a85dab5dbf8 Mon Sep 17 00:00:00 2001 From: Alex Yong Date: Thu, 15 Oct 2026 17:06:40 -0400 Subject: [PATCH 1/3] [AI-FSSDK] [FSSDK-12958] Continue rule evaluation after local holdout hit --- .../java/com/optimizely/ab/Optimizely.java | 26 ++- .../ab/bucketing/DecisionService.java | 48 +++- .../ab/bucketing/FeatureDecision.java | 14 +- .../ab/OptimizelyUserContextTest.java | 154 +++++++++++++ .../ab/bucketing/DecisionServiceTest.java | 45 ++-- .../config/local-holdouts-project-config.json | 209 ++++++++++++++++++ 6 files changed, 446 insertions(+), 50 deletions(-) create mode 100644 core-api/src/test/resources/config/local-holdouts-project-config.json diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 2c51bf6d8..5ad3fa4ba 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/core-api/src/main/java/com/optimizely/ab/Optimizely.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2016-2024, Optimizely, Inc. and contributors * + * Copyright 2016-2024, 2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -1356,17 +1356,19 @@ private OptimizelyDecision createOptimizelyDecision( cmabUuid); } - if (flagDecision.holdoutDecision != null && !allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { - decisionEventDispatched = sendImpression( - projectConfig, - flagDecision.holdoutDecision.experiment, - userId, - copiedAttributes, - flagDecision.holdoutDecision.variation, - flagKey, - flagDecision.holdoutDecision.decisionSource != null ? flagDecision.holdoutDecision.decisionSource.toString() : FeatureDecision.DecisionSource.HOLDOUT.toString(), - flagDecision.holdoutDecision.variation != null && flagDecision.holdoutDecision.variation.getFeatureEnabled(), - null) || decisionEventDispatched; + if (!allOptions.contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { + for (FeatureDecision holdoutDecision : flagDecision.holdoutDecisions) { + decisionEventDispatched = sendImpression( + projectConfig, + holdoutDecision.experiment, + userId, + copiedAttributes, + holdoutDecision.variation, + flagKey, + holdoutDecision.decisionSource != null ? holdoutDecision.decisionSource.toString() : FeatureDecision.DecisionSource.HOLDOUT.toString(), + holdoutDecision.variation != null && holdoutDecision.variation.getFeatureEnabled(), + null) || decisionEventDispatched; + } } DecisionNotification decisionNotification = DecisionNotification.newFlagDecisionNotificationBuilder() 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 35952dd39..0082b0a53 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 @@ -351,38 +351,42 @@ public List> getVariationsForFeatureList(@Non reasons.addInfo("Holdout \"%s\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.", globalHoldoutDecision.experiment.getKey()); } - if (globalHoldoutDecision == null) { + List holdoutDecisions = new ArrayList<>(); + if (globalHoldoutDecision != null) { + holdoutDecisions.add(globalHoldoutDecision); + } else { DecisionResponse decisionVariationResponse = getVariationFromExperiment(projectConfig, featureFlag, user, options, userProfileTracker, decisionPath); reasons.merge(decisionVariationResponse.getReasons()); FeatureDecision decision = decisionVariationResponse.getResult(); boolean error = decisionVariationResponse.isError(); - if (decision != null) { + if (decision != null && (decision.variation != null || error)) { decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUuid)); continue flagLoop; } + if (decision != null) { + holdoutDecisions.addAll(decision.holdoutDecisions); + } } DecisionResponse decisionFeatureResponse = getVariationForFeatureInRollout(featureFlag, user, projectConfig); reasons.merge(decisionFeatureResponse.getReasons()); FeatureDecision decision = decisionFeatureResponse.getResult(); - if (decision != null && decision.variation != null) { - if (globalHoldoutDecision != null) { - decision.setHoldoutDecision(globalHoldoutDecision); + if (!holdoutDecisions.isEmpty()) { + if (decision == null) { + decision = new FeatureDecision(null, null, null); } + decision.holdoutDecisions.addAll(0, holdoutDecisions); + } + + if (decision != null && decision.variation != null) { String message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", user.getUserId(), featureFlag.getKey()); logger.info(message); decisions.add(new DecisionResponse(decision, reasons)); } else { - if (globalHoldoutDecision != null) { - if (decision == null) { - decision = new FeatureDecision(null, null, null); - } - decision.setHoldoutDecision(globalHoldoutDecision); - } String message = reasons.addInfo("The user \"%s\" was not bucketed into a rollout for feature flag \"%s\".", user.getUserId(), featureFlag.getKey()); logger.info(message); @@ -422,6 +426,7 @@ DecisionResponse getVariationFromExperiment(@Nonnull ProjectCon DecisionReasons reasons = DefaultDecisionReasons.newInstance(); // Cache flagKey once to avoid multiple getKey() calls (important for mock-based tests) String flagKey = featureFlag.getKey(); + List holdoutDecisions = new ArrayList<>(); if (!featureFlag.getExperimentIds().isEmpty()) { for (String experimentId : featureFlag.getExperimentIds()) { Experiment experiment = projectConfig.getExperimentIdMapping().get(experimentId); @@ -430,7 +435,12 @@ DecisionResponse getVariationFromExperiment(@Nonnull ProjectCon getVariationFromExperimentRule(projectConfig, flagKey, experiment, user, options, userProfileTracker, decisionPath); reasons.merge(decisionVariation.getReasons()); FeatureDecision featureDecision = decisionVariation.getResult(); + if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) { + holdoutDecisions.add(featureDecision); + continue; + } if (decisionVariation.isError() || (featureDecision != null && featureDecision.variation != null)) { + featureDecision.holdoutDecisions.addAll(holdoutDecisions); return new DecisionResponse(featureDecision, reasons, decisionVariation.isError(), decisionVariation.getCmabUuid()); } } @@ -439,6 +449,11 @@ DecisionResponse getVariationFromExperiment(@Nonnull ProjectCon logger.info(message); } + if (!holdoutDecisions.isEmpty()) { + FeatureDecision emptyDecision = new FeatureDecision(null, null, null); + emptyDecision.holdoutDecisions.addAll(holdoutDecisions); + return new DecisionResponse(emptyDecision, reasons); + } return new DecisionResponse(null, reasons); } @@ -480,6 +495,7 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu } + List holdoutDecisions = new ArrayList<>(); int index = 0; while (index < rolloutRulesLength) { Experiment rolloutRule = rollout.getExperiments().get(index); @@ -496,7 +512,13 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu AbstractMap.SimpleEntry response = decisionVariationResponse.getResult(); FeatureDecision featureDecision = response.getKey(); Boolean skipToEveryoneElse = response.getValue(); + if (featureDecision != null && featureDecision.decisionSource == FeatureDecision.DecisionSource.HOLDOUT) { + holdoutDecisions.add(featureDecision); + index++; + continue; + } if (featureDecision != null) { + featureDecision.holdoutDecisions.addAll(holdoutDecisions); return new DecisionResponse(featureDecision, reasons); } @@ -504,7 +526,9 @@ DecisionResponse getVariationForFeatureInRollout(@Nonnull Featu index = skipToEveryoneElse ? (rolloutRulesLength - 1) : (index + 1); } - return new DecisionResponse(new FeatureDecision(null, null, null), reasons); + FeatureDecision emptyDecision = new FeatureDecision(null, null, null); + emptyDecision.holdoutDecisions.addAll(holdoutDecisions); + return new DecisionResponse(emptyDecision, reasons); } /** diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java index 678ab77dc..4b561846e 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017, 2019, Optimizely, Inc. and contributors * + * Copyright 2017, 2019, 2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -15,8 +15,12 @@ ***************************************************************************/ package com.optimizely.ab.bucketing; +import javax.annotation.Nonnull; import javax.annotation.Nullable; +import java.util.ArrayList; +import java.util.List; + import com.optimizely.ab.config.ExperimentCore; import com.optimizely.ab.config.Variation; @@ -45,12 +49,8 @@ public class FeatureDecision { @Nullable public String cmabUuid; - @Nullable - public FeatureDecision holdoutDecision; - - public void setHoldoutDecision(@Nullable FeatureDecision holdoutDecision) { - this.holdoutDecision = holdoutDecision; - } + @Nonnull + public List holdoutDecisions = new ArrayList<>(); public enum DecisionSource { FEATURE_TEST("feature-test"), diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 807576513..817220d69 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -2204,4 +2204,158 @@ public void decide_for_keys_with_holdout() throws Exception { logbackVerifier.expectMessage(Level.INFO, expectedReason); } + private Optimizely createOptimizelyWithLocalHoldouts(boolean sendFlagDecisions) throws Exception { + String datafile = Resources.toString(Resources.getResource("config/local-holdouts-project-config.json"), Charsets.UTF_8); + if (!sendFlagDecisions) { + datafile = datafile.replace("\"sendFlagDecisions\": true", "\"sendFlagDecisions\": false"); + } + return new Optimizely.Builder().withDatafile(datafile).withEventProcessor(new ForwardingEventProcessor(eventHandler, null)).build(); + } + + private void expectLocalHoldoutImpression(String holdoutId, String holdoutKey, String flagKey, Map attributes) { + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey(holdoutKey) + .setRuleType("holdout") + .setVariationKey("ho_off_key") + .setEnabled(false) + .build(); + eventHandler.expectImpression(holdoutId, null, userId, attributes, metadata); + } + + private void expectRuleImpression(String experimentId, String variationId, String flagKey, String ruleKey, String ruleType, String variationKey, Map attributes) { + DecisionMetadata metadata = new DecisionMetadata.Builder() + .setFlagKey(flagKey) + .setRuleKey(ruleKey) + .setRuleType(ruleType) + .setVariationKey(variationKey) + .setEnabled(true) + .build(); + eventHandler.expectImpression(experimentId, variationId, userId, attributes, metadata); + } + + @Test + public void decide_localHoldoutOnFirstExperimentRule_servedBySecondExperimentRule() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "exp1"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag"); + + assertEquals("exp_2_on", decision.getVariationKey()); + assertEquals("exp_rule_2", decision.getRuleKey()); + assertTrue(decision.getEnabled()); + expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs); + expectLocalHoldoutImpression("5300000001", "lh_exp_rule_1", "lh_flag", attrs); + } + + @Test + public void decide_localHoldoutsOnSeveralRules_sendEventPerHit() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "exp1,exp2"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag"); + + assertEquals("delivery_on", decision.getVariationKey()); + assertEquals("delivery_rule", decision.getRuleKey()); + expectRuleImpression("5400000001", "5400000011", "lh_flag", "delivery_rule", "rollout", "delivery_on", attrs); + expectLocalHoldoutImpression("5300000001", "lh_exp_rule_1", "lh_flag", attrs); + expectLocalHoldoutImpression("5300000002", "lh_exp_rule_2", "lh_flag", attrs); + } + + @Test + public void decide_localHoldoutHitAndNoRuleServes_stillSendsHoldoutEvents() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "delivery,everyone"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_delivery_flag"); + + assertNull(decision.getVariationKey()); + assertNull(decision.getRuleKey()); + assertFalse(decision.getEnabled()); + DecisionMetadata offMetadata = new DecisionMetadata.Builder() + .setFlagKey("lh_delivery_flag") + .setRuleKey("") + .setRuleType("rollout") + .setVariationKey("") + .setEnabled(false) + .build(); + eventHandler.expectImpression(null, null, userId, attrs, offMetadata); + expectLocalHoldoutImpression("5300000003", "lh_delivery_rule", "lh_delivery_flag", attrs); + expectLocalHoldoutImpression("5300000004", "lh_everyone_else_rule", "lh_delivery_flag", attrs); + } + + @Test + public void decide_localHoldoutOnDeliveryRule_sendsHoldoutEventWhenSendFlagDecisionsOff() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(false); + Map attrs = Collections.singletonMap("lh", "delivery"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_delivery_flag"); + + assertEquals("everyone_else_on", decision.getVariationKey()); + assertEquals("everyone_else_rule", decision.getRuleKey()); + assertTrue(decision.getEnabled()); + expectLocalHoldoutImpression("5300000003", "lh_delivery_rule", "lh_delivery_flag", attrs); + } + + @Test + public void decide_localHoldoutOnFirstDeliveryRule_servedBySecondDeliveryRule() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.emptyMap(); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_multi_delivery_flag"); + + assertEquals("second_delivery_on", decision.getVariationKey()); + assertEquals("second_delivery_rule", decision.getRuleKey()); + assertTrue(decision.getEnabled()); + expectRuleImpression("5400000004", "5400000014", "lh_multi_delivery_flag", "second_delivery_rule", "rollout", "second_delivery_on", attrs); + expectLocalHoldoutImpression("5300000006", "lh_first_delivery_rule", "lh_multi_delivery_flag", attrs); + } + + @Test + public void decide_localHoldoutWithDisableDecisionEvent_sendsNoEvents() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "exp1,delivery"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs) + .decide("lh_flag", Collections.singletonList(OptimizelyDecideOption.DISABLE_DECISION_EVENT)); + + assertEquals("exp_2_on", decision.getVariationKey()); + eventHandler.expectCalls(0); + } + + @Test + public void decide_oneLocalHoldoutOnTwoRules_sendsEventPerRule() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "multi"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag"); + + assertEquals("delivery_on", decision.getVariationKey()); + expectRuleImpression("5400000001", "5400000011", "lh_flag", "delivery_rule", "rollout", "delivery_on", attrs); + expectLocalHoldoutImpression("5300000005", "lh_both_exp_rules", "lh_flag", attrs); + expectLocalHoldoutImpression("5300000005", "lh_both_exp_rules", "lh_flag", attrs); + } + + @Test + public void decide_noLocalHoldoutHit_servedByFirstRule() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "none"); + + OptimizelyDecision decision = opt.createUserContext(userId, attrs).decide("lh_flag"); + + assertEquals("exp_1_on", decision.getVariationKey()); + assertEquals("exp_rule_1", decision.getRuleKey()); + expectRuleImpression("5200000001", "5200000011", "lh_flag", "exp_rule_1", "feature-test", "exp_1_on", attrs); + } + + @Test + public void isFeatureEnabled_skipsLocallyHeldOutRuleWithoutHoldoutEvent() throws Exception { + Optimizely opt = createOptimizelyWithLocalHoldouts(true); + Map attrs = Collections.singletonMap("lh", "exp1"); + + assertTrue(opt.isFeatureEnabled("lh_flag", userId, attrs)); + + expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs); + } + } diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index e7877df81..b29b799b4 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -1,5 +1,5 @@ /**************************************************************************** - * Copyright 2017-2022, 2024, Optimizely, Inc. and contributors * + * Copyright 2017-2022, 2024, 2026, Optimizely, Inc. and contributors * * * * Licensed under the Apache License, Version 2.0 (the "License"); * * you may not use this file except in compliance with the License. * @@ -1832,10 +1832,10 @@ public void localHoldout_globalHoldoutEvaluatedAtFlagLevelBeforeRules() { /** * Local holdout hit: a user bucketed into a local holdout targeting experiment rule X - * receives the holdout variation for that rule; regular rule evaluation is skipped. + * is excluded from that rule; the hit is recorded and evaluation continues. */ @Test - public void localHoldout_userInLocalHoldoutReceivesHoldoutVariation() { + public void localHoldout_hitIsRecordedAndEvaluationContinues() { // Config has only a local holdout targeting EXPERIMENT_BASIC_EXPERIMENT_ID (100% traffic) ProjectConfig localHoldoutConfig = ValidProjectConfigV4.generateValidProjectConfigV4_localHoldout(); @@ -1850,10 +1850,15 @@ public void localHoldout_userInLocalHoldoutReceivesHoldoutVariation() { localHoldoutConfig ).getResult(); - assertEquals("User should be in holdout, not regular experiment", - FeatureDecision.DecisionSource.HOLDOUT, featureDecision.decisionSource); - assertEquals(ValidProjectConfigV4.HOLDOUT_LOCAL_FOR_BASIC_EXPERIMENT, featureDecision.experiment); - assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, featureDecision.variation); + assertNotNull(featureDecision); + assertNull(featureDecision.experiment); + assertNull(featureDecision.variation); + assertNull(featureDecision.decisionSource); + assertEquals(1, featureDecision.holdoutDecisions.size()); + FeatureDecision holdoutDecision = featureDecision.holdoutDecisions.get(0); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, holdoutDecision.decisionSource); + assertEquals(ValidProjectConfigV4.HOLDOUT_LOCAL_FOR_BASIC_EXPERIMENT, holdoutDecision.experiment); + assertEquals(VARIATION_HOLDOUT_VARIATION_OFF, holdoutDecision.variation); logbackVerifier.expectMessage(Level.INFO, "User (any_user) is in variation (ho_off_key) of holdout (local_holdout_basic_experiment)."); @@ -1986,8 +1991,8 @@ public void excludeTargetedDeliveries_globalHoldoutTrue_skipsExperimentRules() { assertTrue("With excludeTargetedDeliveries=true and no rollout, variation should be null", decision == null || decision.variation == null); if (decision != null) { - assertNotNull("holdoutDecision should be attached", decision.holdoutDecision); - assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); + assertEquals("holdoutDecision should be attached", 1, decision.holdoutDecisions.size()); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); } } @@ -2025,8 +2030,8 @@ public void excludeTargetedDeliveries_globalHoldoutTrue_noDeliveryMatch_returnsN assertTrue("When excludeTargetedDeliveries=true and no delivery match, variation should be null", decision == null || decision.variation == null); if (decision != null) { - assertNotNull("holdoutDecision should be attached", decision.holdoutDecision); - assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); + assertEquals("holdoutDecision should be attached", 1, decision.holdoutDecisions.size()); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); } } @@ -2062,8 +2067,9 @@ public void excludeTargetedDeliveries_localHoldoutTrue_appliesForExperimentRules ).getResult(); assertNotNull(decision); - assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource); - assertEquals(HOLDOUT_LOCAL_EXCLUDE_TARGETED_DELIVERIES, decision.experiment); + assertNotEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource); + assertEquals(1, decision.holdoutDecisions.size()); + assertEquals(HOLDOUT_LOCAL_EXCLUDE_TARGETED_DELIVERIES, decision.holdoutDecisions.get(0).experiment); } @Test @@ -2123,8 +2129,9 @@ public void localHoldout_ignoresExcludeTargetedDeliveries() { ).getResult(); assertNotNull(decision); - assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource); - assertEquals(HOLDOUT_LOCAL_EXCLUDE_TARGETED_DELIVERIES, decision.experiment); + assertNotEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.decisionSource); + assertEquals(1, decision.holdoutDecisions.size()); + assertEquals(HOLDOUT_LOCAL_EXCLUDE_TARGETED_DELIVERIES, decision.holdoutDecisions.get(0).experiment); } @Test @@ -2162,10 +2169,10 @@ public void globalHoldout_excludeTD_holdoutDecisionAttached() { assertNotNull(decision); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, decision.decisionSource); - assertNotNull("holdoutDecision should be attached when user is in holdout with excludeTargetedDeliveries=true", - decision.holdoutDecision); - assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); - assertEquals(HOLDOUT_GLOBAL_EXCLUDE_TARGETED_DELIVERIES, decision.holdoutDecision.experiment); + assertEquals("holdoutDecision should be attached when user is in holdout with excludeTargetedDeliveries=true", + 1, decision.holdoutDecisions.size()); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); + assertEquals(HOLDOUT_GLOBAL_EXCLUDE_TARGETED_DELIVERIES, decision.holdoutDecisions.get(0).experiment); } @Test diff --git a/core-api/src/test/resources/config/local-holdouts-project-config.json b/core-api/src/test/resources/config/local-holdouts-project-config.json new file mode 100644 index 000000000..208d718ca --- /dev/null +++ b/core-api/src/test/resources/config/local-holdouts-project-config.json @@ -0,0 +1,209 @@ +{ + "accountId": "2360254204", + "anonymizeIP": true, + "botFiltering": false, + "sendFlagDecisions": true, + "projectId": "3918735994", + "revision": "1", + "sdkKey": "LocalHoldoutsProjectConfig", + "environmentKey": "production", + "version": "4", + "audiences": [], + "typedAudiences": [ + { + "id": "5100000001", + "name": "lh_exp1_audience", + "conditions": ["and", ["or", ["or", {"name": "lh", "type": "custom_attribute", "match": "substring", "value": "exp1"}]]] + }, + { + "id": "5100000002", + "name": "lh_exp2_audience", + "conditions": ["and", ["or", ["or", {"name": "lh", "type": "custom_attribute", "match": "substring", "value": "exp2"}]]] + }, + { + "id": "5100000003", + "name": "lh_delivery_audience", + "conditions": ["and", ["or", ["or", {"name": "lh", "type": "custom_attribute", "match": "substring", "value": "delivery"}]]] + }, + { + "id": "5100000004", + "name": "lh_everyone_else_audience", + "conditions": ["and", ["or", ["or", {"name": "lh", "type": "custom_attribute", "match": "substring", "value": "everyone"}]]] + }, + { + "id": "5100000005", + "name": "lh_multi_audience", + "conditions": ["and", ["or", ["or", {"name": "lh", "type": "custom_attribute", "match": "substring", "value": "multi"}]]] + } + ], + "attributes": [ + {"id": "5000000001", "key": "lh"} + ], + "events": [], + "experiments": [ + { + "id": "5200000001", + "key": "exp_rule_1", + "layerId": "5200000101", + "status": "Running", + "variations": [{"id": "5200000011", "key": "exp_1_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5200000011", "endOfRange": 10000}], + "audienceIds": [], + "forcedVariations": {} + }, + { + "id": "5200000002", + "key": "exp_rule_2", + "layerId": "5200000102", + "status": "Running", + "variations": [{"id": "5200000012", "key": "exp_2_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5200000012", "endOfRange": 10000}], + "audienceIds": [], + "forcedVariations": {} + } + ], + "holdouts": [], + "localHoldouts": [ + { + "id": "5300000001", + "key": "lh_exp_rule_1", + "status": "Running", + "audienceIds": ["5100000001"], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5200000001"] + }, + { + "id": "5300000002", + "key": "lh_exp_rule_2", + "status": "Running", + "audienceIds": ["5100000002"], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5200000002"] + }, + { + "id": "5300000003", + "key": "lh_delivery_rule", + "status": "Running", + "audienceIds": ["5100000003"], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5400000001"] + }, + { + "id": "5300000004", + "key": "lh_everyone_else_rule", + "status": "Running", + "audienceIds": ["5100000004"], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5400000002"] + }, + { + "id": "5300000005", + "key": "lh_both_exp_rules", + "status": "Running", + "audienceIds": ["5100000005"], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5200000001", "5200000002"] + }, + { + "id": "5300000006", + "key": "lh_first_delivery_rule", + "status": "Running", + "audienceIds": [], + "trafficAllocation": [{"endOfRange": 10000, "entityId": "$opt_dummy_variation_id"}], + "variations": [{"featureEnabled": false, "id": "$opt_dummy_variation_id", "key": "ho_off_key"}], + "includedRules": ["5400000003"] + } + ], + "groups": [], + "featureFlags": [ + { + "id": "5500000001", + "key": "lh_flag", + "rolloutId": "5400000100", + "experimentIds": ["5200000001", "5200000002"], + "variables": [] + }, + { + "id": "5500000002", + "key": "lh_delivery_flag", + "rolloutId": "5400000100", + "experimentIds": [], + "variables": [] + }, + { + "id": "5500000003", + "key": "lh_multi_delivery_flag", + "rolloutId": "5400000200", + "experimentIds": [], + "variables": [] + } + ], + "rollouts": [ + { + "id": "5400000100", + "experiments": [ + { + "id": "5400000001", + "key": "delivery_rule", + "status": "Running", + "layerId": "5400000100", + "audienceIds": [], + "forcedVariations": {}, + "variations": [{"id": "5400000011", "key": "delivery_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5400000011", "endOfRange": 10000}] + }, + { + "id": "5400000002", + "key": "everyone_else_rule", + "status": "Running", + "layerId": "5400000100", + "audienceIds": [], + "forcedVariations": {}, + "variations": [{"id": "5400000012", "key": "everyone_else_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5400000012", "endOfRange": 10000}] + } + ] + }, + { + "id": "5400000200", + "experiments": [ + { + "id": "5400000003", + "key": "first_delivery_rule", + "status": "Running", + "layerId": "5400000200", + "audienceIds": [], + "forcedVariations": {}, + "variations": [{"id": "5400000013", "key": "first_delivery_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5400000013", "endOfRange": 10000}] + }, + { + "id": "5400000004", + "key": "second_delivery_rule", + "status": "Running", + "layerId": "5400000200", + "audienceIds": [], + "forcedVariations": {}, + "variations": [{"id": "5400000014", "key": "second_delivery_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5400000014", "endOfRange": 10000}] + }, + { + "id": "5400000005", + "key": "multi_everyone_else_rule", + "status": "Running", + "layerId": "5400000200", + "audienceIds": [], + "forcedVariations": {}, + "variations": [{"id": "5400000015", "key": "multi_everyone_else_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [{"entityId": "5400000015", "endOfRange": 10000}] + } + ] + } + ], + "integrations": [] +} From d7f392edebb9647ef22549476b2c808c0ba4017b Mon Sep 17 00:00:00 2001 From: Alex Yong Date: Fri, 16 Oct 2026 09:36:26 -0400 Subject: [PATCH 2/3] [AI-FSSDK] [FSSDK-12958] Keep deprecated holdoutDecision alongside holdoutDecisions --- .../ab/bucketing/DecisionService.java | 9 +++ .../ab/bucketing/FeatureDecision.java | 15 +++++ .../ab/bucketing/DecisionServiceTest.java | 61 +++++++++++++++++++ 3 files changed, 85 insertions(+) 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 0082b0a53..bfda827d9 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 @@ -362,6 +362,7 @@ public List> getVariationsForFeatureList(@Non boolean error = decisionVariationResponse.isError(); if (decision != null && (decision.variation != null || error)) { + syncDeprecatedHoldoutDecision(decision); decisions.add(new DecisionResponse(decision, reasons, error, decision.cmabUuid)); continue flagLoop; } @@ -380,6 +381,7 @@ public List> getVariationsForFeatureList(@Non } decision.holdoutDecisions.addAll(0, holdoutDecisions); } + syncDeprecatedHoldoutDecision(decision); if (decision != null && decision.variation != null) { String message = reasons.addInfo("The user \"%s\" was bucketed into a rollout for feature flag \"%s\".", @@ -401,6 +403,13 @@ public List> getVariationsForFeatureList(@Non return decisions; } + @SuppressWarnings("deprecation") + private static void syncDeprecatedHoldoutDecision(@Nullable FeatureDecision decision) { + if (decision != null && !decision.holdoutDecisions.isEmpty()) { + decision.setHoldoutDecision(decision.holdoutDecisions.get(0)); + } + } + @Nonnull public DecisionResponse getVariationForFeature(@Nonnull FeatureFlag featureFlag, @Nonnull OptimizelyUserContext user, diff --git a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java index 4b561846e..9b45fca7a 100644 --- a/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java +++ b/core-api/src/main/java/com/optimizely/ab/bucketing/FeatureDecision.java @@ -49,9 +49,24 @@ public class FeatureDecision { @Nullable public String cmabUuid; + /** + * @deprecated use {@link #holdoutDecisions} + */ + @Deprecated + @Nullable + public FeatureDecision holdoutDecision; + @Nonnull public List holdoutDecisions = new ArrayList<>(); + /** + * @deprecated use {@link #holdoutDecisions} + */ + @Deprecated + public void setHoldoutDecision(@Nullable FeatureDecision holdoutDecision) { + this.holdoutDecision = holdoutDecision; + } + public enum DecisionSource { FEATURE_TEST("feature-test"), ROLLOUT("rollout"), diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index b29b799b4..d23bf6c2c 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -29,6 +29,7 @@ import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertSame; import static org.junit.Assert.assertTrue; import org.junit.Before; import org.junit.Rule; @@ -59,6 +60,9 @@ import com.optimizely.ab.cmab.service.CmabService; import com.optimizely.ab.cmab.service.CmabDecision; import com.optimizely.ab.config.Cmab; +import com.google.common.base.Charsets; +import com.google.common.io.Resources; +import com.optimizely.ab.config.DatafileProjectConfig; import com.optimizely.ab.config.DatafileProjectConfigTestUtils; import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.noAudienceProjectConfigV3; import static com.optimizely.ab.config.DatafileProjectConfigTestUtils.validProjectConfigV3; @@ -1864,6 +1868,52 @@ public void localHoldout_hitIsRecordedAndEvaluationContinues() { "User (any_user) is in variation (ho_off_key) of holdout (local_holdout_basic_experiment)."); } + private ProjectConfig localHoldoutsProjectConfig() throws Exception { + String datafile = Resources.toString(Resources.getResource("config/local-holdouts-project-config.json"), Charsets.UTF_8); + return new DatafileProjectConfig.Builder().withDatafile(datafile).build(); + } + + @Test + @SuppressWarnings("deprecation") + public void localHoldout_hitsOnSeveralRules_deprecatedHoldoutDecisionIsFirstHit() throws Exception { + ProjectConfig config = localHoldoutsProjectConfig(); + DecisionService decisionService = new DecisionService(new Bucketer(), mockErrorHandler, null, mockCmabService); + + FeatureDecision decision = decisionService.getVariationForFeature( + config.getFeatureKeyMapping().get("lh_flag"), + optimizely.createUserContext("any_user", Collections.singletonMap("lh", "exp1,exp2")), + config + ).getResult(); + + assertNotNull(decision); + assertEquals(FeatureDecision.DecisionSource.ROLLOUT, decision.decisionSource); + assertEquals("delivery_rule", decision.experiment.getKey()); + assertEquals(2, decision.holdoutDecisions.size()); + assertEquals("lh_exp_rule_1", decision.holdoutDecisions.get(0).experiment.getKey()); + assertEquals("lh_exp_rule_2", decision.holdoutDecisions.get(1).experiment.getKey()); + assertSame(decision.holdoutDecisions.get(0), decision.holdoutDecision); + } + + @Test + @SuppressWarnings("deprecation") + public void localHoldout_hitThenServedByLaterExperimentRule_deprecatedHoldoutDecisionIsFirstHit() throws Exception { + ProjectConfig config = localHoldoutsProjectConfig(); + DecisionService decisionService = new DecisionService(new Bucketer(), mockErrorHandler, null, mockCmabService); + + FeatureDecision decision = decisionService.getVariationForFeature( + config.getFeatureKeyMapping().get("lh_flag"), + optimizely.createUserContext("any_user", Collections.singletonMap("lh", "exp1")), + config + ).getResult(); + + assertNotNull(decision); + assertEquals(FeatureDecision.DecisionSource.FEATURE_TEST, decision.decisionSource); + assertEquals("exp_rule_2", decision.experiment.getKey()); + assertEquals(1, decision.holdoutDecisions.size()); + assertEquals("lh_exp_rule_1", decision.holdoutDecisions.get(0).experiment.getKey()); + assertSame(decision.holdoutDecisions.get(0), decision.holdoutDecision); + } + /** * Local holdout miss: when a user does not hit the local holdout, they fall through * to regular rule evaluation. @@ -1976,6 +2026,7 @@ public void excludeTargetedDeliveries_globalHoldoutFalse_blocksAllRules() { } @Test + @SuppressWarnings("deprecation") public void excludeTargetedDeliveries_globalHoldoutTrue_skipsExperimentRules() { ProjectConfig config = ValidProjectConfigV4.generateValidProjectConfigV4_globalHoldoutExcludeTargetedDeliveries(); @@ -1991,6 +2042,8 @@ public void excludeTargetedDeliveries_globalHoldoutTrue_skipsExperimentRules() { assertTrue("With excludeTargetedDeliveries=true and no rollout, variation should be null", decision == null || decision.variation == null); if (decision != null) { + assertNotNull("holdoutDecision should be attached", decision.holdoutDecision); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); assertEquals("holdoutDecision should be attached", 1, decision.holdoutDecisions.size()); assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); } @@ -2015,6 +2068,7 @@ public void excludeTargetedDeliveries_globalHoldoutTrue_allowsDeliveryRules() { } @Test + @SuppressWarnings("deprecation") public void excludeTargetedDeliveries_globalHoldoutTrue_noDeliveryMatch_returnsNullWithHoldoutAttached() { ProjectConfig config = ValidProjectConfigV4.generateValidProjectConfigV4_globalHoldoutExcludeTargetedDeliveries(); @@ -2030,6 +2084,8 @@ public void excludeTargetedDeliveries_globalHoldoutTrue_noDeliveryMatch_returnsN assertTrue("When excludeTargetedDeliveries=true and no delivery match, variation should be null", decision == null || decision.variation == null); if (decision != null) { + assertNotNull("holdoutDecision should be attached", decision.holdoutDecision); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); assertEquals("holdoutDecision should be attached", 1, decision.holdoutDecisions.size()); assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); } @@ -2155,6 +2211,7 @@ public void globalHoldout_excludeTD_rolloutReturnsNull_returnsNull() { } @Test + @SuppressWarnings("deprecation") public void globalHoldout_excludeTD_holdoutDecisionAttached() { ProjectConfig config = ValidProjectConfigV4.generateValidProjectConfigV4_globalHoldoutExcludeTargetedDeliveries(); @@ -2169,6 +2226,10 @@ public void globalHoldout_excludeTD_holdoutDecisionAttached() { assertNotNull(decision); assertEquals(FeatureDecision.DecisionSource.ROLLOUT, decision.decisionSource); + assertNotNull("holdoutDecision should be attached when user is in holdout with excludeTargetedDeliveries=true", + decision.holdoutDecision); + assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecision.decisionSource); + assertEquals(HOLDOUT_GLOBAL_EXCLUDE_TARGETED_DELIVERIES, decision.holdoutDecision.experiment); assertEquals("holdoutDecision should be attached when user is in holdout with excludeTargetedDeliveries=true", 1, decision.holdoutDecisions.size()); assertEquals(FeatureDecision.DecisionSource.HOLDOUT, decision.holdoutDecisions.get(0).decisionSource); From 5f4f11b7ceeeb2d27003b96fb89b523353d31c4b Mon Sep 17 00:00:00 2001 From: Alex Yong Date: Fri, 16 Oct 2026 17:57:28 -0400 Subject: [PATCH 3/3] [AI-FSSDK] [FSSDK-12958] Test decision error discards local holdout events --- .../ab/OptimizelyUserContextTest.java | 26 +++++++++++++++++++ .../config/local-holdouts-project-config.json | 18 +++++++++++++ 2 files changed, 44 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java index 817220d69..abdf83901 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyUserContextTest.java @@ -2358,4 +2358,30 @@ public void isFeatureEnabled_skipsLocallyHeldOutRuleWithoutHoldoutEvent() throws expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs); } + @Test + public void decideForKeys_localHoldoutHitThenCmabError_returnsErrorAndSendsNoEventsForFlag() throws Exception { + String datafile = Resources.toString(Resources.getResource("config/local-holdouts-project-config.json"), Charsets.UTF_8); + com.optimizely.ab.cmab.service.CmabService mockCmabService = mock(com.optimizely.ab.cmab.service.CmabService.class); + when(mockCmabService.getDecision(any(), any(), any(), any())).thenThrow(new RuntimeException("CMAB fetch failed")); + Optimizely opt = new Optimizely.Builder() + .withDatafile(datafile) + .withEventProcessor(new ForwardingEventProcessor(eventHandler, null)) + .withCmabService(mockCmabService) + .build(); + Map attrs = Collections.singletonMap("lh", "exp1"); + + Map decisions = opt.createUserContext(userId, attrs) + .decideForKeys(Arrays.asList("lh_cmab_flag", "lh_flag")); + + OptimizelyDecision cmabDecision = decisions.get("lh_cmab_flag"); + assertNull(cmabDecision.getVariationKey()); + assertFalse(cmabDecision.getEnabled()); + assertNull(cmabDecision.getRuleKey()); + assertTrue(cmabDecision.getReasons().contains("Failed to fetch CMAB data for experiment cmab_rule.")); + + assertEquals("exp_2_on", decisions.get("lh_flag").getVariationKey()); + expectRuleImpression("5200000002", "5200000012", "lh_flag", "exp_rule_2", "feature-test", "exp_2_on", attrs); + expectLocalHoldoutImpression("5300000001", "lh_exp_rule_1", "lh_flag", attrs); + } + } diff --git a/core-api/src/test/resources/config/local-holdouts-project-config.json b/core-api/src/test/resources/config/local-holdouts-project-config.json index 208d718ca..449de8c9f 100644 --- a/core-api/src/test/resources/config/local-holdouts-project-config.json +++ b/core-api/src/test/resources/config/local-holdouts-project-config.json @@ -60,6 +60,17 @@ "trafficAllocation": [{"entityId": "5200000012", "endOfRange": 10000}], "audienceIds": [], "forcedVariations": {} + }, + { + "id": "5200000003", + "key": "cmab_rule", + "layerId": "5200000103", + "status": "Running", + "variations": [{"id": "5200000013", "key": "cmab_on", "featureEnabled": true, "variables": []}], + "trafficAllocation": [], + "audienceIds": [], + "forcedVariations": {}, + "cmab": {"attributeIds": [], "trafficAllocation": 10000} } ], "holdouts": [], @@ -141,6 +152,13 @@ "rolloutId": "5400000200", "experimentIds": [], "variables": [] + }, + { + "id": "5500000004", + "key": "lh_cmab_flag", + "rolloutId": "5400000100", + "experimentIds": ["5200000001", "5200000003"], + "variables": [] } ], "rollouts": [