mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 13:12:53 +00:00
Let temporary account states recover on their own
"Could not be reached" now expires after a minute. It stops reads from going to the server at all, so nothing could ever discover that the server came back — and a timeout during an outage produces no connectivity change to react to either. The credential is untouched throughout; the next request settles it either way. A rejection is remembered against the credential that earned it. Signing in again on another device replaces that secret under the same key, and preserving the rejection regardless left the newly synced credential unusable with no way to prove otherwise. An account that syncs ahead of its credential — the ordinary case on a new device, since iCloud Keychain delivers on its own schedule and announces nothing — is re-checked for a couple of minutes instead of sitting at "sign in required" until something unrelated happens. It gives up rather than polling forever: on tvOS the wait is permanent by design. The feed cache no longer restores a purged account's videos from disk if the load was already in flight when the purge happened, and no longer leaves its loading flag set afterwards — which blocked the replacement refresh and left the new account on an empty feed.
This commit is contained in:
@@ -547,20 +547,52 @@ final class AccountsManager {
|
||||
/// 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) {
|
||||
/// - Parameter date: when the verdict was reached. Only `unreachable` uses
|
||||
/// it — to decide when to try again — and only tests pass anything but
|
||||
/// the default.
|
||||
func setStatus(_ status: AccountStatus, for accountID: UUID, at date: Date = Date()) {
|
||||
if status == .unreachable {
|
||||
unreachableSince[accountID] = date
|
||||
} else {
|
||||
unreachableSince[accountID] = nil
|
||||
}
|
||||
|
||||
guard accountStatuses[accountID] != status else { return }
|
||||
accountStatuses[accountID] = status
|
||||
NotificationCenter.default.post(name: .accountStatusesDidChange, object: nil)
|
||||
}
|
||||
|
||||
/// When each unreachable verdict was reached.
|
||||
private var unreachableSince: [UUID: Date] = [:]
|
||||
|
||||
/// How long "could not be reached" stands before the app tries again.
|
||||
///
|
||||
/// It has to expire on its own. The state stops reads from going to the
|
||||
/// server at all, so nothing would ever discover that the server came back
|
||||
/// — and a timeout during an outage never produces a connectivity change to
|
||||
/// react to either.
|
||||
private static let unreachableRetryInterval: TimeInterval = 60
|
||||
|
||||
/// An account with no recorded status is *unknown*, not signed out. While
|
||||
/// the identity scope is unresolved nothing can read the Keychain under the
|
||||
/// right scope, and reporting a missing credential then would tell a user
|
||||
/// with a perfectly good login to sign in again — and drop reads to local
|
||||
/// data — for as long as iCloud stayed unreachable.
|
||||
func status(for accountID: UUID) -> AccountStatus {
|
||||
if let status = accountStatuses[accountID] { return status }
|
||||
return identityScope.isResolved ? .credentialMissing : .checking
|
||||
guard let status = accountStatuses[accountID] else {
|
||||
return identityScope.isResolved ? .credentialMissing : .checking
|
||||
}
|
||||
|
||||
// A server that did not answer a minute ago is worth trying again. The
|
||||
// credential is untouched by any of this, so the account goes back to
|
||||
// being usable and the next request settles it either way.
|
||||
if status == .unreachable,
|
||||
let since = unreachableSince[accountID],
|
||||
Date().timeIntervalSince(since) > Self.unreachableRetryInterval {
|
||||
return .signedIn
|
||||
}
|
||||
|
||||
return status
|
||||
}
|
||||
|
||||
// MARK: - Legacy migration
|
||||
|
||||
@@ -73,6 +73,13 @@ final class AppEnvironment {
|
||||
/// The context the account-scoped caches currently hold data for.
|
||||
@ObservationIgnored private var lastCachedContext: DataContext = .local
|
||||
|
||||
/// The credential each rejection was recorded against, so a replacement
|
||||
/// arriving over iCloud Keychain is not treated as still rejected.
|
||||
@ObservationIgnored private var rejectedCredentialFingerprints: [UUID: String] = [:]
|
||||
|
||||
/// Re-reads the Keychain for a while after an account turns up without one.
|
||||
@ObservationIgnored private var credentialArrivalTask: Task<Void, Never>?
|
||||
|
||||
// MARK: - Shared Instance
|
||||
|
||||
/// The single, process-wide app environment.
|
||||
@@ -829,23 +836,84 @@ final class AppEnvironment {
|
||||
|
||||
// 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.
|
||||
// report "signed in" over it. What it *can* tell is whether the
|
||||
// credential is still the same one — signing in again on another
|
||||
// device replaces it under the same key — and a rejection only
|
||||
// applies to the secret that earned it.
|
||||
//
|
||||
// `unreachable` is deliberately *not* preserved. It is transient,
|
||||
// `unreachable` is deliberately not preserved here. 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: continue
|
||||
case .checking, .signedIn, .credentialMissing, .unreachable: break
|
||||
// would strand the account: nothing would call out to discover that
|
||||
// the server came back.
|
||||
let fingerprint = credentials.credential(for: account)
|
||||
.map { CloudKitIdentity.shortDigest(of: Data($0.utf8)) }
|
||||
|
||||
if accountsManager.status(for: account.id) == .credentialRejected {
|
||||
if let rejected = rejectedCredentialFingerprints[account.id], rejected != fingerprint {
|
||||
LoggingService.shared.info(
|
||||
"The rejected credential has been replaced — clearing the rejection",
|
||||
category: .keychain,
|
||||
details: "accountID=\(account.id)"
|
||||
)
|
||||
} else {
|
||||
rejectedCredentialFingerprints[account.id] = fingerprint
|
||||
continue
|
||||
}
|
||||
}
|
||||
|
||||
rejectedCredentialFingerprints[account.id] = nil
|
||||
|
||||
accountsManager.setStatus(.signedIn, for: account.id)
|
||||
}
|
||||
|
||||
waitForArrivingCredentialsIfNeeded()
|
||||
}
|
||||
|
||||
/// Re-reads the Keychain for a couple of minutes while any account is
|
||||
/// missing its credential.
|
||||
///
|
||||
/// iCloud Keychain delivers on its own schedule and announces nothing when
|
||||
/// it does, so an account that syncs ahead of its credential — the ordinary
|
||||
/// case on a new device — would sit at "sign in required", with reads
|
||||
/// falling back to local, until something unrelated happened to recompute
|
||||
/// statuses. Stops as soon as nothing is waiting, and gives up rather than
|
||||
/// polling forever: on tvOS, where app Keychain items never sync at all,
|
||||
/// the wait is permanent by design.
|
||||
private func waitForArrivingCredentialsIfNeeded() {
|
||||
let isWaiting = accountsManager.accounts.contains { accountsManager.status(for: $0.id) == .credentialMissing }
|
||||
|
||||
guard isWaiting else {
|
||||
credentialArrivalTask?.cancel()
|
||||
credentialArrivalTask = nil
|
||||
return
|
||||
}
|
||||
|
||||
guard credentialArrivalTask == nil else { return }
|
||||
|
||||
credentialArrivalTask = Task { [weak self] in
|
||||
for _ in 0 ..< 8 {
|
||||
try? await Task.sleep(for: .seconds(15))
|
||||
guard !Task.isCancelled, let self else { return }
|
||||
|
||||
let stillWaiting = self.accountsManager.accounts.contains {
|
||||
self.accountsManager.status(for: $0.id) == .credentialMissing
|
||||
}
|
||||
guard stillWaiting else { break }
|
||||
|
||||
for account in self.accountsManager.accounts
|
||||
where self.accountsManager.status(for: account.id) == .credentialMissing {
|
||||
guard let credentials = self.accountCredentialsManager(for: account),
|
||||
credentials.isScopeResolved else { continue }
|
||||
|
||||
credentials.refreshLoginStatus(for: account)
|
||||
if credentials.isLoggedIn(for: account) {
|
||||
self.accountsManager.setStatus(.signedIn, for: account.id)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
self?.credentialArrivalTask = nil
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Credentials Management
|
||||
|
||||
@@ -129,8 +129,21 @@ final class SubscriptionFeedCache {
|
||||
return
|
||||
}
|
||||
diskCacheLoaded = true
|
||||
let generation = accountGeneration
|
||||
|
||||
if let cacheData = await FeedCache.shared.load() {
|
||||
// The identity or the login can change while this is awaiting the
|
||||
// disk. Applying what it brings back would restore the previous
|
||||
// account's private feed on top of a cache that was deliberately
|
||||
// emptied.
|
||||
guard generation == accountGeneration else {
|
||||
LoggingService.shared.info(
|
||||
"Discarding a disk feed that belongs to the previous login",
|
||||
category: .general
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
videos = cacheData.videos
|
||||
lastUpdated = cacheData.lastUpdated
|
||||
hasLoadedOnce = true
|
||||
@@ -222,6 +235,11 @@ final class SubscriptionFeedCache {
|
||||
accountGeneration += 1
|
||||
clear()
|
||||
feedLoadState = .idle
|
||||
// The request that was running belongs to the previous account and its
|
||||
// result is now fenced out, so it must not go on blocking the
|
||||
// replacement — the new account would sit on an empty feed until
|
||||
// something else asked for a refresh.
|
||||
isLoading = false
|
||||
}
|
||||
|
||||
/// Which account's data the running request belongs to.
|
||||
|
||||
@@ -186,6 +186,21 @@ struct AccountsManagerTests {
|
||||
#expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id))
|
||||
}
|
||||
|
||||
@Test("An unreachable server is retried rather than written off")
|
||||
func unreachableStatusExpires() {
|
||||
let manager = makeManager()
|
||||
let account = manager.addAccount(instanceID: UUID())
|
||||
|
||||
manager.setStatus(.unreachable, for: account.id)
|
||||
#expect(manager.status(for: account.id) == .unreachable)
|
||||
|
||||
// The state stops reads from going out at all, so nothing would ever
|
||||
// discover that the server came back — a timeout during an outage
|
||||
// produces no connectivity change to react to either.
|
||||
manager.setStatus(.unreachable, for: account.id, at: Date().addingTimeInterval(-3_600))
|
||||
#expect(manager.status(for: account.id) == .signedIn)
|
||||
}
|
||||
|
||||
@Test("A sign-in that could not be stored leaves no migration marker")
|
||||
func rolledBackLoginRetractsItsMarker() {
|
||||
let manager = makeManager()
|
||||
|
||||
Reference in New Issue
Block a user