Fence the last two caches and undo failed logins properly

Home feeds and subscription providers join the subscriptions feed in
dropping results that belong to a login the app has since moved off.
Clearing a cache does not cancel what is already awaiting the server,
and an Apple-account switch discarded providers without invalidating the
fetches holding them.

A sign-in whose credential the Keychain refused to store is now rolled
back rather than deleted. Creating an account also queues the permanent
marker that stops every other device synthesizing an account for that
instance's pre-revamp login; leaving it behind would suppress a still
valid legacy login everywhere on the strength of an account nobody has.
An account the server already confirmed is still deleted properly,
tombstone and all.

Caches with no recorded owner are discarded even when the identity
cannot be resolved at all — that is the same first launch after
upgrading, only offline.

An account whose credential is missing no longer hides a signed-in
sibling on the same instance, which made the whole instance report
itself as logged out. And connectivity returning recomputes statuses, so
an account marked unreachable retries instead of waiting for a
foreground transition it has no way to cause.
This commit is contained in:
Arkadiusz Fal
2026-08-27 20:45:54 +02:00
parent b07af6c153
commit e094770047
7 changed files with 140 additions and 7 deletions

View File

@@ -224,6 +224,47 @@ final class AccountsManager {
NotificationCenter.default.post(name: .accountsDidChange, object: nil)
}
/// Undoes an account that was created moments ago and could not be
/// completed — a sign-in whose credential the Keychain refused to store.
///
/// Not the same as deleting it. Creating an account also marks its instance
/// migrated and queues the permanent marker that stops every other device
/// from synthesizing an account for that instance's pre-revamp login. A
/// deletion is meant to leave that marker standing; a creation that never
/// happened must not, or a legacy login that is still perfectly valid would
/// be suppressed everywhere on the strength of an account nobody has.
func rollbackAccountCreation(_ accountID: UUID) {
guard let account = account(id: accountID) else { return }
// Confirmed by the server already? Then it is a real account elsewhere,
// and undoing it means a real deletion, tombstone and all.
guard pendingUploads.contains(accountID) else {
removeAccount(accountID)
return
}
accounts.removeAll { $0.id == accountID }
accountStatuses[accountID] = nil
pendingUploads.remove(accountID)
// Retracted only while the marker is still ours to retract: never
// confirmed by the server, and nothing else on the instance relying on
// it. A tombstone for the migrated account keeps synthesis suppressed
// on its own, so nothing is resurrected by this.
if pendingMarkerUploads.contains(account.instanceID),
accounts(forInstance: account.instanceID).isEmpty {
pendingMarkerUploads.remove(account.instanceID)
migratedInstanceIDs.remove(account.instanceID)
}
if storedContext.context == .account(accountID) {
correctContextAfterDeletion()
}
save()
NotificationCenter.default.post(name: .accountsDidChange, object: nil)
}
/// Drops an account and everything keyed to it, and records the permanent
/// local tombstone.
///

View File

