Skip to content

Commit 13be12c

Browse files
Mat001claudejunaed-optimizely
authored
[AI-FSSDK] [FSSDK-12368] Remove legacy flag-level holdout fields (#404)
* [FSSDK-12368] Remove legacy flag-level holdout fields Remove deprecated IncludedFlags and ExcludedFlags from Holdout entity and remove GetHoldoutsForFlag method from ProjectConfig. - Removed IncludedFlags and ExcludedFlags properties from Holdout - Removed GetHoldoutsForFlag() from HoldoutConfig, ProjectConfig interface, and DatafileProjectConfig - Simplified HoldoutConfig by removing flag-level targeting logic - Updated DecisionService to use all holdouts instead of GetHoldoutsForFlag - Deleted HoldoutConfigTests.cs (344 lines of legacy tests) - Removed 7 test methods testing deleted functionality - Created HoldoutConfigBasicTests.cs for remaining functionality Net change: -644 lines (650 deletions, 6 additions) All requirements met. CI will validate compilation and tests. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> * [FSSDK-12368] file format fix * [FSSDK-12368] test file ref removal * [FSSDK-12368] comment fix --------- Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com> Co-authored-by: Md Junaed Hossain <169046794+junaed-optimizely@users.noreply.github.com>
1 parent 91cfa69 commit 13be12c

11 files changed

Lines changed: 148 additions & 651 deletions

File tree

OptimizelySDK.Tests/EntityTests/HoldoutTests.cs

Lines changed: 1 addition & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -53,57 +53,6 @@ public void TestHoldoutDeserialization()
5353
Assert.AreEqual(1, globalHoldout.Variations.Length);
5454
Assert.IsNotNull(globalHoldout.TrafficAllocation);
5555
Assert.AreEqual(1, globalHoldout.TrafficAllocation.Length);
56-
Assert.IsNotNull(globalHoldout.IncludedFlags);
57-
Assert.AreEqual(0, globalHoldout.IncludedFlags.Length);
58-
Assert.IsNotNull(globalHoldout.ExcludedFlags);
59-
Assert.AreEqual(0, globalHoldout.ExcludedFlags.Length);
60-
}
61-
62-
[Test]
63-
public void TestHoldoutWithIncludedFlags()
64-
{
65-
var includedHoldoutJson = testData["includedFlagsHoldout"].ToString();
66-
var includedHoldout = JsonConvert.DeserializeObject<Holdout>(includedHoldoutJson);
67-
68-
Assert.IsNotNull(includedHoldout);
69-
Assert.AreEqual("holdout_included_1", includedHoldout.Id);
70-
Assert.AreEqual("included_holdout", includedHoldout.Key);
71-
Assert.IsNotNull(includedHoldout.IncludedFlags);
72-
Assert.AreEqual(2, includedHoldout.IncludedFlags.Length);
73-
Assert.Contains("flag_1", includedHoldout.IncludedFlags);
74-
Assert.Contains("flag_2", includedHoldout.IncludedFlags);
75-
Assert.IsNotNull(includedHoldout.ExcludedFlags);
76-
Assert.AreEqual(0, includedHoldout.ExcludedFlags.Length);
77-
}
78-
79-
[Test]
80-
public void TestHoldoutWithExcludedFlags()
81-
{
82-
var excludedHoldoutJson = testData["excludedFlagsHoldout"].ToString();
83-
var excludedHoldout = JsonConvert.DeserializeObject<Holdout>(excludedHoldoutJson);
84-
85-
Assert.IsNotNull(excludedHoldout);
86-
Assert.AreEqual("holdout_excluded_1", excludedHoldout.Id);
87-
Assert.AreEqual("excluded_holdout", excludedHoldout.Key);
88-
Assert.IsNotNull(excludedHoldout.IncludedFlags);
89-
Assert.AreEqual(0, excludedHoldout.IncludedFlags.Length);
90-
Assert.IsNotNull(excludedHoldout.ExcludedFlags);
91-
Assert.AreEqual(2, excludedHoldout.ExcludedFlags.Length);
92-
Assert.Contains("flag_3", excludedHoldout.ExcludedFlags);
93-
Assert.Contains("flag_4", excludedHoldout.ExcludedFlags);
94-
}
95-
96-
[Test]
97-
public void TestHoldoutWithEmptyFlags()
98-
{
99-
var globalHoldoutJson = testData["globalHoldout"].ToString();
100-
var globalHoldout = JsonConvert.DeserializeObject<Holdout>(globalHoldoutJson);
101-
102-
Assert.IsNotNull(globalHoldout);
103-
Assert.IsNotNull(globalHoldout.IncludedFlags);
104-
Assert.AreEqual(0, globalHoldout.IncludedFlags.Length);
105-
Assert.IsNotNull(globalHoldout.ExcludedFlags);
106-
Assert.AreEqual(0, globalHoldout.ExcludedFlags.Length);
10756
}
10857

