diff --git a/Sources/BrowserServicesKit/FeatureFlagger/ExperimentCohortsManager.swift b/Sources/BrowserServicesKit/FeatureFlagger/ExperimentCohortsManager.swift index 1f047adb9..e04e5feec 100644 --- a/Sources/BrowserServicesKit/FeatureFlagger/ExperimentCohortsManager.swift +++ b/Sources/BrowserServicesKit/FeatureFlagger/ExperimentCohortsManager.swift @@ -51,26 +51,26 @@ public protocol ExperimentCohortsManaging { /// for the specified experiment. If the assigned cohort is valid (i.e., it matches /// one of the experiment's defined cohorts), the method returns the assigned cohort. /// Otherwise, the invalid cohort is removed, and a new cohort is assigned if - /// `allowCohortReassignment` is `true`. + /// `allowCohortAssignment` is `true`. /// /// - Parameters: /// - experiment: The `ExperimentSubfeature` representing the experiment and its associated cohorts. - /// - allowCohortReassignment: A Boolean value indicating whether cohort assignment is allowed + /// - allowCohortAssignment: A Boolean value indicating whether cohort assignment is allowed /// if the user is not already assigned to a valid cohort. /// /// - Returns: The valid `CohortID` assigned to the user for the experiment, or `nil` - /// if no valid cohort exists and `allowCohortReassignment` is `false`. + /// if no valid cohort exists and `allowCohortAssignment` is `false`. /// /// - Behavior: /// 1. Retrieves the currently assigned cohort for the experiment using the `subfeatureID`. /// 2. Validates if the assigned cohort exists within the experiment's cohort list: /// - If valid, the assigned cohort is returned. /// - If invalid, the cohort is removed from storage. - /// 3. If cohort assignment is enabled (`allowCohortReassignment` is `true`), a new cohort + /// 3. If cohort assignment is enabled (`allowCohortAssignment` is `true`), a new cohort /// is assigned based on the experiment's cohort weights and saved in storage. /// - Cohort assignment is probabilistic, determined by the cohort weights. /// - func resolveCohort(for experiment: ExperimentSubfeature, allowCohortReassignment: Bool) -> CohortID? + func resolveCohort(for experiment: ExperimentSubfeature, allowCohortAssignment: Bool) -> CohortID? } public class ExperimentCohortsManager: ExperimentCohortsManaging { @@ -95,14 +95,14 @@ public class ExperimentCohortsManager: ExperimentCohortsManaging { self.fireCohortAssigned = fireCohortAssigned } - public func resolveCohort(for experiment: ExperimentSubfeature, allowCohortReassignment: Bool) -> CohortID? { + public func resolveCohort(for experiment: ExperimentSubfeature, allowCohortAssignment: Bool) -> CohortID? { queue.sync { let assignedCohort = cohort(for: experiment.subfeatureID) if experiment.cohorts.contains(where: { $0.name == assignedCohort }) { return assignedCohort } removeCohort(from: experiment.subfeatureID) - return allowCohortReassignment ? assignCohort(to: experiment) : nil + return allowCohortAssignment ? assignCohort(to: experiment) : nil } } } diff --git a/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagLocalOverrides.swift b/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagLocalOverrides.swift index 62b556aa5..993384d5b 100644 --- a/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagLocalOverrides.swift +++ b/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagLocalOverrides.swift @@ -28,11 +28,23 @@ public protocol FeatureFlagLocalOverridesPersisting { /// func value(for flag: Flag) -> Bool? + /// Return value for the cohort override for the experiment flag. + /// + /// If there's no override, this function should return `nil`. + /// + func value(for flag: Flag) -> CohortID? + /// Set new override for the feature flag. /// /// Flag can be overridden to `true` or `false`. Setting `nil` clears the override. /// func set(_ value: Bool?, for flag: Flag) + + /// Sets a new cohort override for the specified experiment feature flag. + /// + /// Flag can be overridden to one of the cohort of the feature. Setting `nil` clears the override. + /// + func setExperiment(_ value: CohortID?, for flag: Flag) } public struct FeatureFlagLocalOverridesUserDefaultsPersistor: FeatureFlagLocalOverridesPersisting { @@ -48,11 +60,21 @@ public struct FeatureFlagLocalOverridesUserDefaultsPersistor: FeatureFlagLocalOv return keyValueStore.object(forKey: key) as? Bool } + public func value(for flag: Flag) -> CohortID? { + let key = experimentKey(for: flag) + return keyValueStore.object(forKey: key) as? CohortID + } + public func set(_ value: Bool?, for flag: Flag) { let key = key(for: flag) keyValueStore.set(value, forKey: key) } + public func setExperiment(_ value: CohortID?, for flag: Flag) { + let key = experimentKey(for: flag) + keyValueStore.set(value, forKey: key) + } + /// This function returns the User Defaults key for a feature flag override. /// /// It uses camel case to simplify inter-process User Defaults KVO. @@ -60,6 +82,14 @@ public struct FeatureFlagLocalOverridesUserDefaultsPersistor: FeatureFlagLocalOv private func key(for flag: Flag) -> String { return "localOverride\(flag.rawValue.capitalizedFirstLetter)" } + + /// This function returns the User Defaults key for a feature flag cohort override. + /// + /// It uses camel case to simplify inter-process User Defaults KVO. + /// + private func experimentKey(for flag: Flag) -> String { + return "localOverride\(flag.rawValue.capitalizedFirstLetter)_cohort" + } } private extension String { @@ -77,6 +107,13 @@ public protocol FeatureFlagLocalOverridesHandling { /// It can be implemented by client apps to react to changes to feature flag /// value in runtime, caused by adjusting its local override. func flagDidChange(_ featureFlag: Flag, isEnabled: Bool) + + /// This function is called whenever the effective cohort of a feature flag changes due to a local override. + /// changes as a result of adding or removing a local override. + /// + /// It can be implemented by client apps to react to changes to feature flag + /// value in runtime, caused by adjusting its local override. + func experimentFlagDidChange(_ featureFlag: Flag, cohort: CohortID) } /// `FeatureFlagLocalOverridesHandling` implementation providing Combine publisher for flag changes. @@ -87,15 +124,24 @@ public struct FeatureFlagOverridesPublishingHandler: F public let flagDidChangePublisher: AnyPublisher<(F, Bool), Never> private let flagDidChangeSubject = PassthroughSubject<(F, Bool), Never>() + public let experimentFlagDidChangePublisher: AnyPublisher<(F, CohortID), Never> + private let experimentFlagDidChangeSubject = PassthroughSubject<(F, CohortID), Never>() public init() { flagDidChangePublisher = flagDidChangeSubject.eraseToAnyPublisher() + experimentFlagDidChangePublisher = experimentFlagDidChangeSubject.eraseToAnyPublisher() } public func flagDidChange(_ featureFlag: Flag, isEnabled: Bool) { guard let flag = featureFlag as? F else { return } flagDidChangeSubject.send((flag, isEnabled)) } + + public func experimentFlagDidChange(_ featureFlag: Flag, cohort: CohortID) { + guard let flag = featureFlag as? F else { return } + experimentFlagDidChangeSubject.send((flag, cohort)) + } + } /// This protocol defines the interface for feature flag overriding mechanism. @@ -117,12 +163,21 @@ public protocol FeatureFlagLocalOverriding: AnyObject { /// Returns the current override for a feature flag, or `nil` if override is not set. func override(for featureFlag: Flag) -> Bool? + /// Returns the current cohort override for a feature flag, or `nil` if override is not set. + func experimentOverride(for featureFlag: Flag) -> CohortID? + /// Toggles override for a feature flag. /// /// If override is not currently present, it sets the override to the opposite of the current flag value. /// func toggleOverride(for featureFlag: Flag) + /// Sets the cohort override for a feature flag. + /// + /// If override is not currently present, it sets the override to the opposite of the current flag value. + /// + func setExperimentCohortOverride(for featureFlag: Flag, cohort: CohortID) + /// Clears override for a feature flag. /// /// Calls `FeatureFlagLocalOverridesHandling.flagDidChange` if the effective flag value @@ -135,11 +190,17 @@ public protocol FeatureFlagLocalOverriding: AnyObject { /// This function calls `clearOverride(for:)` for each flag. /// func clearAllOverrides(for flagType: Flag.Type) + + /// Returns the effective value of a feature flag, considering the current state and overrides. + func currentValue(for featureFlag: Flag) -> Bool? + + /// Returns the effective cohort of a feature flag, considering the current state and overrides. + func currentExperimentCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? } public final class FeatureFlagLocalOverrides: FeatureFlagLocalOverriding { - public let actionHandler: FeatureFlagLocalOverridesHandling + public var actionHandler: FeatureFlagLocalOverridesHandling public weak var featureFlagger: FeatureFlagger? private let persistor: FeatureFlagLocalOverridesPersisting @@ -168,6 +229,13 @@ public final class FeatureFlagLocalOverrides: FeatureFlagLocalOverriding { return persistor.value(for: featureFlag) } + public func experimentOverride(for featureFlag: Flag) -> CohortID? { + guard featureFlag.supportsLocalOverriding else { + return nil + } + return persistor.value(for: featureFlag) + } + public func toggleOverride(for featureFlag: Flag) { guard featureFlag.supportsLocalOverriding else { return @@ -178,13 +246,30 @@ public final class FeatureFlagLocalOverrides: FeatureFlagLocalOverriding { actionHandler.flagDidChange(featureFlag, isEnabled: newValue) } - public func clearOverride(for featureFlag: Flag) { - guard let override = override(for: featureFlag) else { + public func setExperimentCohortOverride(for featureFlag: Flag, cohort: CohortID) { + guard featureFlag.supportsLocalOverriding else { return } - persistor.set(nil, for: featureFlag) - if let defaultValue = currentValue(for: featureFlag), defaultValue != override { - actionHandler.flagDidChange(featureFlag, isEnabled: defaultValue) + let newValue = cohort + persistor.setExperiment(newValue, for: featureFlag) + actionHandler.experimentFlagDidChange(featureFlag, cohort: cohort) + } + + public func clearOverride(for featureFlag: Flag) { + // Clear the regular override + if let override = override(for: featureFlag) { + persistor.set(nil, for: featureFlag) + if let defaultValue = currentValue(for: featureFlag), defaultValue != override { + actionHandler.flagDidChange(featureFlag, isEnabled: defaultValue) + } + } + + // Clear the experiment cohort override + if let experimentOverride = experimentOverride(for: featureFlag) { + persistor.setExperiment(nil, for: featureFlag) + if let defaultValue = currentExperimentCohort(for: featureFlag)?.rawValue, defaultValue != experimentOverride { + actionHandler.experimentFlagDidChange(featureFlag, cohort: defaultValue) + } } } @@ -194,7 +279,12 @@ public final class FeatureFlagLocalOverrides: FeatureFlagLocalOverriding { } } - private func currentValue(for featureFlag: Flag) -> Bool? { + public func currentValue(for featureFlag: Flag) -> Bool? { featureFlagger?.isFeatureOn(for: featureFlag, allowOverride: true) } + + public func currentExperimentCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? { + guard let flagger = featureFlagger as? CurrentExperimentCohortProviding else { return nil } + return flagger.assignedCohort(for: featureFlag) + } } diff --git a/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagger.swift b/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagger.swift index 3ecb53d1e..03f378da0 100644 --- a/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagger.swift +++ b/Sources/BrowserServicesKit/FeatureFlagger/FeatureFlagger.swift @@ -18,8 +18,6 @@ import Foundation -public protocol FlagCohort: RawRepresentable, CaseIterable where RawValue == CohortID {} - /// This protocol defines a common interface for feature flags managed by FeatureFlagger. /// /// It should be implemented by the feature flag type in client apps. @@ -64,65 +62,86 @@ public protocol FeatureFlagDescribing: CaseIterable { /// } /// ``` var source: FeatureFlagSource { get } -} -/// This protocol defines a common interface for experiment feature flags managed by FeatureFlagger. -/// -/// It should be implemented by the feature flag type in client apps. -/// -public protocol FeatureFlagExperimentDescribing { - - /// Returns a string representation of the flag - var rawValue: String { get } - - /// Defines the source of the experiment feature flag, which corresponds to - /// where the final flag value should come from. + /// Defines the type of cohort associated with the feature flag, if any. /// - /// Example client implementation: + /// This property allows feature flags to define and associate with specific cohorts, + /// which are groups of users categorized for experimentation or feature rollouts. + /// + /// - Returns: A type conforming to `FeatureFlagCohortDescribing`, or `nil` if no cohort is associated + /// with the feature flag. + /// + /// ### Example: + /// For a feature flag with cohorts like "control" and "treatment": /// /// ``` - /// public enum FeatureFlag: FeatureFlagDescribing { - /// case sync - /// case autofill - /// case cookieConsent - /// case duckPlayer + /// public enum ExampleFeatureFlag: FeatureFlagDescribing { + /// case experimentalFeature /// - /// var source: FeatureFlagSource { - /// case .sync: - /// return .disabled - /// case .cookieConsent: - /// return .internalOnly(cohort) - /// case .credentialsAutofill: - /// return .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill)) - /// case .duckPlayer: - /// return .remoteReleasable(.feature(.duckPlayer)) - /// } + /// var cohortType: (any FeatureFlagCohortDescribing.Type)? { + /// return ExampleCohort.self + /// } + /// + /// public enum ExampleCohort: String, FeatureFlagCohortDescribing { + /// case control + /// case treatment + /// } /// } /// ``` - var source: FeatureFlagSource { get } + /// + /// If `cohortType` is `nil`, the feature flag does not have associated cohorts. + var cohortType: (any FeatureFlagCohortDescribing.Type)? { get } +} + +/// A protocol that defines a set of cohorts for feature flags. +/// +/// Cohorts represent groups of users categorized for A/B testing. Each cohort has a unique identifier (`CohortID`). +/// +/// Types conforming to `FeatureFlagCohortDescribing` must be an `enum`, conform to +/// `CaseIterable`, and use a `RawValue` of type `CohortID` (typically a `String`). +/// +/// ## Usage +/// +/// To define cohorts for a feature flag, create an `enum` conforming to `FeatureFlagCohortDescribing`: +/// +/// ```swift +/// public enum ExampleCohort: String, FeatureFlagCohortDescribing { +/// case control +/// case treatment +/// } +/// ``` +/// +/// These cohorts can then be associated with feature flags to segment users into different +/// groups for experimentation: +/// +/// ```swift +/// public enum ExampleFeatureFlag: FeatureFlagDescribing { +/// case newUI +/// +/// var cohortType: (any FeatureFlagCohortDescribing.Type)? { +/// return ExampleCohort.self +/// } +/// } +/// ``` +/// +/// ## Provided Utility Methods +/// +/// - `cohort(for rawValue: CohortID) -> Self?`: Retrieves the cohort instance from its raw value. +/// - `cohorts: [Self]`: Returns an array of all defined cohorts. +public protocol FeatureFlagCohortDescribing: CaseIterable, RawRepresentable where RawValue == CohortID {} - /// Represents the possible groups or variants within an experiment. - /// - /// The `Cohort` type is used to define user groups or test variations for feature - /// experimentation. Each cohort typically corresponds to a specific behavior or configuration - /// applied to a subset of users. For example, in an A/B test, you might define cohorts such as - /// `control` and `treatment`. - /// - /// Each cohort must conform to the `CohortEnum` protocol, which ensures that the cohort type - /// is an `enum` with `String` raw values and provides access to all possible cases - /// through `CaseIterable`. - /// - /// Example: - /// ``` - /// public enum AutofillCohorts: String, CohortEnum { - /// case control - /// case treatment - /// } - /// ``` - /// - /// The `Cohort` type allows dynamic resolution of cohorts by their raw `String` value, - /// making it easy to map user configurations to specific cohort groups. - associatedtype CohortType: FlagCohort +/// A protocol for retrieving the current experiment cohort for feature flags if one has already been assigned. +protocol CurrentExperimentCohortProviding { + func assignedCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? +} + +public extension FeatureFlagCohortDescribing { + static func cohort(for rawValue: CohortID) -> Self? { + return Self.allCases.first { $0.rawValue == rawValue } + } + static var cohorts: [Self] { + return Array(Self.allCases) + } } public enum FeatureFlagSource { @@ -130,7 +149,7 @@ public enum FeatureFlagSource { case disabled /// Enabled for internal users only. Cannot be toggled remotely - case internalOnly((any FlagCohort)? = nil) + case internalOnly((any FeatureFlagCohortDescribing)? = nil) /// Toggled remotely using PrivacyConfiguration but only for internal users. Otherwise, disabled. case remoteDevelopment(PrivacyConfigFeatureLevel) @@ -184,10 +203,49 @@ public protocol FeatureFlagger: AnyObject { /// - For `.disabled`: Returns `nil`. /// - For `.internalOnly`: Returns the cohort if the user is an internal user. /// - For `.remoteDevelopment` and `.remoteReleasable`: - /// - If the feature is a subfeature, resolves its cohort using `getCohortIfEnabled(_ subfeature:)`. + /// - If the feature is a subfeature, resolves its cohort using `resolveCohort(_ subfeature:)`. /// - Returns `nil` if the user is not eligible. /// - func getCohortIfEnabled(for featureFlag: Flag) -> (any FlagCohort)? + /// > Note: Setting `allowOverride` to `false` skips checking local overrides. This can be used + /// when the non-overridden feature flag value is required. + + /// Retrieves or attempts to assign a cohort for a feature flag if the feature is enabled. + /// + /// This method checks whether the feature flag is active based on its source configuration. + /// If the flag is enabled and supports cohorts, it returns the assigned cohort if one exists. + /// Otherwise, it attempts to resolve and assign the appropriate cohort from the available options. + /// + /// If local overrides are enabled (`allowOverride = true`) and the user is internal, the overridden + /// cohort is returned before any other logic is applied. + /// + /// ## Behavior: + /// - **For `.disabled` flags**: Returns `nil`. + /// - **For `.internalOnly` flags**: Returns the predefined cohort if the user is an internal user. + /// - **For `.remoteDevelopment` and `.remoteReleasable` flags**: + /// - If the feature is a subfeature, resolves its cohort using `resolveCohort(_ subfeature:)`. + /// - If no cohort is assigned yet, attempts to assign one from the available cohorts. + /// + /// > **Note**: If `allowOverride` is `false`, local overrides are ignored. + /// + /// ## Example: + /// ```swift + /// if let cohort = featureFlagger.resolveCohort(for: .newUI) as? ExampleCohort { + /// switch cohort { + /// case .treatment: + /// print("treatment") + /// case .control: + /// print("control") + /// } + /// } + /// ``` + /// + /// In this example, `ExampleCohort` is the cohort type associated with the `.newUI` feature flag. + /// The switch statement handles the assigned cohort values accordingly. + /// + /// - Parameter featureFlag: The feature flag for which to retrieve or assign a cohort. + /// - Parameter allowOverride: Whether local overrides should be considered. + /// - Returns: The assigned `FeatureFlagCohortDescribing` instance if the feature is enabled, or `nil` otherwise. + func resolveCohort(for featureFlag: Flag, allowOverride: Bool) -> (any FeatureFlagCohortDescribing)? /// Retrieves all active experiments currently assigned to the user. /// @@ -205,7 +263,7 @@ public protocol FeatureFlagger: AnyObject { /// - Validates its assigned cohort using `resolveCohort` in the `ExperimentManager`. /// 3. If the experiment passes validation, it is added to the result dictionary. /// - func getAllActiveExperiments() -> Experiments + var allActiveExperiments: Experiments { get } } public extension FeatureFlagger { @@ -219,6 +277,16 @@ public extension FeatureFlagger { func isFeatureOn(for featureFlag: Flag) -> Bool { isFeatureOn(for: featureFlag, allowOverride: true) } + + /// Called from app features to determine the cohort for a given feature, if enabled. + /// + /// Feature Flag's `source` is checked to determine if the flag is enabled. If the feature + /// flagger provides an overrides mechanism (`localOverrides` is not `nil`) and the user + /// is internal, local overrides are checked first and, if present, returned as the cohort. + /// + func resolveCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? { + resolveCohort(for: featureFlag, allowOverride: true) + } } public class DefaultFeatureFlagger: FeatureFlagger { @@ -277,7 +345,7 @@ public class DefaultFeatureFlagger: FeatureFlagger { } } - public func getAllActiveExperiments() -> Experiments { + public var allActiveExperiments: Experiments { guard let enrolledExperiments = experimentManager?.experiments else { return [:] } var activeExperiments = [String: ExperimentData]() let config = privacyConfigManager.privacyConfig @@ -288,14 +356,25 @@ public class DefaultFeatureFlagger: FeatureFlagger { let cohorts = config.cohorts(subfeatureID: subfeatureID, parentFeatureID: experimentData.parentID) ?? [] let experimentSubfeature = ExperimentSubfeature(parentID: experimentData.parentID, subfeatureID: subfeatureID, cohorts: cohorts) - if experimentManager?.resolveCohort(for: experimentSubfeature, allowCohortReassignment: false) == experimentData.cohortID { + if experimentManager?.resolveCohort(for: experimentSubfeature, allowCohortAssignment: false) == experimentData.cohortID { activeExperiments[subfeatureID] = experimentData } } return activeExperiments } - public func getCohortIfEnabled(for featureFlag: Flag) -> (any FlagCohort)? { + public func resolveCohort(for featureFlag: Flag, allowOverride: Bool) -> (any FeatureFlagCohortDescribing)? { + // Check for local overrides + if allowOverride, internalUserDecider.isInternalUser, let localOverride = localOverrides?.experimentOverride(for: featureFlag) { + return featureFlag.cohortType?.cohorts.first { $0.rawValue == localOverride } + } + + // Handle feature cohort sources + return handleCohortResolutionBasedOnSources(for: featureFlag, allowCohortAssignment: true) + } + + private func handleCohortResolutionBasedOnSources(for featureFlag: Flag, allowCohortAssignment: Bool) -> (any FeatureFlagCohortDescribing)? { + // Handle feature cohort sources switch featureFlag.source { case .disabled: return nil @@ -304,8 +383,8 @@ public class DefaultFeatureFlagger: FeatureFlagger { case .remoteReleasable(let featureType), .remoteDevelopment(let featureType) where internalUserDecider.isInternalUser: if case .subfeature(let subfeature) = featureType { - if let resolvedCohortID = getCohortIfEnabled(subfeature) { - return Flag.CohortType.allCases.first { return $0.rawValue == resolvedCohortID } + if let resolvedCohortID = resolveCohort(subfeature, allowCohortAssignment: allowCohortAssignment) { + return featureFlag.cohortType?.cohort(for: resolvedCohortID) } } return nil @@ -314,16 +393,16 @@ public class DefaultFeatureFlagger: FeatureFlagger { } } - private func getCohortIfEnabled(_ subfeature: any PrivacySubfeature) -> CohortID? { + private func resolveCohort(_ subfeature: any PrivacySubfeature, allowCohortAssignment: Bool = true) -> CohortID? { let config = privacyConfigManager.privacyConfig let featureState = config.stateFor(subfeature) let cohorts = config.cohorts(for: subfeature) let experiment = ExperimentSubfeature(parentID: subfeature.parent.rawValue, subfeatureID: subfeature.rawValue, cohorts: cohorts ?? []) switch featureState { case .enabled: - return experimentManager?.resolveCohort(for: experiment, allowCohortReassignment: true) + return experimentManager?.resolveCohort(for: experiment, allowCohortAssignment: allowCohortAssignment) case .disabled(.targetDoesNotMatch): - return experimentManager?.resolveCohort(for: experiment, allowCohortReassignment: false) + return experimentManager?.resolveCohort(for: experiment, allowCohortAssignment: false) default: return nil } @@ -338,3 +417,9 @@ public class DefaultFeatureFlagger: FeatureFlagger { } } } + +extension DefaultFeatureFlagger: CurrentExperimentCohortProviding { + func assignedCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? { + return handleCohortResolutionBasedOnSources(for: featureFlag, allowCohortAssignment: false) + } +} diff --git a/Sources/Configuration/TrackerDataURLOverrider.swift b/Sources/Configuration/TrackerDataURLOverrider.swift index 8e44436be..24ce19e31 100644 --- a/Sources/Configuration/TrackerDataURLOverrider.swift +++ b/Sources/Configuration/TrackerDataURLOverrider.swift @@ -41,7 +41,7 @@ public final class TrackerDataURLOverrider: TrackerDataURLProviding { public var trackerDataURL: URL? { for experimentType in TDSExperimentType.allCases { - if let cohort = featureFlagger.getCohortIfEnabled(for: experimentType.experiment) as? TDSNextExperimentFlag.Cohort, + if let cohort = featureFlagger.resolveCohort(for: experimentType, allowOverride: false) as? TDSExperimentType.Cohort, let url = trackerDataURL(for: experimentType.subfeature, cohort: cohort) { return url } @@ -49,7 +49,7 @@ public final class TrackerDataURLOverrider: TrackerDataURLProviding { return nil } - private func trackerDataURL(for subfeature: any PrivacySubfeature, cohort: TDSNextExperimentFlag.Cohort) -> URL? { + private func trackerDataURL(for subfeature: any PrivacySubfeature, cohort: TDSExperimentType.Cohort) -> URL? { guard let settings = privacyConfigurationManager.privacyConfig.settings(for: subfeature), let jsonData = settings.data(using: .utf8) else { return nil } do { @@ -64,7 +64,7 @@ public final class TrackerDataURLOverrider: TrackerDataURLProviding { } } -public enum TDSExperimentType: Int, CaseIterable { +public enum TDSExperimentType: String, CaseIterable { case baseline case feb25 case mar25 @@ -78,10 +78,6 @@ public enum TDSExperimentType: Int, CaseIterable { case nov25 case dec25 - public var experiment: any FeatureFlagExperimentDescribing { - TDSNextExperimentFlag(subfeature: self.subfeature) - } - public var subfeature: any PrivacySubfeature { switch self { case .baseline: @@ -110,21 +106,22 @@ public enum TDSExperimentType: Int, CaseIterable { ContentBlockingSubfeature.tdsNextExperimentDec25 } } - } -public struct TDSNextExperimentFlag: FeatureFlagExperimentDescribing { - public var rawValue: String - public var source: FeatureFlagSource +extension TDSExperimentType: FeatureFlagDescribing { + public var supportsLocalOverriding: Bool { + return false + } - public init(subfeature: any PrivacySubfeature) { - self.source = .remoteReleasable(.subfeature(subfeature)) - self.rawValue = subfeature.rawValue + public var source: FeatureFlagSource { + return .remoteReleasable(.subfeature(self.subfeature)) } - public typealias CohortType = Cohort + public var cohortType: (any FeatureFlagCohortDescribing.Type)? { + return Cohort.self + } - public enum Cohort: String, FlagCohort { + public enum Cohort: String, FeatureFlagCohortDescribing { case control case treatment } diff --git a/Sources/PixelExperimentKit/PixelExperimentKit.swift b/Sources/PixelExperimentKit/PixelExperimentKit.swift index ecfb35482..5dfe3cafb 100644 --- a/Sources/PixelExperimentKit/PixelExperimentKit.swift +++ b/Sources/PixelExperimentKit/PixelExperimentKit.swift @@ -93,7 +93,7 @@ extension PixelKit { assertionFailure("PixelKit is not configured for experiments") return } - guard let experimentData = featureFlagger.getAllActiveExperiments()[subfeatureID] else { return } + guard let experimentData = featureFlagger.allActiveExperiments[subfeatureID] else { return } // Check if within conversion window guard isUserInConversionWindow(conversionWindowDays, enrollmentDate: experimentData.enrollmentDate) else { return } @@ -123,7 +123,7 @@ extension PixelKit { assertionFailure("PixelKit is not configured for experiments") return } - guard let experimentData = featureFlagger.getAllActiveExperiments()[subfeatureID] else { return } + guard let experimentData = featureFlagger.allActiveExperiments[subfeatureID] else { return } fireExperimentPixelForActiveExperiment(subfeatureID, experimentData: experimentData, @@ -151,7 +151,7 @@ extension PixelKit { assertionFailure("PixelKit is not configured for experiments") return } - featureFlagger.getAllActiveExperiments().forEach { experiment in + featureFlagger.allActiveExperiments.forEach { experiment in fireExperimentPixels(for: experiment.key, experimentData: experiment.value, @@ -180,7 +180,7 @@ extension PixelKit { assertionFailure("PixelKit is not configured for experiments") return } - featureFlagger.getAllActiveExperiments().forEach { experiment in + featureFlagger.allActiveExperiments.forEach { experiment in fireExperimentPixels( for: experiment.key, experimentData: experiment.value, diff --git a/Sources/PixelExperimentKit/TDSOverrideExperimentMetrics.swift b/Sources/PixelExperimentKit/TDSOverrideExperimentMetrics.swift index 626975778..24e80b71e 100644 --- a/Sources/PixelExperimentKit/TDSOverrideExperimentMetrics.swift +++ b/Sources/PixelExperimentKit/TDSOverrideExperimentMetrics.swift @@ -49,7 +49,7 @@ public struct TDSOverrideExperimentMetrics { guard let featureFlagger = PixelKit.ExperimentConfig.featureFlagger else { return nil } return TDSExperimentType.allCases.compactMap { experimentType in - guard let experimentData = featureFlagger.getAllActiveExperiments()[experimentType.subfeature.rawValue] else { return nil } + guard let experimentData = featureFlagger.allActiveExperiments[experimentType.subfeature.rawValue] else { return nil } return "\(experimentType.subfeature.rawValue)_\(experimentData.cohortID)" }.first } @@ -77,7 +77,7 @@ public struct TDSOverrideExperimentMetrics { fire: @escaping FireDebugExperiment) { guard let featureFlagger = PixelKit.ExperimentConfig.featureFlagger, - let experimentData = featureFlagger.getAllActiveExperiments()[experimentType.subfeature.rawValue] + let experimentData = featureFlagger.allActiveExperiments[experimentType.subfeature.rawValue] else { return } let experimentName: String = experimentType.subfeature.rawValue + experimentData.cohortID diff --git a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift index bc979233e..14cec14d7 100644 --- a/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift +++ b/Tests/BrowserServicesKitTests/ContentBlocker/WebViewTestHelper.swift @@ -227,7 +227,7 @@ final class WebKitTestHelper { } class MockExperimentCohortsManager: ExperimentCohortsManaging { - func resolveCohort(for experiment: ExperimentSubfeature, allowCohortReassignment: Bool) -> CohortID? { + func resolveCohort(for experiment: ExperimentSubfeature, allowCohortAssignment: Bool) -> CohortID? { return nil } diff --git a/Tests/BrowserServicesKitTests/FeatureFlagging/DefaultFeatureFlaggerTests.swift b/Tests/BrowserServicesKitTests/FeatureFlagging/DefaultFeatureFlaggerTests.swift index dcc6d7978..333f9393c 100644 --- a/Tests/BrowserServicesKitTests/FeatureFlagging/DefaultFeatureFlaggerTests.swift +++ b/Tests/BrowserServicesKitTests/FeatureFlagging/DefaultFeatureFlaggerTests.swift @@ -22,11 +22,12 @@ import XCTest final class CapturingFeatureFlagOverriding: FeatureFlagLocalOverriding { var overrideCalls: [any FeatureFlagDescribing] = [] - var toggleOverideCalls: [any FeatureFlagDescribing] = [] + var toggleOverrideCalls: [any FeatureFlagDescribing] = [] var clearOverrideCalls: [any FeatureFlagDescribing] = [] var clearAllOverrideCallCount: Int = 0 var override: (any FeatureFlagDescribing) -> Bool? = { _ in nil } + var experimentOverride: (any FeatureFlagDescribing) -> CohortID? = { _ in nil } var actionHandler: any FeatureFlagLocalOverridesHandling = CapturingFeatureFlagLocalOverridesHandler() weak var featureFlagger: FeatureFlagger? @@ -37,7 +38,7 @@ final class CapturingFeatureFlagOverriding: FeatureFlagLocalOverriding { } func toggleOverride(for featureFlag: Flag) { - toggleOverideCalls.append(featureFlag) + toggleOverrideCalls.append(featureFlag) } func clearOverride(for featureFlag: Flag) { @@ -47,6 +48,23 @@ final class CapturingFeatureFlagOverriding: FeatureFlagLocalOverriding { func clearAllOverrides(for flagType: Flag.Type) { clearAllOverrideCallCount += 1 } + + func experimentOverride(for featureFlag: Flag) -> CohortID? where Flag: FeatureFlagDescribing { + overrideCalls.append(featureFlag) + return experimentOverride(featureFlag) + } + + func setExperimentCohortOverride(for featureFlag: Flag, cohort: CohortID) where Flag: FeatureFlagDescribing { + return + } + + func currentValue(for featureFlag: Flag) -> Bool? where Flag: FeatureFlagDescribing { + return nil + } + + func currentExperimentCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? where Flag: FeatureFlagDescribing { + return nil + } } final class DefaultFeatureFlaggerTests: XCTestCase { @@ -145,123 +163,123 @@ final class DefaultFeatureFlaggerTests: XCTestCase { // MARK: - Experiments - func testWhenGetCohortIfEnabled_andSourceDisabled_returnsNil() { + func testWhenResolveCohort_andSourceDisabled_returnsNil() { let featureFlagger = createFeatureFlagger() - let flag = FakeExperimentFlag(source: .disabled) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let flag = FakeExperimentFlags.disabledFlag + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func estWhenGetCohortIfEnabled_andSourceInternal_returnsPassedCohort() { + func testWhenResolveCohort_andSourceInternal_returnsPassedCohort() { let featureFlagger = createFeatureFlagger() - let flag = FakeExperimentFlag(source: .internalOnly(AutofillCohort.blue)) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) - XCTAssertEqual(cohort?.rawValue, AutofillCohort.blue.rawValue) + let flag = FakeExperimentFlags.internalFlag + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) + XCTAssertEqual(cohort?.rawValue, FakeExperimentFlagsCohort.blue.rawValue) } - func testWhenGetCohortIfEnabled_andRemoteInternal_andInternalStateTrue_and_cohortAssigned_returnsAssignedCohort() { + func testWhenResolveCohort_andRemoteInternal_andInternalStateTrue_and_cohortAssigned_returnsAssignedCohort() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill - experimentManager.cohortToReturn = AutofillCohort.control.rawValue + experimentManager.cohortToReturn = FakeExperimentFlagsCohort.control.rawValue let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteDeveloperFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) - XCTAssertEqual(cohort?.rawValue, AutofillCohort.control.rawValue) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) + XCTAssertEqual(cohort?.rawValue, FakeExperimentFlagsCohort.control.rawValue) } - func testWhenGetCohortIfEnabled_andRemoteInternal_andInternalStateFalse_and_cohortAssigned_returnsNil() { + func testWhenResolveCohort_andRemoteInternal_andInternalStateFalse_and_cohortAssigned_returnsNil() { internalUserDeciderStore.isInternalUser = false let subfeature = AutofillSubfeature.credentialsAutofill - experimentManager.cohortToReturn = AutofillCohort.control.rawValue + experimentManager.cohortToReturn = FakeExperimentFlagsCohort.control.rawValue let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteDeveloperFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteInternal_andInternalStateTrue_and_cohortAssigned_andFeaturePassed_returnsNil() { + func testWhenResolveCohort_andRemoteInternal_andInternalStateTrue_and_cohortAssigned_andFeaturePassed_returnsNil() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill - experimentManager.cohortToReturn = AutofillCohort.control.rawValue + experimentManager.cohortToReturn = FakeExperimentFlagsCohort.control.rawValue let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteDevelopment(.feature(.autofill))) + let flag = FakeExperimentFlags.remoteDevelopmentFeature let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteInternal_andInternalStateTrue_and_cohortNotAssigned_returnsNil() { + func testWhenResolveCohort_andRemoteInternal_andInternalStateTrue_and_cohortNotAssigned_returnsNil() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill experimentManager.cohortToReturn = nil let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteDeveloperFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteInternal_andInternalStateTrue_and_cohortAssignedButNorMatchingEnum_returnsNil() { + func testWhenResolveCohort_andRemoteInternal_andInternalStateTrue_and_cohortAssignedButNorMatchingEnum_returnsNil() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill experimentManager.cohortToReturn = "some" let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteDeveloperFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteReleasable_and_cohortAssigned_returnsAssignedCohort() { + func testWhenResolveCohort_andRemoteReleasable_and_cohortAssigned_returnsAssignedCohort() { let subfeature = AutofillSubfeature.credentialsAutofill - experimentManager.cohortToReturn = AutofillCohort.control.rawValue + experimentManager.cohortToReturn = FakeExperimentFlagsCohort.control.rawValue let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteReleasable(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteReleasableFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) - XCTAssertEqual(cohort?.rawValue, AutofillCohort.control.rawValue) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) + XCTAssertEqual(cohort?.rawValue, FakeExperimentFlagsCohort.control.rawValue) } - func testWhenGetCohortIfEnabled_andRemoteReleasable_and_cohortAssigned_andFeaturePassed_returnsNil() { + func testWhenResolveCohort_andRemoteReleasable_and_cohortAssigned_andFeaturePassed_returnsNil() { let subfeature = AutofillSubfeature.credentialsAutofill - experimentManager.cohortToReturn = AutofillCohort.control.rawValue + experimentManager.cohortToReturn = FakeExperimentFlagsCohort.control.rawValue let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteReleasable(.feature(.autofill))) + let flag = FakeExperimentFlags.remoteReleasableFeature let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteReleasable_and_cohortNotAssigned_andFeaturePassed_returnsNil() { + func testWhenResolveCohort_andRemoteReleasable_and_cohortNotAssigned_andFeaturePassed_returnsNil() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill experimentManager.cohortToReturn = nil let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteReleasable(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteReleasableFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } - func testWhenGetCohortIfEnabled_andRemoteReleasable_and_cohortAssignedButNotMatchingEnum_returnsNil() { + func testWhenResolveCohort_andRemoteReleasable_and_cohortAssignedButNotMatchingEnum_returnsNil() { internalUserDeciderStore.isInternalUser = true let subfeature = AutofillSubfeature.credentialsAutofill experimentManager.cohortToReturn = "some" let embeddedData = Self.embeddedConfig(autofillSubfeatureForState: (subfeature: subfeature, state: "enabled")) - let flag = FakeExperimentFlag(source: .remoteReleasable(.subfeature(AutofillSubfeature.credentialsAutofill))) + let flag = FakeExperimentFlags.remoteReleasableFlag let featureFlagger = createFeatureFlagger(withMockedConfigData: embeddedData) - let cohort = featureFlagger.getCohortIfEnabled(for: flag) + let cohort = featureFlagger.resolveCohort(for: flag, allowOverride: true) XCTAssertNil(cohort) } @@ -284,6 +302,18 @@ final class DefaultFeatureFlaggerTests: XCTestCase { XCTAssertEqual(try XCTUnwrap(overrides.overrideCalls.first as? TestFeatureFlag), .overridableFlagDisabledByDefault) } + func testWhenLocalExperimentOverridesIsSetUpAndUserIsInternalThenLocalOverrideTakesPrecedenceWhenCheckingFlagValue() throws { + let featureFlagger = createFeatureFlaggerWithLocalOverrides() + internalUserDeciderStore.isInternalUser = true + + overrides.experimentOverride = { _ in return TestFeatureFlag.FakeExperimentCohort.cohortA.rawValue } + let actualCohort = featureFlagger.resolveCohort(for: TestFeatureFlag.overridableExperimentFlagWithCohortBByDefault, allowOverride: true) + + XCTAssertEqual(actualCohort?.rawValue, TestFeatureFlag.FakeExperimentCohort.cohortA.rawValue) + XCTAssertEqual(overrides.overrideCalls.count, 1) + XCTAssertEqual(try XCTUnwrap(overrides.overrideCalls.first as? TestFeatureFlag), .overridableExperimentFlagWithCohortBByDefault) + } + func testWhenLocalOverridesIsSetUpAndUserIsInternalAndAllowOverrideIsFalseThenLocalOverrideIsNotCheckedWhenCheckingFlagValue() throws { let featureFlagger = createFeatureFlaggerWithLocalOverrides() internalUserDeciderStore.isInternalUser = true @@ -292,6 +322,16 @@ final class DefaultFeatureFlaggerTests: XCTestCase { XCTAssertTrue(overrides.overrideCalls.isEmpty) } + func testWhenLocalExperimentOverridesIsSetUpAndUserIsInternalAndAllowOverrideIsFalseThenLocalOverrideIsNotCheckedWhenCheckingFlagValue() throws { + let featureFlagger = createFeatureFlaggerWithLocalOverrides() + internalUserDeciderStore.isInternalUser = true + overrides.experimentOverride = { _ in return TestFeatureFlag.FakeExperimentCohort.cohortA.rawValue } + let actualCohort = featureFlagger.resolveCohort(for: TestFeatureFlag.overridableExperimentFlagWithCohortBByDefault, allowOverride: false) + + XCTAssertEqual(actualCohort?.rawValue, TestFeatureFlag.FakeExperimentCohort.cohortB.rawValue) + XCTAssertTrue(overrides.overrideCalls.isEmpty) + } + func testWhenLocalOverridesIsSetUpAndUserIsNotInternalThenLocalOverrideIsNotCheckedWhenCheckingFlagValue() throws { let featureFlagger = createFeatureFlaggerWithLocalOverrides() internalUserDeciderStore.isInternalUser = false @@ -300,6 +340,15 @@ final class DefaultFeatureFlaggerTests: XCTestCase { XCTAssertTrue(overrides.overrideCalls.isEmpty) } + func testWhenLocalExperimentOverridesIsSetUpAndUserIsNotInternalThenLocalOverrideIsNotCheckedWhenCheckingFlagValue() throws { + let featureFlagger = createFeatureFlaggerWithLocalOverrides() + internalUserDeciderStore.isInternalUser = false + overrides.experimentOverride = { _ in return TestFeatureFlag.FakeExperimentCohort.cohortA.rawValue } + + XCTAssertFalse(featureFlagger.isFeatureOn(for: TestFeatureFlag.overridableExperimentFlagWithCohortBByDefault)) + XCTAssertTrue(overrides.overrideCalls.isEmpty) + } + // MARK: - Helpers private func createFeatureFlagger(withMockedConfigData data: Data = DefaultFeatureFlaggerTests.embeddedConfig()) -> DefaultFeatureFlagger { @@ -363,6 +412,7 @@ final class DefaultFeatureFlaggerTests: XCTestCase { } extension FeatureFlagSource: FeatureFlagDescribing { + public var cohortType: (any FeatureFlagCohortDescribing.Type)? { nil } public static let allCases: [FeatureFlagSource] = [] public var supportsLocalOverriding: Bool { false } public var rawValue: String { "rawValue" } @@ -373,20 +423,43 @@ class MockExperimentManager: ExperimentCohortsManaging { var cohortToReturn: CohortID? var experiments: BrowserServicesKit.Experiments? - func resolveCohort(for experiment: BrowserServicesKit.ExperimentSubfeature, allowCohortReassignment: Bool) -> CohortID? { + func resolveCohort(for experiment: BrowserServicesKit.ExperimentSubfeature, allowCohortAssignment: Bool) -> CohortID? { return cohortToReturn } } +private enum FakeExperimentFlags: String, CaseIterable { + case disabledFlag + case internalFlag + case remoteDeveloperFlag + case remoteDevelopmentFeature + case remoteReleasableFlag + case remoteReleasableFeature +} -private struct FakeExperimentFlag: FeatureFlagExperimentDescribing { - typealias CohortType = AutofillCohort - - var rawValue: String = "fake-experiment" - - var source: FeatureFlagSource +extension FakeExperimentFlags: FeatureFlagDescribing { + var supportsLocalOverriding: Bool { true } + + var cohortType: (any FeatureFlagCohortDescribing.Type)? { FakeExperimentFlagsCohort.self} + + var source: FeatureFlagSource { + switch self { + case .disabledFlag: + .disabled + case .internalFlag: + .internalOnly(FakeExperimentFlagsCohort.blue) + case .remoteDeveloperFlag: + .remoteDevelopment(.subfeature(AutofillSubfeature.credentialsAutofill)) + case .remoteDevelopmentFeature: + .remoteDevelopment(.feature(.autofill)) + case .remoteReleasableFlag: + .remoteReleasable(.subfeature(AutofillSubfeature.credentialsAutofill)) + case .remoteReleasableFeature: + .remoteReleasable(.feature(.autofill)) + } + } } -private enum AutofillCohort: String, FlagCohort { +private enum FakeExperimentFlagsCohort: String, FeatureFlagCohortDescribing { case control case blue } diff --git a/Tests/BrowserServicesKitTests/FeatureFlagging/ExperimentCohortsManagerTests.swift b/Tests/BrowserServicesKitTests/FeatureFlagging/ExperimentCohortsManagerTests.swift index 7a2be7abc..7b5894d57 100644 --- a/Tests/BrowserServicesKitTests/FeatureFlagging/ExperimentCohortsManagerTests.swift +++ b/Tests/BrowserServicesKitTests/FeatureFlagging/ExperimentCohortsManagerTests.swift @@ -103,8 +103,8 @@ final class ExperimentCohortsManagerTests: XCTestCase { mockStore.experiments = [subfeatureName1: experimentData1, subfeatureName2: experimentData2] // WHEN - let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort1, cohort2]), allowCohortReassignment: false) - let result2 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData2.parentID, subfeatureID: subfeatureName2, cohorts: [cohort2, cohort3]), allowCohortReassignment: false) + let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort1, cohort2]), allowCohortAssignment: false) + let result2 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData2.parentID, subfeatureID: subfeatureName2, cohorts: [cohort2, cohort3]), allowCohortAssignment: false) // THEN XCTAssertEqual(result1, experimentData1.cohortID) @@ -120,7 +120,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let experiment = ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortReassignment: true) + let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortAssignment: true) // THEN XCTAssertNotNil(result) @@ -138,7 +138,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let experiment = ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: cohorts) // WHEN - let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortReassignment: false) + let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortAssignment: false) // THEN XCTAssertNil(result) @@ -152,7 +152,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { let experiment = ExperimentSubfeature(parentID: "TestParent", subfeatureID: "NonExistentSubfeature", cohorts: []) // WHEN - let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortReassignment: true) + let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortAssignment: true) // THEN XCTAssertNil(result) @@ -165,7 +165,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), allowCohortReassignment: true) + let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), allowCohortAssignment: true) // THEN XCTAssertEqual(result1, experimentData3.cohortID) @@ -180,7 +180,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { mockStore.experiments = [subfeatureName1: experimentData1] // WHEN - let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), allowCohortReassignment: false) + let result1 = experimentCohortsManager.resolveCohort(for: ExperimentSubfeature(parentID: experimentData1.parentID, subfeatureID: subfeatureName1, cohorts: [cohort2, cohort3]), allowCohortAssignment: false) // THEN XCTAssertNil(result1) @@ -202,7 +202,7 @@ final class ExperimentCohortsManagerTests: XCTestCase { ) // WHEN - let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortReassignment: true) + let result = experimentCohortsManager.resolveCohort(for: experiment, allowCohortAssignment: true) // THEN XCTAssertEqual(result, experimentData3.cohortID) diff --git a/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlagLocalOverridesTests.swift b/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlagLocalOverridesTests.swift index 977ac5766..1cda47bb5 100644 --- a/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlagLocalOverridesTests.swift +++ b/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlagLocalOverridesTests.swift @@ -21,14 +21,20 @@ import PersistenceTestingUtils import XCTest final class CapturingFeatureFlagLocalOverridesHandler: FeatureFlagLocalOverridesHandling { + struct Parameters: Equatable { let rawValue: String - let isEnabled: Bool + let isEnabled: Bool? + let cohort: CohortID? } var calls: [Parameters] = [] func flagDidChange(_ featureFlag: Flag, isEnabled: Bool) { - calls.append(.init(rawValue: featureFlag.rawValue, isEnabled: isEnabled)) + calls.append(.init(rawValue: featureFlag.rawValue, isEnabled: isEnabled, cohort: nil)) + } + + func experimentFlagDidChange(_ featureFlag: Flag, cohort: CohortID) where Flag: FeatureFlagDescribing { + calls.append(.init(rawValue: featureFlag.rawValue, isEnabled: nil, cohort: cohort)) } } @@ -98,12 +104,12 @@ final class FeatureFlagLocalOverridesTests: XCTestCase { XCTAssertEqual( actionHandler.calls, [ - .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: true), - .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: false), - .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false), - .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true), - .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: false), - .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true) + .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: true, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: false, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: false, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true, cohort: nil) ] ) } @@ -117,8 +123,8 @@ final class FeatureFlagLocalOverridesTests: XCTestCase { XCTAssertEqual( actionHandler.calls, [ - .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: true), - .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false) + .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: true, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false, cohort: nil) ] ) } @@ -159,8 +165,8 @@ final class FeatureFlagLocalOverridesTests: XCTestCase { XCTAssertEqual( actionHandler.calls, [ - .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false), - .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true) + .init(rawValue: TestFeatureFlag.overridableFlagDisabledByDefault.rawValue, isEnabled: false, cohort: nil), + .init(rawValue: TestFeatureFlag.overridableFlagEnabledByDefault.rawValue, isEnabled: true, cohort: nil) ] ) } diff --git a/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlaggerExperimentsTests.swift b/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlaggerExperimentsTests.swift index 933eb11b3..538307103 100644 --- a/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlaggerExperimentsTests.swift +++ b/Tests/BrowserServicesKitTests/FeatureFlagging/FeatureFlaggerExperimentsTests.swift @@ -19,49 +19,55 @@ import XCTest @testable import BrowserServicesKit -struct CredentialsSavingFlag: FeatureFlagExperimentDescribing { - - typealias CohortType = Cohort +enum TestExperimentFlags: String, CaseIterable { + case credentialsSaving + case inlineIconCredentials + case accessCredentialManagement +} - var rawValue = "credentialSaving" +extension TestExperimentFlags: FeatureFlagDescribing { + var supportsLocalOverriding: Bool { true } + + var source: FeatureFlagSource { + switch self { + case .credentialsSaving: + .remoteReleasable(.subfeature(AutofillSubfeature.credentialsSaving)) + case .inlineIconCredentials: + .remoteReleasable(.subfeature(AutofillSubfeature.inlineIconCredentials)) + case .accessCredentialManagement: + .remoteReleasable(.subfeature(AutofillSubfeature.accessCredentialManagement)) + } + } - var source: FeatureFlagSource = .remoteReleasable(.subfeature(AutofillSubfeature.credentialsSaving)) + var cohortType: (any FeatureFlagCohortDescribing.Type)? { + switch self { + case .credentialsSaving: + CredentialsSavingCohort.self + case .inlineIconCredentials: + InlineIconCredentialsCohort.self + case .accessCredentialManagement: + AccessCredentialManagementCohort.self + } + } - enum Cohort: String, FlagCohort { + enum CredentialsSavingCohort: String, FeatureFlagCohortDescribing { case control case blue case red } -} - -struct InlineIconCredentialsFlag: FeatureFlagExperimentDescribing { - - typealias CohortType = Cohort - var rawValue = "inlineIconCredentials" - - var source: FeatureFlagSource = .remoteReleasable(.subfeature(AutofillSubfeature.inlineIconCredentials)) - - enum Cohort: String, FlagCohort { + enum InlineIconCredentialsCohort: String, FeatureFlagCohortDescribing { case control case blue case green } -} - -struct AccessCredentialManagementFlag: FeatureFlagExperimentDescribing { - typealias CohortType = Cohort - - var rawValue = "accessCredentialManagement" - - var source: FeatureFlagSource = .remoteReleasable(.subfeature(AutofillSubfeature.accessCredentialManagement)) - - enum Cohort: String, FlagCohort { + enum AccessCredentialManagementCohort: String, FeatureFlagCohortDescribing { case control case blue case green } + } final class FeatureFlaggerExperimentsTests: XCTestCase { @@ -130,7 +136,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "", data: featureJson) let config = manager.privacyConfig - // we haven't called getCohortIfEnabled yet, so cohorts should not be yet assigned + // we haven't called resolveCohort yet, so cohorts should not be yet assigned XCTAssertNil(mockStore.experiments) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) @@ -140,8 +146,8 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { XCTAssertNil(mockStore.experiments) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) - // we call getCohortIfEnabled(cohort), then we should assign cohort - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + // we call resolveCohort(cohort), then we should assign cohort + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") } @@ -177,7 +183,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "", data: featureJson) var config = manager.privacyConfig - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -208,7 +214,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "", data: featureJson) config = manager.privacyConfig - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -233,7 +239,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "", data: featureJson) config = manager.privacyConfig - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertTrue(mockStore.experiments?.isEmpty ?? false) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) @@ -269,7 +275,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "2", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -303,14 +309,14 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "2", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.red.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.red.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "red") } func testDisablingFeatureDisablesCohort() { // Initially subfeature for both cohorts is disabled - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertNil(mockStore.experiments) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) @@ -344,7 +350,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -378,7 +384,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -412,7 +418,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") } @@ -454,23 +460,23 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { } manager.reload(etag: "", data: featureJson(country: "FR", language: "fr")) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertNil(mockStore.experiments) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) manager.reload(etag: "", data: featureJson(country: "US", language: "en")) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertNil(mockStore.experiments) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) manager.reload(etag: "", data: featureJson(country: "US", language: "fr")) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) 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")) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -498,13 +504,13 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson2) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) 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")) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)) XCTAssertTrue(mockStore.experiments?.isEmpty ?? false) XCTAssertNil(experimentManager.cohort(for: subfeatureName)) } @@ -544,7 +550,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -583,7 +589,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -622,7 +628,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -665,7 +671,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -711,7 +717,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { XCTAssertTrue(mockStore.experiments?.isEmpty ?? true) XCTAssertNil(experimentManager.cohort(for: subfeatureName), "control") - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) let currentTime = Date().timeIntervalSince1970 let enrollmentTime = mockStore.experiments?[subfeatureName]?.enrollmentDate.timeIntervalSince1970 @@ -767,7 +773,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { clearRolloutData(feature: "autofill", subFeature: "credentialsSaving") XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -814,7 +820,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { config = manager.privacyConfig XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -857,7 +863,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { var config = manager.privacyConfig XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -898,7 +904,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { config = manager.privacyConfig XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "control") @@ -935,7 +941,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { config = manager.privacyConfig XCTAssertTrue(config.isSubfeatureEnabled(AutofillSubfeature.credentialsSaving)) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.CohortType.blue.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.blue.rawValue) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: subfeatureName), "blue") } @@ -980,7 +986,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { """.data(using: .utf8)! manager.reload(etag: "foo", data: featureJson) - let activeExperiments = featureFlagger.getAllActiveExperiments() + let activeExperiments = featureFlagger.allActiveExperiments XCTAssertTrue(activeExperiments.isEmpty) XCTAssertNil(mockStore.experiments) } @@ -1059,9 +1065,9 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "foo", data: featureJson) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: CredentialsSavingFlag())?.rawValue, CredentialsSavingFlag.Cohort.control.rawValue) - XCTAssertEqual(featureFlagger.getCohortIfEnabled(for: InlineIconCredentialsFlag())?.rawValue, InlineIconCredentialsFlag.Cohort.green.rawValue) - XCTAssertNil(featureFlagger.getCohortIfEnabled(for: AccessCredentialManagementFlag())) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.credentialsSaving, allowOverride: true)?.rawValue, TestExperimentFlags.CredentialsSavingCohort.control.rawValue) + XCTAssertEqual(featureFlagger.resolveCohort(for: TestExperimentFlags.inlineIconCredentials, allowOverride: true)?.rawValue, TestExperimentFlags.InlineIconCredentialsCohort.green.rawValue) + XCTAssertNil(featureFlagger.resolveCohort(for: TestExperimentFlags.accessCredentialManagement, allowOverride: true)) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) XCTAssertEqual(experimentManager.cohort(for: AutofillSubfeature.credentialsSaving.rawValue), "control") @@ -1069,7 +1075,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { XCTAssertNil(experimentManager.cohort(for: AutofillSubfeature.accessCredentialManagement.rawValue)) XCTAssertFalse(mockStore.experiments?.isEmpty ?? true) - var activeExperiments = featureFlagger.getAllActiveExperiments() + var activeExperiments = featureFlagger.allActiveExperiments XCTAssertEqual(activeExperiments.count, 2) XCTAssertEqual(activeExperiments[AutofillSubfeature.credentialsSaving.rawValue]?.cohortID, "control") XCTAssertEqual(activeExperiments[AutofillSubfeature.inlineIconCredentials.rawValue]?.cohortID, "green") @@ -1145,7 +1151,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "foo", data: featureJson) - activeExperiments = featureFlagger.getAllActiveExperiments() + activeExperiments = featureFlagger.allActiveExperiments XCTAssertEqual(activeExperiments.count, 1) XCTAssertNil(activeExperiments[AutofillSubfeature.credentialsSaving.rawValue]) XCTAssertEqual(activeExperiments[AutofillSubfeature.inlineIconCredentials.rawValue]?.cohortID, "green") @@ -1221,7 +1227,7 @@ final class FeatureFlaggerExperimentsTests: XCTestCase { manager.reload(etag: "foo", data: featureJson) - activeExperiments = featureFlagger.getAllActiveExperiments() + activeExperiments = featureFlagger.allActiveExperiments XCTAssertTrue(activeExperiments.isEmpty) } diff --git a/Tests/BrowserServicesKitTests/FeatureFlagging/TestFeatureFlag.swift b/Tests/BrowserServicesKitTests/FeatureFlagging/TestFeatureFlag.swift index 8d8ec9929..b53651806 100644 --- a/Tests/BrowserServicesKitTests/FeatureFlagging/TestFeatureFlag.swift +++ b/Tests/BrowserServicesKitTests/FeatureFlagging/TestFeatureFlag.swift @@ -19,15 +19,25 @@ import BrowserServicesKit enum TestFeatureFlag: String, FeatureFlagDescribing { + var cohortType: (any FeatureFlagCohortDescribing.Type)? { + switch self { + case .nonOverridableFlag, .overridableFlagDisabledByDefault, .overridableFlagEnabledByDefault: + nil + case .overridableExperimentFlagWithCohortBByDefault: + FakeExperimentCohort.self + } + } + case nonOverridableFlag case overridableFlagDisabledByDefault case overridableFlagEnabledByDefault + case overridableExperimentFlagWithCohortBByDefault var supportsLocalOverriding: Bool { switch self { case .nonOverridableFlag: return false - case .overridableFlagDisabledByDefault, .overridableFlagEnabledByDefault: + case .overridableFlagDisabledByDefault, .overridableFlagEnabledByDefault, .overridableExperimentFlagWithCohortBByDefault: return true } } @@ -40,6 +50,13 @@ enum TestFeatureFlag: String, FeatureFlagDescribing { return .disabled case .overridableFlagEnabledByDefault: return .internalOnly() + case .overridableExperimentFlagWithCohortBByDefault: + return .internalOnly(FakeExperimentCohort.cohortB) } } + + enum FakeExperimentCohort: String, FeatureFlagCohortDescribing { + case cohortA + case cohortB + } } diff --git a/Tests/ConfigurationTests/TrackerDataURLOverriderTests.swift b/Tests/ConfigurationTests/TrackerDataURLOverriderTests.swift index 93c0aa3ad..4a87958d0 100644 --- a/Tests/ConfigurationTests/TrackerDataURLOverriderTests.swift +++ b/Tests/ConfigurationTests/TrackerDataURLOverriderTests.swift @@ -45,7 +45,7 @@ final class TrackerDataURLOverriderTests: XCTestCase { func testTrackerDataURL_forControlCohort_returnsControlUrl() throws { // GIVEN mockFeatureFlagger.mockCohorts = [ - TDSExperimentType(rawValue: 0)!.subfeature.rawValue: TDSNextExperimentFlag.Cohort.control] + TDSExperimentType.allCases[0].rawValue: TDSExperimentType.Cohort.control] let privacyConfig = MockPrivacyConfiguration() privacyConfig.subfeatureSettings = "{ \"controlUrl\": \"\(controlURL)\", \"treatmentUrl\": \"\(treatmentURL)\"}" mockPrivacyConfigurationManager.privacyConfig = privacyConfig @@ -60,7 +60,7 @@ final class TrackerDataURLOverriderTests: XCTestCase { func testTrackerDataURL_forTreatmentCohort_returnsTreatmentUrl() throws { // GIVEN mockFeatureFlagger.mockCohorts = [ - TDSExperimentType(rawValue: 0)!.subfeature.rawValue: TDSNextExperimentFlag.Cohort.treatment] + TDSExperimentType.value(at: 0)!.rawValue: TDSExperimentType.Cohort.treatment] let privacyConfig = MockPrivacyConfiguration() privacyConfig.subfeatureSettings = "{ \"controlUrl\": \"\(controlURL)\", \"treatmentUrl\": \"\(treatmentURL)\"}" mockPrivacyConfigurationManager.privacyConfig = privacyConfig @@ -75,7 +75,7 @@ final class TrackerDataURLOverriderTests: XCTestCase { func testTrackerDataURL_ifNoSettings_returnsDefaultURL() throws { // GIVEN mockFeatureFlagger.mockCohorts = [ - TDSExperimentType(rawValue: 0)!.subfeature.rawValue: TDSNextExperimentFlag.Cohort.treatment] + TDSExperimentType.value(at: 0)!.rawValue: TDSExperimentType.Cohort.treatment] let privacyConfig = MockPrivacyConfiguration() mockPrivacyConfigurationManager.privacyConfig = privacyConfig @@ -106,19 +106,19 @@ final class TrackerDataURLOverriderTests: XCTestCase { let thirdExperimentTreatmentURL = "third-treatment.json" let privacyConfig = MockPrivacyConfiguration() privacyConfig.mockSubfeatureSettings = [ - TDSExperimentType(rawValue: 0)!.subfeature.rawValue: """ + TDSExperimentType.value(at: 0)!.rawValue: """ { "controlUrl": "\(firstExperimentControlURL)", "treatmentUrl": "first-treatment.json" } """, - TDSExperimentType(rawValue: 1)!.subfeature.rawValue: """ + TDSExperimentType.value(at: 1)!.subfeature.rawValue: """ { "controlUrl": "second-control.json", "treatmentUrl": "\(secondExperimentTreatmentURL)" } """, - TDSExperimentType(rawValue: 2)!.subfeature.rawValue: """ + TDSExperimentType.value(at: 2)!.subfeature.rawValue: """ { "controlUrl": "third-control.json", "treatmentUrl": "\(thirdExperimentTreatmentURL)" @@ -127,8 +127,8 @@ final class TrackerDataURLOverriderTests: XCTestCase { ] mockPrivacyConfigurationManager.privacyConfig = privacyConfig mockFeatureFlagger.mockCohorts = [ - TDSExperimentType(rawValue: 1)!.subfeature.rawValue: TDSNextExperimentFlag.Cohort.treatment, - TDSExperimentType(rawValue: 2)!.subfeature.rawValue: TDSNextExperimentFlag.Cohort.treatment + TDSExperimentType.value(at: 1)!.rawValue: TDSExperimentType.Cohort.treatment, + TDSExperimentType.value(at: 2)!.rawValue: TDSExperimentType.Cohort.treatment ] // WHEN @@ -143,22 +143,22 @@ final class TrackerDataURLOverriderTests: XCTestCase { private class MockFeatureFlaggerMockSettings: FeatureFlagger { var internalUserDecider: InternalUserDecider = DefaultInternalUserDecider(store: MockInternalUserStoring()) var localOverrides: FeatureFlagLocalOverriding? - var mockCohorts: [String: any FlagCohort] = [:] + var mockCohorts: [String: any FeatureFlagCohortDescribing] = [:] var isFeatureOn = true func isFeatureOn(for featureFlag: Flag, allowOverride: Bool) -> Bool { return isFeatureOn } - func getCohortIfEnabled(_ subfeature: any PrivacySubfeature) -> CohortID? { + func resolveCohort(_ subfeature: any PrivacySubfeature) -> CohortID? { return nil } - func getCohortIfEnabled(for featureFlag: Flag) -> (any FlagCohort)? where Flag: FeatureFlagExperimentDescribing { + func resolveCohort(for featureFlag: Flag, allowOverride: Bool) -> (any FeatureFlagCohortDescribing)? where Flag: FeatureFlagDescribing { return mockCohorts[featureFlag.rawValue] } - func getAllActiveExperiments() -> Experiments { + var allActiveExperiments: Experiments { return [:] } } @@ -258,3 +258,10 @@ extension DefaultInternalUserDecider { self.init(store: mockedStore) } } + +extension TDSExperimentType { + static func value(at index: Int) -> TDSExperimentType? { + guard index >= 0 && index < allCases.count else { return nil } + return allCases[index] + } +} diff --git a/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift b/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift index 545bc68ce..010064137 100644 --- a/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift +++ b/Tests/PixelExperimentKitTests/PixelExperimentKitTests.swift @@ -606,17 +606,21 @@ class MockExperimentActionPixelStore: ExperimentActionPixelStore { } class MockFeatureFlagger: FeatureFlagger { + func resolveCohort(for featureFlag: Flag, allowOverride: Bool) -> (any FeatureFlagCohortDescribing)? where Flag: FeatureFlagDescribing { + nil + } + var experiments: Experiments = [:] var internalUserDecider: any InternalUserDecider = MockInternalUserDecider() var localOverrides: (any BrowserServicesKit.FeatureFlagLocalOverriding)? - func getCohortIfEnabled(for featureFlag: Flag) -> (any FlagCohort)? where Flag: FeatureFlagExperimentDescribing { + func resolveCohort(for featureFlag: Flag) -> (any FeatureFlagCohortDescribing)? where Flag: FeatureFlagDescribing { return nil } - func getAllActiveExperiments() -> Experiments { + var allActiveExperiments: Experiments { return experiments } diff --git a/Tests/PixelExperimentKitTests/TDSOverrideExperimentMetricsTests.swift b/Tests/PixelExperimentKitTests/TDSOverrideExperimentMetricsTests.swift index 95cf05c2f..cd3eba041 100644 --- a/Tests/PixelExperimentKitTests/TDSOverrideExperimentMetricsTests.swift +++ b/Tests/PixelExperimentKitTests/TDSOverrideExperimentMetricsTests.swift @@ -59,7 +59,7 @@ final class TDSOverrideExperimentMetricsTests: XCTestCase { XCTAssertEqual(pixelCalls.first?.3, "1", "expected Value should be passed as parameter") XCTAssertEqual(debugCalls.count, 6, "fireDebugExperiment should be called for each conversionWindow on one experiment.") XCTAssertEqual(debugCalls.first?["tdsEtag"], "testEtag") - XCTAssertEqual(debugCalls.first?["experiment"], "\(TDSExperimentType.allCases[3].experiment.rawValue)testCohort") + XCTAssertEqual(debugCalls.first?["experiment"], "\(TDSExperimentType.allCases[3].subfeature.rawValue)testCohort") } func test_OnfireTdsExperimentMetricPrivacyToggleUsed_WhenNoExperimentActive_ThenCorrectPixelFunctionsCalled() {