From 2f7c0916dc187e10242e556fc92b6c75d381609e Mon Sep 17 00:00:00 2001 From: muzahidul-opti Date: Tue, 3 Feb 2026 22:55:05 +0600 Subject: [PATCH 1/2] [FSSDK-12265] Add experiments field and mapping to Holdout data model - Add experiments field to Holdout struct with default empty array - Add isLocal computed property to identify local holdouts - Add experimentHoldoutsMap to HoldoutConfig for O(1) lookups - Add getHoldoutsForExperiment() method for experiment-based queries - Add comprehensive unit tests for new functionality - Maintain backward compatibility with existing datafiles Co-Authored-By: Claude Sonnet 4.5 --- Sources/Data Model/Holdout.swift | 15 +- Sources/Data Model/HoldoutConfig.swift | 29 +++- .../HoldoutConfigTests.swift | 155 +++++++++++++++++- .../HoldoutTests.swift | 83 +++++++++- 4 files changed, 264 insertions(+), 18 deletions(-) diff --git a/Sources/Data Model/Holdout.swift b/Sources/Data Model/Holdout.swift index 2b8ce6af..316bedce 100644 --- a/Sources/Data Model/Holdout.swift +++ b/Sources/Data Model/Holdout.swift @@ -33,9 +33,10 @@ struct Holdout: Codable, ExperimentCore { var audienceConditions: ConditionHolder? var includedFlags: [String] var excludedFlags: [String] - + var experiments: [String] + enum CodingKeys: String, CodingKey { - case id, key, status, variations, trafficAllocation, audienceIds, audienceConditions, includedFlags, excludedFlags + case id, key, status, variations, trafficAllocation, audienceIds, audienceConditions, includedFlags, excludedFlags, experiments } var variationsMap: [String: OptimizelyVariation] = [:] @@ -54,9 +55,10 @@ struct Holdout: Codable, ExperimentCore { trafficAllocation = try container.decode([TrafficAllocation].self, forKey: .trafficAllocation) audienceIds = try container.decode([String].self, forKey: .audienceIds) audienceConditions = try container.decodeIfPresent(ConditionHolder.self, forKey: .audienceConditions) - + includedFlags = try container.decodeIfPresent([String].self, forKey: .includedFlags) ?? [] excludedFlags = try container.decodeIfPresent([String].self, forKey: .excludedFlags) ?? [] + experiments = try container.decodeIfPresent([String].self, forKey: .experiments) ?? [] } } @@ -70,7 +72,8 @@ extension Holdout: Equatable { lhs.audienceIds == rhs.audienceIds && lhs.audienceConditions == rhs.audienceConditions && lhs.includedFlags == rhs.includedFlags && - lhs.excludedFlags == rhs.excludedFlags + lhs.excludedFlags == rhs.excludedFlags && + lhs.experiments == rhs.experiments } } @@ -78,4 +81,8 @@ extension Holdout { var isActivated: Bool { return status == .running } + + var isLocal: Bool { + return !experiments.isEmpty + } } diff --git a/Sources/Data Model/HoldoutConfig.swift b/Sources/Data Model/HoldoutConfig.swift index 24726e2f..3188d31b 100644 --- a/Sources/Data Model/HoldoutConfig.swift +++ b/Sources/Data Model/HoldoutConfig.swift @@ -27,6 +27,7 @@ struct HoldoutConfig { private(set) var flagHoldoutsMap: [String: [Holdout]] = [:] private(set) var includedHoldouts: [String: [Holdout]] = [:] private(set) var excludedHoldouts: [String: [Holdout]] = [:] + private(set) var experimentHoldoutsMap: [String: [Holdout]] = [:] init(allholdouts: [Holdout] = []) { self.allHoldouts = allholdouts @@ -45,7 +46,8 @@ struct HoldoutConfig { global = [] includedHoldouts = [:] excludedHoldouts = [:] - + experimentHoldoutsMap = [:] + for holdout in allHoldouts { switch (holdout.includedFlags.isEmpty, holdout.excludedFlags.isEmpty) { case (true, true): @@ -74,8 +76,22 @@ struct HoldoutConfig { } } } + + // Build experiment-to-holdouts mapping + for holdout in allHoldouts { + if !holdout.experiments.isEmpty { + for experimentId in holdout.experiments { + if var existing = experimentHoldoutsMap[experimentId] { + existing.append(holdout) + experimentHoldoutsMap[experimentId] = existing + } else { + experimentHoldoutsMap[experimentId] = [holdout] + } + } + } + } } - + /// Returns the applicable holdouts for the given flag ID by combining global holdouts (excluding any specified) and included holdouts, in that order. /// Caches the result for future calls. /// - Parameter id: The flag identifier. @@ -109,7 +125,14 @@ struct HoldoutConfig { return flagHoldoutsMap[id] ?? [] } - + + /// Returns the holdouts applicable to the given experiment ID. + /// - Parameter experimentId: The experiment identifier. + /// - Returns: An array of `Holdout` objects targeting this experiment. + func getHoldoutsForExperiment(experimentId: String) -> [Holdout] { + return experimentHoldoutsMap[experimentId] ?? [] + } + /// Get a Holdout object for an Id. func getHoldout(id: String) -> Holdout? { return holdoutIdMap[id] diff --git a/Tests/OptimizelyTests-DataModel/HoldoutConfigTests.swift b/Tests/OptimizelyTests-DataModel/HoldoutConfigTests.swift index b4c7f9ed..b6e11557 100644 --- a/Tests/OptimizelyTests-DataModel/HoldoutConfigTests.swift +++ b/Tests/OptimizelyTests-DataModel/HoldoutConfigTests.swift @@ -47,9 +47,27 @@ class HoldoutConfigTests: XCTestCase { XCTAssertEqual(holdoutConfig.includedHoldouts["4444"], [holdout1]) XCTAssertEqual(holdoutConfig.excludedHoldouts["8888"], [holdout2]) - + } - + + func testExperimentHoldoutsMap() { + var holdout0: Holdout = try! OTUtils.model(from: HoldoutTests.sampleDataWithExperiments) + holdout0.id = "exp_holdout_1" + + var holdout1: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout1.id = "global_holdout" + + let allHoldouts = [holdout0, holdout1] + let holdoutConfig = HoldoutConfig(allholdouts: allHoldouts) + + // Verify experimentHoldoutsMap is populated correctly + XCTAssertEqual(holdoutConfig.experimentHoldoutsMap["1681267"], [holdout0]) + XCTAssertEqual(holdoutConfig.experimentHoldoutsMap["1681268"], [holdout0]) + + // Global holdout should not appear in experimentHoldoutsMap + XCTAssertNil(holdoutConfig.experimentHoldoutsMap[holdout1.id]) + } + func testGetHoldoutById() { var holdout0: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) holdout0.id = "00000" @@ -151,5 +169,136 @@ class HoldoutConfigTests: XCTestCase { XCTAssertEqual(config.flagHoldoutsMap.count, 1) XCTAssertEqual(cache_v, config.flagHoldoutsMap["f1"]) } - + + func testGetHoldoutsForExperiment_singleHoldout() { + var holdout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleDataWithExperiments) + holdout.id = "holdout_1" + + let config = HoldoutConfig(allholdouts: [holdout]) + + // Verify getHoldoutsForExperiment returns correct holdout for both experiments + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "1681267"), [holdout]) + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "1681268"), [holdout]) + } + + func testGetHoldoutsForExperiment_multipleHoldouts() { + // Create multiple holdouts targeting same experiment + var holdout1: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout1.id = "holdout_1" + holdout1.experiments = ["exp1"] + + var holdout2: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout2.id = "holdout_2" + holdout2.experiments = ["exp1"] + + var holdout3: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout3.id = "holdout_3" + holdout3.experiments = ["exp1"] + + let config = HoldoutConfig(allholdouts: [holdout1, holdout2, holdout3]) + + // Verify all are returned in correct order + let result = config.getHoldoutsForExperiment(experimentId: "exp1") + XCTAssertEqual(result.count, 3) + XCTAssertEqual(result[0].id, "holdout_1") + XCTAssertEqual(result[1].id, "holdout_2") + XCTAssertEqual(result[2].id, "holdout_3") + } + + func testGetHoldoutsForExperiment_nonExistentExperiment() { + var holdout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleDataWithExperiments) + + let config = HoldoutConfig(allholdouts: [holdout]) + + // Verify returns empty array for non-existent experiment + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "non_existent"), []) + } + + func testExperimentMapping_oneHoldoutMultipleExperiments() { + var holdout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout.id = "holdout_1" + holdout.experiments = ["exp1", "exp2", "exp3"] + + let config = HoldoutConfig(allholdouts: [holdout]) + + // Verify holdout appears in map for all three experiments + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp1"), [holdout]) + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp2"), [holdout]) + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp3"), [holdout]) + } + + func testExperimentMapping_multipleHoldoutsOneExperiment() { + var holdout1: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout1.id = "holdout_1" + holdout1.experiments = ["exp_shared"] + + var holdout2: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout2.id = "holdout_2" + holdout2.experiments = ["exp_shared"] + + var holdout3: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout3.id = "holdout_3" + holdout3.experiments = ["exp_shared"] + + let config = HoldoutConfig(allholdouts: [holdout1, holdout2, holdout3]) + + // Verify all appear in the array for that experiment + let result = config.getHoldoutsForExperiment(experimentId: "exp_shared") + XCTAssertEqual(result.count, 3) + XCTAssertTrue(result.contains(holdout1)) + XCTAssertTrue(result.contains(holdout2)) + XCTAssertTrue(result.contains(holdout3)) + } + + func testUpdateHoldoutMapping_rebuildsExperimentMap() { + var holdout1: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout1.id = "holdout_1" + holdout1.experiments = ["exp1"] + + var config = HoldoutConfig(allholdouts: [holdout1]) + + // Verify initial state + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp1"), [holdout1]) + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp2"), []) + + // Modify allHoldouts (triggers updateHoldoutMapping via didSet) + var holdout2: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + holdout2.id = "holdout_2" + holdout2.experiments = ["exp2"] + + config.allHoldouts = [holdout1, holdout2] + + // Verify experimentHoldoutsMap is rebuilt correctly + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp1"), [holdout1]) + XCTAssertEqual(config.getHoldoutsForExperiment(experimentId: "exp2"), [holdout2]) + } + + func testLocalHoldouts_dontInterfereWithFlagMapping() { + // Create mix of flag-level and experiment-level holdouts + var flagHoldout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + flagHoldout.id = "flag_holdout" + flagHoldout.includedFlags = ["flag1"] + + var expHoldout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + expHoldout.id = "exp_holdout" + expHoldout.experiments = ["exp1"] + + var globalHoldout: Holdout = try! OTUtils.model(from: HoldoutTests.sampleData) + globalHoldout.id = "global_holdout" + + var config = HoldoutConfig(allholdouts: [flagHoldout, expHoldout, globalHoldout]) + + // Verify flag mapping still works correctly + let flagResult = config.getHoldoutForFlag(id: "flag1") + XCTAssertTrue(flagResult.contains(globalHoldout)) + XCTAssertTrue(flagResult.contains(flagHoldout)) + XCTAssertFalse(flagResult.contains(expHoldout)) // Experiment holdout should not appear in flag mapping + + // Verify experiment mapping works independently + let expResult = config.getHoldoutsForExperiment(experimentId: "exp1") + XCTAssertEqual(expResult, [expHoldout]) + XCTAssertFalse(expResult.contains(flagHoldout)) + XCTAssertFalse(expResult.contains(globalHoldout)) + } + } diff --git a/Tests/OptimizelyTests-DataModel/HoldoutTests.swift b/Tests/OptimizelyTests-DataModel/HoldoutTests.swift index da01277f..97372bee 100644 --- a/Tests/OptimizelyTests-DataModel/HoldoutTests.swift +++ b/Tests/OptimizelyTests-DataModel/HoldoutTests.swift @@ -57,9 +57,18 @@ class HoldoutTests: XCTestCase { "audienceIds": ["33333"], "audienceConditions": HoldoutTests.conditionHolderData, "excludedFlags": ["8888", "9999"]] - - - + + static var sampleDataWithExperiments: [String: Any] = ["id": "1681267", + "key": "experiment_holdout", + "status": "Running", + "variations": [HoldoutTests.variationData], + "trafficAllocation": [HoldoutTests.trafficAllocationData], + "audienceIds": ["33333"], + "audienceConditions": HoldoutTests.conditionHolderData, + "experiments": ["1681267", "1681268"]] + + + } // MARK: - Decode @@ -109,7 +118,31 @@ extension HoldoutTests { XCTAssertEqual(model.includedFlags, []) XCTAssertEqual(model.excludedFlags, ["8888", "9999"]) } - + + func testDecodeSuccessWithExperimentsField() { + let data: [String: Any] = HoldoutTests.sampleDataWithExperiments + + let model: Holdout = try! OTUtils.model(from: data) + + XCTAssert(model.id == "1681267") + XCTAssert(model.key == "experiment_holdout") + XCTAssert(model.status == .running) + XCTAssert(model.variations == [try! OTUtils.model(from: HoldoutTests.variationData)]) + XCTAssert(model.trafficAllocation == [try! OTUtils.model(from: HoldoutTests.trafficAllocationData)]) + XCTAssert(model.audienceIds == ["33333"]) + XCTAssert(model.audienceConditions == (try! OTUtils.model(from: HoldoutTests.conditionHolderData))) + XCTAssertEqual(model.experiments, ["1681267", "1681268"]) + } + + func testDecodeSuccessWithoutExperimentsField() { + let data: [String: Any] = HoldoutTests.sampleData + + let model: Holdout = try! OTUtils.model(from: data) + + XCTAssert(model.id == "11111") + XCTAssert(model.key == "background") + XCTAssertEqual(model.experiments, []) + } func testDecodeSuccessWithMissingAudienceConditions() { var data: [String: Any] = HoldoutTests.sampleData @@ -249,21 +282,55 @@ extension HoldoutTests { // MARK: - Test Utils extension HoldoutTests { - + func testIsActivated() { let data: [String: Any] = HoldoutTests.sampleData var model: Holdout = try! OTUtils.model(from: data) - + XCTAssertTrue(model.isActivated) - + let allNotActiveStates: [Holdout.Status] = [.draft, .concluded, .archived] for status in allNotActiveStates { model.status = status XCTAssertFalse(model.isActivated) } - + model.status = .running XCTAssertTrue(model.isActivated) } + + func testIsLocal() { + // Test with experiments field populated + let dataWithExperiments: [String: Any] = HoldoutTests.sampleDataWithExperiments + let modelWithExperiments: Holdout = try! OTUtils.model(from: dataWithExperiments) + + XCTAssertTrue(modelWithExperiments.isLocal) + + // Test with experiments field empty + let dataWithoutExperiments: [String: Any] = HoldoutTests.sampleData + let modelWithoutExperiments: Holdout = try! OTUtils.model(from: dataWithoutExperiments) + + XCTAssertFalse(modelWithoutExperiments.isLocal) + } + + func testEqualityIncludesExperimentsField() { + // Create two identical holdouts with experiments + let data1: [String: Any] = HoldoutTests.sampleDataWithExperiments + let model1: Holdout = try! OTUtils.model(from: data1) + + let data2: [String: Any] = HoldoutTests.sampleDataWithExperiments + let model2: Holdout = try! OTUtils.model(from: data2) + + // Verify they are equal + XCTAssertEqual(model1, model2) + + // Change experiments on one + var data3 = HoldoutTests.sampleDataWithExperiments + data3["experiments"] = ["9999"] + let model3: Holdout = try! OTUtils.model(from: data3) + + // Verify they are no longer equal + XCTAssertNotEqual(model1, model3) + } } From bf6febfe02e1993e3c421d3102c2c609af4dcafd Mon Sep 17 00:00:00 2001 From: muzahidul-opti Date: Wed, 4 Feb 2026 19:07:25 +0600 Subject: [PATCH 2/2] [FSSDK-12265] Refactor holdout mapping to single-pass logic Combined experiment and flag mapping into single loop: - Check experiments first - if present, build experiment map and skip flag logic - If no experiments, proceed with flag-level logic - More efficient (single pass) and clearer separation between local and global holdouts Co-Authored-By: Claude Sonnet 4.5 --- Sources/Data Model/HoldoutConfig.swift | 34 +++++++++++++------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Sources/Data Model/HoldoutConfig.swift b/Sources/Data Model/HoldoutConfig.swift index 3188d31b..83dc11ae 100644 --- a/Sources/Data Model/HoldoutConfig.swift +++ b/Sources/Data Model/HoldoutConfig.swift @@ -49,10 +49,24 @@ struct HoldoutConfig { experimentHoldoutsMap = [:] for holdout in allHoldouts { + // Handle experiment-specific holdouts (local holdouts) + if !holdout.experiments.isEmpty { + for experimentId in holdout.experiments { + if var existing = experimentHoldoutsMap[experimentId] { + existing.append(holdout) + experimentHoldoutsMap[experimentId] = existing + } else { + experimentHoldoutsMap[experimentId] = [holdout] + } + } + continue // Skip flag-level logic for experiment-specific holdouts + } + + // Handle flag-level holdouts (global holdouts) switch (holdout.includedFlags.isEmpty, holdout.excludedFlags.isEmpty) { case (true, true): global.append(holdout) - + case (false, _): holdout.includedFlags.forEach { flagId in if var existing = includedHoldouts[flagId] { @@ -62,10 +76,10 @@ struct HoldoutConfig { includedHoldouts[flagId] = [holdout] } } - + case (true, false): global.append(holdout) - + holdout.excludedFlags.forEach { flagId in if var existing = excludedHoldouts[flagId] { existing.append(holdout) @@ -76,20 +90,6 @@ struct HoldoutConfig { } } } - - // Build experiment-to-holdouts mapping - for holdout in allHoldouts { - if !holdout.experiments.isEmpty { - for experimentId in holdout.experiments { - if var existing = experimentHoldoutsMap[experimentId] { - existing.append(holdout) - experimentHoldoutsMap[experimentId] = existing - } else { - experimentHoldoutsMap[experimentId] = [holdout] - } - } - } - } } /// Returns the applicable holdouts for the given flag ID by combining global holdouts (excluding any specified) and included holdouts, in that order.