diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index b07474f5..ecac8256 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -70,6 +70,9 @@ final class AppEnvironment { @ObservationIgnored private var controlsSettingsObservers: [NSObjectProtocol] = [] @ObservationIgnored private var contextMirrorObservers: [NSObjectProtocol] = [] + /// The context the account-scoped caches currently hold data for. + @ObservationIgnored private var lastMirroredContext: DataContext = .local + // MARK: - Shared Instance /// The single, process-wide app environment. @@ -390,6 +393,10 @@ final class AppEnvironment { // 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. + // 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] { contextMirrorObservers.append( NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in @@ -401,8 +408,21 @@ final class AppEnvironment { // 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 + accountSync.onAccountDeleted = { [weak self] accountID, account in guard let self else { return } + + guard let account else { + // No metadata for this account here, so its backend is unknown + // and the right credential manager cannot be picked. The scoped + // key is the account ID either way, so both are asked to drop + // it; at most one of them holds anything. The unscoped legacy + // item is left alone — deleting it needs the instance, and its + // ownership check is bound to one. + self.invidiousCredentialsManager.deleteCredential(forAccountID: accountID) + self.pipedCredentialsManager.deleteCredential(forAccountID: accountID) + return + } + let credentials = self.accountCredentialsManager(for: account) credentials?.deleteCredential(for: account) credentials?.deleteLegacyCredentialIfOwned(for: account) @@ -424,11 +444,11 @@ final class AppEnvironment { 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() + // Cleared unconditionally, because the identity moving means + // everything cached belongs to the Apple account that just went + // away — even when the new namespace happens to select the same + // context and the change comparison would find nothing. + self.clearAccountScopedCaches() self.mirrorContextToLegacySelection(resetWhenUnresolvable: true) } } @@ -544,9 +564,20 @@ final class AppEnvironment { /// 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 context = accountsManager.currentContext + + // Cached data is invalidated on the *context*, not on the legacy value + // it maps to: two logins on the same instance collapse onto the same + // legacy selection, so comparing that would let one account's private + // feed stay on screen after switching to the other. + if context != lastMirroredContext { + lastMirroredContext = context + clearAccountScopedCaches() + } + let legacy: SubscriptionAccount - switch accountsManager.currentContext { + switch context { case .local: legacy = .local @@ -554,7 +585,8 @@ final class AppEnvironment { // An account whose instance has not arrived yet cannot be mapped // onto the legacy shape. Leaving the old value alone is right: the // context is preserved, and the validator reports it unavailable - // rather than pointing the UI at something else. + // rather than pointing the UI at something else. A later arrival + // runs this again and restores the mapping. guard let account = accountsManager.account(id: id), let instance = accountsManager.instance(for: account) else { if resetWhenUnresolvable, settingsManager.subscriptionAccount != .local { @@ -573,14 +605,23 @@ final class AppEnvironment { guard settingsManager.subscriptionAccount != legacy else { return } settingsManager.subscriptionAccount = legacy - SubscriptionFeedCache.shared.handleAccountChange() LoggingService.shared.info( - "Data context changed — cleared the feed cache and updated the legacy selection", + "Data context changed — updated the legacy subscription selection", category: .general ) } + /// Drops everything cached for whichever account was being shown. + /// + /// Neither cache is keyed by account: the subscriptions feed is a single + /// global list, and home feeds are keyed by instance alone. Both would + /// otherwise keep serving one login's private videos to another. + private func clearAccountScopedCaches() { + SubscriptionFeedCache.shared.handleAccountChange() + HomeInstanceCache.shared.clearAllFeeds() + } + /// Deletes an instance together with the accounts that live on it. /// /// The cascade has to happen here, while the instance is still resolvable: diff --git a/Yattee/Services/CloudKit/AccountSyncCoordinator.swift b/Yattee/Services/CloudKit/AccountSyncCoordinator.swift index df21de98..b8ada013 100644 --- a/Yattee/Services/CloudKit/AccountSyncCoordinator.swift +++ b/Yattee/Services/CloudKit/AccountSyncCoordinator.swift @@ -58,7 +58,13 @@ final class AccountSyncCoordinator { /// Called when an account is deleted anywhere, so its credential and stored /// login identifier go with it. - var onAccountDeleted: (@MainActor (Account) -> Void)? + /// + /// The account is `nil` when this device never had its metadata — a + /// tombstone and the account it kills are separate records, and a fresh + /// device can meet the tombstone first. The credential may still have + /// arrived on its own through iCloud Keychain, so the ID alone has to be + /// enough to remove it. + var onAccountDeleted: (@MainActor (UUID, Account?) -> Void)? // MARK: - Identity @@ -372,8 +378,7 @@ final class AccountSyncCoordinator { } private func cleanUpCredentials(for account: Account?, accountID: UUID) { - guard let account else { return } - onAccountDeleted?(account) + onAccountDeleted?(accountID, account) LoggingService.shared.logCloudKit("Removed deleted account from iCloud: \(accountID)") } diff --git a/Yattee/Services/Credentials/AccountCredentialsManager.swift b/Yattee/Services/Credentials/AccountCredentialsManager.swift index 226c9d8f..d88d4c94 100644 --- a/Yattee/Services/Credentials/AccountCredentialsManager.swift +++ b/Yattee/Services/Credentials/AccountCredentialsManager.swift @@ -32,6 +32,15 @@ protocol AccountCredentialsManager: AnyObject, Observable { func credential(for account: Account) -> String? func deleteCredential(for account: Account) + /// Deletes the credential and stored login identifier for an account this + /// device may know nothing else about. + /// + /// A deletion can reach this device before the account it deletes does — + /// the tombstone and the account are separate records, and iCloud Keychain + /// runs on its own timeline. The secret must go regardless, and the scoped + /// key needs only the account's ID. + func deleteCredential(forAccountID accountID: UUID) + /// The login identifier the user typed. Kept in the Keychain beside the /// credential so it never enters synced metadata. func setUsername(_ username: String, for account: Account) diff --git a/Yattee/Services/Credentials/AccountKeychainCredentials.swift b/Yattee/Services/Credentials/AccountKeychainCredentials.swift index 28ad0ebe..5f0958f4 100644 --- a/Yattee/Services/Credentials/AccountKeychainCredentials.swift +++ b/Yattee/Services/Credentials/AccountKeychainCredentials.swift @@ -107,19 +107,22 @@ final class AccountKeychainCredentials { } func deleteCredential(for account: Account) { + deleteCredential(forAccountID: account.id) + HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed) + } + + func deleteCredential(forAccountID accountID: UUID) { guard let scope = currentScope() else { return } - store.delete(key: KeychainCredentialStore.credentialKey(scope: scope, accountID: account.id)) - store.delete(key: KeychainCredentialStore.usernameKey(scope: scope, accountID: account.id)) - loginsByScope[scope]?.remove(account.id) + store.delete(key: KeychainCredentialStore.credentialKey(scope: scope, accountID: accountID)) + store.delete(key: KeychainCredentialStore.usernameKey(scope: scope, accountID: accountID)) + loginsByScope[scope]?.remove(accountID) LoggingService.shared.info( "Deleted \(credentialNoun)", category: .keychain, - details: "accountID=\(account.id)" + details: "accountID=\(accountID)" ) - - HomeInstanceCache.shared.clear(instanceID: account.instanceID, contentType: .feed) } // MARK: - Username diff --git a/Yattee/Services/Credentials/InvidiousCredentialsManager.swift b/Yattee/Services/Credentials/InvidiousCredentialsManager.swift index 2ecf8dec..808eecda 100644 --- a/Yattee/Services/Credentials/InvidiousCredentialsManager.swift +++ b/Yattee/Services/Credentials/InvidiousCredentialsManager.swift @@ -285,6 +285,10 @@ extension InvidiousCredentialsManager: AccountCredentialsManager { accountCredentials.deleteCredential(for: account) } + func deleteCredential(forAccountID accountID: UUID) { + accountCredentials.deleteCredential(forAccountID: accountID) + } + func setUsername(_ username: String, for account: Account) { accountCredentials.setUsername(username, for: account) } diff --git a/Yattee/Services/Credentials/PipedCredentialsManager.swift b/Yattee/Services/Credentials/PipedCredentialsManager.swift index d7d8cf12..9c46a9b4 100644 --- a/Yattee/Services/Credentials/PipedCredentialsManager.swift +++ b/Yattee/Services/Credentials/PipedCredentialsManager.swift @@ -214,6 +214,10 @@ extension PipedCredentialsManager: AccountCredentialsManager { accountCredentials.deleteCredential(for: account) } + func deleteCredential(forAccountID accountID: UUID) { + accountCredentials.deleteCredential(forAccountID: accountID) + } + func setUsername(_ username: String, for account: Account) { accountCredentials.setUsername(username, for: account) } diff --git a/Yattee/Services/HomeInstanceCache.swift b/Yattee/Services/HomeInstanceCache.swift index 404b9de5..d46ecaf9 100644 --- a/Yattee/Services/HomeInstanceCache.swift +++ b/Yattee/Services/HomeInstanceCache.swift @@ -176,6 +176,32 @@ final class HomeInstanceCache { } /// Clears all cached content for a specific instance. + /// Drops every cached feed, on every instance. + /// + /// Feeds are the one content type here that is private to a login, and the + /// cache key names only the instance — two accounts on the same instance, + /// or the same instance ID retained across a change of Apple account, would + /// otherwise be served each other's. Popular and trending are public and + /// stay. + func clearAllFeeds() { + let keysToRemove = cache.keys.filter { $0.hasSuffix("_\(InstanceContentType.feed.rawValue)") } + guard !keysToRemove.isEmpty else { return } + + for key in keysToRemove { + cache.removeValue(forKey: key) + lastUpdated.removeValue(forKey: key) + } + + LoggingService.shared.debug( + "HomeInstanceCache.clearAllFeeds: Cleared \(keysToRemove.count) cached feeds", + category: .general + ) + + Task { + await saveToDisk() + } + } + func clearAllForInstance(_ instanceID: UUID) { let keysToRemove = cache.keys.filter { $0.hasPrefix(instanceID.uuidString) } for key in keysToRemove { diff --git a/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift b/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift index 201ca752..f11e5be8 100644 --- a/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift +++ b/Yattee/Views/Playlist/UnifiedPlaylistDetailView.swift @@ -779,7 +779,11 @@ struct UnifiedPlaylistDetailView: View { // Check if this is an Invidious user playlist (IVPL prefix) that requires authentication let isInvidiousUserPlaylist = playlistID.playlistID.hasPrefix("IVPL") let isInvidiousInstance = resolvedInstance.type == .invidious - let sid = appEnvironment.invidiousCredentialsManager.sid(for: resolvedInstance) + // Resolved through the account, not the instance: a login created + // after the revamp only ever writes an account-scoped credential, + // so the instance-keyed lookup would miss it and fall through to + // the public endpoint — which cannot see a private playlist. + let sid = appEnvironment.credential(forInstance: resolvedInstance) if isInvidiousUserPlaylist && isInvidiousInstance, let sid { // Use authenticated endpoint for Invidious user playlists (including private ones) diff --git a/Yattee/Views/Settings/SourcesListView.swift b/Yattee/Views/Settings/SourcesListView.swift index 03111d5d..da3e703c 100644 --- a/Yattee/Views/Settings/SourcesListView.swift +++ b/Yattee/Views/Settings/SourcesListView.swift @@ -584,8 +584,8 @@ struct SourcesListView: View { // MARK: - Actions private func confirmDelete() { - if let instance = pendingDeleteInstance, let manager = instancesManager { - manager.remove(instance) + if let instance = pendingDeleteInstance { + appEnvironment?.removeInstance(instance) pendingDeleteInstance = nil }