Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 113 additions & 2 deletions OptimizelySDK.Tests/DecisionServiceHoldoutTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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<string>(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<string>(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");
}
}
}
3 changes: 3 additions & 0 deletions OptimizelySDK.Tests/ProjectConfigTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
29 changes: 27 additions & 2 deletions OptimizelySDK.Tests/TestData/HoldoutTestData.json
Original file line number Diff line number Diff line change
Expand Up @@ -390,8 +390,11 @@
}
],
"audienceIds": [],
"audienceConditions": []
},
"audienceConditions": [],
"includedRules": ["should_be_stripped"]
}
],
"localHoldouts": [
{
"id": "holdout_local_rule1",
"key": "local_holdout_rule1",
Expand Down Expand Up @@ -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",
Expand Down
62 changes: 29 additions & 33 deletions OptimizelySDK.Tests/UtilsTests/HoldoutConfigBasicTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 });

Expand All @@ -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 });

Expand All @@ -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 });
Expand All @@ -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 });
Expand All @@ -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 });
Expand All @@ -228,42 +223,42 @@ 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);
}

[Test]
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 });
Expand All @@ -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 });
Expand Down
2 changes: 1 addition & 1 deletion OptimizelySDK/Bucketing/DecisionService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,7 @@ public virtual Result<FeatureDecision> 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)
{
Expand Down
Loading
Loading