10958
[Test]
@@ -161,7 +110,7 @@ public void TestHoldoutTrafficAllocationDeserialization()
161110
[Test]
162111
public void TestHoldoutNullSafety()
163112
{
164-
// Test that holdout can handle null/missing includedFlags and excludedFlags
113+
// Test that holdout can handle minimal JSON
165114
var minimalHoldoutJson = @"{
166115
""id"": ""test_holdout"",
167116
""key"": ""test_key"",
@@ -177,8 +126,6 @@ public void TestHoldoutNullSafety()
177126
Assert.IsNotNull(holdout);
178127
Assert.AreEqual("test_holdout", holdout.Id);
179128
Assert.AreEqual("test_key", holdout.Key);
180-
Assert.IsNotNull(holdout.IncludedFlags);
181-
Assert.IsNotNull(holdout.ExcludedFlags);
182129
}
183130
}
184131
}

OptimizelySDK.Tests/OptimizelySDK.Tests.csproj

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,6 @@
137137
<Compile Include="OptimizelyFactoryTest.cs"/>
138138
<Compile Include="Utils\TestData.cs"/>
139139
<Compile Include="Utils\Reflection.cs"/>
140-
<Compile Include="UtilsTests\HoldoutConfigTests.cs"/>
141140
<Compile Include="ConfigTest\ProjectConfigProps.cs"/>
142141
<Compile Include="Utils\TestHttpProjectConfigManagerUtil.cs"/>
143142
</ItemGroup>

OptimizelySDK.Tests/OptimizelyUserContextHoldoutTest.cs

Lines changed: 0 additions & 93 deletions
Original file line numberDiff line numberDiff line change
@@ -95,66 +95,6 @@ public void TestDecide_GlobalHoldout()
9595
"Variation key should be valid or null");
9696
}
9797

