From 5ac6b7188d130889018e68f3379ae63476402f1a Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Mon, 7 Sep 2026 18:48:50 +0200 Subject: [PATCH] Ask what a backend supports instead of what it is --- Yattee/Core/AppEnvironment.swift | 11 ++++ Yattee/Core/InstancesManager.swift | 20 ++++-- Yattee/Localizable.xcstrings | 20 ------ Yattee/Models/AccountCapabilities.swift | 13 ++++ Yattee/Models/Instance.swift | 32 ++++++--- Yattee/Models/SubscriptionAccountType.swift | 66 ++++--------------- .../LegacyDataMigrationService.swift | 5 ++ Yattee/Views/Settings/EditSourceView.swift | 63 ++++++++++-------- .../Views/Settings/ImportPlaylistsView.swift | 4 +- .../Settings/ImportSubscriptionsView.swift | 2 +- .../Subscriptions/SubscriptionsView.swift | 2 +- YatteeTests/AccountsManagerTests.swift | 57 ++++++++++++++++ YatteeTests/InstancesManagerTests.swift | 16 +++++ 13 files changed, 187 insertions(+), 124 deletions(-) diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index 31ca3328..70fb2397 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -933,6 +933,10 @@ final class AppEnvironment { // playlists would keep answering for an account that no longer exists. discardAccountData(for: account.id) + // Lowering the pre-revamp flag once nothing is left signed in on the + // instance. Migration-only state (see `Instance.usesAccountLogin`): + // left raised, it is what `needsCredentials(for:)` would keep reporting + // a missing credential from. if let instance, accountsManager.accounts(forInstance: instance.id).isEmpty { instancesManager.setUsesAccountLogin(false, for: instance) } @@ -1174,6 +1178,13 @@ final class AppEnvironment { /// before these flags existed, based on credentials currently in the Keychain. /// Only ever sets the flags — a missing Keychain entry (e.g. right after a /// reinstall) must not clear a previously recorded flag. + /// + /// The `usesAccountLogin` half is one-way migration machinery: an upgrading + /// install whose flag was lost (it syncs through iCloud key-value storage) + /// but whose instance-keyed credential survived would otherwise never have + /// its legacy account synthesized by + /// `AccountsManager.reconcileLegacyAccounts()`. Fresh sign-ins never set + /// the flag. private func backfillCredentialFlags() { for instance in instancesManager.instances { if !instance.usesBasicAuth, diff --git a/Yattee/Core/InstancesManager.swift b/Yattee/Core/InstancesManager.swift index fb935402..2999b780 100644 --- a/Yattee/Core/InstancesManager.swift +++ b/Yattee/Core/InstancesManager.swift @@ -197,9 +197,13 @@ final class InstancesManager { saveInstances() } - /// Records whether the user has logged into an account on an instance. - /// Persisted (and synced to iCloud) so missing Keychain credentials can be - /// detected after a reinstall. + /// Records that a pre-revamp single login existed on an instance. + /// + /// Migration-only: every caller is migration machinery (a Yattee 1 import, + /// the credential backfill, or the last account on the instance going + /// away). Signing in deliberately never calls this — accounts describe + /// logins now (see `AccountSignInHandler`, and `Instance.usesAccountLogin` + /// for who writes it and why). func setUsesAccountLogin(_ value: Bool, for instance: Instance) { guard let index = instances.firstIndex(where: { $0.id == instance.id }), instances[index].usesAccountLogin != value else { return } @@ -260,12 +264,14 @@ final class InstancesManager { instances.contains { $0.isYatteeServerInstance } } - var invidiousPipedInstances: [Instance] { - instances.filter { $0.type == .invidious || $0.type == .piped } + /// Instances whose backend has accounts at all, asked of the capability + /// table rather than a type list. + var accountCapableInstances: [Instance] { + instances.filter { $0.type.accountCapabilities.supportsAccounts } } - var hasInvidiousPipedInstances: Bool { - instances.contains { $0.type == .invidious || $0.type == .piped } + var hasAccountCapableInstances: Bool { + instances.contains { $0.type.accountCapabilities.supportsAccounts } } var enabledYatteeServerInstances: [Instance] { diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index c974576e..7e7007d2 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -18136,16 +18136,6 @@ }, "subscription.error.providerNotAvailable" : { - }, - "subscriptions.account.invidious" : { - "localizations" : { - "en" : { - "stringUnit" : { - "state" : "translated", - "value" : "Invidious Account" - } - } - } }, "subscriptions.account.invidiousInstance %@" : { "localizations" : { @@ -18167,16 +18157,6 @@ } } }, - "subscriptions.account.piped" : { - "localizations" : { - "en" : { - "stringUnit" : { - "state" : "translated", - "value" : "Piped Account" - } - } - } - }, "subscriptions.account.pipedInstance %@" : { "localizations" : { "en" : { diff --git a/Yattee/Models/AccountCapabilities.swift b/Yattee/Models/AccountCapabilities.swift index 5c64e75e..d61723c5 100644 --- a/Yattee/Models/AccountCapabilities.swift +++ b/Yattee/Models/AccountCapabilities.swift @@ -31,6 +31,19 @@ struct AccountCapabilities: Hashable, Sendable { supportsPlaylistPrivacy: false ) + /// Whether the backend has accounts at all — true as soon as an account + /// could do anything on it. This is what decides whether an instance offers + /// a sign-in; it is deliberately broader than ``isSelectableContext``, which + /// asks the narrower question of whether an account is worth switching *to*. + var supportsAccounts: Bool { + supportsSubscriptions + || supportsFeed + || supportsPlaylistReads + || supportsPlaylistWrites + || supportsWatchHistory + || supportsPlaylistPrivacy + } + /// Whether this account can act as a `DataContext` at all. An account with /// nothing to offer is never listed as a switch target. var isSelectableContext: Bool { diff --git a/Yattee/Models/Instance.swift b/Yattee/Models/Instance.swift index 0d531cd4..1b64ccc6 100644 --- a/Yattee/Models/Instance.swift +++ b/Yattee/Models/Instance.swift @@ -67,9 +67,6 @@ struct Instance: Identifiable, Codable, Hashable, Sendable { /// The date this instance was added. let dateAdded: Date - /// Optional API key if required by the instance. - var apiKey: String? - /// Whether to allow invalid/self-signed SSL certificates. var allowInvalidCertificates: Bool @@ -82,8 +79,21 @@ struct Instance: Identifiable, Codable, Hashable, Sendable { /// Yattee Server always requires basic auth regardless of this flag. var usesBasicAuth: Bool - /// Whether the user has logged into an account on this instance (Invidious/Piped). - /// Set on login, so a missing Keychain credential can be detected after reinstall. + /// Whether a pre-revamp single login existed on this instance. + /// + /// Migration-only state. Signing in no longer sets it (see + /// `AccountSignInHandler`) — accounts describe logins now. It survives + /// because `AccountsManager.reconcileLegacyAccounts()` reads it to + /// synthesize the account an upgrading install should have, and because a + /// missing Keychain credential on an instance that has no accounts yet can + /// still only be detected through it. + /// + /// Everything that writes it is migration machinery: + /// `LegacyDataMigrationService` raises it for a login imported from + /// Yattee 1, `AppEnvironment.backfillCredentialFlags()` revives it when the + /// flag was lost but its instance-keyed credential was not, and + /// `AppEnvironment.signOutAndRemoveAccount(_:)` lowers it once the last + /// account on the instance is gone. var usesAccountLogin: Bool // MARK: - Initialization @@ -95,7 +105,6 @@ struct Instance: Identifiable, Codable, Hashable, Sendable { name: String? = nil, isEnabled: Bool = true, dateAdded: Date = Date(), - apiKey: String? = nil, allowInvalidCertificates: Bool = false, proxiesVideos: Bool = false, usesBasicAuth: Bool = false, @@ -107,7 +116,6 @@ struct Instance: Identifiable, Codable, Hashable, Sendable { self.name = name self.isEnabled = isEnabled self.dateAdded = dateAdded - self.apiKey = apiKey self.allowInvalidCertificates = allowInvalidCertificates self.proxiesVideos = proxiesVideos self.usesBasicAuth = usesBasicAuth @@ -122,7 +130,6 @@ struct Instance: Identifiable, Codable, Hashable, Sendable { name = try container.decodeIfPresent(String.self, forKey: .name) isEnabled = try container.decode(Bool.self, forKey: .isEnabled) dateAdded = try container.decode(Date.self, forKey: .dateAdded) - apiKey = try container.decodeIfPresent(String.self, forKey: .apiKey) allowInvalidCertificates = try container.decode(Bool.self, forKey: .allowInvalidCertificates) proxiesVideos = try container.decodeIfPresent(Bool.self, forKey: .proxiesVideos) ?? false usesBasicAuth = try container.decodeIfPresent(Bool.self, forKey: .usesBasicAuth) ?? false @@ -175,13 +182,18 @@ extension Instance { } /// Whether this instance supports user authentication/login. + /// + /// Answered by the capability table rather than a type list: an instance + /// offers a sign-in exactly when an account on it can do something, so a + /// backend that gains accounts gets its login by adding one row in + /// ``InstanceType/accountCapabilities``. var supportsAuthentication: Bool { - type == .invidious || type == .piped + type.accountCapabilities.supportsAccounts } /// Whether this instance supports subscription feed. var supportsFeed: Bool { - type == .invidious || type == .piped + type.accountCapabilities.supportsFeed } /// Whether this instance supports search suggestions/autocomplete. diff --git a/Yattee/Models/SubscriptionAccountType.swift b/Yattee/Models/SubscriptionAccountType.swift index f0cbffe0..d2c33a9c 100644 --- a/Yattee/Models/SubscriptionAccountType.swift +++ b/Yattee/Models/SubscriptionAccountType.swift @@ -2,79 +2,35 @@ // SubscriptionAccountType.swift // Yattee // -// Subscription account type and configuration for managing channel subscriptions. +// Pre-revamp subscription account state, kept only so it can be decoded. // import Foundation /// The type of account used for managing subscriptions. +/// +/// Legacy: superseded by `DataContext` and `Account`. Nothing writes this type +/// any more — it survives so `AccountsManager.legacySubscriptionAccount()` can +/// read the selection an upgrading install persisted, and +/// `DataContext.init(legacy:)` can map it onto a context. Do not add members. enum SubscriptionAccountType: String, Codable, Sendable, Equatable, CaseIterable { /// Local subscriptions synced via iCloud (requires Yattee Server for feed). case local - + /// Subscriptions managed by an Invidious instance account. case invidious - + /// Subscriptions managed by a Piped instance account. case piped - - /// Display name for the account type. - var displayName: String { - switch self { - case .local: - return String(localized: "subscriptions.account.local") - case .invidious: - return String(localized: "subscriptions.account.invidious") - case .piped: - return String(localized: "subscriptions.account.piped") - } - } } -/// Represents a configured subscription account. -/// Combines the account type with an optional instance identifier. +/// A configured subscription account as the previous release persisted it. +/// Decode-only; see `SubscriptionAccountType`. struct SubscriptionAccount: Codable, Sendable, Equatable, Hashable { /// The type of subscription account. let type: SubscriptionAccountType - + /// The instance UUID for instance-based accounts (Invidious/Piped). /// Nil for local accounts. let instanceID: UUID? - - /// Local subscription account (iCloud-synced, requires Yattee Server). - static let local = SubscriptionAccount(type: .local, instanceID: nil) - - /// Creates an Invidious subscription account for the given instance. - /// - Parameter instanceID: The UUID of the Invidious instance. - /// - Returns: A subscription account configured for Invidious. - static func invidious(_ instanceID: UUID) -> SubscriptionAccount { - SubscriptionAccount(type: .invidious, instanceID: instanceID) - } - - /// Creates a Piped subscription account for the given instance. - /// - Parameter instanceID: The UUID of the Piped instance. - /// - Returns: A subscription account configured for Piped. - static func piped(_ instanceID: UUID) -> SubscriptionAccount { - SubscriptionAccount(type: .piped, instanceID: instanceID) - } - - /// Whether this account requires an authenticated instance. - var requiresAuthentication: Bool { - switch type { - case .local: - return false - case .invidious, .piped: - return true - } - } - - /// Whether this account requires Yattee Server for feed fetching. - var requiresYatteeServer: Bool { - switch type { - case .local: - return true - case .invidious, .piped: - return false - } - } } diff --git a/Yattee/Services/Migration/LegacyDataMigrationService.swift b/Yattee/Services/Migration/LegacyDataMigrationService.swift index 817e720a..8ac6a047 100644 --- a/Yattee/Services/Migration/LegacyDataMigrationService.swift +++ b/Yattee/Services/Migration/LegacyDataMigrationService.swift @@ -192,6 +192,11 @@ final class LegacyDataMigrationService { /// Re-creates a legacy account by signing in with fresh credentials. /// The matching source is created if needed; otherwise the existing matching source is reused. + /// + /// The instance-keyed credential and `usesAccountLogin` written here are + /// deliberately pre-revamp shaped: this is a Yattee 1 login arriving, and + /// `AccountsManager.reconcileLegacyAccounts()` is what turns it into an + /// account (see `Instance.usesAccountLogin`). /// - Parameters: /// - item: The legacy account to import /// - username: Username/email to use for the v2 login diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index 446e57b4..9583699b 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -199,42 +199,49 @@ private struct EditRemoteServerContent: View { if instance.supportsAuthentication { InstanceAccountsSection(instance: instance, loginTarget: $loginTarget) - // Import section - show when logged in for Invidious and Piped instances - if isLoggedIn && (instance.type == .invidious || instance.type == .piped) { + // Import section - each entry offered only where the backend + // has something to import from. + if isLoggedIn, + instance.type.accountCapabilities.supportsSubscriptions + || instance.type.accountCapabilities.supportsPlaylistReads { Section { - NavigationLink { - #if os(tvOS) - TVSidebarDetailContainer( - systemImage: "person.2", - title: String(localized: "sources.import.subscriptions"), - showsDismissButton: true - ) { + if instance.type.accountCapabilities.supportsSubscriptions { + NavigationLink { + #if os(tvOS) + TVSidebarDetailContainer( + systemImage: "person.2", + title: String(localized: "sources.import.subscriptions"), + showsDismissButton: true + ) { + ImportSubscriptionsView(instance: instance) + } + #else ImportSubscriptionsView(instance: instance) + #endif + } label: { + Label(String(localized: "sources.import.subscriptions"), systemImage: "person.2") } - #else - ImportSubscriptionsView(instance: instance) - #endif - } label: { - Label(String(localized: "sources.import.subscriptions"), systemImage: "person.2") + .accessibilityIdentifier("sources.import.subscriptions") } - .accessibilityIdentifier("sources.import.subscriptions") - NavigationLink { - #if os(tvOS) - TVSidebarDetailContainer( - systemImage: "list.bullet.rectangle", - title: String(localized: "sources.import.playlists"), - showsDismissButton: true - ) { + if instance.type.accountCapabilities.supportsPlaylistReads { + NavigationLink { + #if os(tvOS) + TVSidebarDetailContainer( + systemImage: "list.bullet.rectangle", + title: String(localized: "sources.import.playlists"), + showsDismissButton: true + ) { + ImportPlaylistsView(instance: instance) + } + #else ImportPlaylistsView(instance: instance) + #endif + } label: { + Label(String(localized: "sources.import.playlists"), systemImage: "list.bullet.rectangle") } - #else - ImportPlaylistsView(instance: instance) - #endif - } label: { - Label(String(localized: "sources.import.playlists"), systemImage: "list.bullet.rectangle") + .accessibilityIdentifier("sources.import.playlists") } - .accessibilityIdentifier("sources.import.playlists") } header: { Text(String(localized: "sources.header.import")) } diff --git a/Yattee/Views/Settings/ImportPlaylistsView.swift b/Yattee/Views/Settings/ImportPlaylistsView.swift index 4536d7c7..e028d700 100644 --- a/Yattee/Views/Settings/ImportPlaylistsView.swift +++ b/Yattee/Views/Settings/ImportPlaylistsView.swift @@ -310,7 +310,7 @@ struct ImportPlaylistsView: View { importProgress = nil do { - guard instance.type == .invidious || instance.type == .piped else { + guard instance.type.accountCapabilities.supportsPlaylistReads else { throw ImportError.notSupported } @@ -430,7 +430,7 @@ struct ImportPlaylistsView: View { existingLocalPlaylist = nil do { - guard instance.type == .invidious || instance.type == .piped else { + guard instance.type.accountCapabilities.supportsPlaylistReads else { throw ImportError.notSupported } diff --git a/Yattee/Views/Settings/ImportSubscriptionsView.swift b/Yattee/Views/Settings/ImportSubscriptionsView.swift index cf114bba..ab7b82a8 100644 --- a/Yattee/Views/Settings/ImportSubscriptionsView.swift +++ b/Yattee/Views/Settings/ImportSubscriptionsView.swift @@ -224,7 +224,7 @@ struct ImportSubscriptionsView: View { error = nil do { - guard instance.type == .invidious || instance.type == .piped else { + guard instance.type.accountCapabilities.supportsSubscriptions else { throw ImportError.notSupported } diff --git a/Yattee/Views/Subscriptions/SubscriptionsView.swift b/Yattee/Views/Subscriptions/SubscriptionsView.swift index 856cd4eb..351856e3 100644 --- a/Yattee/Views/Subscriptions/SubscriptionsView.swift +++ b/Yattee/Views/Subscriptions/SubscriptionsView.swift @@ -194,7 +194,7 @@ struct SubscriptionsView: View { } return instances .filter { $0.type == .yatteeServer } - .map { "\($0.id):\($0.isEnabled):\($0.apiKey?.isEmpty == false)" } + .map { "\($0.id):\($0.isEnabled)" } .joined(separator: "|") } diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index e5236a37..4c35ec6a 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -1209,6 +1209,63 @@ struct AccountCapabilitiesTests { #expect(!InstanceType.peertube.accountCapabilities.isSelectableContext) #expect(!InstanceType.yatteeServer.accountCapabilities.isSelectableContext) } + + @Test("A backend has accounts as soon as one flag is set") + func supportsAccountsIsAnyCapability() { + #expect(!AccountCapabilities.none.supportsAccounts) + + // Broader than isSelectableContext deliberately: a backend whose only + // capability is watch history still has an account to sign into, even + // though there is nothing there worth switching to. + var historyOnly = AccountCapabilities.none + historyOnly.supportsWatchHistory = true + #expect(historyOnly.supportsAccounts) + #expect(!historyOnly.isSelectableContext) + + #expect(InstanceType.invidious.accountCapabilities.supportsAccounts) + #expect(InstanceType.piped.accountCapabilities.supportsAccounts) + #expect(!InstanceType.peertube.accountCapabilities.supportsAccounts) + #expect(!InstanceType.yatteeServer.accountCapabilities.supportsAccounts) + } + + @Test("Instance capabilities are answered by the capability table") + func instanceFlagsDelegateToCapabilities() { + for type in InstanceType.allCases { + let instance = Instance(type: type, url: URL(string: "https://example.com")!) + #expect(instance.supportsAuthentication == type.accountCapabilities.supportsAccounts) + #expect(instance.supportsFeed == type.accountCapabilities.supportsFeed) + } + } + + @Test("Import is offered only where the backend has something to import") + func importGuardsFollowCapabilities() { + // What EditSourceView's menu entries and the two import views' guards + // ask before fetching. + #expect(InstanceType.piped.accountCapabilities.supportsSubscriptions) + #expect(InstanceType.piped.accountCapabilities.supportsPlaylistReads) + #expect(InstanceType.invidious.accountCapabilities.supportsSubscriptions) + #expect(InstanceType.invidious.accountCapabilities.supportsPlaylistReads) + + for type in [InstanceType.peertube, .yatteeServer] { + #expect(!type.accountCapabilities.supportsSubscriptions) + #expect(!type.accountCapabilities.supportsPlaylistReads) + } + } +} + +/// Constructors for the pre-revamp value the migration decodes. They live here +/// rather than on the model because nothing in the app builds one any more — +/// `SubscriptionAccount` exists only to be read off disk. +extension SubscriptionAccount { + static let local = SubscriptionAccount(type: .local, instanceID: nil) + + static func invidious(_ instanceID: UUID) -> SubscriptionAccount { + SubscriptionAccount(type: .invidious, instanceID: instanceID) + } + + static func piped(_ instanceID: UUID) -> SubscriptionAccount { + SubscriptionAccount(type: .piped, instanceID: instanceID) + } } @Suite("DataContext legacy migration") diff --git a/YatteeTests/InstancesManagerTests.swift b/YatteeTests/InstancesManagerTests.swift index 673aee63..16d65213 100644 --- a/YatteeTests/InstancesManagerTests.swift +++ b/YatteeTests/InstancesManagerTests.swift @@ -28,6 +28,22 @@ struct InstanceFilteringTests { #expect(youtubeInstances.allSatisfy { $0.type == .invidious || $0.type == .piped }) } + @Test("Account-capable instances come from the capability table") + func accountCapableInstancesFollowCapabilities() { + let manager = makeTestInstancesManager() + #expect(!manager.hasAccountCapableInstances) + + manager.add(Instance(type: .peertube, url: URL(string: "https://pt.example.com")!)) + manager.add(Instance(type: .yatteeServer, url: URL(string: "https://ys.example.com")!)) + #expect(manager.accountCapableInstances.isEmpty) + #expect(!manager.hasAccountCapableInstances) + + manager.add(Instance(type: .invidious, url: URL(string: "https://inv.example.com")!)) + manager.add(Instance(type: .piped, url: URL(string: "https://piped.example.com")!)) + #expect(manager.accountCapableInstances.map(\.type) == [.invidious, .piped]) + #expect(manager.hasAccountCapableInstances) + } + @Test("Filter PeerTube instances") func filterPeerTubeInstances() { let instances = [