From cda2e61ffd89f40203c0555a0affb5a225d0720a Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Tue, 21 Jul 2026 13:34:04 -0500 Subject: [PATCH 1/3] [AI-FSSDK] [FSSDK-12735] Add exclude_targeted_deliveries support to holdout logic --- .../DecisionServiceHoldoutTest.cs | 249 ++++++- .../TestData/HoldoutTestData.json | 628 +++++++++++++++++- .../UtilsTests/HoldoutConfigBasicTests.cs | 2 +- OptimizelySDK/Bucketing/DecisionService.cs | 75 ++- OptimizelySDK/Entity/Experiment.cs | 4 +- OptimizelySDK/Entity/Holdout.cs | 4 + 6 files changed, 924 insertions(+), 38 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index b85ee7da..0afde899 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs @@ -334,7 +334,7 @@ public void TestImpressionEventForHoldout_DisableDecisionEvent() } // ===================================================================== - // Level 2: Decision Service Tests for Local Holdouts (FSSDK-12369) + // Level 2: Decision Service Tests for Local Holdouts // ===================================================================== private DatafileProjectConfig LocalHoldoutsConfig; @@ -643,5 +643,252 @@ public void TestLocalHoldouts_AppliesToExperimentRules() Assert.AreEqual("local_holdout_exp_rule1", decision.Experiment?.Key, "Decision experiment should be the local holdout targeting exp_rule_id_1"); } + + // ===================================================================== + // Exclude Targeted Deliveries Tests + // ===================================================================== + + private DatafileProjectConfig ExcludeTDConfig; + private Optimizely ExcludeTDOptimizely; + private DatafileProjectConfig NoExcludeTDConfig; + private Optimizely NoExcludeTDOptimizely; + + private void InitializeExcludeTDConfig() + { + var datafile = TestData["datafileWithExcludeTargetedDeliveries"].ToString(); + ExcludeTDConfig = DatafileProjectConfig.Create( + datafile, LoggerMock.Object, + new NoOpErrorHandler()) as DatafileProjectConfig; + + var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object); + ExcludeTDOptimizely = new Optimizely( + datafile, eventDispatcher, LoggerMock.Object, new NoOpErrorHandler()); + } + + private void InitializeNoExcludeTDConfig() + { + var datafile = TestData["datafileWithoutExcludeTargetedDeliveries"].ToString(); + NoExcludeTDConfig = DatafileProjectConfig.Create( + datafile, LoggerMock.Object, + new NoOpErrorHandler()) as DatafileProjectConfig; + + var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object); + NoExcludeTDOptimizely = new Optimizely( + datafile, eventDispatcher, LoggerMock.Object, new NoOpErrorHandler()); + } + + [Test] + public void TestExcludeTargetedDeliveries_DefaultFalse_HoldoutAppliesNormally() + { + InitializeNoExcludeTDConfig(); + + var globalHoldout = NoExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsFalse(globalHoldout.ExcludeTargetedDeliveries); + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = NoExcludeTDConfig.FeatureKeyMap["test_flag_1"]; + var userContext = new OptimizelyUserContext(NoExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, NoExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "Global holdout with exclude_targeted_deliveries=false should apply to TD rules"); + } + + [Test] + public void TestExcludeTargetedDeliveries_True_TDRuleEvaluatesNormally() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_1"]; + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_ROLLOUT, decision.Source, + "With exclude_targeted_deliveries=true, TD rules should evaluate normally (not blocked by holdout)"); + } + + [Test] + public void TestExcludeTargetedDeliveries_True_ABRuleStillBlocked() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_2"]; + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "With exclude_targeted_deliveries=true, A/B experiment rules should still be blocked by holdout"); + } + + [Test] + public void TestExcludeTargetedDeliveries_MissingField_DefaultsFalse() + { + InitializeNoExcludeTDConfig(); + + var globalHoldout = NoExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsFalse(globalHoldout.ExcludeTargetedDeliveries, + "Missing exclude_targeted_deliveries should default to false"); + + foreach (var localHoldout in NoExcludeTDConfig.LocalHoldouts) + { + Assert.IsFalse(localHoldout.ExcludeTargetedDeliveries, + $"Local holdout {localHoldout.Key} should default exclude_targeted_deliveries to false"); + } + } + + [Test] + public void TestLocalHoldout_ExcludeTargetedDeliveries_True_SkippedForTDRule() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + globalHoldout.TrafficAllocation = new TrafficAllocation[0]; + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_1"]; + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreNotEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "Local holdout with exclude_targeted_deliveries=true should be skipped for TD rules"); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_ROLLOUT, decision.Source, + "TD rule should evaluate normally when local holdout excludes targeted deliveries"); + } + + [Test] + public void TestLocalHoldout_ExcludeTargetedDeliveries_True_StillAppliesForABRule() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + globalHoldout.TrafficAllocation = new TrafficAllocation[0]; + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_2"]; + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "Local holdout with exclude_targeted_deliveries=true should still apply for A/B rules"); + } + + [Test] + public void TestGlobalHoldout_ExcludeTargetedDeliveries_NoTDRuleMatch_FallsBackToHoldout() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + // test_flag_1 has only rollout (TD) rules, no experiments. + // With exclude_targeted_deliveries=true, TD rules evaluate normally. + // But test_flag_2 has experiments. If no experiment matches traffic, + // the holdout decision should still be returned. + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_2"]; + + // Zero out experiment traffic so no experiment matches + foreach (var exp in ExcludeTDConfig.ExperimentIdMap.Values) + { + if (featureFlag.ExperimentIds.Contains(exp.Id)) + { + exp.TrafficAllocation = new List().ToArray(); + } + } + + // Zero out local holdout traffic + foreach (var lh in ExcludeTDConfig.LocalHoldouts) + { + lh.TrafficAllocation = new TrafficAllocation[0]; + } + + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "When no TD rule matches and experiments are blocked, holdout decision should be returned as fallback"); + } } } + diff --git a/OptimizelySDK.Tests/TestData/HoldoutTestData.json b/OptimizelySDK.Tests/TestData/HoldoutTestData.json index 2c440ed8..c86d5b19 100644 --- a/OptimizelySDK.Tests/TestData/HoldoutTestData.json +++ b/OptimizelySDK.Tests/TestData/HoldoutTestData.json @@ -44,7 +44,10 @@ ], "audienceIds": [], "audienceConditions": [], - "includedFlags": ["flag_1", "flag_2"], + "includedFlags": [ + "flag_1", + "flag_2" + ], "excludedFlags": [] }, "excludedFlagsHoldout": { @@ -69,7 +72,10 @@ "audienceIds": [], "audienceConditions": [], "includedFlags": [], - "excludedFlags": ["flag_3", "flag_4"] + "excludedFlags": [ + "flag_3", + "flag_4" + ] }, "datafileWithHoldouts": { "version": "4", @@ -170,7 +176,10 @@ ], "audienceIds": [], "audienceConditions": [], - "includedFlags": ["flag_1", "flag_2"], + "includedFlags": [ + "flag_1", + "flag_2" + ], "excludedFlags": [] }, { @@ -195,7 +204,10 @@ "audienceIds": [], "audienceConditions": [], "includedFlags": [], - "excludedFlags": ["flag_3", "flag_4"] + "excludedFlags": [ + "flag_3", + "flag_4" + ] }, { "id": "holdout_empty_1", @@ -277,7 +289,9 @@ ], "audienceIds": [], "audienceConditions": [], - "includedRules": ["should_be_ignored_rule"] + "includedRules": [ + "should_be_ignored_rule" + ] } ], "localHoldouts": [ @@ -302,7 +316,9 @@ ], "audienceIds": [], "audienceConditions": [], - "includedRules": ["rule_a"] + "includedRules": [ + "rule_a" + ] }, { "id": "section_local_invalid", @@ -474,7 +490,10 @@ { "id": "flag_2", "key": "test_flag_2", - "experimentIds": ["exp_rule_id_1", "exp_rule_id_2"], + "experimentIds": [ + "exp_rule_id_1", + "exp_rule_id_2" + ], "rolloutId": "", "variables": [] }, @@ -532,7 +551,9 @@ ], "audienceIds": [], "audienceConditions": [], - "includedRules": ["rule_id_1"] + "includedRules": [ + "rule_id_1" + ] }, { "id": "holdout_local_rule2", @@ -555,7 +576,9 @@ ], "audienceIds": [], "audienceConditions": [], - "includedRules": ["rule_id_2"] + "includedRules": [ + "rule_id_2" + ] }, { "id": "holdout_local_empty_rules", @@ -601,7 +624,592 @@ ], "audienceIds": [], "audienceConditions": [], - "includedRules": ["exp_rule_id_1"] + "includedRules": [ + "exp_rule_id_1" + ] + } + ] + }, + "datafileWithExcludeTargetedDeliveries": { + "version": "4", + "rollouts": [ + { + "id": "rollout_1", + "experiments": [ + { + "id": "rule_id_1", + "key": "delivery_rule_1", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "rule1_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "rule1_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + }, + { + "id": "rule_id_2", + "key": "delivery_rule_2", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "rule2_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "rule2_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + }, + { + "id": "rule_id_everyone_else", + "key": "everyone_else_rule", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "everyone_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "everyone_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + } + ] + } + ], + "projectId": "test_project", + "experiments": [ + { + "id": "exp_rule_id_1", + "key": "experiment_rule_1", + "status": "Running", + "layerId": "exp_layer_1", + "variations": [ + { + "id": "exp1_var_1", + "key": "variation_a", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp1_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {} + }, + { + "id": "exp_rule_id_2", + "key": "experiment_rule_2", + "status": "Running", + "layerId": "exp_layer_2", + "variations": [ + { + "id": "exp2_var_1", + "key": "variation_b", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp2_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {} + } + ], + "groups": [], + "attributes": [], + "audiences": [], + "layers": [], + "events": [], + "revision": "2", + "accountId": "12345", + "anonymizeIP": false, + "featureFlags": [ + { + "id": "flag_1", + "key": "test_flag_1", + "experimentIds": [], + "rolloutId": "rollout_1", + "variables": [] + }, + { + "id": "flag_2", + "key": "test_flag_2", + "experimentIds": [ + "exp_rule_id_1", + "exp_rule_id_2" + ], + "rolloutId": "", + "variables": [] + }, + { + "id": "flag_3", + "key": "test_flag_3", + "experimentIds": [], + "rolloutId": "", + "variables": [] + } + ], + "holdouts": [ + { + "id": "holdout_global_2", + "key": "global_holdout_2", + "status": "Running", + "layerId": "layer_g2", + "variations": [ + { + "id": "gvar_1", + "key": "holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "gvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "exclude_targeted_deliveries": true + } + ], + "localHoldouts": [ + { + "id": "holdout_local_rule1", + "key": "local_holdout_rule1", + "status": "Running", + "layerId": "layer_local_1", + "variations": [ + { + "id": "lvar_1", + "key": "local_holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "rule_id_1" + ], + "exclude_targeted_deliveries": true + }, + { + "id": "holdout_local_rule2", + "key": "local_holdout_rule2", + "status": "Running", + "layerId": "layer_local_2", + "variations": [ + { + "id": "lvar_2", + "key": "local_holdout_off_2", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_2", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "rule_id_2" + ], + "exclude_targeted_deliveries": true + }, + { + "id": "holdout_local_empty_rules", + "key": "local_holdout_empty_rules", + "status": "Running", + "layerId": "layer_local_empty", + "variations": [ + { + "id": "lvar_empty", + "key": "local_empty_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_empty", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [], + "exclude_targeted_deliveries": true + }, + { + "id": "holdout_local_exp_rule1", + "key": "local_holdout_exp_rule1", + "status": "Running", + "layerId": "layer_local_exp1", + "variations": [ + { + "id": "exp_lvar_1", + "key": "local_exp_holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp_lvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "exp_rule_id_1" + ], + "exclude_targeted_deliveries": true + } + ] + }, + "datafileWithoutExcludeTargetedDeliveries": { + "version": "4", + "rollouts": [ + { + "id": "rollout_1", + "experiments": [ + { + "id": "rule_id_1", + "key": "delivery_rule_1", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "rule1_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "rule1_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + }, + { + "id": "rule_id_2", + "key": "delivery_rule_2", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "rule2_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "rule2_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + }, + { + "id": "rule_id_everyone_else", + "key": "everyone_else_rule", + "status": "Running", + "layerId": "rollout_layer_1", + "variations": [ + { + "id": "everyone_var_1", + "key": "enabled", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "everyone_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {}, + "type": "td" + } + ] + } + ], + "projectId": "test_project", + "experiments": [ + { + "id": "exp_rule_id_1", + "key": "experiment_rule_1", + "status": "Running", + "layerId": "exp_layer_1", + "variations": [ + { + "id": "exp1_var_1", + "key": "variation_a", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp1_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {} + }, + { + "id": "exp_rule_id_2", + "key": "experiment_rule_2", + "status": "Running", + "layerId": "exp_layer_2", + "variations": [ + { + "id": "exp2_var_1", + "key": "variation_b", + "featureEnabled": true, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp2_var_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "forcedVariations": {} + } + ], + "groups": [], + "attributes": [], + "audiences": [], + "layers": [], + "events": [], + "revision": "2", + "accountId": "12345", + "anonymizeIP": false, + "featureFlags": [ + { + "id": "flag_1", + "key": "test_flag_1", + "experimentIds": [], + "rolloutId": "rollout_1", + "variables": [] + }, + { + "id": "flag_2", + "key": "test_flag_2", + "experimentIds": [ + "exp_rule_id_1", + "exp_rule_id_2" + ], + "rolloutId": "", + "variables": [] + }, + { + "id": "flag_3", + "key": "test_flag_3", + "experimentIds": [], + "rolloutId": "", + "variables": [] + } + ], + "holdouts": [ + { + "id": "holdout_global_2", + "key": "global_holdout_2", + "status": "Running", + "layerId": "layer_g2", + "variations": [ + { + "id": "gvar_1", + "key": "holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "gvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [] + } + ], + "localHoldouts": [ + { + "id": "holdout_local_rule1", + "key": "local_holdout_rule1", + "status": "Running", + "layerId": "layer_local_1", + "variations": [ + { + "id": "lvar_1", + "key": "local_holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "rule_id_1" + ] + }, + { + "id": "holdout_local_rule2", + "key": "local_holdout_rule2", + "status": "Running", + "layerId": "layer_local_2", + "variations": [ + { + "id": "lvar_2", + "key": "local_holdout_off_2", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_2", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "rule_id_2" + ] + }, + { + "id": "holdout_local_empty_rules", + "key": "local_holdout_empty_rules", + "status": "Running", + "layerId": "layer_local_empty", + "variations": [ + { + "id": "lvar_empty", + "key": "local_empty_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_empty", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [] + }, + { + "id": "holdout_local_exp_rule1", + "key": "local_holdout_exp_rule1", + "status": "Running", + "layerId": "layer_local_exp1", + "variations": [ + { + "id": "exp_lvar_1", + "key": "local_exp_holdout_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "exp_lvar_1", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [], + "includedRules": [ + "exp_rule_id_1" + ] } ] } diff --git a/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs b/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs index 47857b44..aa7741e9 100644 --- a/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs +++ b/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs @@ -126,7 +126,7 @@ public void TestNullHoldouts() } // ===================================================================== - // Level 1: Local Holdout / IsGlobal Classification Tests (FSSDK-12369) + // Level 1: Local Holdout / IsGlobal Classification Tests // ===================================================================== [Test] diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index a287bc59..36354f2d 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -1,5 +1,5 @@ /* -* Copyright 2017-2022, 2024 Optimizely +* Copyright 2017-2022, 2024, 2026 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -686,8 +686,7 @@ ProjectConfig config reasons); } - // Check local holdouts targeting this specific delivery rule (FSSDK-12369) - var localHoldoutResult = EvaluateLocalHoldouts(rule.Id, user, config); + var localHoldoutResult = EvaluateLocalHoldouts(rule.Id, user, config, rule.Type); reasons += localHoldoutResult.DecisionReasons; if (localHoldoutResult.ResultObject != null) { @@ -814,8 +813,7 @@ public virtual Result GetVariationForFeatureExperiment( } else { - // Check local holdouts targeting this specific experiment rule (FSSDK-12369) - var localHoldoutResult = EvaluateLocalHoldouts(experiment.Id, user, config); + var localHoldoutResult = EvaluateLocalHoldouts(experiment.Id, user, config, experiment.Type); reasons += localHoldoutResult.DecisionReasons; if (localHoldoutResult.ResultObject != null) { @@ -916,7 +914,7 @@ public virtual Result GetDecisionForFlag( var userId = user.GetUserId(); - // Check global holdouts first (highest priority — evaluated at flag level, before any rules) + FeatureDecision globalHoldoutDecision = null; var globalHoldouts = projectConfig.GetGlobalHoldouts(); foreach (var holdout in globalHoldouts) { @@ -925,24 +923,42 @@ public virtual Result GetDecisionForFlag( if (holdoutDecision.ResultObject != null) { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - $"The user \"{userId}\" is bucketed into holdout \"{holdout.Key}\" for feature flag \"{featureFlag.Key}\".")); - return Result.NewResult(holdoutDecision.ResultObject, reasons); + if (holdout.ExcludeTargetedDeliveries) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"User \"{userId}\" is in holdout \"{holdout.Key}\" which excludes targeted deliveries. Targeted delivery rules will be evaluated normally.")); + globalHoldoutDecision = holdoutDecision.ResultObject; + } + else + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"The user \"{userId}\" is bucketed into holdout \"{holdout.Key}\" for feature flag \"{featureFlag.Key}\".")); + return Result.NewResult(holdoutDecision.ResultObject, reasons); + } + break; } } - // Check if the feature flag has an experiment and the user is bucketed into that experiment. - var experimentDecision = GetVariationForFeatureExperiment(featureFlag, user, - filteredAttributes, projectConfig, options, userProfileTracker); - reasons += experimentDecision.DecisionReasons; + if (globalHoldoutDecision == null) + { + var experimentDecision = GetVariationForFeatureExperiment(featureFlag, user, + filteredAttributes, projectConfig, options, userProfileTracker); + reasons += experimentDecision.DecisionReasons; - if (experimentDecision.ResultObject != null) + if (experimentDecision.ResultObject != null) + { + return Result.NewResult(experimentDecision.ResultObject, reasons); + } + } + else { - return Result.NewResult(experimentDecision.ResultObject, reasons); + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"Skipping experiment rules for user \"{userId}\" — holdout applies to A/B and MAB rules.")); } - // Check if the feature flag has rollout and the user is bucketed into one of its rules. var rolloutDecision = GetVariationForFeatureRollout(featureFlag, user, projectConfig); reasons += rolloutDecision.DecisionReasons; @@ -953,15 +969,18 @@ public virtual Result GetDecisionForFlag( $"The user \"{userId}\" is bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); return Result.NewResult(rolloutDecision.ResultObject, reasons); } - else + + if (globalHoldoutDecision != null) { - Logger.Log(LogLevel.INFO, - reasons.AddInfo( - $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); - return Result.NewResult( - new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), - reasons); + return Result.NewResult(globalHoldoutDecision, reasons); } + + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"The user \"{userId}\" is not bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); + return Result.NewResult( + new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT), + reasons); } public virtual List> GetVariationsForFeatureList( @@ -1062,13 +1081,19 @@ private Result GetBucketingId(string userId, UserAttributes filteredAttr private Result EvaluateLocalHoldouts( string ruleId, OptimizelyUserContext user, - ProjectConfig config + ProjectConfig config, + string ruleType ) { var reasons = new DecisionReasons(); var localHoldouts = config.GetHoldoutsForRule(ruleId); foreach (var localHoldout in localHoldouts) { + if (localHoldout.ExcludeTargetedDeliveries && ruleType == Experiment.EXPERIMENT_TYPE_TD) + { + reasons.AddInfo($"Holdout \"{localHoldout.Key}\" excludes targeted deliveries — skipping for TD rule."); + continue; + } var holdoutDecision = GetVariationForHoldout(localHoldout, user, config); reasons += holdoutDecision.DecisionReasons; if (holdoutDecision.ResultObject != null) diff --git a/OptimizelySDK/Entity/Experiment.cs b/OptimizelySDK/Entity/Experiment.cs index 183a2f7d..234ac3ae 100644 --- a/OptimizelySDK/Entity/Experiment.cs +++ b/OptimizelySDK/Entity/Experiment.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017-2019, Optimizely + * Copyright 2017-2019, 2026 Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,6 +25,8 @@ public class Experiment : ExperimentCore public const string EXPERIMENT_TYPE_FR = "fr"; + public const string EXPERIMENT_TYPE_TD = "td"; + /// /// Group ID for the experiment /// diff --git a/OptimizelySDK/Entity/Holdout.cs b/OptimizelySDK/Entity/Holdout.cs index 96a33922..e4ebdaa4 100644 --- a/OptimizelySDK/Entity/Holdout.cs +++ b/OptimizelySDK/Entity/Holdout.cs @@ -15,6 +15,7 @@ */ using System.Collections.Generic; +using Newtonsoft.Json; namespace OptimizelySDK.Entity { @@ -51,6 +52,9 @@ public override string LayerId /// public string[] IncludedRules { get; set; } + [JsonProperty("exclude_targeted_deliveries")] + public bool ExcludeTargetedDeliveries { get; set; } + /// /// True when global (IncludedRules is null). Consistent with section membership /// because the config parser strips IncludedRules on 'holdouts'-section entries. From f1a57f533dd47a0b7bc281667f103406c52b9591 Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 14:47:19 -0500 Subject: [PATCH 2/3] [AI-FSSDK] [FSSDK-12735] Update holdout exclusion: global-only flag, HO events always sent, no null TD fallback --- .../DecisionServiceHoldoutTest.cs | 124 ++++++++++++++---- OptimizelySDK/Bucketing/DecisionService.cs | 23 ++-- OptimizelySDK/Entity/FeatureDecision.cs | 1 + OptimizelySDK/Optimizely.cs | 13 ++ 4 files changed, 129 insertions(+), 32 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index 0afde899..69c3d211 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs @@ -711,6 +711,11 @@ public void TestExcludeTargetedDeliveries_True_TDRuleEvaluatesNormally() { InitializeExcludeTDConfig(); + foreach (var lh in ExcludeTDConfig.LocalHoldouts) + { + lh.TrafficAllocation = new TrafficAllocation[0]; + } + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); @@ -740,6 +745,11 @@ public void TestExcludeTargetedDeliveries_True_ABRuleStillBlocked() { InitializeExcludeTDConfig(); + foreach (var lh in ExcludeTDConfig.LocalHoldouts) + { + lh.TrafficAllocation = new TrafficAllocation[0]; + } + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); @@ -761,8 +771,12 @@ public void TestExcludeTargetedDeliveries_True_ABRuleStillBlocked() var decision = result[0].ResultObject; Assert.IsNotNull(decision); - Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, - "With exclude_targeted_deliveries=true, A/B experiment rules should still be blocked by holdout"); + Assert.AreNotEqual(FeatureDecision.DECISION_SOURCE_FEATURE_TEST, decision.Source, + "With exclude_targeted_deliveries=true, A/B experiment rules should be skipped (not evaluated)"); + Assert.IsNull(decision.Variation, + "Variation should be null since A/B was blocked and no TD rule matched"); + Assert.IsNotNull(decision.HoldoutDecision, + "HoldoutDecision should be attached showing the user was in a holdout that blocked A/B rules"); } [Test] @@ -782,7 +796,7 @@ public void TestExcludeTargetedDeliveries_MissingField_DefaultsFalse() } [Test] - public void TestLocalHoldout_ExcludeTargetedDeliveries_True_SkippedForTDRule() + public void TestLocalHoldout_ExcludeTargetedDeliveries_True_StillAppliesForTDRule() { InitializeExcludeTDConfig(); @@ -806,10 +820,8 @@ public void TestLocalHoldout_ExcludeTargetedDeliveries_True_SkippedForTDRule() var decision = result[0].ResultObject; Assert.IsNotNull(decision); - Assert.AreNotEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, - "Local holdout with exclude_targeted_deliveries=true should be skipped for TD rules"); - Assert.AreEqual(FeatureDecision.DECISION_SOURCE_ROLLOUT, decision.Source, - "TD rule should evaluate normally when local holdout excludes targeted deliveries"); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, + "Local holdout must apply to TD rules even when exclude_targeted_deliveries=true (flag is only for global holdouts)"); } [Test] @@ -842,7 +854,7 @@ public void TestLocalHoldout_ExcludeTargetedDeliveries_True_StillAppliesForABRul } [Test] - public void TestGlobalHoldout_ExcludeTargetedDeliveries_NoTDRuleMatch_FallsBackToHoldout() + public void TestGlobalHoldout_ExcludeTargetedDeliveries_NoTDRuleMatch_ReturnsNullDecision() { InitializeExcludeTDConfig(); @@ -853,22 +865,14 @@ public void TestGlobalHoldout_ExcludeTargetedDeliveries_NoTDRuleMatch_FallsBackT var decisionService = new DecisionService(realBucketer, new NoOpErrorHandler(), null, LoggerMock.Object, null); - // test_flag_1 has only rollout (TD) rules, no experiments. - // With exclude_targeted_deliveries=true, TD rules evaluate normally. - // But test_flag_2 has experiments. If no experiment matches traffic, - // the holdout decision should still be returned. - var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_2"]; + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_1"]; - // Zero out experiment traffic so no experiment matches - foreach (var exp in ExcludeTDConfig.ExperimentIdMap.Values) + var rollout = ExcludeTDConfig.GetRolloutFromId(featureFlag.RolloutId); + foreach (var rule in rollout.Experiments) { - if (featureFlag.ExperimentIds.Contains(exp.Id)) - { - exp.TrafficAllocation = new List().ToArray(); - } + rule.TrafficAllocation = new List().ToArray(); } - // Zero out local holdout traffic foreach (var lh in ExcludeTDConfig.LocalHoldouts) { lh.TrafficAllocation = new TrafficAllocation[0]; @@ -886,8 +890,84 @@ public void TestGlobalHoldout_ExcludeTargetedDeliveries_NoTDRuleMatch_FallsBackT var decision = result[0].ResultObject; Assert.IsNotNull(decision); - Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.Source, - "When no TD rule matches and experiments are blocked, holdout decision should be returned as fallback"); + Assert.IsNull(decision.Variation, + "When TD returns no match with exclude_targeted_deliveries=true, variation should be null (no holdout fallback)"); + Assert.IsNull(decision.Experiment, + "When TD returns no match with exclude_targeted_deliveries=true, experiment should be null (no holdout fallback)"); + Assert.IsNotNull(decision.HoldoutDecision, + "HoldoutDecision should be attached for event dispatch even when decision is null"); + } + [Test] + public void TestExcludeTargetedDeliveries_HoldoutEventSentWhenTDBypasses() + { + InitializeExcludeTDConfig(); + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); + + var eventDispatcher = new Event.Dispatcher.DefaultEventDispatcher(LoggerMock.Object); + var optimizelyWithMockedEvents = new Optimizely( + TestData["datafileWithExcludeTargetedDeliveries"].ToString(), + eventDispatcher, + LoggerMock.Object, + new NoOpErrorHandler(), + null, + false, + EventProcessorMock.Object + ); + + EventProcessorMock.Setup(ep => ep.Process(It.IsAny())); + + var userContext = optimizelyWithMockedEvents.CreateUserContext(TestUserId, new UserAttributes()); + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_1"]; + var decision = userContext.Decide(featureFlag.Key); + + Assert.IsNotNull(decision); + + EventProcessorMock.Verify(ep => ep.Process(It.IsAny()), Times.AtLeast(2), + "Both holdout and TD impression events should be sent when holdout is bypassed for TD rules"); + + EventProcessorMock.Verify(ep => ep.Process(It.Is(ie => + ie.Experiment.Key == globalHoldout.Key + )), Times.Once, "Holdout impression event should be sent even when holdout is bypassed for TD rules"); + } + + [Test] + public void TestExcludeTargetedDeliveries_HoldoutDecisionAttachedToTDResult() + { + InitializeExcludeTDConfig(); + + foreach (var lh in ExcludeTDConfig.LocalHoldouts) + { + lh.TrafficAllocation = new TrafficAllocation[0]; + } + + var globalHoldout = ExcludeTDConfig.GetGlobalHoldouts()[0]; + Assert.IsTrue(globalHoldout.ExcludeTargetedDeliveries); + + var realBucketer = new Bucketer(LoggerMock.Object); + var decisionService = new DecisionService(realBucketer, + new NoOpErrorHandler(), null, LoggerMock.Object, null); + + var featureFlag = ExcludeTDConfig.FeatureKeyMap["test_flag_1"]; + var userContext = new OptimizelyUserContext(ExcludeTDOptimizely, TestUserId, null, + new NoOpErrorHandler(), LoggerMock.Object); + + var result = decisionService.GetVariationsForFeatureList( + new List { featureFlag }, userContext, ExcludeTDConfig, + new UserAttributes(), new OptimizelyDecideOption[0]); + + Assert.IsNotNull(result); + Assert.AreEqual(1, result.Count); + + var decision = result[0].ResultObject; + Assert.IsNotNull(decision); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_ROLLOUT, decision.Source, + "TD rule should be returned as the decision"); + Assert.IsNotNull(decision.HoldoutDecision, + "HoldoutDecision should be attached when holdout was bypassed for TD"); + Assert.AreEqual(FeatureDecision.DECISION_SOURCE_HOLDOUT, decision.HoldoutDecision.Source, + "Attached HoldoutDecision should have holdout source"); } } } diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 36354f2d..21affc6b 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -686,7 +686,7 @@ ProjectConfig config reasons); } - var localHoldoutResult = EvaluateLocalHoldouts(rule.Id, user, config, rule.Type); + var localHoldoutResult = EvaluateLocalHoldouts(rule.Id, user, config); reasons += localHoldoutResult.DecisionReasons; if (localHoldoutResult.ResultObject != null) { @@ -813,7 +813,7 @@ public virtual Result GetVariationForFeatureExperiment( } else { - var localHoldoutResult = EvaluateLocalHoldouts(experiment.Id, user, config, experiment.Type); + var localHoldoutResult = EvaluateLocalHoldouts(experiment.Id, user, config); reasons += localHoldoutResult.DecisionReasons; if (localHoldoutResult.ResultObject != null) { @@ -967,12 +967,21 @@ public virtual Result GetDecisionForFlag( Logger.Log(LogLevel.INFO, reasons.AddInfo( $"The user \"{userId}\" is bucketed into a rollout for feature flag \"{featureFlag.Key}\".")); + if (globalHoldoutDecision != null) + { + rolloutDecision.ResultObject.HoldoutDecision = globalHoldoutDecision; + } return Result.NewResult(rolloutDecision.ResultObject, reasons); } if (globalHoldoutDecision != null) { - return Result.NewResult(globalHoldoutDecision, reasons); + var nullDecision = new FeatureDecision(null, null, FeatureDecision.DECISION_SOURCE_ROLLOUT); + nullDecision.HoldoutDecision = globalHoldoutDecision; + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"The user \"{userId}\" is not bucketed into any targeted delivery for feature flag \"{featureFlag.Key}\". Holdout impression will be sent but holdout variation is not applied as fallback.")); + return Result.NewResult(nullDecision, reasons); } Logger.Log(LogLevel.INFO, @@ -1081,19 +1090,13 @@ private Result GetBucketingId(string userId, UserAttributes filteredAttr private Result EvaluateLocalHoldouts( string ruleId, OptimizelyUserContext user, - ProjectConfig config, - string ruleType + ProjectConfig config ) { var reasons = new DecisionReasons(); var localHoldouts = config.GetHoldoutsForRule(ruleId); foreach (var localHoldout in localHoldouts) { - if (localHoldout.ExcludeTargetedDeliveries && ruleType == Experiment.EXPERIMENT_TYPE_TD) - { - reasons.AddInfo($"Holdout \"{localHoldout.Key}\" excludes targeted deliveries — skipping for TD rule."); - continue; - } var holdoutDecision = GetVariationForHoldout(localHoldout, user, config); reasons += holdoutDecision.DecisionReasons; if (holdoutDecision.ResultObject != null) diff --git a/OptimizelySDK/Entity/FeatureDecision.cs b/OptimizelySDK/Entity/FeatureDecision.cs index a536def6..b340bb5a 100644 --- a/OptimizelySDK/Entity/FeatureDecision.cs +++ b/OptimizelySDK/Entity/FeatureDecision.cs @@ -26,6 +26,7 @@ public class FeatureDecision public string Source { get; } public string CmabUuid { get; } public bool Error { get; } + public FeatureDecision HoldoutDecision { get; set; } public FeatureDecision(ExperimentCore experiment, Variation variation, string source, string cmabUuid = null, bool error = false) { diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index ed4f7469..8161401e 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1125,6 +1125,19 @@ ProjectConfig projectConfig // add to event metadata as well (currently set to experimentKey) var ruleKey = flagDecision.Experiment?.Key; + if (flagDecision?.HoldoutDecision != null && !allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) + { + SendImpressionEvent( + flagDecision.HoldoutDecision.Experiment, + flagDecision.HoldoutDecision.Variation, + userId, user.GetAttributes(), projectConfig, + flagKey, FeatureDecision.DECISION_SOURCE_HOLDOUT, true +#if USE_CMAB + , null +#endif + ); + } + var decisionEventDispatched = false; if (!allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { From 35fa75d5f932701956b58bc0d0f8ac710b6165ff Mon Sep 17 00:00:00 2001 From: esrakartalOpt Date: Wed, 22 Jul 2026 16:38:37 -0500 Subject: [PATCH 3/3] [AI-FSSDK] [FSSDK-12735] Fix decisionEventDispatched for holdout impression and add excludeTargetedDeliveries bypass reason --- .../DecisionServiceHoldoutTest.cs | 24 +++++++++++++++++++ OptimizelySDK/Bucketing/DecisionService.cs | 9 +++++++ OptimizelySDK/Optimizely.cs | 7 +++--- 3 files changed, 36 insertions(+), 4 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index 69c3d211..6c6a767f 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs @@ -28,6 +28,7 @@ using OptimizelySDK.Event; using OptimizelySDK.Event.Entity; using OptimizelySDK.Logger; +using OptimizelySDK.Notifications; using OptimizelySDK.OptimizelyDecisions; using OptimizelySDK.Utils; @@ -738,6 +739,14 @@ public void TestExcludeTargetedDeliveries_True_TDRuleEvaluatesNormally() Assert.IsNotNull(decision); Assert.AreEqual(FeatureDecision.DECISION_SOURCE_ROLLOUT, decision.Source, "With exclude_targeted_deliveries=true, TD rules should evaluate normally (not blocked by holdout)"); + + var reasons = result[0].DecisionReasons.ToReport(true); + var excludeTDReason = reasons.FirstOrDefault(r => + r.Contains("has excludeTargetedDeliveries enabled, continuing to rollout evaluation")); + Assert.IsNotNull(excludeTDReason, + "Reasons should include excludeTargetedDeliveries bypass message"); + Assert.IsTrue(excludeTDReason.Contains(globalHoldout.Key), + "Reason should reference the holdout name"); } [Test] @@ -930,6 +939,21 @@ public void TestExcludeTargetedDeliveries_HoldoutEventSentWhenTDBypasses() EventProcessorMock.Verify(ep => ep.Process(It.Is(ie => ie.Experiment.Key == globalHoldout.Key )), Times.Once, "Holdout impression event should be sent even when holdout is bypassed for TD rules"); + + Dictionary lastNotification = null; + optimizelyWithMockedEvents.NotificationCenter.AddNotification( + NotificationCenter.NotificationType.Decision, + (NotificationCenter.DecisionCallback)((type, userId, userAttributes, decisionInfo) => + { + lastNotification = decisionInfo; + })); + + var decision2 = userContext.Decide(featureFlag.Key); + Assert.IsNotNull(lastNotification, "Decision notification should have been sent"); + Assert.IsTrue(lastNotification.ContainsKey("decisionEventDispatched"), + "Notification should contain decisionEventDispatched"); + Assert.IsTrue((bool)lastNotification["decisionEventDispatched"], + "decisionEventDispatched should be true when holdout impression is sent"); } [Test] diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 21affc6b..9302793f 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -915,6 +915,7 @@ public virtual Result GetDecisionForFlag( var userId = user.GetUserId(); FeatureDecision globalHoldoutDecision = null; + Holdout matchedHoldout = null; var globalHoldouts = projectConfig.GetGlobalHoldouts(); foreach (var holdout in globalHoldouts) { @@ -929,6 +930,7 @@ public virtual Result GetDecisionForFlag( reasons.AddInfo( $"User \"{userId}\" is in holdout \"{holdout.Key}\" which excludes targeted deliveries. Targeted delivery rules will be evaluated normally.")); globalHoldoutDecision = holdoutDecision.ResultObject; + matchedHoldout = holdout; } else { @@ -957,6 +959,13 @@ public virtual Result GetDecisionForFlag( Logger.Log(LogLevel.INFO, reasons.AddInfo( $"Skipping experiment rules for user \"{userId}\" — holdout applies to A/B and MAB rules.")); + + if (matchedHoldout != null && matchedHoldout.ExcludeTargetedDeliveries) + { + Logger.Log(LogLevel.INFO, + reasons.AddInfo( + $"Holdout \"{matchedHoldout.Key}\" has excludeTargetedDeliveries enabled, continuing to rollout evaluation.")); + } } var rolloutDecision = GetVariationForFeatureRollout(featureFlag, user, projectConfig); diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 8161401e..c694b4b4 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -1125,9 +1125,10 @@ ProjectConfig projectConfig // add to event metadata as well (currently set to experimentKey) var ruleKey = flagDecision.Experiment?.Key; + var decisionEventDispatched = false; if (flagDecision?.HoldoutDecision != null && !allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { - SendImpressionEvent( + decisionEventDispatched = SendImpressionEvent( flagDecision.HoldoutDecision.Experiment, flagDecision.HoldoutDecision.Variation, userId, user.GetAttributes(), projectConfig, @@ -1135,10 +1136,8 @@ ProjectConfig projectConfig #if USE_CMAB , null #endif - ); + ) || decisionEventDispatched; } - - var decisionEventDispatched = false; if (!allOptions.Contains(OptimizelyDecideOption.DISABLE_DECISION_EVENT)) { decisionEventDispatched = SendImpressionEvent(