mirror of
https://github.com/yattee/yattee.git
synced 2026-10-23 05:02:53 +00:00
Bind results to the account and identity they were requested for
Fetched account records are staged under the identity the fetch began under, not whichever is current when they arrive — the engine's fetch spans several delegate events and the Apple account can move between them, which would have filed one identity's accounts under another's. Subscription fetches are fenced against context switches, not only against a provider being discarded. Providers are deliberately kept across a switch — each is one account's cache — so an earlier account's fetch could finish afterwards and put its channels on screen while unsubscribes went to the account now selected. The subscriptions view reloads when the *effective* context moves, which a credential arriving, expiring or being rejected does without the stored context changing at all. Feed requests report what they learn about the account, like every other authenticated call: a credential that expired during a feed-only refresh kept looking signed in, and the fallback never engaged. The classification behind that verdict now exists once, on AccountsManager, rather than in each provider.
This commit is contained in:
@@ -606,6 +606,41 @@ final class AccountsManager {
|
||||
accountStatuses[accountID] ?? (identityScope.isResolved ? .credentialMissing : .checking)
|
||||
}
|
||||
|
||||
/// Records what a server call implies about the account that made it.
|
||||
///
|
||||
/// Deliberately conservative about rejection: a raw 401 marks the
|
||||
/// credential only when no reverse-proxy basic auth is configured, because
|
||||
/// otherwise the 401 may well have come from the proxy. The two-stage probe
|
||||
/// that disambiguates the rest is Phase 2 work; until then an ambiguous
|
||||
/// failure leaves the status alone rather than wrongly telling the user to
|
||||
/// sign in again.
|
||||
///
|
||||
/// Shared by the subscription providers and the feed cache: a credential
|
||||
/// that expires during a feed-only refresh has to mark the account too, or
|
||||
/// nothing ever reports it and the fallback never engages.
|
||||
func recordServerOutcome(_ error: Error?, for account: Account) {
|
||||
guard let error else {
|
||||
setStatus(.signedIn, for: account.id)
|
||||
return
|
||||
}
|
||||
|
||||
// A server that cannot be reached says nothing about the credential.
|
||||
// Recorded all the same: the account has to stop reporting itself as
|
||||
// usable, or reads never fall back and the user is shown failures
|
||||
// instead of local data.
|
||||
switch error {
|
||||
case APIError.timeout, APIError.noConnection:
|
||||
setStatus(.unreachable, for: account.id)
|
||||
case APIError.cancelled:
|
||||
return
|
||||
case APIError.unauthorized:
|
||||
guard instance(for: account)?.usesBasicAuth == false else { return }
|
||||
setStatus(.credentialRejected, for: account.id)
|
||||
default:
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Legacy migration
|
||||
|
||||
/// Turns each pre-revamp single-login instance into an account.
|
||||
|
||||
@@ -208,6 +208,18 @@ final class AccountSyncCoordinator {
|
||||
|
||||
// MARK: - Fetching
|
||||
|
||||
/// Notes which identity the fetch now starting belongs to.
|
||||
///
|
||||
/// The engine's fetch spans several delegate events, and the Apple account
|
||||
/// can move between them. Staging under whatever is current when the
|
||||
/// records happen to arrive would file one identity's accounts under
|
||||
/// another's.
|
||||
func fetchDidBegin() {
|
||||
fetchIdentity = (namespace, identityGeneration)
|
||||
}
|
||||
|
||||
private var fetchIdentity: (namespace: String?, generation: Int)?
|
||||
|
||||
/// Stages the account-category part of one fetch event.
|
||||
///
|
||||
/// Called before the delegate returns, so the staged state is never behind
|
||||
@@ -218,13 +230,17 @@ final class AccountSyncCoordinator {
|
||||
|
||||
guard !accountRecords.isEmpty || !accountDeletions.isEmpty else { return }
|
||||
|
||||
guard let namespace else {
|
||||
// The identity the *fetch* began under, not whatever is current now.
|
||||
let owner = fetchIdentity ?? (namespace, identityGeneration)
|
||||
|
||||
guard let namespace = owner.namespace, owner.generation == identityGeneration else {
|
||||
// Staging under a guessed identity is the one thing worse than
|
||||
// losing the records: they would be applied to, and uploaded from,
|
||||
// an account they never belonged to.
|
||||
// an account they never belonged to. A bootstrap recovers them for
|
||||
// whichever identity they turn out to belong to.
|
||||
store.hasPendingIdentityRecovery = true
|
||||
LoggingService.shared.logCloudKit(
|
||||
"Dropping \(accountRecords.count + accountDeletions.count) account records fetched before the identity resolved"
|
||||
"Dropping \(accountRecords.count + accountDeletions.count) account records fetched under an identity that has since moved"
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
@@ -1716,6 +1716,9 @@ extension CloudKitSyncEngine: CKSyncEngineDelegate {
|
||||
case .willFetchChanges:
|
||||
LoggingService.shared.logCloudKit("CKSyncEngine will fetch changes (auto-triggered)")
|
||||
isReceivingChanges = true
|
||||
// Binds everything this fetch delivers to the identity it
|
||||
// started under; the Apple account can move mid-fetch.
|
||||
accountSync?.fetchDidBegin()
|
||||
|
||||
case .didFetchChanges:
|
||||
LoggingService.shared.logCloudKit("CKSyncEngine finished fetching changes")
|
||||
|
||||
@@ -237,6 +237,19 @@ final class SubscriptionFeedCache {
|
||||
saveToDisk()
|
||||
}
|
||||
|
||||
/// Reports what a feed request implies about the account it used.
|
||||
///
|
||||
/// The feed talks to the same servers as the subscription providers but
|
||||
/// does not go through one, so without this a credential that expires
|
||||
/// during a feed-only refresh would keep looking signed in and the local
|
||||
/// fallback would never engage.
|
||||
private func recordOutcome(_ error: Error?, backend: AppEnvironment.FeedBackend, using appEnvironment: AppEnvironment) {
|
||||
let type: InstanceType = backend == .invidious ? .invidious : .piped
|
||||
guard let account = appEnvironment.account(forSelectedContext: type) else { return }
|
||||
|
||||
appEnvironment.accountsManager.recordServerOutcome(error, for: account)
|
||||
}
|
||||
|
||||
// MARK: - Account Change Handling
|
||||
|
||||
/// Clears cache when subscription account changes.
|
||||
@@ -412,6 +425,7 @@ final class SubscriptionFeedCache {
|
||||
guard isCurrentAccountsResult(generation) else { return }
|
||||
|
||||
LoggingService.shared.info("Piped feed: Received \(feedVideos.count) videos", category: .general)
|
||||
recordOutcome(nil, backend: .piped, using: appEnvironment)
|
||||
|
||||
update(videos: feedVideos, generation: generation)
|
||||
appEnvironment.dataManager.updateLastVideoPublishedDates(from: feedVideos)
|
||||
@@ -419,6 +433,7 @@ final class SubscriptionFeedCache {
|
||||
feedLoadState = .ready
|
||||
} catch {
|
||||
LoggingService.shared.error("Failed to fetch Piped feed: \(error.localizedDescription)", category: .general)
|
||||
recordOutcome(error, backend: .piped, using: appEnvironment)
|
||||
guard isCurrentAccountsResult(generation) else { return }
|
||||
feedLoadState = .error(.networkError(error.localizedDescription))
|
||||
}
|
||||
@@ -474,6 +489,7 @@ final class SubscriptionFeedCache {
|
||||
"Invidious feed: Received \(feedVideos.count) videos (page \(currentPage))",
|
||||
category: .general
|
||||
)
|
||||
recordOutcome(nil, backend: .invidious, using: appEnvironment)
|
||||
|
||||
// For first page, replace all videos. For subsequent pages, this shouldn't be called.
|
||||
update(videos: feedVideos, generation: generation)
|
||||
@@ -492,6 +508,7 @@ final class SubscriptionFeedCache {
|
||||
"Failed to fetch Invidious feed: \(error.localizedDescription)",
|
||||
category: .general
|
||||
)
|
||||
recordOutcome(error, backend: .invidious, using: appEnvironment)
|
||||
guard isCurrentAccountsResult(generation) else { return }
|
||||
feedLoadState = .error(.networkError(error.localizedDescription))
|
||||
}
|
||||
|
||||
@@ -103,6 +103,18 @@ final class SubscriptionService {
|
||||
) { [weak self] _ in
|
||||
MainActor.assumeIsolated { self?.discardAllProviders() }
|
||||
}
|
||||
|
||||
// Switching context deliberately keeps the providers — each is one
|
||||
// account's cache and stays valid — but a fetch already running belongs
|
||||
// to the account that was selected when it started, and finishing after
|
||||
// the switch would put its channels on screen for another one.
|
||||
NotificationCenter.default.addObserver(
|
||||
forName: .dataContextDidChange,
|
||||
object: nil,
|
||||
queue: .main
|
||||
) { [weak self] _ in
|
||||
MainActor.assumeIsolated { self?.providerGeneration += 1 }
|
||||
}
|
||||
}
|
||||
|
||||
// MARK: - Current Provider
|
||||
@@ -174,7 +186,7 @@ final class SubscriptionService {
|
||||
/// 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
|
||||
fileprivate var providerGeneration = 0
|
||||
|
||||
/// The provider for **writes**.
|
||||
///
|
||||
|
||||
@@ -24,39 +24,11 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider {
|
||||
private let credentialsManager: InvidiousCredentialsManager
|
||||
private let accountsManager: AccountsManager
|
||||
|
||||
/// Records the outcome of a server call against the account.
|
||||
///
|
||||
/// Deliberately conservative: a raw 401 marks the credential rejected only
|
||||
/// when no reverse-proxy basic auth is configured, because otherwise the
|
||||
/// 401 may well have come from the proxy rather than the account. The
|
||||
/// two-stage probe that disambiguates the remaining case is Phase 2 work;
|
||||
/// until then an ambiguous failure leaves the status untouched rather than
|
||||
/// wrongly telling the user to sign in again.
|
||||
/// Records the outcome of a server call against the account. The
|
||||
/// classification lives in one place so the feed cache, which talks to the
|
||||
/// same servers without going through a provider, reaches the same verdict.
|
||||
private func recordOutcome(_ error: Error?) {
|
||||
guard let error else {
|
||||
accountsManager.setStatus(.signedIn, for: account.id)
|
||||
return
|
||||
}
|
||||
|
||||
// A server that cannot be reached says nothing about the credential.
|
||||
// Recorded all the same: the account has to stop reporting itself as
|
||||
// usable, or reads never fall back and the user is shown failures
|
||||
// instead of local data.
|
||||
switch error {
|
||||
case APIError.timeout, APIError.noConnection:
|
||||
accountsManager.setStatus(.unreachable, for: account.id)
|
||||
return
|
||||
case APIError.cancelled:
|
||||
return
|
||||
case APIError.unauthorized:
|
||||
break
|
||||
default:
|
||||
return
|
||||
}
|
||||
|
||||
guard accountsManager.instance(for: account)?.usesBasicAuth == false else { return }
|
||||
|
||||
accountsManager.setStatus(.credentialRejected, for: account.id)
|
||||
accountsManager.recordServerOutcome(error, for: account)
|
||||
}
|
||||
private let settingsManager: SettingsManager
|
||||
|
||||
|
||||
@@ -24,39 +24,11 @@ final class PipedSubscriptionProvider: SubscriptionProvider {
|
||||
private let credentialsManager: PipedCredentialsManager
|
||||
private let accountsManager: AccountsManager
|
||||
|
||||
/// Records the outcome of a server call against the account.
|
||||
///
|
||||
/// Deliberately conservative: a raw 401 marks the credential rejected only
|
||||
/// when no reverse-proxy basic auth is configured, because otherwise the
|
||||
/// 401 may well have come from the proxy rather than the account. The
|
||||
/// two-stage probe that disambiguates the remaining case is Phase 2 work;
|
||||
/// until then an ambiguous failure leaves the status untouched rather than
|
||||
/// wrongly telling the user to sign in again.
|
||||
/// Records the outcome of a server call against the account. The
|
||||
/// classification lives in one place so the feed cache, which talks to the
|
||||
/// same servers without going through a provider, reaches the same verdict.
|
||||
private func recordOutcome(_ error: Error?) {
|
||||
guard let error else {
|
||||
accountsManager.setStatus(.signedIn, for: account.id)
|
||||
return
|
||||
}
|
||||
|
||||
// A server that cannot be reached says nothing about the credential.
|
||||
// Recorded all the same: the account has to stop reporting itself as
|
||||
// usable, or reads never fall back and the user is shown failures
|
||||
// instead of local data.
|
||||
switch error {
|
||||
case APIError.timeout, APIError.noConnection:
|
||||
accountsManager.setStatus(.unreachable, for: account.id)
|
||||
return
|
||||
case APIError.cancelled:
|
||||
return
|
||||
case APIError.unauthorized:
|
||||
break
|
||||
default:
|
||||
return
|
||||
}
|
||||
|
||||
guard accountsManager.instance(for: account)?.usesBasicAuth == false else { return }
|
||||
|
||||
accountsManager.setStatus(.credentialRejected, for: account.id)
|
||||
accountsManager.recordServerOutcome(error, for: account)
|
||||
}
|
||||
private let settingsManager: SettingsManager
|
||||
|
||||
|
||||
@@ -22,6 +22,9 @@ struct SubscriptionsView: View {
|
||||
@State private var feedCache = SubscriptionFeedCache.shared
|
||||
@State private var subscriptions: [Subscription] = []
|
||||
@State private var subscriptionsLoaded = false
|
||||
/// The effective context the visible subscriptions were loaded for, so a
|
||||
/// status change that does not move it does not force a reload.
|
||||
@State private var loadedContext: DataContext?
|
||||
@State private var selectedChannelID: String? = nil
|
||||
@State private var errorMessage: String?
|
||||
@State private var watchEntriesMap: [String: WatchEntry] = [:]
|
||||
@@ -507,15 +510,17 @@ struct SubscriptionsView: View {
|
||||
// switching between them would change nothing here and this
|
||||
// view would keep showing the first account's subscriptions
|
||||
// while writes went to the second.
|
||||
//
|
||||
// Status changes count too, through the *effective* context:
|
||||
// a credential arriving or being rejected moves reads
|
||||
// between the account and local without the stored context
|
||||
// moving at all.
|
||||
.onReceive(NotificationCenter.default.publisher(for: .dataContextDidChange)) { _ in
|
||||
feedCache.handleAccountChange()
|
||||
subscriptions = []
|
||||
subscriptionsLoaded = false
|
||||
Task {
|
||||
guard let appEnvironment else { return }
|
||||
await loadSubscriptionsAsync()
|
||||
await feedCache.refresh(using: appEnvironment)
|
||||
}
|
||||
reloadForContextChange()
|
||||
}
|
||||
.onReceive(NotificationCenter.default.publisher(for: .accountStatusesDidChange)) { _ in
|
||||
guard appEnvironment?.dataContextValidator.effectiveContext != loadedContext else { return }
|
||||
reloadForContextChange()
|
||||
}
|
||||
.task(id: instanceConfigurationID) {
|
||||
LoggingService.shared.debug("SubscriptionsView task triggered, instanceConfigurationID: \(instanceConfigurationID)", category: .general)
|
||||
@@ -1333,9 +1338,28 @@ struct SubscriptionsView: View {
|
||||
|
||||
/// Loads subscriptions asynchronously from the current provider.
|
||||
/// For Invidious, this fetches from the API and creates temporary Subscription objects for UI.
|
||||
/// Drops what is on screen and reloads it for whatever context is current.
|
||||
private func reloadForContextChange() {
|
||||
loadedContext = appEnvironment?.dataContextValidator.effectiveContext
|
||||
feedCache.handleAccountChange()
|
||||
subscriptions = []
|
||||
subscriptionsLoaded = false
|
||||
|
||||
Task {
|
||||
guard let appEnvironment else { return }
|
||||
await loadSubscriptionsAsync()
|
||||
await feedCache.refresh(using: appEnvironment)
|
||||
}
|
||||
}
|
||||
|
||||
private func loadSubscriptionsAsync() async {
|
||||
guard let subscriptionService, let appEnvironment else { return }
|
||||
|
||||
// Recorded on every load, not only on a context change: the first load
|
||||
// of a session has to establish what is on screen, or the first status
|
||||
// change would look like a move and reload for nothing.
|
||||
loadedContext = appEnvironment.dataContextValidator.effectiveContext
|
||||
|
||||
// For local account, just load from DataManager (fast)
|
||||
if appEnvironment.accountsManager.currentContext.isLocal {
|
||||
subscriptions = dataManager?.subscriptions() ?? []
|
||||
|
||||
Reference in New Issue
Block a user