From 90652346066588142d1327d83c357c029073c35d Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Mon, 24 Aug 2026 23:56:40 +0200 Subject: [PATCH] Key subscription providers by account rather than backend type SubscriptionService held one provider per backend and each resolved its instance from settingsManager.subscriptionAccount, falling back to "the first enabled Invidious instance" when that was unset. With several logins possible on one instance that model breaks twice: two accounts would share a single in-memory subscription cache, and the fallback could send an account's request to a server it holds no credential for. Providers are now built per account and cached by account ID, and each resolves its instance and credential from the account it was built for - no fallback. The protocol's accountType becomes a DataContext, which is what actually identifies a provider now. Views that branched on "is the subscription account local" read the context instead, and the settings picker writes both stores. Keeping the legacy setting in step is deliberate for now: several views still read it, and letting the two diverge would show one account in the UI while serving another's data. The Phase 3 switcher removes the last readers. Signing out discards the account's provider so its cached subscriptions cannot be served after the account is gone. --- Yattee/Core/AppEnvironment.swift | 1 + Yattee/Services/SubscriptionService.swift | 104 ++++++++++++------ .../InvidiousSubscriptionProvider.swift | 33 +++--- .../LocalSubscriptionProvider.swift | 2 +- .../PipedSubscriptionProvider.swift | 33 +++--- .../Subscriptions/SubscriptionProvider.swift | 8 +- .../Settings/InstanceAccountsSection.swift | 3 + .../Settings/NotificationSettingsView.swift | 2 +- .../Settings/SubscriptionsSettingsView.swift | 7 +- .../Subscriptions/SubscriptionsView.swift | 4 +- 10 files changed, 120 insertions(+), 77 deletions(-) diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index caf8eba6..3f0b4c43 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -175,6 +175,7 @@ final class AppEnvironment { invidiousAPI: invidiousAPI, pipedAPI: pipedAPI ) + subscriptionService.setAccountsManager(accounts) // Initialize CloudKit Sync Engine let cloudKit = CloudKitSyncEngine( diff --git a/Yattee/Services/SubscriptionService.swift b/Yattee/Services/SubscriptionService.swift index 07b8eed6..ffb35548 100644 --- a/Yattee/Services/SubscriptionService.swift +++ b/Yattee/Services/SubscriptionService.swift @@ -47,8 +47,12 @@ final class SubscriptionService { // MARK: - Providers private var localProvider: LocalSubscriptionProvider? - private var invidiousProvider: InvidiousSubscriptionProvider? - private var pipedProvider: PipedSubscriptionProvider? + + /// One provider per account, built on demand and kept so its in-memory + /// subscription cache survives context switches. + private var accountProviders: [UUID: any SubscriptionProvider] = [:] + + private weak var accountsManager: AccountsManager? // MARK: - State @@ -77,41 +81,77 @@ final class SubscriptionService { self.invidiousAPI = invidiousAPI self.pipedAPI = pipedAPI - // Initialize providers + // Account providers are built lazily: which accounts exist is not known + // until AccountsManager has loaded and iCloud has delivered instances. self.localProvider = LocalSubscriptionProvider(dataManager: dataManager) - self.invidiousProvider = InvidiousSubscriptionProvider( - invidiousAPI: invidiousAPI, - credentialsManager: invidiousCredentialsManager, - instancesManager: instancesManager, - settingsManager: settingsManager - ) - self.pipedProvider = PipedSubscriptionProvider( - pipedAPI: pipedAPI, - credentialsManager: pipedCredentialsManager, - instancesManager: instancesManager, - settingsManager: settingsManager - ) + } + + func setAccountsManager(_ manager: AccountsManager) { + accountsManager = manager } // MARK: - Current Provider - /// Returns the current subscription provider based on the user's account setting. - var currentProvider: (any SubscriptionProvider)? { - let account = settingsManager.subscriptionAccount - - switch account.type { + /// The provider for a context, built on first use. + /// + /// Returns nil for an account whose instance or backend cannot supply + /// subscriptions — a caller that gets nil must surface that rather than + /// quietly falling back to local data the user did not select. + func provider(for context: DataContext) -> (any SubscriptionProvider)? { + switch context { case .local: return localProvider - case .invidious: - return invidiousProvider - case .piped: - return pipedProvider + + case let .account(id): + if let existing = accountProviders[id] { return existing } + + guard let accountsManager, + let account = accountsManager.account(id: id), + let instance = accountsManager.instance(for: account), + instance.type.accountCapabilities.supportsSubscriptions + else { return nil } + + let provider: (any SubscriptionProvider)? + switch instance.type { + case .invidious: + provider = InvidiousSubscriptionProvider( + account: account, + invidiousAPI: invidiousAPI, + credentialsManager: invidiousCredentialsManager, + accountsManager: accountsManager, + settingsManager: settingsManager + ) + case .piped: + provider = PipedSubscriptionProvider( + account: account, + pipedAPI: pipedAPI, + credentialsManager: pipedCredentialsManager, + accountsManager: accountsManager, + settingsManager: settingsManager + ) + default: + provider = nil + } + + accountProviders[id] = provider + return provider } } - /// The current subscription account type. - var currentAccountType: SubscriptionAccountType { - settingsManager.subscriptionAccount.type + /// Drops a removed account's provider so its cached subscriptions cannot be + /// served after the account is gone. + func discardProvider(for accountID: UUID) { + accountProviders[accountID] = nil + } + + /// Returns the current subscription provider for the selected context. + var currentProvider: (any SubscriptionProvider)? { + provider(for: currentContext) + } + + /// The selected context, or `.local` before accounts have loaded. + var currentContext: DataContext { + accountsManager?.currentContext ?? .local } /// Subscription count for the active account, when known without a network fetch. @@ -137,7 +177,7 @@ final class SubscriptionService { try await provider.subscribe(to: channel) postChangeNotificationForServerAccount() LoggingService.shared.info( - "Subscribed to \(channel.name) via \(provider.accountType)", + "Subscribed to \(channel.name) via \(provider.context)", category: .general ) } catch { @@ -183,7 +223,7 @@ final class SubscriptionService { try await provider.unsubscribe(from: channelID) postChangeNotificationForServerAccount() LoggingService.shared.info( - "Unsubscribed from \(channelID) via \(provider.accountType)", + "Unsubscribed from \(channelID) via \(provider.context)", category: .general ) } catch { @@ -234,7 +274,7 @@ final class SubscriptionService { /// Synchronously fetches subscriptions for local provider. /// Returns nil if the current provider is not local (requires async fetch). func fetchSubscriptionsSync() -> [Channel]? { - guard currentAccountType == .local else { + guard currentContext.isLocal else { return nil } @@ -276,7 +316,7 @@ final class SubscriptionService { /// Local accounts write to SwiftData; server accounts subscribe on the server, /// one channel at a time. Channels that are already subscribed are skipped. func importSubscriptions(_ channels: [(channelID: String, name: String)]) async -> (imported: Int, skipped: Int) { - guard currentAccountType != .local else { + guard !currentContext.isLocal else { return dataManager.importSubscriptionsFromExternal(channels) } @@ -316,7 +356,7 @@ final class SubscriptionService { /// Posts a subscriptions-changed notification for server accounts. /// Local accounts already post it from DataManager with a change payload. private func postChangeNotificationForServerAccount() { - guard currentAccountType != .local else { return } + guard !currentContext.isLocal else { return } NotificationCenter.default.post(name: .subscriptionsDidChange, object: nil) } } diff --git a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift index 7b3a02ca..a25cf079 100644 --- a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift @@ -15,11 +15,14 @@ import Foundation final class InvidiousSubscriptionProvider: SubscriptionProvider { // MARK: - Properties - let accountType: SubscriptionAccountType = .invidious + /// The account this provider is bound to. One provider per account, so two + /// logins on the same instance never share a subscription cache. + let account: Account + var context: DataContext { .account(account.id) } private let invidiousAPI: InvidiousAPI private let credentialsManager: InvidiousCredentialsManager - private let instancesManager: InstancesManager + private let accountsManager: AccountsManager private let settingsManager: SettingsManager /// In-memory cache of subscribed channel IDs for fast lookup. @@ -34,14 +37,16 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { // MARK: - Initialization init( + account: Account, invidiousAPI: InvidiousAPI, credentialsManager: InvidiousCredentialsManager, - instancesManager: InstancesManager, + accountsManager: AccountsManager, settingsManager: SettingsManager ) { + self.account = account self.invidiousAPI = invidiousAPI self.credentialsManager = credentialsManager - self.instancesManager = instancesManager + self.accountsManager = accountsManager self.settingsManager = settingsManager } @@ -117,24 +122,14 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { /// Gets the authenticated Invidious instance and session ID from account settings. private func getAuthenticatedInstance() async throws -> (Instance, String) { - // Get the instance ID from subscription account settings - let account = settingsManager.subscriptionAccount - - let instance: Instance? - if let instanceID = account.instanceID { - // Use the specific instance from account settings - instance = instancesManager.instances.first { $0.id == instanceID && $0.isEnabled } - } else { - // Fallback to first enabled Invidious instance - instance = instancesManager.instances.first { $0.type == .invidious && $0.isEnabled } - } - - guard let instance else { + // The account names its instance directly - no falling back to + // "some enabled Invidious instance", which could silently send one + // account's request to a server it has no credential for. + guard let instance = accountsManager.instance(for: account), instance.isEnabled else { throw SubscriptionProviderError.instanceNotConfigured } - // Get session ID - guard let sid = credentialsManager.sid(for: instance) else { + guard let sid = credentialsManager.credential(for: account) else { throw SubscriptionProviderError.notAuthenticated } diff --git a/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift b/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift index c6a7c469..e0fd989e 100644 --- a/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift @@ -14,7 +14,7 @@ import Foundation final class LocalSubscriptionProvider: SubscriptionProvider { // MARK: - Properties - let accountType: SubscriptionAccountType = .local + let context: DataContext = .local private let dataManager: DataManager // MARK: - Initialization diff --git a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift index 29465044..0e02752a 100644 --- a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift @@ -15,11 +15,14 @@ import Foundation final class PipedSubscriptionProvider: SubscriptionProvider { // MARK: - Properties - let accountType: SubscriptionAccountType = .piped + /// The account this provider is bound to. One provider per account, so two + /// logins on the same instance never share a subscription cache. + let account: Account + var context: DataContext { .account(account.id) } private let pipedAPI: PipedAPI private let credentialsManager: PipedCredentialsManager - private let instancesManager: InstancesManager + private let accountsManager: AccountsManager private let settingsManager: SettingsManager /// In-memory cache of subscribed channel IDs for fast lookup. @@ -34,14 +37,16 @@ final class PipedSubscriptionProvider: SubscriptionProvider { // MARK: - Initialization init( + account: Account, pipedAPI: PipedAPI, credentialsManager: PipedCredentialsManager, - instancesManager: InstancesManager, + accountsManager: AccountsManager, settingsManager: SettingsManager ) { + self.account = account self.pipedAPI = pipedAPI self.credentialsManager = credentialsManager - self.instancesManager = instancesManager + self.accountsManager = accountsManager self.settingsManager = settingsManager } @@ -117,24 +122,14 @@ final class PipedSubscriptionProvider: SubscriptionProvider { /// Gets the authenticated Piped instance and auth token from account settings. private func getAuthenticatedInstance() async throws -> (Instance, String) { - // Get the instance ID from subscription account settings - let account = settingsManager.subscriptionAccount - - let instance: Instance? - if let instanceID = account.instanceID { - // Use the specific instance from account settings - instance = instancesManager.instances.first { $0.id == instanceID && $0.isEnabled } - } else { - // Fallback to first enabled Piped instance - instance = instancesManager.instances.first { $0.type == .piped && $0.isEnabled } - } - - guard let instance else { + // The account names its instance directly - no falling back to + // "some enabled Piped instance", which could silently send one + // account's request to a server it has no credential for. + guard let instance = accountsManager.instance(for: account), instance.isEnabled else { throw SubscriptionProviderError.instanceNotConfigured } - // Get auth token - guard let authToken = credentialsManager.credential(for: instance) else { + guard let authToken = credentialsManager.credential(for: account) else { throw SubscriptionProviderError.notAuthenticated } diff --git a/Yattee/Services/Subscriptions/SubscriptionProvider.swift b/Yattee/Services/Subscriptions/SubscriptionProvider.swift index 144e1a80..95c4a7c4 100644 --- a/Yattee/Services/Subscriptions/SubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/SubscriptionProvider.swift @@ -12,8 +12,12 @@ import Foundation /// Each provider implementation handles a specific subscription source. @MainActor protocol SubscriptionProvider: Sendable { - /// The type of subscription account this provider handles. - var accountType: SubscriptionAccountType { get } + /// The data context this provider serves. + /// + /// A context rather than a backend type: an instance can hold several + /// accounts, so "which Invidious" is no longer enough to identify a + /// provider — each account gets its own, with its own cache. + var context: DataContext { get } /// Fetches all subscriptions from the provider. /// - Returns: Array of channels the user is subscribed to. diff --git a/Yattee/Views/Settings/InstanceAccountsSection.swift b/Yattee/Views/Settings/InstanceAccountsSection.swift index a0ad7ead..0a4eda07 100644 --- a/Yattee/Views/Settings/InstanceAccountsSection.swift +++ b/Yattee/Views/Settings/InstanceAccountsSection.swift @@ -171,6 +171,9 @@ struct InstanceAccountsSection: View { appEnvironment.accountCredentialsManager(for: account)?.deleteCredential(for: account) appEnvironment.accountCredentialsManager(for: account)?.deleteLegacyCredentialIfOwned(for: account) appEnvironment.accountsManager.removeAccount(account.id) + // Drop the provider too, or its in-memory subscription cache would keep + // answering for an account that no longer exists. + appEnvironment.subscriptionService.discardProvider(for: account.id) if appEnvironment.accountsManager.accounts(forInstance: instance.id).isEmpty { appEnvironment.instancesManager.setUsesAccountLogin(false, for: instance) diff --git a/Yattee/Views/Settings/NotificationSettingsView.swift b/Yattee/Views/Settings/NotificationSettingsView.swift index 11b362a5..0bac6611 100644 --- a/Yattee/Views/Settings/NotificationSettingsView.swift +++ b/Yattee/Views/Settings/NotificationSettingsView.swift @@ -258,7 +258,7 @@ struct ManageChannelNotificationsView: View { defer { isLoading = false } // For local account, load from DataManager - if appEnvironment.settingsManager.subscriptionAccount.type == .local { + if appEnvironment.accountsManager.currentContext.isLocal { subscriptions = appEnvironment.dataManager.subscriptions() return } diff --git a/Yattee/Views/Settings/SubscriptionsSettingsView.swift b/Yattee/Views/Settings/SubscriptionsSettingsView.swift index 8bc3d99d..22dd73e6 100644 --- a/Yattee/Views/Settings/SubscriptionsSettingsView.swift +++ b/Yattee/Views/Settings/SubscriptionsSettingsView.swift @@ -201,7 +201,12 @@ struct SubscriptionsSettingsView: View { private func confirmAccountSwitch() { guard let newAccount = pendingAccountChange else { return } + // Both stores are written: the context is what the data layer reads, + // the legacy setting is kept in step until the Phase 3 switcher removes + // the last readers of it. Letting them diverge would show one account + // in the UI while serving another's data. settingsManager?.subscriptionAccount = newAccount + appEnvironment?.accountsManager.setContext(DataContext(legacy: newAccount)) SubscriptionFeedCache.shared.handleAccountChange() // Trigger feed refresh in background @@ -432,7 +437,7 @@ struct SubscriptionsSettingsView: View { if let cached = subscriptionService.cachedSubscriptionCount { accountSubscriptionCount = cached } - if subscriptionService.currentAccountType != .local { + if !subscriptionService.currentContext.isLocal { if let channels = try? await subscriptionService.fetchSubscriptions() { accountSubscriptionCount = channels.count } diff --git a/Yattee/Views/Subscriptions/SubscriptionsView.swift b/Yattee/Views/Subscriptions/SubscriptionsView.swift index bdb084b6..d765d831 100644 --- a/Yattee/Views/Subscriptions/SubscriptionsView.swift +++ b/Yattee/Views/Subscriptions/SubscriptionsView.swift @@ -1315,7 +1315,7 @@ struct SubscriptionsView: View { private func loadSubscriptions() { // For local account, load from DataManager // For Invidious, subscriptions will be loaded async in loadSubscriptionsAsync - if appEnvironment?.settingsManager.subscriptionAccount.type == .local { + if appEnvironment?.accountsManager.currentContext.isLocal == true { subscriptions = dataManager?.subscriptions() ?? [] subscriptionsLoaded = true sidebarExpected = subscriptions.count > 1 @@ -1333,7 +1333,7 @@ struct SubscriptionsView: View { guard let subscriptionService, let appEnvironment else { return } // For local account, just load from DataManager (fast) - if appEnvironment.settingsManager.subscriptionAccount.type == .local { + if appEnvironment.accountsManager.currentContext.isLocal { subscriptions = dataManager?.subscriptions() ?? [] subscriptionsLoaded = true sidebarExpected = subscriptions.count > 1