@@ -463,13 +463,14 @@ final class AppEnvironment {
// 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
.dataContextDidChange, .accountsDidChange, .instancesDidChange,
.accountStatusesDidChange, .connectivityDidReturn
] {
contextMirrorObservers.append(
NotificationCenter.default.addObserver(forName: name, object: nil, queue: .main) { [weak self] _ in
MainActor.assumeIsolated {
guard let self else { return }
if name == .accountsDidChange || name == .instancesDidChange {
if name != .dataContextDidChange, name != .accountStatusesDidChange {
self.refreshAccountStatuses()
}
self.mirrorContextToLegacySelection()
@@ -678,7 +679,11 @@ final class AppEnvironment {
let defaults = UserDefaults.standard
guard let owner = credentialIdentityScope.current else {
guard atLaunch, defaults.string(forKey: Self.cacheOwnerKey) != nil else { return }
// No recorded owner is not the same as "nothing to protect": the
// first launch after upgrading finds caches written before anyone
// recorded whose they are, which is exactly the case that must not
// be adopted.
guard atLaunch else { return }
clearAccountScopedCaches()
defaults.removeObject(forKey: Self.cacheOwnerKey)
@@ -745,9 +750,13 @@ final class AppEnvironment {
/// instead of the pre-revamp instance-keyed Keychain lookup, which a new
/// login no longer writes.
func account(forInstance instance: Instance) -> Account? {
// The selected account is preferred only when it can actually answer.
// Returning a credential-less one would report the whole instance as
// signed out while a sibling login sits right there, usable.
if case let .account(id) = accountsManager.currentContext,
let account = accountsManager.account(id: id),
account.instanceID == instance.id {
account.instanceID == instance.id,
accountCredentialsManager(for: account)?.isLoggedIn(for: account) == true {
return account
}

View File

@@ -28,6 +28,12 @@ final class HomeInstanceCache {
/// entries that load carry the same purge.
private var pendingFeedPurge = false
/// Which login the cached feeds belong to. Bumped whenever a feed is
/// dropped because the account behind it changed, so a request that was
/// already awaiting the server cannot write the previous user's videos back
/// in — clearing a cache does not cancel what is in flight.
private var accountGeneration = 0
private init() {}
// MARK: - Cache Key Management
@@ -100,7 +106,8 @@ final class HomeInstanceCache {
}
let key = cacheKey(instanceID: instanceID, contentType: contentType)
let generation = accountGeneration
LoggingService.shared.debug(
"HomeInstanceCache.refresh: Fetching \(contentType.rawValue) from \(instance.displayName)",
category: .general
@@ -138,6 +145,14 @@ final class HomeInstanceCache {
videos = try await appEnvironment.contentService.feed(for: instance, credential: credential)
}
guard generation == accountGeneration else {
LoggingService.shared.info(
"Discarding a \(contentType.rawValue) result that belongs to the previous login",
category: .general
)
return
}
// Update cache
cache[key] = videos
lastUpdated[key] = Date()
@@ -170,6 +185,8 @@ final class HomeInstanceCache {
/// Clears cached content for a specific instance and content type.
func clear(instanceID: UUID, contentType: InstanceContentType) {
accountGeneration += 1
let key = cacheKey(instanceID: instanceID, contentType: contentType)
cache.removeValue(forKey: key)
lastUpdated.removeValue(forKey: key)
@@ -193,6 +210,8 @@ final class HomeInstanceCache {
/// otherwise be served each other's. Popular and trending are public and
/// stay.
func clearAllFeeds() {
accountGeneration += 1
// A purge can be asked for before the disk cache has been read — the
// identity check runs at startup, the load is async. Dropping only what
// is in memory would then be undone by the load, so the requirement is

View File

@@ -42,7 +42,17 @@ final class ConnectivityMonitor: @unchecked Sendable {
func startMonitoring() {
monitor.pathUpdateHandler = { [weak self] path in
Task { @MainActor [weak self] in
let wasOffline = self?.isOnline == false
self?.isOnline = path.status == .satisfied
// Anything that gave up while there was no network needs to
// hear that there is one again — an account marked unreachable
// stops reads from going out at all, so nothing else would ever
// discover it.
if wasOffline, path.status == .satisfied {
NotificationCenter.default.post(name: .connectivityDidReturn, object: nil)
}
self?.isCellular = path.usesInterfaceType(.cellular)
self?.isExpensive = path.isExpensive
self?.isConstrained = path.isConstrained
@@ -56,3 +66,8 @@ final class ConnectivityMonitor: @unchecked Sendable {
monitor.cancel()
}
}
extension Notification.Name {
/// Posted when the device goes from offline to online.
static let connectivityDidReturn = Notification.Name("stream.yattee.connectivityDidReturn")
}

View File

@@ -101,7 +101,7 @@ final class SubscriptionService {
object: nil,
queue: .main
) { [weak self] _ in
MainActor.assumeIsolated { self?.accountProviders.removeAll() }
MainActor.assumeIsolated { self?.discardAllProviders() }
}
}
@@ -160,6 +160,17 @@ final class SubscriptionService {
providerGeneration += 1
}
/// Drops every cached provider, for a change of Apple account.
///
/// Bumping the generation is the half that matters: emptying the dictionary
/// only affects future lookups, while a fetch already awaiting a response
/// still holds its provider and would otherwise deliver the previous
/// identity's subscriptions.
func discardAllProviders() {
accountProviders.removeAll()
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.

View File

@@ -418,7 +418,11 @@ private struct EditRemoteServerContent: View {
// behind it.
guard credentials.credential(for: account) != nil else {
if target.account == nil {
appEnvironment.accountsManager.removeAccount(account.id)
// Rolled back, not deleted: creating it also queued the marker
// that stops other devices synthesizing an account for this
// instance's pre-revamp login, and a sign-in that never
// completed must not leave that behind.
appEnvironment.accountsManager.rollbackAccountCreation(account.id)
}
appEnvironment.toastManager.showError(
String(localized: "accounts.signIn.storeFailed")

View File

@@ -186,6 +186,40 @@ struct AccountsManagerTests {
#expect(manager.context(forLegacyInstanceID: instanceID) == .account(second.id))
}
@Test("A sign-in that could not be stored leaves no migration marker")
func rolledBackLoginRetractsItsMarker() {
let manager = makeManager()
let instanceID = UUID()
let account = manager.addAccount(instanceID: instanceID)
#expect(manager.migratedInstanceIDs.contains(instanceID))
manager.rollbackAccountCreation(account.id)
// The marker would have told every other device that this instance's
// pre-revamp login is already represented — on the strength of an
// account that does not exist anywhere.
#expect(!manager.migratedInstanceIDs.contains(instanceID))
#expect(!manager.pendingMarkerUploads.contains(instanceID))
#expect(manager.account(id: account.id) == nil)
// And no tombstone: nothing was ever published to retract.
#expect(!manager.isDeleted(account.id))
#expect(!manager.pendingDeletions.contains(account.id))
}
@Test("Rolling back a confirmed account is a real deletion")
func rolledBackConfirmedAccountIsDeleted() {
let manager = makeManager()
let account = manager.addAccount(instanceID: UUID())
manager.confirmUpload(account.id)
manager.rollbackAccountCreation(account.id)
// It exists on the server, so other devices have to be told.
#expect(manager.isDeleted(account.id))
#expect(manager.pendingDeletions.contains(account.id))
}
@Test("An unresolved identity is not a missing credential")
func unknownStatusIsNotSignedOut() {
let defaults = makeDefaults()