From 216a00e6f78f40d13ee31ccc3c9179f2deb0bfca Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 13:05:41 +0100 Subject: [PATCH 01/18] implement experiment manager --- .../ExperimentCohortsManager.swift | 130 ++++++++ .../PrivacyConfigurationData.swift | 23 ++ .../ExperimentCohortsManagerTests.swift | 287 ++++++++++++++++++ .../PrivacyConfigurationDataTests.swift | 3 + .../Resources/privacy-config-example.json | 17 +- 5 files changed, 459 insertions(+), 1 deletion(-) create mode 100644 Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift create mode 100644 Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift new file mode 100644 index 000000000..74d648e0c --- /dev/null +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -0,0 +1,130 @@ +// +// ExperimentCohortsManager.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +struct ExperimentSubfeature { + let subfeatureID: SubfeatureID + let cohorts: [PrivacyConfigurationData.Cohort] +} + +typealias CohortID = String +typealias SubfeatureID = String + +struct ExperimentData: Codable { + let cohort: String + let enrollmentDate: Date +} + +typealias Experiments = [String: ExperimentData] + +protocol ExperimentCohortsManaging { + /// Retrieves the cohort ID associated with the specified subfeature. + /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. + /// - Returns: The cohort ID as a `String` if one exists; otherwise, returns `nil`. + func cohort(for subfeatureID: SubfeatureID) -> CohortID? + + /// Retrieves the enrollment date for the specified subfeature. + /// - Parameter subfeatureID: The experiment subfeature for which the enrollment date is needed. + /// - Returns: The `Date` of enrollment if one exists; otherwise, returns `nil`. + func enrolmentDate(for subfeatureID: SubfeatureID) -> Date? + + /// Assigns a cohort to the given subfeature based on defined weights and saves it to UserDefaults. + /// - Parameter subfeature: The experiment subfeature to assign a cohort for. + /// - Returns: The name of the assigned cohort, or `nil` if no cohort could be assigned. + func assignCohort(for subfeature: ExperimentSubfeature) -> CohortID? + + /// Removes the assigned cohort data for the specified subfeature. + /// - Parameter subfeature: The experiment subfeature for which the cohort data should be removed. + func removeCohort(for subfeatureID: SubfeatureID) +} + +struct ExperimentCohortsManager: ExperimentCohortsManaging { + + private let userDefaults: UserDefaults + private let decoder = JSONDecoder() + private let encoder = JSONEncoder() + private let queue = DispatchQueue(label: "com.experimentManager.queue") + private let experimentsDataKey = "ExperimentsData" + private let randomizer: (Range) -> Double + + init(userDefaults: UserDefaults = UserDefaults.standard, randomizer: @escaping (Range) -> Double) { + self.userDefaults = userDefaults + self.randomizer = randomizer + encoder.dateEncodingStrategy = .secondsSince1970 + decoder.dateDecodingStrategy = .secondsSince1970 + } + + func cohort(for subfeatureID: SubfeatureID) -> CohortID? { + guard let experiments = getExperimentData() else { return nil } + return experiments[subfeatureID]?.cohort + } + + func enrolmentDate(for subfeatureID: SubfeatureID) -> Date? { + guard let experiments = getExperimentData() else { return nil } + return experiments[subfeatureID]?.enrollmentDate + } + + func assignCohort(for subfeature: ExperimentSubfeature) -> CohortID? { + let cohorts = subfeature.cohorts + let totalWeight = cohorts.reduce(0, { $0 + $1.weight }) + guard totalWeight > 0 else { return nil } + + let randomValue = randomizer(0.. Experiments? { + queue.sync { + guard let savedData = userDefaults.data(forKey: experimentsDataKey) else { return nil } + return try? decoder.decode(Experiments.self, from: savedData) + } + } + + private func saveExperimentData(_ experiments: Experiments) { + queue.sync { + if let encodedData = try? encoder.encode(experiments) { + userDefaults.set(encodedData, forKey: experimentsDataKey) + } + } + } + + private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { + var experiments = getExperimentData() ?? Experiments() + let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) + experiments[experimentID] = experimentData + saveExperimentData(experiments) + } +} + diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index 813c87503..eb9267505 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -36,6 +36,20 @@ public struct PrivacyConfigurationData { static public let enabled = "enabled" } + public struct Cohort { + public let name: String + public let weight: Int + + public init?(json: [String: Any]) { + guard let name = json["name"] as? String, + let weightValue = json["weight"] as? Int else { + return nil + } + + self.name = name + self.weight = weightValue + } + } public let features: [FeatureName: PrivacyFeature] public let trackerAllowlist: TrackerAllowlist public let unprotectedTemporary: [ExceptionEntry] @@ -121,6 +135,7 @@ public struct PrivacyConfigurationData { case state case minSupportedVersion case rollout + case cohorts } public struct Rollout: Hashable { @@ -157,6 +172,7 @@ public struct PrivacyConfigurationData { public let state: FeatureState public let minSupportedVersion: FeatureSupportedVersion? public let rollout: Rollout? + public let cohorts: [Cohort]? public init?(json: [String: Any]) { guard let state = json[CodingKeys.state.rawValue] as? String else { @@ -171,6 +187,13 @@ public struct PrivacyConfigurationData { } else { self.rollout = nil } + + if let cohortData = json[CodingKeys.cohorts.rawValue] as? [[String: Any]] { + let parsedCohorts = cohortData.compactMap { Cohort(json: $0) } + self.cohorts = parsedCohorts.isEmpty ? nil : parsedCohorts + } else { + self.cohorts = nil + } } } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift new file mode 100644 index 000000000..f3e68a2d6 --- /dev/null +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -0,0 +1,287 @@ +// +// ExperimentCohortsManagerTests.swift +// DuckDuckGo +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import BrowserServicesKit + +final class ExperimentCohortsManagerTests: XCTestCase { + + var mockUserDefaults: UserDefaults! + var experimentCohortsManager: ExperimentCohortsManager! + + let subfeatureName1 = "TestSubfeature1" + var expectedDate1: Date! + var experimentData1: ExperimentData! + + let subfeatureName2 = "TestSubfeature2" + var expectedDate2: Date! + var experimentData2: ExperimentData! + + let encoder: JSONEncoder = { + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .secondsSince1970 + return encoder + }() + + override func setUp() { + super.setUp() + mockUserDefaults = UserDefaults(suiteName: "com.test.ExperimentCohortsManagerTests") + mockUserDefaults.removePersistentDomain(forName: "com.test.ExperimentCohortsManagerTests") + + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 50.0 } + ) + + expectedDate1 = Date() + experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: expectedDate1) + + expectedDate2 = Date().addingTimeInterval(60) // Second subfeature with a different date + experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: expectedDate2) + } + + override func tearDown() { + mockUserDefaults.removePersistentDomain(forName: "com.test.ExperimentCohortsManagerTests") + mockUserDefaults = nil + experimentCohortsManager = nil + expectedDate1 = nil + experimentData1 = nil + expectedDate2 = nil + experimentData2 = nil + super.tearDown() + } + + private func saveExperimentData(_ data: [String: ExperimentData]) { + if let encodedData = try? encoder.encode(data) { + mockUserDefaults.set(encodedData, forKey: "ExperimentsData") + } + } + + + func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { + // GIVEN + saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) + + // WHEN + let result1 = experimentCohortsManager.cohort(for: subfeatureName1) + let result2 = experimentCohortsManager.cohort(for: subfeatureName2) + + // THEN + XCTAssertEqual(result1, experimentData1.cohort) + XCTAssertEqual(result2, experimentData2.cohort) + } + + func testEnrolmentDateReturnsCorrectDateIfExists() { + // GIVEN + saveExperimentData([subfeatureName1: experimentData1]) + + // WHEN + let result1 = experimentCohortsManager.enrolmentDate(for: subfeatureName1) + let result2 = experimentCohortsManager.enrolmentDate(for: subfeatureName2) + + // THEN + let timeDifference1 = abs(expectedDate1.timeIntervalSince(result1 ?? Date())) + + XCTAssertLessThanOrEqual(timeDifference1, 1.0, "Expected enrollment date for subfeatureName1 to match at the second level") + XCTAssertNil(result2) + } + + func testCohortReturnsNilIfCohortDoesNotExist() { + // GIVEN + let subfeatureName = "TestSubfeature" + + // WHEN + let result = experimentCohortsManager.cohort(for: subfeatureName) + + // THEN + XCTAssertNil(result) + } + + func testEnrolmentDateReturnsNilIfDateDoesNotExist() { + // GIVEN + let subfeatureName = "TestSubfeature" + + // WHEN + let result = experimentCohortsManager.enrolmentDate(for: subfeatureName) + + // THEN + XCTAssertNil(result) + } + + func testRemoveCohortSuccessfullyRemovesData() { + // GIVEN + saveExperimentData([subfeatureName1: experimentData1]) + let initialData = mockUserDefaults.data(forKey: "ExperimentsData") + XCTAssertNotNil(initialData, "Expected initial data to be saved in UserDefaults.") + + // WHEN + experimentCohortsManager.removeCohort(for: subfeatureName1) + + // THEN + if let remainingData = mockUserDefaults.data(forKey: "ExperimentsData") { + let decoder = JSONDecoder() + let experiments = try? decoder.decode(Experiments.self, from: remainingData) + XCTAssertNil(experiments?[subfeatureName1]) + } + } + + func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { + // GIVEN + saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) + let initialData = mockUserDefaults.data(forKey: "ExperimentsData") + XCTAssertNotNil(initialData, "Expected initial data to be saved in UserDefaults.") + + // WHEN + experimentCohortsManager.removeCohort(for: "someOtherSubfeature") + + // THEN + if let remainingData = mockUserDefaults.data(forKey: "ExperimentsData") { + let decoder = JSONDecoder() + let experiments = try? decoder.decode(Experiments.self, from: remainingData) + XCTAssertNotNil(experiments?[subfeatureName1]) + XCTAssertNotNil(experiments?[subfeatureName2]) + } + } + + func testAssignCohortReturnsNilIfNoCohorts() { + // GIVEN + let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: []) + + // WHEN + let result = experimentCohortsManager.assignCohort(for: subfeature) + + // THEN + XCTAssertNil(result) + } + + func testAssignCohortReturnsNilIfAllWeightsAreZero() { + // GIVEN + let jsonCohort1: [String: Any] = ["name": "TestCohort", "weight": 0] + let jsonCohort2: [String: Any] = ["name": "TestCohort", "weight": 0] + let cohorts = [ + PrivacyConfigurationData.Cohort(json: jsonCohort1)!, + PrivacyConfigurationData.Cohort(json: jsonCohort2)! + ] + let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + + // WHEN + let result = experimentCohortsManager.assignCohort(for: subfeature) + + // THEN + XCTAssertNil(result) + } + + func testAssignCohortSelectsCorrectCohortBasedOnWeight() { + // Cohort1 has weight 1, Cohort2 has weight 3 + // Total weight is 1 + 3 = 4 + let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] + let jsonCohort2: [String: Any] = ["name": "Cohort2", "weight": 3] + let cohorts = [ + PrivacyConfigurationData.Cohort(json: jsonCohort1)!, + PrivacyConfigurationData.Cohort(json: jsonCohort2)! + ] + let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + let expectedTotalWeight = 4.0 + + // Use a custom randomizer to verify the range + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { range in + // Assert that the range lower bound is 0 + XCTAssertEqual(range.lowerBound, 0.0) + // Assert that the range upper bound is the total weight + XCTAssertEqual(range.upperBound, expectedTotalWeight) + return 0.0 + } + ) + + // Test case where random value is at the very start of Cohort1's range (0) + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 0.0 } + ) + let resultStartOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) + XCTAssertEqual(resultStartOfCohort1, "Cohort1") + + // Test case where random value is at the end of Cohort1's range (0.9) + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 0.9 } + ) + let resultEndOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) + XCTAssertEqual(resultEndOfCohort1, "Cohort1") + + // Test case where random value is at the start of Cohort2's range (1.00 to 4) + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 1.00 } + ) + let resultStartOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + XCTAssertEqual(resultStartOfCohort2, "Cohort2") + + // Test case where random value falls exactly within Cohort2's range (2.5) + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 2.5 } + ) + let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + XCTAssertEqual(resultMiddleOfCohort2, "Cohort2") + + // Test case where random value is at the end of Cohort2's range (4) + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { _ in 3.9 } + ) + let resultEndOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + XCTAssertEqual(resultEndOfCohort2, "Cohort2") + } + + func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() { + // GIVEN + let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] + let cohorts = [ + PrivacyConfigurationData.Cohort(json: jsonCohort1)! + ] + let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + let expectedTotalWeight = 1.0 + + // Use a custom randomizer to verify the range + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { range in + // Assert that the range lower bound is 0 + XCTAssertEqual(range.lowerBound, 0.0) + // Assert that the range upper bound is the total weight + XCTAssertEqual(range.upperBound, expectedTotalWeight) + return 0.0 + } + ) + + // WHEN + experimentCohortsManager = ExperimentCohortsManager( + userDefaults: mockUserDefaults, + randomizer: { range in Double.random(in: range)} + ) + let result = experimentCohortsManager.assignCohort(for: subfeature) + + // THEN + XCTAssertEqual(result, "Cohort1") + } + +} diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift index f4406afef..fac814b1c 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift @@ -65,6 +65,9 @@ class PrivacyConfigurationDataTests: XCTestCase { XCTAssertEqual(subfeatures["disabledSubfeature"]?.state, "disabled") XCTAssertEqual(subfeatures["minSupportedSubfeature"]?.minSupportedVersion, "1.36.0") XCTAssertEqual(subfeatures["enabledSubfeature"]?.state, "enabled") + XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?.count, 3) + XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].name, "myExperimentControl") + XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].weight, 1) XCTAssertEqual(subfeatures["internalSubfeature"]?.state, "internal") } else { XCTFail("Could not parse subfeatures") diff --git a/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json b/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json index 2728eaf55..3fd5be5a5 100644 --- a/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json +++ b/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json @@ -170,7 +170,22 @@ "minSupportedVersion": "1.36.0" }, "enabledSubfeature": { - "state": "enabled" + "state": "enabled", + "description": "A description of the sub-feature", + "cohorts": [ + { + "name": "myExperimentControl", + "weight": 1 + }, + { + "name": "myExperimentBlue", + "weight": 1 + }, + { + "name": "myExperimentRed", + "weight": 1 + } + ] }, "internalSubfeature": { "state": "internal" From 914a5ed08996a96426e302ed9afac4147ff7f254 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 13:32:49 +0100 Subject: [PATCH 02/18] fix lint issues --- .../PrivacyConfig/ExperimentCohortsManager.swift | 2 -- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 1 - 2 files changed, 3 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 74d648e0c..db137f414 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -1,6 +1,5 @@ // // ExperimentCohortsManager.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // @@ -127,4 +126,3 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { saveExperimentData(experiments) } } - diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index f3e68a2d6..e6b5a9734 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -1,6 +1,5 @@ // // ExperimentCohortsManagerTests.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // From 93247a3c241c7d4259a7cbe7b999a4f69db95ae2 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 13:47:57 +0100 Subject: [PATCH 03/18] fix linting issue --- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 1 - 1 file changed, 1 deletion(-) diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index e6b5a9734..fd1ad68eb 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -72,7 +72,6 @@ final class ExperimentCohortsManagerTests: XCTestCase { } } - func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { // GIVEN saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) From 5d530f29cc3357bfe236a4b7c32a895ac3344ce3 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 15:00:48 +0100 Subject: [PATCH 04/18] wrap UserDefaults --- .../ExperimentCohortsManager.swift | 19 +++++-- .../ExperimentCohortsManagerTests.swift | 57 +++++++++++-------- 2 files changed, 47 insertions(+), 29 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index db137f414..6f21016c8 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -33,6 +33,11 @@ struct ExperimentData: Codable { typealias Experiments = [String: ExperimentData] +protocol ExperimentDataStoring { + func data(forKey defaultName: String) -> Data? + func set(_ value: Any?, forKey defaultName: String) +} + protocol ExperimentCohortsManaging { /// Retrieves the cohort ID associated with the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. @@ -56,15 +61,15 @@ protocol ExperimentCohortsManaging { struct ExperimentCohortsManager: ExperimentCohortsManaging { - private let userDefaults: UserDefaults + private let store: ExperimentDataStoring private let decoder = JSONDecoder() private let encoder = JSONEncoder() private let queue = DispatchQueue(label: "com.experimentManager.queue") - private let experimentsDataKey = "ExperimentsData" private let randomizer: (Range) -> Double + private let experimentsDataKey = "ExperimentsData" - init(userDefaults: UserDefaults = UserDefaults.standard, randomizer: @escaping (Range) -> Double) { - self.userDefaults = userDefaults + init(store: ExperimentDataStoring = UserDefaults.standard, randomizer: @escaping (Range) -> Double) { + self.store = store self.randomizer = randomizer encoder.dateEncodingStrategy = .secondsSince1970 decoder.dateDecodingStrategy = .secondsSince1970 @@ -106,7 +111,7 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { private func getExperimentData() -> Experiments? { queue.sync { - guard let savedData = userDefaults.data(forKey: experimentsDataKey) else { return nil } + guard let savedData = store.data(forKey: experimentsDataKey) else { return nil } return try? decoder.decode(Experiments.self, from: savedData) } } @@ -114,7 +119,7 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { private func saveExperimentData(_ experiments: Experiments) { queue.sync { if let encodedData = try? encoder.encode(experiments) { - userDefaults.set(encodedData, forKey: experimentsDataKey) + store.set(encodedData, forKey: experimentsDataKey) } } } @@ -126,3 +131,5 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { saveExperimentData(experiments) } } + +extension UserDefaults: ExperimentDataStoring {} diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index fd1ad68eb..3a134e2dd 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -21,7 +21,7 @@ import XCTest final class ExperimentCohortsManagerTests: XCTestCase { - var mockUserDefaults: UserDefaults! + var mockStore: MockExperimentDataStore! var experimentCohortsManager: ExperimentCohortsManager! let subfeatureName1 = "TestSubfeature1" @@ -40,11 +40,9 @@ final class ExperimentCohortsManagerTests: XCTestCase { override func setUp() { super.setUp() - mockUserDefaults = UserDefaults(suiteName: "com.test.ExperimentCohortsManagerTests") - mockUserDefaults.removePersistentDomain(forName: "com.test.ExperimentCohortsManagerTests") - + mockStore = MockExperimentDataStore() experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 50.0 } ) @@ -56,8 +54,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { } override func tearDown() { - mockUserDefaults.removePersistentDomain(forName: "com.test.ExperimentCohortsManagerTests") - mockUserDefaults = nil + mockStore = nil experimentCohortsManager = nil expectedDate1 = nil experimentData1 = nil @@ -68,7 +65,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { private func saveExperimentData(_ data: [String: ExperimentData]) { if let encodedData = try? encoder.encode(data) { - mockUserDefaults.set(encodedData, forKey: "ExperimentsData") + mockStore.dataToReturn = encodedData } } @@ -125,14 +122,13 @@ final class ExperimentCohortsManagerTests: XCTestCase { func testRemoveCohortSuccessfullyRemovesData() { // GIVEN saveExperimentData([subfeatureName1: experimentData1]) - let initialData = mockUserDefaults.data(forKey: "ExperimentsData") - XCTAssertNotNil(initialData, "Expected initial data to be saved in UserDefaults.") // WHEN experimentCohortsManager.removeCohort(for: subfeatureName1) // THEN - if let remainingData = mockUserDefaults.data(forKey: "ExperimentsData") { + + if let remainingData = mockStore.dataSaved { let decoder = JSONDecoder() let experiments = try? decoder.decode(Experiments.self, from: remainingData) XCTAssertNil(experiments?[subfeatureName1]) @@ -142,14 +138,12 @@ final class ExperimentCohortsManagerTests: XCTestCase { func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { // GIVEN saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) - let initialData = mockUserDefaults.data(forKey: "ExperimentsData") - XCTAssertNotNil(initialData, "Expected initial data to be saved in UserDefaults.") // WHEN experimentCohortsManager.removeCohort(for: "someOtherSubfeature") // THEN - if let remainingData = mockUserDefaults.data(forKey: "ExperimentsData") { + if let remainingData = mockStore.dataSaved { let decoder = JSONDecoder() let experiments = try? decoder.decode(Experiments.self, from: remainingData) XCTAssertNotNil(experiments?[subfeatureName1]) @@ -199,7 +193,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Use a custom randomizer to verify the range experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { range in // Assert that the range lower bound is 0 XCTAssertEqual(range.lowerBound, 0.0) @@ -211,7 +205,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Test case where random value is at the very start of Cohort1's range (0) experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 0.0 } ) let resultStartOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) @@ -219,7 +213,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Test case where random value is at the end of Cohort1's range (0.9) experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 0.9 } ) let resultEndOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) @@ -227,7 +221,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Test case where random value is at the start of Cohort2's range (1.00 to 4) experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 1.00 } ) let resultStartOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) @@ -235,7 +229,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Test case where random value falls exactly within Cohort2's range (2.5) experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 2.5 } ) let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) @@ -243,14 +237,14 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Test case where random value is at the end of Cohort2's range (4) experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { _ in 3.9 } ) let resultEndOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) XCTAssertEqual(resultEndOfCohort2, "Cohort2") } - func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() { + func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() throws { // GIVEN let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] let cohorts = [ @@ -261,7 +255,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { // Use a custom randomizer to verify the range experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { range in // Assert that the range lower bound is 0 XCTAssertEqual(range.lowerBound, 0.0) @@ -273,13 +267,30 @@ final class ExperimentCohortsManagerTests: XCTestCase { // WHEN experimentCohortsManager = ExperimentCohortsManager( - userDefaults: mockUserDefaults, + store: mockStore, randomizer: { range in Double.random(in: range)} ) let result = experimentCohortsManager.assignCohort(for: subfeature) + let savedData = try XCTUnwrap(mockStore.dataSaved) // THEN XCTAssertEqual(result, "Cohort1") + let decodedSavedData = try XCTUnwrap(JSONDecoder().decode(Experiments.self, from: savedData)) + XCTAssertEqual(cohorts[0].name, decodedSavedData[subfeature.subfeatureID]?.cohort) + } + +} + +class MockExperimentDataStore: ExperimentDataStoring { + var dataToReturn: Data? + var dataSaved: Data? + + func data(forKey defaultName: String) -> Data? { + dataToReturn + } + + func set(_ value: Any?, forKey defaultName: String) { + dataSaved = value as? Data } } From 2dcef1efe1cbf601fe790db786a74b06b73fefb8 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 15:03:05 +0100 Subject: [PATCH 05/18] fix linting --- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 3a134e2dd..6c1a1ef4c 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -127,7 +127,6 @@ final class ExperimentCohortsManagerTests: XCTestCase { experimentCohortsManager.removeCohort(for: subfeatureName1) // THEN - if let remainingData = mockStore.dataSaved { let decoder = JSONDecoder() let experiments = try? decoder.decode(Experiments.self, from: remainingData) @@ -288,7 +287,7 @@ class MockExperimentDataStore: ExperimentDataStoring { func data(forKey defaultName: String) -> Data? { dataToReturn } - + func set(_ value: Any?, forKey defaultName: String) { dataSaved = value as? Data } From 5aec88df61e668ba36ea46c39ceffeb1d820fd0e Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 16:22:35 +0100 Subject: [PATCH 06/18] refactor --- .../ExperimentCohortsManager.swift | 30 +---- .../PrivacyConfig/ExperimentsDataStore.swift | 60 ++++++++++ .../ExperimentCohortsManagerTests.swift | 59 +++------- .../ExperimentsDataStoreTests.swift | 105 ++++++++++++++++++ 4 files changed, 186 insertions(+), 68 deletions(-) create mode 100644 Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift create mode 100644 Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 6f21016c8..ba167a0e4 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -26,18 +26,13 @@ struct ExperimentSubfeature { typealias CohortID = String typealias SubfeatureID = String -struct ExperimentData: Codable { +struct ExperimentData: Codable, Equatable { let cohort: String let enrollmentDate: Date } typealias Experiments = [String: ExperimentData] -protocol ExperimentDataStoring { - func data(forKey defaultName: String) -> Data? - func set(_ value: Any?, forKey defaultName: String) -} - protocol ExperimentCohortsManaging { /// Retrieves the cohort ID associated with the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. @@ -59,20 +54,16 @@ protocol ExperimentCohortsManaging { func removeCohort(for subfeatureID: SubfeatureID) } -struct ExperimentCohortsManager: ExperimentCohortsManaging { +class ExperimentCohortsManager: ExperimentCohortsManaging { - private let store: ExperimentDataStoring - private let decoder = JSONDecoder() - private let encoder = JSONEncoder() + private var store: ExperimentsDataStoring private let queue = DispatchQueue(label: "com.experimentManager.queue") private let randomizer: (Range) -> Double private let experimentsDataKey = "ExperimentsData" - init(store: ExperimentDataStoring = UserDefaults.standard, randomizer: @escaping (Range) -> Double) { + init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { self.store = store self.randomizer = randomizer - encoder.dateEncodingStrategy = .secondsSince1970 - decoder.dateDecodingStrategy = .secondsSince1970 } func cohort(for subfeatureID: SubfeatureID) -> CohortID? { @@ -110,18 +101,11 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { } private func getExperimentData() -> Experiments? { - queue.sync { - guard let savedData = store.data(forKey: experimentsDataKey) else { return nil } - return try? decoder.decode(Experiments.self, from: savedData) - } + return store.experiments } private func saveExperimentData(_ experiments: Experiments) { - queue.sync { - if let encodedData = try? encoder.encode(experiments) { - store.set(encodedData, forKey: experimentsDataKey) - } - } + store.experiments = experiments } private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { @@ -131,5 +115,3 @@ struct ExperimentCohortsManager: ExperimentCohortsManaging { saveExperimentData(experiments) } } - -extension UserDefaults: ExperimentDataStoring {} diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift new file mode 100644 index 000000000..849f9168b --- /dev/null +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -0,0 +1,60 @@ +// +// ExperimentsDataStore.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import Foundation + +protocol ExperimentsDataStoring { + var experiments: Experiments? { get set } +} + +protocol LocalDataStoring { + func data(forKey defaultName: String) -> Data? + func set(_ value: Any?, forKey defaultName: String) +} + +struct ExperimentsDataStore: ExperimentsDataStoring { + private let localDataStoring: LocalDataStoring + private let experimentsDataKey = "ExperimentsData" + private let queue = DispatchQueue(label: "com.experimentManager.queue") + private let decoder = JSONDecoder() + private let encoder = JSONEncoder() + + init(localDataStoring: LocalDataStoring = UserDefaults.standard) { + self.localDataStoring = localDataStoring + encoder.dateEncodingStrategy = .secondsSince1970 + decoder.dateDecodingStrategy = .secondsSince1970 + } + + var experiments: Experiments? { + get { + queue.sync { + guard let savedData = localDataStoring.data(forKey: experimentsDataKey) else { return nil } + return try? decoder.decode(Experiments.self, from: savedData) + } + } + set { + queue.sync { + if let encodedData = try? encoder.encode(newValue) { + localDataStoring.set(encodedData, forKey: experimentsDataKey) + } + } + } + } +} + +extension UserDefaults: LocalDataStoring {} diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 6c1a1ef4c..bc7f787cf 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -25,11 +25,9 @@ final class ExperimentCohortsManagerTests: XCTestCase { var experimentCohortsManager: ExperimentCohortsManager! let subfeatureName1 = "TestSubfeature1" - var expectedDate1: Date! var experimentData1: ExperimentData! let subfeatureName2 = "TestSubfeature2" - var expectedDate2: Date! var experimentData2: ExperimentData! let encoder: JSONEncoder = { @@ -46,32 +44,24 @@ final class ExperimentCohortsManagerTests: XCTestCase { randomizer: { _ in 50.0 } ) - expectedDate1 = Date() + let expectedDate1 = Date() experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: expectedDate1) - expectedDate2 = Date().addingTimeInterval(60) // Second subfeature with a different date + let expectedDate2 = Date().addingTimeInterval(60) experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: expectedDate2) } override func tearDown() { mockStore = nil experimentCohortsManager = nil - expectedDate1 = nil experimentData1 = nil - expectedDate2 = nil experimentData2 = nil super.tearDown() } - private func saveExperimentData(_ data: [String: ExperimentData]) { - if let encodedData = try? encoder.encode(data) { - mockStore.dataToReturn = encodedData - } - } - func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { // GIVEN - saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) + mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN let result1 = experimentCohortsManager.cohort(for: subfeatureName1) @@ -84,14 +74,14 @@ final class ExperimentCohortsManagerTests: XCTestCase { func testEnrolmentDateReturnsCorrectDateIfExists() { // GIVEN - saveExperimentData([subfeatureName1: experimentData1]) + mockStore.experiments = [subfeatureName1: experimentData1] // WHEN let result1 = experimentCohortsManager.enrolmentDate(for: subfeatureName1) let result2 = experimentCohortsManager.enrolmentDate(for: subfeatureName2) // THEN - let timeDifference1 = abs(expectedDate1.timeIntervalSince(result1 ?? Date())) + let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result1 ?? Date())) XCTAssertLessThanOrEqual(timeDifference1, 1.0, "Expected enrollment date for subfeatureName1 to match at the second level") XCTAssertNil(result2) @@ -119,35 +109,28 @@ final class ExperimentCohortsManagerTests: XCTestCase { XCTAssertNil(result) } - func testRemoveCohortSuccessfullyRemovesData() { + func testRemoveCohortSuccessfullyRemovesData() throws { // GIVEN - saveExperimentData([subfeatureName1: experimentData1]) + mockStore.experiments = [subfeatureName1: experimentData1] // WHEN experimentCohortsManager.removeCohort(for: subfeatureName1) // THEN - if let remainingData = mockStore.dataSaved { - let decoder = JSONDecoder() - let experiments = try? decoder.decode(Experiments.self, from: remainingData) - XCTAssertNil(experiments?[subfeatureName1]) - } + let experiments = try XCTUnwrap(mockStore.experiments) + XCTAssertTrue(experiments.isEmpty) } func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { // GIVEN - saveExperimentData([subfeatureName1: experimentData1, subfeatureName2: experimentData2]) + let expectedExperiments: Experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] + mockStore.experiments = expectedExperiments // WHEN experimentCohortsManager.removeCohort(for: "someOtherSubfeature") // THEN - if let remainingData = mockStore.dataSaved { - let decoder = JSONDecoder() - let experiments = try? decoder.decode(Experiments.self, from: remainingData) - XCTAssertNotNil(experiments?[subfeatureName1]) - XCTAssertNotNil(experiments?[subfeatureName2]) - } + XCTAssertEqual( mockStore.experiments, expectedExperiments) } func testAssignCohortReturnsNilIfNoCohorts() { @@ -270,26 +253,14 @@ final class ExperimentCohortsManagerTests: XCTestCase { randomizer: { range in Double.random(in: range)} ) let result = experimentCohortsManager.assignCohort(for: subfeature) - let savedData = try XCTUnwrap(mockStore.dataSaved) // THEN XCTAssertEqual(result, "Cohort1") - let decodedSavedData = try XCTUnwrap(JSONDecoder().decode(Experiments.self, from: savedData)) - XCTAssertEqual(cohorts[0].name, decodedSavedData[subfeature.subfeatureID]?.cohort) + XCTAssertEqual(cohorts[0].name, mockStore.experiments?[subfeature.subfeatureID]?.cohort) } } -class MockExperimentDataStore: ExperimentDataStoring { - var dataToReturn: Data? - var dataSaved: Data? - - func data(forKey defaultName: String) -> Data? { - dataToReturn - } - - func set(_ value: Any?, forKey defaultName: String) { - dataSaved = value as? Data - } - +class MockExperimentDataStore: ExperimentsDataStoring { + var experiments: Experiments? = nil } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift new file mode 100644 index 000000000..0466155b0 --- /dev/null +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift @@ -0,0 +1,105 @@ +// +// ExperimentsDataStoreTests.swift +// +// Copyright © 2024 DuckDuckGo. All rights reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +import XCTest +@testable import BrowserServicesKit + +final class ExperimentsDataStoreTests: XCTestCase { + + let subfeatureName1 = "TestSubfeature1" + var expectedDate1: Date! + var experimentData1: ExperimentData! + + let subfeatureName2 = "TestSubfeature2" + var expectedDate2: Date! + var experimentData2: ExperimentData! + + var mockDataStore: MockLocalDataStore! + var experimentsDataStore: ExperimentsDataStore! + let testExperimentKey = "ExperimentsData" + + override func setUp() { + super.setUp() + mockDataStore = MockLocalDataStore() + experimentsDataStore = ExperimentsDataStore(localDataStoring: mockDataStore) + } + + override func tearDown() { + mockDataStore = nil + experimentsDataStore = nil + super.tearDown() + } + + func testExperimentsGetReturnsDecodedExperiments() { + // GIVEN + let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) + let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) + let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] + + let encoder = JSONEncoder() + encoder.dateEncodingStrategy = .secondsSince1970 + let encodedData = try? encoder.encode(experiments) + mockDataStore.data = encodedData + + // WHEN + let result = experimentsDataStore.experiments + + // THEN + let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result?[subfeatureName1]?.enrollmentDate ?? Date())) + let timeDifference2 = abs(experimentData2.enrollmentDate.timeIntervalSince(result?[subfeatureName2]?.enrollmentDate ?? Date())) + XCTAssertEqual(result?[subfeatureName1]?.cohort, experimentData1.cohort) + XCTAssertLessThanOrEqual(timeDifference1, 1.0) + + XCTAssertEqual(result?[subfeatureName2]?.cohort, experimentData2.cohort) + XCTAssertLessThanOrEqual(timeDifference2, 1.0) + } + + func testExperimentsSetEncodesAndStoresData() throws { + // GIVEN + let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) + let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) + let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] + + // WHEN + experimentsDataStore.experiments = experiments + + // THEN + let storedData = try XCTUnwrap(mockDataStore.data) + let decoder = JSONDecoder() + decoder.dateDecodingStrategy = .secondsSince1970 + let decodedExperiments = try? decoder.decode(Experiments.self, from: storedData) + let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(decodedExperiments?[subfeatureName1]?.enrollmentDate ?? Date())) + let timeDifference2 = abs(experimentData2.enrollmentDate.timeIntervalSince(decodedExperiments?[subfeatureName2]?.enrollmentDate ?? Date())) + XCTAssertEqual(decodedExperiments?[subfeatureName1]?.cohort, experimentData1.cohort) + XCTAssertLessThanOrEqual(timeDifference1, 1.0) + XCTAssertEqual(decodedExperiments?[subfeatureName2]?.cohort, experimentData2.cohort) + XCTAssertLessThanOrEqual(timeDifference2, 1.0) + } +} + +class MockLocalDataStore: LocalDataStoring { + var data: Data? + + func data(forKey defaultName: String) -> Data? { + return data + } + + func set(_ value: Any?, forKey defaultName: String) { + data = value as? Data + } +} From b188d6718c0c6fda5a760031d4ea57c120dae91c Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 16:24:48 +0100 Subject: [PATCH 07/18] fix linting --- .../PrivacyConfig/ExperimentCohortsManager.swift | 2 +- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index ba167a0e4..3974d2d45 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -54,7 +54,7 @@ protocol ExperimentCohortsManaging { func removeCohort(for subfeatureID: SubfeatureID) } -class ExperimentCohortsManager: ExperimentCohortsManaging { +final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let queue = DispatchQueue(label: "com.experimentManager.queue") diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index bc7f787cf..25be855d4 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -262,5 +262,5 @@ final class ExperimentCohortsManagerTests: XCTestCase { } class MockExperimentDataStore: ExperimentsDataStoring { - var experiments: Experiments? = nil + var experiments: Experiments? } From 1054923d7b71487d95d8abd5b80526ecc5ac9766 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 17:58:11 +0100 Subject: [PATCH 08/18] address some comments --- .../ExperimentCohortsManager.swift | 36 +++++++++---------- .../PrivacyConfigurationData.swift | 4 +-- .../ExperimentCohortsManagerTests.swift | 30 ++++++++-------- 3 files changed, 34 insertions(+), 36 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 3974d2d45..0288de356 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -42,24 +42,26 @@ protocol ExperimentCohortsManaging { /// Retrieves the enrollment date for the specified subfeature. /// - Parameter subfeatureID: The experiment subfeature for which the enrollment date is needed. /// - Returns: The `Date` of enrollment if one exists; otherwise, returns `nil`. - func enrolmentDate(for subfeatureID: SubfeatureID) -> Date? + func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? /// Assigns a cohort to the given subfeature based on defined weights and saves it to UserDefaults. /// - Parameter subfeature: The experiment subfeature to assign a cohort for. /// - Returns: The name of the assigned cohort, or `nil` if no cohort could be assigned. - func assignCohort(for subfeature: ExperimentSubfeature) -> CohortID? + func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? /// Removes the assigned cohort data for the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort data should be removed. - func removeCohort(for subfeatureID: SubfeatureID) + func removeCohort(from subfeatureID: SubfeatureID) } final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring - private let queue = DispatchQueue(label: "com.experimentManager.queue") private let randomizer: (Range) -> Double - private let experimentsDataKey = "ExperimentsData" + + var experiments: Experiments? { + store.experiments + } init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { self.store = store @@ -67,16 +69,16 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { } func cohort(for subfeatureID: SubfeatureID) -> CohortID? { - guard let experiments = getExperimentData() else { return nil } + guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.cohort } - func enrolmentDate(for subfeatureID: SubfeatureID) -> Date? { - guard let experiments = getExperimentData() else { return nil } + func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { + guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.enrollmentDate } - func assignCohort(for subfeature: ExperimentSubfeature) -> CohortID? { + func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { let cohorts = subfeature.cohorts let totalWeight = cohorts.reduce(0, { $0 + $1.weight }) guard totalWeight > 0 else { return nil } @@ -94,24 +96,20 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { return nil } - func removeCohort(for subfeatureID: SubfeatureID) { - guard var experiments = getExperimentData() else { return } + func removeCohort(from subfeatureID: SubfeatureID) { + guard var experiments = experiments else { return } experiments.removeValue(forKey: subfeatureID) - saveExperimentData(experiments) - } - - private func getExperimentData() -> Experiments? { - return store.experiments + saveExperiment(experiments) } - private func saveExperimentData(_ experiments: Experiments) { + private func saveExperiment(_ experiments: Experiments) { store.experiments = experiments } private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { - var experiments = getExperimentData() ?? Experiments() + var experiments = experiments ?? Experiments() let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) experiments[experimentID] = experimentData - saveExperimentData(experiments) + saveExperiment(experiments) } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index eb9267505..a7673d64c 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -42,12 +42,12 @@ public struct PrivacyConfigurationData { public init?(json: [String: Any]) { guard let name = json["name"] as? String, - let weightValue = json["weight"] as? Int else { + let weight = json["weight"] as? Int else { return nil } self.name = name - self.weight = weightValue + self.weight = weight } } public let features: [FeatureName: PrivacyFeature] diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 25be855d4..518249560 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -72,13 +72,13 @@ final class ExperimentCohortsManagerTests: XCTestCase { XCTAssertEqual(result2, experimentData2.cohort) } - func testEnrolmentDateReturnsCorrectDateIfExists() { + func testEnrollmentDateReturnsCorrectDateIfExists() { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result1 = experimentCohortsManager.enrolmentDate(for: subfeatureName1) - let result2 = experimentCohortsManager.enrolmentDate(for: subfeatureName2) + let result1 = experimentCohortsManager.enrollmentDate(for: subfeatureName1) + let result2 = experimentCohortsManager.enrollmentDate(for: subfeatureName2) // THEN let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result1 ?? Date())) @@ -98,12 +98,12 @@ final class ExperimentCohortsManagerTests: XCTestCase { XCTAssertNil(result) } - func testEnrolmentDateReturnsNilIfDateDoesNotExist() { + func testEnrollmentDateReturnsNilIfDateDoesNotExist() { // GIVEN let subfeatureName = "TestSubfeature" // WHEN - let result = experimentCohortsManager.enrolmentDate(for: subfeatureName) + let result = experimentCohortsManager.enrollmentDate(for: subfeatureName) // THEN XCTAssertNil(result) @@ -114,7 +114,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - experimentCohortsManager.removeCohort(for: subfeatureName1) + experimentCohortsManager.removeCohort(from: subfeatureName1) // THEN let experiments = try XCTUnwrap(mockStore.experiments) @@ -127,7 +127,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { mockStore.experiments = expectedExperiments // WHEN - experimentCohortsManager.removeCohort(for: "someOtherSubfeature") + experimentCohortsManager.removeCohort(from: "someOtherSubfeature") // THEN XCTAssertEqual( mockStore.experiments, expectedExperiments) @@ -138,7 +138,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: []) // WHEN - let result = experimentCohortsManager.assignCohort(for: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) @@ -155,7 +155,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.assignCohort(for: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) @@ -190,7 +190,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.0 } ) - let resultStartOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) + let resultStartOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort1, "Cohort1") // Test case where random value is at the end of Cohort1's range (0.9) @@ -198,7 +198,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.9 } ) - let resultEndOfCohort1 = experimentCohortsManager.assignCohort(for: subfeature) + let resultEndOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort1, "Cohort1") // Test case where random value is at the start of Cohort2's range (1.00 to 4) @@ -206,7 +206,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 1.00 } ) - let resultStartOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + let resultStartOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort2, "Cohort2") // Test case where random value falls exactly within Cohort2's range (2.5) @@ -214,7 +214,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 2.5 } ) - let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultMiddleOfCohort2, "Cohort2") // Test case where random value is at the end of Cohort2's range (4) @@ -222,7 +222,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 3.9 } ) - let resultEndOfCohort2 = experimentCohortsManager.assignCohort(for: subfeature) + let resultEndOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort2, "Cohort2") } @@ -252,7 +252,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { range in Double.random(in: range)} ) - let result = experimentCohortsManager.assignCohort(for: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertEqual(result, "Cohort1") From c05aa96ea141104efb4e12a30df9c3ad996ad563 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 18:01:43 +0100 Subject: [PATCH 09/18] minor refactor --- .../PrivacyConfig/ExperimentCohortsManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 0288de356..271b934d0 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -80,7 +80,7 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { let cohorts = subfeature.cohorts - let totalWeight = cohorts.reduce(0, { $0 + $1.weight }) + let totalWeight = cohorts.map(\.weight).reduce(0, +) guard totalWeight > 0 else { return nil } let randomValue = randomizer(0.. Date: Fri, 8 Nov 2024 18:19:50 +0100 Subject: [PATCH 10/18] use Constants enum --- .../PrivacyConfig/ExperimentsDataStore.swift | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 849f9168b..338c575d7 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -28,8 +28,11 @@ protocol LocalDataStoring { } struct ExperimentsDataStore: ExperimentsDataStoring { + + private enum Constants { + static let experimentsDataKey = "ExperimentsData" + } private let localDataStoring: LocalDataStoring - private let experimentsDataKey = "ExperimentsData" private let queue = DispatchQueue(label: "com.experimentManager.queue") private let decoder = JSONDecoder() private let encoder = JSONEncoder() @@ -43,14 +46,14 @@ struct ExperimentsDataStore: ExperimentsDataStoring { var experiments: Experiments? { get { queue.sync { - guard let savedData = localDataStoring.data(forKey: experimentsDataKey) else { return nil } + guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } return try? decoder.decode(Experiments.self, from: savedData) } } set { queue.sync { if let encodedData = try? encoder.encode(newValue) { - localDataStoring.set(encodedData, forKey: experimentsDataKey) + localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) } } } From f387c7bd800d69bb73cbdfa0fcff949639a7115c Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 13:22:32 +0100 Subject: [PATCH 11/18] use actor --- .../ExperimentCohortsManager.swift | 40 +++++-------- .../PrivacyConfig/ExperimentsDataStore.swift | 26 ++++---- .../ExperimentCohortsManagerTests.swift | 60 +++++++++++-------- .../ExperimentsDataStoreTests.swift | 8 +-- 4 files changed, 65 insertions(+), 69 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 271b934d0..bd5b6451d 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -37,21 +37,21 @@ protocol ExperimentCohortsManaging { /// Retrieves the cohort ID associated with the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. /// - Returns: The cohort ID as a `String` if one exists; otherwise, returns `nil`. - func cohort(for subfeatureID: SubfeatureID) -> CohortID? + func cohort(for subfeatureID: SubfeatureID) async -> CohortID? /// Retrieves the enrollment date for the specified subfeature. /// - Parameter subfeatureID: The experiment subfeature for which the enrollment date is needed. /// - Returns: The `Date` of enrollment if one exists; otherwise, returns `nil`. - func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? + func enrollmentDate(for subfeatureID: SubfeatureID) async -> Date? /// Assigns a cohort to the given subfeature based on defined weights and saves it to UserDefaults. /// - Parameter subfeature: The experiment subfeature to assign a cohort for. /// - Returns: The name of the assigned cohort, or `nil` if no cohort could be assigned. - func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? + func assignCohort(to subfeature: ExperimentSubfeature) async -> CohortID? /// Removes the assigned cohort data for the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort data should be removed. - func removeCohort(from subfeatureID: SubfeatureID) + func removeCohort(from subfeatureID: SubfeatureID) async } final class ExperimentCohortsManager: ExperimentCohortsManaging { @@ -59,26 +59,22 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let randomizer: (Range) -> Double - var experiments: Experiments? { - store.experiments - } - init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { self.store = store self.randomizer = randomizer } - func cohort(for subfeatureID: SubfeatureID) -> CohortID? { - guard let experiments = experiments else { return nil } + func cohort(for subfeatureID: SubfeatureID) async -> CohortID? { + guard let experiments = await store.getExperiments() else { return nil } return experiments[subfeatureID]?.cohort } - func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { - guard let experiments = experiments else { return nil } + func enrollmentDate(for subfeatureID: SubfeatureID) async -> Date? { + guard let experiments = await store.getExperiments() else { return nil } return experiments[subfeatureID]?.enrollmentDate } - func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { + func assignCohort(to subfeature: ExperimentSubfeature) async -> CohortID? { let cohorts = subfeature.cohorts let totalWeight = cohorts.map(\.weight).reduce(0, +) guard totalWeight > 0 else { return nil } @@ -89,27 +85,23 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { for cohort in cohorts { cumulativeWeight += Double(cohort.weight) if randomValue < cumulativeWeight { - saveCohort(cohort.name, in: subfeature.subfeatureID) + await saveCohort(cohort.name, in: subfeature.subfeatureID) return cohort.name } } return nil } - func removeCohort(from subfeatureID: SubfeatureID) { - guard var experiments = experiments else { return } + func removeCohort(from subfeatureID: SubfeatureID) async { + guard var experiments = await store.getExperiments() else { return } experiments.removeValue(forKey: subfeatureID) - saveExperiment(experiments) - } - - private func saveExperiment(_ experiments: Experiments) { - store.experiments = experiments + await store.setExperiments(experiments) } - private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { - var experiments = experiments ?? Experiments() + private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) async { + var experiments = await store.getExperiments() ?? Experiments() let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) experiments[experimentID] = experimentData - saveExperiment(experiments) + await store.setExperiments(experiments) } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 338c575d7..830db2674 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -19,7 +19,8 @@ import Foundation protocol ExperimentsDataStoring { - var experiments: Experiments? { get set } + func getExperiments() async -> Experiments? + func setExperiments(_ experiments: Experiments?) async } protocol LocalDataStoring { @@ -27,7 +28,7 @@ protocol LocalDataStoring { func set(_ value: Any?, forKey defaultName: String) } -struct ExperimentsDataStore: ExperimentsDataStoring { +actor ExperimentsDataStore: ExperimentsDataStoring { private enum Constants { static let experimentsDataKey = "ExperimentsData" @@ -43,19 +44,14 @@ struct ExperimentsDataStore: ExperimentsDataStoring { decoder.dateDecodingStrategy = .secondsSince1970 } - var experiments: Experiments? { - get { - queue.sync { - guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } - return try? decoder.decode(Experiments.self, from: savedData) - } - } - set { - queue.sync { - if let encodedData = try? encoder.encode(newValue) { - localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) - } - } + func getExperiments() async -> Experiments? { + guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } + return try? decoder.decode(Experiments.self, from: savedData) + } + + func setExperiments(_ experiments: Experiments?) async { + if let encodedData = try? encoder.encode(experiments) { + localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) } } } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 518249560..61f9b1836 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -59,26 +59,26 @@ final class ExperimentCohortsManagerTests: XCTestCase { super.tearDown() } - func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { + func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() async { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - let result1 = experimentCohortsManager.cohort(for: subfeatureName1) - let result2 = experimentCohortsManager.cohort(for: subfeatureName2) + let result1 = await experimentCohortsManager.cohort(for: subfeatureName1) + let result2 = await experimentCohortsManager.cohort(for: subfeatureName2) // THEN XCTAssertEqual(result1, experimentData1.cohort) XCTAssertEqual(result2, experimentData2.cohort) } - func testEnrollmentDateReturnsCorrectDateIfExists() { + func testEnrollmentDateReturnsCorrectDateIfExists() async { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result1 = experimentCohortsManager.enrollmentDate(for: subfeatureName1) - let result2 = experimentCohortsManager.enrollmentDate(for: subfeatureName2) + let result1 = await experimentCohortsManager.enrollmentDate(for: subfeatureName1) + let result2 = await experimentCohortsManager.enrollmentDate(for: subfeatureName2) // THEN let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result1 ?? Date())) @@ -87,64 +87,64 @@ final class ExperimentCohortsManagerTests: XCTestCase { XCTAssertNil(result2) } - func testCohortReturnsNilIfCohortDoesNotExist() { + func testCohortReturnsNilIfCohortDoesNotExist() async { // GIVEN let subfeatureName = "TestSubfeature" // WHEN - let result = experimentCohortsManager.cohort(for: subfeatureName) + let result = await experimentCohortsManager.cohort(for: subfeatureName) // THEN XCTAssertNil(result) } - func testEnrollmentDateReturnsNilIfDateDoesNotExist() { + func testEnrollmentDateReturnsNilIfDateDoesNotExist() async { // GIVEN let subfeatureName = "TestSubfeature" // WHEN - let result = experimentCohortsManager.enrollmentDate(for: subfeatureName) + let result = await experimentCohortsManager.enrollmentDate(for: subfeatureName) // THEN XCTAssertNil(result) } - func testRemoveCohortSuccessfullyRemovesData() throws { + func testRemoveCohortSuccessfullyRemovesData() async throws { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - experimentCohortsManager.removeCohort(from: subfeatureName1) + await experimentCohortsManager.removeCohort(from: subfeatureName1) // THEN let experiments = try XCTUnwrap(mockStore.experiments) XCTAssertTrue(experiments.isEmpty) } - func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { + func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() async { // GIVEN let expectedExperiments: Experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] mockStore.experiments = expectedExperiments // WHEN - experimentCohortsManager.removeCohort(from: "someOtherSubfeature") + await experimentCohortsManager.removeCohort(from: "someOtherSubfeature") // THEN XCTAssertEqual( mockStore.experiments, expectedExperiments) } - func testAssignCohortReturnsNilIfNoCohorts() { + func testAssignCohortReturnsNilIfNoCohorts() async { // GIVEN let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: []) // WHEN - let result = experimentCohortsManager.assignCohort(to: subfeature) + let result = await experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) } - func testAssignCohortReturnsNilIfAllWeightsAreZero() { + func testAssignCohortReturnsNilIfAllWeightsAreZero() async { // GIVEN let jsonCohort1: [String: Any] = ["name": "TestCohort", "weight": 0] let jsonCohort2: [String: Any] = ["name": "TestCohort", "weight": 0] @@ -155,13 +155,13 @@ final class ExperimentCohortsManagerTests: XCTestCase { let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.assignCohort(to: subfeature) + let result = await experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) } - func testAssignCohortSelectsCorrectCohortBasedOnWeight() { + func testAssignCohortSelectsCorrectCohortBasedOnWeight() async { // Cohort1 has weight 1, Cohort2 has weight 3 // Total weight is 1 + 3 = 4 let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] @@ -190,7 +190,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.0 } ) - let resultStartOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) + let resultStartOfCohort1 = await experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort1, "Cohort1") // Test case where random value is at the end of Cohort1's range (0.9) @@ -198,7 +198,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.9 } ) - let resultEndOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) + let resultEndOfCohort1 = await experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort1, "Cohort1") // Test case where random value is at the start of Cohort2's range (1.00 to 4) @@ -206,7 +206,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 1.00 } ) - let resultStartOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) + let resultStartOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort2, "Cohort2") // Test case where random value falls exactly within Cohort2's range (2.5) @@ -214,7 +214,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 2.5 } ) - let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) + let resultMiddleOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultMiddleOfCohort2, "Cohort2") // Test case where random value is at the end of Cohort2's range (4) @@ -222,11 +222,11 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 3.9 } ) - let resultEndOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) + let resultEndOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort2, "Cohort2") } - func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() throws { + func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() async throws { // GIVEN let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] let cohorts = [ @@ -252,7 +252,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { range in Double.random(in: range)} ) - let result = experimentCohortsManager.assignCohort(to: subfeature) + let result = await experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertEqual(result, "Cohort1") @@ -262,5 +262,13 @@ final class ExperimentCohortsManagerTests: XCTestCase { } class MockExperimentDataStore: ExperimentsDataStoring { + func getExperiments() async -> BrowserServicesKit.Experiments? { + return experiments + } + + func setExperiments(_ experiments: BrowserServicesKit.Experiments?) async { + self.experiments = experiments + } + var experiments: Experiments? } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift index 0466155b0..275accb13 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift @@ -45,7 +45,7 @@ final class ExperimentsDataStoreTests: XCTestCase { super.tearDown() } - func testExperimentsGetReturnsDecodedExperiments() { + func testExperimentsGetReturnsDecodedExperiments() async { // GIVEN let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) @@ -57,7 +57,7 @@ final class ExperimentsDataStoreTests: XCTestCase { mockDataStore.data = encodedData // WHEN - let result = experimentsDataStore.experiments + let result = await experimentsDataStore.getExperiments() // THEN let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result?[subfeatureName1]?.enrollmentDate ?? Date())) @@ -69,14 +69,14 @@ final class ExperimentsDataStoreTests: XCTestCase { XCTAssertLessThanOrEqual(timeDifference2, 1.0) } - func testExperimentsSetEncodesAndStoresData() throws { + func testExperimentsSetEncodesAndStoresData() async throws { // GIVEN let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - experimentsDataStore.experiments = experiments + await experimentsDataStore.setExperiments(experiments) // THEN let storedData = try XCTUnwrap(mockDataStore.data) From 403d906d26d45ce8db7fd5df5c6b1c69a13afdb5 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 13:25:07 +0100 Subject: [PATCH 12/18] fix linting --- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 61f9b1836..f41f7e6f7 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -265,10 +265,10 @@ class MockExperimentDataStore: ExperimentsDataStoring { func getExperiments() async -> BrowserServicesKit.Experiments? { return experiments } - + func setExperiments(_ experiments: BrowserServicesKit.Experiments?) async { self.experiments = experiments } - + var experiments: Experiments? } From 6825975aa64b055717e43146de0a57814c7d41b5 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 14:51:40 +0100 Subject: [PATCH 13/18] Revert "fix linting" This reverts commit 403d906d26d45ce8db7fd5df5c6b1c69a13afdb5. --- .../PrivacyConfig/ExperimentCohortsManagerTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index f41f7e6f7..61f9b1836 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -265,10 +265,10 @@ class MockExperimentDataStore: ExperimentsDataStoring { func getExperiments() async -> BrowserServicesKit.Experiments? { return experiments } - + func setExperiments(_ experiments: BrowserServicesKit.Experiments?) async { self.experiments = experiments } - + var experiments: Experiments? } From 5829e298bb9854e0d0f5ae0b85eaf52289a5feca Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 14:51:56 +0100 Subject: [PATCH 14/18] Revert "use actor" This reverts commit f387c7bd800d69bb73cbdfa0fcff949639a7115c. --- .../ExperimentCohortsManager.swift | 40 ++++++++----- .../PrivacyConfig/ExperimentsDataStore.swift | 26 ++++---- .../ExperimentCohortsManagerTests.swift | 60 ++++++++----------- .../ExperimentsDataStoreTests.swift | 8 +-- 4 files changed, 69 insertions(+), 65 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index bd5b6451d..271b934d0 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -37,21 +37,21 @@ protocol ExperimentCohortsManaging { /// Retrieves the cohort ID associated with the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. /// - Returns: The cohort ID as a `String` if one exists; otherwise, returns `nil`. - func cohort(for subfeatureID: SubfeatureID) async -> CohortID? + func cohort(for subfeatureID: SubfeatureID) -> CohortID? /// Retrieves the enrollment date for the specified subfeature. /// - Parameter subfeatureID: The experiment subfeature for which the enrollment date is needed. /// - Returns: The `Date` of enrollment if one exists; otherwise, returns `nil`. - func enrollmentDate(for subfeatureID: SubfeatureID) async -> Date? + func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? /// Assigns a cohort to the given subfeature based on defined weights and saves it to UserDefaults. /// - Parameter subfeature: The experiment subfeature to assign a cohort for. /// - Returns: The name of the assigned cohort, or `nil` if no cohort could be assigned. - func assignCohort(to subfeature: ExperimentSubfeature) async -> CohortID? + func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? /// Removes the assigned cohort data for the specified subfeature. /// - Parameter subfeature: The experiment subfeature for which the cohort data should be removed. - func removeCohort(from subfeatureID: SubfeatureID) async + func removeCohort(from subfeatureID: SubfeatureID) } final class ExperimentCohortsManager: ExperimentCohortsManaging { @@ -59,22 +59,26 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let randomizer: (Range) -> Double + var experiments: Experiments? { + store.experiments + } + init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { self.store = store self.randomizer = randomizer } - func cohort(for subfeatureID: SubfeatureID) async -> CohortID? { - guard let experiments = await store.getExperiments() else { return nil } + func cohort(for subfeatureID: SubfeatureID) -> CohortID? { + guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.cohort } - func enrollmentDate(for subfeatureID: SubfeatureID) async -> Date? { - guard let experiments = await store.getExperiments() else { return nil } + func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { + guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.enrollmentDate } - func assignCohort(to subfeature: ExperimentSubfeature) async -> CohortID? { + func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { let cohorts = subfeature.cohorts let totalWeight = cohorts.map(\.weight).reduce(0, +) guard totalWeight > 0 else { return nil } @@ -85,23 +89,27 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { for cohort in cohorts { cumulativeWeight += Double(cohort.weight) if randomValue < cumulativeWeight { - await saveCohort(cohort.name, in: subfeature.subfeatureID) + saveCohort(cohort.name, in: subfeature.subfeatureID) return cohort.name } } return nil } - func removeCohort(from subfeatureID: SubfeatureID) async { - guard var experiments = await store.getExperiments() else { return } + func removeCohort(from subfeatureID: SubfeatureID) { + guard var experiments = experiments else { return } experiments.removeValue(forKey: subfeatureID) - await store.setExperiments(experiments) + saveExperiment(experiments) + } + + private func saveExperiment(_ experiments: Experiments) { + store.experiments = experiments } - private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) async { - var experiments = await store.getExperiments() ?? Experiments() + private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { + var experiments = experiments ?? Experiments() let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) experiments[experimentID] = experimentData - await store.setExperiments(experiments) + saveExperiment(experiments) } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 830db2674..338c575d7 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -19,8 +19,7 @@ import Foundation protocol ExperimentsDataStoring { - func getExperiments() async -> Experiments? - func setExperiments(_ experiments: Experiments?) async + var experiments: Experiments? { get set } } protocol LocalDataStoring { @@ -28,7 +27,7 @@ protocol LocalDataStoring { func set(_ value: Any?, forKey defaultName: String) } -actor ExperimentsDataStore: ExperimentsDataStoring { +struct ExperimentsDataStore: ExperimentsDataStoring { private enum Constants { static let experimentsDataKey = "ExperimentsData" @@ -44,14 +43,19 @@ actor ExperimentsDataStore: ExperimentsDataStoring { decoder.dateDecodingStrategy = .secondsSince1970 } - func getExperiments() async -> Experiments? { - guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } - return try? decoder.decode(Experiments.self, from: savedData) - } - - func setExperiments(_ experiments: Experiments?) async { - if let encodedData = try? encoder.encode(experiments) { - localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) + var experiments: Experiments? { + get { + queue.sync { + guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } + return try? decoder.decode(Experiments.self, from: savedData) + } + } + set { + queue.sync { + if let encodedData = try? encoder.encode(newValue) { + localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) + } + } } } } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 61f9b1836..518249560 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -59,26 +59,26 @@ final class ExperimentCohortsManagerTests: XCTestCase { super.tearDown() } - func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() async { + func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - let result1 = await experimentCohortsManager.cohort(for: subfeatureName1) - let result2 = await experimentCohortsManager.cohort(for: subfeatureName2) + let result1 = experimentCohortsManager.cohort(for: subfeatureName1) + let result2 = experimentCohortsManager.cohort(for: subfeatureName2) // THEN XCTAssertEqual(result1, experimentData1.cohort) XCTAssertEqual(result2, experimentData2.cohort) } - func testEnrollmentDateReturnsCorrectDateIfExists() async { + func testEnrollmentDateReturnsCorrectDateIfExists() { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result1 = await experimentCohortsManager.enrollmentDate(for: subfeatureName1) - let result2 = await experimentCohortsManager.enrollmentDate(for: subfeatureName2) + let result1 = experimentCohortsManager.enrollmentDate(for: subfeatureName1) + let result2 = experimentCohortsManager.enrollmentDate(for: subfeatureName2) // THEN let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result1 ?? Date())) @@ -87,64 +87,64 @@ final class ExperimentCohortsManagerTests: XCTestCase { XCTAssertNil(result2) } - func testCohortReturnsNilIfCohortDoesNotExist() async { + func testCohortReturnsNilIfCohortDoesNotExist() { // GIVEN let subfeatureName = "TestSubfeature" // WHEN - let result = await experimentCohortsManager.cohort(for: subfeatureName) + let result = experimentCohortsManager.cohort(for: subfeatureName) // THEN XCTAssertNil(result) } - func testEnrollmentDateReturnsNilIfDateDoesNotExist() async { + func testEnrollmentDateReturnsNilIfDateDoesNotExist() { // GIVEN let subfeatureName = "TestSubfeature" // WHEN - let result = await experimentCohortsManager.enrollmentDate(for: subfeatureName) + let result = experimentCohortsManager.enrollmentDate(for: subfeatureName) // THEN XCTAssertNil(result) } - func testRemoveCohortSuccessfullyRemovesData() async throws { + func testRemoveCohortSuccessfullyRemovesData() throws { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - await experimentCohortsManager.removeCohort(from: subfeatureName1) + experimentCohortsManager.removeCohort(from: subfeatureName1) // THEN let experiments = try XCTUnwrap(mockStore.experiments) XCTAssertTrue(experiments.isEmpty) } - func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() async { + func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { // GIVEN let expectedExperiments: Experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] mockStore.experiments = expectedExperiments // WHEN - await experimentCohortsManager.removeCohort(from: "someOtherSubfeature") + experimentCohortsManager.removeCohort(from: "someOtherSubfeature") // THEN XCTAssertEqual( mockStore.experiments, expectedExperiments) } - func testAssignCohortReturnsNilIfNoCohorts() async { + func testAssignCohortReturnsNilIfNoCohorts() { // GIVEN let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: []) // WHEN - let result = await experimentCohortsManager.assignCohort(to: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) } - func testAssignCohortReturnsNilIfAllWeightsAreZero() async { + func testAssignCohortReturnsNilIfAllWeightsAreZero() { // GIVEN let jsonCohort1: [String: Any] = ["name": "TestCohort", "weight": 0] let jsonCohort2: [String: Any] = ["name": "TestCohort", "weight": 0] @@ -155,13 +155,13 @@ final class ExperimentCohortsManagerTests: XCTestCase { let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = await experimentCohortsManager.assignCohort(to: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertNil(result) } - func testAssignCohortSelectsCorrectCohortBasedOnWeight() async { + func testAssignCohortSelectsCorrectCohortBasedOnWeight() { // Cohort1 has weight 1, Cohort2 has weight 3 // Total weight is 1 + 3 = 4 let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] @@ -190,7 +190,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.0 } ) - let resultStartOfCohort1 = await experimentCohortsManager.assignCohort(to: subfeature) + let resultStartOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort1, "Cohort1") // Test case where random value is at the end of Cohort1's range (0.9) @@ -198,7 +198,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 0.9 } ) - let resultEndOfCohort1 = await experimentCohortsManager.assignCohort(to: subfeature) + let resultEndOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort1, "Cohort1") // Test case where random value is at the start of Cohort2's range (1.00 to 4) @@ -206,7 +206,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 1.00 } ) - let resultStartOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) + let resultStartOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultStartOfCohort2, "Cohort2") // Test case where random value falls exactly within Cohort2's range (2.5) @@ -214,7 +214,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 2.5 } ) - let resultMiddleOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) + let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultMiddleOfCohort2, "Cohort2") // Test case where random value is at the end of Cohort2's range (4) @@ -222,11 +222,11 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { _ in 3.9 } ) - let resultEndOfCohort2 = await experimentCohortsManager.assignCohort(to: subfeature) + let resultEndOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) XCTAssertEqual(resultEndOfCohort2, "Cohort2") } - func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() async throws { + func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() throws { // GIVEN let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] let cohorts = [ @@ -252,7 +252,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { store: mockStore, randomizer: { range in Double.random(in: range)} ) - let result = await experimentCohortsManager.assignCohort(to: subfeature) + let result = experimentCohortsManager.assignCohort(to: subfeature) // THEN XCTAssertEqual(result, "Cohort1") @@ -262,13 +262,5 @@ final class ExperimentCohortsManagerTests: XCTestCase { } class MockExperimentDataStore: ExperimentsDataStoring { - func getExperiments() async -> BrowserServicesKit.Experiments? { - return experiments - } - - func setExperiments(_ experiments: BrowserServicesKit.Experiments?) async { - self.experiments = experiments - } - var experiments: Experiments? } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift index 275accb13..0466155b0 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift @@ -45,7 +45,7 @@ final class ExperimentsDataStoreTests: XCTestCase { super.tearDown() } - func testExperimentsGetReturnsDecodedExperiments() async { + func testExperimentsGetReturnsDecodedExperiments() { // GIVEN let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) @@ -57,7 +57,7 @@ final class ExperimentsDataStoreTests: XCTestCase { mockDataStore.data = encodedData // WHEN - let result = await experimentsDataStore.getExperiments() + let result = experimentsDataStore.experiments // THEN let timeDifference1 = abs(experimentData1.enrollmentDate.timeIntervalSince(result?[subfeatureName1]?.enrollmentDate ?? Date())) @@ -69,14 +69,14 @@ final class ExperimentsDataStoreTests: XCTestCase { XCTAssertLessThanOrEqual(timeDifference2, 1.0) } - func testExperimentsSetEncodesAndStoresData() async throws { + func testExperimentsSetEncodesAndStoresData() throws { // GIVEN let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - await experimentsDataStore.setExperiments(experiments) + experimentsDataStore.experiments = experiments // THEN let storedData = try XCTUnwrap(mockDataStore.data) From 4ce4cba16d4895a5fd640c1216fe9dcaa059d960 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 15:30:42 +0100 Subject: [PATCH 15/18] remove queue --- .../PrivacyConfig/ExperimentsDataStore.swift | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 338c575d7..75c3b1d14 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -33,28 +33,23 @@ struct ExperimentsDataStore: ExperimentsDataStoring { static let experimentsDataKey = "ExperimentsData" } private let localDataStoring: LocalDataStoring - private let queue = DispatchQueue(label: "com.experimentManager.queue") private let decoder = JSONDecoder() private let encoder = JSONEncoder() - + init(localDataStoring: LocalDataStoring = UserDefaults.standard) { self.localDataStoring = localDataStoring encoder.dateEncodingStrategy = .secondsSince1970 decoder.dateDecodingStrategy = .secondsSince1970 } - + var experiments: Experiments? { get { - queue.sync { - guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } - return try? decoder.decode(Experiments.self, from: savedData) - } + guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } + return try? decoder.decode(Experiments.self, from: savedData) } set { - queue.sync { - if let encodedData = try? encoder.encode(newValue) { - localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) - } + if let encodedData = try? encoder.encode(newValue) { + localDataStoring.set(encodedData, forKey: Constants.experimentsDataKey) } } } From b4ec061cd694f0d88943dbca4c37c69caefdab57 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 15:33:43 +0100 Subject: [PATCH 16/18] fix linting --- .../PrivacyConfig/ExperimentsDataStore.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 75c3b1d14..bdf82819a 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -35,13 +35,13 @@ struct ExperimentsDataStore: ExperimentsDataStoring { private let localDataStoring: LocalDataStoring private let decoder = JSONDecoder() private let encoder = JSONEncoder() - + init(localDataStoring: LocalDataStoring = UserDefaults.standard) { self.localDataStoring = localDataStoring encoder.dateEncodingStrategy = .secondsSince1970 decoder.dateDecodingStrategy = .secondsSince1970 } - + var experiments: Experiments? { get { guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } From ae423074516bd87f7f69d9625c9033de2593c168 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 15:36:46 +0100 Subject: [PATCH 17/18] remove unwanted selg --- .../PrivacyConfig/PrivacyConfigurationData.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index a7673d64c..7cbd2bc71 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -190,9 +190,9 @@ public struct PrivacyConfigurationData { if let cohortData = json[CodingKeys.cohorts.rawValue] as? [[String: Any]] { let parsedCohorts = cohortData.compactMap { Cohort(json: $0) } - self.cohorts = parsedCohorts.isEmpty ? nil : parsedCohorts + cohorts = parsedCohorts.isEmpty ? nil : parsedCohorts } else { - self.cohorts = nil + cohorts = nil } } } From a49bda137191d13e76cc423b0aaf7b23021b89f8 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 15:48:03 +0100 Subject: [PATCH 18/18] update docs --- .../ExperimentCohortsManager.swift | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 271b934d0..abd01290b 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -35,22 +35,22 @@ typealias Experiments = [String: ExperimentData] protocol ExperimentCohortsManaging { /// Retrieves the cohort ID associated with the specified subfeature. - /// - Parameter subfeature: The experiment subfeature for which the cohort ID is needed. + /// - Parameter subfeatureID: The name of the experiment subfeature for which the cohort ID is needed. /// - Returns: The cohort ID as a `String` if one exists; otherwise, returns `nil`. func cohort(for subfeatureID: SubfeatureID) -> CohortID? /// Retrieves the enrollment date for the specified subfeature. - /// - Parameter subfeatureID: The experiment subfeature for which the enrollment date is needed. + /// - Parameter subfeatureID: The name of the experiment subfeature for which the enrollment date is needed. /// - Returns: The `Date` of enrollment if one exists; otherwise, returns `nil`. func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? /// Assigns a cohort to the given subfeature based on defined weights and saves it to UserDefaults. - /// - Parameter subfeature: The experiment subfeature to assign a cohort for. + /// - Parameter subfeature: The ExperimentSubfeature to which a cohort needs to be assigned to. /// - Returns: The name of the assigned cohort, or `nil` if no cohort could be assigned. func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? /// Removes the assigned cohort data for the specified subfeature. - /// - Parameter subfeature: The experiment subfeature for which the cohort data should be removed. + /// - Parameter subfeatureID: The name of the experiment subfeature for which the cohort data should be removed. func removeCohort(from subfeatureID: SubfeatureID) } @@ -59,22 +59,18 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let randomizer: (Range) -> Double - var experiments: Experiments? { - store.experiments - } - init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { self.store = store self.randomizer = randomizer } func cohort(for subfeatureID: SubfeatureID) -> CohortID? { - guard let experiments = experiments else { return nil } + guard let experiments = store.experiments else { return nil } return experiments[subfeatureID]?.cohort } func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { - guard let experiments = experiments else { return nil } + guard let experiments = store.experiments else { return nil } return experiments[subfeatureID]?.enrollmentDate } @@ -97,19 +93,15 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { } func removeCohort(from subfeatureID: SubfeatureID) { - guard var experiments = experiments else { return } + guard var experiments = store.experiments else { return } experiments.removeValue(forKey: subfeatureID) - saveExperiment(experiments) - } - - private func saveExperiment(_ experiments: Experiments) { store.experiments = experiments } private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { - var experiments = experiments ?? Experiments() + var experiments = store.experiments ?? Experiments() let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) experiments[experimentID] = experimentData - saveExperiment(experiments) + store.experiments = experiments } }