diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 64bcc1f9..4d8e3266 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -207,8 +207,11 @@ final class AccountsManager { func removeAccount(_ accountID: UUID) { guard account(id: accountID) != nil else { return } - forgetAccount(accountID) + // Queued before anything else can persist: a crash in between would + // otherwise leave the account deleted here with no tombstone owed to + // the server, and every other device would keep — or recreate — it. pendingDeletions.insert(accountID) + forgetAccount(accountID) save() @@ -232,10 +235,24 @@ final class AccountsManager { // 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) + correctContextAfterDeletion() } } + /// Repairs a context left pointing at an account that is gone. + /// + /// Deliberately not a new choice: it keeps the timestamp of the selection + /// it replaces and is never published. Stamping it "now" would let an + /// automatic fallback outrank a deliberate choice another device made + /// between the deletion and this device hearing about it — and publishing + /// it would push that fallback over their choice. Every device that sees + /// the deletion performs the same repair, so they converge without anyone + /// writing anything. + private func correctContextAfterDeletion() { + storedContext = StoredDataContext(context: .local, changedAt: storedContext.changedAt) + NotificationCenter.default.post(name: .dataContextDidChange, object: nil) + } + /// Marks a deletion as confirmed by the server. Until then it stays in the /// outbox and is retried, because a tombstone that never lands lets a stale /// device resurrect the account. @@ -366,8 +383,8 @@ final class AccountsManager { @discardableResult func applyRemoteAccountDeletionEvent(_ accountID: UUID) -> Account? { let removed = account(id: accountID) - forgetAccount(accountID) pendingDeletions.insert(accountID) + forgetAccount(accountID) save() NotificationCenter.default.post(name: .accountsDidChange, object: nil) @@ -649,6 +666,10 @@ final class AccountsManager { reconcileLegacyAccounts() NotificationCenter.default.post(name: .accountsDidChange, object: nil) + // The context belongs to the namespace too, so this reload replaced it. + // Anything keyed to the selection — the legacy setting, the feed cache — + // would otherwise keep serving the previous Apple account's data. + NotificationCenter.default.post(name: .dataContextDidChange, object: nil) } } diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 5e53eace..b07474f5 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -68,7 +68,7 @@ final class AppEnvironment { private(set) var activeControlsCenterSettings: CenterSectionSettings = .default @ObservationIgnored private var controlsSettingsObservers: [NSObjectProtocol] = [] - @ObservationIgnored private var contextMirrorObserver: NSObjectProtocol? + @ObservationIgnored private var contextMirrorObservers: [NSObjectProtocol] = [] // MARK: - Shared Instance @@ -385,12 +385,17 @@ final class AppEnvironment { // setting several views still read has to follow it, and the feed cache // is global rather than account-keyed, so it would otherwise keep // serving the previous account's videos. - contextMirrorObserver = NotificationCenter.default.addObserver( - forName: .dataContextDidChange, - object: nil, - queue: .main - ) { [weak self] _ in - MainActor.assumeIsolated { self?.mirrorContextToLegacySelection() } + // + // Accounts arriving is a second trigger, not a redundant one: a context + // naming an account that has not synced yet cannot be mapped onto the + // legacy shape, and this is what restores the mapping once it can be + // made rather than leaving that moment permanent. + for name: Notification.Name in [.dataContextDidChange, .accountsDidChange] { + contextMirrorObservers.append( + NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in + MainActor.assumeIsolated { self?.mirrorContextToLegacySelection() } + } + ) } // A deletion that arrives from another device takes the credential with @@ -416,7 +421,15 @@ final class AppEnvironment { // relying on observer registration order to land after it would be // silently fragile. Task { @MainActor [weak self] in - self?.refreshAccountStatuses() + guard let self else { return } + self.refreshAccountStatuses() + + // The feed cache is not keyed by account, so whatever it holds + // belongs to the Apple account that just went away. Cleared + // unconditionally: unlike a context change, there is no state + // here worth keeping across identities. + SubscriptionFeedCache.shared.handleAccountChange() + self.mirrorContextToLegacySelection(resetWhenUnresolvable: true) } } @@ -525,7 +538,12 @@ final class AppEnvironment { /// selection synced from another device, or the selected account being /// deleted there. Phase 3's switcher removes the last readers of the legacy /// setting and this mirror with them. - private func mirrorContextToLegacySelection() { + /// - Parameter resetWhenUnresolvable: whether a context that cannot be + /// mapped should fall back to `.local` instead of leaving the previous + /// value in place. True only when the Apple account changed: keeping the + /// previous identity's selection is worse than losing a mapping, and the + /// correct value returns as soon as the new namespace's accounts arrive. + private func mirrorContextToLegacySelection(resetWhenUnresolvable: Bool = false) { let legacy: SubscriptionAccount switch accountsManager.currentContext { @@ -538,7 +556,12 @@ final class AppEnvironment { // context is preserved, and the validator reports it unavailable // rather than pointing the UI at something else. guard let account = accountsManager.account(id: id), - let instance = accountsManager.instance(for: account) else { return } + let instance = accountsManager.instance(for: account) else { + if resetWhenUnresolvable, settingsManager.subscriptionAccount != .local { + settingsManager.subscriptionAccount = .local + } + return + } switch instance.type { case .invidious: legacy = .invidious(instance.id) diff --git a/Yattee/Services/BackgroundRefresh/BackgroundFeedRefresher.swift b/Yattee/Services/BackgroundRefresh/BackgroundFeedRefresher.swift index 4a0787d3..a473de9a 100644 --- a/Yattee/Services/BackgroundRefresh/BackgroundFeedRefresher.swift +++ b/Yattee/Services/BackgroundRefresh/BackgroundFeedRefresher.swift @@ -41,10 +41,12 @@ final class BackgroundFeedRefresher { return } - // Route on the selected data context. An account context resolves its - // own instance, so a second login on the same instance refreshes the - // right user's feed. - switch appEnvironment.accountsManager.currentContext { + // Route on the *effective* context. An account context resolves its own + // instance, so a second login on the same instance refreshes the right + // user's feed — but an account whose instance is switched off or whose + // credential has not arrived falls back to local rather than sending + // background requests to a server the user is not currently using. + switch appEnvironment.dataContextValidator.effectiveContext { case .local: await performLocalAccountRefresh() case let .account(id): diff --git a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift index 9c0f2722..1581a846 100644 --- a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift +++ b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift @@ -127,6 +127,24 @@ final class SubscriptionAccountValidator { return } + // A selection whose account still exists is never corrected here. This + // validator predates account-scoped credentials and reads "not signed + // in" as "not available", which is now routinely temporary — an + // unresolved identity scope, or a credential iCloud Keychain has yet to + // deliver. Correcting on that would silently discard the user's choice + // and desynchronize this setting from the data context, which is + // preserved. Only a deletion removes an account, and that path corrects + // the context and this setting together. + if let instanceID = settingsManager.subscriptionAccount.instanceID, + let accountsManager, + !accountsManager.accounts(forInstance: instanceID).isEmpty { + LoggingService.shared.debug( + "Leaving the subscription account alone — its login has not been removed", + category: .general + ) + return + } + let previousAccount = settingsManager.subscriptionAccount // Find first available account to switch to diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 9c056aed..29c70fe3 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -186,6 +186,50 @@ struct AccountsManagerTests { #expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id)) } + @Test("Correcting away from a deleted account never outranks a real choice") + func deletionCorrectionIsNotAChoice() { + let manager = makeManager() + let account = manager.addAccount(instanceID: UUID()) + + manager.setContext(.account(account.id)) + let chosenAt = manager.storedContext.changedAt + manager.confirmContextUpload(changedAt: chosenAt) + + manager.removeAccount(account.id) + #expect(manager.currentContext == .local) + + // Another device picked something else between the deletion and this + // device hearing about it. Timestamping the fallback "now" would sink + // that choice — and publishing it would push the fallback over it. + #expect(manager.storedContext.changedAt == chosenAt) + #expect(!manager.pendingContextUpload) + + let elsewhere = StoredDataContext( + context: .account(UUID()), + changedAt: chosenAt.addingTimeInterval(1) + ) + #expect(!manager.applySyncedContext(elsewhere)) + #expect(manager.currentContext == elsewhere.context) + } + + @Test("A deletion is queued before anything else can persist") + func deletionOutboxIsQueuedFirst() { + let defaults = makeDefaults() + let manager = makeManager(defaults: defaults) + let account = manager.addAccount(instanceID: UUID()) + manager.setContext(.account(account.id)) + + manager.removeAccount(account.id) + + // Deleting the *selected* account also corrects the context, and every + // step of that writes storage. A crash before the tombstone was owed + // would leave the account gone here and alive everywhere else. + let relaunched = makeManager(defaults: defaults) + #expect(relaunched.pendingDeletions.contains(account.id)) + #expect(relaunched.isDeleted(account.id)) + #expect(relaunched.currentContext == .local) + } + @Test("A migrated context loses to any explicit choice") func migratedContextLosesToExplicitChoice() { let manager = makeManager()