From b07af6c15347a35baef8716d61af97f5cad5ec6d Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Thu, 27 Aug 2026 18:19:29 +0200 Subject: [PATCH] Separate account status from the account list, and stop stranding accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Recomputing statuses reads the Keychain, which cannot know that a server rejected a credential or failed to answer — and it ran in reaction to the same notification those verdicts were announced with, so it erased them as they were recorded. Status changes now have their own notification. With that separation `unreachable` no longer has to be preserved to survive: it is transient, and preserving it stopped every read from reaching the server, so nothing would ever discover that the network had come back. A fetch already running when its provider is discarded now has its result dropped. Signing in again — or switching Apple account — changes neither the context nor the account ID, so the previous login's channels would otherwise arrive as the current account's. Caches with no recorded owner are discarded rather than adopted. The first launch after upgrading finds feeds written before anyone recorded whose they were, and the Apple account may have changed since. The legacy validator's auto-correction moves the data context with it, and Manage Channels reloads on the context rather than the legacy value — it could list one account's channels while unsubscribes went to another. --- Yattee/Core/AccountsManager.swift | 11 ++++- Yattee/Core/AppEnvironment.swift | 42 +++++++++++++------ Yattee/Services/SubscriptionService.swift | 22 ++++++++++ .../SubscriptionAccountValidator.swift | 16 +++++++ .../Subscriptions/ManageChannelsView.swift | 6 ++- 5 files changed, 83 insertions(+), 14 deletions(-) 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() }