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.
This commit is contained in:
Arkadiusz Fal
2026-08-24 23:56:40 +02:00
parent 66f721e3ac
commit 9065234606
10 changed files with 120 additions and 77 deletions

View File

@@ -175,6 +175,7 @@ final class AppEnvironment {
invidiousAPI: invidiousAPI,
pipedAPI: pipedAPI
)
subscriptionService.setAccountsManager(accounts)
// Initialize CloudKit Sync Engine
let cloudKit = CloudKitSyncEngine(

View File

@@ -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)
}
}

View File

@@ -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
}

View File

@@ -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

View File

@@ -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
}

View File

@@ -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.

View File

@@ -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)

View File

@@ -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
}

View File

@@ -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
}

View File

@@ -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