commit 4a95670cb5de9d92a861301a8f8c1b0aabec308a
parent 0db18d26817aeb30740f74d66d08c53cac72c305
Author: Michael Camilleri <[email protected]>
Date: Fri, 12 Jun 2026 08:44:08 +0900
Preserve the failing Core Data store before destructive recovery
PersistenceController.recreateStore wiped the on-disk store outright
when a load or migration failed, on the rationale that the store is a
rebuildable cache of CloudKit. That rationale fails when iCloud sync is
disabled — the local store is then the only copy of the user's games —
and even with sync on, offline edits not yet uploaded are lost.
Recovery now moves the store files (including the -wal/-shm sidecars)
aside under a .broken-<timestamp> suffix before destroying and
rebuilding, so the data stays inspectable and recoverable, and the
event is surfaced through the diagnostics EventLog with the original
error and the preserved file names. The move is best effort: a file
that can't be moved is left for destroyPersistentStore so recovery
always proceeds.
PersistenceController gains an injectable store URL so tests can drive
the on-disk path against a throwaway location. New tests cover the
failure path (broken store preserved byte-for-byte, empty store
rebuilt) and the healthy path (reopening an existing store triggers no
recovery).
Co-Authored-By: Claude Fable 5 <[email protected]>
Diffstat:
3 files changed, 135 insertions(+), 7 deletions(-)
diff --git a/Crossmate.xcodeproj/project.pbxproj b/Crossmate.xcodeproj/project.pbxproj
@@ -30,6 +30,7 @@
1F4E5473F78A5CEDBA9719CE /* NYTAuthService.swift in Sources */ = {isa = PBXBuildFile; fileRef = A253416F4FEA271A80B22A73 /* NYTAuthService.swift */; };
262A9CE8C3CB93869190CFF1 /* GameStoreMergedAuthorCellsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 122BC1863D12DE06388D5DA7 /* GameStoreMergedAuthorCellsTests.swift */; };
267ED5B329F05A30430B73A0 /* EngagementHost.swift in Sources */ = {isa = PBXBuildFile; fileRef = 18C701DAE36000DE19F7CC95 /* EngagementHost.swift */; };
+ 26DC22F88FA10C47BC06975E /* PersistenceRecoveryTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A467BC00116EEC8500BE6A1 /* PersistenceRecoveryTests.swift */; };
2A8FB9C020B2072659C24C8E /* CompactSlider.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0CE803B2DF05DDF457B27FE2 /* CompactSlider.swift */; };
2AF2550B08CE79F8615B3076 /* FriendZone.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A4AFF292381C9B33C0F2CD6 /* FriendZone.swift */; };
2B03A1A36AB55495ED0E8684 /* HardwareKeyboardInputView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 947102E58EFCF898D258AC3E /* HardwareKeyboardInputView.swift */; };
@@ -249,6 +250,7 @@
465F2BB469EFE84CF3733398 /* Game.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Game.swift; sourceTree = "<group>"; };
46801B570FC0B2C791ECDED3 /* PlayerSessionNavigationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PlayerSessionNavigationTests.swift; sourceTree = "<group>"; };
47532AED239AEF476D8E9206 /* NotificationStateTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationStateTests.swift; sourceTree = "<group>"; };
+ 4A467BC00116EEC8500BE6A1 /* PersistenceRecoveryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PersistenceRecoveryTests.swift; sourceTree = "<group>"; };
4AF633D73818BD59F759FAC4 /* AboutView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AboutView.swift; sourceTree = "<group>"; };
4B33C21324E1474BCC126AA0 /* MovesCodecLegacyDecodeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MovesCodecLegacyDecodeTests.swift; sourceTree = "<group>"; };
4DB0580C9B7C778F34BE6AC2 /* EngagementLifecycle.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EngagementLifecycle.swift; sourceTree = "<group>"; };
@@ -441,6 +443,7 @@
C54223FED97577A593B7964E /* NYTToXDConverterTests.swift */,
A8CA0FC750259EB1D762B0EE /* OpenPuzzleBannerTests.swift */,
D491B7232333AA8957732387 /* PendingEditFlagTests.swift */,
+ 4A467BC00116EEC8500BE6A1 /* PersistenceRecoveryTests.swift */,
5DE04D53EC3BC7D2DA0093C3 /* PlayerNamePublisherTests.swift */,
1813630FA05C194AFF43855C /* PlayerRosterTests.swift */,
FF159746D076E051C2CB590C /* PlayerSelectionPublisherTests.swift */,
@@ -809,6 +812,7 @@
A458AF9CA8579AB51B695B08 /* PendingChangeReapTests.swift in Sources */,
8225918652DCC822CA1C862F /* PendingEditFlagTests.swift in Sources */,
014134FB81566B5D41168260 /* PerGameZoneTests.swift in Sources */,
+ 26DC22F88FA10C47BC06975E /* PersistenceRecoveryTests.swift in Sources */,
CEDF853009D0C367035F1F76 /* PlayerNamePublisherTests.swift in Sources */,
7BD1A9F69953F9C3288969AF /* PlayerRecordPresenceTests.swift in Sources */,
04062BCD473ED244159B1066 /* PlayerRosterTests.swift in Sources */,
diff --git a/Crossmate/Persistence/PersistenceController.swift b/Crossmate/Persistence/PersistenceController.swift
@@ -14,7 +14,7 @@ final class PersistenceController {
private let eventLog: EventLog?
- init(inMemory: Bool = false, eventLog: EventLog? = nil) {
+ init(inMemory: Bool = false, storeURL: URL? = nil, eventLog: EventLog? = nil) {
self.eventLog = eventLog
container = NSPersistentContainer(
name: "CrossmateModel",
@@ -29,6 +29,14 @@ final class PersistenceController {
description.type = NSInMemoryStoreType
container.persistentStoreDescriptions = [description]
} else {
+ // The app always uses the container's default location; tests
+ // point the store at a throwaway URL to exercise the on-disk
+ // load/recovery path without touching real data.
+ if let storeURL {
+ container.persistentStoreDescriptions = [
+ NSPersistentStoreDescription(url: storeURL)
+ ]
+ }
// Enable lightweight migration so additive schema changes — and
// attribute renames carrying a `renamingIdentifier` in the model
// — apply on launch without a hand-written mapping. A non-additive
@@ -57,16 +65,22 @@ final class PersistenceController {
}
}
- /// Discards the on-disk store and rebuilds it empty. The local store is a
- /// rebuildable cache of CloudKit, so when an existing store can't be opened
- /// against the current model — e.g. after an in-place schema change with no
- /// migration source — wiping it is recoverable: the sync engine refetches
- /// every record on the next sync. Any never-synced or not-yet-uploaded
- /// local state is intentionally sacrificed for a clean store.
+ /// Rebuilds the store empty when an existing one can't be opened against
+ /// the current model — e.g. after an in-place schema change with no
+ /// migration source. The store is *usually* a rebuildable cache of CloudKit
+ /// (the sync engine refetches every record on the next sync), but not
+ /// always: iCloud sync is user-toggleable, making the local store the only
+ /// copy of that user's games, and even with sync on, offline edits may not
+ /// have uploaded yet. The failing files are therefore moved aside under a
+ /// `.broken-<timestamp>` suffix rather than destroyed, so the data stays
+ /// inspectable and recoverable, and the recovery is surfaced through
+ /// diagnostics.
private func recreateStore(after originalError: Error) {
let coordinator = container.persistentStoreCoordinator
+ var preserved: [String] = []
for description in container.persistentStoreDescriptions {
guard let url = description.url else { continue }
+ preserved.append(contentsOf: preserveBrokenStore(at: url))
do {
try coordinator.destroyPersistentStore(
at: url,
@@ -86,6 +100,39 @@ final class PersistenceController {
)
}
}
+ eventLog?.note(
+ "PersistenceController: store load failed; rebuilt empty"
+ + (preserved.isEmpty
+ ? " (no files to preserve)"
+ : " (broken store preserved as \(preserved.joined(separator: ", ")))")
+ + " — \(originalError)",
+ level: "error"
+ )
+ }
+
+ /// Moves the failing store's files (including the `-wal`/`-shm` sidecars)
+ /// aside before destructive recovery, returning the names of the files it
+ /// preserved. Best effort: a file that can't be moved is left in place for
+ /// `destroyPersistentStore` to clear, so recovery always proceeds.
+ private func preserveBrokenStore(at url: URL) -> [String] {
+ let fileManager = FileManager.default
+ let formatter = DateFormatter()
+ formatter.dateFormat = "yyyyMMdd-HHmmss"
+ formatter.timeZone = TimeZone(secondsFromGMT: 0)
+ let timestamp = formatter.string(from: Date())
+ var preserved: [String] = []
+ for suffix in ["", "-wal", "-shm"] {
+ let source = URL(fileURLWithPath: url.path + suffix)
+ guard fileManager.fileExists(atPath: source.path) else { continue }
+ let destination = URL(fileURLWithPath: source.path + ".broken-\(timestamp)")
+ do {
+ try fileManager.moveItem(at: source, to: destination)
+ preserved.append(destination.lastPathComponent)
+ } catch {
+ // Leave the file for destroyPersistentStore.
+ }
+ }
+ return preserved
}
/// One-shot pass for `GameEntity` rows created before cached summary
diff --git a/Tests/Unit/PersistenceRecoveryTests.swift b/Tests/Unit/PersistenceRecoveryTests.swift
@@ -0,0 +1,77 @@
+import CoreData
+import Foundation
+import Testing
+
+@testable import Crossmate
+
+/// `PersistenceController`'s destructive recovery path: when the on-disk store
+/// can't be opened, the failing files must be moved aside — not destroyed —
+/// before the empty rebuild. The store is the *only* copy of a user's games
+/// when iCloud sync is disabled, so a silent wipe is permanent data loss.
+@Suite("Persistence recovery", .serialized)
+@MainActor
+struct PersistenceRecoveryTests {
+
+ private func makeStoreDirectory() throws -> URL {
+ let directory = FileManager.default.temporaryDirectory
+ .appendingPathComponent("PersistenceRecoveryTests-\(UUID().uuidString)", isDirectory: true)
+ try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
+ return directory
+ }
+
+ @Test func brokenStoreIsPreservedBeforeRebuild() throws {
+ let fileManager = FileManager.default
+ let directory = try makeStoreDirectory()
+ defer { try? fileManager.removeItem(at: directory) }
+
+ // Garbage well past the SQLite header size, so the open fails as
+ // not-a-database rather than being adopted as a fresh empty store.
+ let storeURL = directory.appendingPathComponent("CrossmateModel.sqlite")
+ let garbage = Data(repeating: 0x6E, count: 1024)
+ try garbage.write(to: storeURL)
+
+ let controller = PersistenceController(storeURL: storeURL)
+
+ // Recovery rebuilt a usable, empty store at the original URL.
+ #expect(controller.container.persistentStoreCoordinator.persistentStores.count == 1)
+ let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ #expect(try controller.viewContext.count(for: request) == 0)
+
+ // The broken file was moved aside with its bytes intact.
+ let backups = try fileManager.contentsOfDirectory(atPath: directory.path)
+ .filter { $0.contains(".broken-") }
+ #expect(backups.count == 1)
+ let backup = try #require(backups.first)
+ #expect(try Data(contentsOf: directory.appendingPathComponent(backup)) == garbage)
+ }
+
+ @Test func healthyStoreLoadsWithoutBackup() throws {
+ let fileManager = FileManager.default
+ let directory = try makeStoreDirectory()
+ defer { try? fileManager.removeItem(at: directory) }
+ let storeURL = directory.appendingPathComponent("CrossmateModel.sqlite")
+
+ // First launch creates the store and saves a row.
+ let gameID = UUID()
+ do {
+ let controller = PersistenceController(storeURL: storeURL)
+ let entity = GameEntity(context: controller.viewContext)
+ entity.id = gameID
+ entity.title = "Persisted Game"
+ entity.puzzleSource = "Title: Persisted\n\n\nAB\n\n\nA1. _ ~ AB"
+ entity.createdAt = Date()
+ entity.updatedAt = Date()
+ try controller.viewContext.save()
+ }
+
+ // A relaunch against the same files reopens them without triggering
+ // recovery: the row survives and nothing was moved aside.
+ let controller = PersistenceController(storeURL: storeURL)
+ let request = NSFetchRequest<GameEntity>(entityName: "GameEntity")
+ let games = try controller.viewContext.fetch(request)
+ #expect(games.compactMap { $0.id } == [gameID])
+ let backups = try fileManager.contentsOfDirectory(atPath: directory.path)
+ .filter { $0.contains(".broken-") }
+ #expect(backups.isEmpty)
+ }
+}