98-
[Test]
99-
public void TestDecide_IncludedFlagsHoldout()
100-
{
101-
// Test holdout with includedFlags configuration
102-
var featureFlag = Config.FeatureKeyMap["test_flag_1"];
103-
Assert.IsNotNull(featureFlag, "Feature flag should exist");
104-
105-
// Check if there's a holdout that includes this flag
106-
var includedHoldout = Config.Holdouts.FirstOrDefault(h =>
107-
h.IncludedFlags != null && h.IncludedFlags.Contains(featureFlag.Id));
108-
109-
if (includedHoldout != null)
110-
{
111-
var userContext = OptimizelyInstance.CreateUserContext(TestUserId,
112-
new UserAttributes { { "country", "us" } });
113-
114-
var decision = userContext.Decide("test_flag_1");
115-
116-
Assert.IsNotNull(decision, "Decision should not be null");
117-
Assert.AreEqual("test_flag_1", decision.FlagKey, "Flag key should match");
118-
119-
// Verify decision is valid
120-
Assert.IsTrue(decision.VariationKey != null || decision.VariationKey == null,
121-
"Decision should have valid structure");
122-
}
123-
else
124-
{
125-
Assert.Inconclusive("No included holdout found for test_flag_1");
126-
}
127-
}
128-
129-
[Test]
130-
public void TestDecide_ExcludedFlagsHoldout()
131-
{
132-
// Test holdout with excludedFlags configuration
133-
// Based on test data, flag_3 and flag_4 are excluded by holdout_excluded_1
134-
var userContext = OptimizelyInstance.CreateUserContext(TestUserId,
135-
new UserAttributes { { "country", "us" } });
136-
137-
// Test with an excluded flag (test_flag_3 maps to flag_3)
138-
var excludedDecision = userContext.Decide("test_flag_3");
139-
140-
Assert.IsNotNull(excludedDecision, "Decision should not be null for excluded flag");
141-
Assert.AreEqual("test_flag_3", excludedDecision.FlagKey, "Flag key should match");
142-
143-
// For excluded flags, the decision should not come from the excluded holdout
144-
// The excluded holdout has key "excluded_holdout"
145-
Assert.AreNotEqual("excluded_holdout", excludedDecision.RuleKey,
146-
"Decision should not come from excluded holdout for flag_3");
147-
148-
// Also test with a non-excluded flag (test_flag_1 maps to flag_1)
149-
var nonExcludedDecision = userContext.Decide("test_flag_1");
150-
151-
Assert.IsNotNull(nonExcludedDecision, "Decision should not be null for non-excluded flag");
152-
Assert.AreEqual("test_flag_1", nonExcludedDecision.FlagKey, "Flag key should match");
153-
154-
// For non-excluded flags, they can potentially be affected by holdouts
155-
// (depending on other holdout configurations like global or included holdouts)
156-
}
157-
15898
[Test]
15999
public void TestDecideAll_MultipleHoldouts()
160100
{
@@ -327,39 +267,6 @@ public void TestDecide_WithDecisionReasons()
327267
Assert.IsTrue(decision.Reasons.Length >= 0, "Decision reasons should be present");
328268
}
329269

330-
[Test]
331-
public void TestDecide_HoldoutPriority()
332-
{
333-
// Test holdout evaluation priority (global vs included vs excluded)
334-
var featureFlag = Config.FeatureKeyMap["test_flag_1"];
335-
Assert.IsNotNull(featureFlag, "Feature flag should exist");
336-
337-
// Check if we have multiple holdouts
338-
var globalHoldouts = Config.Holdouts.Where(h =>
339-
h.IncludedFlags == null || h.IncludedFlags.Length == 0).ToList();
340-
var includedHoldouts = Config.Holdouts.Where(h =>
341-
h.IncludedFlags != null && h.IncludedFlags.Contains(featureFlag.Id)).ToList();
342-
343-
if (globalHoldouts.Count > 0 || includedHoldouts.Count > 0)
344-
{
345-
var userContext = OptimizelyInstance.CreateUserContext(TestUserId,
346-
new UserAttributes { { "country", "us" } });
347-
348-
var decision = userContext.Decide("test_flag_1");
349-
350-
Assert.IsNotNull(decision, "Decision should not be null");
351-
Assert.AreEqual("test_flag_1", decision.FlagKey, "Flag key should match");
352-
353-
// Decision should be valid regardless of which holdout is selected
354-
Assert.IsTrue(decision.VariationKey != null || decision.VariationKey == null,
355-
"Decision should have valid structure");
356-
}
357-
else
358-
{
359-
Assert.Inconclusive("No holdouts found to test priority");
360-
}
361-
}
362-
363270
#endregion
364271

365272
#region Holdout Decision Reasons Tests

OptimizelySDK.Tests/ProjectConfigTest.cs

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1371,33 +1371,6 @@ public void TestHoldoutDeserialization_FromDatafile()
13711371
Assert.AreEqual(4, datafileProjectConfig.Holdouts.Length);
13721372
}
13731373

