diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index 4d8e3266..df973fd6 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -250,6 +250,10 @@ final class AccountsManager { /// writing anything. private func correctContextAfterDeletion() { storedContext = StoredDataContext(context: .local, changedAt: storedContext.changedAt) + // Including any upload the deleted choice had queued: sending it now + // would publish this fallback under that choice's timestamp, which is + // exactly what not stamping it "now" is meant to prevent. + pendingContextUpload = false NotificationCenter.default.post(name: .dataContextDidChange, object: nil) } @@ -444,10 +448,20 @@ final class AccountsManager { /// pushed back over the stale server copy. @discardableResult func applySyncedContext(_ incoming: StoredDataContext) -> Bool { + // Before the user has ever chosen here there is nothing to order + // against: the local value is a default stamped when this manager was + // constructed, so a synced choice made yesterday would lose to it and a + // fresh device would never adopt the selection at all. + guard hasStoredContext else { + storedContext = incoming + hasStoredContext = true + save() + NotificationCenter.default.post(name: .dataContextDidChange, object: nil) + return false + } + 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 } + guard storedContext.changedAt > incoming.changedAt else { return false } pendingContextUpload = true save() return true diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index ecac8256..a05ab153 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -389,18 +389,29 @@ final class AppEnvironment { // is global rather than account-keyed, so it would otherwise keep // serving the previous account's videos. // - // 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. + // Accounts and instances arriving are triggers too, not redundant ones: + // a context naming an account that has not synced yet — or one whose + // instance has not — cannot be mapped onto the legacy shape, and these + // are what restore the mapping once it can be made rather than leaving + // that moment permanent. The same arrivals are what make a synced + // 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. + // // Seeded with what is on screen now, so launching does not look like a // change and throw away caches that are still the right ones. lastMirroredContext = accounts.currentContext - for name: Notification.Name in [.dataContextDidChange, .accountsDidChange] { + for name: Notification.Name in [.dataContextDidChange, .accountsDidChange, .instancesDidChange] { contextMirrorObservers.append( NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in - MainActor.assumeIsolated { self?.mirrorContextToLegacySelection() } + MainActor.assumeIsolated { + guard let self else { return } + if name != .dataContextDidChange { + self.refreshAccountStatuses() + } + self.mirrorContextToLegacySelection() + } } ) } @@ -612,6 +623,23 @@ final class AppEnvironment { ) } + /// Discards everything cached under an account's previous credential. + /// + /// Signing in again keeps the account ID and the selected context, so + /// nothing about the *context* changes — but the login behind it may be a + /// different person entirely, and their subscriptions and feed would + /// otherwise stay on screen. + func accountCredentialDidChange(for account: Account) { + subscriptionService.discardProvider(for: account.id) + HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed) + + if accountsManager.currentContext == .account(account.id) { + SubscriptionFeedCache.shared.handleAccountChange() + } + + refreshAccountStatuses() + } + /// Drops everything cached for whichever account was being shown. /// /// Neither cache is keyed by account: the subscriptions feed is a single diff --git a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift index e4400ac9..c6d25d0a 100644 --- a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift @@ -110,8 +110,12 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { func unsubscribe(from channelID: String) async throws { let (instance, sid) = try await getAuthenticatedInstance() - // Unsubscribe on Invidious server - try await invidiousAPI.unsubscribe(from: channelID, instance: instance, sid: sid) + // Unsubscribe on Invidious server. Tracked like every other + // authenticated call: a rejected credential noticed here must mark the + // account, or it keeps reporting itself as signed in. + try await tracking { + try await invidiousAPI.unsubscribe(from: channelID, instance: instance, sid: sid) + } // Update in-memory cache only cachedChannelIDs.remove(channelID) diff --git a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift index 61da5db7..5ac93231 100644 --- a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift @@ -93,8 +93,12 @@ final class PipedSubscriptionProvider: SubscriptionProvider { func subscribe(to channel: Channel) async throws { let (instance, authToken) = try await getAuthenticatedInstance() - // Subscribe on Piped server - try await pipedAPI.subscribe(channelID: channel.id.channelID, instance: instance, authToken: authToken) + // Subscribe on Piped server. Tracked like every other authenticated + // call: a rejected credential noticed here must mark the account, or it + // keeps reporting itself as signed in. + try await tracking { + try await pipedAPI.subscribe(channelID: channel.id.channelID, instance: instance, authToken: authToken) + } // Update in-memory cache only cachedChannelIDs.insert(channel.id.channelID) @@ -109,7 +113,9 @@ final class PipedSubscriptionProvider: SubscriptionProvider { let (instance, authToken) = try await getAuthenticatedInstance() // Unsubscribe on Piped server - try await pipedAPI.unsubscribe(channelID: channelID, instance: instance, authToken: authToken) + try await tracking { + try await pipedAPI.unsubscribe(channelID: channelID, instance: instance, authToken: authToken) + } // Update in-memory cache only cachedChannelIDs.remove(channelID) diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index 419d1a02..8db4fc65 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -327,6 +327,12 @@ private struct EditRemoteServerContent: View { } } #endif + // Signing out happens in the accounts section below, which cannot reach + // this state directly. Without this the authenticated import actions + // stay on screen after the last account is removed, and then fail. + .onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in + isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false + } .onAppear { isLoggedIn = appEnvironment?.isSignedIn(to: instance) ?? false @@ -422,6 +428,12 @@ private struct EditRemoteServerContent: View { appEnvironment.accountsManager.setStatus(.signedIn, for: account.id) appEnvironment.instancesManager.setUsesAccountLogin(true, for: instance) + + // Signing in again keeps the same account, so nothing downstream sees a + // change of context — but the credential behind it may now belong to + // somebody else, and their data must not survive the swap. + appEnvironment.accountCredentialDidChange(for: account) + isLoggedIn = true } diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 29c70fe3..ce1f0c3e 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -186,6 +186,24 @@ struct AccountsManagerTests { #expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id)) } + @Test("A fresh device adopts the synced context") + func firstSyncedContextIsAdopted() { + let manager = makeManager() + let accountID = UUID() + + // The local value is a default stamped when the manager was built, so + // it is always "newer" than a choice made on another device yesterday. + // Ordering against it would leave a fresh device on local forever. + let synced = StoredDataContext( + context: .account(accountID), + changedAt: Date().addingTimeInterval(-86_400) + ) + + #expect(!manager.applySyncedContext(synced)) + #expect(manager.currentContext == .account(accountID)) + #expect(!manager.pendingContextUpload) + } + @Test("Correcting away from a deleted account never outranks a real choice") func deletionCorrectionIsNotAChoice() { let manager = makeManager() @@ -202,6 +220,7 @@ struct AccountsManagerTests { // 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) + // Including any upload the deleted choice had queued. #expect(!manager.pendingContextUpload) let elsewhere = StoredDataContext(