From 0872c27fcea933e1083f7df829a7654c10f069b5 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sun, 10 Sep 2023 10:34:42 +0200 Subject: [PATCH] Move CloudKit logic to PersistenceManager (#355) Observe updates rather than execute operations imperatively. Also refine responsibilities of AppContext and CoreContext. --- Passepartout.xcodeproj/project.pbxproj | 4 - .../App/Context/AppContext+Shared.swift | 16 ++- Passepartout/App/Context/AppContext.swift | 54 ++++------- Passepartout/App/Context/CoreContext.swift | 60 ++++-------- Passepartout/App/Domain/AppPreference.swift | 2 - .../Extensions/KeyValueStore+CloudKit.swift | 63 ------------ .../DefaultUpgradeManagerStrategy.swift | 2 +- .../App/Managers/PersistenceManager.swift | 97 ++++++++++++++++++- Passepartout/App/Views/SettingsView.swift | 25 ++--- .../Views/LaunchOnLoginItem+ViewModel.swift | 2 +- .../Data/VPNPersistence.swift | 12 --- 11 files changed, 151 insertions(+), 186 deletions(-) delete mode 100644 Passepartout/App/Extensions/KeyValueStore+CloudKit.swift diff --git a/Passepartout.xcodeproj/project.pbxproj b/Passepartout.xcodeproj/project.pbxproj index 219bf928..4b58faa5 100644 --- a/Passepartout.xcodeproj/project.pbxproj +++ b/Passepartout.xcodeproj/project.pbxproj @@ -50,7 +50,6 @@ 0E34AC7827F840890042F2AB /* OrganizerView+Scene.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E34AC7727F840890042F2AB /* OrganizerView+Scene.swift */; }; 0E34AC8227F892C40042F2AB /* OnDemandView+SSID.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E34AC8127F892C40042F2AB /* OnDemandView+SSID.swift */; }; 0E35C09A280E95BB0071FA35 /* ProviderProfileAvailability.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E35C099280E95BB0071FA35 /* ProviderProfileAvailability.swift */; }; - 0E3A3C102AA9AA530003A5F6 /* KeyValueStore+CloudKit.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E3A3C0F2AA9AA530003A5F6 /* KeyValueStore+CloudKit.swift */; }; 0E3A3C132AAB7C480003A5F6 /* UpgradeManager.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E3A3C112AAB7C470003A5F6 /* UpgradeManager.swift */; }; 0E3A3C142AAB7C480003A5F6 /* DefaultUpgradeManagerStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E3A3C122AAB7C480003A5F6 /* DefaultUpgradeManagerStrategy.swift */; }; 0E3A3C162AAB8AB80003A5F6 /* UpgradeManagerStrategy.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E3A3C152AAB8AB80003A5F6 /* UpgradeManagerStrategy.swift */; }; @@ -344,7 +343,6 @@ 0E34AC7727F840890042F2AB /* OrganizerView+Scene.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OrganizerView+Scene.swift"; sourceTree = ""; }; 0E34AC8127F892C40042F2AB /* OnDemandView+SSID.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "OnDemandView+SSID.swift"; sourceTree = ""; }; 0E35C099280E95BB0071FA35 /* ProviderProfileAvailability.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ProviderProfileAvailability.swift; sourceTree = ""; }; - 0E3A3C0F2AA9AA530003A5F6 /* KeyValueStore+CloudKit.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "KeyValueStore+CloudKit.swift"; sourceTree = ""; }; 0E3A3C112AAB7C470003A5F6 /* UpgradeManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpgradeManager.swift; sourceTree = ""; }; 0E3A3C122AAB7C480003A5F6 /* DefaultUpgradeManagerStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DefaultUpgradeManagerStrategy.swift; sourceTree = ""; }; 0E3A3C152AAB8AB80003A5F6 /* UpgradeManagerStrategy.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = UpgradeManagerStrategy.swift; sourceTree = ""; }; @@ -746,7 +744,6 @@ isa = PBXGroup; children = ( 0EBC075C27EC529000208AD9 /* DebugLog+Constants.swift */, - 0E3A3C0F2AA9AA530003A5F6 /* KeyValueStore+CloudKit.swift */, 0EB17EB927D2560300D473B5 /* PassepartoutProviders+Extensions.swift */, 0E35C099280E95BB0071FA35 /* ProviderProfileAvailability.swift */, 0E2DE71B27DCCFE80067B9E1 /* TunnelKit+Extensions.swift */, @@ -1545,7 +1542,6 @@ 0E53249927D26B51002565C3 /* ProductManager.swift in Sources */, 0E9C233027F47032007D5FC7 /* IntentsManager.swift in Sources */, 0EB4042C27CA0E8C00378B1A /* Unlocalized.swift in Sources */, - 0E3A3C102AA9AA530003A5F6 /* KeyValueStore+CloudKit.swift in Sources */, 0EB4042E27CA136300378B1A /* AddingTextField.swift in Sources */, 0EE8B7E327FF340F00B68621 /* VPNProtocolType+FileExtensions.swift in Sources */, 0EF2212B27E667EA001D0BD7 /* AddProviderView+Name.swift in Sources */, diff --git a/Passepartout/App/Context/AppContext+Shared.swift b/Passepartout/App/Context/AppContext+Shared.swift index f77783d0..a4543355 100644 --- a/Passepartout/App/Context/AppContext+Shared.swift +++ b/Passepartout/App/Context/AppContext+Shared.swift @@ -28,20 +28,26 @@ import PassepartoutLibrary // safer alternative to @EnvironmentObject -extension AppContext { - private static let coreContext = CoreContext(store: UserDefaultsStore(defaults: .standard, key: \.key)) +// MARK: App - static let shared = AppContext(coreContext: coreContext) +extension AppContext { + static let shared = AppContext(store: UserDefaultsStore(defaults: .standard, key: \.key)) +} + +extension UpgradeManager { + static let shared = AppContext.shared.upgradeManager } extension ProductManager { static let shared = AppContext.shared.productManager } -extension UpgradeManager { - static let shared = AppContext.shared.upgradeManager +extension PersistenceManager { + static let shared = AppContext.shared.persistenceManager } +// MARK: App -> Core + extension ProfileManager { static let shared = AppContext.shared.profileManager } diff --git a/Passepartout/App/Context/AppContext.swift b/Passepartout/App/Context/AppContext.swift index 6aad8548..9c1c1d1e 100644 --- a/Passepartout/App/Context/AppContext.swift +++ b/Passepartout/App/Context/AppContext.swift @@ -31,34 +31,48 @@ import PassepartoutLibrary final class AppContext { private let coreContext: CoreContext - private var lastIsCloudSyncingEnabled: Bool? + let upgradeManager: UpgradeManager let productManager: ProductManager + let persistenceManager: PersistenceManager + private let reviewer: Reviewer private var cancellables: Set = [] - init(coreContext: CoreContext) { - self.coreContext = coreContext + init(store: KeyValueStore) { + let logger = SwiftyBeaverLogger( + logFile: Constants.Log.App.url, + logLevel: Constants.Log.level, + logFormat: Constants.Log.App.format + ) + Passepartout.shared.logger = logger + pp_log.info("Logging to: \(logger.logFile!)") + + upgradeManager = UpgradeManager( + store: store, + strategy: DefaultUpgradeManagerStrategy() + ) + upgradeManager.migrate(toVersion: Constants.Global.appVersionNumber) productManager = ProductManager( overriddenAppType: Constants.InApp.overriddenAppType, buildProducts: Constants.InApp.buildProducts ) + persistenceManager = PersistenceManager(store: store) + reviewer = Reviewer() reviewer.eventCountBeforeRating = Constants.Rating.eventCount + coreContext = CoreContext(persistenceManager: persistenceManager) + // post configureObjects() } - var upgradeManager: UpgradeManager { - coreContext.upgradeManager - } - var providerManager: ProviderManager { coreContext.providerManager } @@ -116,29 +130,3 @@ private extension AppContext { return true } } - -// MARK: CloudKit - -extension AppContext { - var shouldEnableCloudSyncing: Bool { - get { - coreContext.store.shouldEnableCloudSyncing - } - set { - coreContext.store.shouldEnableCloudSyncing = newValue - - // iCloud may be externally disabled from the device settings - let isCloudSyncingEnabled = coreContext.store.isCloudSyncingEnabled - guard isCloudSyncingEnabled != lastIsCloudSyncingEnabled else { - pp_log.debug("CloudKit state did not change") - return - } - coreContext.reloadCloudKitObjects(isEnabled: isCloudSyncingEnabled) - lastIsCloudSyncingEnabled = isCloudSyncingEnabled - } - } - - func eraseCloudKitStore() async { - await coreContext.eraseCloudKitStore() - } -} diff --git a/Passepartout/App/Context/CoreContext.swift b/Passepartout/App/Context/CoreContext.swift index a243795f..545d68d8 100644 --- a/Passepartout/App/Context/CoreContext.swift +++ b/Passepartout/App/Context/CoreContext.swift @@ -34,12 +34,6 @@ import TunnelKitManager final class CoreContext { let store: KeyValueStore - private let persistenceManager: PersistenceManager - - private(set) var vpnPersistence: VPNPersistence - - let upgradeManager: UpgradeManager - let providerManager: ProviderManager let profileManager: ProfileManager @@ -48,27 +42,11 @@ final class CoreContext { private var cancellables: Set = [] - init(store: KeyValueStore) { - self.store = store + init(persistenceManager: PersistenceManager) { + store = persistenceManager.store - let logger = SwiftyBeaverLogger( - logFile: Constants.Log.App.url, - logLevel: Constants.Log.level, - logFormat: Constants.Log.App.format - ) - Passepartout.shared.logger = logger - pp_log.info("Logging to: \(logger.logFile!)") - - upgradeManager = UpgradeManager( - store: store, - strategy: DefaultUpgradeManagerStrategy() - ) - upgradeManager.migrate(toVersion: Constants.Global.appVersionNumber) - - persistenceManager = PersistenceManager(store: store) - vpnPersistence = persistenceManager.vpnPersistence( - withName: Constants.Persistence.profilesContainerName, - cloudKit: store.isCloudSyncingEnabled + let vpnPersistence = persistenceManager.vpnPersistence( + withName: Constants.Persistence.profilesContainerName ) let providersPersistence = persistenceManager.providersPersistence( withName: Constants.Persistence.providersContainerName @@ -119,12 +97,12 @@ final class CoreContext { // post - configureObjects() + configureObjects(persistenceManager: persistenceManager) } } private extension CoreContext { - func configureObjects() { + func configureObjects(persistenceManager: PersistenceManager) { providerManager.rateLimitMilliseconds = Constants.RateLimit.providerManager vpnManager.tunnelLogPath = Constants.Log.Tunnel.path vpnManager.tunnelLogFormat = Constants.Log.Tunnel.format @@ -133,27 +111,25 @@ private extension CoreContext { vpnManager.observeUpdates() CoreConfiguration.masksPrivateData = vpnManager.masksPrivateData - vpnManager.didUpdatePreferences.sink { - CoreConfiguration.masksPrivateData = $0.masksPrivateData - }.store(in: &cancellables) + vpnManager.didUpdatePreferences + .sink { + CoreConfiguration.masksPrivateData = $0.masksPrivateData + }.store(in: &cancellables) + + persistenceManager.didChangePersistence + .sink { [weak self] in + self?.reloadCloudKitObjects(persistenceManager: persistenceManager) + }.store(in: &cancellables) } } // MARK: CloudKit extension CoreContext { - func reloadCloudKitObjects(isEnabled: Bool) { - vpnPersistence = persistenceManager.vpnPersistence( - withName: Constants.Persistence.profilesContainerName, - cloudKit: isEnabled + func reloadCloudKitObjects(persistenceManager: PersistenceManager) { + let vpnPersistence = persistenceManager.vpnPersistence( + withName: Constants.Persistence.profilesContainerName ) profileManager.swapProfileRepository(vpnPersistence.profileRepository()) } - - func eraseCloudKitStore() async { - await vpnPersistence.eraseCloudKitStore( - fromContainerWithId: Constants.CloudKit.containerId, - zoneId: .init(zoneName: Constants.CloudKit.coreDataZone) - ) - } } diff --git a/Passepartout/App/Domain/AppPreference.swift b/Passepartout/App/Domain/AppPreference.swift index daa44e66..f15ec4bc 100644 --- a/Passepartout/App/Domain/AppPreference.swift +++ b/Passepartout/App/Domain/AppPreference.swift @@ -29,8 +29,6 @@ import PassepartoutLibrary enum AppPreference: String, KeyStoreDomainLocation { case launchesOnLogin - case shouldEnableCloudSyncing - case isShowingFavorites case didHandleSubreddit diff --git a/Passepartout/App/Extensions/KeyValueStore+CloudKit.swift b/Passepartout/App/Extensions/KeyValueStore+CloudKit.swift deleted file mode 100644 index d4be177e..00000000 --- a/Passepartout/App/Extensions/KeyValueStore+CloudKit.swift +++ /dev/null @@ -1,63 +0,0 @@ -// -// KeyValueStore+CloudKit.swift -// Passepartout -// -// Created by Davide De Rosa on 9/7/23. -// Copyright (c) 2023 Davide De Rosa. All rights reserved. -// -// https://github.com/passepartoutvpn -// -// This file is part of Passepartout. -// -// Passepartout is free software: you can redistribute it and/or modify -// it under the terms of the GNU General Public License as published by -// the Free Software Foundation, either version 3 of the License, or -// (at your option) any later version. -// -// Passepartout is distributed in the hope that it will be useful, -// but WITHOUT ANY WARRANTY; without even the implied warranty of -// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -// GNU General Public License for more details. -// -// You should have received a copy of the GNU General Public License -// along with Passepartout. If not, see . -// - -import Foundation -import PassepartoutLibrary - -extension KeyValueStore { - - // MARK: Support - - private var cloudKitToken: Any? { - FileManager.default.ubiquityIdentityToken - } - - var isCloudKitSupported: Bool { - cloudKitToken != nil - } - - // MARK: Preference - - var shouldEnableCloudSyncing: Bool { - get { - value(forLocation: AppPreference.shouldEnableCloudSyncing) ?? false - } - set { - setValue(newValue, forLocation: AppPreference.shouldEnableCloudSyncing) - } - } - - // MARK: Computed state - - var isCloudSyncingEnabled: Bool { - guard isCloudKitSupported else { - pp_log.debug("CloudKit unavailable") - return false - } - let isEnabled = shouldEnableCloudSyncing - pp_log.debug("CloudKit enabled: \(isEnabled)") - return isEnabled - } -} diff --git a/Passepartout/App/Managers/DefaultUpgradeManagerStrategy.swift b/Passepartout/App/Managers/DefaultUpgradeManagerStrategy.swift index 394ce1c9..2892b9c5 100644 --- a/Passepartout/App/Managers/DefaultUpgradeManagerStrategy.swift +++ b/Passepartout/App/Managers/DefaultUpgradeManagerStrategy.swift @@ -36,7 +36,7 @@ public final class DefaultUpgradeManagerStrategy: UpgradeManagerStrategy { let isUpgradeFromBefore_2_2_0: Bool? = store.value(forLocation: UpgradeManager.StoreKey.existingKeyBefore_2_2_0) if isUpgradeFromBefore_2_2_0 != nil { pp_log.debug("Upgrading from < 2.2.0, iCloud syncing defaults to enabled") - store.setValue(true, forLocation: AppPreference.shouldEnableCloudSyncing) + store.setValue(true, forLocation: PersistenceManager.StoreKey.shouldEnableCloudSyncing) store.removeValue(forLocation: UpgradeManager.StoreKey.existingKeyBefore_2_2_0) } diff --git a/Passepartout/App/Managers/PersistenceManager.swift b/Passepartout/App/Managers/PersistenceManager.swift index 16f908bc..64e8aa3d 100644 --- a/Passepartout/App/Managers/PersistenceManager.swift +++ b/Passepartout/App/Managers/PersistenceManager.swift @@ -23,15 +23,29 @@ // along with Passepartout. If not, see . // +import CloudKit +import Combine import CoreData import Foundation import PassepartoutLibrary -final class PersistenceManager { - private let store: KeyValueStore +final class PersistenceManager: ObservableObject { + let store: KeyValueStore + + private(set) var isCloudSyncingEnabled: Bool { + didSet { + pp_log.info("CloudKit enabled: \(isCloudSyncingEnabled)") + didChangePersistence.send() + } + } + + @Published private(set) var isErasingCloudKitStore = false + + let didChangePersistence = PassthroughSubject() init(store: KeyValueStore) { self.store = store + isCloudSyncingEnabled = store.canEnableCloudSyncing // set once if persistenceAuthor == nil { @@ -39,8 +53,8 @@ final class PersistenceManager { } } - func vpnPersistence(withName containerName: String, cloudKit: Bool) -> VPNPersistence { - VPNPersistence(withName: containerName, cloudKit: cloudKit, author: persistenceAuthor) + func vpnPersistence(withName containerName: String) -> VPNPersistence { + VPNPersistence(withName: containerName, cloudKit: isCloudSyncingEnabled, author: persistenceAuthor) } func providersPersistence(withName containerName: String) -> ProvidersPersistence { @@ -48,8 +62,59 @@ final class PersistenceManager { } } +// MARK: CloudKit + +extension PersistenceManager { + func eraseCloudKitStore() async { + await MainActor.run { + isErasingCloudKitStore = true + } + await Self.eraseCloudKitStore( + fromContainerWithId: Constants.CloudKit.containerId, + zoneId: .init(zoneName: Constants.CloudKit.coreDataZone) + ) + await MainActor.run { + isErasingCloudKitStore = false + } + } + + // WARNING: this is not running on main actor + private static func eraseCloudKitStore(fromContainerWithId containerId: String, zoneId: CKRecordZone.ID) async { + do { + let container = CKContainer(identifier: containerId) + let db = container.privateCloudDatabase + try await db.deleteRecordZone(withID: zoneId) + } catch { + pp_log.error("Unable to erase CloudKit store: \(error)") + } + } +} + // MARK: KeyValueStore +private extension KeyValueStore { + private var cloudKitToken: Any? { + FileManager.default.ubiquityIdentityToken + } + + private var isCloudKitSupported: Bool { + cloudKitToken != nil + } + + var canEnableCloudSyncing: Bool { + isCloudKitSupported && shouldEnableCloudSyncing + } + + var shouldEnableCloudSyncing: Bool { + get { + value(forLocation: PersistenceManager.StoreKey.shouldEnableCloudSyncing) ?? false + } + set { + setValue(newValue, forLocation: PersistenceManager.StoreKey.shouldEnableCloudSyncing) + } + } +} + extension PersistenceManager { private(set) var persistenceAuthor: String? { get { @@ -59,12 +124,34 @@ extension PersistenceManager { store.setValue(newValue, forLocation: StoreKey.persistenceAuthor) } } + + var shouldEnableCloudSyncing: Bool { + get { + store.shouldEnableCloudSyncing + } + set { + objectWillChange.send() + store.shouldEnableCloudSyncing = newValue + + // iCloud may be externally disabled from the device settings + let newIsCloudSyncingEnabled = store.canEnableCloudSyncing + guard newIsCloudSyncingEnabled != isCloudSyncingEnabled else { + pp_log.debug("CloudKit state did not change") + return + } + isCloudSyncingEnabled = newIsCloudSyncingEnabled + } + } } -private extension PersistenceManager { +// TODO: iCloud, restore private after dropping migration from 2.2.0 +// private extension PersistenceManager { +extension PersistenceManager { enum StoreKey: String, KeyStoreDomainLocation { case persistenceAuthor + case shouldEnableCloudSyncing + var domain: String { "Passepartout.PersistenceManager" } diff --git a/Passepartout/App/Views/SettingsView.swift b/Passepartout/App/Views/SettingsView.swift index 4c832627..903bb4ae 100644 --- a/Passepartout/App/Views/SettingsView.swift +++ b/Passepartout/App/Views/SettingsView.swift @@ -31,27 +31,18 @@ struct SettingsView: View { @ObservedObject private var productManager: ProductManager + @ObservedObject private var persistenceManager: PersistenceManager + @Environment(\.presentationMode) private var presentationMode @AppStorage(AppPreference.locksInBackground.key) private var locksInBackground = false - @Binding private var shouldEnableCloudSyncing: Bool - - @State private var isErasingCloudStore = false - private let versionString = Constants.Global.appVersionString init() { profileManager = .shared productManager = .shared - - _shouldEnableCloudSyncing = .init { - AppContext.shared.shouldEnableCloudSyncing - } set: { isEnabled in - withAnimation { - AppContext.shared.shouldEnableCloudSyncing = isEnabled - } - } + persistenceManager = .shared } var body: some View { @@ -82,15 +73,13 @@ private extension SettingsView { var iCloudSection: some View { Section { - Toggle(L10n.Settings.Items.ShouldEnableCloudSyncing.caption, isOn: $shouldEnableCloudSyncing) + Toggle(L10n.Settings.Items.ShouldEnableCloudSyncing.caption, isOn: $persistenceManager.shouldEnableCloudSyncing.themeAnimation()) Button(L10n.Settings.Items.EraseCloudStore.caption) { - isErasingCloudStore = true Task { - await AppContext.shared.eraseCloudKitStore() - isErasingCloudStore = false + await persistenceManager.eraseCloudKitStore() } - }.withTrailingProgress(when: isErasingCloudStore) - .disabled(shouldEnableCloudSyncing || isErasingCloudStore) + }.withTrailingProgress(when: persistenceManager.isErasingCloudKitStore) + .disabled(persistenceManager.shouldEnableCloudSyncing || persistenceManager.isErasingCloudKitStore) } header: { Text(Unlocalized.Other.iCloud) } footer: { diff --git a/Passepartout/Mac/Views/LaunchOnLoginItem+ViewModel.swift b/Passepartout/Mac/Views/LaunchOnLoginItem+ViewModel.swift index 0fc2b7cd..f1212404 100644 --- a/Passepartout/Mac/Views/LaunchOnLoginItem+ViewModel.swift +++ b/Passepartout/Mac/Views/LaunchOnLoginItem+ViewModel.swift @@ -43,8 +43,8 @@ extension LaunchOnLoginItem { guard SMLoginItemSetEnabled(Constants.Mac.appLauncherId as CFString, newValue) else { return } - launchesOnLogin = newValue objectWillChange.send() + launchesOnLogin = newValue } } diff --git a/PassepartoutLibrary/Sources/PassepartoutVPNImpl/Data/VPNPersistence.swift b/PassepartoutLibrary/Sources/PassepartoutVPNImpl/Data/VPNPersistence.swift index 0855ad88..89eeb61c 100644 --- a/PassepartoutLibrary/Sources/PassepartoutVPNImpl/Data/VPNPersistence.swift +++ b/PassepartoutLibrary/Sources/PassepartoutVPNImpl/Data/VPNPersistence.swift @@ -23,7 +23,6 @@ // along with Passepartout. If not, see . // -import CloudKit import CoreData import Foundation import PassepartoutCore @@ -55,15 +54,4 @@ public final class VPNPersistence { public func profileRepository() -> ProfileRepository { CDProfileRepository(store.context) } - - // WARNING: this is not running on main actor - public func eraseCloudKitStore(fromContainerWithId containerId: String, zoneId: CKRecordZone.ID) async { - do { - let container = CKContainer(identifier: containerId) - let db = container.privateCloudDatabase - try await db.deleteRecordZone(withID: zoneId) - } catch { - pp_log.error("Unable to erase CloudKit store: \(error)") - } - } }