1374-
[Test]
1375-
public void TestGetHoldoutsForFlag_Integration()
1376-
{
1377-
var testDataPath = Path.Combine(TestContext.CurrentContext.TestDirectory,
1378-
"TestData", "HoldoutTestData.json");
1379-
var jsonContent = File.ReadAllText(testDataPath);
1380-
var testData = JObject.Parse(jsonContent);
1381-
1382-
var datafileJson = testData["datafileWithHoldouts"].ToString();
1383-
1384-
var datafileProjectConfig = DatafileProjectConfig.Create(datafileJson,
1385-
new NoOpLogger(), new NoOpErrorHandler()) as DatafileProjectConfig;
1386-
1387-
// Test GetHoldoutsForFlag method
1388-
var holdoutsForFlag1 = datafileProjectConfig.GetHoldoutsForFlag("flag_1");
1389-
Assert.IsNotNull(holdoutsForFlag1);
1390-
Assert.AreEqual(4, holdoutsForFlag1.Length); // Global + excluded holdout (applies to all except flag_3/flag_4) + included holdout + empty holdout
1391-
1392-
var holdoutsForFlag3 = datafileProjectConfig.GetHoldoutsForFlag("flag_3");
1393-
Assert.IsNotNull(holdoutsForFlag3);
1394-
Assert.AreEqual(2, holdoutsForFlag3.Length); // Global + empty holdout (excluded holdout excludes flag_3, included holdout doesn't include flag_3)
1395-
1396-
var holdoutsForUnknownFlag = datafileProjectConfig.GetHoldoutsForFlag("unknown_flag");
1397-
Assert.IsNotNull(holdoutsForUnknownFlag);
1398-
Assert.AreEqual(3, holdoutsForUnknownFlag.Length); // Global + excluded holdout (unknown_flag not in excluded list) + empty holdout
1399-
}
1400-
14011374
[Test]
14021375
public void TestGetHoldout_Integration()
14031376
{
@@ -1446,10 +1419,6 @@ public void TestMissingHoldoutsField_BackwardCompatibility()
14461419
Assert.AreEqual(0, datafileProjectConfig.Holdouts.Length);
14471420

14481421
// Methods should still work with empty holdouts
1449-
var holdouts = datafileProjectConfig.GetHoldoutsForFlag("any_flag");
1450-
Assert.IsNotNull(holdouts);
1451-
Assert.AreEqual(0, holdouts.Length);
1452-
14531422
var holdout = datafileProjectConfig.GetHoldout("any_id");
14541423
Assert.IsNull(holdout);
14551424
}
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright 2025, Optimizely
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
using NUnit.Framework;
18+
using OptimizelySDK.Entity;
19+
using OptimizelySDK.Utils;
20+
21+
namespace OptimizelySDK.Tests
22+
{
23+
[TestFixture]
24+
public class HoldoutConfigBasicTests
25+
{
26+
[Test]
27+
public void TestEmptyHoldouts_ShouldHaveEmptyMaps()
28+
{
29+
var config = new HoldoutConfig(new Holdout[0]);
30+
31+
Assert.IsNotNull(config.HoldoutIdMap);
32+
Assert.AreEqual(0, config.HoldoutIdMap.Count);
33+
Assert.AreEqual(0, config.HoldoutCount);
34+
}
35+
36+
[Test]
37+
public void TestHoldoutIdMapping()
38+
{
39+
var holdout1 = CreateTestHoldout("holdout_1", "h1");
40+
var holdout2 = CreateTestHoldout("holdout_2", "h2");
41+
var allHoldouts = new[] { holdout1, holdout2 };
42+
var config = new HoldoutConfig(allHoldouts);
43+
44+
Assert.IsNotNull(config.HoldoutIdMap);
45+
Assert.AreEqual(2, config.HoldoutIdMap.Count);
46+
Assert.AreEqual(2, config.HoldoutCount);
47+
48+
Assert.IsTrue(config.HoldoutIdMap.ContainsKey("holdout_1"));
49+
Assert.IsTrue(config.HoldoutIdMap.ContainsKey("holdout_2"));
50+
51+
Assert.AreEqual(holdout1.Id, config.HoldoutIdMap["holdout_1"].Id);
52+
Assert.AreEqual(holdout2.Id, config.HoldoutIdMap["holdout_2"].Id);
53+
}
54+
55+
[Test]
56+
public void TestGetHoldoutById()
57+
{
58+
var holdout = CreateTestHoldout("holdout_1", "h1");
59+
var config = new HoldoutConfig(new[] { holdout });
60+
61+
var retrieved = config.GetHoldout("holdout_1");
62+
63+
Assert.IsNotNull(retrieved);
64+
Assert.AreEqual("holdout_1", retrieved.Id);
65+
Assert.AreEqual("h1", retrieved.Key);
66+
}
67+
68+
[Test]
69+
public void TestGetHoldoutById_InvalidId()
70+
{
71+
var holdout = CreateTestHoldout("holdout_1", "h1");
72+
var config = new HoldoutConfig(new[] { holdout });
73+
74+
var result = config.GetHoldout("invalid_id");
75+
Assert.IsNull(result);
76+
}
77+
78+
[Test]
79+
public void TestGetHoldoutById_NullId()
80+
{
81+
var holdout = CreateTestHoldout("holdout_1", "h1");
82+
var config = new HoldoutConfig(new[] { holdout });
83+
84+
var result = config.GetHoldout(null);
85+
Assert.IsNull(result);
86+
}
87+
88+
[Test]
89+
public void TestGetHoldoutById_EmptyId()
90+
{
91+
var holdout = CreateTestHoldout("holdout_1", "h1");
92+
var config = new HoldoutConfig(new[] { holdout });
93+
94+
var result = config.GetHoldout("");
95+
Assert.IsNull(result);
96+
}
97+
98+
[Test]
99+
public void TestUpdateHoldoutMapping()
100+
{
101+
var holdout1 = CreateTestHoldout("holdout_1", "h1");
102+
var config = new HoldoutConfig(new[] { holdout1 });
103+
104+
// Initial state
105+
Assert.AreEqual(1, config.HoldoutIdMap.Count);
106+
Assert.AreEqual(1, config.HoldoutCount);
107+
108+
// Update with new holdouts
109+
var holdout2 = CreateTestHoldout("holdout_2", "h2");
110+
config.UpdateHoldoutMapping(new[] { holdout1, holdout2 });
111+
112+
Assert.AreEqual(2, config.HoldoutIdMap.Count);
113+
Assert.AreEqual(2, config.HoldoutCount);
114+
Assert.IsTrue(config.HoldoutIdMap.ContainsKey("holdout_1"));
115+
Assert.IsTrue(config.HoldoutIdMap.ContainsKey("holdout_2"));
116+
}
117+
118+
[Test]
119+
public void TestNullHoldouts()
120+
{
121+
var config = new HoldoutConfig(null);
122+
123+
Assert.IsNotNull(config.HoldoutIdMap);
124+
Assert.AreEqual(0, config.HoldoutIdMap.Count);
125+
Assert.AreEqual(0, config.HoldoutCount);
126+
}
127+
128+
// Helper method to create test holdouts
129+
private Holdout CreateTestHoldout(string id, string key)
130+
{
131+
return new Holdout
132+
{
133+
Id = id,
134+
Key = key,
135+
Status = "Running",
136+
Variations = new Variation[0],
137+
TrafficAllocation = new TrafficAllocation[0],
138+
AudienceIds = new string[0],
139+
AudienceConditions = null
140+
};
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)