Fix regressions with CloudKit synchronization (#1029)

The remote container is shared by ProfileManager and
PreferencesManager, but it must be the same for CloudKit sync
to work properly.

Externalize the logic of onEligibleFeatures() so that the
AppContext singleton can update the managers (and their
repositories) with the new remote store.

Now that the remote profile repository is reloaded every time that
eligible features change, the .removeDuplicates() may also be
restored. Just add a .dropFirst() to skip the initially empty
value of eligible features. Even when features are eventually empty,
a value is always emitted after IAPManager.reloadReceipt()

Lastly, enable Core Data lightweight migration.

Regressions from #1017

Library/Sources/AppUIMain/Views/App/ProfileRowView.swift

@@ -212,7 +212,7 @@ private extension ProfileRowView {
     }
     .task {
         do {
-            try await profileManager.observeRemote(true)
+            try await profileManager.observeRemote(repository: InMemoryProfileRepository())
             try await profileManager.save(profile, isLocal: true, remotelyShared: true)
         } catch {
             fatalError(error.localizedDescription)

Library/Sources/CommonLibrary/Business/PreferencesManager.swift

@@ -29,30 +29,27 @@ import PassepartoutKit
 
 @MainActor
 public final class PreferencesManager: ObservableObject {
-    private let modulesFactory: (UUID) throws -> ModulePreferencesRepository
+    public var modulesRepositoryFactory: (UUID) throws -> ModulePreferencesRepository
 
-    private let providersFactory: (ProviderID) throws -> ProviderPreferencesRepository
+    public var providersRepositoryFactory: (ProviderID) throws -> ProviderPreferencesRepository
 
-    public init(
-        modulesFactory: ((UUID) throws -> ModulePreferencesRepository)? = nil,
-        providersFactory: ((ProviderID) throws -> ProviderPreferencesRepository)? = nil
-    ) {
-        self.modulesFactory = modulesFactory ?? { _ in
+    public init() {
+        modulesRepositoryFactory = { _ in
             DummyModulePreferencesRepository()
         }
-        self.providersFactory = providersFactory ?? { _ in
+        providersRepositoryFactory = { _ in
             DummyProviderPreferencesRepository()
         }
     }
 }
 
 extension PreferencesManager {
     public func preferencesRepository(forModuleWithId moduleId: UUID) throws -> ModulePreferencesRepository {
-        try modulesFactory(moduleId)
+        try modulesRepositoryFactory(moduleId)
     }
 
     public func preferencesRepository(forProviderWithId providerId: ProviderID) throws -> ProviderPreferencesRepository {
-        try providersFactory(providerId)
+        try providersRepositoryFactory(providerId)
     }
 }
 

Library/Sources/CommonLibrary/Business/ProfileManager.swift

@@ -59,8 +59,6 @@ public final class ProfileManager: ObservableObject {
 
     private let backupRepository: ProfileRepository?
 
-    private let remoteRepositoryBlock: ((Bool) -> ProfileRepository)?
-
     private var remoteRepository: ProfileRepository?
 
     private let mirrorsRemoteRepository: Bool
@@ -94,7 +92,7 @@ public final class ProfileManager: ObservableObject {
     private var requiredFeatures: [Profile.ID: Set<AppFeature>]
 
     @Published
-    public private(set) var isRemoteImportingEnabled: Bool
+    public var isRemoteImportingEnabled = false
 
     private var waitingObservers: Set<Observer> {
         didSet {
@@ -120,34 +118,25 @@ public final class ProfileManager: ObservableObject {
 
     // for testing/previews
     public convenience init(profiles: [Profile]) {
-        self.init(
-            repository: InMemoryProfileRepository(profiles: profiles),
-            remoteRepositoryBlock: { _ in
-                InMemoryProfileRepository()
-            }
-        )
+        self.init(repository: InMemoryProfileRepository(profiles: profiles))
     }
 
     public init(
+        processor: ProfileProcessor? = nil,
         repository: ProfileRepository,
         backupRepository: ProfileRepository? = nil,
-        remoteRepositoryBlock: ((Bool) -> ProfileRepository)?,
-        mirrorsRemoteRepository: Bool = false,
-        processor: ProfileProcessor? = nil
+        mirrorsRemoteRepository: Bool = false
     ) {
-        precondition(!mirrorsRemoteRepository || remoteRepositoryBlock != nil, "mirrorsRemoteRepository requires a non-nil remoteRepositoryBlock")
+        self.processor = processor
         self.repository = repository
         self.backupRepository = backupRepository
-        self.remoteRepositoryBlock = remoteRepositoryBlock
         self.mirrorsRemoteRepository = mirrorsRemoteRepository
-        self.processor = processor
 
         allProfiles = [:]
         allRemoteProfiles = [:]
         filteredProfiles = []
         requiredFeatures = [:]
-        isRemoteImportingEnabled = false
-        if remoteRepositoryBlock != nil {
+        if mirrorsRemoteRepository {
             waitingObservers = [.local, .remote]
         } else {
             waitingObservers = [.local]
@@ -341,24 +330,13 @@ extension ProfileManager {
             }
     }
 
-    public func observeRemote(_ isRemoteImportingEnabled: Bool) async throws {
-        guard let remoteRepositoryBlock else {
-//            preconditionFailure("Missing remoteRepositoryBlock")
-            return
-        }
-        guard remoteRepository == nil || isRemoteImportingEnabled != self.isRemoteImportingEnabled else {
-            return
-        }
-
-        self.isRemoteImportingEnabled = isRemoteImportingEnabled
-
+    public func observeRemote(repository: ProfileRepository) async throws {
         remoteSubscription = nil
-        let newRepository = remoteRepositoryBlock(isRemoteImportingEnabled)
-        let initialProfiles = try await newRepository.fetchProfiles()
+        remoteRepository = repository
+        let initialProfiles = try await repository.fetchProfiles()
         reloadRemoteProfiles(initialProfiles)
-        remoteRepository = newRepository
 
-        remoteSubscription = remoteRepository?
+        remoteSubscription = repository
             .profilesPublisher
             .dropFirst()
             .receive(on: DispatchQueue.main)
@@ -422,6 +400,7 @@ private extension ProfileManager {
         if waitingObservers.contains(.remote) {
             waitingObservers.remove(.remote)
         }
+
         Task { [weak self] in
             self?.didChange.send(.startRemoteImport)
             await self?.importRemoteProfiles(result)

Library/Sources/CommonUtils/Business/CoreDataPersistentStore.swift

@@ -90,6 +90,10 @@ public final class CoreDataPersistentStore: Sendable {
         // container was formerly created with CloudKit option
         desc.setOption(true as NSNumber, forKey: NSPersistentHistoryTrackingKey)
 
+        // migrate automatically
+        desc.setOption(true as NSNumber, forKey: NSMigratePersistentStoresAutomaticallyOption)
+        desc.setOption(true as NSNumber, forKey: NSInferMappingModelAutomaticallyOption)
+
         // report remote notifications (do this BEFORE loadPersistentStores)
         //
         // https://stackoverflow.com/a/69507329/784615

Library/Sources/CommonUtils/Business/CoreDataRepository.swift

@@ -180,20 +180,19 @@ private extension CoreDataRepository {
         request.predicate = predicate
         beforeFetch?(request)
 
-        let newController = try await context.perform {
-            let newController = NSFetchedResultsController(
-                fetchRequest: request,
-                managedObjectContext: self.context,
-                sectionNameKeyPath: nil,
-                cacheName: nil
-            )
-            newController.delegate = self
+        let newController = NSFetchedResultsController(
+            fetchRequest: request,
+            managedObjectContext: self.context,
+            sectionNameKeyPath: nil,
+            cacheName: nil
+        )
+        newController.delegate = self
+        resultsController = newController
+
+        return try await context.perform {
             try newController.performFetch()
-            return newController
+            return self.unsafeSendResults(from: newController)
         }
-
-        resultsController = newController
-        return await sendResults(from: newController)
     }
 
     @discardableResult

Library/Sources/UILibrary/Business/AppContext.swift

@@ -48,6 +48,8 @@ public final class AppContext: ObservableObject, Sendable {
 
     private let tunnelReceiptURL: URL?
 
+    private let onEligibleFeaturesBlock: ((Set<AppFeature>) async -> Void)?
+
     private var launchTask: Task<Void, Error>?
 
     private var pendingTask: Task<Void, Never>?
@@ -62,7 +64,8 @@ public final class AppContext: ObservableObject, Sendable {
         preferencesManager: PreferencesManager,
         registry: Registry,
         tunnel: ExtendedTunnel,
-        tunnelReceiptURL: URL?
+        tunnelReceiptURL: URL?,
+        onEligibleFeaturesBlock: ((Set<AppFeature>) async -> Void)? = nil
     ) {
         self.iapManager = iapManager
         self.migrationManager = migrationManager
@@ -72,6 +75,7 @@ public final class AppContext: ObservableObject, Sendable {
         self.registry = registry
         self.tunnel = tunnel
         self.tunnelReceiptURL = tunnelReceiptURL
+        self.onEligibleFeaturesBlock = onEligibleFeaturesBlock
         subscriptions = []
     }
 }
@@ -99,22 +103,16 @@ private extension AppContext {
         pp_log(.App.profiles, .info, "\tObserve in-app events...")
         iapManager.observeObjects()
 
-        // load in background, see comment right below
+        // defer load receipt
         Task {
             await iapManager.reloadReceipt()
         }
 
-        // using Task above (#1019) causes the receipt to be loaded asynchronously.
-        // the initial call to onEligibleFeatures() may execute before the receipt is
-        // loaded and therefore do nothing. with .removeDuplicates(), there would
-        // not be a second chance to call onEligibleFeatures() if the eligible
-        // features haven't changed after reloading the receipt (this is the case
-        // for TestFlight where some features are set statically). that's why it's
-        // commented now
         pp_log(.App.profiles, .info, "\tObserve eligible features...")
         iapManager
             .$eligibleFeatures
-//            .removeDuplicates()
+            .dropFirst()
+            .removeDuplicates()
             .sink { [weak self] eligible in
                 Task {
                     try await self?.onEligibleFeatures(eligible)
@@ -184,19 +182,7 @@ private extension AppContext {
 
         pp_log(.app, .notice, "Application did update eligible features")
         pendingTask = Task {
-
-            // toggle sync based on .sharing eligibility
-            let isEligibleForSharing = features.contains(.sharing)
-            do {
-                pp_log(.App.profiles, .info, "\tRefresh remote profiles observers (eligible=\(isEligibleForSharing), CloudKit=\(isCloudKitEnabled))...")
-                try await profileManager.observeRemote(isEligibleForSharing && isCloudKitEnabled)
-            } catch {
-                pp_log(.App.profiles, .error, "\tUnable to re-observe remote profiles: \(error)")
-            }
-
-            // refresh required profile features
-            pp_log(.App.profiles, .info, "\tReload profiles required features...")
-            profileManager.reloadRequiredFeatures()
+            await onEligibleFeaturesBlock?(features)
         }
         await pendingTask?.value
         pendingTask = nil
@@ -262,18 +248,3 @@ private extension AppContext {
         return didLaunch
     }
 }
-
-// MARK: - Helpers
-
-private extension AppContext {
-    var isCloudKitEnabled: Bool {
-#if os(tvOS)
-        true
-#else
-        if AppCommandLine.contains(.uiTesting) {
-            return true
-        }
-        return FileManager.default.ubiquityIdentityToken != nil
-#endif
-    }
-}