commit eb2ae22303c2f408d41f25c7c08d504aee3337de
parent 7b460c39362197673a899079521fb3193c51d88e
Author: Michael Camilleri <[email protected]>
Date: Wed, 22 Apr 2026 18:00:41 +0900
Avoid data loss caused by pending updates
It was possible for data loss to occur if a pending write was in the outbox
(which did happen due to unrelated throttling issues). This commit fixes it.
Co-Authored-By: Claude Opus 4.7 <[email protected]>
Diffstat:
2 files changed, 210 insertions(+), 0 deletions(-)
diff --git a/Crossmate/Sync/RecordSerializer.swift b/Crossmate/Sync/RecordSerializer.swift
@@ -138,8 +138,21 @@ enum RecordSerializer {
game: game
)
+ // System fields are always server-authoritative: we need the latest
+ // change tag for the next push, regardless of who wins on content.
entity.ckRecordName = recordName
entity.ckSystemFields = encodeSystemFields(of: record)
+
+ let serverUpdatedAt = record["updatedAt"] as? Date ?? .distantPast
+ if localCellWins(
+ recordName: recordName,
+ localEntityUpdatedAt: entity.updatedAt,
+ serverUpdatedAt: serverUpdatedAt,
+ in: context
+ ) {
+ return entity
+ }
+
entity.letter = record["letter"] as? String ?? ""
entity.markKind = record["markKind"] as? Int16 ?? 0
entity.checkedWrong = record["checkedWrong"] as? Bool ?? false
@@ -149,6 +162,36 @@ enum RecordSerializer {
return entity
}
+ /// Decides whether the local copy of a cell should be preserved rather
+ /// than overwritten by an incoming server record. A pending outbox entry
+ /// at or after the server's `updatedAt` means the user has unpushed
+ /// local state that must not be clobbered; a locally-newer `updatedAt`
+ /// is a defensive fallback for cases where the outbox has already been
+ /// drained but the push has not yet been confirmed by the server.
+ static func localCellWins(
+ recordName: String,
+ localEntityUpdatedAt: Date?,
+ serverUpdatedAt: Date,
+ in context: NSManagedObjectContext
+ ) -> Bool {
+ let pendingRequest = NSFetchRequest<PendingChangeEntity>(entityName: "PendingChangeEntity")
+ pendingRequest.predicate = NSPredicate(format: "recordName == %@", recordName)
+ pendingRequest.fetchLimit = 1
+ if let pending = try? context.fetch(pendingRequest).first,
+ let payloadJSON = pending.payload,
+ let data = payloadJSON.data(using: .utf8),
+ let payload = try? JSONDecoder().decode(PendingChangePayload.self, from: data) {
+ let localUpdatedAt = payload.updatedAt ?? .distantPast
+ return localUpdatedAt >= serverUpdatedAt
+ }
+
+ if let local = localEntityUpdatedAt, local > serverUpdatedAt {
+ return true
+ }
+
+ return false
+ }
+
// MARK: - Pending change construction
/// Enqueues a Game pending change for the given entity. Used both when a
diff --git a/Tests/Unit/RecordSerializerTests.swift b/Tests/Unit/RecordSerializerTests.swift
@@ -1,4 +1,5 @@
import CloudKit
+import CoreData
import Foundation
import Testing
@@ -110,3 +111,169 @@ struct PendingChangePayloadTests {
#expect(decoded.completedAt == nil)
}
}
+
+/// Regression coverage for the fetch-side overwrite bug documented in PLAN.md:
+/// an incoming CKRecord must not clobber unpushed local state when a pending
+/// outbox entry exists (or when the local entity is strictly newer).
+@Suite("RecordSerializer.applyCellRecord LWW", .serialized)
+@MainActor
+struct ApplyCellRecordLWWTests {
+
+ private func makeCellRecord(
+ gameID: UUID,
+ row: Int,
+ col: Int,
+ letter: String,
+ updatedAt: Date
+ ) -> CKRecord {
+ let zoneID = RecordSerializer.zoneID()
+ let recordID = CKRecord.ID(
+ recordName: RecordSerializer.recordName(forCellInGame: gameID, row: row, col: col),
+ zoneID: zoneID
+ )
+ let record = CKRecord(recordType: "Cell", recordID: recordID)
+ record["letter"] = letter as CKRecordValue
+ record["markKind"] = 0 as CKRecordValue
+ record["checkedWrong"] = false as CKRecordValue
+ record["updatedAt"] = updatedAt as CKRecordValue
+ return record
+ }
+
+ private func firstCell(
+ in game: GameEntity,
+ row: Int16 = 0,
+ col: Int16 = 0
+ ) -> CellEntity {
+ let cells = (game.cells as? Set<CellEntity>) ?? []
+ return cells.first { $0.row == row && $0.col == col }!
+ }
+
+ private func gameID(from entity: GameEntity) -> UUID {
+ entity.id!
+ }
+
+ @Test("Server record applies when no pending change exists")
+ func serverWinsWithNoPending() throws {
+ let (_, _, gameEntity, _) = try makeTestGame()
+ let context = gameEntity.managedObjectContext!
+
+ let cell = firstCell(in: gameEntity)
+ cell.letter = ""
+ cell.updatedAt = Date(timeIntervalSince1970: 1_000)
+ try context.save()
+
+ let record = makeCellRecord(
+ gameID: gameID(from: gameEntity),
+ row: 0,
+ col: 0,
+ letter: "A",
+ updatedAt: Date(timeIntervalSince1970: 2_000)
+ )
+
+ _ = RecordSerializer.applyCellRecord(record, to: context, game: gameEntity)
+
+ #expect(firstCell(in: gameEntity).letter == "A")
+ }
+
+ @Test("Pending change with newer updatedAt preserves local letter")
+ func localWinsWhenPendingIsNewer() throws {
+ let (_, _, gameEntity, _) = try makeTestGame()
+ let context = gameEntity.managedObjectContext!
+ let id = gameID(from: gameEntity)
+
+ let cell = firstCell(in: gameEntity)
+ cell.letter = "A"
+ let localUpdatedAt = Date(timeIntervalSince1970: 2_000)
+ cell.updatedAt = localUpdatedAt
+ RecordSerializer.enqueueCellPending(for: cell, in: context)
+ try context.save()
+
+ let staleServerRecord = makeCellRecord(
+ gameID: id,
+ row: 0,
+ col: 0,
+ letter: "",
+ updatedAt: Date(timeIntervalSince1970: 1_000)
+ )
+
+ _ = RecordSerializer.applyCellRecord(staleServerRecord, to: context, game: gameEntity)
+
+ #expect(firstCell(in: gameEntity).letter == "A")
+ #expect(firstCell(in: gameEntity).updatedAt == localUpdatedAt)
+ }
+
+ @Test("Pending change with older updatedAt yields to server")
+ func serverWinsWhenPendingIsOlder() throws {
+ let (_, _, gameEntity, _) = try makeTestGame()
+ let context = gameEntity.managedObjectContext!
+ let id = gameID(from: gameEntity)
+
+ let cell = firstCell(in: gameEntity)
+ cell.letter = "A"
+ cell.updatedAt = Date(timeIntervalSince1970: 1_000)
+ RecordSerializer.enqueueCellPending(for: cell, in: context)
+ try context.save()
+
+ let newerServerRecord = makeCellRecord(
+ gameID: id,
+ row: 0,
+ col: 0,
+ letter: "B",
+ updatedAt: Date(timeIntervalSince1970: 2_000)
+ )
+
+ _ = RecordSerializer.applyCellRecord(newerServerRecord, to: context, game: gameEntity)
+
+ #expect(firstCell(in: gameEntity).letter == "B")
+ }
+
+ @Test("Local entity updatedAt strictly newer than server preserves local")
+ func localEntityNewerFallback() throws {
+ let (_, _, gameEntity, _) = try makeTestGame()
+ let context = gameEntity.managedObjectContext!
+ let id = gameID(from: gameEntity)
+
+ let cell = firstCell(in: gameEntity)
+ cell.letter = "A"
+ cell.updatedAt = Date(timeIntervalSince1970: 2_000)
+ try context.save()
+
+ let staleServerRecord = makeCellRecord(
+ gameID: id,
+ row: 0,
+ col: 0,
+ letter: "",
+ updatedAt: Date(timeIntervalSince1970: 1_000)
+ )
+
+ _ = RecordSerializer.applyCellRecord(staleServerRecord, to: context, game: gameEntity)
+
+ #expect(firstCell(in: gameEntity).letter == "A")
+ }
+
+ @Test("System fields are updated even when local content wins")
+ func systemFieldsAlwaysUpdated() throws {
+ let (_, _, gameEntity, _) = try makeTestGame()
+ let context = gameEntity.managedObjectContext!
+ let id = gameID(from: gameEntity)
+
+ let cell = firstCell(in: gameEntity)
+ cell.letter = "A"
+ cell.updatedAt = Date(timeIntervalSince1970: 2_000)
+ cell.ckSystemFields = nil
+ RecordSerializer.enqueueCellPending(for: cell, in: context)
+ try context.save()
+
+ let staleServerRecord = makeCellRecord(
+ gameID: id,
+ row: 0,
+ col: 0,
+ letter: "",
+ updatedAt: Date(timeIntervalSince1970: 1_000)
+ )
+
+ _ = RecordSerializer.applyCellRecord(staleServerRecord, to: context, game: gameEntity)
+
+ #expect(firstCell(in: gameEntity).ckSystemFields != nil)
+ }
+}
