mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Resolve contexts and credentials through real accounts
Round two of review on the accounts layer. The settings picker mapped its legacy instance-keyed selection straight onto DataContext(legacy:), which turns an instance UUID into an account UUID. That identity only holds for migrated accounts - a login created after the revamp has a random ID - so selecting a freshly added account named one that does not exist and left the provider nil. Selections now resolve through AccountsManager to an actual account on that instance, preferring a signed-in one. The picker still lists one entry per instance and so cannot show a second login; the Phase 3 switcher replaces it. BackgroundFeedRefresher was missed in the previous sweep because it resolves credentials differently: it routed on the legacy setting and looked up "first enabled Invidious instance" with an instance-keyed credential. After any new-style login that silently found nothing and skipped the refresh. It now routes on the selected context and takes the credential from that account. Subscription reads used the raw stored context, so an account waiting on its instance or credential rendered empty instead of falling back to local data as the tri-state design specifies. Reads go through the validator's effective context; writes deliberately do not, so a subscribe fails visibly rather than landing in local data the user did not choose. Account statuses are cleared when the identity namespace reloads, but nothing recomputed them once a deferred scope resolution succeeded - a synced account kept showing "sign in required" until relaunch. The scope change now triggers a refresh, deferred by one turn so it cannot depend on observer registration order relative to the store's own reload.
This commit is contained in:
@@ -209,6 +209,23 @@ final class AccountsManager {
|
||||
|
||||
// MARK: - Context
|
||||
|
||||
/// Resolves a legacy instance-keyed selection onto a real account context.
|
||||
///
|
||||
/// `DataContext(legacy:)` maps an instance UUID straight onto an account
|
||||
/// UUID, which only holds for *migrated* accounts — those deliberately
|
||||
/// reuse the instance UUID. A login created after the revamp has a random
|
||||
/// ID, so the legacy mapping would name an account that does not exist.
|
||||
func context(forLegacyInstanceID instanceID: UUID?) -> DataContext {
|
||||
guard let instanceID else { return .local }
|
||||
|
||||
let candidates = accounts(forInstance: instanceID)
|
||||
guard !candidates.isEmpty else { return .local }
|
||||
|
||||
// Prefer a signed-in account when the instance holds several.
|
||||
let chosen = candidates.first { status(for: $0.id) == .signedIn } ?? candidates[0]
|
||||
return .account(chosen.id)
|
||||
}
|
||||
|
||||
/// Records an explicit user choice of context.
|
||||
func setContext(_ context: DataContext) {
|
||||
storedContext = StoredDataContext(context: context)
|
||||
|
||||
@@ -364,6 +364,24 @@ final class AppEnvironment {
|
||||
backfillCredentialFlags()
|
||||
|
||||
dataContextValidator.setAppEnvironment(self)
|
||||
subscriptionService.setContextValidator(dataContextValidator)
|
||||
|
||||
// Statuses are cleared whenever the identity namespace reloads, so they
|
||||
// have to be recomputed once the new scope's Keychain is readable —
|
||||
// otherwise a synced account shows "sign in required" until relaunch.
|
||||
NotificationCenter.default.addObserver(
|
||||
forName: .credentialIdentityScopeDidChange,
|
||||
object: nil,
|
||||
queue: .main
|
||||
) { [weak self] _ in
|
||||
// Deferred rather than run inline: AccountsManager observes the same
|
||||
// notification to reload its namespace and clear statuses, and
|
||||
// relying on observer registration order to land after it would be
|
||||
// silently fragile.
|
||||
Task { @MainActor [weak self] in
|
||||
self?.refreshAccountStatuses()
|
||||
}
|
||||
}
|
||||
|
||||
// Turn any pre-revamp single-instance logins into accounts. Repeatable:
|
||||
// it runs again whenever instances arrive from iCloud.
|
||||
@@ -492,6 +510,17 @@ final class AppEnvironment {
|
||||
}
|
||||
}
|
||||
|
||||
/// The selected context's account, when it lives on an instance of the
|
||||
/// given type. Used by paths that still need to know which backend they are
|
||||
/// talking to before they can build a request.
|
||||
func account(forSelectedContext type: InstanceType) -> Account? {
|
||||
guard case let .account(id) = accountsManager.currentContext,
|
||||
let account = accountsManager.account(id: id),
|
||||
accountsManager.instance(for: account)?.type == type
|
||||
else { return nil }
|
||||
return account
|
||||
}
|
||||
|
||||
/// The stored credential for an instance, resolved through its accounts.
|
||||
func credential(forInstance instance: Instance) -> String? {
|
||||
guard let account = account(forInstance: instance) else { return nil }
|
||||
|
||||
@@ -41,14 +41,32 @@ final class BackgroundFeedRefresher {
|
||||
return
|
||||
}
|
||||
|
||||
// Route based on subscription account type
|
||||
switch appEnvironment.settingsManager.subscriptionAccount.type {
|
||||
// Route on the selected data context. An account context resolves its
|
||||
// own instance, so a second login on the same instance refreshes the
|
||||
// right user's feed.
|
||||
switch appEnvironment.accountsManager.currentContext {
|
||||
case .local:
|
||||
await performLocalAccountRefresh()
|
||||
case .invidious:
|
||||
await performInvidiousAccountRefresh()
|
||||
case .piped:
|
||||
await performPipedAccountRefresh()
|
||||
case let .account(id):
|
||||
guard let account = appEnvironment.accountsManager.account(id: id),
|
||||
let instance = appEnvironment.accountsManager.instance(for: account)
|
||||
else {
|
||||
LoggingService.shared.debug(
|
||||
"Background refresh skipped: selected account has no resolved instance",
|
||||
category: .notifications
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
switch instance.type {
|
||||
case .invidious: await performInvidiousAccountRefresh()
|
||||
case .piped: await performPipedAccountRefresh()
|
||||
default:
|
||||
LoggingService.shared.debug(
|
||||
"Background refresh not supported for this account type",
|
||||
category: .notifications
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -169,10 +187,12 @@ final class BackgroundFeedRefresher {
|
||||
private func performInvidiousAccountRefresh() async {
|
||||
guard let appEnvironment else { return }
|
||||
|
||||
// Require authenticated Invidious instance
|
||||
guard let invidiousInstance = appEnvironment.instancesManager.instances
|
||||
.first(where: { $0.type == .invidious && $0.isEnabled }),
|
||||
let sid = appEnvironment.invidiousCredentialsManager.sid(for: invidiousInstance) else {
|
||||
// Resolve through the selected account: a fresh login writes only the
|
||||
// account-scoped Keychain key, and "first enabled Invidious instance"
|
||||
// could pick a server this account has no credential for.
|
||||
guard let account = appEnvironment.account(forSelectedContext: .invidious),
|
||||
let invidiousInstance = appEnvironment.accountsManager.instance(for: account),
|
||||
let sid = appEnvironment.accountCredentialsManager(for: account)?.credential(for: account) else {
|
||||
LoggingService.shared.debug("Background refresh requires authenticated Invidious instance", category: .notifications)
|
||||
return
|
||||
}
|
||||
@@ -274,11 +294,9 @@ final class BackgroundFeedRefresher {
|
||||
private func performPipedAccountRefresh() async {
|
||||
guard let appEnvironment else { return }
|
||||
|
||||
// Get the instance ID from subscription account settings
|
||||
let account = appEnvironment.settingsManager.subscriptionAccount
|
||||
guard let instanceID = account.instanceID,
|
||||
let pipedInstance = appEnvironment.instancesManager.instances.first(where: { $0.id == instanceID }),
|
||||
let authToken = appEnvironment.pipedCredentialsManager.credential(for: pipedInstance) else {
|
||||
guard let account = appEnvironment.account(forSelectedContext: .piped),
|
||||
let pipedInstance = appEnvironment.accountsManager.instance(for: account),
|
||||
let authToken = appEnvironment.accountCredentialsManager(for: account)?.credential(for: account) else {
|
||||
LoggingService.shared.debug("Background refresh requires authenticated Piped instance", category: .notifications)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -53,6 +53,7 @@ final class SubscriptionService {
|
||||
private var accountProviders: [UUID: any SubscriptionProvider] = [:]
|
||||
|
||||
private weak var accountsManager: AccountsManager?
|
||||
private weak var contextValidator: DataContextValidator?
|
||||
|
||||
// MARK: - State
|
||||
|
||||
@@ -86,6 +87,10 @@ final class SubscriptionService {
|
||||
self.localProvider = LocalSubscriptionProvider(dataManager: dataManager)
|
||||
}
|
||||
|
||||
func setContextValidator(_ validator: DataContextValidator) {
|
||||
contextValidator = validator
|
||||
}
|
||||
|
||||
func setAccountsManager(_ manager: AccountsManager) {
|
||||
accountsManager = manager
|
||||
|
||||
@@ -154,11 +159,25 @@ final class SubscriptionService {
|
||||
accountProviders[accountID] = nil
|
||||
}
|
||||
|
||||
/// Returns the current subscription provider for the selected context.
|
||||
/// The provider for **writes**.
|
||||
///
|
||||
/// Always the raw selected context, never the read fallback: a subscribe
|
||||
/// must fail visibly when the account is unavailable rather than silently
|
||||
/// landing in local data the user did not choose.
|
||||
var currentProvider: (any SubscriptionProvider)? {
|
||||
provider(for: currentContext)
|
||||
}
|
||||
|
||||
/// The provider for **reads**.
|
||||
///
|
||||
/// Uses the validator's effective context, which falls back to local while
|
||||
/// the selected account is waiting on its instance or credential. The
|
||||
/// fallback is not persisted, so the user's choice takes effect again on
|
||||
/// its own once the account resolves.
|
||||
var readProvider: (any SubscriptionProvider)? {
|
||||
provider(for: contextValidator?.effectiveContext ?? currentContext)
|
||||
}
|
||||
|
||||
/// The selected context, or `.local` before accounts have loaded.
|
||||
var currentContext: DataContext {
|
||||
accountsManager?.currentContext ?? .local
|
||||
@@ -167,7 +186,7 @@ final class SubscriptionService {
|
||||
/// Subscription count for the active account, when known without a network fetch.
|
||||
/// Always available for local accounts; nil for server accounts until their cache is populated.
|
||||
var cachedSubscriptionCount: Int? {
|
||||
currentProvider?.cachedSubscriptionCount
|
||||
readProvider?.cachedSubscriptionCount
|
||||
}
|
||||
|
||||
// MARK: - Subscribe
|
||||
@@ -254,7 +273,7 @@ final class SubscriptionService {
|
||||
/// - Parameter channelID: The channel ID to check.
|
||||
/// - Returns: `true` if subscribed, `false` otherwise.
|
||||
func isSubscribed(to channelID: String) async -> Bool {
|
||||
guard let provider = currentProvider else {
|
||||
guard let provider = readProvider else {
|
||||
return false
|
||||
}
|
||||
return await provider.isSubscribed(to: channelID)
|
||||
@@ -263,7 +282,7 @@ final class SubscriptionService {
|
||||
/// Fetches all subscriptions from the current provider.
|
||||
/// - Returns: Array of subscribed channels.
|
||||
func fetchSubscriptions() async throws -> [Channel] {
|
||||
guard let provider = currentProvider else {
|
||||
guard let provider = readProvider else {
|
||||
throw SubscriptionError.providerNotAvailable
|
||||
}
|
||||
|
||||
@@ -284,7 +303,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 currentContext.isLocal else {
|
||||
guard (contextValidator?.effectiveContext ?? currentContext).isLocal else {
|
||||
return nil
|
||||
}
|
||||
|
||||
@@ -303,7 +322,7 @@ final class SubscriptionService {
|
||||
|
||||
/// Refreshes the subscription cache from the current provider.
|
||||
func refreshCache() async throws {
|
||||
guard let provider = currentProvider else {
|
||||
guard let provider = readProvider else {
|
||||
throw SubscriptionError.providerNotAvailable
|
||||
}
|
||||
|
||||
|
||||
@@ -206,7 +206,9 @@ struct SubscriptionsSettingsView: View {
|
||||
// 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))
|
||||
appEnvironment?.accountsManager.setContext(
|
||||
appEnvironment?.accountsManager.context(forLegacyInstanceID: newAccount.instanceID) ?? .local
|
||||
)
|
||||
SubscriptionFeedCache.shared.handleAccountChange()
|
||||
|
||||
// Trigger feed refresh in background
|
||||
|
||||
@@ -157,6 +157,35 @@ struct AccountsManagerTests {
|
||||
#expect(manager2.currentContext == .local)
|
||||
}
|
||||
|
||||
@Test("A legacy instance selection resolves to a real account")
|
||||
func legacySelectionResolvesToRealAccount() {
|
||||
let manager = makeManager()
|
||||
let instanceID = UUID()
|
||||
|
||||
// No account yet on that instance: nothing to select.
|
||||
#expect(manager.context(forLegacyInstanceID: instanceID) == .local)
|
||||
|
||||
// A login created after the revamp has a random ID, so the legacy
|
||||
// instance-ID-as-account-ID mapping would name something nonexistent.
|
||||
let account = manager.addAccount(instanceID: instanceID)
|
||||
#expect(account.id != instanceID)
|
||||
#expect(manager.context(forLegacyInstanceID: instanceID) == .account(account.id))
|
||||
|
||||
#expect(manager.context(forLegacyInstanceID: nil) == .local)
|
||||
}
|
||||
|
||||
@Test("A signed-in account is preferred when an instance holds several")
|
||||
func legacySelectionPrefersSignedIn() {
|
||||
let manager = makeManager()
|
||||
let instanceID = UUID()
|
||||
|
||||
_ = manager.addAccount(instanceID: instanceID)
|
||||
let second = manager.addAccount(instanceID: instanceID)
|
||||
manager.setStatus(.signedIn, for: second.id)
|
||||
|
||||
#expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id))
|
||||
}
|
||||
|
||||
@Test("A migrated context loses to any explicit choice")
|
||||
func migratedContextLosesToExplicitChoice() {
|
||||
let manager = makeManager()
|
||||
|
||||
Reference in New Issue
Block a user