refactor(accounts): extract AccountRegistryStore state collaborator (Wave 3 / 3a-2) - #1363
Conversation
c6bc8fb to
7d355fa
Compare
🧪 Unit Test Results📊 View Full Interactive Report ✅ ALL TESTS PASSED8189 tests | 8051 passed | 0 failed | 136 skipped | ⏱️ 11m 23s | 📊 98.3% | 📈 44.6% coverage All 945 classes (full matrix — click to expand)
📊 Full interactive matrix: report 📊 Testing Coverage BreakdownUnit Test Line Coverage (testable surfaces): 44.6% Total coverage incl. UI/lifecycle: 43.6% (17 files excluded from testable denominator — see
🔗 Interactive HTML Report | CI Run Details Counts above were produced by this CI run's xcresult parse — reproduce via the run link. 📦 Downloadable Artifacts
|
…Wave 3 / 3a-2) Second in-target collaborator split out of AccountsManager. The account-registry STATE and all thread-safe access — the current hash, the [hash → [Account]] sets, the O(1) accountByUUID index kept in barrier-lockstep with them, the separate slim launch fallback, and the concurrent accountSetsLock sync-read/barrier-write primitives — move into an injected AccountRegistryStore. The hub keeps only thin forwarding facades. Behaviour-identical. Stacked on 3a-1. - New AccountRegistryStore.swift: a `final class … @unchecked Sendable` (NOT an actor — account(_:) is a synchronous @objc TPPLibraryAccountsProvider requirement and the drain choreography depends on sync `.sync` reads; an actor would force await through the conformance and delete that timing) that owns the concurrent DispatchQueue verbatim. The two barrier carriers move with it. - `accountSet` (current hash) moves into the store so ONE lock still guards {currentHash, accountSets, accountByUUID}. accountsForCurrentHash() / currentBucketIsLoaded() read the hash + its bucket in ONE performRead — preserving the multi-field snapshot atomicity a library switch would otherwise tear (the facade must NOT use accounts(forKey: currentHash), which splits it across two lock acquisitions). The :1900 load-trigger keeps its polarity (!bucketIsNonEmpty). - Hub: deleted the state + performRead/performWrite/mutateAccountSets/storeSlim/ slimAccount + the buildAccountIndex impl; injected `registryStore` via init default; retyped ~15 call sites; kept account/accounts/accountsHaveLoaded facades and a buildAccountIndex static shim (AccountsManagerAccountIndexTests calls it). The @unchecked Sendable audit block refreshed to point at the store. - Tests: AccountRegistryStoreSeamTests — index-coherence-under-churn, no-torn-read, slim-isolation, cross-switch atomicity (Finding-1 pin), and hub- delegation routing; new makeFreshAccountsManager(defaults:registryStore:) helper. Verified: god-class freeze re-baselined DOWN 2173 -> 2080 (-93); locator 304 / shared-read 213 at baseline; package-purity clean; Palace-noDRM BUILD SUCCEEDED (production compile). Architect BLOCKED then APPROVED after the atomicity + polarity fixes. **Scope:** the account-registry state + thread-safe access out of AccountsManager into an injected store; Accounts area only. **Not done:** the DRM PalaceTests run (incl. the new concurrency tests) executes in CI, not locally — this change was authored in a git worktree whose Adobe-DRM test host can't build (private dp_all.h); noDRM compile-verify passed locally. carveSlimFeed still deferred (3a-1). **Deferred:** the remaining three 3a collaborators (AuthDocumentLoader, RegistryLoader, CredentialResolver, CurrentAccountStore) are separate PRs. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
7d355fa to
fd213c1
Compare
🏗️ CodeAtlas Ledger Analysis✅ All Checks Passed♿ Accessibility (via AccessLint)
✅ No accessibility issues detected 🏛️ Architecture Analysis
🔄 Dependency CyclesSee full report for cycle details. 📋 Architecture FindingsSee full report for detailed findings. 🔍 Reachability Analysis
ℹ️ No architecture-relevant files in this diff — reachability not evaluated. 📊 0 files analyzed | 📦 Download Full Report Powered by CodeAtlas Ledger |
What & why
Second of the 3a
PalaceAccountsin-target collaborator splits. The account-registry state and all thread-safe access — the current hash, the[hash → [Account]]sets, the O(1)accountByUUIDindex kept in barrier-lockstep with them, the separate slim launch fallback, and the concurrentaccountSetsLocksync-read/barrier-write primitives — move out ofAccountsManagerinto an injectedAccountRegistryStore. After this the hub holds no registry-state concurrency machinery. Behaviour-identical.Design decisions (both architect-validated)
final class, not anactor.account(_:)is a synchronous@objc TPPLibraryAccountsProviderrequirement, and the background-work drain choreography depends on a synchronousaccountSetsLock.syncread blocking behind the barrier. An actor would forceawaitthrough the@objcconformance and delete that timing. The store owns the same concurrentDispatchQueueverbatim.accountSet(current hash) moves into the store so one lock still guards{currentHash, accountSets, accountByUUID}.accountsForCurrentHash()/currentBucketIsLoaded()read the hash and its bucket in oneperformRead— preserving the multi-field snapshot atomicity a library switch would otherwise tear. The hub facade routesaccounts(nil)/accountsHaveLoadedto these atomic methods, neveraccounts(forKey: currentHash)(which splits the read across two lock acquisitions — caught at contract review as Finding-1).Changes
AccountRegistryStore.swift(@unchecked Sendable; the two barrier carriers move with it); index rebuilt in the same barrier as any mutation via the pure staticbuildAccountIndex.performRead/performWrite/mutateAccountSets/storeSlim/slimAccount+ thebuildAccountIndeximpl; injectedregistryStorevia init default; retyped ~15 call sites; keptaccount/accounts/accountsHaveLoadedfacades and abuildAccountIndexstatic shim (AccountsManagerAccountIndexTestscalls it).@unchecked Sendableaudit + drain comments refreshed to name the store.accountSetseed becomesstore.setCurrentHash(...)aftersuper.init(); the sync preload observes it via GCD barrier FIFO.AccountRegistryStoreSeamTests— index-coherence-under-churn (_coherentSnapshot), no-torn-read, slim-isolation, deterministic current-hash correctness, torn-read robustness, and hub-delegation routing.Verification
PalaceTests(incl. the concurrency suite)dp_all.h); verified by reading, not a local run. CI must be green before merge.Review rigor
/rigorous-fix: fix-contract → architect pre-review BLOCKED (atomicity split-read + polarity) → fixed → APPROVED → implement → 3× SoD (architect,qa_test,blast_radius). qa initially BLOCKED one over-claiming atomicity test (inert against a split-read mutant); fixed by reframing it honestly + adding a deterministic correctness test + a torn-read robustness test → all three APPROVED. Reviewers are session-spawned agents (the[Self-Approval]flag is the known false-positive), not an independent human review.Not in this PR
carveSlimFeed(3a-1 deferral). The remaining three 3a collaborators (AuthDocumentLoader, RegistryLoader, CredentialResolver, CurrentAccountStore) are separate PRs.🤖 Generated with Claude Code