diff --git a/Palace.xcodeproj/project.pbxproj b/Palace.xcodeproj/project.pbxproj index bc3c23158..06f08bfcf 100644 --- a/Palace.xcodeproj/project.pbxproj +++ b/Palace.xcodeproj/project.pbxproj @@ -58,6 +58,7 @@ 08D9D47ABBE9431A93B73510 /* AudiobookDataManagerModelsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D45E9ABFFE8142AFB578979F /* AudiobookDataManagerModelsTests.swift */; }; 08E5DBD9DF8A02E6C155BA2C /* ReaderTypographyButton.swift in Sources */ = {isa = PBXBuildFile; fileRef = C41E92244AABB4F1FE944CF8 /* ReaderTypographyButton.swift */; }; 091A2B3C4D5E6F7081921324 /* TPPPreferredAuthSelectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F8091A2B3C4D5E6F70819213 /* TPPPreferredAuthSelectionTests.swift */; }; + 092C37A2AA45C900DCCD7A8E /* AccountRegistryStoreSeamTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1D5D921DA42F7B66434E0D5F /* AccountRegistryStoreSeamTests.swift */; }; 097BAE0BCFDD15E6EC7C7854 /* BadgesViewModelTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A8B7ED878FCA626AD3519F6E /* BadgesViewModelTests.swift */; }; 09D98A92B2EE80E093A1EDD5 /* AudiobookSessionManaging.swift in Sources */ = {isa = PBXBuildFile; fileRef = F9F719C4DEA011976097371B /* AudiobookSessionManaging.swift */; }; 0A007DDD59BEB4E160FFA769 /* OfflineAction.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8095146D959C854EDE502449 /* OfflineAction.swift */; }; @@ -66,6 +67,7 @@ 0AD530FB6EABC6946F799D87 /* EPUBPositionAdapter.swift in Sources */ = {isa = PBXBuildFile; fileRef = B3FBCE9F366E7A29724332D4 /* EPUBPositionAdapter.swift */; }; 0AE0A0299BFC49CE91440D54 /* AccountDetailView+Constants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 23EE1204A0914891AF2B954A /* AccountDetailView+Constants.swift */; }; 0AE0A02A9BFC49CE91440D55 /* AccountDetailView+Constants.swift in Sources */ = {isa = PBXBuildFile; fileRef = 23EE1204A0914891AF2B954A /* AccountDetailView+Constants.swift */; }; + 0AE214B06AC5C39759675519 /* AccountRegistryStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = C49DDACF93DCF459DC310653 /* AccountRegistryStore.swift */; }; 0B1838E6F21342A31DC01D12 /* AdobeDRMHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = FE0CCABB8C99DA55C43F3DA3 /* AdobeDRMHandler.swift */; }; 0B461FFD655EC35ED6C3990A /* OPDSFeedCache.swift in Sources */ = {isa = PBXBuildFile; fileRef = ACE8017D2ECF7E0FEBFAAB1A /* OPDSFeedCache.swift */; }; 0B55EFB324FAF34D779EB878 /* StreamingReaderPresentationContractTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3A213D524079F39BA952025A /* StreamingReaderPresentationContractTests.swift */; }; @@ -434,6 +436,7 @@ 46D82B974AA50B5FF3AABA1F /* AudiobookLoaderDispatchTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2D831302F218AC0EE46308A0 /* AudiobookLoaderDispatchTests.swift */; }; 46E0E670ECA43B1BE1769CA7 /* AccountSwitchBorrowReauthCouplingContractTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = CE04BB7BA065CC171F20537B /* AccountSwitchBorrowReauthCouplingContractTests.swift */; }; 4714FCCE477E3480FCAB241C /* BadgeServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C927D5FB4DCC629EEF09DE4C /* BadgeServiceTests.swift */; }; + 4731E519F95F0E4A6F77BFBF /* AccountRegistryStore.swift in Sources */ = {isa = PBXBuildFile; fileRef = C49DDACF93DCF459DC310653 /* AccountRegistryStore.swift */; }; 47E7D42D22FE22D4C9489931 /* SignInModalSheetPresenter.swift in Sources */ = {isa = PBXBuildFile; fileRef = A8829DAED777C8EBAB199D77 /* SignInModalSheetPresenter.swift */; }; 483261822C71A1A249B7F279 /* ContinueRowSectionTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4057C151D980D26B04B8FC53 /* ContinueRowSectionTests.swift */; }; 48333F55E5F2AC51E90C1DFC /* PDFKitThumbnailProviderTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = BCEAFA525379AFA3FE0553DE /* PDFKitThumbnailProviderTests.swift */; }; @@ -2299,6 +2302,7 @@ 1BA91B27F363A5BBFB4F5BB0 /* Adapters+Production.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = "Adapters+Production.swift"; sourceTree = ""; }; 1CC83EBCF4E254E0E8077589 /* DownloadTaskPersistence.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = DownloadTaskPersistence.swift; sourceTree = ""; }; 1D4B77CEDF228DC03714AD31 /* LocalFileAdapter.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = LocalFileAdapter.swift; sourceTree = ""; }; + 1D5D921DA42F7B66434E0D5F /* AccountRegistryStoreSeamTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AccountRegistryStoreSeamTests.swift; sourceTree = ""; }; 1DA098468E3C460A466D22BD /* TPPPDFReaderSearchBindingTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TPPPDFReaderSearchBindingTests.swift; sourceTree = ""; }; 1DC9FC06EF6CCEED742C0D44 /* TPPSignInBusinessLogicExtendedTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = TPPSignInBusinessLogicExtendedTests.swift; sourceTree = ""; }; 1DE545556CC9FC4FC7DFF4B8 /* DeveloperSettingsView.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = DeveloperSettingsView.swift; sourceTree = ""; }; @@ -3064,6 +3068,7 @@ C3D4E5F60718394051627384 /* MockSAMLAuthContext.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = MockSAMLAuthContext.swift; sourceTree = ""; }; C41E92244AABB4F1FE944CF8 /* ReaderTypographyButton.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReaderTypographyButton.swift; sourceTree = ""; }; C49B562EC2240BFF6303698C /* AudiobookSkipIntervalSettingsTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AudiobookSkipIntervalSettingsTests.swift; sourceTree = ""; }; + C49DDACF93DCF459DC310653 /* AccountRegistryStore.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = AccountRegistryStore.swift; sourceTree = ""; }; C4B5C6D7E8F9A0B1C2D3E4F5 /* DownloadStartCoordinatorTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = DownloadStartCoordinatorTests.swift; sourceTree = ""; }; C4BD28685CFAA8815B0DE215 /* ExtensionCoverageTests.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = ExtensionCoverageTests.swift; sourceTree = ""; }; C54010275AF91385B8410DF1 /* BookOpenTracker.swift */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.swift; path = BookOpenTracker.swift; sourceTree = ""; }; @@ -4976,6 +4981,7 @@ 2C353AC0344431D385FC89E6 /* AccountSwitchDependencies.swift */, 5CDD5AA4C49CC5D388BA9F31 /* AccountNetworking.swift */, EF3B037BFF273D82D2D88CE5 /* AccountRegistryCache.swift */, + C49DDACF93DCF459DC310653 /* AccountRegistryStore.swift */, ); path = Library; sourceTree = ""; @@ -6067,6 +6073,7 @@ 41F50CFA2EBD6E2F19F568E0 /* TPPSignInCapabilitiesCharacterizationTests.swift */, 9963A35F959660D52B46E3EC /* AccountNetworkingSeamTests.swift */, 7A2745714D1B395FCB45D9C3 /* AccountRegistryCacheSeamTests.swift */, + 1D5D921DA42F7B66434E0D5F /* AccountRegistryStoreSeamTests.swift */, ); name = Decomp; path = Decomp; @@ -8068,6 +8075,7 @@ 2FEE2557210153D5C94C4D28 /* MyBooksDownloadCenterAccountScopeSeamTests.swift in Sources */, DE253F8C8C4CC3BA89F45090 /* AccountNetworkingSeamTests.swift in Sources */, 181FDBB5DF170830CFDA55D2 /* AccountRegistryCacheSeamTests.swift in Sources */, + 092C37A2AA45C900DCCD7A8E /* AccountRegistryStoreSeamTests.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -8637,6 +8645,7 @@ 4BD9BDFB996BF9E0576420CB /* AccountNetworking.swift in Sources */, A7D4A0D17DACC00FB34F5289 /* TPPNetworkExecutor+AccountNetworking.swift in Sources */, AEFABDFE2619088183740385 /* AccountRegistryCache.swift in Sources */, + 0AE214B06AC5C39759675519 /* AccountRegistryStore.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; @@ -9209,6 +9218,7 @@ A81B474445D23D25DD33F00F /* AccountNetworking.swift in Sources */, 31E755A021BE24D982712F6B /* TPPNetworkExecutor+AccountNetworking.swift in Sources */, D8AEB498F4BA7DC04FC5A30E /* AccountRegistryCache.swift in Sources */, + 4731E519F95F0E4A6F77BFBF /* AccountRegistryStore.swift in Sources */, ); runOnlyForDeploymentPostprocessing = 0; }; diff --git a/Palace/Accounts/Library/AccountRegistryStore.swift b/Palace/Accounts/Library/AccountRegistryStore.swift new file mode 100644 index 000000000..90439651d --- /dev/null +++ b/Palace/Accounts/Library/AccountRegistryStore.swift @@ -0,0 +1,219 @@ +// +// AccountRegistryStore.swift +// Palace +// +// god-class decomposition — Wave 3 / 3a-2 (the second in-target collaborator +// split out of `AccountsManager`). +// +// Owns the account-registry STATE and all thread-safe access to it: the current +// catalog hash, the `[hash → [Account]]` sets, the O(1) `uuid → Account` index kept +// in lockstep with them, and the separate slim launch-hydration fallback. Extracted +// behind an injected collaborator so the hub carries no registry-state concurrency +// machinery and a packaged `AccountsManager` names none of it — the +// `AccountRegistryCache` / `AccountStateStore` precedent. +// +// A `final class`, NOT an `actor`, and NOT a protocol: `AccountsManager.account(_:)` +// is a SYNCHRONOUS `@objc TPPLibraryAccountsProvider` requirement, and the +// background-work drain choreography (`cancelAndDrainBackgroundWork`) depends on a +// synchronous `accountSetsLock.sync` read blocking behind the barrier — an actor +// would force `await` through the `@objc` conformance and delete that timing. The +// store therefore OWNS the same concurrent `DispatchQueue` and preserves the exact +// sync-read / `.async(flags:.barrier)`-write model verbatim. It is pure in-target +// state with no outward app-target edge, so it is a concrete injected instance +// (like `AccountStateStore`) and travels into `PalaceAccounts` with the hub. +// +// `@unchecked Sendable` invariant: the only mutable state is `_currentHash`, +// `accountSets`, and `accountByUUID`, read exclusively via `accountSetsLock.sync` +// (`performRead`) and written exclusively on the serial `.barrier` of the concurrent +// `accountSetsLock` (`performWrite` / `mutate`); `accountByUUID` is rebuilt inside the +// SAME barrier as `accountSets` so the two never desync. `slimAccountsByUUID` is +// guarded by its own `slimAccountsLock` NSLock — deliberately separate so the +// `account(_:)` full-index read and the slim fallback never contend on one lock; the +// slim set is a launch fallback only and never flips `currentBucketIsLoaded`. All +// boxed closures are invoked exactly once on that serial barrier, never concurrently. +// +// Copyright © 2026 The Palace Project. All rights reserved. +// + +import Foundation + +/// Documented carrier for a non-Sendable `() -> Void` handed to the `accountSetsLock` +/// barrier in `performWrite`. `@unchecked Sendable` invariant: the wrapped closure is +/// invoked exactly once, on the serial barrier of the concurrent `accountSetsLock`, +/// never concurrently. +private struct VoidWorkBox: @unchecked Sendable { + let work: () -> Void +} + +/// Documented carrier for the non-Sendable `(inout [String: [Account]]) -> Void` +/// mutation closure handed to the `accountSetsLock` barrier in `mutate`. Same +/// `@unchecked Sendable` invariant as `VoidWorkBox`: invoked exactly once, on the +/// serial `.barrier`, never concurrently. Timing and the mutate-then-rebuild-index +/// contract are unchanged. +private struct AccountSetsMutationBox: @unchecked Sendable { + let mutate: (inout [String: [Account]]) -> Void +} + +/// Thread-safe holder of the account-registry state. See the file header for the +/// `@unchecked Sendable` invariant and the class-not-actor rationale. +final class AccountRegistryStore: @unchecked Sendable { + + /// The current catalog hash (`prod` / `beta` / custom-URL). Written on the + /// barrier, read under `accountSetsLock` — bundled into ONE critical section with + /// the bucket read by `accountsForCurrentHash` / `currentBucketIsLoaded` so a + /// concurrent library switch can never key the bucket to a stale hash. + private var _currentHash: String + + private var accountSets = [String: [Account]]() + + /// O(1) `uuid → Account` index derived from `accountSets`, kept in lockstep with + /// it under `accountSetsLock` (rebuilt in the SAME barrier as any `accountSets` + /// mutation — see `mutate`). Lets `account(_:)` resolve a UUID without a linear + /// scan over ~1142 accounts on the main-thread display path. MUST only change via + /// `mutate` so it can never desync from `accountSets`. + private var accountByUUID = [String: Account]() + + private let accountSetsLock = DispatchQueue(label: "com.tpp.accountSetsLock", attributes: .concurrent) + + /// Launch-hydration (CP-D1) slim lookup: the current + settings accounts (~2), + /// decoded synchronously at launch so `currentAccount` resolves within a few ms. + /// Deliberately SEPARATE from `accountSets`: the slim set backs `account(_:)`'s + /// FALLBACK only and MUST NOT flip `currentBucketIsLoaded` true (a truncated-picker + /// bug). Guarded by its own `NSLock` so the `account(_:)` `accountSetsLock` read and + /// this fallback never contend on the same lock. + private var slimAccountsByUUID = [String: Account]() + private let slimAccountsLock = NSLock() + + init(currentHash: String = "") { + self._currentHash = currentHash + } + + // MARK: - Concurrency primitives (private) + + private func performRead(_ block: () -> T) -> T { + return accountSetsLock.sync { + block() + } + } + + private func performWrite(_ block: @escaping () -> Void) { + let box = VoidWorkBox(work: block) + accountSetsLock.async(flags: .barrier) { + box.work() + } + } + + // MARK: - Current hash + + /// Single synchronized read of the current hash, for callers that need only the + /// hash (never paired atomically with a bucket read — those use the atomic + /// methods below). + var currentHash: String { + performRead { self._currentHash } + } + + func setCurrentHash(_ hash: String) { + performWrite { self._currentHash = hash } + } + + // MARK: - Reads + + func account(_ uuid: String) -> Account? { + if let full = performRead({ accountByUUID[uuid] }) { + return full + } + // CP-D1 launch-hydration fallback: before the full account list has + // materialized off-main, the slim snapshot backs current-account resolution. + // Full instances take precedence (checked first, above) once present. + return slimAccount(uuid) + } + + /// Accounts for an explicit hash. Used by callers that already hold the hash they + /// want (the hub `accounts(key)` facade when a key is passed, plus tests). + func accounts(forKey key: String) -> [Account] { + performRead { self.accountSets[key] ?? [] } + } + + /// Accounts for the CURRENT hash, read ATOMICALLY: the hash and the bucket are + /// sampled in ONE `performRead`, so a library-switch barrier cannot land between + /// them and key the bucket to a stale hash. Do NOT reimplement as + /// `accounts(forKey: currentHash)` — that is two separate lock acquisitions. + func accountsForCurrentHash() -> [Account] { + performRead { self.accountSets[self._currentHash] ?? [] } + } + + /// Whether the CURRENT hash's bucket is non-empty, read ATOMICALLY (same + /// single-critical-section snapshot as `accountsForCurrentHash`). Reflects the + /// FULL account list only — the slim fallback never flips this true. + func currentBucketIsLoaded() -> Bool { + performRead { !(self.accountSets[self._currentHash]?.isEmpty ?? true) } + } + + /// Whether an explicit hash's bucket is non-empty. + func bucketIsNonEmpty(hash: String) -> Bool { + performRead { self.accountSets[hash]?.isEmpty == false } + } + + // MARK: - Writes + + /// The ONLY sanctioned way to mutate `accountSets`. Applies `mutate` inside the + /// `accountSetsLock` barrier, then rebuilds `accountByUUID` in the SAME critical + /// section so the two can never desync. Adding a write that bypasses this silently + /// breaks `account(_:)` lookups (guarded by the index-coherence tests). + func mutate(_ mutate: @escaping (inout [String: [Account]]) -> Void) { + let box = AccountSetsMutationBox(mutate: mutate) + accountSetsLock.async(flags: .barrier) { + box.mutate(&self.accountSets) + self.accountByUUID = AccountRegistryStore.buildAccountIndex(self.accountSets) + } + } + + // MARK: - Slim launch-hydration fallback + + func storeSlim(_ accounts: [Account]) { + slimAccountsLock.lock() + defer { slimAccountsLock.unlock() } + for account in accounts { + slimAccountsByUUID[account.uuid] = account + } + } + + func slimAccount(_ uuid: String) -> Account? { + slimAccountsLock.lock() + defer { slimAccountsLock.unlock() } + return slimAccountsByUUID[uuid] + } + + // MARK: - Pure + + /// Pure: flatten `accountSets` into a `uuid → Account` index. When a UUID appears + /// in more than one bucket, the last-enumerated wins — equivalent to the + /// nondeterministic "first across `values`" the prior linear scan returned. + static func buildAccountIndex(_ sets: [String: [Account]]) -> [String: Account] { + var index = [String: Account]() + for accounts in sets.values { + for account in accounts { + index[account.uuid] = account + } + } + return index + } + + #if DEBUG + /// Test-only: reads `accountByUUID` and a freshly-rebuilt index in ONE + /// `performRead` and returns whether they are identical (by key set + object + /// identity). A mutant that rebuilds the index outside the mutation barrier opens + /// a window where a sync read observes updated `accountSets` but a stale + /// `accountByUUID`; hammering this under concurrent `mutate` catches it. + func _coherentSnapshot() -> Bool { + performRead { + let rebuilt = AccountRegistryStore.buildAccountIndex(self.accountSets) + guard rebuilt.count == self.accountByUUID.count else { return false } + for (uuid, account) in rebuilt { + guard let indexed = self.accountByUUID[uuid], indexed === account else { return false } + } + return true + } + } + #endif +} diff --git a/Palace/Accounts/Library/AccountsManager.swift b/Palace/Accounts/Library/AccountsManager.swift index 8ff1f167f..d966f49f7 100644 --- a/Palace/Accounts/Library/AccountsManager.swift +++ b/Palace/Accounts/Library/AccountsManager.swift @@ -43,25 +43,6 @@ private final class AccountsManagerBoolFlag: @unchecked Sendable { } } -/// Documented carrier for a non-Sendable `() -> Void` handed to a -/// `DispatchQueue` barrier block in `AccountsManager.performWrite`. -/// `@unchecked Sendable` invariant: the wrapped closure is invoked exactly -/// once, on the serial barrier of the concurrent `accountSetsLock`, never -/// concurrently. -private struct VoidWorkBox: @unchecked Sendable { - let work: () -> Void -} - -/// Documented carrier for the non-Sendable `(inout [String: [Account]]) -> Void` -/// mutation closure handed to the `accountSetsLock` barrier in -/// `AccountsManager.mutateAccountSets`. Same `@unchecked Sendable` invariant as -/// `VoidWorkBox`: the wrapped closure is invoked exactly once, on the serial -/// `.barrier` of the concurrent `accountSetsLock`, never concurrently. Timing -/// and the mutate-then-rebuild-index contract are unchanged. -private struct AccountSetsMutationBox: @unchecked Sendable { - let mutate: (inout [String: [Account]]) -> Void -} - /// Documented carrier for a non-Sendable `(Bool) -> Void` load-completion /// handler stored into the `loadingHandlersQueue` barrier in /// `AccountsManager.addLoadingHandler`. `@unchecked Sendable` invariant: the @@ -105,10 +86,10 @@ private struct CrawlerHandoffBox: @unchecked Sendable { /// `TPPUserAccount` and `AccountStateStore`'s own `@unchecked` justification): /// /// Instance mutable state: -/// - `accountSet`, `accountSets`, `accountByUUID` → read via `performRead` -/// (`accountSetsLock.sync`), written via `performWrite` / `mutateAccountSets` -/// (`accountSetsLock.async(flags:.barrier)`). `accountByUUID` is only ever -/// rebuilt inside the same barrier as `accountSets`, so the two never desync. +/// - the account-registry state (current hash, `accountSets`, the `accountByUUID` +/// index, and the slim fallback) → moved to the injected `AccountRegistryStore` +/// (Wave 3 / 3a-2), which owns their concurrent `accountSetsLock` sync-read / +/// barrier-write model; the hub holds the store as an immutable `Sendable` `let`. /// - `loadingCompletionHandlers` → read via `loadingHandlersQueue.sync`, /// written via `loadingHandlersQueue.async(flags:.barrier)`. /// - `inflightAuthDocFetches` → read/written only under `inflightAuthDocLock`. @@ -133,9 +114,9 @@ private struct CrawlerHandoffBox: @unchecked Sendable { /// `cancelBackgroundWork()` / `_injectBackgroundFetchTaskForTesting` seams. /// /// Immutable (`let`) state — inherently safe: `tppAccountUUID`, `ageCheck`, -/// `settings`, `defaults`, `accountSetsLock`, `catalogPreloader`, -/// `loadingHandlersQueue`, `inflightAuthDocLock`, `userAccountsLock`, -/// `_trackedCrawlTasksLock`. +/// `settings`, `defaults`, `registryStore` (internally synchronized), +/// `registryCache`, `catalogPreloader`, `loadingHandlersQueue`, +/// `inflightAuthDocLock`, `userAccountsLock`, `_trackedCrawlTasksLock`. /// /// `currentAccountId` is a computed property backed by the injected /// `UserDefaults` (`defaults`), which is itself internally thread-safe — no @@ -217,37 +198,15 @@ private struct CrawlerHandoffBox: @unchecked Sendable { /// choreography cancels + awaits, so no un-drainable write channel survives a /// test boundary. Self-pruning + internally synchronized. private let ownedCrawlTasks = OwnedCrawlTaskRegistry() - private var accountSet: String - private var accountSets = [String: [Account]]() - /// O(1) `uuid → Account` index derived from `accountSets`, kept in lockstep - /// with it under `accountSetsLock`. Lets `account(_:)` resolve a UUID without - /// a linear scan over every bucket — the registry snapshot holds ~1142 - /// accounts, and `account(_:)` is on the main-thread display path for every - /// account-change-driven view refresh, so the old `accountSets.values.first { - /// $0.contains(where:) }` scan saturated the main thread (the test-suite hang - /// class, and a live-app cost on every library switch). MUST only be mutated - /// via `mutateAccountSets` so it can never desync from `accountSets`. - private var accountByUUID = [String: Account]() - private let accountSetsLock = DispatchQueue(label: "com.tpp.accountSetsLock", attributes: .concurrent) - - /// Launch-hydration (CP-D1) slim lookup: the current + `settingsAccountIdsList` - /// accounts (~2 accounts, a few KB) decoded SYNCHRONOUSLY at launch from the - /// small `accounts_catalog_slim_.json` snapshot, so `currentAccount` - /// resolves — and its `awaitReady()` gate is driven — within a few ms of - /// launch instead of paying the full ~207ms (fast sim) / ~0.3-0.6s (device) - /// 1142-account decode+map on the launch main thread. - /// - /// Deliberately kept SEPARATE from `accountSets`: `accountsHaveLoaded` and - /// `accounts()` MUST keep reflecting the FULL 1142-account list (the library - /// picker — `TPPAccountList` / `TPPAppDelegate.presentFirstRunFlowIfNeeded` - /// — reads them), so the ~2-account slim set must NOT flip `accountsHaveLoaded` - /// true (a truncated-picker bug). This structure only backs `account(_:)`'s - /// FALLBACK; full instances (in `accountByUUID`) always win once the full - /// list materializes off-main via the background `loadCatalogs`. Guarded by - /// its own `NSLock` so `account(_:)`'s `accountSetsLock` read and this - /// fallback never contend on the same lock. - private var slimAccountsByUUID = [String: Account]() - private let slimAccountsLock = NSLock() + /// Account-registry state + all thread-safe access (Wave 3 / 3a-2): the current + /// catalog hash, the `[hash → [Account]]` sets, the O(1) `uuid → Account` index + /// rebuilt in-barrier-lockstep with them, and the separate slim launch-hydration + /// fallback. Injected `let` — the concrete `AccountRegistryStore` by default, a + /// recording double under test. All registry-state concurrency (the concurrent + /// `accountSetsLock` sync-read / barrier-write model) lives in the store now, so + /// the hub carries none of it. See `AccountRegistryStore.swift` for the + /// `@unchecked Sendable` invariant and the class-not-actor rationale. + private let registryStore: AccountRegistryStore private let catalogPreloader = CatalogPreloader() @@ -513,20 +472,28 @@ private struct CrawlerHandoffBox: @unchecked Sendable { borrowReauthResetter: any BorrowReauthResetting = DownloadCenterBorrowReauthResetter(), crawlScheduler: CrawlTaskScheduler = .production, switchDependencies: AccountSwitchDependencies = .production, - registryCache: any AccountRegistryCaching = DiskAccountRegistryCache() + registryCache: any AccountRegistryCaching = DiskAccountRegistryCache(), + registryStore: AccountRegistryStore = AccountRegistryStore() ) { self.defaults = defaults self.borrowReauthResetter = borrowReauthResetter self.crawlScheduler = crawlScheduler self.switchDeps = switchDependencies self.registryCache = registryCache + self.registryStore = registryStore self.settings = TPPSettings() - self.accountSet = TPPConfiguration.customUrlHash() - ?? (settings.useBetaLibraries - ? TPPConfiguration.betaUrlHash - : TPPConfiguration.prodUrlHash) self.ageCheck = TPPAgeCheck(ageCheckChoiceStorage: settings) super.init() + // Seed the registry store's current hash (was the hub's `accountSet` stored + // property, now owned by the store — Wave 3 / 3a-2). Written on the store's + // barrier; the synchronous `preloadAccountsFromDiskCacheSync` read below + // observes it via GCD barrier FIFO ordering (the one deliberate init delta). + registryStore.setCurrentHash( + TPPConfiguration.customUrlHash() + ?? (settings.useBetaLibraries + ? TPPConfiguration.betaUrlHash + : TPPConfiguration.prodUrlHash) + ) NotificationCenter.default.addObserver( self, selector: #selector(updateAccountSetFromNotification(_:)), @@ -599,7 +566,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { /// Exposed `internal` so contract-snapshot tests can drive the preload /// path directly after seeding the on-disk cache. internal func preloadAccountsFromDiskCacheSync() { - let hash = self.accountSet + let hash = registryStore.currentHash // Fast path (CP-D1 LaunchHydration): when a slim snapshot exists, decode // ONLY the current + settings accounts (a few KB) synchronously so // `currentAccount` resolves and its auth-doc drive fires within a few ms @@ -665,7 +632,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { !accounts.contains(where: { $0.uuid == currentId }) { return false } - storeSlimAccounts(accounts) + registryStore.storeSlim(accounts) for account in accounts { if case .notLoaded = switchDeps.accountStateStore.state(for: account.uuid) { account._setState(.basicInfoLoaded) @@ -712,7 +679,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { let accounts = feed.catalogs.map { Account(publication: $0, imageCache: switchDeps.imageCache) } - mutateAccountSets { $0[hash] = accounts } + registryStore.mutate { $0[hash] = accounts } // Account state-machine wiring (3.2.0): Phase 1 — drive every // preloaded account into `.basicInfoLoaded`. Display-only // consumers (Settings/Libraries) can render the row immediately; @@ -731,22 +698,6 @@ private struct CrawlerHandoffBox: @unchecked Sendable { } } - /// Thread-safe write of the slim launch-hydration accounts. CP-D1. - private func storeSlimAccounts(_ accounts: [Account]) { - slimAccountsLock.lock() - defer { slimAccountsLock.unlock() } - for account in accounts { - slimAccountsByUUID[account.uuid] = account - } - } - - /// Thread-safe read of a slim launch-hydration account. CP-D1. - private func slimAccount(_ uuid: String) -> Account? { - slimAccountsLock.lock() - defer { slimAccountsLock.unlock() } - return slimAccountsByUUID[uuid] - } - /// Rebuild the slim launch snapshot from the authoritative full on-disk /// catalog cache, OFF the main thread. Best-effort: keeps the slim file in /// sync with the latest current + settings selection so the NEXT cold @@ -829,55 +780,14 @@ private struct CrawlerHandoffBox: @unchecked Sendable { return try? JSONSerialization.data(withJSONObject: slimRoot) } - // MARK: – Thread‐safe accountSets access + // MARK: – Account index (static shim) - private func performRead(_ block: () -> T) -> T { - return accountSetsLock.sync { - block() - } - } - - private func performWrite(_ block: @escaping () -> Void) { - // Carry the non-Sendable `block` into the barrier block via a - // documented box. `@unchecked Sendable` invariant: `block` is - // invoked exactly once, on the serial `.barrier` of the concurrent - // `accountSetsLock`, never concurrently — the same execution - // contract this method already guaranteed. Timing is unchanged. - let box = VoidWorkBox(work: block) - accountSetsLock.async(flags: .barrier) { - box.work() - } - } - - /// The ONLY sanctioned way to mutate `accountSets`. Applies `mutate` to the - /// dictionary inside the `accountSetsLock` barrier, then rebuilds the - /// `accountByUUID` index in the same critical section so the two can never - /// desync. Every write site (preload hydrate, network load, the test seams) - /// goes through here — adding a new write that bypasses it would silently - /// break `account(_:)` lookups, which the index-coherence test guards against. - private func mutateAccountSets(_ mutate: @escaping (inout [String: [Account]]) -> Void) { - // Carry the non-Sendable `mutate` into the `@Sendable` barrier block via - // a documented box (mirrors `performWrite`'s `VoidWorkBox`). Same serial- - // barrier execution contract; timing unchanged. - let box = AccountSetsMutationBox(mutate: mutate) - accountSetsLock.async(flags: .barrier) { - box.mutate(&self.accountSets) - self.accountByUUID = AccountsManager.buildAccountIndex(self.accountSets) - } - } - - /// Pure: flatten `accountSets` into a `uuid → Account` index. When a UUID - /// appears in more than one bucket (e.g. an account present in both the - /// prod and beta registries), the last-enumerated wins — equivalent to the - /// nondeterministic "first across `values`" the prior linear scan returned. + /// Pure `uuid → Account` index builder. The implementation and ALL thread-safe + /// `accountSets` access moved to `AccountRegistryStore` (Wave 3 / 3a-2); this thin + /// static forwards so `AccountsManager.buildAccountIndex(...)` stays a stable name + /// for `AccountsManagerAccountIndexTests`. static func buildAccountIndex(_ sets: [String: [Account]]) -> [String: Account] { - var index = [String: Account]() - for accounts in sets.values { - for account in accounts { - index[account.uuid] = account - } - } - return index + AccountRegistryStore.buildAccountIndex(sets) } // MARK: - Account Retrieval @@ -971,7 +881,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { // Off-main + best-effort + XCTest-gated (see the method); no launch // main-thread cost. The `hydrateSlimLaunchSnapshot` current-account // presence check is the belt-and-suspenders if this ever lags. - refreshSlimLaunchSnapshotOffMain(hash: self.accountSet) + refreshSlimLaunchSnapshotOffMain(hash: registryStore.currentHash) NotificationCenter.default.post(name: .TPPCurrentAccountDidChange, object: nil) } } @@ -1002,22 +912,17 @@ private struct CrawlerHandoffBox: @unchecked Sendable { } func account(_ uuid: String) -> Account? { - if let full = performRead({ accountByUUID[uuid] }) { - return full - } - // CP-D1 launch-hydration fallback: before the full 1142-account list has - // materialized off-main, the slim snapshot backs current-account - // resolution so `currentAccount` (and its auth-doc drive) work in the - // pre-materialization window. Full instances take precedence (checked - // first, above) once present. - return slimAccount(uuid) + return registryStore.account(uuid) } func accounts(_ key: String? = nil) -> [Account] { - return performRead { - let k = key ?? self.accountSet - return self.accountSets[k] ?? [] + // Atomic on the nil path: the store reads currentHash + its bucket in ONE + // critical section, so a concurrent library switch can't key the bucket to a + // stale hash. Do NOT collapse this to `accounts(forKey: currentHash)`. + if let key { + return registryStore.accounts(forKey: key) } + return registryStore.accountsForCurrentHash() } #if DEBUG @@ -1039,8 +944,8 @@ private struct CrawlerHandoffBox: @unchecked Sendable { /// NOT exposed in production builds. @discardableResult func _seedAccountForTesting(_ account: Account) -> () -> Void { - let seedKey = self.accountSet - mutateAccountSets { + let seedKey = registryStore.currentHash + registryStore.mutate { var seeded = $0[seedKey] ?? [] seeded.removeAll { $0.uuid == account.uuid } seeded.append(account) @@ -1049,7 +954,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { let previousId = defaults.string(forKey: currentAccountIdentifierKey) defaults.set(account.uuid, forKey: currentAccountIdentifierKey) return { - self.mutateAccountSets { + self.registryStore.mutate { $0[seedKey]?.removeAll { $0.uuid == account.uuid } } if let prev = previousId { @@ -1062,9 +967,8 @@ private struct CrawlerHandoffBox: @unchecked Sendable { #endif var accountsHaveLoaded: Bool { - return performRead { - !(self.accountSets[self.accountSet]?.isEmpty ?? true) - } + // Atomic: the store samples currentHash + its bucket in ONE critical section. + return registryStore.currentBucketIsLoaded() } // MARK: - Per-Account User Credentials @@ -1186,7 +1090,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { .trimmingCharacters(in: ["="]) // 1. If already loaded in memory, return immediately - if performRead({ self.accountSets[hash]?.isEmpty == false }) { + if registryStore.bucketIsNonEmpty(hash: hash) { // State-machine wiring (3.2.0): the cold path drives the // current account's LoadState past `.basicInfoLoaded` via // `loadAccountSetsAndAuthDoc → fetchAuthDocumentWithStateMachine`. @@ -1769,7 +1673,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { // `oldAccountsByUUID` carries the auth-doc forward via the loop // below and fresh network data must win, so we do NOT reuse then. if oldAccountsByUUID[publication.metadata.id] == nil, - let slim = slimAccount(publication.metadata.id) { + let slim = registryStore.slimAccount(publication.metadata.id) { return slim } return Account(publication: publication, imageCache: switchDeps.imageCache) @@ -1791,7 +1695,7 @@ private struct CrawlerHandoffBox: @unchecked Sendable { } } - self.mutateAccountSets { $0[hash] = newAccounts } + self.registryStore.mutate { $0[hash] = newAccounts } // Account state-machine wiring (3.2.0): Phase 1 — drive every // freshly-constructed account into its post-load terminal state. @@ -1878,9 +1782,10 @@ private struct CrawlerHandoffBox: @unchecked Sendable { @objc private func updateAccountSetFromNotification(_ notif: Notification) { // Run off the poster's thread. `.TPPUseBetaDidChange` is delivered // synchronously by `NotificationCenter` on whatever thread posted it — - // typically MAIN, from a Settings toggle. `updateAccountSet` does an - // `accountSetsLock.sync` read (`performRead`) that blocks until any - // in-flight background catalog-refresh barrier on that lock drains; under + // typically MAIN, from a Settings toggle. `updateAccountSet` does a + // synchronous read on the registry store's concurrent `accountSetsLock` + // (via `registryStore.bucketIsNonEmpty`) that blocks until any in-flight + // background catalog-refresh barrier on that lock drains; under // load that barrier can take a long time, so reacting synchronously stalls // the poster (the Settings UI — and any test that posts this notification, // which is how it surfaced as a 120s hang). The account-set update is @@ -1896,8 +1801,11 @@ private struct CrawlerHandoffBox: @unchecked Sendable { ? TPPConfiguration.betaUrlHash : TPPConfiguration.prodUrlHash) - performWrite { self.accountSet = newHash } - if performRead({ self.accountSets[newHash]?.isEmpty ?? true }) || TPPConfiguration.customUrlHash() != nil { + registryStore.setCurrentHash(newHash) + // Original was `accountSets[newHash]?.isEmpty ?? true` (true when empty/missing); + // bucketIsNonEmpty is its inverse, so this MUST be negated to preserve the + // load-trigger polarity. + if !registryStore.bucketIsNonEmpty(hash: newHash) || TPPConfiguration.customUrlHash() != nil { loadCatalogs(completion: completion) } else { completion?(true) @@ -1958,12 +1866,13 @@ extension AccountsManager { } /// Test-only seam: populate an accountSets bucket without going through - /// OPDS2 parsing. Routes through `performWrite` so concurrent-access - /// invariants are preserved. Used by mutation-killing tests for + /// OPDS2 parsing. Routes through `registryStore.mutate` (the store's barrier) + /// so concurrent-access invariants — including the in-barrier `accountByUUID` + /// rebuild — are preserved. Used by mutation-killing tests for /// `account(_ uuid:)` — multi-bucket scenarios are not otherwise /// reachable from outside the class. func _testSetAccountSet(_ accounts: [Account], forKey key: String) { - mutateAccountSets { $0[key] = accounts } + registryStore.mutate { $0[key] = accounts } } /// Test-only: cancel the in-flight background `loadCatalogs` Task (if any) @@ -2021,9 +1930,9 @@ extension AccountsManager { /// Why this exists (WS-0 follow-up — closes the residual race documented on /// `cancelBackgroundWork()` above): the cooperative `cancelBackgroundWork()` /// returns immediately, so a just-cancelled crawl can still be mid-flight — - /// holding the `performWrite` `.barrier` on `accountSetsLock` — when the - /// NEXT test's `@MainActor` reauth path does a synchronous - /// `currentUserAccount` / `performRead` `.sync` read. That read blocks + /// holding the `.barrier` on the registry store's `accountSetsLock` (via + /// `registryStore.mutate`) — when the NEXT test's `@MainActor` reauth path does a + /// synchronous `currentUserAccount` / store `.sync` read. That read blocks /// behind the barrier; because the crawl hops to the main actor to complete, /// and main is now blocked on the read, the two deadlock → the victim test's /// 5s `waitForExpectations` timer never fires → 120s main-thread jam. This diff --git a/PalaceTests/Decomp/AccountRegistryStoreSeamTests.swift b/PalaceTests/Decomp/AccountRegistryStoreSeamTests.swift new file mode 100644 index 000000000..04a5f1695 --- /dev/null +++ b/PalaceTests/Decomp/AccountRegistryStoreSeamTests.swift @@ -0,0 +1,212 @@ +// +// AccountRegistryStoreSeamTests.swift +// PalaceTests +// +// Pins the Wave 3 / 3a-2 `AccountRegistryStore` seam: the account-registry state and +// its concurrency now live in an injected store, and `AccountsManager`'s retrieval +// facades delegate to it. +// +// Two lenses: +// 1. Concurrency (real store) — the index-coherence-under-barrier invariant, no +// torn reads, and slim-fallback isolation. These are the guarantees the +// extraction MUST preserve; each kills a specific locking-model mutant. +// 2. Routing (hub delegation) — inject a store, drive state through it, assert the +// hub `account(_:)` / `accounts()` / `accountsHaveLoaded` facades reflect it. +// +// Copyright © 2026 The Palace Project. All rights reserved. +// + +import XCTest +import PalaceCatalog +import PalaceBookModel +@testable import Palace + +@MainActor +final class AccountRegistryStoreSeamTests: PalaceWiringTestCase { + + // MARK: - Concurrency (real store) + + /// Contract: `accountByUUID` is rebuilt inside the SAME barrier as `accountSets`, + /// so a concurrent reader never observes a stale index. + /// + /// Kill case: rebuild the index in a SEPARATE barrier (or async) after the mutate + /// → a sampler lands in the window with updated sets + stale index → `_coherentSnapshot` + /// returns false. + func testMutate_indexStaysCoherentUnderConcurrentChurn() { + let store = AccountRegistryStore(currentHash: "h") + // Pre-build accounts on the MAIN thread. `Account.init` touches UIKit + // (`UIImage(named:)`) + the image cache; creating them inside the concurrent + // barrier while the test blocks the main thread risks a main-affine hang. The + // concurrent section below only inserts pre-made (`Sendable`) accounts. + let pool = (0..<8).map { Self.makeAccount("uuid-\($0)") } + let incoherent = CoherenceBox(), lock = NSLock() + + // Bounded fan-out via `concurrentPerform` (self-joining, thread-pool-bounded) + // rather than an unbounded `global().async` + `group.wait()` storm — the latter + // starves under CI parallel-clone load (see deflake-parallel-clone-starvation). + DispatchQueue.concurrentPerform(iterations: 300) { i in + if i % 2 == 0 { + store.mutate { $0["h"] = [pool[i % pool.count]] } + } else if !store._coherentSnapshot() { + lock.lock(); incoherent.flag = true; lock.unlock() + } + } + // Sync read: FIFO-ordered after every enqueued barrier, so it also drains them. + XCTAssertTrue(store._coherentSnapshot(), "index must be coherent once churn settles") + XCTAssertFalse(incoherent.flag, "accountByUUID must never desync from accountSets under concurrent mutate") + } + + /// Contract: `account(_:)` under concurrent reseeds never returns a phantom (an + /// Account whose uuid differs from the one requested) and never crashes. + /// + /// Kill case: read `accountByUUID` outside `performRead` (unsynchronized) → torn + /// read / phantom / crash under churn. + func testAccount_concurrentMutate_neverReturnsPhantom() { + let store = AccountRegistryStore(currentHash: "h") + let uuids = (0..<8).map { "uuid-\($0)" } + let pool = uuids.map { Self.makeAccount($0) } // pre-create on main (see churn test) + let mismatch = CoherenceBox(), lock = NSLock() + + DispatchQueue.concurrentPerform(iterations: 300) { i in + if i % 2 == 0 { + store.mutate { $0["h"] = pool.shuffledStable(seed: i) } + } else { + let want = uuids[i % uuids.count] + if let got = store.account(want), got.uuid != want { + lock.lock(); mismatch.flag = true; lock.unlock() + } + } + } + _ = store._coherentSnapshot() // drain pending barriers before teardown + XCTAssertFalse(mismatch.flag, "account(uuid) must never return an Account with a different uuid") + } + + /// Contract: slim writes back `account(_:)`'s fallback but MUST NOT flip + /// `currentBucketIsLoaded` (which reflects the FULL list only — a truncated-picker + /// guard); a full `mutate` DOES flip it. + /// + /// Kill case: let slim writes touch `accountSets`/`accountByUUID` → the ~2-account + /// slim set reports the picker as loaded. + func testSlim_backsFallbackButDoesNotFlipCurrentBucketIsLoaded() { + let store = AccountRegistryStore(currentHash: "h") + + store.storeSlim([Self.makeAccount("slim-1")]) + XCTAssertNotNil(store.account("slim-1"), "slim account resolves as the pre-materialization fallback") + XCTAssertFalse(store.currentBucketIsLoaded(), "slim writes must NOT flip currentBucketIsLoaded true") + XCTAssertTrue(store.accounts(forKey: "h").isEmpty, "slim writes must NOT populate the full bucket") + + store.mutate { $0["h"] = [Self.makeAccount("full-1")] } + XCTAssertTrue(store.currentBucketIsLoaded(), "a full mutate flips currentBucketIsLoaded true") + } + + /// Contract (DETERMINISTIC): `accountsForCurrentHash` / `currentBucketIsLoaded` + /// reflect the CURRENT hash's bucket — following `setCurrentHash`, the reads track + /// the new hash (barrier FIFO makes the sync read observe the barrier-write seed). + /// + /// Kill case: a mutant that reads a hardcoded/other hash instead of `_currentHash`, + /// or that never re-reads the hash after a switch → the assertions below (which + /// flip between a loaded and an empty bucket as the current hash moves) fail. + /// + /// NOTE on the Finding-1 two-lock split: `accountsForCurrentHash` reads `_currentHash` + /// and its bucket in ONE `performRead`, so the hash+bucket pairing is atomic BY + /// CONSTRUCTION. A split (`accounts(forKey: currentHash)`) tears only under a + /// specific concurrent interleaving and — because a split is still internally + /// self-consistent (it returns the bucket for the hash it sampled) — is not + /// deterministically observable from outside. That protection is therefore + /// structural (single critical section) + code-reviewed, not asserted here; this + /// test pins the current-hash CORRECTNESS the atomic method must have. + func testCurrentHashReads_reflectTheCurrentHashsBucket() { + let store = AccountRegistryStore(currentHash: "A") + store.mutate { $0["A"] = [Self.makeAccount("a-1")]; $0["B"] = [] } // A loaded, B empty + + store.setCurrentHash("A") + XCTAssertEqual(store.accountsForCurrentHash().map(\.uuid), ["a-1"], + "current is A → A's bucket") + XCTAssertTrue(store.currentBucketIsLoaded(), "current is A (loaded) → loaded") + + store.setCurrentHash("B") + XCTAssertTrue(store.accountsForCurrentHash().isEmpty, + "current is B (empty) → empty bucket, NOT A's stale contents") + XCTAssertFalse(store.currentBucketIsLoaded(), + "current is B (empty) → not loaded, NOT A's stale readiness") + } + + /// Robustness: under concurrent switching + reads, `accountsForCurrentHash` never + /// crashes and never returns a TORN bucket (a mix of two libraries' accounts). Both + /// buckets are seeded distinctly, so a torn read would surface a foreign/mixed uuid + /// set. Kills an unsynchronized bucket read (outside `performRead`). + func testCurrentHashReads_neverTearUnderConcurrentSwitch() { + let store = AccountRegistryStore(currentHash: "A") + let a = Self.makeAccount("a-1"), b = Self.makeAccount("b-1") // pre-create on main + store.mutate { $0["A"] = [a]; $0["B"] = [b] } + _ = store.currentBucketIsLoaded() // barrier fence — buckets populated before churn + + let torn = CoherenceBox(), lock = NSLock() + DispatchQueue.concurrentPerform(iterations: 300) { i in + if i % 2 == 0 { + store.setCurrentHash(i % 4 == 0 ? "A" : "B") + } else { + let uuids = Set(store.accountsForCurrentHash().map(\.uuid)) + if !(uuids == ["a-1"] || uuids == ["b-1"] || uuids.isEmpty) { + lock.lock(); torn.flag = true; lock.unlock() + } + } + } + _ = store.currentBucketIsLoaded() // drain pending barriers before teardown + XCTAssertFalse(torn.flag, "accountsForCurrentHash must return exactly one library's bucket, never a torn mix") + } + + // MARK: - Routing (hub delegation) + + /// Contract: the hub's `account(_:)`, `accounts()`, and `accountsHaveLoaded` + /// facades delegate to the injected store — data placed in the store post-construction + /// is observable through the manager. + /// + /// Kill case: a facade that reads hub-local state instead of the store → the + /// store-injected data is invisible. + func testManagerFacades_delegateToInjectedStore() { + let store = AccountRegistryStore() + let manager = makeFreshAccountsManager(defaults: Self.testUserDefaults(), registryStore: store) + + // Drive state through the store AFTER construction (init seeds its own hash). + store.setCurrentHash("routing-h") + store.mutate { $0["routing-h"] = [Self.makeAccount("routed-1")] } + + XCTAssertEqual(manager.account("routed-1")?.uuid, "routed-1", + "manager.account must delegate to the injected store") + XCTAssertEqual(manager.accounts().map(\.uuid), ["routed-1"], + "manager.accounts() must delegate to the injected store's current bucket") + XCTAssertTrue(manager.accountsHaveLoaded, + "manager.accountsHaveLoaded must delegate to the injected store") + } + + // MARK: - Helpers + + /// Link-less account whose `metadata.id` is the uuid; no network on any drive. + nonisolated static func makeAccount(_ uuid: String) -> Account { + let metadata = OPDS2Publication.Metadata( + updated: Date(), + description: "registry-store seam", + id: uuid, + title: "Store \(uuid)" + ) + return Account(publication: OPDS2Publication(links: [], metadata: metadata, images: nil), + imageCache: MockImageCache()) + } +} + +/// `@unchecked Sendable` flag holder for cross-thread test assertions (guarded by the +/// test's own `NSLock`). +fileprivate final class CoherenceBox: @unchecked Sendable { + var flag = false +} + +private extension Array { + /// Deterministic index-driven rotation (no `Math.random`, which is unavailable in + /// some harness contexts) so each round reseeds a different order. + func shuffledStable(seed: Int) -> [Element] { + guard !isEmpty else { return self } + let k = seed % count + return Array(self[k...] + self[.. Void = { _ in } + ) -> AccountsManager { + #if DEBUG + AccountsManager.deferInitialLoadCatalogsForTesting = true + #endif + let manager = AccountsManager(defaults: defaults, registryStore: registryStore) + configure(manager) + managersToCancelOnTearDown.append(manager) + return manager + } + // MARK: - Disk-cache cleanup /// Remove every on-disk catalog/auth/crawl cache file in the test diff --git a/scripts/godclass-loc-baseline.txt b/scripts/godclass-loc-baseline.txt index ff5d5a170..82a2e6d69 100644 --- a/scripts/godclass-loc-baseline.txt +++ b/scripts/godclass-loc-baseline.txt @@ -60,6 +60,15 @@ # seams were inverted behind the injected `AccountSwitchDependencies` bundle; the # net is slightly NEGATIVE (the extracted nav-pop closure + deleted inline reaches # outweigh the stored dependency + its docs). Re-baselined by the exact delta. +# Wave 3 / 3a-2 (-91 on AccountsManager, 2173 -> 2082): the account-registry STATE + +# thread-safe access (the current-hash, `accountSets`, the lockstep `accountByUUID` +# index, the slim fallback, the concurrent `accountSetsLock` sync-read/barrier-write +# primitives, and the two barrier carriers) EXTRACTED to the injected +# `AccountRegistryStore` (a `final class`, not an actor — `account(_:)` is a +# synchronous `@objc` requirement and the drain path depends on sync `.sync` reads). +# The hub keeps thin forwarding facades (`account`/`accounts`/`accountsHaveLoaded`/a +# `buildAccountIndex` static shim). Second of the 3a collaborator splits; strongly +# NEGATIVE. Re-baselined DOWN. # Wave 3 / 3a-1 (-210 on AccountsManager, 2383 -> 2173): the on-disk catalog cache # concern (the `CatalogCacheMetadata` value type + the FileManager read/write/ # staleness/clear bodies) was EXTRACTED to the injected `AccountRegistryCaching` @@ -80,7 +89,7 @@ # PalaceDownloads and the account-scope wiring goes with it. Re-baselined by the exact # delta; the freeze still catches any further unrelated growth. 3093 Palace/Audiobooks/AudiobookSessionManager.swift -2173 Palace/Accounts/Library/AccountsManager.swift +2082 Palace/Accounts/Library/AccountsManager.swift 2184 Palace/MyBooks/MyBooksDownloadCenter.swift 1378 Palace/Book/UI/BookDetail/BookDetailViewModel.swift 1261 Palace/SignInLogic/TPPSignInBusinessLogic.swift