From 216a00e6f78f40d13ee31ccc3c9179f2deb0bfca Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 8 Nov 2024 13:05:41 +0100 Subject: [PATCH 01/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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/19] 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 e862ad8640793604f006dc3bf78d20b337af4b0a Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Mon, 11 Nov 2024 13:14:36 +0100 Subject: [PATCH 11/19] add Targets and configurations --- .../AppPrivacyConfiguration.swift | 16 ++ .../PrivacyConfig/PrivacyConfiguration.swift | 1 + .../PrivacyConfigurationData.swift | 35 +++- .../PrivacyConfigurationManager.swift | 5 + .../AppPrivacyConfigurationTests.swift | 180 ++++++++++++++++++ .../PrivacyConfigurationDataTests.swift | 3 + .../Resources/privacy-config-example.json | 10 + 7 files changed, 248 insertions(+), 2 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 89e64093d..3c07a7918 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -33,6 +33,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { private let locallyUnprotected: DomainsProtectionStore private let internalUserDecider: InternalUserDecider private let userDefaults: UserDefaults + private let locale: Locale private let installDate: Date? public init(data: PrivacyConfigurationData, @@ -40,12 +41,14 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { localProtection: DomainsProtectionStore, internalUserDecider: InternalUserDecider, userDefaults: UserDefaults = UserDefaults(), + locale: Locale = Locale.current, installDate: Date? = nil) { self.data = data self.identifier = identifier self.locallyUnprotected = localProtection self.internalUserDecider = internalUserDecider self.userDefaults = userDefaults + self.locale = locale self.installDate = installDate } @@ -193,9 +196,11 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { public func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + // Check parent feature state let parentState = stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) guard case .enabled = parentState else { return parentState } + // Check sub-feature state let subfeatures = subfeatures(for: subfeature.parent) let subfeatureData = subfeatures[subfeature.rawValue] @@ -210,6 +215,12 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { default: return .disabled(.disabledInConfig) } + // Check Targets + // It should not be wrapped in an array and will be removed at some point + if let target = subfeatureData?.targets?.first, !matchTarget(target: target){ + return .disabled(.targetDoesNotMatch) + } + // Handle Rollouts if let rollout = subfeatureData?.rollout, !isRolloutEnabled(subfeature: subfeature, rolloutSteps: rollout.steps, randomizer: randomizer) { @@ -219,6 +230,11 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { return .enabled } + private func matchTarget(target: PrivacyConfigurationData.PrivacyFeature.Feature.Target) -> Bool{ + return target.localeCountry == locale.regionCode && + target.localeLanguage == locale.languageCode + } + private func subfeatures(for feature: PrivacyFeature) -> PrivacyConfigurationData.PrivacyFeature.Features { return data.features[feature.rawValue]?.features ?? [:] } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift index 37d5ace68..a5b63ea30 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift @@ -30,6 +30,7 @@ public enum PrivacyConfigurationFeatureDisabledReason: Equatable { case tooOldInstallation case limitedToInternalUsers case stillInRollout + case targetDoesNotMatch } public protocol PrivacyConfiguration { diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index a7673d64c..069cda914 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -136,6 +136,8 @@ public struct PrivacyConfigurationData { case minSupportedVersion case rollout case cohorts + case targets + case config } public struct Rollout: Hashable { @@ -169,10 +171,27 @@ public struct PrivacyConfigurationData { } } + public struct Target { + enum CodingKeys: String { + case localeCountry + case localeLanguage + } + + public let localeCountry: String + public let localeLanguage: String + + public init(json: [String: Any]) { + self.localeCountry = json[CodingKeys.localeCountry.rawValue] as? String ?? "" + self.localeLanguage = json[CodingKeys.localeLanguage.rawValue] as? String ?? "" + } + } + public let state: FeatureState public let minSupportedVersion: FeatureSupportedVersion? public let rollout: Rollout? public let cohorts: [Cohort]? + public let targets: [Target]? + public let config: [String : String]? public init?(json: [String: Any]) { guard let state = json[CodingKeys.state.rawValue] as? String else { @@ -190,9 +209,21 @@ 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 { + cohorts = nil + } + + if let targetData = json[CodingKeys.targets.rawValue] as? [[String: Any]] { + targets = targetData.compactMap { Target(json: $0) } + } else { + targets = nil + } + + if let configData = json[CodingKeys.config.rawValue] as? [String: String] { + config = configData } else { - self.cohorts = nil + config = nil } } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift index 219a01613..33e60f898 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift @@ -55,6 +55,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { private let localProtection: DomainsProtectionStore private let errorReporting: EventMapping? private let installDate: Date? + private let locale: Locale public let internalUserDecider: InternalUserDecider @@ -110,12 +111,14 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { localProtection: DomainsProtectionStore, errorReporting: EventMapping? = nil, internalUserDecider: InternalUserDecider, + locale: Locale = Locale.current, installDate: Date? = nil ) { self.embeddedDataProvider = embeddedDataProvider self.localProtection = localProtection self.errorReporting = errorReporting self.internalUserDecider = internalUserDecider + self.locale = locale self.installDate = installDate reload(etag: fetchedETag, data: fetchedData) @@ -127,6 +130,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { identifier: fetchedData.etag, localProtection: localProtection, internalUserDecider: internalUserDecider, + locale: locale, installDate: installDate) } @@ -134,6 +138,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { identifier: embeddedConfigData.etag, localProtection: localProtection, internalUserDecider: internalUserDecider, + locale: locale, installDate: installDate) } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift index 252a7b866..0929b7c44 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift @@ -907,6 +907,186 @@ class AppPrivacyConfigurationTests: XCTestCase { XCTAssertEqual(configAfterUpdate.stateFor(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:)), .disabled(.disabledInConfig)) } + let exampleSubfeatureEnabledWithTarget = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "targets": [ + { + "localeCountry": "US", + "localeLanguage": "fr" + } + ] + } + } + } + } + } + """.data(using: .utf8)! + + func testWhenCheckingSubfeatureStateWithSubfeatureEnabledWhenTargetMatches_SubfeatureShouldBeEnabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureEnabledWithTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "fr_US") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) + } + + func testWhenCheckingSubfeatureStateWithSubfeatureEnabledWhenRegionDoesNotMatches_SubfeatureShouldBeDisabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureEnabledWithTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "fr_FR") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .disabled(.targetDoesNotMatch)) + } + + func testWhenCheckingSubfeatureStateWithSubfeatureEnabledWhenLanguageDoesNotMatches_SubfeatureShouldBeDisabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureEnabledWithTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "it_US") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .disabled(.targetDoesNotMatch)) + } + + let exampleSubfeatureDisabledWithTarget = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "disabled", + "targets": [ + { + "localeCountry": "US", + "localeLanguage": "fr" + } + ] + } + } + } + } + } + """.data(using: .utf8)! + + func testWhenCheckingSubfeatureStateWithSubfeatureDisabledWhenTargetMatches_SubfeatureShouldBeDisabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureDisabledWithTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "fr_US") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .disabled(.disabledInConfig)) + } + + let exampleSubfeatureEnabledWithRolloutAndTarget = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "targets": [ + { + "localeCountry": "US", + "localeLanguage": "fr" + } + ], + "rollout": { + "steps": [{ + "percent": 5.0 + }] + } + } + } + } + } + } + """.data(using: .utf8)! + + func testWhenCheckingSubfeatureStateWithSubfeatureEnabledAndTargetMatchesWhenNotInRollout_SubfeatureShouldBeDisabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureEnabledWithRolloutAndTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "fr_US") + mockRandomValue = 7.0 + clearRolloutData(feature: "autofill", subFeature: "credentialsSaving") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .disabled(.stillInRollout)) + } + + func testWhenCheckingSubfeatureStateWithSubfeatureEnabledAndTargetMatchesWhenInRollout_SubfeatureShouldBeEnabled() { + let mockEmbeddedData = MockEmbeddedDataProvider(data: exampleSubfeatureEnabledWithRolloutAndTarget, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + let locale = Locale(identifier: "fr_US") + mockRandomValue = 2.0 + clearRolloutData(feature: "autofill", subFeature: "credentialsSaving") + + let manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale) + let config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) + } + let exampleEnabledSubfeatureWithRollout = """ { diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift index fac814b1c..3b4a997f1 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift @@ -68,6 +68,9 @@ class PrivacyConfigurationDataTests: XCTestCase { XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?.count, 3) XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].name, "myExperimentControl") XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].weight, 1) + XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeCountry, "US") + XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeLanguage, "fr") + XCTAssertEqual(subfeatures["enabledSubfeature"]?.config, ["foo" : "foo/value", "bar": "bar/value"]) 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 3fd5be5a5..8523bc0fe 100644 --- a/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json +++ b/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json @@ -171,6 +171,16 @@ }, "enabledSubfeature": { "state": "enabled", + "targets": [ + { + "localeCountry": "US", + "localeLanguage": "fr" + } + ], + "config": { + "foo": "foo/value", + "bar": "bar/value" + }, "description": "A description of the sub-feature", "cohorts": [ { From 6090a1513982a6b2ba614176d07fa4b2af7a0989 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Mon, 11 Nov 2024 13:32:10 +0100 Subject: [PATCH 12/19] fix linting issues --- .../PrivacyConfig/PrivacyConfigurationData.swift | 2 +- .../PrivacyConfig/PrivacyConfigurationManager.swift | 2 +- .../PrivacyConfig/AppPrivacyConfigurationTests.swift | 4 ++-- .../PrivacyConfig/PrivacyConfigurationDataTests.swift | 2 +- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index 069cda914..42df09e91 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -191,7 +191,7 @@ public struct PrivacyConfigurationData { public let rollout: Rollout? public let cohorts: [Cohort]? public let targets: [Target]? - public let config: [String : String]? + public let config: [String: String]? public init?(json: [String: Any]) { guard let state = json[CodingKeys.state.rawValue] as? String else { diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift index 33e60f898..004729e70 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift @@ -138,7 +138,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { identifier: embeddedConfigData.etag, localProtection: localProtection, internalUserDecider: internalUserDecider, - locale: locale, + locale: locale, installDate: installDate) } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift index 0929b7c44..9388b14e9 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift @@ -1064,7 +1064,7 @@ class AppPrivacyConfigurationTests: XCTestCase { locale: locale) let config = manager.privacyConfig - XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .disabled(.stillInRollout)) } @@ -1083,7 +1083,7 @@ class AppPrivacyConfigurationTests: XCTestCase { locale: locale) let config = manager.privacyConfig - XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, randomizer: mockRandom(in:))) XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift index 3b4a997f1..88119434b 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift @@ -70,7 +70,7 @@ class PrivacyConfigurationDataTests: XCTestCase { XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].weight, 1) XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeCountry, "US") XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeLanguage, "fr") - XCTAssertEqual(subfeatures["enabledSubfeature"]?.config, ["foo" : "foo/value", "bar": "bar/value"]) + XCTAssertEqual(subfeatures["enabledSubfeature"]?.config, ["foo": "foo/value", "bar": "bar/value"]) XCTAssertEqual(subfeatures["internalSubfeature"]?.state, "internal") } else { XCTFail("Could not parse subfeatures") From 14f92e927fbd455bf40040c6aa1f9a55312ced50 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Tue, 12 Nov 2024 12:04:48 +0100 Subject: [PATCH 13/19] initial implementation --- .../AppPrivacyConfiguration.swift | 65 +++++++++++++++++-- .../ExperimentCohortsManager.swift | 25 +++---- .../PrivacyConfig/ExperimentsDataStore.swift | 10 +-- .../PrivacyConfig/PrivacyConfiguration.swift | 13 ++-- .../UserContentControllerTests.swift | 8 +-- .../AppPrivacyConfigurationTests.swift | 14 ++-- ...SubscriptionFeatureAvailabilityTests.swift | 4 +- Tests/DDGSyncTests/Mocks/Mocks.swift | 8 +-- 8 files changed, 98 insertions(+), 49 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 3c07a7918..5a55c2ba9 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -34,6 +34,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { private let internalUserDecider: InternalUserDecider private let userDefaults: UserDefaults private let locale: Locale + private let experimentManager: ExperimentCohortsManaging private let installDate: Date? public init(data: PrivacyConfigurationData, @@ -42,6 +43,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { internalUserDecider: InternalUserDecider, userDefaults: UserDefaults = UserDefaults(), locale: Locale = Locale.current, + experimentManager: ExperimentCohortsManaging = ExperimentCohortsManager(), installDate: Date? = nil) { self.data = data self.identifier = identifier @@ -49,6 +51,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { self.internalUserDecider = internalUserDecider self.userDefaults = userDefaults self.locale = locale + self.experimentManager = experimentManager self.installDate = installDate } @@ -183,10 +186,11 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } public func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, + cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Bool { - - switch stateFor(subfeature, versionProvider: versionProvider, randomizer: randomizer) { + + switch stateFor(subfeature, cohortID: cohortID, versionProvider: versionProvider, randomizer: randomizer) { case .enabled: return true case .disabled: @@ -194,18 +198,26 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } } - public func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + private func isParentFeatureEnabed(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider) -> PrivacyConfigurationFeatureState { + return stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) + } + + public func stateFor(_ subfeature: any PrivacySubfeature, + cohortID: CohortID?, + versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - // Check parent feature state + // Step 1: Check parent feature state let parentState = stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) guard case .enabled = parentState else { return parentState } - // Check sub-feature state + // Step 2: Retrieve subfeature data and check version let subfeatures = subfeatures(for: subfeature.parent) let subfeatureData = subfeatures[subfeature.rawValue] let satisfiesMinVersion = satisfiesMinVersion(subfeatureData?.minSupportedVersion, versionProvider: versionProvider) + // Step 3: Check sub-feature state switch subfeatureData?.state { case PrivacyConfigurationData.State.enabled: guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } @@ -215,6 +227,48 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { default: return .disabled(.disabledInConfig) } + // Step 4: Check if a cohort was passed in the func + // If no corhort passed check for Target and Rollout + guard let passedCohort = cohortID else { + return checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) + } + + // Step 5: Verify there are cohorts in the subfeature data + // If not remove cohort (in case it was previously assigned) + // and check for Target and Rollout + guard let cohorts = subfeatureData?.cohorts else { + experimentManager.removeCohort(from: subfeature.rawValue) + return checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) + } + + // Step 6: Verify there the cohorts in the subfeature contain the cohort passed in the func + // If not remove cohort (in case it was previously assigned) before proceeding + if !cohorts.contains(where: { $0.name == passedCohort + }) { + experimentManager.removeCohort(from: subfeature.rawValue) + } + + // Step 7: Check if a cohort was already assigned + // If so check if it matches the one passed in the func and return .enable or disabled accordingly + if let assignedCohort = experimentManager.cohort(for: subfeature.rawValue) { + return (assignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) + } + + // Step 8: check Target and Rollout + // if disabled return .disabled otherwise continue + let targetAndRolloutState = checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) + if targetAndRolloutState != .enabled { + return targetAndRolloutState + } + + // Step 9: Assign cohort and check if they match + let newAssignedCohort = experimentManager.assignCohort(to: ExperimentSubfeature(subfeatureID: subfeature.rawValue, cohorts: cohorts)) + return (newAssignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) + } + + private func checkTargetAndRollouts(_ subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature?, + subfeature: any PrivacySubfeature, + randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { // Check Targets // It should not be wrapped in an array and will be removed at some point if let target = subfeatureData?.targets?.first, !matchTarget(target: target){ @@ -230,6 +284,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { return .enabled } + private func matchTarget(target: PrivacyConfigurationData.PrivacyFeature.Feature.Target) -> Bool{ return target.localeCountry == locale.regionCode && target.localeLanguage == locale.languageCode diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 271b934d0..19f8a5355 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -18,22 +18,22 @@ import Foundation -struct ExperimentSubfeature { +public struct ExperimentSubfeature { let subfeatureID: SubfeatureID let cohorts: [PrivacyConfigurationData.Cohort] } -typealias CohortID = String -typealias SubfeatureID = String +public typealias CohortID = String +public typealias SubfeatureID = String -struct ExperimentData: Codable, Equatable { +public struct ExperimentData: Codable, Equatable { let cohort: String let enrollmentDate: Date } -typealias Experiments = [String: ExperimentData] +public typealias Experiments = [String: ExperimentData] -protocol ExperimentCohortsManaging { +public 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`. @@ -54,7 +54,7 @@ protocol ExperimentCohortsManaging { func removeCohort(from subfeatureID: SubfeatureID) } -final class ExperimentCohortsManager: ExperimentCohortsManaging { +public final class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let randomizer: (Range) -> Double @@ -63,22 +63,23 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { store.experiments } - init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double) { + public init(store: ExperimentsDataStoring = ExperimentsDataStore(), + randomizer: @escaping (Range) -> Double = Double.random(in:)) { self.store = store self.randomizer = randomizer } - func cohort(for subfeatureID: SubfeatureID) -> CohortID? { + public func cohort(for subfeatureID: SubfeatureID) -> CohortID? { guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.cohort } - func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { + public func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { guard let experiments = experiments else { return nil } return experiments[subfeatureID]?.enrollmentDate } - func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { + public func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { let cohorts = subfeature.cohorts let totalWeight = cohorts.map(\.weight).reduce(0, +) guard totalWeight > 0 else { return nil } @@ -96,7 +97,7 @@ final class ExperimentCohortsManager: ExperimentCohortsManaging { return nil } - func removeCohort(from subfeatureID: SubfeatureID) { + public func removeCohort(from subfeatureID: SubfeatureID) { guard var experiments = experiments else { return } experiments.removeValue(forKey: subfeatureID) saveExperiment(experiments) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift index 338c575d7..bb43ab426 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentsDataStore.swift @@ -18,16 +18,16 @@ import Foundation -protocol ExperimentsDataStoring { +public protocol ExperimentsDataStoring { var experiments: Experiments? { get set } } -protocol LocalDataStoring { +public protocol LocalDataStoring { func data(forKey defaultName: String) -> Data? func set(_ value: Any?, forKey defaultName: String) } -struct ExperimentsDataStore: ExperimentsDataStoring { +public struct ExperimentsDataStore: ExperimentsDataStoring { private enum Constants { static let experimentsDataKey = "ExperimentsData" @@ -37,13 +37,13 @@ struct ExperimentsDataStore: ExperimentsDataStoring { private let decoder = JSONDecoder() private let encoder = JSONEncoder() - init(localDataStoring: LocalDataStoring = UserDefaults.standard) { + public init(localDataStoring: LocalDataStoring = UserDefaults.standard) { self.localDataStoring = localDataStoring encoder.dateEncodingStrategy = .secondsSince1970 decoder.dateDecodingStrategy = .secondsSince1970 } - var experiments: Experiments? { + public var experiments: Experiments? { get { queue.sync { guard let savedData = localDataStoring.data(forKey: Constants.experimentsDataKey) else { return nil } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift index a5b63ea30..94304aa27 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift @@ -31,6 +31,7 @@ public enum PrivacyConfigurationFeatureDisabledReason: Equatable { case limitedToInternalUsers case stillInRollout case targetDoesNotMatch + case experimentCohortDoesNotMatch } public protocol PrivacyConfiguration { @@ -57,8 +58,8 @@ public protocol PrivacyConfiguration { func isEnabled(featureKey: PrivacyFeature, versionProvider: AppVersionProvider) -> Bool func stateFor(featureKey: PrivacyFeature, versionProvider: AppVersionProvider) -> PrivacyConfigurationFeatureState - func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Bool - func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState + func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Bool + func stateFor(_ subfeature: any PrivacySubfeature, cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState /// Domains for which given PrivacyFeature is disabled. /// @@ -114,11 +115,11 @@ public extension PrivacyConfiguration { return stateFor(featureKey: featureKey, versionProvider: AppVersionProvider()) } - func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, randomizer: (Range) -> Double = Double.random(in:)) -> Bool { - return isSubfeatureEnabled(subfeature, versionProvider: AppVersionProvider(), randomizer: randomizer) + func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, cohortID: CohortID? = nil, randomizer: (Range) -> Double = Double.random(in:)) -> Bool { + return isSubfeatureEnabled(subfeature, cohortID: cohortID, versionProvider: AppVersionProvider(), randomizer: randomizer) } - func stateFor(_ subfeature: any PrivacySubfeature, randomizer: (Range) -> Double = Double.random(in:)) -> PrivacyConfigurationFeatureState { - return stateFor(subfeature, versionProvider: AppVersionProvider(), randomizer: randomizer) + func stateFor(_ subfeature: any PrivacySubfeature, cohortID: CohortID? = nil, randomizer: (Range) -> Double = Double.random(in:)) -> PrivacyConfigurationFeatureState { + return stateFor(subfeature, cohortID: cohortID, versionProvider: AppVersionProvider(), randomizer: randomizer) } } diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/UserContentControllerTests.swift b/Tests/BrowserServicesKitTests/ContentBlocker/UserContentControllerTests.swift index e1cb475cd..78d527308 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/UserContentControllerTests.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/UserContentControllerTests.swift @@ -318,15 +318,11 @@ class PrivacyConfigurationMock: PrivacyConfiguration { return .enabled } - func isSubfeatureEnabled( - _ subfeature: any PrivacySubfeature, - versionProvider: AppVersionProvider, - randomizer: (Range) -> Double - ) -> Bool { + func isSubfeatureEnabled(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> Bool { true } - func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + func stateFor(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> BrowserServicesKit.PrivacyConfigurationFeatureState { return .enabled } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift index 9388b14e9..e3be10a1f 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AppPrivacyConfigurationTests.swift @@ -586,13 +586,13 @@ class AppPrivacyConfigurationTests: XCTestCase { let config = manager.privacyConfig let oldVersionProvider = MockAppVersionProvider(appVersion: "1.35.0") - XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, versionProvider: oldVersionProvider, randomizer: Double.random(in:))) - XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving, versionProvider: oldVersionProvider, randomizer: Double.random(in:)), .disabled(.appVersionNotSupported)) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: oldVersionProvider, randomizer: Double.random(in:))) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: oldVersionProvider, randomizer: Double.random(in:)), .disabled(.appVersionNotSupported)) let currentVersionProvider = MockAppVersionProvider(appVersion: "1.36.0") - XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, versionProvider: currentVersionProvider, randomizer: Double.random(in:))) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: currentVersionProvider, randomizer: Double.random(in:))) XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) let futureVersionProvider = MockAppVersionProvider(appVersion: "2.16.0") - XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, versionProvider: futureVersionProvider, randomizer: Double.random(in:))) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: futureVersionProvider, randomizer: Double.random(in:))) XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) } @@ -661,12 +661,12 @@ class AppPrivacyConfigurationTests: XCTestCase { let oldVersionProvider = MockAppVersionProvider(appVersion: "1.35.0") XCTAssertFalse(config.isEnabled(featureKey: .autofill, versionProvider: oldVersionProvider)) - XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, versionProvider: oldVersionProvider, randomizer: Double.random(in:))) - XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving, versionProvider: oldVersionProvider, randomizer: Double.random(in:)), .disabled(.appVersionNotSupported)) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: oldVersionProvider, randomizer: Double.random(in:))) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: oldVersionProvider, randomizer: Double.random(in:)), .disabled(.appVersionNotSupported)) let currentVersionProvider = MockAppVersionProvider(appVersion: "1.36.0") XCTAssertTrue(config.isEnabled(featureKey: .autofill, versionProvider: currentVersionProvider)) - XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, versionProvider: currentVersionProvider, randomizer: Double.random(in:))) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: nil, versionProvider: currentVersionProvider, randomizer: Double.random(in:))) XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) } diff --git a/Tests/BrowserServicesKitTests/Subscription/SubscriptionFeatureAvailabilityTests.swift b/Tests/BrowserServicesKitTests/Subscription/SubscriptionFeatureAvailabilityTests.swift index a85dadc1e..0334f4f7a 100644 --- a/Tests/BrowserServicesKitTests/Subscription/SubscriptionFeatureAvailabilityTests.swift +++ b/Tests/BrowserServicesKitTests/Subscription/SubscriptionFeatureAvailabilityTests.swift @@ -221,11 +221,11 @@ class MockPrivacyConfiguration: PrivacyConfiguration { var isSubfeatureEnabledCheck: ((any PrivacySubfeature) -> Bool)? - func isSubfeatureEnabled(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Bool { + func isSubfeatureEnabled(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> Bool { isSubfeatureEnabledCheck?(subfeature) ?? false } - func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + func stateFor(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> BrowserServicesKit.PrivacyConfigurationFeatureState { if isSubfeatureEnabledCheck?(subfeature) == true { return .enabled } diff --git a/Tests/DDGSyncTests/Mocks/Mocks.swift b/Tests/DDGSyncTests/Mocks/Mocks.swift index 013123c6d..b65b8abdb 100644 --- a/Tests/DDGSyncTests/Mocks/Mocks.swift +++ b/Tests/DDGSyncTests/Mocks/Mocks.swift @@ -169,15 +169,11 @@ class MockPrivacyConfiguration: PrivacyConfiguration { return .enabled } - func isSubfeatureEnabled( - _ subfeature: any PrivacySubfeature, - versionProvider: AppVersionProvider, - randomizer: (Range) -> Double - ) -> Bool { + func isSubfeatureEnabled(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> Bool { true } - func stateFor(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + func stateFor(_ subfeature: any BrowserServicesKit.PrivacySubfeature, cohortID: BrowserServicesKit.CohortID?, versionProvider: BrowserServicesKit.AppVersionProvider, randomizer: (Range) -> Double) -> BrowserServicesKit.PrivacyConfigurationFeatureState { return .enabled } From 8bda08cea171d7838fe0810de9fab550c64b8d12 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Thu, 14 Nov 2024 13:06:43 +0100 Subject: [PATCH 14/19] add tests --- .../AppPrivacyConfiguration.swift | 153 ++-- .../ExperimentCohortsManager.swift | 13 +- .../Features/PrivacyFeature.swift | 2 +- .../PrivacyConfigurationData.swift | 8 +- .../PrivacyConfigurationManager.swift | 7 +- ...dPrivacyConfigurationExperimentTests.swift | 749 ++++++++++++++++++ .../ExperimentCohortsManagerTests.swift | 12 +- .../ExperimentsDataStoreTests.swift | 8 +- 8 files changed, 862 insertions(+), 90 deletions(-) create mode 100644 Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 5a55c2ba9..49d5c87ac 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -36,6 +36,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { private let locale: Locale private let experimentManager: ExperimentCohortsManaging private let installDate: Date? + private let experimentManagerQueue = DispatchQueue(label: "com.experimentManager.queue") public init(data: PrivacyConfigurationData, identifier: String, @@ -198,96 +199,110 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } } - private func isParentFeatureEnabed(_ subfeature: any PrivacySubfeature, versionProvider: AppVersionProvider) -> PrivacyConfigurationFeatureState { - return stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) - } - - public func stateFor(_ subfeature: any PrivacySubfeature, + public func stateFor(experiment: ExperimentData, + parentID: String, cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - - // Step 1: Check parent feature state - let parentState = stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) + let parentFeature = PrivacyFeature(rawValue: parentID)! + let parentState = stateFor(featureKey: parentFeature, versionProvider: versionProvider) guard case .enabled = parentState else { return parentState } + let subfeatures = subfeatures(for: parentFeature) + let subfeatureData = subfeatures[subfeatureID] + subfeatureData. - // Step 2: Retrieve subfeature data and check version - let subfeatures = subfeatures(for: subfeature.parent) - let subfeatureData = subfeatures[subfeature.rawValue] - - let satisfiesMinVersion = satisfiesMinVersion(subfeatureData?.minSupportedVersion, versionProvider: versionProvider) - - // Step 3: Check sub-feature state - switch subfeatureData?.state { - case PrivacyConfigurationData.State.enabled: - guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } - case PrivacyConfigurationData.State.internal: - guard internalUserDecider.isInternalUser else { return .disabled(.limitedToInternalUsers) } - guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } - default: return .disabled(.disabledInConfig) - } + } - // Step 4: Check if a cohort was passed in the func - // If no corhort passed check for Target and Rollout - guard let passedCohort = cohortID else { - return checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) - } + public func getAllActiveExperiments() -> Experiments { - // Step 5: Verify there are cohorts in the subfeature data - // If not remove cohort (in case it was previously assigned) - // and check for Target and Rollout - guard let cohorts = subfeatureData?.cohorts else { - experimentManager.removeCohort(from: subfeature.rawValue) - return checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) - } + } - // Step 6: Verify there the cohorts in the subfeature contain the cohort passed in the func - // If not remove cohort (in case it was previously assigned) before proceeding - if !cohorts.contains(where: { $0.name == passedCohort - }) { - experimentManager.removeCohort(from: subfeature.rawValue) - } + public func stateFor(_ subfeature: any PrivacySubfeature, + cohortID: CohortID?, + versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + return experimentManagerQueue.sync { + // Step 1: Check parent feature state + let parentState = stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) + guard case .enabled = parentState else { return parentState } + + // Step 2: Retrieve subfeature data and check version + let subfeatures = subfeatures(for: subfeature.parent) + let subfeatureData = subfeatures[subfeature.rawValue] + + let satisfiesMinVersion = satisfiesMinVersion(subfeatureData?.minSupportedVersion, versionProvider: versionProvider) + + // Step 3: Check sub-feature state + switch subfeatureData?.state { + case PrivacyConfigurationData.State.enabled: + guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } + case PrivacyConfigurationData.State.internal: + guard internalUserDecider.isInternalUser else { return .disabled(.limitedToInternalUsers) } + guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } + default: return .disabled(.disabledInConfig) + } - // Step 7: Check if a cohort was already assigned - // If so check if it matches the one passed in the func and return .enable or disabled accordingly - if let assignedCohort = experimentManager.cohort(for: subfeature.rawValue) { - return (assignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) - } + // // Step 4: Handle Rollouts + if let rollout = subfeatureData?.rollout, + !isRolloutEnabled(subfeature: subfeature, rolloutSteps: rollout.steps, randomizer: randomizer) { + return .disabled(.stillInRollout) + } - // Step 8: check Target and Rollout - // if disabled return .disabled otherwise continue - let targetAndRolloutState = checkTargetAndRollouts(subfeatureData, subfeature: subfeature, randomizer: randomizer) - if targetAndRolloutState != .enabled { - return targetAndRolloutState + // Step 5: Check if a cohort was passed in the func + // If no corhort passed check for Target and Rollout + guard let passedCohort = cohortID else { + return checkTargets(subfeatureData) + } + + // Step 6: Verify there are cohorts in the subfeature data + // If not remove cohort (in case it was previously assigned) + // and check for Target and Rollout + guard let cohorts = subfeatureData?.cohorts else { + experimentManager.removeCohort(from: subfeature.rawValue) + return .disabled(.experimentCohortDoesNotMatch) + } + + // Step 7: Verify there the cohorts in the subfeature contain the cohort passed in the func + // If not remove cohort (in case it was previously assigned) before proceeding + if !cohorts.contains(where: { $0.name == passedCohort + }) { + experimentManager.removeCohort(from: subfeature.rawValue) + } + + // Step 8: Check if a cohort was already assigned + // If so check if it matches the one passed in the func and return .enable or disabled accordingly + if let assignedCohort = experimentManager.cohort(for: subfeature.rawValue) { + return (assignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) + } + + // Step 9: check Target and Rollout + // if disabled return .disabled otherwise continue + let targetsState = checkTargets(subfeatureData) + if targetsState != .enabled { + return targetsState + } + + // Step 10: Assign cohort and check if they match + let newAssignedCohort = experimentManager.assignCohort(to: ExperimentSubfeature(parentID: subfeature.parent.rawValue, subfeatureID: subfeature.rawValue, cohorts: cohorts)) + return (newAssignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) } - - // Step 9: Assign cohort and check if they match - let newAssignedCohort = experimentManager.assignCohort(to: ExperimentSubfeature(subfeatureID: subfeature.rawValue, cohorts: cohorts)) - return (newAssignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) } - private func checkTargetAndRollouts(_ subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature?, - subfeature: any PrivacySubfeature, - randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + private func checkTargets(_ subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature?) -> PrivacyConfigurationFeatureState { // Check Targets - // It should not be wrapped in an array and will be removed at some point - if let target = subfeatureData?.targets?.first, !matchTarget(target: target){ + if let targets = subfeatureData?.targets, !matchTargets(targets: targets){ return .disabled(.targetDoesNotMatch) } - // Handle Rollouts - if let rollout = subfeatureData?.rollout, - !isRolloutEnabled(subfeature: subfeature, rolloutSteps: rollout.steps, randomizer: randomizer) { - return .disabled(.stillInRollout) - } - return .enabled } - private func matchTarget(target: PrivacyConfigurationData.PrivacyFeature.Feature.Target) -> Bool{ - return target.localeCountry == locale.regionCode && - target.localeLanguage == locale.languageCode + private func matchTargets(targets: [PrivacyConfigurationData.PrivacyFeature.Feature.Target]) -> Bool { + return targets.contains { target in + (target.localeCountry == nil || target.localeCountry == locale.regionCode) && + (target.localeLanguage == nil || target.localeLanguage == locale.languageCode) + } } private func subfeatures(for feature: PrivacyFeature) -> PrivacyConfigurationData.PrivacyFeature.Features { diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 19f8a5355..5872e03ad 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -19,16 +19,19 @@ import Foundation public struct ExperimentSubfeature { + let parentID: ParentFeatureID let subfeatureID: SubfeatureID let cohorts: [PrivacyConfigurationData.Cohort] } public typealias CohortID = String public typealias SubfeatureID = String +public typealias ParentFeatureID = String public struct ExperimentData: Codable, Equatable { - let cohort: String - let enrollmentDate: Date + public let parentID: String + public let cohort: String + public let enrollmentDate: Date } public typealias Experiments = [String: ExperimentData] @@ -90,7 +93,7 @@ public final class ExperimentCohortsManager: ExperimentCohortsManaging { for cohort in cohorts { cumulativeWeight += Double(cohort.weight) if randomValue < cumulativeWeight { - saveCohort(cohort.name, in: subfeature.subfeatureID) + saveCohort(cohort.name, in: subfeature.subfeatureID, parentID: subfeature.parentID) return cohort.name } } @@ -107,9 +110,9 @@ public final class ExperimentCohortsManager: ExperimentCohortsManaging { store.experiments = experiments } - private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID) { + private func saveCohort(_ cohort: CohortID, in experimentID: SubfeatureID, parentID: ParentFeatureID) { var experiments = experiments ?? Experiments() - let experimentData = ExperimentData(cohort: cohort, enrollmentDate: Date()) + let experimentData = ExperimentData(parentID: parentID, cohort: cohort, enrollmentDate: Date()) experiments[experimentID] = experimentData saveExperiment(experiments) } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index f21c9dc12..027b210c2 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -19,7 +19,7 @@ import Foundation /// Features whose `rawValue` should be the key to access their corresponding `PrivacyConfigurationData.PrivacyFeature` object -public enum PrivacyFeature: String { +public enum PrivacyFeature: String, CaseIterable { case contentBlocking case duckPlayer case fingerprintingTemporaryStorage diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index 42df09e91..2d67ef5e4 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -177,12 +177,12 @@ public struct PrivacyConfigurationData { case localeLanguage } - public let localeCountry: String - public let localeLanguage: String + public let localeCountry: String? + public let localeLanguage: String? public init(json: [String: Any]) { - self.localeCountry = json[CodingKeys.localeCountry.rawValue] as? String ?? "" - self.localeLanguage = json[CodingKeys.localeLanguage.rawValue] as? String ?? "" + self.localeCountry = json[CodingKeys.localeCountry.rawValue] as? String + self.localeLanguage = json[CodingKeys.localeLanguage.rawValue] as? String } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift index 004729e70..7b216b04b 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift @@ -56,6 +56,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { private let errorReporting: EventMapping? private let installDate: Date? private let locale: Locale + private let experimentCohortManager: ExperimentCohortsManaging public let internalUserDecider: InternalUserDecider @@ -112,12 +113,14 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { errorReporting: EventMapping? = nil, internalUserDecider: InternalUserDecider, locale: Locale = Locale.current, + experimentCohortManager: ExperimentCohortsManaging = ExperimentCohortsManager(), installDate: Date? = nil ) { self.embeddedDataProvider = embeddedDataProvider self.localProtection = localProtection self.errorReporting = errorReporting self.internalUserDecider = internalUserDecider + self.experimentCohortManager = experimentCohortManager self.locale = locale self.installDate = installDate @@ -131,6 +134,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { localProtection: localProtection, internalUserDecider: internalUserDecider, locale: locale, + experimentManager: experimentCohortManager, installDate: installDate) } @@ -138,7 +142,8 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { identifier: embeddedConfigData.etag, localProtection: localProtection, internalUserDecider: internalUserDecider, - locale: locale, + locale: locale, + experimentManager: experimentCohortManager, installDate: installDate) } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift new file mode 100644 index 000000000..ebd15b267 --- /dev/null +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift @@ -0,0 +1,749 @@ +// +// AddPrivacyConfigurationExperimentTests.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 +import BrowserServicesKit + +final class AddPrivacyConfigurationExperimentTests: XCTestCase { + + var featureJson: Data = "{}".data(using: .utf8)! + var mockEmbeddedData: MockEmbeddedDataProvider! + var mockStore: MockExperimentDataStore! + var experimentManager: ExperimentCohortsManager! + var manager: PrivacyConfigurationManager! + var locale: Locale! + + let subfeatureName = "credentialsSaving" + + + override func setUp() { + locale = Locale(identifier: "fr_US") + mockEmbeddedData = MockEmbeddedDataProvider(data: featureJson, etag: "test") + let mockInternalUserStore = MockInternalUserStoring() + mockStore = MockExperimentDataStore() + experimentManager = ExperimentCohortsManager(store: mockStore) + manager = PrivacyConfigurationManager(fetchedETag: nil, + fetchedData: nil, + embeddedDataProvider: mockEmbeddedData, + localProtection: MockDomainsProtectionStore(), + internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), + locale: locale, + experimentCohortManager: experimentManager) + } + + override func tearDown() { + featureJson = "".data(using: .utf8)! + mockEmbeddedData = nil + mockStore = nil + experimentManager = nil + manager = nil + } + + + func testCohortOnlyAssignedWhenCallingStateForSubfeature() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + let config = manager.privacyConfig + + // we haven't called isEnabled yet, so cohorts should not be yet assigned + XCTAssertNil(mockStore.experiments) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + // we call isEnabled() without cohort, cohort should not be assigned either + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving), .enabled) + XCTAssertNil(mockStore.experiments) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + // we call isEnabled(cohort), then we should assign cohort + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertEqual(config.stateFor(AutofillSubfeature.credentialsSaving, cohortID: "blue"), .disabled(.experimentCohortDoesNotMatch)) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + } + + func testRemoveAllCohortsRemotelyRemovesAssignedCohort() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // remove blue cohort + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // remove all remaining cohorts + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2 + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertTrue(mockStore.experiments?.isEmpty ?? false) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + } + + func testDisablingFeatureDisablesCohort() { + // Initially subfeature for both cohorts is disabled + var config = manager.privacyConfig + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertNil(mockStore.experiments) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + // When features with cohort the cohort with weight 1 is enabled + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // If the subfeature is then disabled isSubfeatureEnabled should return false + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "disabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // If the subfeature is parent feature disabled isSubfeatureEnabled should return false + featureJson = + """ + { + "features": { + "autofill": { + "state": "disabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + } + + func testCohortsAndTargetsInteraction() { + func featureJson(country: String, language: String) -> Data { + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeLanguage": "\(language)", + "localeCountry": "\(country)" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + } + manager.reload(etag: "", data: featureJson(country: "FR", language: "fr")) + var config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertNil(mockStore.experiments) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + manager.reload(etag: "", data: featureJson(country: "US", language: "en")) + config = manager.privacyConfig + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertNil(mockStore.experiments) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + manager.reload(etag: "", data: featureJson(country: "US", language: "fr")) + config = manager.privacyConfig + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // once cohort is assigned, changing targets shall not affect feature state + manager.reload(etag: "", data: featureJson(country: "IT", language: "it")) + config = manager.privacyConfig + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + let featureJson2 = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "FR" + } + ], + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson2) + config = manager.privacyConfig + + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertTrue(mockStore.experiments?.isEmpty ?? false) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + + // re-populate experiment to re-assign new cohort, should not be assigned as it has wrong targets + manager.reload(etag: "", data: featureJson(country: "IT", language: "it")) + config = manager.privacyConfig + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertTrue(mockStore.experiments?.isEmpty ?? false) + XCTAssertNil(experimentManager.cohort(for: subfeatureName)) + } + + func testChangeRemoteCohortsAfterAssignmentShouldNoop() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // changing targets should not change cohort assignment + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "IT" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // changing cohort weight should not change current assignment + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 0 + }, + { + "name": "blue", + "weight": 1 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // adding cohorts should not change current assignment + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 1 + }, + { + "name": "red", + "weight": 1 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + } + + func testEnrollmentDate() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertTrue(mockStore.experiments?.isEmpty ?? true) + XCTAssertNil(experimentManager.cohort(for: subfeatureName), "control") + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + let currentTime = Date().timeIntervalSince1970 + let enrollmentTime = mockStore.experiments?[subfeatureName]?.enrollmentDate.timeIntervalSince1970 + + XCTAssertNotNil(enrollmentTime) + if let enrollmentTime = enrollmentTime { + let tolerance: TimeInterval = 60 // 1 minute in seconds + XCTAssertEqual(currentTime, enrollmentTime, accuracy: tolerance) + } + } + + func testRollbackCohortExperiments() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "rollout": { + "steps": [ + { + "percent": 100 + } + ] + }, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + var config = manager.privacyConfig + clearRolloutData(feature: "autofill", subFeature: "credentialsSaving") + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "rollout": { + "steps": [ + { + "percent": 0 + } + ] + }, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + } + + func clearRolloutData(feature: String, subFeature: String) { + UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).enabled") + UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).lastRolloutCount") + } +} diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index 518249560..b4bdb88a1 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -45,10 +45,10 @@ final class ExperimentCohortsManagerTests: XCTestCase { ) let expectedDate1 = Date() - experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: expectedDate1) + experimentData1 = ExperimentData(parentID: "TestParent", cohort: "TestCohort1", enrollmentDate: expectedDate1) let expectedDate2 = Date().addingTimeInterval(60) - experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: expectedDate2) + experimentData2 = ExperimentData(parentID: "TestParent", cohort: "TestCohort2", enrollmentDate: expectedDate2) } override func tearDown() { @@ -135,7 +135,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { func testAssignCohortReturnsNilIfNoCohorts() { // GIVEN - let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: []) + let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: []) // WHEN let result = experimentCohortsManager.assignCohort(to: subfeature) @@ -152,7 +152,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { PrivacyConfigurationData.Cohort(json: jsonCohort1)!, PrivacyConfigurationData.Cohort(json: jsonCohort2)! ] - let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN let result = experimentCohortsManager.assignCohort(to: subfeature) @@ -170,7 +170,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { PrivacyConfigurationData.Cohort(json: jsonCohort1)!, PrivacyConfigurationData.Cohort(json: jsonCohort2)! ] - let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) let expectedTotalWeight = 4.0 // Use a custom randomizer to verify the range @@ -232,7 +232,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let cohorts = [ PrivacyConfigurationData.Cohort(json: jsonCohort1)! ] - let subfeature = ExperimentSubfeature(subfeatureID: subfeatureName1, cohorts: cohorts) + let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) let expectedTotalWeight = 1.0 // Use a custom randomizer to verify the range diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift index 0466155b0..e5deb4f60 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift @@ -47,8 +47,8 @@ final class ExperimentsDataStoreTests: XCTestCase { func testExperimentsGetReturnsDecodedExperiments() { // GIVEN - let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) - let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) + let experimentData1 = ExperimentData(parentID: "parent", cohort: "TestCohort1", enrollmentDate: Date()) + let experimentData2 = ExperimentData(parentID: "parent", cohort: "TestCohort2", enrollmentDate: Date()) let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] let encoder = JSONEncoder() @@ -71,8 +71,8 @@ final class ExperimentsDataStoreTests: XCTestCase { func testExperimentsSetEncodesAndStoresData() throws { // GIVEN - let experimentData1 = ExperimentData(cohort: "TestCohort1", enrollmentDate: Date()) - let experimentData2 = ExperimentData(cohort: "TestCohort2", enrollmentDate: Date()) + let experimentData1 = ExperimentData(parentID: "parent", cohort: "TestCohort1", enrollmentDate: Date()) + let experimentData2 = ExperimentData(parentID: "parent", cohort: "TestCohort2", enrollmentDate: Date()) let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN From 56c54a777fb2339997d3e79974c50028a2cf5cd5 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 15 Nov 2024 11:39:08 +0100 Subject: [PATCH 15/19] refactor --- .../AppPrivacyConfiguration.swift | 126 +++++------ .../ExperimentCohortsManager.swift | 80 ++++--- .../PrivacyConfig/PrivacyConfiguration.swift | 8 + .../PrivacyConfigurationData.swift | 9 + .../PrivacyConfigurationManager.swift | 2 +- .../ContentBlocker/WebViewTestHelper.swift | 12 +- .../GPC/GPCTests.swift | 3 +- ...dPrivacyConfigurationExperimentTests.swift | 201 ++++++++++++++++- .../ExperimentCohortsManagerTests.swift | 213 ++++++------------ .../ExperimentsDataStoreTests.swift | 4 +- .../PrivacyConfigurationDataTests.swift | 1 + .../PrivacyConfigurationReferenceTests.swift | 3 +- .../Resources/privacy-config-example.json | 5 +- 13 files changed, 419 insertions(+), 248 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index c99e89a4e..30f33784a 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -36,7 +36,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { private let locale: Locale private let experimentManager: ExperimentCohortsManaging private let installDate: Date? - private let experimentManagerQueue = DispatchQueue(label: "com.experimentManager.queue") + static let experimentManagerQueue = DispatchQueue(label: "com.experimentManager.queue") public init(data: PrivacyConfigurationData, identifier: String, @@ -44,7 +44,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { internalUserDecider: InternalUserDecider, userDefaults: UserDefaults = UserDefaults(), locale: Locale = Locale.current, - experimentManager: ExperimentCohortsManaging = ExperimentCohortsManager(), + experimentManager: ExperimentCohortsManaging, installDate: Date? = nil) { self.data = data self.identifier = identifier @@ -144,13 +144,14 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } } - private func isRolloutEnabled(subfeature: any PrivacySubfeature, + private func isRolloutEnabled(subfeatureID: SubfeatureID, + parentID: ParentFeatureID, rolloutSteps: [PrivacyConfigurationData.PrivacyFeature.Feature.RolloutStep], randomizer: (Range) -> Double) -> Bool { // Empty rollouts should be default enabled guard !rolloutSteps.isEmpty else { return true } - let defsPrefix = "config.\(subfeature.parent.rawValue).\(subfeature.rawValue)" + let defsPrefix = "config.\(parentID).\(subfeatureID)" if userDefaults.bool(forKey: "\(defsPrefix).\(Constants.enabledKey)") { return true } @@ -199,41 +200,55 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } } -// public func stateFor(experiment: ExperimentData, -// parentID: String, -// cohortID: CohortID?, -// versionProvider: AppVersionProvider, -// randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { -// let parentFeature = PrivacyFeature(rawValue: parentID)! -// let parentState = stateFor(featureKey: parentFeature, versionProvider: versionProvider) -// guard case .enabled = parentState else { return parentState } -// let subfeatures = subfeatures(for: parentFeature) -// let subfeatureData = subfeatures[subfeatureID] -// subfeatureData. -// -// } -// public func getAllActiveExperiments() -> Experiments { -// -// } + public func getAllActiveExperiments(versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> Experiments { + Self.experimentManagerQueue.sync { + guard let assignedExperiments = experimentManager.experiments else { return [:] } + var experiments: Experiments = [:] + for (key, value) in assignedExperiments { + if stateFor(subfeatureID: key, experimentData: value, versionProvider: versionProvider, randomizer: randomizer) == .enabled { + experiments[key] = value + } + } + return experiments + } + } - public func stateFor(_ subfeature: any PrivacySubfeature, + private func stateFor(subfeatureID: SubfeatureID, experimentData: ExperimentData, versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + guard let parentFeature = PrivacyFeature(rawValue: experimentData.parentID) else { return .disabled(.featureMissing) } + let subfeatures = subfeatures(for: parentFeature) + guard let subfeatureData = subfeatures[subfeatureID] else { return .disabled(.featureMissing) } + return stateFor(parentFeature: parentFeature, subfeatureData: subfeatureData, subfeatureID: subfeatureID, cohortID: experimentData.cohort, versionProvider: versionProvider, randomizer: randomizer) + } + + public func stateFor(_ subfeature: any PrivacySubfeature, cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - return experimentManagerQueue.sync { + let subfeatures = subfeatures(for: subfeature.parent) + guard let subfeatureData = subfeatures[subfeature.rawValue] else { return .disabled(.featureMissing) } + + return stateFor(parentFeature: subfeature.parent, subfeatureData: subfeatureData, subfeatureID: subfeature.rawValue, cohortID: cohortID, versionProvider: versionProvider, randomizer: randomizer) + } + + private func stateFor(parentFeature: PrivacyFeature, + subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature, + subfeatureID: SubfeatureID, + cohortID: CohortID?, + versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { + Self.experimentManagerQueue.sync { // Step 1: Check parent feature state - let parentState = stateFor(featureKey: subfeature.parent, versionProvider: versionProvider) + let parentState = stateFor(featureKey: parentFeature, versionProvider: versionProvider) guard case .enabled = parentState else { return parentState } - // Step 2: Retrieve subfeature data and check version - let subfeatures = subfeatures(for: subfeature.parent) - let subfeatureData = subfeatures[subfeature.rawValue] - - let satisfiesMinVersion = satisfiesMinVersion(subfeatureData?.minSupportedVersion, versionProvider: versionProvider) + // Step 2: Check version + let satisfiesMinVersion = satisfiesMinVersion(subfeatureData.minSupportedVersion, versionProvider: versionProvider) // Step 3: Check sub-feature state - switch subfeatureData?.state { + switch subfeatureData.state { case PrivacyConfigurationData.State.enabled: guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } case PrivacyConfigurationData.State.internal: @@ -241,50 +256,33 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } default: return .disabled(.disabledInConfig) } - - // // Step 4: Handle Rollouts - if let rollout = subfeatureData?.rollout, - !isRolloutEnabled(subfeature: subfeature, rolloutSteps: rollout.steps, randomizer: randomizer) { + + // Step 4: Handle Rollouts + if let rollout = subfeatureData.rollout, + !isRolloutEnabled(subfeatureID: subfeatureID, parentID: parentFeature.rawValue, rolloutSteps: rollout.steps, randomizer: randomizer) { return .disabled(.stillInRollout) } - + // Step 5: Check if a cohort was passed in the func // If no corhort passed check for Target and Rollout guard let passedCohort = cohortID else { return checkTargets(subfeatureData) } - // Step 6: Verify there are cohorts in the subfeature data - // If not remove cohort (in case it was previously assigned) - // and check for Target and Rollout - guard let cohorts = subfeatureData?.cohorts else { - experimentManager.removeCohort(from: subfeature.rawValue) - return .disabled(.experimentCohortDoesNotMatch) - } - - // Step 7: Verify there the cohorts in the subfeature contain the cohort passed in the func - // If not remove cohort (in case it was previously assigned) before proceeding - if !cohorts.contains(where: { $0.name == passedCohort - }) { - experimentManager.removeCohort(from: subfeature.rawValue) - } - - // Step 8: Check if a cohort was already assigned - // If so check if it matches the one passed in the func and return .enable or disabled accordingly - if let assignedCohort = experimentManager.cohort(for: subfeature.rawValue) { - return (assignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) - } - - // Step 9: check Target and Rollout - // if disabled return .disabled otherwise continue + // Step 6: Verify there the cohort + // Check if cohort assigned and matches passed cohort + // If cohort not assigned + // Tries to assign if matching target + // Check if cohort assigned and matches passed cohort + let cohorts = subfeatureData.cohorts ?? [] let targetsState = checkTargets(subfeatureData) - if targetsState != .enabled { - return targetsState + let assignedCohortResponse = experimentManager.cohort(for: ExperimentSubfeature(parentID: parentFeature.rawValue, subfeatureID: subfeatureID, cohorts: cohorts), assignIfEnabled: targetsState == .enabled) + let possibleDisabledReason: PrivacyConfigurationFeatureDisabledReason = assignedCohortResponse.didAttemptAssignment && targetsState != .enabled ? .targetDoesNotMatch : .experimentCohortDoesNotMatch + if let assignedCohort = assignedCohortResponse.cohortID { + return (assignedCohort == passedCohort) ? .enabled : .disabled(possibleDisabledReason) + } else { + return .disabled(possibleDisabledReason) } - - // Step 10: Assign cohort and check if they match - let newAssignedCohort = experimentManager.assignCohort(to: ExperimentSubfeature(parentID: subfeature.parent.rawValue, subfeatureID: subfeature.rawValue, cohorts: cohorts)) - return (newAssignedCohort == passedCohort) ? .enabled : .disabled(.experimentCohortDoesNotMatch) } } @@ -293,11 +291,9 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { if let targets = subfeatureData?.targets, !matchTargets(targets: targets){ return .disabled(.targetDoesNotMatch) } - return .enabled } - private func matchTargets(targets: [PrivacyConfigurationData.PrivacyFeature.Feature.Target]) -> Bool { return targets.contains { target in (target.localeCountry == nil || target.localeCountry == locale.regionCode) && diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 78b404c53..2173160f8 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -37,48 +37,59 @@ public struct ExperimentData: Codable, Equatable { public typealias Experiments = [String: ExperimentData] public protocol ExperimentCohortsManaging { - /// Retrieves the cohort ID associated with the specified subfeature. - /// - 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 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 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 subfeatureID: The name of the experiment subfeature for which the cohort data should be removed. - func removeCohort(from subfeatureID: SubfeatureID) + /// Retrieves all the experiments a user is enrolled into + var experiments: Experiments? { get } + + /// Retrieves the assigned cohort for a given experiment subfeature, or attempts to assign a new cohort if none is currently assigned + /// and `assignIfEnabled` is set to true. If a cohort is already assigned but does not match any valid cohorts for the experiment, + /// the cohort will be removed. + /// + /// - Parameters: + /// - experiment: The `ExperimentSubfeature` for which to retrieve, assign, or remove a cohort. This subfeature includes + /// relevant identifiers and potential cohorts that may be assigned. + /// - assignIfEnabled: A Boolean value that determines whether a new cohort should be assigned if none is currently assigned. + /// If `true`, the function will attempt to assign a cohort from the available options; otherwise, it will only check for existing assignments. + /// + /// - Returns: A tuple containing: + /// - `cohortID`: The identifier of the assigned cohort if one exists, or `nil` if no cohort was assigned, if assignment failed, or if the cohort was removed. + /// - `didAttemptAssignment`: A Boolean indicating whether an assignment attempt was made. This will be `true` if `assignIfEnabled` + /// is `true` and no cohort was previously assigned, and `false` otherwise. + func cohort(for experiment: ExperimentSubfeature, assignIfEnabled: Bool) -> (cohortID: CohortID?, didAttemptAssignment: Bool) } -public final class ExperimentCohortsManager: ExperimentCohortsManaging { +public class ExperimentCohortsManager: ExperimentCohortsManaging { private var store: ExperimentsDataStoring private let randomizer: (Range) -> Double + private let queue = DispatchQueue(label: "com.ExperimentCohortsManager.queue") + public var experiments: Experiments? { + get { + queue.sync { + store.experiments + } + } + } - public init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double = Double.random(in:)) { + public init(store: ExperimentsDataStoring, randomizer: @escaping (Range) -> Double = Double.random(in:)) { self.store = store self.randomizer = randomizer } - public func cohort(for subfeatureID: SubfeatureID) -> CohortID? { - guard let experiments = store.experiments else { return nil } - return experiments[subfeatureID]?.cohort - } + public func cohort(for experiment: ExperimentSubfeature, assignIfEnabled: Bool) -> (cohortID: CohortID?, didAttemptAssignment: Bool) { + queue.sync { + let assignedCohort = cohort(for: experiment.subfeatureID) + if experiment.cohorts.contains(where: { $0.name == assignedCohort }) { + return (assignedCohort, false) + } else { + removeCohort(from: experiment.subfeatureID) + } - public func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { - guard let experiments = store.experiments else { return nil } - return experiments[subfeatureID]?.enrollmentDate + return assignIfEnabled ? (assignCohort(to: experiment), true) : (nil, true) + } } - public func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { + private func assignCohort(to subfeature: ExperimentSubfeature) -> CohortID? { let cohorts = subfeature.cohorts let totalWeight = cohorts.map(\.weight).reduce(0, +) guard totalWeight > 0 else { return nil } @@ -96,7 +107,17 @@ public final class ExperimentCohortsManager: ExperimentCohortsManaging { return nil } - public func removeCohort(from subfeatureID: SubfeatureID) { + func cohort(for subfeatureID: SubfeatureID) -> CohortID? { + guard let experiments = store.experiments else { return nil } + return experiments[subfeatureID]?.cohort + } + + private func enrollmentDate(for subfeatureID: SubfeatureID) -> Date? { + guard let experiments = store.experiments else { return nil } + return experiments[subfeatureID]?.enrollmentDate + } + + private func removeCohort(from subfeatureID: SubfeatureID) { guard var experiments = store.experiments else { return } experiments.removeValue(forKey: subfeatureID) store.experiments = experiments @@ -108,4 +129,5 @@ public final class ExperimentCohortsManager: ExperimentCohortsManaging { experiments[experimentID] = experimentData store.experiments = experiments } + } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift index 94304aa27..9131566e0 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfiguration.swift @@ -104,6 +104,10 @@ public protocol PrivacyConfiguration { func userEnabledProtection(forDomain: String) /// Adds given domain to locally unprotected list. func userDisabledProtection(forDomain: String) + + /// Gives the list of all the active experiments an user is enrolled in + func getAllActiveExperiments(versionProvider: AppVersionProvider, + randomizer: (Range) -> Double) -> Experiments } public extension PrivacyConfiguration { @@ -122,4 +126,8 @@ public extension PrivacyConfiguration { func stateFor(_ subfeature: any PrivacySubfeature, cohortID: CohortID? = nil, randomizer: (Range) -> Double = Double.random(in:)) -> PrivacyConfigurationFeatureState { return stateFor(subfeature, cohortID: cohortID, versionProvider: AppVersionProvider(), randomizer: randomizer) } + + func getAllActiveExperiments(versionProvider: AppVersionProvider = AppVersionProvider(), randomizer: (Range) -> Double = Double.random(in:)) -> Experiments { + return getAllActiveExperiments(versionProvider: versionProvider, randomizer: randomizer) + } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift index ec2e2ca20..f6f91567f 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationData.swift @@ -137,6 +137,7 @@ public struct PrivacyConfigurationData { case rollout case cohorts case targets + case settings } public struct Rollout: Hashable { @@ -190,6 +191,7 @@ public struct PrivacyConfigurationData { public let rollout: Rollout? public let cohorts: [Cohort]? public let targets: [Target]? + public let settings: String? public init?(json: [String: Any]) { guard let state = json[CodingKeys.state.rawValue] as? String else { @@ -217,6 +219,13 @@ public struct PrivacyConfigurationData { } else { targets = nil } + + if let settingsData = json[CodingKeys.settings.rawValue] { + let jsonData = try? JSONSerialization.data(withJSONObject: settingsData, options: []) + settings = jsonData != nil ? String(data: jsonData!, encoding: .utf8) : nil + } else { + settings = nil + } } } diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift index 7b216b04b..6fa567e3a 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift @@ -113,7 +113,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { errorReporting: EventMapping? = nil, internalUserDecider: InternalUserDecider, locale: Locale = Locale.current, - experimentCohortManager: ExperimentCohortsManaging = ExperimentCohortsManager(), + experimentCohortManager: ExperimentCohortsManaging = ExperimentCohortsManager(store: ExperimentsDataStore()), installDate: Date? = nil ) { self.embeddedDataProvider = embeddedDataProvider diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift index 31370ce4a..bced88b71 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift @@ -197,7 +197,7 @@ final class WebKitTestHelper { return AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider()) + internalUserDecider: DefaultInternalUserDecider(), experimentManager: MockExperimentCohortsManager()) } static func prepareContentBlockingRules(trackerData: TrackerData, @@ -225,3 +225,13 @@ final class WebKitTestHelper { } } } + +class MockExperimentCohortsManager: ExperimentCohortsManaging { + var experiments: BrowserServicesKit.Experiments? + + func cohort(for experiment: BrowserServicesKit.ExperimentSubfeature, assignIfEnabled: Bool) -> (cohortID: BrowserServicesKit.CohortID?, didAttemptAssignment: Bool) { + return (nil, true) + } + + +} diff --git a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift index 1eee4af76..8ee3ec2e2 100644 --- a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift +++ b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift @@ -42,7 +42,8 @@ final class GPCTests: XCTestCase { appConfig = AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider()) + internalUserDecider: DefaultInternalUserDecider(), + experimentManager: MockExperimentCohortsManager()) } func testWhenGPCEnableDomainIsHttpThenISGPCEnabledTrue() { diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift index ebd15b267..3ecc0eb9e 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift @@ -18,7 +18,7 @@ // import XCTest -import BrowserServicesKit +@testable import BrowserServicesKit final class AddPrivacyConfigurationExperimentTests: XCTestCase { @@ -200,6 +200,79 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) } + func testRemoveAssignedCohortsRemotelyRemovesAssignedCohortAndTriesToReassign() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "2", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // remove blue cohort + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "cohorts": [ + { + "name": "red", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "2", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "red")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "red") + } + func testDisablingFeatureDisablesCohort() { // Initially subfeature for both cohorts is disabled var config = manager.privacyConfig @@ -625,7 +698,7 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { } """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - var config = manager.privacyConfig + let config = manager.privacyConfig XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) XCTAssertTrue(mockStore.experiments?.isEmpty ?? true) @@ -742,6 +815,130 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { } + func testCohortEnabledAndStopEnrollmentAndRhenRollBack() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // Stop enrollment, should keep assigned cohorts + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 0 + }, + { + "name": "blue", + "weight": 1 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") + + // remove control, should re-allocate to blue + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "blue", + "weight": 1 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "blue")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "blue") + } + + func clearRolloutData(feature: String, subFeature: String) { UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).enabled") UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).lastRolloutCount") diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift index b4bdb88a1..0a5114d50 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentCohortsManagerTests.swift @@ -21,6 +21,11 @@ import XCTest final class ExperimentCohortsManagerTests: XCTestCase { + let cohort1 = PrivacyConfigurationData.Cohort(json: ["name": "Cohort1", "weight": 1])! + let cohort2 = PrivacyConfigurationData.Cohort(json: ["name": "Cohort2", "weight": 0])! + let cohort3 = PrivacyConfigurationData.Cohort(json: ["name": "Cohort3", "weight": 2])! + let cohort4 = PrivacyConfigurationData.Cohort(json: ["name": "Cohort4", "weight": 0])! + var mockStore: MockExperimentDataStore! var experimentCohortsManager: ExperimentCohortsManager! @@ -30,6 +35,12 @@ final class ExperimentCohortsManagerTests: XCTestCase { let subfeatureName2 = "TestSubfeature2" var experimentData2: ExperimentData! + let subfeatureName3 = "TestSubfeature3" + var experimentData3: ExperimentData! + + let subfeatureName4 = "TestSubfeature4" + var experimentData4: ExperimentData! + let encoder: JSONEncoder = { let encoder = JSONEncoder() encoder.dateEncodingStrategy = .secondsSince1970 @@ -40,15 +51,20 @@ final class ExperimentCohortsManagerTests: XCTestCase { super.setUp() mockStore = MockExperimentDataStore() experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - randomizer: { _ in 50.0 } + store: mockStore ) let expectedDate1 = Date() - experimentData1 = ExperimentData(parentID: "TestParent", cohort: "TestCohort1", enrollmentDate: expectedDate1) + experimentData1 = ExperimentData(parentID: "TestParent", cohort: cohort1.name, enrollmentDate: expectedDate1) let expectedDate2 = Date().addingTimeInterval(60) - experimentData2 = ExperimentData(parentID: "TestParent", cohort: "TestCohort2", enrollmentDate: expectedDate2) + experimentData2 = ExperimentData(parentID: "TestParent", cohort: cohort2.name, enrollmentDate: expectedDate2) + + let expectedDate3 = Date() + experimentData3 = ExperimentData(parentID: "TestParent", cohort: cohort3.name, enrollmentDate: expectedDate3) + + let expectedDate4 = Date().addingTimeInterval(60) + experimentData4 = ExperimentData(parentID: "TestParent", cohort: cohort4.name, enrollmentDate: expectedDate4) } override func tearDown() { @@ -59,206 +75,117 @@ final class ExperimentCohortsManagerTests: XCTestCase { super.tearDown() } - func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { + func testExperimentReturnAssignedExperiments() { // GIVEN mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - let result1 = experimentCohortsManager.cohort(for: subfeatureName1) - let result2 = experimentCohortsManager.cohort(for: subfeatureName2) + let experiments = experimentCohortsManager.experiments // THEN - XCTAssertEqual(result1, experimentData1.cohort) - XCTAssertEqual(result2, experimentData2.cohort) + XCTAssertEqual(experiments?.count, 2) + XCTAssertEqual(experiments?[subfeatureName1], experimentData1) + XCTAssertEqual(experiments?[subfeatureName2], experimentData2) + XCTAssertNil(experiments?[subfeatureName3]) } - func testEnrollmentDateReturnsCorrectDateIfExists() { + func testCohortReturnsCohortIDIfExistsForMultipleSubfeatures() { // GIVEN - mockStore.experiments = [subfeatureName1: experimentData1] + mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - let result1 = experimentCohortsManager.enrollmentDate(for: subfeatureName1) - let result2 = experimentCohortsManager.enrollmentDate(for: subfeatureName2) + let result1 = experimentCohortsManager.cohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort1, cohort2]), assignIfEnabled: false).cohortID + let result2 = experimentCohortsManager.cohort(for: ExperimentSubfeature(parentID: experimentData2.parentID, subfeatureID: subfeatureName2, cohorts: [cohort2, cohort3]), assignIfEnabled: false).cohortID // THEN - 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) + XCTAssertEqual(result1, experimentData1.cohort) + XCTAssertEqual(result2, experimentData2.cohort) } - func testCohortReturnsNilIfCohortDoesNotExist() { + func testCohortAssignIfEnabledWhenNoCohortExists() { // GIVEN - let subfeatureName = "TestSubfeature" + mockStore.experiments = [:] + let cohorts = [cohort1, cohort2] + let experiment = ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.cohort(for: subfeatureName) + let result = experimentCohortsManager.cohort(for: experiment, assignIfEnabled: true) // THEN - XCTAssertNil(result) + XCTAssertNotNil(result.cohortID) + XCTAssertTrue(result.didAttemptAssignment) + XCTAssertEqual(result.cohortID, experimentData1.cohort) } - func testEnrollmentDateReturnsNilIfDateDoesNotExist() { + func testCohortDoesNotAssignIfAssignIfEnabledIsFalse() { // GIVEN - let subfeatureName = "TestSubfeature" + mockStore.experiments = [:] + let cohorts = [cohort1, cohort2] + let experiment = ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.enrollmentDate(for: subfeatureName) + let result = experimentCohortsManager.cohort(for: experiment, assignIfEnabled: false) // THEN - XCTAssertNil(result) + XCTAssertNil(result.cohortID) + XCTAssertTrue(result.didAttemptAssignment) } - func testRemoveCohortSuccessfullyRemovesData() throws { + func testCohortDoesNotAssignIfAssignIfEnabledIsTrueButNoCohortsAvailable() { // GIVEN - mockStore.experiments = [subfeatureName1: experimentData1] + mockStore.experiments = [:] + let experiment = ExperimentSubfeature(parentID: "TestParent", subfeatureID: "NonExistentSubfeature", cohorts: []) // WHEN - experimentCohortsManager.removeCohort(from: subfeatureName1) + let result = experimentCohortsManager.cohort(for: experiment, assignIfEnabled: true) // THEN - let experiments = try XCTUnwrap(mockStore.experiments) - XCTAssertTrue(experiments.isEmpty) + XCTAssertNil(result.cohortID) + XCTAssertTrue(result.didAttemptAssignment) } - func testRemoveCohortDoesNothingIfSubfeatureDoesNotExist() { + func testCohortReassignsCohortIfAssignedCohortDoesNotExistAndAssignIfEnabledIsTrue() { // GIVEN - let expectedExperiments: Experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] - mockStore.experiments = expectedExperiments + mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - experimentCohortsManager.removeCohort(from: "someOtherSubfeature") + let result1 = experimentCohortsManager.cohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), assignIfEnabled: true).cohortID // THEN - XCTAssertEqual( mockStore.experiments, expectedExperiments) + XCTAssertEqual(result1, experimentData3.cohort) } - func testAssignCohortReturnsNilIfNoCohorts() { + func testCohortDoesNotReassignsCohortIfAssignedCohortDoesNotExistAndAssignIfEnabledIsTrue() { // GIVEN - let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: []) + mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result = experimentCohortsManager.assignCohort(to: subfeature) + let result1 = experimentCohortsManager.cohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), assignIfEnabled: false).cohortID // THEN - XCTAssertNil(result) + XCTAssertNil(result1) } - func testAssignCohortReturnsNilIfAllWeightsAreZero() { + func testCohortAssignsBasedOnWeight() { // 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(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) - - // WHEN - let result = experimentCohortsManager.assignCohort(to: 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(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) - let expectedTotalWeight = 4.0 - - // Use a custom randomizer to verify the range - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - 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( - store: mockStore, - randomizer: { _ in 0.0 } - ) - let resultStartOfCohort1 = experimentCohortsManager.assignCohort(to: subfeature) - XCTAssertEqual(resultStartOfCohort1, "Cohort1") - - // Test case where random value is at the end of Cohort1's range (0.9) - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - randomizer: { _ in 0.9 } - ) - 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) - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - randomizer: { _ in 1.00 } - ) - let resultStartOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) - XCTAssertEqual(resultStartOfCohort2, "Cohort2") + let experiment = ExperimentSubfeature(parentID: experimentData3.parentID, subfeatureID: subfeatureName3, cohorts: [cohort3, cohort4]) - // Test case where random value falls exactly within Cohort2's range (2.5) - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - randomizer: { _ in 2.5 } - ) - let resultMiddleOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) - XCTAssertEqual(resultMiddleOfCohort2, "Cohort2") + let randomizer: (Range) -> Double = { range in + return 1.5 + } - // Test case where random value is at the end of Cohort2's range (4) experimentCohortsManager = ExperimentCohortsManager( store: mockStore, - randomizer: { _ in 3.9 } - ) - let resultEndOfCohort2 = experimentCohortsManager.assignCohort(to: subfeature) - XCTAssertEqual(resultEndOfCohort2, "Cohort2") - } - - func testAssignCohortWithSingleCohortAlwaysSelectsThatCohort() throws { - // GIVEN - let jsonCohort1: [String: Any] = ["name": "Cohort1", "weight": 1] - let cohorts = [ - PrivacyConfigurationData.Cohort(json: jsonCohort1)! - ] - let subfeature = ExperimentSubfeature(parentID: "parent", subfeatureID: subfeatureName1, cohorts: cohorts) - let expectedTotalWeight = 1.0 - - // Use a custom randomizer to verify the range - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - 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 - } + randomizer: randomizer ) // WHEN - experimentCohortsManager = ExperimentCohortsManager( - store: mockStore, - randomizer: { range in Double.random(in: range)} - ) - let result = experimentCohortsManager.assignCohort(to: subfeature) + let result = experimentCohortsManager.cohort(for: experiment, assignIfEnabled: true) // THEN - XCTAssertEqual(result, "Cohort1") - XCTAssertEqual(cohorts[0].name, mockStore.experiments?[subfeature.subfeatureID]?.cohort) + XCTAssertEqual(result.cohortID, experimentData3.cohort) + XCTAssertTrue(result.didAttemptAssignment) } - } class MockExperimentDataStore: ExperimentsDataStoring { diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift index e5deb4f60..8816daec2 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/ExperimentsDataStoreTests.swift @@ -72,7 +72,7 @@ final class ExperimentsDataStoreTests: XCTestCase { func testExperimentsSetEncodesAndStoresData() throws { // GIVEN let experimentData1 = ExperimentData(parentID: "parent", cohort: "TestCohort1", enrollmentDate: Date()) - let experimentData2 = ExperimentData(parentID: "parent", cohort: "TestCohort2", enrollmentDate: Date()) + let experimentData2 = ExperimentData(parentID: "parent2", cohort: "TestCohort2", enrollmentDate: Date()) let experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN @@ -86,8 +86,10 @@ final class ExperimentsDataStoreTests: XCTestCase { 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) + XCTAssertEqual(decodedExperiments?[subfeatureName1]?.parentID, experimentData1.parentID) XCTAssertLessThanOrEqual(timeDifference1, 1.0) XCTAssertEqual(decodedExperiments?[subfeatureName2]?.cohort, experimentData2.cohort) + XCTAssertEqual(decodedExperiments?[subfeatureName2]?.parentID, experimentData2.parentID) XCTAssertLessThanOrEqual(timeDifference2, 1.0) } } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift index 720393246..08ecbec85 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationDataTests.swift @@ -70,6 +70,7 @@ class PrivacyConfigurationDataTests: XCTestCase { XCTAssertEqual(subfeatures["enabledSubfeature"]?.cohorts?[0].weight, 1) XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeCountry, "US") XCTAssertEqual(subfeatures["enabledSubfeature"]?.targets?[0].localeLanguage, "fr") + XCTAssertEqual(subfeatures["enabledSubfeature"]?.settings, "{\"foo\":\"foo\\/value\",\"bar\":\"bar\\/value\"}") XCTAssertEqual(subfeatures["internalSubfeature"]?.state, "internal") } else { XCTFail("Could not parse subfeatures") diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift index 2eed811a6..81c01d9bb 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift @@ -42,7 +42,8 @@ final class PrivacyConfigurationReferenceTests: XCTestCase { let privacyConfiguration = AppPrivacyConfiguration(data: privacyConfigurationData, identifier: UUID().uuidString, localProtection: MockDomainsProtectionStore(), - internalUserDecider: DefaultInternalUserDecider()) + internalUserDecider: DefaultInternalUserDecider(), + experimentManager: MockExperimentCohortsManager()) for test in testConfig.tests { if test.exceptPlatforms.contains(.macosBrowser) || test.exceptPlatforms.contains(.iosBrowser) { os_log("Skipping test %@", test.name) diff --git a/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json b/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json index 31046de83..735c3914f 100644 --- a/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json +++ b/Tests/BrowserServicesKitTests/Resources/privacy-config-example.json @@ -171,19 +171,16 @@ }, "enabledSubfeature": { "state": "enabled", -<<<<<<< HEAD "targets": [ { "localeCountry": "US", "localeLanguage": "fr" } ], - "config": { + "settings": { "foo": "foo/value", "bar": "bar/value" }, -======= ->>>>>>> main "description": "A description of the sub-feature", "cohorts": [ { From 86b7c97c55a963dfa9b3a13db6d8017a5e291756 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 15 Nov 2024 12:46:06 +0100 Subject: [PATCH 16/19] getAllActiveExperiments tests --- .../AppPrivacyConfiguration.swift | 147 +++++---- ...dPrivacyConfigurationExperimentTests.swift | 284 ++++++++++++++++++ 2 files changed, 371 insertions(+), 60 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 30f33784a..4fc56d76c 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -204,85 +204,112 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { public func getAllActiveExperiments(versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Experiments { Self.experimentManagerQueue.sync { - guard let assignedExperiments = experimentManager.experiments else { return [:] } - var experiments: Experiments = [:] - for (key, value) in assignedExperiments { - if stateFor(subfeatureID: key, experimentData: value, versionProvider: versionProvider, randomizer: randomizer) == .enabled { - experiments[key] = value - } - } - return experiments - } + guard let assignedExperiments = experimentManager.experiments else { return [:] } + return assignedExperiments.filter { key, value in + stateFor(subfeatureID: key, experimentData: value, versionProvider: versionProvider, randomizer: randomizer) == .enabled + } + } } private func stateFor(subfeatureID: SubfeatureID, experimentData: ExperimentData, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - guard let parentFeature = PrivacyFeature(rawValue: experimentData.parentID) else { return .disabled(.featureMissing) } - let subfeatures = subfeatures(for: parentFeature) - guard let subfeatureData = subfeatures[subfeatureID] else { return .disabled(.featureMissing) } - return stateFor(parentFeature: parentFeature, subfeatureData: subfeatureData, subfeatureID: subfeatureID, cohortID: experimentData.cohort, versionProvider: versionProvider, randomizer: randomizer) + guard let parentFeature = PrivacyFeature(rawValue: experimentData.parentID), + let subfeatureData = subfeatures(for: parentFeature)[subfeatureID] else { + return .disabled(.featureMissing) + } + return stateFor(parentFeature: parentFeature, + subfeatureData: subfeatureData, + subfeatureID: subfeatureID, + cohortID: experimentData.cohort, + assignCohortEnabled: false, + versionProvider: versionProvider, + randomizer: randomizer) } public func stateFor(_ subfeature: any PrivacySubfeature, cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - let subfeatures = subfeatures(for: subfeature.parent) - guard let subfeatureData = subfeatures[subfeature.rawValue] else { return .disabled(.featureMissing) } + Self.experimentManagerQueue.sync { + guard let subfeatureData = subfeatures(for: subfeature.parent)[subfeature.rawValue] else { + return .disabled(.featureMissing) + } - return stateFor(parentFeature: subfeature.parent, subfeatureData: subfeatureData, subfeatureID: subfeature.rawValue, cohortID: cohortID, versionProvider: versionProvider, randomizer: randomizer) + return stateFor(parentFeature: subfeature.parent, + subfeatureData: subfeatureData, + subfeatureID: subfeature.rawValue, + cohortID: cohortID, + versionProvider: versionProvider, + randomizer: randomizer) + } } private func stateFor(parentFeature: PrivacyFeature, subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature, subfeatureID: SubfeatureID, cohortID: CohortID?, + assignCohortEnabled: Bool = true, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> PrivacyConfigurationFeatureState { - Self.experimentManagerQueue.sync { - // Step 1: Check parent feature state - let parentState = stateFor(featureKey: parentFeature, versionProvider: versionProvider) - guard case .enabled = parentState else { return parentState } - - // Step 2: Check version - let satisfiesMinVersion = satisfiesMinVersion(subfeatureData.minSupportedVersion, versionProvider: versionProvider) - - // Step 3: Check sub-feature state - switch subfeatureData.state { - case PrivacyConfigurationData.State.enabled: - guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } - case PrivacyConfigurationData.State.internal: - guard internalUserDecider.isInternalUser else { return .disabled(.limitedToInternalUsers) } - guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } - default: return .disabled(.disabledInConfig) - } - - // Step 4: Handle Rollouts - if let rollout = subfeatureData.rollout, - !isRolloutEnabled(subfeatureID: subfeatureID, parentID: parentFeature.rawValue, rolloutSteps: rollout.steps, randomizer: randomizer) { - return .disabled(.stillInRollout) - } - - // Step 5: Check if a cohort was passed in the func - // If no corhort passed check for Target and Rollout - guard let passedCohort = cohortID else { - return checkTargets(subfeatureData) - } - - // Step 6: Verify there the cohort - // Check if cohort assigned and matches passed cohort - // If cohort not assigned - // Tries to assign if matching target - // Check if cohort assigned and matches passed cohort - let cohorts = subfeatureData.cohorts ?? [] - let targetsState = checkTargets(subfeatureData) - let assignedCohortResponse = experimentManager.cohort(for: ExperimentSubfeature(parentID: parentFeature.rawValue, subfeatureID: subfeatureID, cohorts: cohorts), assignIfEnabled: targetsState == .enabled) - let possibleDisabledReason: PrivacyConfigurationFeatureDisabledReason = assignedCohortResponse.didAttemptAssignment && targetsState != .enabled ? .targetDoesNotMatch : .experimentCohortDoesNotMatch - if let assignedCohort = assignedCohortResponse.cohortID { - return (assignedCohort == passedCohort) ? .enabled : .disabled(possibleDisabledReason) - } else { - return .disabled(possibleDisabledReason) - } + // Step 1: Check parent feature state + let parentState = stateFor(featureKey: parentFeature, versionProvider: versionProvider) + guard case .enabled = parentState else { return parentState } + + // Step 2: Check version + let satisfiesMinVersion = satisfiesMinVersion(subfeatureData.minSupportedVersion, versionProvider: versionProvider) + + // Step 3: Check sub-feature state + switch subfeatureData.state { + case PrivacyConfigurationData.State.enabled: + guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } + case PrivacyConfigurationData.State.internal: + guard internalUserDecider.isInternalUser else { return .disabled(.limitedToInternalUsers) } + guard satisfiesMinVersion else { return .disabled(.appVersionNotSupported) } + default: return .disabled(.disabledInConfig) + } + + // Step 4: Handle Rollouts + if let rollout = subfeatureData.rollout, + !isRolloutEnabled(subfeatureID: subfeatureID, parentID: parentFeature.rawValue, rolloutSteps: rollout.steps, randomizer: randomizer) { + return .disabled(.stillInRollout) + } + + // Step 5: Check if a cohort was passed in the func + // If no corhort passed check for Target and Rollout + guard let passedCohort = cohortID else { + return checkTargets(subfeatureData) + } + + // Step 5: Cohort handling + // Check if cohort assigned and matches passed cohort + // If cohort not assigned + // Tries to assign if matching target + // Check if cohort assigned and matches passed cohort + return checkCohortState(subfeatureData, + passedCohort: passedCohort, + assignCohortEnabled: assignCohortEnabled, + subfeatureID: subfeatureID, + parentFeature: parentFeature) + } + + // Check if cohort assigned and matches passed cohort + // If cohort not assigned + // Tries to assign if matching target + // Check if cohort assigned and matches passed cohort + private func checkCohortState(_ subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature, + passedCohort: CohortID?, + assignCohortEnabled: Bool, + subfeatureID: SubfeatureID, + parentFeature: PrivacyFeature) -> PrivacyConfigurationFeatureState { + let cohorts = subfeatureData.cohorts ?? [] + let targetsState = checkTargets(subfeatureData) + let assignIfEnabled = assignCohortEnabled && targetsState == .enabled + let assignedCohortResponse = experimentManager.cohort(for: ExperimentSubfeature(parentID: parentFeature.rawValue, subfeatureID: subfeatureID, cohorts: cohorts), assignIfEnabled: assignIfEnabled) + let possibleDisabledReason: PrivacyConfigurationFeatureDisabledReason = assignedCohortResponse.didAttemptAssignment && targetsState != .enabled ? .targetDoesNotMatch : .experimentCohortDoesNotMatch + if let assignedCohort = assignedCohortResponse.cohortID { + return (assignedCohort == passedCohort) ? .enabled : .disabled(possibleDisabledReason) + } else { + return .disabled(possibleDisabledReason) } } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift index 3ecc0eb9e..c2a8d63ed 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift @@ -943,4 +943,288 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).enabled") UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).lastRolloutCount") } + + func testAllActiveExperimentsEmptyIfNoAssignedExperiment() { + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + manager.reload(etag: "foo", data: featureJson) + let config = manager.privacyConfig + + let activeExperiments = config.getAllActiveExperiments() + XCTAssertTrue(activeExperiments.isEmpty) + XCTAssertNil(mockStore.experiments) + } + + func testAllActiveExperimentsReturnsOnlyActiveExperiments() { + var featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "blue", + "weight": 0 + } + ] + }, + "inlineIconCredentials": { + "state": "enabled", + "minSupportedVersion": 1, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 0 + }, + { + "name": "green", + "weight": 1 + } + ] + }, + "accessCredentialManagement": { + "state": "enabled", + "minSupportedVersion": 3, + "targets": [ + { + "localeCountry": "CA" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "green", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + + manager.reload(etag: "foo", data: featureJson) + var config = manager.privacyConfig + + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving, cohortID: "control")) + XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.inlineIconCredentials, cohortID: "green")) + XCTAssertFalse(config.isSubfeatureEnabled(AutofillSubfeature.accessCredentialManagement, cohortID: "control")) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + XCTAssertEqual(experimentManager.cohort(for: AutofillSubfeature.credentialsSaving.rawValue), "control") + XCTAssertEqual(experimentManager.cohort(for: AutofillSubfeature.inlineIconCredentials.rawValue), "green") + XCTAssertNil(experimentManager.cohort(for: AutofillSubfeature.accessCredentialManagement.rawValue)) + XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) + + var activeExperiments = config.getAllActiveExperiments() + XCTAssertEqual(activeExperiments.count, 2) + XCTAssertEqual(activeExperiments[AutofillSubfeature.credentialsSaving.rawValue]?.cohort, "control") + XCTAssertEqual(activeExperiments[AutofillSubfeature.inlineIconCredentials.rawValue]?.cohort, "green") + XCTAssertNil(activeExperiments[AutofillSubfeature.accessCredentialManagement.rawValue]) + + // When an assigned cohort is removed it's not part of active experiments + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "blue", + "weight": 1 + } + ] + }, + "inlineIconCredentials": { + "state": "enabled", + "minSupportedVersion": 1, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 0 + }, + { + "name": "green", + "weight": 1 + } + ] + }, + "accessCredentialManagement": { + "state": "enabled", + "minSupportedVersion": 3, + "targets": [ + { + "localeCountry": "CA" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "green", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + + manager.reload(etag: "foo", data: featureJson) + config = manager.privacyConfig + + activeExperiments = config.getAllActiveExperiments() + XCTAssertEqual(activeExperiments.count, 1) + XCTAssertNil(activeExperiments[AutofillSubfeature.credentialsSaving.rawValue]) + XCTAssertEqual(activeExperiments[AutofillSubfeature.inlineIconCredentials.rawValue]?.cohort, "green") + XCTAssertNil(activeExperiments[AutofillSubfeature.accessCredentialManagement.rawValue]) + + // When feature disabled an assigned cohort it's not part of active experiments + featureJson = + """ + { + "features": { + "autofill": { + "state": "enabled", + "exceptions": [], + "features": { + "credentialsSaving": { + "state": "enabled", + "minSupportedVersion": 2, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "blue", + "weight": 1 + } + ] + }, + "inlineIconCredentials": { + "state": "disabled", + "minSupportedVersion": 1, + "targets": [ + { + "localeCountry": "US" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 0 + }, + { + "name": "green", + "weight": 1 + } + ] + }, + "accessCredentialManagement": { + "state": "enabled", + "minSupportedVersion": 3, + "targets": [ + { + "localeCountry": "CA" + } + ], + "cohorts": [ + { + "name": "control", + "weight": 1 + }, + { + "name": "green", + "weight": 0 + } + ] + } + } + } + } + } + """.data(using: .utf8)! + + manager.reload(etag: "foo", data: featureJson) + config = manager.privacyConfig + + activeExperiments = config.getAllActiveExperiments() + XCTAssertTrue(activeExperiments.isEmpty) + } + } From 2595fcf64be99675ecbe7dc5bf51d7c15005d343 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 15 Nov 2024 12:52:53 +0100 Subject: [PATCH 17/19] sort linting issues --- .../PrivacyConfig/AppPrivacyConfiguration.swift | 1 - .../PrivacyConfig/PrivacyConfigurationManager.swift | 2 +- .../ContentBlocker/WebViewTestHelper.swift | 5 ++--- Tests/BrowserServicesKitTests/GPC/GPCTests.swift | 2 +- .../AddPrivacyConfigurationExperimentTests.swift | 6 +----- .../PrivacyConfig/PrivacyConfigurationReferenceTests.swift | 2 +- 6 files changed, 6 insertions(+), 12 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 4fc56d76c..960256120 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -191,7 +191,6 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { cohortID: CohortID?, versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Bool { - switch stateFor(subfeature, cohortID: cohortID, versionProvider: versionProvider, randomizer: randomizer) { case .enabled: return true diff --git a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift index 6fa567e3a..fff2f1482 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/PrivacyConfigurationManager.swift @@ -142,7 +142,7 @@ public class PrivacyConfigurationManager: PrivacyConfigurationManaging { identifier: embeddedConfigData.etag, localProtection: localProtection, internalUserDecider: internalUserDecider, - locale: locale, + locale: locale, experimentManager: experimentCohortManager, installDate: installDate) } diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift index bced88b71..f803d11e9 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift @@ -197,7 +197,8 @@ final class WebKitTestHelper { return AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider(), experimentManager: MockExperimentCohortsManager()) + internalUserDecider: DefaultInternalUserDecider(), + experimentManager: MockExperimentCohortsManager()) } static func prepareContentBlockingRules(trackerData: TrackerData, @@ -232,6 +233,4 @@ class MockExperimentCohortsManager: ExperimentCohortsManaging { func cohort(for experiment: BrowserServicesKit.ExperimentSubfeature, assignIfEnabled: Bool) -> (cohortID: BrowserServicesKit.CohortID?, didAttemptAssignment: Bool) { return (nil, true) } - - } diff --git a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift index 8ee3ec2e2..9ffca4ad1 100644 --- a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift +++ b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift @@ -42,7 +42,7 @@ final class GPCTests: XCTestCase { appConfig = AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider(), + internalUserDecider: DefaultInternalUserDecider(), experimentManager: MockExperimentCohortsManager()) } diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift index c2a8d63ed..8a4591fe7 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/AddPrivacyConfigurationExperimentTests.swift @@ -1,6 +1,5 @@ // // AddPrivacyConfigurationExperimentTests.swift -// DuckDuckGo // // Copyright © 2024 DuckDuckGo. All rights reserved. // @@ -31,7 +30,6 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { let subfeatureName = "credentialsSaving" - override func setUp() { locale = Locale(identifier: "fr_US") mockEmbeddedData = MockEmbeddedDataProvider(data: featureJson, etag: "test") @@ -43,7 +41,7 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { embeddedDataProvider: mockEmbeddedData, localProtection: MockDomainsProtectionStore(), internalUserDecider: DefaultInternalUserDecider(store: mockInternalUserStore), - locale: locale, + locale: locale, experimentCohortManager: experimentManager) } @@ -55,7 +53,6 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { manager = nil } - func testCohortOnlyAssignedWhenCallingStateForSubfeature() { featureJson = """ @@ -938,7 +935,6 @@ final class AddPrivacyConfigurationExperimentTests: XCTestCase { XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "blue") } - func clearRolloutData(feature: String, subFeature: String) { UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).enabled") UserDefaults().set(nil, forKey: "config.\(feature).\(subFeature).lastRolloutCount") diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift index 81c01d9bb..b4d0d1e45 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift @@ -42,7 +42,7 @@ final class PrivacyConfigurationReferenceTests: XCTestCase { let privacyConfiguration = AppPrivacyConfiguration(data: privacyConfigurationData, identifier: UUID().uuidString, localProtection: MockDomainsProtectionStore(), - internalUserDecider: DefaultInternalUserDecider(), + internalUserDecider: DefaultInternalUserDecider(), experimentManager: MockExperimentCohortsManager()) for test in testConfig.tests { if test.exceptPlatforms.contains(.macosBrowser) || test.exceptPlatforms.contains(.iosBrowser) { From 86d6f7bd6dc15a831346c071b0fcee7c7220de06 Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 15 Nov 2024 12:59:52 +0100 Subject: [PATCH 18/19] fix linting --- .../PrivacyConfig/AppPrivacyConfiguration.swift | 9 ++++----- .../ContentBlocker/WebViewTestHelper.swift | 2 +- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 960256120..4e3a09c63 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -199,7 +199,6 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { } } - public func getAllActiveExperiments(versionProvider: AppVersionProvider, randomizer: (Range) -> Double) -> Experiments { Self.experimentManagerQueue.sync { @@ -234,7 +233,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { return .disabled(.featureMissing) } - return stateFor(parentFeature: subfeature.parent, + return stateFor(parentFeature: subfeature.parent, subfeatureData: subfeatureData, subfeatureID: subfeature.rawValue, cohortID: cohortID, @@ -296,9 +295,9 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { // Tries to assign if matching target // Check if cohort assigned and matches passed cohort private func checkCohortState(_ subfeatureData: PrivacyConfigurationData.PrivacyFeature.Feature, - passedCohort: CohortID?, - assignCohortEnabled: Bool, - subfeatureID: SubfeatureID, + passedCohort: CohortID?, + assignCohortEnabled: Bool, + subfeatureID: SubfeatureID, parentFeature: PrivacyFeature) -> PrivacyConfigurationFeatureState { let cohorts = subfeatureData.cohorts ?? [] let targetsState = checkTargets(subfeatureData) diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift index f803d11e9..bf7451670 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift @@ -197,7 +197,7 @@ final class WebKitTestHelper { return AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider(), + internalUserDecider: DefaultInternalUserDecider(), experimentManager: MockExperimentCohortsManager()) } From 95c44700e0d558baad080e855ee204c2a97645fc Mon Sep 17 00:00:00 2001 From: Sabrina Tardio Date: Fri, 15 Nov 2024 13:07:18 +0100 Subject: [PATCH 19/19] don't break the API --- .../PrivacyConfig/AppPrivacyConfiguration.swift | 2 +- .../PrivacyConfig/ExperimentCohortsManager.swift | 2 +- .../ContentBlocker/WebViewTestHelper.swift | 3 +-- Tests/BrowserServicesKitTests/GPC/GPCTests.swift | 3 +-- .../PrivacyConfig/PrivacyConfigurationReferenceTests.swift | 3 +-- 5 files changed, 5 insertions(+), 8 deletions(-) diff --git a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift index 4e3a09c63..46e827a22 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/AppPrivacyConfiguration.swift @@ -44,7 +44,7 @@ public struct AppPrivacyConfiguration: PrivacyConfiguration { internalUserDecider: InternalUserDecider, userDefaults: UserDefaults = UserDefaults(), locale: Locale = Locale.current, - experimentManager: ExperimentCohortsManaging, + experimentManager: ExperimentCohortsManaging = ExperimentCohortsManager(), installDate: Date? = nil) { self.data = data self.identifier = identifier diff --git a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift index 2173160f8..e52e7b569 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/ExperimentCohortsManager.swift @@ -71,7 +71,7 @@ public class ExperimentCohortsManager: ExperimentCohortsManaging { } } - public init(store: ExperimentsDataStoring, randomizer: @escaping (Range) -> Double = Double.random(in:)) { + public init(store: ExperimentsDataStoring = ExperimentsDataStore(), randomizer: @escaping (Range) -> Double = Double.random(in:)) { self.store = store self.randomizer = randomizer } diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift index bf7451670..db78351d9 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift @@ -197,8 +197,7 @@ final class WebKitTestHelper { return AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider(), - experimentManager: MockExperimentCohortsManager()) + internalUserDecider: DefaultInternalUserDecider()) } static func prepareContentBlockingRules(trackerData: TrackerData, diff --git a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift index 9ffca4ad1..1eee4af76 100644 --- a/Tests/BrowserServicesKitTests/GPC/GPCTests.swift +++ b/Tests/BrowserServicesKitTests/GPC/GPCTests.swift @@ -42,8 +42,7 @@ final class GPCTests: XCTestCase { appConfig = AppPrivacyConfiguration(data: privacyData, identifier: "", localProtection: localProtection, - internalUserDecider: DefaultInternalUserDecider(), - experimentManager: MockExperimentCohortsManager()) + internalUserDecider: DefaultInternalUserDecider()) } func testWhenGPCEnableDomainIsHttpThenISGPCEnabledTrue() { diff --git a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift index b4d0d1e45..2eed811a6 100644 --- a/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift +++ b/Tests/BrowserServicesKitTests/PrivacyConfig/PrivacyConfigurationReferenceTests.swift @@ -42,8 +42,7 @@ final class PrivacyConfigurationReferenceTests: XCTestCase { let privacyConfiguration = AppPrivacyConfiguration(data: privacyConfigurationData, identifier: UUID().uuidString, localProtection: MockDomainsProtectionStore(), - internalUserDecider: DefaultInternalUserDecider(), - experimentManager: MockExperimentCohortsManager()) + internalUserDecider: DefaultInternalUserDecider()) for test in testConfig.tests { if test.exceptPlatforms.contains(.macosBrowser) || test.exceptPlatforms.contains(.iosBrowser) { os_log("Skipping test %@", test.name)