diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 74fbefdd..182ebc1d 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -499,10 +499,17 @@ final class AccountsManager { // MARK: - Status + /// Records what the app currently knows about an account's usability. + /// + /// Announced separately from the account *list* changing. Recomputing + /// statuses from the Keychain is one of the things that listens for a list + /// change, and it cannot tell that a server rejected a credential or failed + /// to answer — so sharing one notification meant every status the API layer + /// recorded was overwritten by the reaction to recording it. func setStatus(_ status: AccountStatus, for accountID: UUID) { guard accountStatuses[accountID] != status else { return } accountStatuses[accountID] = status - NotificationCenter.default.post(name: .accountsDidChange, object: nil) + NotificationCenter.default.post(name: .accountStatusesDidChange, object: nil) } /// An account with no recorded status is *unknown*, not signed out. While @@ -719,5 +726,7 @@ final class AccountsManager { extension Notification.Name { static let accountsDidChange = Notification.Name("stream.yattee.accountsDidChange") + /// An account's usability changed — not the list of accounts. + static let accountStatusesDidChange = Notification.Name("stream.yattee.accountStatusesDidChange") static let dataContextDidChange = Notification.Name("stream.yattee.dataContextDidChange") } diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 9aad5949..685eb326 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -457,12 +457,19 @@ final class AppEnvironment { // account's credential findable, so statuses are recomputed with them: // an account that appears mid-session would otherwise sit at "sign in // required" until the app was relaunched. - for name: Notification.Name in [.dataContextDidChange, .accountsDidChange, .instancesDidChange] { + // A status change is deliberately not one of the refresh triggers: the + // refresher reads the Keychain, which cannot know what a server said, + // so reacting to a status by recomputing it would erase every verdict + // the API layer records. It still runs the mirror, because a credential + // arriving or being rejected moves the effective context. + for name: Notification.Name in [ + .dataContextDidChange, .accountsDidChange, .instancesDidChange, .accountStatusesDidChange + ] { contextMirrorObservers.append( NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in MainActor.assumeIsolated { guard let self else { return } - if name != .dataContextDidChange { + if name == .accountsDidChange || name == .instancesDidChange { self.refreshAccountStatuses() } self.mirrorContextToLegacySelection() @@ -685,11 +692,17 @@ final class AppEnvironment { let previous = defaults.string(forKey: Self.cacheOwnerKey) defaults.set(owner, forKey: Self.cacheOwnerKey) - guard let previous, previous != owner else { return } + guard previous != owner else { return } + // A missing owner is not the same as "ours". The first launch after + // upgrading finds caches written before anyone recorded who they belong + // to, and the Apple account may have changed since. Adopting them would + // be a guess with someone's private feed as the stake. clearAccountScopedCaches() LoggingService.shared.info( - "Apple account changed — discarded the previous account's cached data", + previous == nil + ? "Cached account data predates knowing who it belongs to — discarded it" + : "Apple account changed — discarded the previous account's cached data", category: .general ) } @@ -805,16 +818,21 @@ final class AppEnvironment { continue } - // A credential the server rejected — or a server that could not be - // reached — is still a credential, and only the server can say - // otherwise. Reporting "signed in" here because the Keychain item - // exists would erase those the moment they were recorded: the API - // layer sets them, and that posts the notification that runs this. - // The next successful call clears them; storing a new credential + // A credential the server rejected is still a credential: the + // Keychain cannot tell that it stopped working, so this must not + // report "signed in" over it. Only the server clears a rejection — + // on the next successful call — or storing a new credential, which // resets the status explicitly. + // + // `unreachable` is deliberately *not* preserved. It is transient, + // and it stops reads from reaching the server at all, so keeping it + // here would strand the account: nothing would ever call out to + // discover that the network came back. Recomputing it on the events + // that run this — foreground, an account or instance arriving — is + // what lets it retry. switch accountsManager.status(for: account.id) { - case .credentialRejected, .unreachable: continue - case .checking, .signedIn, .credentialMissing: break + case .credentialRejected: continue + case .checking, .signedIn, .credentialMissing, .unreachable: break } accountsManager.setStatus(.signedIn, for: account.id) diff --git a/Yattee/Services/SubscriptionService.swift b/Yattee/Services/SubscriptionService.swift index e7d2c6fa..93b9d641 100644 --- a/Yattee/Services/SubscriptionService.swift +++ b/Yattee/Services/SubscriptionService.swift @@ -157,8 +157,14 @@ final class SubscriptionService { /// served after the account is gone. func discardProvider(for accountID: UUID) { accountProviders[accountID] = nil + providerGeneration += 1 } + /// Which generation of logins the cached providers belong to. Bumped + /// whenever one is discarded, so a fetch that outlived its provider can be + /// told from a current one. + private var providerGeneration = 0 + /// The provider for **writes**. /// /// Always the raw selected context, never the read fallback: a subscribe @@ -289,9 +295,25 @@ final class SubscriptionService { isLoading = true lastError = nil + // Dropping a provider from the cache does not stop a fetch it has + // already started: the running task holds it, and the result would + // arrive afterwards as the current account's subscriptions. Signing in + // again or switching Apple account changes neither the context nor the + // account ID, so nothing else downstream would notice. + let generation = providerGeneration + do { let channels = try await provider.fetchSubscriptions() isLoading = false + + guard generation == providerGeneration else { + LoggingService.shared.info( + "Discarding subscriptions fetched for a login that has since been replaced", + category: .general + ) + throw SubscriptionError.providerNotAvailable + } + return channels } catch { lastError = error diff --git a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift index 1581a846..43733b53 100644 --- a/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift +++ b/Yattee/Services/Subscriptions/SubscriptionAccountValidator.swift @@ -150,6 +150,7 @@ final class SubscriptionAccountValidator { // Find first available account to switch to if let newAccount = availableAccounts.first { settingsManager.subscriptionAccount = newAccount + adoptCorrectedSelection(newAccount) feedCache.handleAccountChange() let newName = displayName(for: newAccount) @@ -166,6 +167,7 @@ final class SubscriptionAccountValidator { // No valid accounts available - set to local anyway // The feed will show an appropriate error state settingsManager.subscriptionAccount = .local + adoptCorrectedSelection(.local) feedCache.handleAccountChange() LoggingService.shared.warning( @@ -175,6 +177,20 @@ final class SubscriptionAccountValidator { } } + /// Moves the data context with a correction made to the legacy setting. + /// + /// Reads and writes resolve through the context now, so correcting only the + /// legacy value would leave the picker naming one account while the data + /// came from another. + private func adoptCorrectedSelection(_ account: SubscriptionAccount) { + guard let accountsManager else { return } + + let context = accountsManager.context(forLegacyInstanceID: account.instanceID) + guard context != accountsManager.currentContext else { return } + + accountsManager.setContext(context) + } + // MARK: - Display Names /// Returns a display name for the given subscription account. diff --git a/Yattee/Views/Subscriptions/ManageChannelsView.swift b/Yattee/Views/Subscriptions/ManageChannelsView.swift index bcb954ff..29f37133 100644 --- a/Yattee/Views/Subscriptions/ManageChannelsView.swift +++ b/Yattee/Views/Subscriptions/ManageChannelsView.swift @@ -281,7 +281,11 @@ struct ManageChannelsView: View { await refreshChannels() } } - .onChange(of: settingsManager?.subscriptionAccount) { _, _ in + // The data context, not the legacy setting: switching between two + // logins on the same instance leaves that value identical, and this + // view would keep listing one account's channels while unsubscribes + // went to the other. + .onReceive(NotificationCenter.default.publisher(for: .dataContextDidChange)) { _ in Task { await refreshChannels() }