crossmate

A collaborative crossword app for iOS
Log | Files | Refs | LICENSE

commit 24ef231551d8a3755259620f735f1e09a25065e1
parent 36031858801573ea8a1e5a129e0b1b09db594da4
Author: Michael Camilleri <[email protected]>
Date:   Tue,  5 May 2026 20:43:52 +0900

Persist local author identity across launches

Prior to this commit, AuthorIdentity fetched the iCloud user record name
asynchronously at app start and held it only in memory.

As a result, tapping a shared game notification could route into
PlayerRoster.refresh() before the fetch completed, at which point the local
authorID fell back to the empty string. The pre-filtered moveAuthorIDs from
that first phase then survived into the second applyRoster call (after the
share fetch completed and currentID had populated), producing a roster with the
local user's authorID listed twice and crashing GridView's
Dictionary(uniqueKeysWithValues:).

This commit caches currentID to UserDefaults so it is non-nil on every launch
after the first successful fetch, and changes PlayerRoster to guard on a known
authorID rather than silently substituting the empty string.

Co-Authored-By: Claude Opus 4.7 <[email protected]>

Diffstat:
MCrossmate/Models/PlayerRoster.swift | 16+++++++++++-----
MCrossmate/Sync/AuthorIdentity.swift | 26+++++++++++++++++++-------
MTests/Unit/PlayerRosterTests.swift | 2+-
MTests/Unit/Sync/AuthorIdentityTests.swift | 16++++++++++++++--
4 files changed, 45 insertions(+), 15 deletions(-)

diff --git a/Crossmate/Models/PlayerRoster.swift b/Crossmate/Models/PlayerRoster.swift @@ -127,7 +127,14 @@ final class PlayerRoster { // MARK: - Refresh func refresh() async { - let localAuthorID = authorIdentity.currentID ?? "" + // Without a known local authorID we can't classify any participant as + // self vs. remote, so the only safe answer is an empty roster. The + // next refresh (after AuthorIdentity populates) will do the real work. + guard let localAuthorID = authorIdentity.currentID else { + entries = [] + remoteSelections = [:] + return + } // Pull Core Data fields off a background context. let ctx = persistence.container.newBackgroundContext() @@ -182,7 +189,7 @@ final class PlayerRoster { ) } - applyRoster(namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: nil) + applyRoster(localAuthorID: localAuthorID, namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: nil) // Fetch the CKShare if not already cached. This can be noticeably // slower on device, so publish the local Core Data roster first and @@ -193,17 +200,16 @@ final class PlayerRoster { ckZoneName: ckZoneName, ckZoneOwnerName: ckZoneOwnerName ) - applyRoster(namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: share) + applyRoster(localAuthorID: localAuthorID, namesMap: namesMap, moveAuthorIDs: moveAuthorIDs, rawSelections: rawSelections, share: share) } private func applyRoster( + localAuthorID: String, namesMap: [String: String], moveAuthorIDs: [String], rawSelections: [RawSelection], share: CKShare? ) { - let localAuthorID = authorIdentity.currentID ?? "" - // Collect all remote participant authorIDs. var otherAuthorIDs = Set<String>() for key in namesMap.keys where key != localAuthorID && !key.isEmpty { diff --git a/Crossmate/Sync/AuthorIdentity.swift b/Crossmate/Sync/AuthorIdentity.swift @@ -1,23 +1,35 @@ import CloudKit import Foundation -/// Cache for the current iCloud user's record name. Populated asynchronously -/// at app start and refreshed on account-change events from `CKSyncEngine`. -/// The synchronous `currentID` getter lets `GameMutator` stamp moves without -/// an `await` at the call site. +/// Cache for the current iCloud user's record name. Persisted to +/// UserDefaults so `currentID` is non-nil on every launch after the first +/// successful CloudKit fetch — without this, a cold launch from a shared-game +/// notification can race the async `userRecordID()` call and treat the local +/// user's authorID as if it belonged to a remote peer. Refreshed on +/// account-change events from `CKSyncEngine`. @MainActor final class AuthorIdentity { + private static let storageKey = "AuthorIdentity.currentID" + private(set) var currentID: String? + private let storage: UserDefaults - init() {} + init(storage: UserDefaults = .standard) { + self.storage = storage + currentID = storage.string(forKey: Self.storageKey) + } - /// Injects a known ID without hitting CloudKit — use in unit tests only. + /// Injects a known ID without hitting CloudKit or persistent storage — + /// use in unit tests only. init(testing fixedID: String) { + self.storage = UserDefaults.standard currentID = fixedID } func refresh(using container: CKContainer) async { guard let recordID = try? await container.userRecordID() else { return } - currentID = recordID.recordName + let newID = recordID.recordName + currentID = newID + storage.set(newID, forKey: Self.storageKey) } } diff --git a/Tests/Unit/PlayerRosterTests.swift b/Tests/Unit/PlayerRosterTests.swift @@ -90,7 +90,7 @@ struct PlayerRosterTests { return PlayerRoster( gameID: gameID, colorStore: store, - authorIdentity: AuthorIdentity(), + authorIdentity: AuthorIdentity(testing: "_Local"), preferences: prefs, persistence: persistence, container: container diff --git a/Tests/Unit/Sync/AuthorIdentityTests.swift b/Tests/Unit/Sync/AuthorIdentityTests.swift @@ -8,9 +8,13 @@ import Testing @MainActor struct AuthorIdentityTests { + private func makeIsolatedStorage() -> UserDefaults { + UserDefaults(suiteName: "test-authoridentity-\(UUID().uuidString)")! + } + @Test("Initial state returns nil before any refresh") func initialStateIsNil() { - let identity = AuthorIdentity() + let identity = AuthorIdentity(storage: makeIsolatedStorage()) #expect(identity.currentID == nil) } @@ -22,7 +26,7 @@ struct AuthorIdentityTests { @Test("refresh with unavailable account leaves currentID nil") func refreshFailurePreservesNil() async { - let identity = AuthorIdentity() + let identity = AuthorIdentity(storage: makeIsolatedStorage()) // Using the default CKContainer without a signed-in account will fail. // The contract is: on failure, currentID stays nil (or unchanged). let containerWithNoAccount = CKContainer(identifier: "iCloud.net.inqk.crossmate.v2") @@ -31,4 +35,12 @@ struct AuthorIdentityTests { // We can only assert it doesn't crash; value depends on sim state. _ = identity.currentID } + + @Test("Persists currentID across instances sharing storage") + func currentIDPersistsAcrossInstances() { + let storage = makeIsolatedStorage() + storage.set("_persisted", forKey: "AuthorIdentity.currentID") + let identity = AuthorIdentity(storage: storage) + #expect(identity.currentID == "_persisted") + } }