From 0c003c71cce26053eebec3ade3149b719e4e09bf Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Tue, 25 Aug 2026 23:56:58 +0200 Subject: [PATCH] Sync accounts through CloudKit records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A login created after the revamp got a random account UUID that never left the device, so its credential was keyed to an ID no other device had and the login was unusable there. Migrated accounts were fine only because their ID is deterministically the instance ID. Accounts, the permanent legacy-migration markers, the permanent deletion tombstones and the selected data context now ride the existing CKSyncEngine as four record types, with the rules the category needs: - Absence proves nothing. A fetch resumes from a change token and never enumerates the zone, so deletions are records of their own and they dominate the account wherever they are seen — including the engine's retry-as-insert path, which is how a queued offline edit would otherwise resurrect a deleted account. - Fetched records are staged durably before each delegate event returns and applied as a set, tombstones first, gated on the zone's own result rather than didFetchChanges, which fires after failed fetches too. - Nothing activates for an identity that has not bootstrapped: a device left on the previous release consumed these record types as unknown and advanced its token past them, so each identity fetches the zone once from a nil token, outside the engine's own token. - A disabled category defers records rather than discarding them; every persisted artifact is namespaced by identity, and async work is generation-checked so a fetch begun under one Apple account can never land in another's state. - Uploads are ordered: a marker never outruns the account it describes, and a context naming an account is never published before that account is server-confirmed. --- Yattee/Core/AccountsManager.swift | 263 ++++++- Yattee/Core/AppEnvironment.swift | 22 + .../CloudKit/AccountSyncCoordinator.swift | 714 ++++++++++++++++++ .../Services/CloudKit/AccountSyncStore.swift | 161 ++++ .../CloudKit/CloudKitConflictResolver.swift | 69 +- .../CloudKit/CloudKitRecordMapper.swift | 216 ++++++ .../CloudKit/CloudKitSyncEngine.swift | 84 ++- Yattee/Services/CloudKit/RecordTypes.swift | 101 ++- .../Views/Settings/iCloudSettingsView.swift | 7 + YatteeTests/AccountSyncTests.swift | 693 +++++++++++++++++ 10 files changed, 2306 insertions(+), 24 deletions(-) create mode 100644 Yattee/Services/CloudKit/AccountSyncCoordinator.swift create mode 100644 Yattee/Services/CloudKit/AccountSyncStore.swift create mode 100644 YatteeTests/AccountSyncTests.swift diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 1b25a2b1..64bcc1f9 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -58,6 +58,23 @@ final class AccountsManager { private(set) var pendingUploads: Set = [] private(set) var pendingDeletions: Set = [] + /// Instances whose migration marker still has to reach the server. + /// + /// Tracked apart from `migratedInstanceIDs` because the marker is held back + /// until the account it describes is confirmed: a marker that outran a + /// silently failed account save would suppress synthesis everywhere for an + /// account no device has. + private(set) var pendingMarkerUploads: Set = [] + + /// Whether the selected context still has to reach the server. + private(set) var pendingContextUpload = false + + /// Accounts known to be deleted. Permanent, and the only thing that + /// authorises removing an account that arrives from sync: a record's + /// *absence* proves nothing, because a fetch resumes from a change token + /// rather than enumerating the zone. + private(set) var deletedAccountIDs: Set = [] + var currentContext: DataContext { storedContext.context } // MARK: - Initialization @@ -154,18 +171,32 @@ final class AccountsManager { // `usesAccountLogin`, which posts instancesDidChange and re-runs // reconciliation immediately — without this the reconciliation would // synthesize a second, credential-less account for the same instance. - migratedInstanceIDs.insert(instanceID) + markMigrated(instanceID) save() + NotificationCenter.default.post(name: .accountsDidChange, object: nil) return account } + /// Records that this instance's legacy login is now represented by an + /// account, and that the fact has to be published. + /// + /// Publishing matters even when the account was created by a fresh sign-in + /// rather than by synthesis: without the marker, every other device would + /// still synthesize its own credential-less account for the same instance. + private func markMigrated(_ instanceID: UUID) { + guard !migratedInstanceIDs.contains(instanceID) else { return } + migratedInstanceIDs.insert(instanceID) + pendingMarkerUploads.insert(instanceID) + } + func updateDisplayName(_ displayName: String?, for accountID: UUID) { guard let index = accounts.firstIndex(where: { $0.id == accountID }) else { return } accounts[index].displayName = displayName accounts[index].updatedAt = Date() pendingUploads.insert(accountID) save() + NotificationCenter.default.post(name: .accountsDidChange, object: nil) } /// Removes an account and everything keyed to it. @@ -174,24 +205,35 @@ final class AccountsManager { /// crash mid-way still replays it — and so a peer that recreates the /// account from a stale queued edit is corrected rather than obeyed. func removeAccount(_ accountID: UUID) { - guard let account = account(id: accountID) else { return } + guard account(id: accountID) != nil else { return } - accounts.removeAll { $0.id == accountID } - accountStatuses[accountID] = nil - pendingUploads.remove(accountID) + forgetAccount(accountID) pendingDeletions.insert(accountID) - // Selecting a deleted account would leave the UI pointing at nothing. - // This is an explicit local deletion, which is one of the few things - // allowed to correct the context. - if storedContext.context == .account(accountID) { - setContext(.local) - } - save() NotificationCenter.default.post(name: .accountsDidChange, object: nil) - _ = account + } + + /// Drops an account and everything keyed to it, and records the permanent + /// local tombstone. + /// + /// Shared by explicit local deletion and by deletions arriving from sync, so + /// that both leave exactly the same state behind — including the tombstone, + /// which is what stops a stale device's queued edit from resurrecting the + /// account here. + private func forgetAccount(_ accountID: UUID) { + accounts.removeAll { $0.id == accountID } + accountStatuses[accountID] = nil + pendingUploads.remove(accountID) + deletedAccountIDs.insert(accountID) + + // Selecting a deleted account would leave the UI pointing at nothing. + // A deletion the app can point at is one of the few things allowed to + // correct the context. + if storedContext.context == .account(accountID) { + setContext(.local) + } } /// Marks a deletion as confirmed by the server. Until then it stays in the @@ -202,11 +244,153 @@ final class AccountsManager { save() } - func confirmUpload(_ accountID: UUID) { + /// Marks an account upload as confirmed by the server. + /// + /// `updatedAt` is the timestamp of the record that was actually sent: an + /// edit made between the record being built and the server acknowledging it + /// leaves the account dirty, or that edit would never be uploaded. + func confirmUpload(_ accountID: UUID, updatedAt: Date? = nil) { + if let updatedAt, let account = account(id: accountID), account.updatedAt > updatedAt { + return + } pendingUploads.remove(accountID) save() } + func confirmMarkerUpload(instanceID: UUID) { + pendingMarkerUploads.remove(instanceID) + save() + } + + /// Marks the context upload as confirmed, unless the user has chosen a + /// different context since the uploaded record was built. + func confirmContextUpload(changedAt: Date) { + guard storedContext.changedAt <= changedAt else { return } + pendingContextUpload = false + save() + } + + func isDeleted(_ accountID: UUID) -> Bool { + deletedAccountIDs.contains(accountID) + } + + /// Marks every account record dirty again. + /// + /// For a server that holds nothing — sync being enabled for the first time, + /// or the zone having been deleted from another device. The outbox + /// otherwise tracks only what changed, which says nothing about records the + /// server never received. Tombstones are included: without them a device + /// that still has a deleted account could put it back. + func markAllRecordsForUpload() { + pendingUploads.formUnion(accounts.map(\.id)) + pendingMarkerUploads.formUnion(migratedInstanceIDs) + pendingDeletions.formUnion(deletedAccountIDs) + pendingContextUpload = hasStoredContext + save() + } + + // MARK: - Upload ordering + + /// Whether an instance's marker may be published yet. + /// + /// Held while any account on that instance is still waiting to upload — see + /// `pendingMarkerUploads`. + func canUploadMarker(instanceID: UUID) -> Bool { + !accounts(forInstance: instanceID).contains { pendingUploads.contains($0.id) } + } + + /// Whether the selected context may be published yet. + /// + /// A context naming an account the server has never seen would let a fresh + /// device fetch the selection, find no such account, and treat a merely + /// pending account as removed. + var canUploadContext: Bool { + guard case let .account(id) = storedContext.context else { return true } + return !pendingUploads.contains(id) + } + + // MARK: - Applying synced state + + /// Applies an account fetched from another device. + /// + /// - Returns: `true` when the local copy is the newer one and has to be + /// pushed back, since the server still holds the older version. + @discardableResult + func applySyncedAccount(_ incoming: Account) -> Bool { + // A tombstone dominates the account everywhere. The caller repairs the + // server copy; here it simply never becomes local state again. + guard !deletedAccountIDs.contains(incoming.id) else { return false } + + if let index = accounts.firstIndex(where: { $0.id == incoming.id }) { + let existing = accounts[index] + guard incoming.updatedAt > existing.updatedAt else { + guard existing.updatedAt > incoming.updatedAt else { return false } + // The server holds the older copy. Marked dirty rather than + // merely re-registered with the engine, so the correction + // survives a crash before it is sent. + pendingUploads.insert(existing.id) + save() + return true + } + accounts[index] = incoming + } else { + accounts.append(incoming) + } + + save() + NotificationCenter.default.post(name: .accountsDidChange, object: nil) + return false + } + + /// Applies a deletion tombstone fetched from another device. + /// + /// The tombstone's presence on the server also settles our own outbox: an + /// entry still queued here would only re-save a record that already exists. + @discardableResult + func applyRemoteTombstone(_ accountID: UUID) -> Account? { + let removed = account(id: accountID) + forgetAccount(accountID) + pendingDeletions.remove(accountID) + save() + + NotificationCenter.default.post(name: .accountsDidChange, object: nil) + return removed + } + + /// Applies a deletion *event* — the account record itself disappeared. + /// + /// Authorises removal just like a tombstone, but leaves the tombstone + /// queued: a bare record deletion is not durable, and the account would come + /// back the moment a device that never fetched this event flushes a queued + /// edit. + @discardableResult + func applyRemoteAccountDeletionEvent(_ accountID: UUID) -> Account? { + let removed = account(id: accountID) + forgetAccount(accountID) + pendingDeletions.insert(accountID) + save() + + NotificationCenter.default.post(name: .accountsDidChange, object: nil) + return removed + } + + /// Applies a legacy-migration marker fetched from another device. + /// + /// The marker only ever *prevents* synthesis. It never retracts an account + /// already synthesized here — an account record may simply be in a later + /// fetch event, or beyond the incremental window. + func applySyncedMigrationMarker(instanceID: UUID) { + let isNew = !migratedInstanceIDs.contains(instanceID) + migratedInstanceIDs.insert(instanceID) + // Already on the server, so ours is redundant. + pendingMarkerUploads.remove(instanceID) + save() + + if isNew { + NotificationCenter.default.post(name: .accountsDidChange, object: nil) + } + } + // MARK: - Context /// Resolves a legacy instance-keyed selection onto a real account context. @@ -230,6 +414,7 @@ final class AccountsManager { func setContext(_ context: DataContext) { storedContext = StoredDataContext(context: context) hasStoredContext = true + pendingContextUpload = true save() NotificationCenter.default.post(name: .dataContextDidChange, object: nil) } @@ -237,12 +422,24 @@ final class AccountsManager { /// Applies a context that arrived from another device, honouring the /// app-managed timestamp: an older value never overwrites a newer one, so a /// migration write can't undo a deliberate choice made elsewhere. - func applySyncedContext(_ incoming: StoredDataContext) { - guard incoming.changedAt > storedContext.changedAt else { return } + /// + /// - Returns: `true` when the local choice is the newer one and has to be + /// pushed back over the stale server copy. + @discardableResult + func applySyncedContext(_ incoming: StoredDataContext) -> Bool { + guard incoming.changedAt > storedContext.changedAt else { + // Nothing to push back before the user has ever chosen: the local + // value is a default, not a decision. + guard hasStoredContext, storedContext.changedAt > incoming.changedAt else { return false } + pendingContextUpload = true + save() + return true + } storedContext = incoming hasStoredContext = true save() NotificationCenter.default.post(name: .dataContextDidChange, object: nil) + return false } // MARK: - Status @@ -276,17 +473,28 @@ final class AccountsManager { guard instance.type.accountCapabilities.isSelectableContext else { continue } guard !migratedInstanceIDs.contains(instance.id) else { continue } + // A synthesized account reuses the instance's UUID, so a tombstone + // for that UUID is this instance's migration already having been + // undone. It can arrive before the marker record does — both are + // permanent, but they are separate records — so the tombstone has + // to gate synthesis in its own right. + guard !deletedAccountIDs.contains(instance.id) else { + markMigrated(instance.id) + didChange = true + continue + } + // Any account on this instance means its login is already // represented — whether by an earlier migration or by a fresh // sign-in. Synthesizing beside it would leave a credential-less // duplicate. guard accounts(forInstance: instance.id).isEmpty else { - migratedInstanceIDs.insert(instance.id) + markMigrated(instance.id) didChange = true continue } - migratedInstanceIDs.insert(instance.id) + markMigrated(instance.id) didChange = true let account = Account.migrated(from: instance.id, dateAdded: instance.dateAdded) @@ -326,6 +534,9 @@ final class AccountsManager { storedContext = .migrated(context) hasStoredContext = true + // Published like any other context, but stamped at the epoch: if a real + // choice exists anywhere it outranks this one whichever arrives first. + pendingContextUpload = true save() LoggingService.shared.info( @@ -392,6 +603,16 @@ final class AccountsManager { if let ids = defaults.array(forKey: key("pendingDeletions")) as? [String] { pendingDeletions = Set(ids.compactMap(UUID.init(uuidString:))) } + + if let ids = defaults.array(forKey: key("pendingMarkerUploads")) as? [String] { + pendingMarkerUploads = Set(ids.compactMap(UUID.init(uuidString:))) + } + + if let ids = defaults.array(forKey: key("deletedAccounts")) as? [String] { + deletedAccountIDs = Set(ids.compactMap(UUID.init(uuidString:))) + } + + pendingContextUpload = defaults.bool(forKey: key("pendingContextUpload")) } private func save() { @@ -404,6 +625,9 @@ final class AccountsManager { defaults.set(migratedInstanceIDs.map(\.uuidString), forKey: key("migrated")) defaults.set(pendingUploads.map(\.uuidString), forKey: key("pendingUploads")) defaults.set(pendingDeletions.map(\.uuidString), forKey: key("pendingDeletions")) + defaults.set(pendingMarkerUploads.map(\.uuidString), forKey: key("pendingMarkerUploads")) + defaults.set(deletedAccountIDs.map(\.uuidString), forKey: key("deletedAccounts")) + defaults.set(pendingContextUpload, forKey: key("pendingContextUpload")) } /// Re-reads storage under the current identity namespace. @@ -417,6 +641,9 @@ final class AccountsManager { migratedInstanceIDs = [] pendingUploads = [] pendingDeletions = [] + pendingMarkerUploads = [] + deletedAccountIDs = [] + pendingContextUpload = false load() reconcileLegacyAccounts() diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 8f3622a2..2463942f 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -52,6 +52,7 @@ final class AppEnvironment { /// resolves the scope for all of them. let credentialIdentityScope: CredentialIdentityScope let accountsManager: AccountsManager + let accountSyncCoordinator: AccountSyncCoordinator let homeInstanceCache: HomeInstanceCache let invidiousAPI: InvidiousAPI let pipedAPI: PipedAPI @@ -186,6 +187,17 @@ final class AppEnvironment { self.cloudKitSync = cloudKit dm.cloudKitSync = cloudKit + // Accounts sync as CloudKit records rather than through KVS: they need + // per-record conflict resolution and durable deletions, neither of + // which a single overwritten key-value document can provide. + let accountSync = AccountSyncCoordinator( + accountsManager: accounts, + identityScope: identityScope, + isCategoryEnabled: { [weak cloudKit] in cloudKit?.canSyncAccounts ?? false } + ) + self.accountSyncCoordinator = accountSync + cloudKit.accountSync = accountSync + // Initialize DeArrow with low-priority networking let lowPrioritySession = URLSessionFactory.shared.lowPrioritySession() let deArrowHTTPClient = HTTPClient(session: lowPrioritySession) @@ -367,6 +379,16 @@ final class AppEnvironment { subscriptionService.setContextValidator(dataContextValidator) subscriptionAccountValidator.setAccountsManager(accounts) + // A deletion that arrives from another device takes the credential with + // it. Only a deletion does this — an account merely missing from a + // fetch keeps everything, because a fetch is not an enumeration. + accountSync.onAccountDeleted = { [weak self] account in + guard let self else { return } + let credentials = self.accountCredentialsManager(for: account) + credentials?.deleteCredential(for: account) + credentials?.deleteLegacyCredentialIfOwned(for: account) + } + // Statuses are cleared whenever the identity namespace reloads, so they // have to be recomputed once the new scope's Keychain is readable — // otherwise a synced account shows "sign in required" until relaunch. diff --git a/Yattee/Services/CloudKit/AccountSyncCoordinator.swift b/Yattee/Services/CloudKit/AccountSyncCoordinator.swift new file mode 100644 index 00000000..df21de98 --- /dev/null +++ b/Yattee/Services/CloudKit/AccountSyncCoordinator.swift @@ -0,0 +1,714 @@ +// +// AccountSyncCoordinator.swift +// Yattee +// +// Syncs accounts, migration markers, deletion tombstones and the selected data +// context through the existing CKSyncEngine. +// + +import CloudKit +import Foundation + +/// Drives the account category of CloudKit sync. +/// +/// Accounts are not SwiftData models, so they do not fit the engine's +/// model-mapping path; and they carry rules the other categories do not: +/// +/// - **Absence proves nothing.** A fetch resumes from a change token and never +/// enumerates the zone, so "no account record arrived" can never mean the +/// account was deleted. Deletions are therefore permanent `AccountDeletion` +/// records, and they dominate the account everywhere they are seen. +/// - **Staging is durable and applied as a set**, tombstones first, only once +/// the zone fetch has actually succeeded — see ``AccountSyncStore``. +/// - **Nothing activates for an identity that has not bootstrapped.** A device +/// left on the previous release consumed these record types as unknown and +/// advanced its token past them, so the first launch that understands them +/// fetches the zone once from a nil token, outside the engine's own token. +/// - **Uploads are ordered.** A marker never outruns the account it describes, +/// and a context naming an account is never published before that account is. +@MainActor +final class AccountSyncCoordinator { + typealias EnqueueChanges = @MainActor ([CKSyncEngine.PendingRecordZoneChange]) -> Void + + // MARK: - Dependencies + + private let accountsManager: AccountsManager + + /// The identity every persisted artifact is keyed by, or `nil` while it is + /// still being resolved. + private let namespaceProvider: @MainActor () -> String? + + private let store: AccountSyncStore + private let mapper: CloudKitRecordMapper + private let conflictResolver: CloudKitConflictResolver + private let zone: CKRecordZone + + /// Whether the accounts category may activate and upload. Accounts ride the + /// instances category (`syncInstances`) under the iCloud master toggle. + private let isCategoryEnabled: @MainActor () -> Bool + + /// Registers changes with the engine. `nil` while no engine exists — the + /// dirty outbox in `AccountsManager` is the durable record, so nothing is + /// lost by not having one. + private var enqueueChanges: EnqueueChanges? + + /// The private database, for the bootstrap fetch that runs outside the + /// engine's change token. + private weak var database: CKDatabase? + + /// Called when an account is deleted anywhere, so its credential and stored + /// login identifier go with it. + var onAccountDeleted: (@MainActor (Account) -> Void)? + + // MARK: - Identity + + /// Bumped on every observed change of identity. Async work captures it and + /// refuses to persist or activate results if it has moved — a fetch begun + /// under one Apple account must never land in another's state. + private var identityGeneration = 0 + + /// The last identity that was actually resolved, which a later `nil` does + /// not clear — it is the other candidate owner of anything fetched while + /// no identity was resolved. + private var lastResolvedNamespace: String? + + /// Set when account records had to be dropped because no identity was + /// resolved yet. The engine's token has already moved past them, so the + /// identity that resolves next has to bootstrap again to recover them. + private var droppedRecordsWhileUnresolved = false + + private var isActivating = false + private var activationRequested = false + + private nonisolated(unsafe) var identityObserver: NSObjectProtocol? + private nonisolated(unsafe) var accountsObserver: NSObjectProtocol? + private nonisolated(unsafe) var contextObserver: NSObjectProtocol? + + /// The identity everything is namespaced by, or `nil` while unresolved. + private var namespace: String? { namespaceProvider() } + + // MARK: - Initialization + + convenience init( + accountsManager: AccountsManager, + identityScope: CredentialIdentityScope, + store: AccountSyncStore? = nil, + conflictResolver: CloudKitConflictResolver = CloudKitConflictResolver(), + zone: CKRecordZone = RecordType.createZone(), + isCategoryEnabled: @escaping @MainActor () -> Bool + ) { + // Accounts share the credential identity namespace: the account list + // and the credentials it points at must always describe the same Apple + // account. Held strongly — the scope is a shared service that knows + // nothing about this coordinator, so there is no cycle to break, and a + // weak reference would silently turn every namespaced operation into a + // no-op if the scope were ever the shorter-lived of the two. + self.init( + accountsManager: accountsManager, + namespaceProvider: { identityScope.current }, + store: store, + conflictResolver: conflictResolver, + zone: zone, + isCategoryEnabled: isCategoryEnabled + ) + } + + /// Designated initializer, and the seam tests use to move the identity + /// deterministically instead of racing a real scope resolution. + init( + accountsManager: AccountsManager, + namespaceProvider: @escaping @MainActor () -> String?, + store: AccountSyncStore? = nil, + conflictResolver: CloudKitConflictResolver = CloudKitConflictResolver(), + zone: CKRecordZone = RecordType.createZone(), + isCategoryEnabled: @escaping @MainActor () -> Bool + ) { + self.accountsManager = accountsManager + self.namespaceProvider = namespaceProvider + self.store = store ?? AccountSyncStore() + self.conflictResolver = conflictResolver + self.zone = zone + self.mapper = CloudKitRecordMapper(zone: zone) + self.isCategoryEnabled = isCategoryEnabled + self.lastResolvedNamespace = namespaceProvider() + + identityObserver = NotificationCenter.default.addObserver( + forName: .credentialIdentityScopeDidChange, + object: nil, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.identityDidChange() } + } + + // Local edits mark themselves dirty in AccountsManager; these turn that + // into engine registrations without every call site knowing about sync. + accountsObserver = NotificationCenter.default.addObserver( + forName: .accountsDidChange, + object: nil, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.enqueueDirtyRecords() } + } + + contextObserver = NotificationCenter.default.addObserver( + forName: .dataContextDidChange, + object: nil, + queue: .main + ) { [weak self] _ in + MainActor.assumeIsolated { self?.enqueueDirtyRecords() } + } + } + + deinit { + for observer in [identityObserver, accountsObserver, contextObserver] { + if let observer { NotificationCenter.default.removeObserver(observer) } + } + } + + /// Handles the identity moving under a running app. + /// + /// Nothing is migrated between namespaces here: the previous identity's + /// staging, outbox and bootstrap flag stay where they are, unread, and the + /// new identity starts from its own. + /// + /// Internal so tests can drive it directly rather than racing a real scope + /// resolution. + func identityDidChange() { + identityGeneration += 1 + + guard let current = namespace else { return } + let previouslyResolved = lastResolvedNamespace + lastResolvedNamespace = current + + // Records dropped for want of a resolved identity belonged to whichever + // Apple account was signed in at the time, and that cannot be + // reconstructed afterwards: a first launch and a sign-out/sign-in look + // identical from here. Rather than guess, every identity that could have + // owned them bootstraps again and recovers its own from its own zone. A + // redundant full fetch is the whole cost of that; guessing wrongly would + // show one Apple account's logins under another's. + if droppedRecordsWhileUnresolved { + droppedRecordsWhileUnresolved = false + + for candidate in Set([current, previouslyResolved].compactMap { $0 }) { + store.clearBootstrapCompletion(in: candidate) + } + + LoggingService.shared.logCloudKit( + "Account records arrived before the identity resolved — bootstrapping again to recover them" + ) + } + + Task { await activate() } + } + + // MARK: - Engine attachment + + /// Connects the coordinator to a live sync engine. + func attach(database: CKDatabase?, enqueueChanges: @escaping EnqueueChanges) { + self.database = database + self.enqueueChanges = enqueueChanges + } + + /// Disconnects when the engine is torn down. The durable state — staging, + /// outbox, bootstrap flag — is deliberately left untouched. + func detach() { + enqueueChanges = nil + database = nil + } + + // MARK: - Fetching + + /// Stages the account-category part of one fetch event. + /// + /// Called before the delegate returns, so the staged state is never behind + /// the engine's change token. + func stageFetched(records: [CKRecord], deletions: [CKRecord.ID]) { + let accountRecords = records.filter { RecordType.accountCategory.contains($0.recordType) } + let accountDeletions = deletions.filter { AccountRecordName.isAccountCategory($0.recordName) } + + guard !accountRecords.isEmpty || !accountDeletions.isEmpty else { return } + + guard let namespace else { + // Staging under a guessed identity is the one thing worse than + // losing the records: they would be applied to, and uploaded from, + // an account they never belonged to. + droppedRecordsWhileUnresolved = true + LoggingService.shared.logCloudKit( + "Dropping \(accountRecords.count + accountDeletions.count) account records fetched before the identity resolved" + ) + return + } + + store.stage(records: accountRecords, deletions: accountDeletions, in: namespace) + + LoggingService.shared.logCloudKit( + "Staged \(accountRecords.count) account records, \(accountDeletions.count) deletions" + ) + } + + /// Notes the outcome of a zone fetch and activates when it succeeded. + /// + /// The zone result — not `didFetchChanges` — is the gate: that event carries + /// only the fetch context and fires after a partial or failed zone fetch + /// just as it does after a complete one. + func zoneFetchCompleted(zoneID: CKRecordZone.ID, error: CKError?) async { + guard zoneID.zoneName == zone.zoneID.zoneName else { return } + + if let error { + LoggingService.shared.logCloudKitError( + "Account records stay staged: the zone fetch failed", + error: error + ) + return + } + + await activate() + } + + // MARK: - Activation + + /// Applies everything staged, once it is allowed to. + /// + /// Deliberately a no-op while the category is off: the records stay staged + /// (the "deferred records" of a disabled category) rather than being + /// discarded, because clearing engine state to refetch them later would + /// throw away other categories' pending changes. + func activate() async { + // The bootstrap inside makes this suspend, and it is driven by several + // independent signals — every zone fetch, an identity change, the + // category being switched back on. Overlapping runs would fetch the + // whole zone twice; a request arriving during one is replayed after it + // rather than dropped, since it may be the one that can finally proceed. + guard !isActivating else { + activationRequested = true + return + } + isActivating = true + defer { isActivating = false } + + repeat { + activationRequested = false + await performActivation() + } while activationRequested + } + + private func performActivation() async { + guard let namespace else { return } + guard isCategoryEnabled() else { + LoggingService.shared.logCloudKit("Account sync category is off — incoming records stay deferred") + return + } + + await runBootstrapIfNeeded(in: namespace) + + guard store.isBootstrapCompleted(in: namespace) else { return } + guard namespace == self.namespace else { return } + + applyStagedChanges(in: namespace) + enqueueDirtyRecords() + } + + private func applyStagedChanges(in namespace: String) { + let staged = store.stagedChanges(in: namespace) + guard !staged.isEmpty else { return } + + // Tombstone-first. A deletion that arrived in a later event than the + // account record it kills must still be applied before it, or the + // account becomes visible — and selectable — in between. + let tombstones = staged.filter { change in + switch AccountRecordName.parse(change.recordName) { + case .accountDeletion: return change.kind == .save + case .account: return change.kind == .delete + default: return false + } + } + let markers = staged.filter { + if case .legacyMigration = AccountRecordName.parse($0.recordName) { return $0.kind == .save } + return false + } + let accounts = staged.filter { + if case .account = AccountRecordName.parse($0.recordName) { return $0.kind == .save } + return false + } + let contexts = staged.filter { + if case .dataContext = AccountRecordName.parse($0.recordName) { return $0.kind == .save } + return false + } + + for change in tombstones { applyTombstone(change) } + for change in markers { applyMarker(change) } + for change in accounts { applyAccount(change) } + for change in contexts { applyContext(change) } + + // Deletions of markers and tombstones are ignored on purpose: both are + // permanent by contract, and honouring their removal would let a device + // resynthesize an account the user deleted. + store.clearStagedChanges(in: namespace) + + LoggingService.shared.logCloudKit( + "Applied account changes: \(tombstones.count) deletions, \(markers.count) markers, \(accounts.count) accounts, \(contexts.count) contexts" + ) + } + + private func applyTombstone(_ change: StagedAccountChange) { + switch AccountRecordName.parse(change.recordName) { + case let .accountDeletion(accountID): + guard let record = change.record(), + let id = try? mapper.toAccountDeletionID(from: record) else { return } + let removed = accountsManager.applyRemoteTombstone(id) + cleanUpCredentials(for: removed, accountID: accountID) + + case let .account(accountID): + // The record itself was deleted. Authoritative, but not durable — + // AccountsManager keeps the tombstone queued so this device + // publishes one. + let removed = accountsManager.applyRemoteAccountDeletionEvent(accountID) + cleanUpCredentials(for: removed, accountID: accountID) + + default: + break + } + } + + private func cleanUpCredentials(for account: Account?, accountID: UUID) { + guard let account else { return } + onAccountDeleted?(account) + LoggingService.shared.logCloudKit("Removed deleted account from iCloud: \(accountID)") + } + + private func applyMarker(_ change: StagedAccountChange) { + guard let record = change.record(), + let instanceID = try? mapper.toLegacyMigrationInstanceID(from: record) else { return } + accountsManager.applySyncedMigrationMarker(instanceID: instanceID) + } + + private func applyAccount(_ change: StagedAccountChange) { + guard let record = change.record() else { return } + + let account: Account + do { + account = try mapper.toAccount(from: record) + } catch { + LoggingService.shared.logCloudKitError("Could not read an account record", error: error) + return + } + + // Convergent repair. CloudKit enforces no relationship between an + // account and its tombstone, so a device that has not fetched the + // deletion can recreate the record. Every device that holds the + // tombstone ignores it locally and deletes it again server-side; the + // window is bounded by fetch propagation, not by trust. + guard !accountsManager.isDeleted(account.id) else { + LoggingService.shared.logCloudKit("Ignoring resurrected account \(account.id) and re-deleting it") + enqueueChanges?([.deleteRecord(recordID(for: .account(id: account.id)))]) + return + } + + if accountsManager.applySyncedAccount(account) { + // The server still holds the older version. + enqueueChanges?([.saveRecord(recordID(for: .account(id: account.id)))]) + } + } + + private func applyContext(_ change: StagedAccountChange) { + guard let record = change.record(), + let incoming = try? mapper.toStoredDataContext(from: record) else { return } + + if accountsManager.applySyncedContext(incoming) { + enqueueChanges?([.saveRecord(recordID(for: .dataContext))]) + } + } + + // MARK: - Bootstrap + + /// Fetches the zone once from a nil change token, applying only account + /// records, so an upgrade picks up everything a previous version consumed + /// as an unknown record type. + /// + /// Runs outside the engine's own token: the engine's state and every other + /// category's pending changes are untouched. + private func runBootstrapIfNeeded(in namespace: String) async { + guard !store.isBootstrapCompleted(in: namespace) else { return } + guard let database else { return } + + let generation = identityGeneration + + LoggingService.shared.logCloudKit("Bootstrapping account records with a full zone fetch") + + // CloudKit invokes the per-record blocks on its own queue, so the + // results are collected behind a lock rather than in captured vars. + let collector = BootstrapCollector() + + let configuration = CKFetchRecordZoneChangesOperation.ZoneConfiguration() + configuration.previousServerChangeToken = nil + + let operation = CKFetchRecordZoneChangesOperation( + recordZoneIDs: [zone.zoneID], + configurationsByRecordZoneID: [zone.zoneID: configuration] + ) + operation.fetchAllChanges = true + + operation.recordWasChangedBlock = { _, result in + guard case let .success(record) = result else { return } + guard RecordType.accountCategory.contains(record.recordType) else { return } + collector.add(record: record) + } + + operation.recordWithIDWasDeletedBlock = { recordID, _ in + guard AccountRecordName.isAccountCategory(recordID.recordName) else { return } + collector.add(deletion: recordID) + } + + operation.recordZoneFetchResultBlock = { _, result in + if case let .failure(error) = result { collector.recordZoneError(error) } + } + + let outcome: Result = await withCheckedContinuation { continuation in + operation.fetchRecordZoneChangesResultBlock = { result in + continuation.resume(returning: result) + } + database.add(operation) + } + + let (fetched, deleted, zoneError) = collector.results() + + // Both results have to succeed. A partial bootstrap leaves the flag + // unset and runs again, which is why nothing is staged until here. + if case let .failure(error) = outcome { + if Self.isMissingZone(error) || zoneError.map(Self.isMissingZone) == true { + // Nothing has ever been written to this zone, so there is + // nothing for a bootstrap to recover. + guard identityGeneration == generation, namespace == self.namespace else { return } + store.markBootstrapCompleted(in: namespace) + return + } + LoggingService.shared.logCloudKitError("Account bootstrap fetch failed", error: error) + return + } + if let zoneError { + if Self.isMissingZone(zoneError) { + guard identityGeneration == generation, namespace == self.namespace else { return } + store.markBootstrapCompleted(in: namespace) + return + } + LoggingService.shared.logCloudKitError("Account bootstrap zone fetch failed", error: zoneError) + return + } + + // The fetch began under one identity and may be finishing under + // another; its results belong to the identity that asked for them. + guard identityGeneration == generation, namespace == self.namespace else { + LoggingService.shared.logCloudKit("Discarding an account bootstrap that finished under a different Apple account") + return + } + + store.stage(records: fetched, deletions: deleted, in: namespace) + store.markBootstrapCompleted(in: namespace) + + LoggingService.shared.logCloudKit( + "Account bootstrap staged \(fetched.count) records, \(deleted.count) deletions" + ) + } + + // MARK: - Uploading + + /// Registers everything the outbox still owes the server. + /// + /// Safe to call repeatedly: the engine collapses duplicate pending changes, + /// and each entry is cleared only by a server confirmation. + func enqueueDirtyRecords() { + guard let enqueueChanges, let namespace else { return } + guard isCategoryEnabled(), store.isBootstrapCompleted(in: namespace) else { return } + + var changes: [CKSyncEngine.PendingRecordZoneChange] = [] + + // Tombstones first, and on their own: the account record's deletion is + // registered when the tombstone is confirmed. A deletion whose + // tombstone never landed is exactly how a stale device resurrects an + // account, so the durable half goes first. + for accountID in accountsManager.pendingDeletions { + changes.append(.saveRecord(recordID(for: .accountDeletion(accountID: accountID)))) + } + + for accountID in accountsManager.pendingUploads { + changes.append(.saveRecord(recordID(for: .account(id: accountID)))) + } + + for instanceID in accountsManager.pendingMarkerUploads + where accountsManager.canUploadMarker(instanceID: instanceID) { + changes.append(.saveRecord(recordID(for: .legacyAccountMigration(instanceID: instanceID)))) + } + + if accountsManager.pendingContextUpload, accountsManager.canUploadContext { + changes.append(.saveRecord(recordID(for: .dataContext))) + } + + guard !changes.isEmpty else { return } + enqueueChanges(changes) + } + + /// Registers every account record for upload, for a zone with nothing in it + /// — first enable, or a zone recreated after being deleted elsewhere. + func uploadEverything() { + accountsManager.markAllRecordsForUpload() + enqueueDirtyRecords() + } + + /// Builds the record for a pending save at send time. + /// + /// Returning `nil` withdraws the change from the engine — used both for + /// records that no longer exist and for records that are being *held* by the + /// upload ordering. Holding is safe because the outbox is durable: the + /// change is registered again as soon as what it waits on is confirmed. + func recordToSave(named recordName: String) -> CKRecord? { + guard let namespace, isCategoryEnabled(), store.isBootstrapCompleted(in: namespace) else { return nil } + + switch AccountRecordName.parse(recordName) { + case let .account(id): + guard !accountsManager.isDeleted(id), let account = accountsManager.account(id: id) else { return nil } + return mapper.toCKRecord(account: account) + + case let .legacyMigration(instanceID): + guard accountsManager.migratedInstanceIDs.contains(instanceID), + accountsManager.canUploadMarker(instanceID: instanceID) else { return nil } + return mapper.toCKRecord(legacyMigrationInstanceID: instanceID) + + case let .accountDeletion(accountID): + guard accountsManager.isDeleted(accountID) else { return nil } + return mapper.toCKRecord(accountDeletionID: accountID) + + case .dataContext: + guard accountsManager.canUploadContext else { return nil } + return mapper.toCKRecord(dataContext: accountsManager.storedContext) + + case nil: + return nil + } + } + + /// Clears outbox entries the server has confirmed, and releases whatever was + /// waiting on them. + func didSave(records: [CKRecord]) { + var didConfirmAccount = false + + for record in records { + switch AccountRecordName.parse(record.recordID.recordName) { + case let .account(id): + accountsManager.confirmUpload(id, updatedAt: record["updatedAt"] as? Date) + didConfirmAccount = true + + case let .legacyMigration(instanceID): + accountsManager.confirmMarkerUpload(instanceID: instanceID) + + case let .accountDeletion(accountID): + accountsManager.confirmDeletion(accountID) + // Only now is removing the account record safe to attempt: the + // durable half of the deletion exists. + enqueueChanges?([.deleteRecord(recordID(for: .account(id: accountID)))]) + + case .dataContext: + if let changedAt = record["changedAt"] as? Date { + accountsManager.confirmContextUpload(changedAt: changedAt) + } + + case nil: + break + } + } + + // A marker or a context held behind one of these accounts can go now. + if didConfirmAccount { + enqueueDirtyRecords() + } + } + + /// Whether the engine should retry a save that failed with `unknownItem` + /// as a fresh insert. + /// + /// It must not for an account a tombstone dominates: reinserting is exactly + /// how a queued offline edit resurrects a deleted account. + func shouldRetryAsInsert(recordName: String) -> Bool { + guard case let .account(id) = AccountRecordName.parse(recordName) else { return true } + return !accountsManager.isDeleted(id) + } + + /// Resolves a conflict on an account-category record, and applies the + /// outcome locally. + /// + /// The local apply matters here in a way it does not for the other + /// categories: CloudKit never delivers back a record this device saved, so + /// a conflict the server won would otherwise leave stale state on screen + /// until some unrelated future change to the same record. + func resolveConflict(local: CKRecord, server: CKRecord) async -> CKRecord { + let resolved: CKRecord + + switch server.recordType { + case RecordType.account: + resolved = await conflictResolver.resolveAccountConflict(local: local, server: server) + if let account = try? mapper.toAccount(from: resolved), !accountsManager.isDeleted(account.id) { + accountsManager.applySyncedAccount(account) + } + + case RecordType.dataContext: + resolved = await conflictResolver.resolveDataContextConflict(local: local, server: server) + if let context = try? mapper.toStoredDataContext(from: resolved) { + accountsManager.applySyncedContext(context) + } + + case RecordType.legacyAccountMigration, RecordType.accountDeletion: + resolved = await conflictResolver.resolveWriteOnceConflict(local: local, server: server) + + default: + resolved = server + } + + return resolved + } + + // MARK: - Helpers + + private func recordID(for type: SyncableRecordType) -> CKRecord.ID { + type.recordID(in: zone) + } + + /// Whether an error means the zone does not exist, in which case there is + /// nothing for a bootstrap to recover and it counts as done. + private static func isMissingZone(_ error: Error) -> Bool { + guard let ckError = error as? CKError else { return false } + + switch ckError.code { + case .zoneNotFound, .userDeletedZone: + return true + case .partialFailure: + let partial = ckError.partialErrorsByItemID?.values ?? [:].values + return partial.contains { isMissingZone($0) } + default: + return false + } + } +} + +/// Gathers a bootstrap fetch's results across CloudKit's callback queue. +private final class BootstrapCollector: @unchecked Sendable { + private let lock = NSLock() + private var records: [CKRecord] = [] + private var deletions: [CKRecord.ID] = [] + private var zoneError: Error? + + func add(record: CKRecord) { + lock.withLock { records.append(record) } + } + + func add(deletion recordID: CKRecord.ID) { + lock.withLock { deletions.append(recordID) } + } + + func recordZoneError(_ error: Error) { + lock.withLock { zoneError = error } + } + + func results() -> ([CKRecord], [CKRecord.ID], Error?) { + lock.withLock { (records, deletions, zoneError) } + } +} diff --git a/Yattee/Services/CloudKit/AccountSyncStore.swift b/Yattee/Services/CloudKit/AccountSyncStore.swift new file mode 100644 index 00000000..475b1f67 --- /dev/null +++ b/Yattee/Services/CloudKit/AccountSyncStore.swift @@ -0,0 +1,161 @@ +// +// AccountSyncStore.swift +// Yattee +// +// Durable, identity-namespaced staging for fetched account-category records. +// + +import CloudKit +import Foundation + +/// One fetched account-category change, held until it may be applied. +struct StagedAccountChange: Codable, Sendable { + enum Kind: String, Codable, Sendable { + case save + case delete + } + + /// Arrival order. Two changes to the same record collapse to the later one; + /// changes to *different* records keep their order, which is what lets a + /// tombstone be applied ahead of an account record it dominates. + let sequence: Int + let recordName: String + let kind: Kind + + /// Archived `CKRecord`. `nil` for deletions, which carry only a record ID. + let recordData: Data? + + func record() -> CKRecord? { + guard let recordData else { return nil } + return try? NSKeyedUnarchiver.unarchivedObject(ofClass: CKRecord.self, from: recordData) + } +} + +/// Storage for everything account sync has to survive a relaunch with. +/// +/// ### Why staging is persisted at all +/// +/// `CKSyncEngine` emits several `fetchedRecordZoneChanges` events between +/// `willFetchChanges` and `didFetchChanges`, and may persist its change token +/// after any of them. Applying records as they arrive would expose an account +/// delivered in the first event before the tombstone that deletes it arrives in +/// the third; holding them only in memory would lose them to a crash while the +/// engine's token had already moved past them. So each event's records are +/// written here before the delegate returns, and applied as a set once the zone +/// fetch has actually succeeded. +/// +/// Durability rests on the same mechanism the engine's own token does: both are +/// `UserDefaults` writes, and they are ordered, so any crash that preserved the +/// token also preserved the staging written before it. +/// +/// ### Why everything is namespaced +/// +/// Every value here belongs to one Apple account. Sharing storage across +/// identities would let a fetch begun under one account be applied under the +/// next, which is precisely the disclosure that scoping accounts prevents. +@MainActor +final class AccountSyncStore { + private let defaults: UserDefaults + + init(defaults: UserDefaults = .standard) { + self.defaults = defaults + } + + private func key(_ name: String, in namespace: String) -> String { + "accountSync.\(namespace).\(name)" + } + + // MARK: - Staging + + func stagedChanges(in namespace: String) -> [StagedAccountChange] { + guard let data = defaults.data(forKey: key("staged", in: namespace)), + let decoded = try? JSONDecoder().decode([StagedAccountChange].self, from: data) else { + return [] + } + return decoded.sorted { $0.sequence < $1.sequence } + } + + /// Stages fetched changes, replacing any earlier change to the same record. + /// + /// Records that fail to archive are dropped rather than silently staged as + /// deletions — losing a record is recoverable by a later fetch or bootstrap, + /// but a phantom deletion is not. + func stage(records: [CKRecord], deletions: [CKRecord.ID], in namespace: String) { + guard !records.isEmpty || !deletions.isEmpty else { return } + + var byName: [String: StagedAccountChange] = [:] + for change in stagedChanges(in: namespace) { + byName[change.recordName] = change + } + + var sequence = byName.values.map(\.sequence).max().map { $0 + 1 } ?? 0 + + for record in records { + guard let data = try? NSKeyedArchiver.archivedData( + withRootObject: record, + requiringSecureCoding: true + ) else { + LoggingService.shared.logCloudKitError( + "Could not archive account record \(record.recordID.recordName) for staging", + error: CloudKitError.recordNotFound + ) + continue + } + + byName[record.recordID.recordName] = StagedAccountChange( + sequence: sequence, + recordName: record.recordID.recordName, + kind: .save, + recordData: data + ) + sequence += 1 + } + + for recordID in deletions { + byName[recordID.recordName] = StagedAccountChange( + sequence: sequence, + recordName: recordID.recordName, + kind: .delete, + recordData: nil + ) + sequence += 1 + } + + let ordered = byName.values.sorted { $0.sequence < $1.sequence } + guard let data = try? JSONEncoder().encode(ordered) else { + LoggingService.shared.logCloudKit("Could not persist staged account changes") + return + } + defaults.set(data, forKey: key("staged", in: namespace)) + } + + func clearStagedChanges(in namespace: String) { + defaults.removeObject(forKey: key("staged", in: namespace)) + } + + // MARK: - Schema bootstrap + + /// Whether this identity has run the one-time full-zone fetch that picks up + /// account records a previous app version consumed without understanding. + func isBootstrapCompleted(in namespace: String) -> Bool { + defaults.integer(forKey: key("schemaVersion", in: namespace)) >= AccountSyncStore.schemaVersion + } + + func markBootstrapCompleted(in namespace: String) { + defaults.set(AccountSyncStore.schemaVersion, forKey: key("schemaVersion", in: namespace)) + } + + /// Forces the bootstrap to run again for this identity. + /// + /// Used when account records had to be dropped because the identity they + /// belonged to could not be established at the time — the engine's token has + /// moved past them, so only a full-zone fetch can recover them. + func clearBootstrapCompletion(in namespace: String) { + defaults.removeObject(forKey: key("schemaVersion", in: namespace)) + } + + /// The account-category schema this build understands. Raising it makes + /// every identity bootstrap once more, which is how a future release picks + /// up record types this one ignores. + static let schemaVersion = 1 +} diff --git a/Yattee/Services/CloudKit/CloudKitConflictResolver.swift b/Yattee/Services/CloudKit/CloudKitConflictResolver.swift index 73bdaf39..f0f4dcbc 100644 --- a/Yattee/Services/CloudKit/CloudKitConflictResolver.swift +++ b/Yattee/Services/CloudKit/CloudKitConflictResolver.swift @@ -329,7 +329,74 @@ actor CloudKitConflictResolver { resolved["updatedAt"] = local["updatedAt"] resolved["layoutJSON"] = local["layoutJSON"] } - + return resolved } + + // MARK: - Account Conflict Resolution + + /// Resolves a conflict between local and server account records. + /// + /// Strategy: + /// - Last write wins on the app-managed `updatedAt`, never on CloudKit's + /// modification date: an edit made offline has to stay orderable against + /// a concurrent edit elsewhere, and a migrated account carries an + /// epoch-old date precisely so any real edit outranks it. + /// - `instanceID` and `dateAdded` are immutable identity, taken from + /// whichever side wins only so a malformed record cannot strip them. + func resolveAccountConflict( + local: CKRecord, + server: CKRecord + ) -> CKRecord { + let localUpdated = (local["updatedAt"] as? Date) ?? Date.distantPast + let serverUpdated = (server["updatedAt"] as? Date) ?? Date.distantPast + + // IMPORTANT: Always start with server record to preserve recordChangeTag + let resolved = server + + if localUpdated > serverUpdated { + resolved["displayName"] = local["displayName"] + resolved["updatedAt"] = local["updatedAt"] + } + + return resolved + } + + // MARK: - DataContext Conflict Resolution + + /// Resolves a conflict between local and server data-context records. + /// + /// Strategy: last write wins on the app-managed `changedAt` — the moment + /// the user chose, not the moment a record happened to be written. + func resolveDataContextConflict( + local: CKRecord, + server: CKRecord + ) -> CKRecord { + let localChanged = (local["changedAt"] as? Date) ?? Date.distantPast + let serverChanged = (server["changedAt"] as? Date) ?? Date.distantPast + + let resolved = server + + if localChanged > serverChanged { + resolved["contextKind"] = local["contextKind"] + resolved["accountID"] = local["accountID"] + resolved["changedAt"] = local["changedAt"] + } + + return resolved + } + + // MARK: - Write-Once Conflict Resolution + + /// Resolves a conflict on a record that only ever means "this happened": + /// the legacy-migration markers and the account deletion tombstones. + /// + /// There is no state to merge — both sides assert the same fact — so the + /// server's copy is kept, which also preserves its change tag. + func resolveWriteOnceConflict( + local _: CKRecord, + server: CKRecord + ) -> CKRecord { + server + } } diff --git a/Yattee/Services/CloudKit/CloudKitRecordMapper.swift b/Yattee/Services/CloudKit/CloudKitRecordMapper.swift index 69941f91..e2a5e1d8 100644 --- a/Yattee/Services/CloudKit/CloudKitRecordMapper.swift +++ b/Yattee/Services/CloudKit/CloudKitRecordMapper.swift @@ -894,4 +894,220 @@ final class CloudKitRecordMapper { layout: layout ) } + + // MARK: - Account Mapping + + /// Converts an `Account` to a CKRecord. + /// + /// The login identifier is deliberately absent: it lives in the Keychain + /// beside the credential, so synced metadata never names a user. + func toCKRecord(account: Account) -> CKRecord { + let recordID = SyncableRecordType.account(id: account.id).recordID(in: zone) + let record = CKRecord(recordType: RecordType.account, recordID: recordID) + + record["schemaVersion"] = currentSchemaVersion as CKRecordValue + record["accountID"] = account.id.uuidString as CKRecordValue + record["instanceID"] = account.instanceID.uuidString as CKRecordValue + record["displayName"] = account.displayName as CKRecordValue? + record["dateAdded"] = account.dateAdded as CKRecordValue + record["updatedAt"] = account.updatedAt as CKRecordValue + + return record + } + + /// Creates an `Account` from a CKRecord. + func toAccount(from record: CKRecord) throws -> Account { + let recordType = RecordType.account + guard record.recordType == recordType else { + throw CloudKitError.recordNotFound + } + + let version = (record["schemaVersion"] as? Int64) ?? 1 + guard version <= currentSchemaVersion else { + throw CloudKitError.unsupportedSchemaVersion(version: version, recordType: recordType) + } + + guard let accountIDString = record["accountID"] as? String else { + throw CloudKitError.missingRequiredField(field: "accountID", recordType: recordType) + } + guard let accountID = UUID(uuidString: accountIDString) else { + throw CloudKitError.typeMismatch(field: "accountID", recordType: recordType, expected: "valid UUID string") + } + guard let instanceIDString = record["instanceID"] as? String else { + throw CloudKitError.missingRequiredField(field: "instanceID", recordType: recordType) + } + guard let instanceID = UUID(uuidString: instanceIDString) else { + throw CloudKitError.typeMismatch(field: "instanceID", recordType: recordType, expected: "valid UUID string") + } + guard let dateAdded = record["dateAdded"] as? Date else { + throw CloudKitError.missingRequiredField(field: "dateAdded", recordType: recordType) + } + guard let updatedAt = record["updatedAt"] as? Date else { + throw CloudKitError.missingRequiredField(field: "updatedAt", recordType: recordType) + } + + // The record name is the identity every device agrees on; a record whose + // payload disagrees with it would resolve conflicts against a different + // account than the one it overwrites. + guard record.recordID.recordName == AccountRecordName.account(accountID) else { + throw CloudKitError.typeMismatch( + field: "accountID", + recordType: recordType, + expected: "value matching the record name" + ) + } + + return Account( + id: accountID, + instanceID: instanceID, + displayName: record["displayName"] as? String, + dateAdded: dateAdded, + updatedAt: updatedAt + ) + } + + // MARK: - Legacy Account Migration Marker Mapping + + /// A permanent marker recording that this instance's legacy login has been + /// turned into an account. Write-once: it carries no mutable state. + func toCKRecord(legacyMigrationInstanceID instanceID: UUID) -> CKRecord { + let recordID = SyncableRecordType.legacyAccountMigration(instanceID: instanceID).recordID(in: zone) + let record = CKRecord(recordType: RecordType.legacyAccountMigration, recordID: recordID) + + record["schemaVersion"] = currentSchemaVersion as CKRecordValue + record["instanceID"] = instanceID.uuidString as CKRecordValue + record["migratedAt"] = Date() as CKRecordValue + + return record + } + + func toLegacyMigrationInstanceID(from record: CKRecord) throws -> UUID { + let recordType = RecordType.legacyAccountMigration + guard record.recordType == recordType else { + throw CloudKitError.recordNotFound + } + + let version = (record["schemaVersion"] as? Int64) ?? 1 + guard version <= currentSchemaVersion else { + throw CloudKitError.unsupportedSchemaVersion(version: version, recordType: recordType) + } + + guard let instanceIDString = record["instanceID"] as? String else { + throw CloudKitError.missingRequiredField(field: "instanceID", recordType: recordType) + } + guard let instanceID = UUID(uuidString: instanceIDString) else { + throw CloudKitError.typeMismatch(field: "instanceID", recordType: recordType, expected: "valid UUID string") + } + guard record.recordID.recordName == AccountRecordName.legacyMigration(instanceID) else { + throw CloudKitError.typeMismatch( + field: "instanceID", + recordType: recordType, + expected: "value matching the record name" + ) + } + + return instanceID + } + + // MARK: - Account Deletion Tombstone Mapping + + /// A permanent tombstone for a deleted account. + func toCKRecord(accountDeletionID accountID: UUID, deletedAt: Date = Date()) -> CKRecord { + let recordID = SyncableRecordType.accountDeletion(accountID: accountID).recordID(in: zone) + let record = CKRecord(recordType: RecordType.accountDeletion, recordID: recordID) + + record["schemaVersion"] = currentSchemaVersion as CKRecordValue + record["accountID"] = accountID.uuidString as CKRecordValue + record["deletedAt"] = deletedAt as CKRecordValue + + return record + } + + func toAccountDeletionID(from record: CKRecord) throws -> UUID { + let recordType = RecordType.accountDeletion + guard record.recordType == recordType else { + throw CloudKitError.recordNotFound + } + + let version = (record["schemaVersion"] as? Int64) ?? 1 + guard version <= currentSchemaVersion else { + throw CloudKitError.unsupportedSchemaVersion(version: version, recordType: recordType) + } + + guard let accountIDString = record["accountID"] as? String else { + throw CloudKitError.missingRequiredField(field: "accountID", recordType: recordType) + } + guard let accountID = UUID(uuidString: accountIDString) else { + throw CloudKitError.typeMismatch(field: "accountID", recordType: recordType, expected: "valid UUID string") + } + guard record.recordID.recordName == AccountRecordName.accountDeletion(accountID) else { + throw CloudKitError.typeMismatch( + field: "accountID", + recordType: recordType, + expected: "value matching the record name" + ) + } + + return accountID + } + + // MARK: - DataContext Mapping + + /// The singleton record holding the selected data context. + /// + /// `changedAt` is app-managed and is the only ordering ever used for this + /// record — CloudKit's modification date says when a *record* was written, + /// not when the user made a choice. + func toCKRecord(dataContext stored: StoredDataContext) -> CKRecord { + let recordID = SyncableRecordType.dataContext.recordID(in: zone) + let record = CKRecord(recordType: RecordType.dataContext, recordID: recordID) + + record["schemaVersion"] = currentSchemaVersion as CKRecordValue + record["changedAt"] = stored.changedAt as CKRecordValue + + switch stored.context { + case .local: + record["contextKind"] = "local" as CKRecordValue + record["accountID"] = nil + case let .account(id): + record["contextKind"] = "account" as CKRecordValue + record["accountID"] = id.uuidString as CKRecordValue + } + + return record + } + + func toStoredDataContext(from record: CKRecord) throws -> StoredDataContext { + let recordType = RecordType.dataContext + guard record.recordType == recordType else { + throw CloudKitError.recordNotFound + } + + let version = (record["schemaVersion"] as? Int64) ?? 1 + guard version <= currentSchemaVersion else { + throw CloudKitError.unsupportedSchemaVersion(version: version, recordType: recordType) + } + + guard let kind = record["contextKind"] as? String else { + throw CloudKitError.missingRequiredField(field: "contextKind", recordType: recordType) + } + guard let changedAt = record["changedAt"] as? Date else { + throw CloudKitError.missingRequiredField(field: "changedAt", recordType: recordType) + } + + switch kind { + case "local": + return StoredDataContext(context: .local, changedAt: changedAt) + case "account": + guard let accountIDString = record["accountID"] as? String else { + throw CloudKitError.missingRequiredField(field: "accountID", recordType: recordType) + } + guard let accountID = UUID(uuidString: accountIDString) else { + throw CloudKitError.typeMismatch(field: "accountID", recordType: recordType, expected: "valid UUID string") + } + return StoredDataContext(context: .account(accountID), changedAt: changedAt) + default: + throw CloudKitError.typeMismatch(field: "contextKind", recordType: recordType, expected: "\"local\" or \"account\"") + } + } } diff --git a/Yattee/Services/CloudKit/CloudKitSyncEngine.swift b/Yattee/Services/CloudKit/CloudKitSyncEngine.swift index 8487b30c..7fba874d 100644 --- a/Yattee/Services/CloudKit/CloudKitSyncEngine.swift +++ b/Yattee/Services/CloudKit/CloudKitSyncEngine.swift @@ -225,6 +225,11 @@ final class CloudKitSyncEngine: @unchecked Sendable { /// Player controls layout service for importing/removing presets from CloudKit. /// Set after initialization to use shared instance instead of creating new ones. weak var playerControlsLayoutService: PlayerControlsLayoutService? + + /// Drives the account category: accounts, migration markers, deletion + /// tombstones and the selected data context. Set after initialization, and + /// nil in tests that only exercise the SwiftData categories. + var accountSync: AccountSyncCoordinator? // MARK: - Initialization @@ -303,6 +308,10 @@ final class CloudKitSyncEngine: @unchecked Sendable { conflictResolvedRecords.removeAll() retryCount.removeAll() deferredPlaylistItems.removeAll() + // The account outbox, staging and bootstrap flag are deliberately left + // alone: they are the durable record of what still has to happen, and + // re-enabling sync must resume rather than start over. + accountSync?.detach() syncEngine = nil updatePendingCount() isSyncing = false @@ -386,6 +395,15 @@ final class CloudKitSyncEngine: @unchecked Sendable { pendingChangesBuffer.removeAll() } + // Accounts activate before the first sync: the bootstrap fetch has + // to complete before anything is uploaded, or a device upgrading + // from the previous release would publish its unmerged local list + // over records it has not seen yet. + accountSync?.attach(database: database) { [weak self] changes in + self?.addPendingChanges(changes) + } + await accountSync?.activate() + if accountChanged { LoggingService.shared.logCloudKit("CloudKit sync engine initialized with new account (fresh sync state)") } else { @@ -560,6 +578,12 @@ final class CloudKitSyncEngine: @unchecked Sendable { private var canSyncControlsPresets: Bool { isSyncEnabled && (settingsManager?.syncSettings ?? true) } + + /// Whether account sync is enabled. Accounts ride the instances category: + /// they describe logins on those instances, and the two are useless apart. + var canSyncAccounts: Bool { + isSyncEnabled && (settingsManager?.syncInstances ?? true) + } /// Manually trigger a full sync. func sync() async { @@ -1534,7 +1558,20 @@ final class CloudKitSyncEngine: @unchecked Sendable { } // MARK: - Initial Upload Coordination - + + /// Re-registers every account record for upload. + /// + /// Used when the zone is being seeded or reseeded — enabling sync, or + /// recreating the zone after it was deleted elsewhere. The account outbox + /// normally tracks only what changed, which is not enough when the server + /// has nothing at all. + func uploadAllAccounts() { + guard canSyncAccounts else { return } + uploadProgress?.currentCategory = "Accounts" + accountSync?.uploadEverything() + } + + /// Performs full initial upload of all local data with progress tracking func performInitialUpload() async { guard isSyncEnabled else { return } @@ -1555,7 +1592,8 @@ final class CloudKitSyncEngine: @unchecked Sendable { await uploadAllRecentPlaylists() await uploadAllLocalChannelNotificationSettings() await uploadAllLocalControlsPresets() - + uploadAllAccounts() + // Mark complete uploadProgress = UploadProgress( currentCategory: "Complete", @@ -1650,6 +1688,12 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { case .fetchedDatabaseChanges(let changes): await handleFetchedDatabaseChanges(changes) + case .didFetchRecordZoneChanges(let changes): + // The zone's own result — not didFetchChanges, which carries only + // the fetch context and fires after failed zone fetches too — is + // what tells staged account records they may be applied. + await accountSync?.zoneFetchCompleted(zoneID: changes.zoneID, error: changes.error) + default: await MainActor.run { switch event { @@ -1674,7 +1718,7 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { LoggingService.shared.logCloudKit("CKSyncEngine finished fetching changes") isReceivingChanges = false - case .willFetchRecordZoneChanges, .didFetchRecordZoneChanges, .willSendChanges, .didSendChanges, .sentDatabaseChanges: + case .willFetchRecordZoneChanges, .willSendChanges, .didSendChanges, .sentDatabaseChanges: break default: @@ -1767,6 +1811,12 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { /// matched on the full scoped record name (a mismatch means it belongs to /// a different source scope). private func materializeRecord(named recordName: String) async -> CKRecord? { + // Accounts are not SwiftData models and are built from the accounts + // store, which also decides whether a record may be sent yet. + if AccountRecordName.isAccountCategory(recordName) { + return accountSync?.recordToSave(named: recordName) + } + guard let dataManager else { return nil } if recordName.hasPrefix("sub-") { @@ -1849,6 +1899,10 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { retryCount.removeValue(forKey: recordName) } + // Clears the account outbox and releases whatever its ordering held + // back. Only a confirmation does this — never the attempt. + accountSync?.didSave(records: changes.savedRecords) + for failedSave in changes.failedRecordSaves { await handleFailedRecordSave(failedSave) } @@ -1902,6 +1956,15 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { // Record vanished server-side while we held its change tag — // drop the cached record and retry as a fresh insert. conflictResolvedRecords.removeValue(forKey: recordName) + + // Unless a deletion tombstone dominates it. Reinserting an account + // another device deleted is exactly how a queued offline edit + // resurrects it. + if let accountSync, !accountSync.shouldRetryAsInsert(recordName: recordName) { + LoggingService.shared.logCloudKit("Record \(recordName) is deleted, not reinserting") + return + } + syncEngine?.state.add(pendingRecordZoneChanges: [.saveRecord(recordID)]) LoggingService.shared.logCloudKit("Record \(recordName) missing on server, retrying as insert") @@ -1966,6 +2029,10 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { return await conflictResolver.resolveRecentPlaylistConflict(local: local, server: server) case RecordType.controlsPreset: return await conflictResolver.resolveLayoutPresetConflict(local: local, server: server) + case RecordType.account, RecordType.dataContext, + RecordType.legacyAccountMigration, RecordType.accountDeletion: + guard let accountSync else { return server } + return await accountSync.resolveConflict(local: local, server: server) default: // Unknown type - use server version (safe fallback) LoggingService.shared.logCloudKit("Unknown record type \(server.recordType), using server version") @@ -2002,6 +2069,15 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { /// Handle fetched changes from CloudKit. private func handleFetchedChanges(_ changes: CKSyncEngine.Event.FetchedRecordZoneChanges) async { + // Staged first, and before any early return: the engine may persist its + // change token after this event, so anything not written down here is + // gone for good. Account records are never applied inline — they are + // applied as a set, tombstones first, once the zone fetch succeeds. + accountSync?.stageFetched( + records: changes.modifications.map(\.record), + deletions: changes.deletions.map(\.recordID) + ) + guard let dataManager else { return } // Load persisted deferred items from previous sessions @@ -2012,6 +2088,7 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { // Apply modifications - process all incoming records for modification in changes.modifications { + guard !RecordType.accountCategory.contains(modification.record.recordType) else { continue } let result = await applyRemoteRecord(modification.record, to: dataManager) if case .deferred(let playlistID, let itemID) = result { newlyDeferredItems.append((modification.record, playlistID, itemID)) @@ -2047,6 +2124,7 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate { // Apply deletions for deletion in changes.deletions { + guard !AccountRecordName.isAccountCategory(deletion.recordID.recordName) else { continue } await applyRemoteDeletion(deletion.recordID, to: dataManager) } diff --git a/Yattee/Services/CloudKit/RecordTypes.swift b/Yattee/Services/CloudKit/RecordTypes.swift index c5dab0e6..3472c1e7 100644 --- a/Yattee/Services/CloudKit/RecordTypes.swift +++ b/Yattee/Services/CloudKit/RecordTypes.swift @@ -23,7 +23,31 @@ enum RecordType { static let mediaSource = "MediaSource" static let channelNotificationSettings = "ChannelNotificationSettings" static let controlsPreset = "ControlsPreset" - + + // MARK: - Account category + + /// A sign-in on a backend instance (`Account`). + static let account = "Account" + + /// Permanent marker recording that a device turned an instance's legacy + /// single login into an account. Never deleted: `Instance.usesAccountLogin` + /// stays true forever, so the marker — not the account's existence — is what + /// stops a deliberately deleted account from being synthesized again. + static let legacyAccountMigration = "LegacyAccountMigration" + + /// Singleton record holding the selected `DataContext`. + static let dataContext = "DataContext" + + /// Permanent tombstone for a deleted account. A CloudKit record deletion is + /// not durable evidence — a device that never fetched the deletion can + /// recreate the record from a queued offline edit — so deletions are + /// recorded as records of their own. + static let accountDeletion = "AccountDeletion" + + /// Every record type in the account category, in the order a fetch applies + /// them: tombstones dominate accounts, and markers gate synthesis. + static let accountCategory = [accountDeletion, legacyAccountMigration, account, dataContext] + /// CloudKit zone name for all user data. static let zoneName = "UserData" @@ -79,7 +103,11 @@ enum SyncableRecordType: Sendable { case mediaSource(id: UUID) case channelNotificationSettings(channelID: String, scope: SourceScope) case controlsPreset(id: UUID) - + case account(id: UUID) + case legacyAccountMigration(instanceID: UUID) + case dataContext + case accountDeletion(accountID: UUID) + /// The CloudKit record type name. var recordTypeName: String { switch self { @@ -96,6 +124,10 @@ enum SyncableRecordType: Sendable { case .mediaSource: RecordType.mediaSource case .channelNotificationSettings: RecordType.channelNotificationSettings case .controlsPreset: RecordType.controlsPreset + case .account: RecordType.account + case .legacyAccountMigration: RecordType.legacyAccountMigration + case .dataContext: RecordType.dataContext + case .accountDeletion: RecordType.accountDeletion } } @@ -140,12 +172,77 @@ enum SyncableRecordType: Sendable { recordName = "channel-notif-\(channelID)\(scope.recordNameSuffix)" case .controlsPreset(let id): recordName = "controls-\(id.uuidString)" + case .account(let id): + recordName = AccountRecordName.account(id) + case .legacyAccountMigration(let instanceID): + recordName = AccountRecordName.legacyMigration(instanceID) + case .dataContext: + recordName = AccountRecordName.dataContext + case .accountDeletion(let accountID): + recordName = AccountRecordName.accountDeletion(accountID) } return CKRecord.ID(recordName: recordName, zoneID: zone.zoneID) } } +/// Record names for the account category, and the parsing that recognises them. +/// +/// Account records are identified by name rather than by fetched record type so +/// that a *deletion* — which carries only a record ID — can be routed to the +/// same handler as a modification. +enum AccountRecordName { + static let accountPrefix = "account-" + static let legacyMigrationPrefix = "legacymigrated-" + static let accountDeletionPrefix = "accountDeletion-" + static let dataContext = "datacontext-current" + + static func account(_ id: UUID) -> String { accountPrefix + id.uuidString } + static func legacyMigration(_ instanceID: UUID) -> String { legacyMigrationPrefix + instanceID.uuidString } + static func accountDeletion(_ accountID: UUID) -> String { accountDeletionPrefix + accountID.uuidString } + + /// What a record name in the account category refers to. + enum Parsed: Hashable, Sendable { + case account(UUID) + case legacyMigration(instanceID: UUID) + case dataContext + case accountDeletion(accountID: UUID) + } + + /// `nil` for any name outside the account category. + /// + /// The deletion prefix is tested first only for readability — `"account-"` + /// cannot match `"accountDeletion-…"`, whose eighth character is `D` — but + /// the ordering makes the intent explicit rather than relying on that. + static func parse(_ recordName: String) -> Parsed? { + if recordName == dataContext { return .dataContext } + + if recordName.hasPrefix(accountDeletionPrefix) { + guard let id = UUID(uuidString: String(recordName.dropFirst(accountDeletionPrefix.count))) else { return nil } + return .accountDeletion(accountID: id) + } + if recordName.hasPrefix(legacyMigrationPrefix) { + guard let id = UUID(uuidString: String(recordName.dropFirst(legacyMigrationPrefix.count))) else { return nil } + return .legacyMigration(instanceID: id) + } + if recordName.hasPrefix(accountPrefix) { + guard let id = UUID(uuidString: String(recordName.dropFirst(accountPrefix.count))) else { return nil } + return .account(id) + } + return nil + } + + /// Whether a record name belongs to the account category at all, including + /// names whose UUID does not parse — those must still be kept away from the + /// other categories' handlers, which would misread them. + static func isAccountCategory(_ recordName: String) -> Bool { + recordName == dataContext + || recordName.hasPrefix(accountDeletionPrefix) + || recordName.hasPrefix(legacyMigrationPrefix) + || recordName.hasPrefix(accountPrefix) + } +} + /// Sync operation type. enum SyncOperation: Sendable { case save diff --git a/Yattee/Views/Settings/iCloudSettingsView.swift b/Yattee/Views/Settings/iCloudSettingsView.swift index cff5dc85..4bcfc34c 100644 --- a/Yattee/Views/Settings/iCloudSettingsView.swift +++ b/Yattee/Views/Settings/iCloudSettingsView.swift @@ -540,6 +540,13 @@ struct iCloudSettingsView: View { case .instances: settingsManager?.syncInstances = true instancesManager?.replaceWithiCloudData() + // Accounts ride this category. Records that arrived while it was off + // were kept, unapplied — activate them now rather than resetting the + // engine's token, which would discard other categories' pending + // changes. + Task { + await appEnvironment?.accountSyncCoordinator.activate() + } case .subscriptions: settingsManager?.syncSubscriptions = true // Upload existing subscriptions to CloudKit diff --git a/YatteeTests/AccountSyncTests.swift b/YatteeTests/AccountSyncTests.swift new file mode 100644 index 00000000..c93702a8 --- /dev/null +++ b/YatteeTests/AccountSyncTests.swift @@ -0,0 +1,693 @@ +// +// AccountSyncTests.swift +// YatteeTests +// +// CloudKit sync for accounts, migration markers, deletion tombstones and the +// selected data context. +// + +import CloudKit +import Foundation +import Testing +@testable import Yattee + +@Suite("Account sync", .serialized) +@MainActor +struct AccountSyncTests { + // MARK: - Fixtures + + private static func makeDefaults() -> UserDefaults { + UserDefaults(suiteName: "yattee.tests.\(UUID().uuidString)")! + } + + private static func makeMapper() -> CloudKitRecordMapper { + CloudKitRecordMapper(zone: RecordType.createZone()) + } + + /// Collects what the coordinator asks the engine to send. + private final class Recorder { + private(set) var changes: [CKSyncEngine.PendingRecordZoneChange] = [] + + func record(_ new: [CKSyncEngine.PendingRecordZoneChange]) { + changes.append(contentsOf: new) + } + + func clear() { changes.removeAll() } + + var savedNames: [String] { + changes.compactMap { + if case let .saveRecord(id) = $0 { return id.recordName } + return nil + } + } + + var deletedNames: [String] { + changes.compactMap { + if case let .deleteRecord(id) = $0 { return id.recordName } + return nil + } + } + } + + private final class CategorySwitch { + var isEnabled: Bool + init(_ isEnabled: Bool) { self.isEnabled = isEnabled } + } + + private struct Harness { + let defaults: UserDefaults + let accounts: AccountsManager + let store: AccountSyncStore + let coordinator: AccountSyncCoordinator + let recorder: Recorder + let category: CategorySwitch + let mapper: CloudKitRecordMapper + let scope: String + } + + /// A coordinator with its bootstrap already satisfied. + /// + /// The bootstrap needs a live database, so tests that are about what happens + /// *after* it record the flag directly; `bootstrapped: false` covers the + /// gate itself. + private static func makeHarness( + defaults: UserDefaults? = nil, + scope: String = "identity-A", + categoryEnabled: Bool = true, + bootstrapped: Bool = true, + instances: InstancesManager? = nil + ) -> Harness { + let defaults = defaults ?? makeDefaults() + let accounts = AccountsManager( + identityScope: CredentialIdentityScope(fixedScope: scope), + instancesManager: instances, + defaults: defaults + ) + let store = AccountSyncStore(defaults: defaults) + if bootstrapped { + store.markBootstrapCompleted(in: scope) + } + + let category = CategorySwitch(categoryEnabled) + let recorder = Recorder() + let coordinator = AccountSyncCoordinator( + accountsManager: accounts, + identityScope: CredentialIdentityScope(fixedScope: scope), + store: store, + isCategoryEnabled: { category.isEnabled } + ) + coordinator.attach(database: nil) { recorder.record($0) } + + return Harness( + defaults: defaults, + accounts: accounts, + store: store, + coordinator: coordinator, + recorder: recorder, + category: category, + mapper: makeMapper(), + scope: scope + ) + } + + // MARK: - Record mapping + + @Suite("Record mapping", .serialized) + @MainActor + struct MappingTests { + @Test("An account survives the record round-trip") + func accountRoundTrip() throws { + let mapper = AccountSyncTests.makeMapper() + let account = Account( + instanceID: UUID(), + displayName: "Main", + dateAdded: Date(timeIntervalSince1970: 100), + updatedAt: Date(timeIntervalSince1970: 200) + ) + + let record = mapper.toCKRecord(account: account) + #expect(record.recordType == RecordType.account) + #expect(record.recordID.recordName == "account-\(account.id.uuidString)") + + let decoded = try mapper.toAccount(from: record) + #expect(decoded == account) + } + + @Test("The login identifier never enters the synced record") + func recordCarriesNoLoginIdentifier() { + let mapper = AccountSyncTests.makeMapper() + let record = mapper.toCKRecord(account: Account(instanceID: UUID(), displayName: "Main")) + + // Only the user-set label syncs; the username lives in the Keychain + // beside the credential. + #expect(record["username"] == nil) + #expect(record.allKeys().sorted() == ["accountID", "dateAdded", "displayName", "instanceID", "schemaVersion", "updatedAt"]) + } + + @Test("A record whose payload disagrees with its name is rejected") + func mismatchedRecordNameIsRejected() { + let mapper = AccountSyncTests.makeMapper() + let record = mapper.toCKRecord(account: Account(instanceID: UUID())) + + // Every device derives the record name from the account ID, so a + // record claiming a different ID would resolve conflicts against an + // account other than the one it overwrites. + record["accountID"] = UUID().uuidString as CKRecordValue + + #expect(throws: (any Error).self) { try mapper.toAccount(from: record) } + } + + @Test("Migration markers and deletion tombstones round-trip") + func writeOnceRecordsRoundTrip() throws { + let mapper = AccountSyncTests.makeMapper() + let instanceID = UUID() + let accountID = UUID() + + let marker = mapper.toCKRecord(legacyMigrationInstanceID: instanceID) + #expect(marker.recordID.recordName == "legacymigrated-\(instanceID.uuidString)") + #expect(try mapper.toLegacyMigrationInstanceID(from: marker) == instanceID) + + let tombstone = mapper.toCKRecord(accountDeletionID: accountID) + #expect(tombstone.recordID.recordName == "accountDeletion-\(accountID.uuidString)") + #expect(try mapper.toAccountDeletionID(from: tombstone) == accountID) + } + + @Test("Both context cases round-trip through the singleton record") + func dataContextRoundTrip() throws { + let mapper = AccountSyncTests.makeMapper() + let accountID = UUID() + + let local = StoredDataContext(context: .local, changedAt: Date(timeIntervalSince1970: 10)) + let localRecord = mapper.toCKRecord(dataContext: local) + #expect(localRecord.recordID.recordName == "datacontext-current") + #expect(try mapper.toStoredDataContext(from: localRecord) == local) + + let account = StoredDataContext(context: .account(accountID), changedAt: Date(timeIntervalSince1970: 20)) + #expect(try mapper.toStoredDataContext(from: mapper.toCKRecord(dataContext: account)) == account) + } + + @Test("An account record name is never mistaken for a deletion record") + func recordNameParsingIsUnambiguous() { + let id = UUID() + + #expect(AccountRecordName.parse("account-\(id.uuidString)") == .account(id)) + #expect(AccountRecordName.parse("accountDeletion-\(id.uuidString)") == .accountDeletion(accountID: id)) + #expect(AccountRecordName.parse("legacymigrated-\(id.uuidString)") == .legacyMigration(instanceID: id)) + #expect(AccountRecordName.parse("datacontext-current") == .dataContext) + + // Other categories must never be routed here. + #expect(AccountRecordName.parse("sub-abc@global:youtube") == nil) + #expect(!AccountRecordName.isAccountCategory("instance-\(id.uuidString)")) + } + } + + // MARK: - Conflict resolution + + @Suite("Conflict resolution", .serialized) + @MainActor + struct ConflictTests { + @Test("An account resolves on the app-managed updatedAt") + func accountResolvesByUpdatedAt() async { + let mapper = AccountSyncTests.makeMapper() + let resolver = CloudKitConflictResolver() + let id = UUID() + let instanceID = UUID() + + let older = mapper.toCKRecord(account: Account( + id: id, instanceID: instanceID, displayName: "Old", + updatedAt: Date(timeIntervalSince1970: 100) + )) + let newer = mapper.toCKRecord(account: Account( + id: id, instanceID: instanceID, displayName: "New", + updatedAt: Date(timeIntervalSince1970: 200) + )) + + let localWins = await resolver.resolveAccountConflict(local: newer, server: older) + #expect(localWins["displayName"] as? String == "New") + + let serverWins = await resolver.resolveAccountConflict(local: older, server: newer) + #expect(serverWins["displayName"] as? String == "New") + } + + @Test("A migrated account loses to any real edit") + func migratedAccountLosesToRealEdit() async { + let mapper = AccountSyncTests.makeMapper() + let resolver = CloudKitConflictResolver() + let instanceID = UUID() + + // Synthesized accounts are stamped at the epoch precisely so an + // explicit edit outranks them whichever arrives first. + let migrated = mapper.toCKRecord(account: .migrated(from: instanceID)) + let edited = mapper.toCKRecord(account: Account( + id: instanceID, instanceID: instanceID, displayName: "Renamed", + updatedAt: Date(timeIntervalSince1970: 1) + )) + + let resolved = await resolver.resolveAccountConflict(local: migrated, server: edited) + #expect(resolved["displayName"] as? String == "Renamed") + } + + @Test("The context resolves on changedAt, not the record's write time") + func contextResolvesByChangedAt() async throws { + let mapper = AccountSyncTests.makeMapper() + let resolver = CloudKitConflictResolver() + let accountID = UUID() + + let older = mapper.toCKRecord(dataContext: StoredDataContext( + context: .local, changedAt: Date(timeIntervalSince1970: 100) + )) + let newer = mapper.toCKRecord(dataContext: StoredDataContext( + context: .account(accountID), changedAt: Date(timeIntervalSince1970: 200) + )) + + let localWins = await resolver.resolveDataContextConflict(local: newer, server: older) + #expect(try mapper.toStoredDataContext(from: localWins).context == .account(accountID)) + + let serverWins = await resolver.resolveDataContextConflict(local: older, server: newer) + #expect(try mapper.toStoredDataContext(from: serverWins).context == .account(accountID)) + } + + @Test("Write-once records resolve to the server copy") + func writeOnceResolvesToServer() async { + let mapper = AccountSyncTests.makeMapper() + let resolver = CloudKitConflictResolver() + let accountID = UUID() + + let local = mapper.toCKRecord(accountDeletionID: accountID, deletedAt: Date(timeIntervalSince1970: 1)) + let server = mapper.toCKRecord(accountDeletionID: accountID, deletedAt: Date(timeIntervalSince1970: 2)) + + let resolved = await resolver.resolveWriteOnceConflict(local: local, server: server) + #expect(resolved["deletedAt"] as? Date == Date(timeIntervalSince1970: 2)) + } + } + + // MARK: - Staging + + @Suite("Fetch staging", .serialized) + @MainActor + struct StagingTests { + @Test("Staged records survive a relaunch") + func stagingIsDurable() { + let defaults = AccountSyncTests.makeDefaults() + let mapper = AccountSyncTests.makeMapper() + let account = Account(instanceID: UUID()) + + AccountSyncStore(defaults: defaults).stage( + records: [mapper.toCKRecord(account: account)], + deletions: [], + in: "identity-A" + ) + + // A crash between the fetch event and its application must not lose + // records the engine's change token has already moved past. + let reopened = AccountSyncStore(defaults: defaults) + let staged = reopened.stagedChanges(in: "identity-A") + #expect(staged.count == 1) + #expect(staged.first?.record()?.recordID.recordName == "account-\(account.id.uuidString)") + } + + @Test("A later change to the same record replaces the earlier one") + func stagingCollapsesByRecordName() { + let store = AccountSyncStore(defaults: AccountSyncTests.makeDefaults()) + let mapper = AccountSyncTests.makeMapper() + let account = Account(instanceID: UUID(), displayName: "First") + + store.stage(records: [mapper.toCKRecord(account: account)], deletions: [], in: "A") + + var renamed = account + renamed.displayName = "Second" + store.stage(records: [mapper.toCKRecord(account: renamed)], deletions: [], in: "A") + + let staged = store.stagedChanges(in: "A") + #expect(staged.count == 1) + #expect(staged.first?.record()?["displayName"] as? String == "Second") + } + + @Test("Staging and the bootstrap flag are per identity") + func stagingIsNamespaced() { + let store = AccountSyncStore(defaults: AccountSyncTests.makeDefaults()) + let mapper = AccountSyncTests.makeMapper() + + store.stage(records: [mapper.toCKRecord(account: Account(instanceID: UUID()))], deletions: [], in: "A") + store.markBootstrapCompleted(in: "A") + + // Each identity fetches, stages and bootstraps entirely on its own. + #expect(store.stagedChanges(in: "B").isEmpty) + #expect(!store.isBootstrapCompleted(in: "B")) + #expect(store.isBootstrapCompleted(in: "A")) + } + } + + // MARK: - Activation + + @Test("A tombstone from a later event still beats the account from an earlier one") + func tombstonesApplyBeforeAccounts() async { + let harness = Self.makeHarness() + let account = Account(instanceID: UUID()) + + // CKSyncEngine emits several fetch events per fetch; the account can + // land in the first and its deletion in the third. + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: account)], deletions: []) + harness.coordinator.stageFetched( + records: [harness.mapper.toCKRecord(accountDeletionID: account.id)], + deletions: [] + ) + + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id) == nil) + #expect(harness.accounts.isDeleted(account.id)) + } + + @Test("Nothing is applied until the zone fetch has actually succeeded") + func failedZoneFetchLeavesRecordsStaged() async { + let harness = Self.makeHarness() + let account = Account(instanceID: UUID()) + + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: account)], deletions: []) + + // didFetchChanges fires after failed zone fetches too, so the zone's own + // result is the gate. + await harness.coordinator.zoneFetchCompleted( + zoneID: RecordType.createZone().zoneID, + error: CKError(.networkFailure) + ) + #expect(harness.accounts.account(id: account.id) == nil) + #expect(!harness.store.stagedChanges(in: harness.scope).isEmpty) + + await harness.coordinator.zoneFetchCompleted(zoneID: RecordType.createZone().zoneID, error: nil) + #expect(harness.accounts.account(id: account.id) != nil) + #expect(harness.store.stagedChanges(in: harness.scope).isEmpty) + } + + @Test("A disabled category defers records instead of discarding them") + func disabledCategoryDefersRecords() async { + let harness = Self.makeHarness(categoryEnabled: false) + let account = Account(instanceID: UUID()) + + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: account)], deletions: []) + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id) == nil) + #expect(!harness.store.stagedChanges(in: harness.scope).isEmpty) + #expect(harness.recorder.changes.isEmpty) + + // Re-enabling applies what was kept — no token reset, which would throw + // away other categories' pending changes. + harness.category.isEnabled = true + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id) != nil) + } + + @Test("Nothing activates or uploads before the identity has bootstrapped") + func bootstrapGatesActivationAndUpload() async { + let harness = Self.makeHarness(bootstrapped: false) + let account = Account(instanceID: UUID()) + + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: account)], deletions: []) + _ = harness.accounts.addAccount(instanceID: UUID()) + harness.recorder.clear() + + // No database is attached, so the bootstrap cannot complete. + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id) == nil) + #expect(harness.recorder.changes.isEmpty) + #expect(harness.coordinator.recordToSave(named: "datacontext-current") == nil) + } + + @Test("An account that outlives its tombstone is ignored and deleted again") + func resurrectedAccountIsRepaired() async { + let harness = Self.makeHarness() + let account = Account(instanceID: UUID()) + + harness.accounts.removeAccount(harness.accounts.addAccount(instanceID: account.instanceID).id) + let deletedID = harness.accounts.deletedAccountIDs.first! + + // A device that never fetched the tombstone can recreate the record; + // CloudKit enforces no relationship between the two. + let resurrected = Account(id: deletedID, instanceID: account.instanceID, updatedAt: Date()) + harness.recorder.clear() + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: resurrected)], deletions: []) + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: deletedID) == nil) + #expect(harness.recorder.deletedNames.contains("account-\(deletedID.uuidString)")) + } + + @Test("An account record deletion removes the account and keeps a tombstone queued") + func deletionEventPublishesItsOwnTombstone() async { + let harness = Self.makeHarness() + let account = Account(instanceID: UUID()) + + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: account)], deletions: []) + await harness.coordinator.activate() + #expect(harness.accounts.account(id: account.id) != nil) + + let recordID = CKRecord.ID( + recordName: "account-\(account.id.uuidString)", + zoneID: RecordType.createZone().zoneID + ) + harness.coordinator.stageFetched(records: [], deletions: [recordID]) + harness.recorder.clear() + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id) == nil) + // A bare record deletion is not durable, so this device publishes the + // tombstone that is. + #expect(harness.accounts.pendingDeletions.contains(account.id)) + #expect(harness.recorder.savedNames.contains("accountDeletion-\(account.id.uuidString)")) + } + + @Test("A local copy that is newer than the server's is pushed back") + func staleServerRecordIsCorrected() async { + let harness = Self.makeHarness() + let account = harness.accounts.addAccount(instanceID: UUID(), displayName: "Local") + harness.accounts.confirmUpload(account.id) + + var stale = account + stale.displayName = "Stale" + stale.updatedAt = account.updatedAt.addingTimeInterval(-60) + + harness.recorder.clear() + harness.coordinator.stageFetched(records: [harness.mapper.toCKRecord(account: stale)], deletions: []) + await harness.coordinator.activate() + + #expect(harness.accounts.account(id: account.id)?.displayName == "Local") + #expect(harness.accounts.pendingUploads.contains(account.id)) + #expect(harness.recorder.savedNames.contains("account-\(account.id.uuidString)")) + } + + // MARK: - Upload ordering + + @Test("A migration marker waits for the account it describes") + func markerWaitsForItsAccount() async { + let harness = Self.makeHarness() + let instanceID = UUID() + let account = harness.accounts.addAccount(instanceID: instanceID) + + #expect(harness.accounts.pendingMarkerUploads.contains(instanceID)) + #expect(!harness.accounts.canUploadMarker(instanceID: instanceID)) + #expect(harness.coordinator.recordToSave(named: "legacymigrated-\(instanceID.uuidString)") == nil) + + harness.recorder.clear() + harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: account)]) + + #expect(!harness.accounts.pendingUploads.contains(account.id)) + #expect(harness.recorder.savedNames.contains("legacymigrated-\(instanceID.uuidString)")) + #expect(harness.coordinator.recordToSave(named: "legacymigrated-\(instanceID.uuidString)") != nil) + } + + @Test("The context waits for the account it names") + func contextWaitsForItsAccount() async { + let harness = Self.makeHarness() + let account = harness.accounts.addAccount(instanceID: UUID()) + harness.accounts.setContext(.account(account.id)) + + // A fresh device could otherwise receive the selection, complete a + // fetch that lacks the account, and read pending as removed. + #expect(!harness.accounts.canUploadContext) + #expect(harness.coordinator.recordToSave(named: "datacontext-current") == nil) + + harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: account)]) + #expect(harness.accounts.canUploadContext) + #expect(harness.coordinator.recordToSave(named: "datacontext-current") != nil) + } + + @Test("A deletion publishes its tombstone before removing the record") + func deletionIsTombstoneFirst() async { + let harness = Self.makeHarness() + let account = harness.accounts.addAccount(instanceID: UUID()) + harness.accounts.confirmUpload(account.id) + + harness.recorder.clear() + harness.accounts.removeAccount(account.id) + harness.coordinator.enqueueDirtyRecords() + + #expect(harness.recorder.savedNames.contains("accountDeletion-\(account.id.uuidString)")) + #expect(!harness.recorder.deletedNames.contains("account-\(account.id.uuidString)")) + + // The record deletion is registered only once the durable half landed. + harness.recorder.clear() + harness.coordinator.didSave(records: [harness.mapper.toCKRecord(accountDeletionID: account.id)]) + + #expect(!harness.accounts.pendingDeletions.contains(account.id)) + #expect(harness.recorder.deletedNames.contains("account-\(account.id.uuidString)")) + } + + @Test("A deleted account is never reinserted after unknownItem") + func unknownItemNeverResurrectsADeletedAccount() { + let harness = Self.makeHarness() + let account = harness.accounts.addAccount(instanceID: UUID()) + + #expect(harness.coordinator.shouldRetryAsInsert(recordName: "account-\(account.id.uuidString)")) + + harness.accounts.removeAccount(account.id) + + // Retrying as an insert is exactly how a queued offline edit brings a + // deleted account back. + #expect(!harness.coordinator.shouldRetryAsInsert(recordName: "account-\(account.id.uuidString)")) + #expect(harness.coordinator.recordToSave(named: "account-\(account.id.uuidString)") == nil) + } + + @Test("An edit made while an upload was in flight stays dirty") + func editDuringUploadIsNotConfirmedAway() { + let harness = Self.makeHarness() + let account = harness.accounts.addAccount(instanceID: UUID()) + let sent = harness.mapper.toCKRecord(account: account) + + harness.accounts.updateDisplayName("Renamed", for: account.id) + harness.coordinator.didSave(records: [sent]) + + #expect(harness.accounts.pendingUploads.contains(account.id)) + } + + @Test("Seeding an empty zone re-registers every account record") + func fullUploadIncludesTombstonesAndMarkers() { + let harness = Self.makeHarness() + let kept = harness.accounts.addAccount(instanceID: UUID()) + let removed = harness.accounts.addAccount(instanceID: UUID()) + harness.accounts.removeAccount(removed.id) + harness.accounts.confirmUpload(kept.id) + harness.accounts.confirmDeletion(removed.id) + harness.accounts.confirmMarkerUpload(instanceID: kept.instanceID) + + harness.recorder.clear() + harness.coordinator.uploadEverything() + + #expect(harness.recorder.savedNames.contains("account-\(kept.id.uuidString)")) + // Without the tombstone, a device that still has the account could put + // it back into the fresh zone. + #expect(harness.recorder.savedNames.contains("accountDeletion-\(removed.id.uuidString)")) + + // The upload ordering still holds during a reseed: the marker follows + // the account it describes rather than racing it. + #expect(!harness.recorder.savedNames.contains("legacymigrated-\(kept.instanceID.uuidString)")) + + harness.recorder.clear() + harness.coordinator.didSave(records: [harness.mapper.toCKRecord(account: kept)]) + #expect(harness.recorder.savedNames.contains("legacymigrated-\(kept.instanceID.uuidString)")) + } + + // MARK: - Identity + + @Test("Records staged for one Apple account are invisible to another") + func stagingNeverCrossesIdentities() async { + let defaults = Self.makeDefaults() + let account = Account(instanceID: UUID()) + + let identityA = Self.makeHarness(defaults: defaults, scope: "identity-A") + identityA.coordinator.stageFetched( + records: [identityA.mapper.toCKRecord(account: account)], + deletions: [] + ) + + let identityB = Self.makeHarness(defaults: defaults, scope: "identity-B") + await identityB.coordinator.activate() + + #expect(identityB.accounts.accounts.isEmpty) + + await identityA.coordinator.activate() + #expect(identityA.accounts.account(id: account.id) != nil) + } + + @Test("Records fetched before the identity resolves are recovered by a bootstrap") + func unresolvedIdentityForcesAnotherBootstrap() { + let defaults = Self.makeDefaults() + let store = AccountSyncStore(defaults: defaults) + store.markBootstrapCompleted(in: "identity-A") + + let accounts = AccountsManager( + identityScope: CredentialIdentityScope(fixedScope: nil), + defaults: defaults + ) + let identity = Identity(nil) + let coordinator = AccountSyncCoordinator( + accountsManager: accounts, + namespaceProvider: { identity.value }, + store: store, + isCategoryEnabled: { true } + ) + + // Staging under a guessed identity would apply — and later upload — one + // Apple account's records under another's. + coordinator.stageFetched( + records: [AccountSyncTests.makeMapper().toCKRecord(account: Account(instanceID: UUID()))], + deletions: [] + ) + #expect(store.stagedChanges(in: "identity-A").isEmpty) + #expect(store.stagedChanges(in: "unresolved").isEmpty) + + // The engine's token has moved past those records, so the identity that + // resolves has to fetch the zone in full again to see them. + identity.value = "identity-A" + coordinator.identityDidChange() + #expect(!store.isBootstrapCompleted(in: "identity-A")) + } + + @Test("A switch of Apple account never claims records dropped while unresolved") + func switchedIdentityDoesNotClaimDroppedRecords() { + let defaults = Self.makeDefaults() + let store = AccountSyncStore(defaults: defaults) + store.markBootstrapCompleted(in: "identity-A") + store.markBootstrapCompleted(in: "identity-B") + + let identity = Identity("identity-A") + let accounts = AccountsManager( + identityScope: CredentialIdentityScope(fixedScope: "identity-A"), + defaults: defaults + ) + let coordinator = AccountSyncCoordinator( + accountsManager: accounts, + namespaceProvider: { identity.value }, + store: store, + isCategoryEnabled: { true } + ) + + // Signed out mid-session, then signed in as someone else. Records that + // arrived in between could have come from either side. + identity.value = nil + coordinator.stageFetched( + records: [AccountSyncTests.makeMapper().toCKRecord(account: Account(instanceID: UUID()))], + deletions: [] + ) + coordinator.identityDidChange() + + identity.value = "identity-B" + coordinator.identityDidChange() + + // Neither identity is handed the other's records; both recover their + // own from their own zone instead. + #expect(store.stagedChanges(in: "identity-A").isEmpty) + #expect(store.stagedChanges(in: "identity-B").isEmpty) + #expect(!store.isBootstrapCompleted(in: "identity-A")) + #expect(!store.isBootstrapCompleted(in: "identity-B")) + } + + /// A mutable identity, for driving scope changes deterministically. + private final class Identity { + var value: String? + init(_ value: String?) { self.value = value } + } +}