From d671accf1bf7097c4e7f5cd55cd1c6dfa005cf92 Mon Sep 17 00:00:00 2001 From: Dominik Kapusta Date: Wed, 20 Dec 2023 16:36:41 +0100 Subject: [PATCH] Add Sync feature flags (#607) Task/Issue URL: https://app.asana.com/0/0/1206046777189407/f Description: This change add support for Sync feature in Privacy Configuration, together with 4 subfeatures defining availability of various parts of Sync experience. DDGSyncing gets a read-only feature flag variable as well as a publisher. PrivacyConfigurationManager is now a Sync dependency, and DDGSync takes care internally of listening to Privacy Config changes and updating feature flags as needed. Feature flag responsible for actual data syncing is handled internally in DDGSync by cancelling all pending sync operations and disabling adding new operations. Other feature flags should be handled by client apps. --- Package.resolved | 4 +- Package.swift | 1 + .../Features/PrivacyFeature.swift | 12 +++ Sources/DDGSync/DDGSync.swift | 34 +++++++- Sources/DDGSync/DDGSyncing.swift | 15 +++- Sources/DDGSync/SyncFeatureFlags.swift | 87 +++++++++++++++++++ .../internal/ProductionDependencies.swift | 15 +++- .../DDGSync/internal/SyncDependencies.swift | 4 +- Sources/DDGSync/internal/SyncQueue.swift | 15 ++++ Tests/DDGSyncTests/Mocks/Mocks.swift | 43 +++++++++ Tests/DDGSyncTests/SyncQueueTests.swift | 25 ++++++ 11 files changed, 244 insertions(+), 11 deletions(-) create mode 100644 Sources/DDGSync/SyncFeatureFlags.swift diff --git a/Package.resolved b/Package.resolved index c65a105fd..2476554ae 100644 --- a/Package.resolved +++ b/Package.resolved @@ -59,8 +59,8 @@ "kind" : "remoteSourceControl", "location" : "https://github.com/apple/swift-argument-parser", "state" : { - "revision" : "8f4d2753f0e4778c76d5f05ad16c74f707390531", - "version" : "1.2.3" + "revision" : "c8ed701b513cf5177118a175d85fbbbcd707ab41", + "version" : "1.3.0" } }, { diff --git a/Package.swift b/Package.swift index a9b092e40..23af46854 100644 --- a/Package.swift +++ b/Package.swift @@ -127,6 +127,7 @@ let package = Package( .target( name: "DDGSync", dependencies: [ + "BrowserServicesKit", "Common", .product(name: "DDGSyncCrypto", package: "sync_crypto"), "Networking" diff --git a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift index d8e85a604..d15e4eca9 100644 --- a/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift +++ b/Sources/BrowserServicesKit/PrivacyConfig/Features/PrivacyFeature.swift @@ -41,6 +41,7 @@ public enum PrivacyFeature: String { case newTabContinueSetUp case networkProtection case dbp + case sync } /// An abstraction to be implemented by any "subfeature" of a given `PrivacyConfiguration` feature. @@ -82,3 +83,14 @@ public enum DBPSubfeature: String, Equatable, PrivacySubfeature { case waitlist case waitlistBetaActive } + +public enum SyncSubfeature: String, PrivacySubfeature { + public var parent: PrivacyFeature { + .sync + } + + case level0ShowSync + case level1AllowDataSyncing + case level2AllowSetupFlows + case level3AllowCreateAccount +} diff --git a/Sources/DDGSync/DDGSync.swift b/Sources/DDGSync/DDGSync.swift index ffae67f50..0df8e1e3f 100644 --- a/Sources/DDGSync/DDGSync.swift +++ b/Sources/DDGSync/DDGSync.swift @@ -16,15 +16,21 @@ // limitations under the License. // -import Foundation +import BrowserServicesKit import Combine -import DDGSyncCrypto import Common +import DDGSyncCrypto +import Foundation public class DDGSync: DDGSyncing { public static let bundle = Bundle.module + @Published public private(set) var featureFlags: SyncFeatureFlags = .all + public var featureFlagsPublisher: AnyPublisher { + $featureFlags.eraseToAnyPublisher() + } + enum Constants { public static let syncEnabledKey = "com.duckduckgo.sync.enabled" } @@ -55,9 +61,15 @@ public class DDGSync: DDGSyncing { /// This is the constructor intended for use by app clients. public convenience init(dataProvidersSource: DataProvidersSource, errorEvents: EventMapping, + privacyConfigurationManager: PrivacyConfigurationManaging, log: @escaping @autoclosure () -> OSLog = .disabled, environment: ServerEnvironment = .production) { - let dependencies = ProductionDependencies(serverEnvironment: environment, errorEvents: errorEvents, log: log()) + let dependencies = ProductionDependencies( + serverEnvironment: environment, + privacyConfigurationManager: privacyConfigurationManager, + errorEvents: errorEvents, + log: log() + ) self.init(dataProvidersSource: dataProvidersSource, dependencies: dependencies) } @@ -189,6 +201,16 @@ public class DDGSync: DDGSyncing { init(dataProvidersSource: DataProvidersSource, dependencies: SyncDependencies) { self.dataProvidersSource = dataProvidersSource self.dependencies = dependencies + + featureFlagsCancellable = self.dependencies.privacyConfigurationManager.updatesPublisher + .compactMap { [weak self] in + self?.dependencies.privacyConfigurationManager.privacyConfig + } + .prepend(dependencies.privacyConfigurationManager.privacyConfig) + .map(SyncFeatureFlags.init) + .removeDuplicates() + .receive(on: DispatchQueue.main) + .assign(to: \.featureFlags, onWeaklyHeld: self) } public func initializeIfNeeded() { @@ -229,6 +251,7 @@ public class DDGSync: DDGSyncing { dependencies.scheduler.isEnabled = false startSyncCancellable?.cancel() syncQueueCancellable?.cancel() + isDataSyncingFeatureFlagEnabledCancellable?.cancel() try syncQueue?.dataProviders.forEach { try $0.deregisterFeature() } syncQueue = nil authState = .inactive @@ -291,6 +314,9 @@ public class DDGSync: DDGSyncing { self?.syncQueue?.resumeQueue() } + isDataSyncingFeatureFlagEnabledCancellable = featureFlagsPublisher.prepend(featureFlags).map { $0.contains(.dataSyncing) } + .assign(to: \.isDataSyncingFeatureFlagEnabled, onWeaklyHeld: syncQueue) + dependencies.scheduler.isEnabled = true self.syncQueue = syncQueue } @@ -317,6 +343,8 @@ public class DDGSync: DDGSyncing { private var startSyncCancellable: AnyCancellable? private var cancelSyncCancellable: AnyCancellable? private var resumeSyncCancellable: AnyCancellable? + private var featureFlagsCancellable: AnyCancellable? + private var isDataSyncingFeatureFlagEnabledCancellable: AnyCancellable? private var syncQueue: SyncQueue? private var syncQueueCancellable: AnyCancellable? diff --git a/Sources/DDGSync/DDGSyncing.swift b/Sources/DDGSync/DDGSyncing.swift index 4dcae0921..71d8dfcb3 100644 --- a/Sources/DDGSync/DDGSyncing.swift +++ b/Sources/DDGSync/DDGSyncing.swift @@ -16,9 +16,10 @@ // limitations under the License. // -import Foundation -import DDGSyncCrypto +import BrowserServicesKit import Combine +import DDGSyncCrypto +import Foundation public enum SyncAuthState: String, Sendable, Codable { /// Sync engine is not initialized. @@ -48,6 +49,16 @@ public protocol DDGSyncing: DDGSyncingDebuggingSupport { var dataProvidersSource: DataProvidersSource? { get set } + /** + Describes current availability of sync features. + */ + var featureFlags: SyncFeatureFlags { get } + + /** + Emits changes to current availability of sync features + */ + var featureFlagsPublisher: AnyPublisher { get } + /** Describes current state of sync account. diff --git a/Sources/DDGSync/SyncFeatureFlags.swift b/Sources/DDGSync/SyncFeatureFlags.swift new file mode 100644 index 000000000..88eea2783 --- /dev/null +++ b/Sources/DDGSync/SyncFeatureFlags.swift @@ -0,0 +1,87 @@ +// +// SyncFeatureFlags.swift +// +// Copyright © 2023 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 BrowserServicesKit +import Foundation + +/** + * This enum describes available Sync features. + */ +public struct SyncFeatureFlags: OptionSet { + public let rawValue: Int + + public init(rawValue: Int) { + self.rawValue = rawValue + } + + // MARK: - Individual Flags + + /// Sync UI is visible + public static let userInterface = SyncFeatureFlags(rawValue: 1 << 0) + + /// Data syncing is available + public static let dataSyncing = SyncFeatureFlags(rawValue: 1 << 1) + + /// Logging in to existing accounts is available (connect flows + account recovery) + public static let accountLogin = SyncFeatureFlags(rawValue: 1 << 2) + + /// Creating new accounts is available + public static let accountCreation = SyncFeatureFlags(rawValue: 1 << 4) + + // MARK: - Helper Flags + + public static let connectFlows = SyncFeatureFlags.accountLogin + public static let accountRecovery = SyncFeatureFlags.accountLogin + + // MARK: - Support levels + + /// Used when all feature flags are disabled + public static let unavailable: SyncFeatureFlags = [] + + /// Level 0 feature flag as defined in Privacy Configuration + public static let level0ShowSync: SyncFeatureFlags = [.userInterface] + /// Level 1 feature flag as defined in Privacy Configuration + public static let level1AllowDataSyncing: SyncFeatureFlags = [.userInterface, .dataSyncing] + /// Level 2 feature flag as defined in Privacy Configuration + public static let level2AllowSetupFlows: SyncFeatureFlags = [.userInterface, .dataSyncing, .accountLogin] + /// Level 3 feature flag as defined in Privacy Configuration + public static let level3AllowCreateAccount: SyncFeatureFlags = [.userInterface, .dataSyncing, .accountLogin, .accountCreation] + + /// Alias for the state when all features are available + public static let all: SyncFeatureFlags = .level3AllowCreateAccount + + // MARK: - + + init(privacyConfig: PrivacyConfiguration) { + guard privacyConfig.isEnabled(featureKey: .sync) else { + self = .unavailable + return + } + if !privacyConfig.isSubfeatureEnabled(SyncSubfeature.level0ShowSync) { + self = .unavailable + } else if !privacyConfig.isSubfeatureEnabled(SyncSubfeature.level1AllowDataSyncing) { + self = .level0ShowSync + } else if !privacyConfig.isSubfeatureEnabled(SyncSubfeature.level2AllowSetupFlows) { + self = .level1AllowDataSyncing + } else if !privacyConfig.isSubfeatureEnabled(SyncSubfeature.level3AllowCreateAccount) { + self = .level2AllowSetupFlows + } else { + self = .level3AllowCreateAccount + } + } +} diff --git a/Sources/DDGSync/internal/ProductionDependencies.swift b/Sources/DDGSync/internal/ProductionDependencies.swift index aff1764de..66724039e 100644 --- a/Sources/DDGSync/internal/ProductionDependencies.swift +++ b/Sources/DDGSync/internal/ProductionDependencies.swift @@ -16,8 +16,9 @@ // limitations under the License. // -import Foundation +import BrowserServicesKit import Common +import Foundation import Persistence struct ProductionDependencies: SyncDependencies { @@ -30,6 +31,7 @@ struct ProductionDependencies: SyncDependencies { let secureStore: SecureStoring let crypter: CryptingInternal let scheduler: SchedulingInternal + let privacyConfigurationManager: PrivacyConfigurationManaging let errorEvents: EventMapping var log: OSLog { @@ -37,12 +39,17 @@ struct ProductionDependencies: SyncDependencies { } private let getLog: () -> OSLog - init(serverEnvironment: ServerEnvironment, errorEvents: EventMapping, log: @escaping @autoclosure () -> OSLog = .disabled) { - + init( + serverEnvironment: ServerEnvironment, + privacyConfigurationManager: PrivacyConfigurationManaging, + errorEvents: EventMapping, + log: @escaping @autoclosure () -> OSLog = .disabled + ) { self.init(fileStorageUrl: FileManager.default.applicationSupportDirectoryForComponent(named: "Sync"), serverEnvironment: serverEnvironment, keyValueStore: UserDefaults(), secureStore: SecureStorage(), + privacyConfigurationManager: privacyConfigurationManager, errorEvents: errorEvents, log: log()) } @@ -52,6 +59,7 @@ struct ProductionDependencies: SyncDependencies { serverEnvironment: ServerEnvironment, keyValueStore: KeyValueStoring, secureStore: SecureStoring, + privacyConfigurationManager: PrivacyConfigurationManaging, errorEvents: EventMapping, log: @escaping @autoclosure () -> OSLog = .disabled ) { @@ -59,6 +67,7 @@ struct ProductionDependencies: SyncDependencies { self.endpoints = Endpoints(serverEnvironment: serverEnvironment) self.keyValueStore = keyValueStore self.secureStore = secureStore + self.privacyConfigurationManager = privacyConfigurationManager self.errorEvents = errorEvents self.getLog = log diff --git a/Sources/DDGSync/internal/SyncDependencies.swift b/Sources/DDGSync/internal/SyncDependencies.swift index d88f643f8..6e5740c0d 100644 --- a/Sources/DDGSync/internal/SyncDependencies.swift +++ b/Sources/DDGSync/internal/SyncDependencies.swift @@ -16,9 +16,10 @@ // limitations under the License. // -import Foundation +import BrowserServicesKit import Combine import Common +import Foundation import Persistence protocol SyncDependenciesDebuggingSupport { @@ -34,6 +35,7 @@ protocol SyncDependencies: SyncDependenciesDebuggingSupport { var secureStore: SecureStoring { get } var crypter: CryptingInternal { get } var scheduler: SchedulingInternal { get } + var privacyConfigurationManager: PrivacyConfigurationManaging { get } var errorEvents: EventMapping { get } var log: OSLog { get } diff --git a/Sources/DDGSync/internal/SyncQueue.swift b/Sources/DDGSync/internal/SyncQueue.swift index 9230a426b..a5a2bd4da 100644 --- a/Sources/DDGSync/internal/SyncQueue.swift +++ b/Sources/DDGSync/internal/SyncQueue.swift @@ -117,7 +117,22 @@ final class SyncQueue { } } + var isDataSyncingFeatureFlagEnabled: Bool = true { + didSet { + if isDataSyncingFeatureFlagEnabled { + os_log(.debug, log: self.log, "Sync Feature has been enabled") + } else { + os_log(.debug, log: self.log, "Sync Feature has been disabled, cancelling all operations") + operationQueue.cancelAllOperations() + } + } + } + func startSync() { + guard isDataSyncingFeatureFlagEnabled else { + os_log(.debug, log: self.log, "Sync Feature is temporarily disabled, not starting sync") + return + } let operation = makeSyncOperation() operationQueue.addOperation(operation) } diff --git a/Tests/DDGSyncTests/Mocks/Mocks.swift b/Tests/DDGSyncTests/Mocks/Mocks.swift index 08ea0168a..884956464 100644 --- a/Tests/DDGSyncTests/Mocks/Mocks.swift +++ b/Tests/DDGSyncTests/Mocks/Mocks.swift @@ -16,6 +16,7 @@ // limitations under the License. // +import BrowserServicesKit import Combine import Common import Foundation @@ -132,6 +133,47 @@ class MockErrorHandler: EventMapping { } } +class MockPrivacyConfigurationManager: PrivacyConfigurationManaging { + var currentConfig: Data = .init() + var updatesSubject = PassthroughSubject() + let updatesPublisher: AnyPublisher + var privacyConfig: PrivacyConfiguration = MockPrivacyConfiguration() + func reload(etag: String?, data: Data?) -> PrivacyConfigurationManager.ReloadResult { + .downloaded + } + + init() { + updatesPublisher = updatesSubject.eraseToAnyPublisher() + } +} + +class MockPrivacyConfiguration: PrivacyConfiguration { + + func isEnabled(featureKey: PrivacyFeature, versionProvider: AppVersionProvider) -> Bool { true } + + func isSubfeatureEnabled( + _ subfeature: any PrivacySubfeature, + versionProvider: AppVersionProvider, + randomizer: (Range) -> Double + ) -> Bool { + true + } + + var identifier: String = "abcd" + var userUnprotectedDomains: [String] = [] + var tempUnprotectedDomains: [String] = [] + var trackerAllowlist: PrivacyConfigurationData.TrackerAllowlist = .init(json: ["state": "disabled"])! + func exceptionsList(forFeature featureKey: PrivacyFeature) -> [String] { [] } + func isFeature(_ feature: PrivacyFeature, enabledForDomain: String?) -> Bool { true } + func isProtected(domain: String?) -> Bool { false } + func isUserUnprotected(domain: String?) -> Bool { false } + func isTempUnprotected(domain: String?) -> Bool { false } + func isInExceptionList(domain: String?, forFeature featureKey: PrivacyFeature) -> Bool { false } + func settings(for feature: PrivacyFeature) -> PrivacyConfigurationData.PrivacyFeature.FeatureSettings { .init() } + func userEnabledProtection(forDomain: String) {} + func userDisabledProtection(forDomain: String) {} +} + struct MockSyncDependencies: SyncDependencies, SyncDependenciesDebuggingSupport { var endpoints: Endpoints = Endpoints(baseURL: URL(string: "https://dev.null")!) var account: AccountManaging = AccountManagingMock() @@ -140,6 +182,7 @@ struct MockSyncDependencies: SyncDependencies, SyncDependenciesDebuggingSupport var crypter: CryptingInternal = CryptingMock() var scheduler: SchedulingInternal = SchedulerMock() var log: OSLog = .default + var privacyConfigurationManager: PrivacyConfigurationManaging = MockPrivacyConfigurationManager() var errorEvents: EventMapping = MockErrorHandler() var keyValueStore: KeyValueStoring = MockKeyValueStore() diff --git a/Tests/DDGSyncTests/SyncQueueTests.swift b/Tests/DDGSyncTests/SyncQueueTests.swift index 107b998da..66f530348 100644 --- a/Tests/DDGSyncTests/SyncQueueTests.swift +++ b/Tests/DDGSyncTests/SyncQueueTests.swift @@ -52,6 +52,31 @@ class SyncQueueTests: XCTestCase { requestMaker = SyncRequestMaker(storage: storage, api: apiMock, endpoints: endpoints) } + func testWhenDataSyncingFeatureFlagIsDisabledThenNewOperationsAreNotEnqueued() async { + let syncQueue = SyncQueue(dataProviders: [], storage: storage, crypter: crypter, api: apiMock, endpoints: endpoints) + XCTAssertFalse(syncQueue.operationQueue.isSuspended) + + var syncDidStartEvents = [Bool]() + let cancellable = syncQueue.isSyncInProgressPublisher.removeDuplicates().filter({ $0 }).sink { syncDidStartEvents.append($0) } + + syncQueue.isDataSyncingFeatureFlagEnabled = false + + await syncQueue.startSync() + await syncQueue.startSync() + await syncQueue.startSync() + + XCTAssertTrue(syncDidStartEvents.isEmpty) + + syncQueue.isDataSyncingFeatureFlagEnabled = true + + await syncQueue.startSync() + await syncQueue.startSync() + await syncQueue.startSync() + + cancellable.cancel() + XCTAssertEqual(syncDidStartEvents.count, 3) + } + func testThatInProgressPublisherEmitsValuesWhenSyncStartsAndEndsWithSuccess() async throws { let feature = Feature(name: "bookmarks") let dataProvider = DataProvidingMock(feature: feature)