mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Separate account status from the account list, and stop stranding accounts
Recomputing statuses reads the Keychain, which cannot know that a server rejected a credential or failed to answer — and it ran in reaction to the same notification those verdicts were announced with, so it erased them as they were recorded. Status changes now have their own notification. With that separation `unreachable` no longer has to be preserved to survive: it is transient, and preserving it stopped every read from reaching the server, so nothing would ever discover that the network had come back. A fetch already running when its provider is discarded now has its result dropped. Signing in again — or switching Apple account — changes neither the context nor the account ID, so the previous login's channels would otherwise arrive as the current account's. Caches with no recorded owner are discarded rather than adopted. The first launch after upgrading finds feeds written before anyone recorded whose they were, and the Apple account may have changed since. The legacy validator's auto-correction moves the data context with it, and Manage Channels reloads on the context rather than the legacy value — it could list one account's channels while unsubscribes went to another.
This commit is contained in:
@@ -499,10 +499,17 @@ final class AccountsManager {
|
||||
|
||||
// MARK: - Status
|
||||
|
||||
/// Records what the app currently knows about an account's usability.
|
||||
///
|
||||
/// Announced separately from the account *list* changing. Recomputing
|
||||
/// statuses from the Keychain is one of the things that listens for a list
|
||||
/// change, and it cannot tell that a server rejected a credential or failed
|
||||
/// to answer — so sharing one notification meant every status the API layer
|
||||
/// recorded was overwritten by the reaction to recording it.
|
||||
func setStatus(_ status: AccountStatus, for accountID: UUID) {
|
||||
guard accountStatuses[accountID] != status else { return }
|
||||
accountStatuses[accountID] = status
|
||||
NotificationCenter.default.post(name: .accountsDidChange, object: nil)
|
||||
NotificationCenter.default.post(name: .accountStatusesDidChange, object: nil)
|
||||
}
|
||||
|
||||
/// An account with no recorded status is *unknown*, not signed out. While
|
||||
@@ -719,5 +726,7 @@ final class AccountsManager {
|
||||
|
||||
extension Notification.Name {
|
||||
static let accountsDidChange = Notification.Name("stream.yattee.accountsDidChange")
|
||||
/// An account's usability changed — not the list of accounts.
|
||||
static let accountStatusesDidChange = Notification.Name("stream.yattee.accountStatusesDidChange")
|
||||
static let dataContextDidChange = Notification.Name("stream.yattee.dataContextDidChange")
|
||||
}
|
||||
|
||||
@@ -457,12 +457,19 @@ final class AppEnvironment {
|
||||
// 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.
|
||||
for name: Notification.Name in [.dataContextDidChange, .accountsDidChange, .instancesDidChange] {
|
||||
// A status change is deliberately not one of the refresh triggers: the
|
||||
// refresher reads the Keychain, which cannot know what a server said,
|
||||
// so reacting to a status by recomputing it would erase every verdict
|
||||
// the API layer records. It still runs the mirror, because a credential
|
||||
// arriving or being rejected moves the effective context.
|
||||
for name: Notification.Name in [
|
||||
.dataContextDidChange, .accountsDidChange, .instancesDidChange, .accountStatusesDidChange
|
||||
] {
|
||||
contextMirrorObservers.append(
|
||||
NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in
|
||||
MainActor.assumeIsolated {
|
||||
guard let self else { return }
|
||||
if name != .dataContextDidChange {
|
||||
if name == .accountsDidChange || name == .instancesDidChange {
|
||||
self.refreshAccountStatuses()
|
||||
}
|
||||
self.mirrorContextToLegacySelection()
|
||||
@@ -685,11 +692,17 @@ final class AppEnvironment {
|
||||
let previous = defaults.string(forKey: Self.cacheOwnerKey)
|
||||
defaults.set(owner, forKey: Self.cacheOwnerKey)
|
||||
|
||||
guard let previous, previous != owner else { return }
|
||||
guard previous != owner else { return }
|
||||
|
||||
// A missing owner is not the same as "ours". The first launch after
|
||||
// upgrading finds caches written before anyone recorded who they belong
|
||||
// to, and the Apple account may have changed since. Adopting them would
|
||||
// be a guess with someone's private feed as the stake.
|
||||
clearAccountScopedCaches()
|
||||
LoggingService.shared.info(
|
||||
"Apple account changed — discarded the previous account's cached data",
|
||||
previous == nil
|
||||
? "Cached account data predates knowing who it belongs to — discarded it"
|
||||
: "Apple account changed — discarded the previous account's cached data",
|
||||
category: .general
|
||||
)
|
||||
}
|
||||
@@ -805,16 +818,21 @@ final class AppEnvironment {
|
||||
continue
|
||||
}
|
||||
|
||||
// A credential the server rejected — or a server that could not be
|
||||
// reached — is still a credential, and only the server can say
|
||||
// otherwise. Reporting "signed in" here because the Keychain item
|
||||
// exists would erase those the moment they were recorded: the API
|
||||
// layer sets them, and that posts the notification that runs this.
|
||||
// The next successful call clears them; storing a new credential
|
||||
// A credential the server rejected is still a credential: the
|
||||
// Keychain cannot tell that it stopped working, so this must not
|
||||
// report "signed in" over it. Only the server clears a rejection —
|
||||
// on the next successful call — or storing a new credential, which
|
||||
// resets the status explicitly.
|
||||
//
|
||||
// `unreachable` is deliberately *not* preserved. It is transient,
|
||||
// and it stops reads from reaching the server at all, so keeping it
|
||||
// here would strand the account: nothing would ever call out to
|
||||
// discover that the network came back. Recomputing it on the events
|
||||
// that run this — foreground, an account or instance arriving — is
|
||||
// what lets it retry.
|
||||
switch accountsManager.status(for: account.id) {
|
||||
case .credentialRejected, .unreachable: continue
|
||||
case .checking, .signedIn, .credentialMissing: break
|
||||
case .credentialRejected: continue
|
||||
case .checking, .signedIn, .credentialMissing, .unreachable: break
|
||||
}
|
||||
|
||||
accountsManager.setStatus(.signedIn, for: account.id)
|
||||
|
||||
@@ -157,8 +157,14 @@ final class SubscriptionService {
|
||||
/// served after the account is gone.
|
||||
func discardProvider(for accountID: UUID) {
|
||||
accountProviders[accountID] = nil
|
||||
providerGeneration += 1
|
||||
}
|
||||
|
||||
/// Which generation of logins the cached providers belong to. Bumped
|
||||
/// whenever one is discarded, so a fetch that outlived its provider can be
|
||||
/// told from a current one.
|
||||
private var providerGeneration = 0
|
||||
|
||||
/// The provider for **writes**.
|
||||
///
|
||||
/// Always the raw selected context, never the read fallback: a subscribe
|
||||
@@ -289,9 +295,25 @@ final class SubscriptionService {
|
||||
isLoading = true
|
||||
lastError = nil
|
||||
|
||||
// Dropping a provider from the cache does not stop a fetch it has
|
||||
// already started: the running task holds it, and the result would
|
||||
// arrive afterwards as the current account's subscriptions. Signing in
|
||||
// again or switching Apple account changes neither the context nor the
|
||||
// account ID, so nothing else downstream would notice.
|
||||
let generation = providerGeneration
|
||||
|
||||
do {
|
||||
let channels = try await provider.fetchSubscriptions()
|
||||
isLoading = false
|
||||
|
||||
guard generation == providerGeneration else {
|
||||
LoggingService.shared.info(
|
||||
"Discarding subscriptions fetched for a login that has since been replaced",
|
||||
category: .general
|
||||
)
|
||||
throw SubscriptionError.providerNotAvailable
|
||||
}
|
||||
|
||||
return channels
|
||||
} catch {
|
||||
lastError = error
|
||||
|
||||
@@ -150,6 +150,7 @@ final class SubscriptionAccountValidator {
|
||||
// Find first available account to switch to
|
||||
if let newAccount = availableAccounts.first {
|
||||
settingsManager.subscriptionAccount = newAccount
|
||||
adoptCorrectedSelection(newAccount)
|
||||
feedCache.handleAccountChange()
|
||||
|
||||
let newName = displayName(for: newAccount)
|
||||
@@ -166,6 +167,7 @@ final class SubscriptionAccountValidator {
|
||||
// No valid accounts available - set to local anyway
|
||||
// The feed will show an appropriate error state
|
||||
settingsManager.subscriptionAccount = .local
|
||||
adoptCorrectedSelection(.local)
|
||||
feedCache.handleAccountChange()
|
||||
|
||||
LoggingService.shared.warning(
|
||||
@@ -175,6 +177,20 @@ final class SubscriptionAccountValidator {
|
||||
}
|
||||
}
|
||||
|
||||
/// Moves the data context with a correction made to the legacy setting.
|
||||
///
|
||||
/// Reads and writes resolve through the context now, so correcting only the
|
||||
/// legacy value would leave the picker naming one account while the data
|
||||
/// came from another.
|
||||
private func adoptCorrectedSelection(_ account: SubscriptionAccount) {
|
||||
guard let accountsManager else { return }
|
||||
|
||||
let context = accountsManager.context(forLegacyInstanceID: account.instanceID)
|
||||
guard context != accountsManager.currentContext else { return }
|
||||
|
||||
accountsManager.setContext(context)
|
||||
}
|
||||
|
||||
// MARK: - Display Names
|
||||
|
||||
/// Returns a display name for the given subscription account.
|
||||
|
||||
@@ -281,7 +281,11 @@ struct ManageChannelsView: View {
|
||||
await refreshChannels()
|
||||
}
|
||||
}
|
||||
.onChange(of: settingsManager?.subscriptionAccount) { _, _ in
|
||||
// The data context, not the legacy setting: switching between two
|
||||
// logins on the same instance leaves that value identical, and this
|
||||
// view would keep listing one account's channels while unsubscribes
|
||||
// went to the other.
|
||||
.onReceive(NotificationCenter.default.publisher(for: .dataContextDidChange)) { _ in
|
||||
Task {
|
||||
await refreshChannels()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user