diff --git a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs index b85ee7da..bce4b97f 100644 --- a/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs @@ -370,9 +370,13 @@ public void TestLocalHoldouts_GlobalHoldoutEvaluatedBeforePerRuleLogic() Assert.AreEqual(1, globalHoldouts.Count, "Should have exactly one global holdout"); Assert.AreEqual("holdout_global_2", globalHoldouts[0].Id, "Global holdout id should match"); - // Verify that the global holdout is classified correctly + // Verify that the global holdout is classified correctly by section membership Assert.IsTrue(globalHoldouts[0].IsGlobal, - "holdout_global_2 should be global (IncludedRules is null)"); + "holdout_global_2 should be global (from holdouts section)"); + + // Verify that IncludedRules is stripped from global holdout at parse time + Assert.IsNull(globalHoldouts[0].IncludedRules, + "IncludedRules should be stripped from global holdouts at parse time"); // Verify GetHoldoutsForRule returns empty for a delivery rule since it's global var localForRule1 = LocalHoldoutsConfig.GetHoldoutsForRule("rule_id_1"); @@ -643,5 +647,112 @@ 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"); } + + // ===================================================================== + // Level 3: localHoldouts datafile section tests (FSSDK-12792) + // ===================================================================== + + [Test] + public void TestLocalHoldoutsSection_ParsedSeparatelyFromHoldoutsSection() + { + // Verify that localHoldouts are parsed from the "localHoldouts" section + // and holdouts from the "holdouts" section + InitializeLocalHoldoutsConfig(); + + // Global holdouts come from "holdouts" section + Assert.IsNotNull(LocalHoldoutsConfig.Holdouts, "Holdouts should not be null"); + Assert.AreEqual(1, LocalHoldoutsConfig.Holdouts.Length, + "Should have 1 global holdout in the holdouts section"); + Assert.AreEqual("holdout_global_2", LocalHoldoutsConfig.Holdouts[0].Id); + Assert.IsTrue(LocalHoldoutsConfig.Holdouts[0].IsGlobal, + "Holdout from holdouts section should have IsGlobal=true"); + + // Local holdouts come from "localHoldouts" section (minus invalid ones) + Assert.IsNotNull(LocalHoldoutsConfig.LocalHoldouts, "LocalHoldouts should not be null"); + // holdout_local_empty_rules and holdout_local_no_rules are invalid (empty/null includedRules) + Assert.AreEqual(3, LocalHoldoutsConfig.LocalHoldouts.Length, + "Should have 3 valid local holdouts (empty-rules and no-rules entries excluded)"); + + foreach (var localHoldout in LocalHoldoutsConfig.LocalHoldouts) + { + Assert.IsFalse(localHoldout.IsGlobal, + $"Local holdout {localHoldout.Key} from localHoldouts section should have IsGlobal=false"); + Assert.IsNotNull(localHoldout.IncludedRules, + $"Valid local holdout {localHoldout.Key} should have non-null IncludedRules"); + Assert.IsTrue(localHoldout.IncludedRules.Length > 0, + $"Valid local holdout {localHoldout.Key} should have non-empty IncludedRules"); + } + } + + [Test] + public void TestIncludedRulesStrippedFromGlobalHoldoutsAtParseTime() + { + // The datafile has "includedRules": ["should_be_stripped"] on holdout_global_2. + // After parsing, IncludedRules should be null because section membership is the sole signal. + InitializeLocalHoldoutsConfig(); + + var globalHoldout = LocalHoldoutsConfig.Holdouts[0]; + Assert.AreEqual("holdout_global_2", globalHoldout.Id); + Assert.IsNull(globalHoldout.IncludedRules, + "IncludedRules on a global holdout (from holdouts section) must be stripped to null at parse time"); + Assert.IsTrue(globalHoldout.IsGlobal, + "Holdout from holdouts section must be global regardless of original includedRules value"); + } + + [Test] + public void TestInvalidLocalHoldout_MissingIncludedRules_ExcludedWithError() + { + // holdout_local_no_rules has no includedRules field → should be excluded with error log + // holdout_local_empty_rules has includedRules: [] → should also be excluded with error log + InitializeLocalHoldoutsConfig(); + + // Verify these invalid entries are excluded + var allLocalHoldoutIds = LocalHoldoutsConfig.LocalHoldouts.Select(h => h.Id).ToList(); + Assert.IsFalse(allLocalHoldoutIds.Contains("holdout_local_no_rules"), + "Local holdout without includedRules should be excluded from evaluation"); + Assert.IsFalse(allLocalHoldoutIds.Contains("holdout_local_empty_rules"), + "Local holdout with empty includedRules should be excluded from evaluation"); + + // Verify error was logged for invalid local holdouts + // Use AtLeastOnce because the shared LoggerMock may accumulate calls from + // other tests in this fixture that also call InitializeLocalHoldoutsConfig(). + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, + It.Is(s => s.Contains("local_holdout_no_rules") && s.Contains("missing includedRules"))), + Times.AtLeastOnce, "Should log error for local holdout missing includedRules"); + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, + It.Is(s => s.Contains("local_holdout_empty_rules") && s.Contains("missing includedRules"))), + Times.AtLeastOnce, "Should log error for local holdout with empty includedRules"); + } + + [Test] + public void TestBackwardCompatibility_DatafileWithoutLocalHoldoutsSection() + { + // The datafileWithHoldouts fixture has no "localHoldouts" key. + // All holdouts in the "holdouts" section should be treated as global. + var datafileWithHoldouts = TestData["datafileWithHoldouts"].ToString(); + var oldConfig = DatafileProjectConfig.Create(datafileWithHoldouts, LoggerMock.Object, + new NoOpErrorHandler()) as DatafileProjectConfig; + + Assert.IsNotNull(oldConfig, "Config should be created from old-format datafile"); + + // LocalHoldouts should be an empty array (initialized by default) + Assert.IsNotNull(oldConfig.LocalHoldouts, "LocalHoldouts should not be null even when absent from datafile"); + Assert.AreEqual(0, oldConfig.LocalHoldouts.Length, + "LocalHoldouts should be empty when not present in datafile"); + + // All holdouts should be global + foreach (var holdout in oldConfig.Holdouts) + { + Assert.IsTrue(holdout.IsGlobal, + $"Holdout {holdout.Key} from holdouts section should be global"); + Assert.IsNull(holdout.IncludedRules, + $"IncludedRules should be stripped from holdout {holdout.Key}"); + } + + // GetGlobalHoldouts should return all holdouts + var globals = oldConfig.GetGlobalHoldouts(); + Assert.AreEqual(oldConfig.Holdouts.Length, globals.Count, + "All holdouts from old-format datafile should be global"); + } } } diff --git a/OptimizelySDK.Tests/ProjectConfigTest.cs b/OptimizelySDK.Tests/ProjectConfigTest.cs index 83f114c9..a1e21b6d 100644 --- a/OptimizelySDK.Tests/ProjectConfigTest.cs +++ b/OptimizelySDK.Tests/ProjectConfigTest.cs @@ -1418,6 +1418,9 @@ public void TestMissingHoldoutsField_BackwardCompatibility() Assert.IsNotNull(datafileProjectConfig.Holdouts); Assert.AreEqual(0, datafileProjectConfig.Holdouts.Length); + Assert.IsNotNull(datafileProjectConfig.LocalHoldouts); + Assert.AreEqual(0, datafileProjectConfig.LocalHoldouts.Length); + // Methods should still work with empty holdouts var holdout = datafileProjectConfig.GetHoldout("any_id"); Assert.IsNull(holdout); diff --git a/OptimizelySDK.Tests/TestData/HoldoutTestData.json b/OptimizelySDK.Tests/TestData/HoldoutTestData.json index eb65ccc6..c10cc1ae 100644 --- a/OptimizelySDK.Tests/TestData/HoldoutTestData.json +++ b/OptimizelySDK.Tests/TestData/HoldoutTestData.json @@ -390,8 +390,11 @@ } ], "audienceIds": [], - "audienceConditions": [] - }, + "audienceConditions": [], + "includedRules": ["should_be_stripped"] + } + ], + "localHoldouts": [ { "id": "holdout_local_rule1", "key": "local_holdout_rule1", @@ -461,6 +464,28 @@ "audienceConditions": [], "includedRules": [] }, + { + "id": "holdout_local_no_rules", + "key": "local_holdout_no_rules", + "status": "Running", + "layerId": "layer_local_no_rules", + "variations": [ + { + "id": "lvar_no_rules", + "key": "local_no_rules_off", + "featureEnabled": false, + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "lvar_no_rules", + "endOfRange": 10000 + } + ], + "audienceIds": [], + "audienceConditions": [] + }, { "id": "holdout_local_exp_rule1", "key": "local_holdout_exp_rule1", diff --git a/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs b/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs index 47857b44..b3f50c79 100644 --- a/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs +++ b/OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs @@ -126,47 +126,38 @@ public void TestNullHoldouts() } // ===================================================================== - // Level 1: Local Holdout / IsGlobal Classification Tests (FSSDK-12369) + // Level 1: IsGlobal Classification Tests (section-based scope) // ===================================================================== [Test] - public void TestIsGlobal_NullIncludedRules_IsGlobal() + public void TestIsGlobal_DefaultIsTrue() { - // A holdout with IncludedRules == null is a global holdout + // By default, IsGlobal is true (holdout from "holdouts" section) var holdout = CreateTestHoldout("h1", "global_holdout"); - holdout.IncludedRules = null; - Assert.IsTrue(holdout.IsGlobal, "Holdout with null IncludedRules should be global"); + Assert.IsTrue(holdout.IsGlobal, "Holdout should default to global (IsGlobal=true)"); } [Test] - public void TestIsGlobal_EmptyIncludedRules_IsNotGlobal() + public void TestIsGlobal_SetToFalse_IsLocal() { - // A holdout with IncludedRules == [] is LOCAL (empty array, not null) - var holdout = CreateTestHoldout("h1", "local_holdout_empty"); - holdout.IncludedRules = new string[0]; - - Assert.IsFalse(holdout.IsGlobal, "Holdout with empty array IncludedRules should NOT be global"); - } - - [Test] - public void TestIsGlobal_NonEmptyIncludedRules_IsNotGlobal() - { - // A holdout with IncludedRules = ["rule_1"] is a local holdout + // Holdout from "localHoldouts" section has IsGlobal=false var holdout = CreateTestHoldout("h1", "local_holdout"); + holdout.IsGlobal = false; holdout.IncludedRules = new[] { "rule_1" }; - Assert.IsFalse(holdout.IsGlobal, "Holdout with non-empty IncludedRules should NOT be global"); + Assert.IsFalse(holdout.IsGlobal, "Holdout with IsGlobal=false should be local"); } [Test] public void TestGetGlobalHoldouts_ReturnsOnlyGlobalHoldouts() { var globalHoldout = CreateTestHoldout("global_id", "global_key"); - globalHoldout.IncludedRules = null; // global + globalHoldout.IsGlobal = true; var localHoldout = CreateTestHoldout("local_id", "local_key"); - localHoldout.IncludedRules = new[] { "rule_1" }; // local + localHoldout.IsGlobal = false; + localHoldout.IncludedRules = new[] { "rule_1" }; var config = new HoldoutConfig(new[] { globalHoldout, localHoldout }); @@ -179,7 +170,8 @@ public void TestGetGlobalHoldouts_ReturnsOnlyGlobalHoldouts() public void TestGetGlobalHoldouts_NoGlobalHoldouts_ReturnsEmpty() { var localHoldout = CreateTestHoldout("local_id", "local_key"); - localHoldout.IncludedRules = new[] { "rule_1" }; // local + localHoldout.IsGlobal = false; + localHoldout.IncludedRules = new[] { "rule_1" }; var config = new HoldoutConfig(new[] { localHoldout }); @@ -191,6 +183,7 @@ public void TestGetGlobalHoldouts_NoGlobalHoldouts_ReturnsEmpty() public void TestGetHoldoutsForRule_ReturnsMatchingLocalHoldout() { var localHoldout = CreateTestHoldout("local_id", "local_key"); + localHoldout.IsGlobal = false; localHoldout.IncludedRules = new[] { "rule_1", "rule_2" }; var config = new HoldoutConfig(new[] { localHoldout }); @@ -207,6 +200,7 @@ public void TestGetHoldoutsForRule_ReturnsMatchingLocalHoldout() public void TestGetHoldoutsForRule_UnknownRuleId_ReturnsEmpty() { var localHoldout = CreateTestHoldout("local_id", "local_key"); + localHoldout.IsGlobal = false; localHoldout.IncludedRules = new[] { "rule_1" }; var config = new HoldoutConfig(new[] { localHoldout }); @@ -219,6 +213,7 @@ public void TestGetHoldoutsForRule_UnknownRuleId_ReturnsEmpty() public void TestGetHoldoutsForRule_NullOrEmptyRuleId_ReturnsEmpty() { var localHoldout = CreateTestHoldout("local_id", "local_key"); + localHoldout.IsGlobal = false; localHoldout.IncludedRules = new[] { "rule_1" }; var config = new HoldoutConfig(new[] { localHoldout }); @@ -228,32 +223,30 @@ public void TestGetHoldoutsForRule_NullOrEmptyRuleId_ReturnsEmpty() } [Test] - public void TestGetHoldoutsForRule_EmptyIncludedRules_NoRuleMatches() + public void TestGetHoldoutsForRule_LocalWithNullIncludedRules_NoRuleMatches() { - // A local holdout with IncludedRules == [] matches NO rules + // A local holdout (IsGlobal=false) with null IncludedRules should not match any rules var localHoldout = CreateTestHoldout("local_id", "local_key"); - localHoldout.IncludedRules = new string[0]; // empty array = local, but no rules + localHoldout.IsGlobal = false; + localHoldout.IncludedRules = null; var config = new HoldoutConfig(new[] { localHoldout }); - // Should not appear in global holdouts - Assert.AreEqual(0, config.GetGlobalHoldouts().Count, "Empty-array holdout should not be global"); - - // Should not match any rule - Assert.AreEqual(0, config.GetHoldoutsForRule("any_rule").Count, "Empty-array holdout should match no rules"); + Assert.AreEqual(0, config.GetGlobalHoldouts().Count, "Local holdout should not be global"); + Assert.AreEqual(0, config.GetHoldoutsForRule("any_rule").Count, "Local holdout with null IncludedRules should match no rules"); } [Test] - public void TestBackwardCompatibility_NullIncludedRulesDefaultsToGlobal() + public void TestBackwardCompatibility_DefaultIsGlobalIsTrue() { - // Old datafile holdouts have no includedRules field → IncludedRules is null → global + // Holdouts default to IsGlobal=true (matching "holdouts" section behavior) var legacyHoldout = CreateTestHoldout("legacy_id", "legacy_key"); - // IncludedRules is null by default (not set) + // IsGlobal defaults to true var config = new HoldoutConfig(new[] { legacyHoldout }); var globals = config.GetGlobalHoldouts(); - Assert.AreEqual(1, globals.Count, "Legacy holdout (null IncludedRules) should be treated as global"); + Assert.AreEqual(1, globals.Count, "Holdout with default IsGlobal=true should be treated as global"); Assert.AreEqual("legacy_id", globals[0].Id); } @@ -261,9 +254,11 @@ public void TestBackwardCompatibility_NullIncludedRulesDefaultsToGlobal() public void TestMultipleLocalHoldoutsForSameRule() { var holdout1 = CreateTestHoldout("local_id_1", "local_key_1"); + holdout1.IsGlobal = false; holdout1.IncludedRules = new[] { "rule_shared" }; var holdout2 = CreateTestHoldout("local_id_2", "local_key_2"); + holdout2.IsGlobal = false; holdout2.IncludedRules = new[] { "rule_shared" }; var config = new HoldoutConfig(new[] { holdout1, holdout2 }); @@ -277,6 +272,7 @@ public void TestCrossRuleTargeting_OneHoldoutTargetsMultipleRules() { // A single local holdout can target rules from multiple flags var crossFlagHoldout = CreateTestHoldout("cross_id", "cross_key"); + crossFlagHoldout.IsGlobal = false; crossFlagHoldout.IncludedRules = new[] { "rule_flag_a", "rule_flag_b", "rule_flag_c" }; var config = new HoldoutConfig(new[] { crossFlagHoldout }); diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index a287bc59..8a5ea912 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -916,7 +916,7 @@ public virtual Result GetDecisionForFlag( var userId = user.GetUserId(); - // Check global holdouts first (highest priority — evaluated at flag level, before any rules) + // Check global holdouts first (from the "holdouts" section — evaluated at flag level, before any rules) var globalHoldouts = projectConfig.GetGlobalHoldouts(); foreach (var holdout in globalHoldouts) { diff --git a/OptimizelySDK/Config/DatafileProjectConfig.cs b/OptimizelySDK/Config/DatafileProjectConfig.cs index 4bf9a00c..f13f3119 100644 --- a/OptimizelySDK/Config/DatafileProjectConfig.cs +++ b/OptimizelySDK/Config/DatafileProjectConfig.cs @@ -299,10 +299,16 @@ private Dictionary> _VariationIdMap public Rollout[] Rollouts { get; set; } /// - /// Associative list of Holdouts. + /// Associative list of global Holdouts (from the "holdouts" section). /// public Holdout[] Holdouts { get; set; } + /// + /// Associative list of local Holdouts (from the "localHoldouts" section). + /// Local holdouts are rule-scoped via their IncludedRules array. + /// + public Holdout[] LocalHoldouts { get; set; } + /// /// Associative list of Integrations. /// @@ -327,7 +333,32 @@ private void Initialize() FeatureFlags = FeatureFlags ?? new FeatureFlag[0]; Rollouts = Rollouts ?? new Rollout[0]; Holdouts = Holdouts ?? new Holdout[0]; + LocalHoldouts = LocalHoldouts ?? new Holdout[0]; Integrations = Integrations ?? new Integration[0]; + + // Mark global holdouts and strip IncludedRules (section membership is the sole signal for scope) + foreach (var holdout in Holdouts) + { + holdout.IsGlobal = true; + holdout.IncludedRules = null; + } + + // Validate and mark local holdouts + var validLocalHoldouts = new List(); + foreach (var holdout in LocalHoldouts) + { + holdout.IsGlobal = false; + if (holdout.IncludedRules == null || holdout.IncludedRules.Length == 0) + { + Logger?.Log(LogLevel.ERROR, + $"Local holdout \"{holdout.Key}\" (id={holdout.Id}) is missing includedRules -- excluding from evaluation."); + continue; + } + + validLocalHoldouts.Add(holdout); + } + + LocalHoldouts = validLocalHoldouts.ToArray(); _ExperimentKeyMap = new Dictionary(); _GroupIdMap = ConfigParser.GenerateMap(Groups, @@ -422,25 +453,24 @@ private void Initialize() } } - // Adding Holdout variations in variation id and key maps. - if (Holdouts != null) + // Adding Holdout variations in variation id and key maps (both global and local holdouts). + var allHoldoutsForMaps = Holdouts + .Concat(LocalHoldouts); + foreach (var holdout in allHoldoutsForMaps) { - foreach (var holdout in Holdouts) - { - _VariationKeyMap[holdout.Key] = new Dictionary(); - _VariationIdMap[holdout.Key] = new Dictionary(); - _VariationIdMapByExperimentId[holdout.Id] = new Dictionary(); - _VariationKeyMapByExperimentId[holdout.Id] = new Dictionary(); + _VariationKeyMap[holdout.Key] = new Dictionary(); + _VariationIdMap[holdout.Key] = new Dictionary(); + _VariationIdMapByExperimentId[holdout.Id] = new Dictionary(); + _VariationKeyMapByExperimentId[holdout.Id] = new Dictionary(); - if (holdout.Variations != null) + if (holdout.Variations != null) + { + foreach (var variation in holdout.Variations) { - foreach (var variation in holdout.Variations) - { - _VariationKeyMap[holdout.Key][variation.Key] = variation; - _VariationIdMap[holdout.Key][variation.Id] = variation; - _VariationKeyMapByExperimentId[holdout.Id][variation.Key] = variation; - _VariationIdMapByExperimentId[holdout.Id][variation.Id] = variation; - } + _VariationKeyMap[holdout.Key][variation.Key] = variation; + _VariationIdMap[holdout.Key][variation.Id] = variation; + _VariationKeyMapByExperimentId[holdout.Id][variation.Key] = variation; + _VariationIdMapByExperimentId[holdout.Id][variation.Id] = variation; } } } @@ -562,7 +592,11 @@ private void Initialize() _FlagVariationMap = flagToVariationsMap; // Initialize HoldoutConfig for managing flag-to-holdout relationships - _holdoutConfig = new HoldoutConfig(Holdouts ?? new Holdout[0]); + // Merge global holdouts and valid local holdouts + var allHoldoutsForConfig = Holdouts + .Concat(LocalHoldouts) + .ToArray(); + _holdoutConfig = new HoldoutConfig(allHoldoutsForConfig); } /// @@ -910,7 +944,7 @@ public Holdout GetHoldout(string holdoutId) } /// - /// Returns all global holdouts (holdouts where IncludedRules is null). + /// Returns all global holdouts (from the "holdouts" datafile section). /// Global holdouts apply to all rules across all flags and are evaluated at flag level. /// /// Read-only list of global holdouts @@ -920,7 +954,7 @@ public List GetGlobalHoldouts() } /// - /// Returns local holdouts that target a specific rule ID. + /// Returns local holdouts (from the "localHoldouts" section) that target a specific rule ID. /// Local holdouts are evaluated per-rule, after forced decisions but before regular rule evaluation. /// /// The rule ID to look up holdouts for diff --git a/OptimizelySDK/Entity/Holdout.cs b/OptimizelySDK/Entity/Holdout.cs index 7ed44835..fcfa7681 100644 --- a/OptimizelySDK/Entity/Holdout.cs +++ b/OptimizelySDK/Entity/Holdout.cs @@ -19,7 +19,9 @@ namespace OptimizelySDK.Entity { /// - /// Represents a holdout in an Optimizely project + /// Represents a holdout in an Optimizely project. + /// Scope (global vs local) is determined by section membership in the datafile, + /// not by the IncludedRules field. /// public class Holdout : ExperimentCore { @@ -47,17 +49,17 @@ public override string LayerId } /// - /// Optional array of rule IDs that this holdout targets (local holdout). - /// When null, the holdout applies to all rules across all flags (global holdout). - /// When set to an array (even empty), the holdout only applies to the specified rules. - /// Rule IDs in this array are experiment/delivery rule IDs from the datafile, NOT flag IDs. + /// Optional array of rule IDs that this holdout targets (used only for local holdouts). + /// For global holdouts (from the "holdouts" section), this is always stripped to null at parse time. + /// For local holdouts (from the "localHoldouts" section), this must be a non-empty array. /// public string[] IncludedRules { get; set; } /// - /// Returns true if this is a global holdout (IncludedRules is null), - /// false if this is a local holdout (IncludedRules is a non-null array). + /// True if this holdout was parsed from the "holdouts" section (global scope). + /// False if parsed from the "localHoldouts" section (rule scope). + /// Set by the config parser; section membership is the sole signal for scope. /// - public bool IsGlobal => IncludedRules == null; + public bool IsGlobal { get; internal set; } = true; } } diff --git a/OptimizelySDK/ProjectConfig.cs b/OptimizelySDK/ProjectConfig.cs index e0321190..5f20d118 100644 --- a/OptimizelySDK/ProjectConfig.cs +++ b/OptimizelySDK/ProjectConfig.cs @@ -181,10 +181,15 @@ public interface ProjectConfig Rollout[] Rollouts { get; set; } /// - /// Associative list of Holdouts. + /// Associative list of global Holdouts (from the "holdouts" section). /// Holdout[] Holdouts { get; set; } + /// + /// Associative list of local Holdouts (from the "localHoldouts" section). + /// + Holdout[] LocalHoldouts { get; set; } + /// /// Associative list of Integrations. /// @@ -333,14 +338,14 @@ public interface ProjectConfig Holdout GetHoldout(string holdoutId); /// - /// Returns all global holdouts (holdouts where IncludedRules is null). + /// Returns all global holdouts (from the "holdouts" datafile section). /// Global holdouts apply to all rules across all flags and are evaluated at flag level. /// /// Read-only list of global holdouts List GetGlobalHoldouts(); /// - /// Returns local holdouts that target a specific rule ID. + /// Returns local holdouts (from the "localHoldouts" section) that target a specific rule ID. /// Local holdouts are evaluated per-rule, after forced decisions but before regular rule evaluation. /// /// The rule ID to look up holdouts for diff --git a/OptimizelySDK/Utils/HoldoutConfig.cs b/OptimizelySDK/Utils/HoldoutConfig.cs index a8c888fe..59e6fc76 100644 --- a/OptimizelySDK/Utils/HoldoutConfig.cs +++ b/OptimizelySDK/Utils/HoldoutConfig.cs @@ -23,6 +23,8 @@ namespace OptimizelySDK.Utils /// /// Configuration manager for holdouts, providing holdout ID mapping, /// global holdout access, and rule-level local holdout lookups. + /// Scope (global vs local) is determined by the IsGlobal property set at parse time, + /// which reflects section membership in the datafile. /// public class HoldoutConfig { @@ -30,14 +32,15 @@ public class HoldoutConfig private readonly Dictionary _holdoutIdMap; /// - /// Global holdouts — holdouts where IncludedRules is null. + /// Global holdouts — holdouts from the "holdouts" section (IsGlobal == true). /// These are evaluated at flag level, before any per-rule logic. /// private readonly List _globalHoldouts; /// /// Maps rule IDs to the local holdouts that target those rules. - /// A local holdout has IncludedRules set to a non-null array of rule IDs. + /// Local holdouts are from the "localHoldouts" section (IsGlobal == false) + /// and use their IncludedRules array for rule targeting. /// private readonly Dictionary> _ruleHoldoutsMap; @@ -75,16 +78,14 @@ private void UpdateHoldoutMapping() // Build ID mapping _holdoutIdMap[holdout.Id] = holdout; - // Classify as global or local based on IncludedRules + // Classify based on IsGlobal (set by section membership at parse time) if (holdout.IsGlobal) { - // IncludedRules == null → global holdout (applies to all rules across all flags) _globalHoldouts.Add(holdout); } - else + else if (holdout.IncludedRules != null) { - // IncludedRules != null → local holdout (applies only to the specified rules) - // An empty IncludedRules array means local but matches no rules (intentional). + // Local holdout — map each targeted rule ID foreach (var ruleId in holdout.IncludedRules) { if (!_ruleHoldoutsMap.ContainsKey(ruleId)) @@ -116,7 +117,7 @@ public Holdout GetHoldout(string holdoutId) } /// - /// Returns all global holdouts (holdouts where IncludedRules is null). + /// Returns all global holdouts (from the "holdouts" datafile section). /// These apply to all rules across all flags and are evaluated at flag level. /// /// List of global holdouts @@ -126,7 +127,7 @@ public List GetGlobalHoldouts() } /// - /// Returns local holdouts that target a specific rule ID. + /// Returns local holdouts (from the "localHoldouts" section) that target a specific rule ID. /// These are evaluated per-rule, after forced decisions but before regular rule evaluation. /// /// The rule ID to look up holdouts for