From e9817b1c3d089075da2297e410d44c18a9def264 Mon Sep 17 00:00:00 2001 From: Michael Yeack Date: Tue, 2 Jun 2026 12:35:11 -0700 Subject: [PATCH] Add sync apply / conflict-resolution unit tests (regression surface for #43/#44) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The sync-apply layer — ContactSyncApply.applyRemoteContact and ModelSyncApply.applyRemoteInteraction — is the exact code behind the recent drift (#44) and launch-crash (#43) incidents and had zero unit coverage. These functions are pure (payload dict + ModelContext), so no protocol seam or network mock is needed; "remote" payloads are built with the real contactToDict/interactionToDict serializers so ISO8601 formatting matches the parser. SyncApplyTests (11): - insert path: absent record inserted + marked synced; round-trip preserves name/emails/phones/score/priority; idempotent re-apply is a UUID upsert (no dup). - conflict resolution: newer remote overwrites; NEWER+PENDING local is protected from a stale remote; NEWER-but-SYNCED local IS clobbered by an older remote (regression guard documenting exactly why edits must flip syncStatus to .pending via markLocallyEdited — the #44 root cause). - malformed payloads: missing id / missing updatedAt are ignored without throwing. - interactions: link to resolved contact; unknown contactId persists with nil contact (heals later) instead of crashing; newer pending local survives stale remote. Network-path integration (URLSession injection into LocalServerSyncService) is the remaining step noted in docs/CODE_REVIEW_2026-06-02.md finding #6. Test count 206 -> 217 XCTest (+ 13 Swift Testing), all green iOS + macOS. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 1 + Blackbook.xcodeproj/project.pbxproj | 4 + BlackbookTests/SyncApplyTests.swift | 244 ++++++++++++++++++++++++++++ 3 files changed, 249 insertions(+) create mode 100644 BlackbookTests/SyncApplyTests.swift diff --git a/.gitignore b/.gitignore index ab6657e..dbe0523 100644 --- a/.gitignore +++ b/.gitignore @@ -31,3 +31,4 @@ Pods/ # IDE .idea/ *.code-workspace +.claude/scheduled_tasks.lock diff --git a/Blackbook.xcodeproj/project.pbxproj b/Blackbook.xcodeproj/project.pbxproj index da2c8a4..87037ef 100644 --- a/Blackbook.xcodeproj/project.pbxproj +++ b/Blackbook.xcodeproj/project.pbxproj @@ -122,6 +122,7 @@ CB7C648273262A5D3B49C2D4 /* RejectedCalendarEvent.swift in Sources */ = {isa = PBXBuildFile; fileRef = 88153A388C33AA1F9FCA3424 /* RejectedCalendarEvent.swift */; }; CBF773685FBB6E09D88D189E /* Group.swift in Sources */ = {isa = PBXBuildFile; fileRef = 59D5A4C8F3FE7AFE76757690 /* Group.swift */; }; CC26CF0556D6D39776B42471 /* TagDetailView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4A1DDF330662D885E7FA4B89 /* TagDetailView.swift */; }; + CF527F41F297B1DDE78591A5 /* SyncApplyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 367B9B783BB9B16DC5A78564 /* SyncApplyTests.swift */; }; CFDEF705ED6BC7629FB51319 /* AuthenticationService.swift in Sources */ = {isa = PBXBuildFile; fileRef = 63C0BE4D5872D0BE63AD0569 /* AuthenticationService.swift */; }; D1D5EC721BB0E44AF9636690 /* LocationIconSuggestionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = F767094E24F344D3643565AA /* LocationIconSuggestionView.swift */; }; D230F0B42992BD121949564C /* BackupServiceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5C9CF11EF94294BB04E3C45E /* BackupServiceTests.swift */; }; @@ -197,6 +198,7 @@ 34F85542EF75DA29ECB0FBC2 /* GroupDetailView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = GroupDetailView.swift; sourceTree = ""; }; 364729C23FA9F3F62DDA06EF /* ContactRelationship.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactRelationship.swift; sourceTree = ""; }; 367B4A2B6BD57C86106A9103 /* InteractionViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InteractionViewModelTests.swift; sourceTree = ""; }; + 367B9B783BB9B16DC5A78564 /* SyncApplyTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SyncApplyTests.swift; sourceTree = ""; }; 367F11DDCEA44032C3674833 /* Reminder.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Reminder.swift; sourceTree = ""; }; 3744DE7F4CFD2EF1B4D4067B /* Blackbook.app */ = {isa = PBXFileReference; includeInIndex = 0; lastKnownFileType = wrapper.application; path = Blackbook.app; sourceTree = BUILT_PRODUCTS_DIR; }; 3C395757BFF2A510337F73D5 /* ContactDeduplicationService.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ContactDeduplicationService.swift; sourceTree = ""; }; @@ -534,6 +536,7 @@ 4353ABA45B9630F4CF7DBF0C /* ReminderModelTests.swift */, B10282AE3F76E876ADDB3A4A /* ReminderServiceTests.swift */, 9AB0EEFC6BF1CA172898A5CC /* SFSymbolSearchServiceTests.swift */, + 367B9B783BB9B16DC5A78564 /* SyncApplyTests.swift */, 6169F3E4455CDCD77042D958 /* SyncTypesTests.swift */, 12AE2904F18728288E8C2218 /* TagModelTests.swift */, E60CB33C4C83A483A548510B /* TestHelpers.swift */, @@ -907,6 +910,7 @@ E331D0D1AF87D4E1FA420AAC /* ReminderModelTests.swift in Sources */, C853CFD3BB51EC978673715D /* ReminderServiceTests.swift in Sources */, BF4B58B36798C30735F316B3 /* SFSymbolSearchServiceTests.swift in Sources */, + CF527F41F297B1DDE78591A5 /* SyncApplyTests.swift in Sources */, 1E261EB5A02886029E363553 /* SyncTypesTests.swift in Sources */, 25A80D3929B5841C1D224E29 /* TagModelTests.swift in Sources */, ADD9B5E47EF47C5E5D0D23F3 /* TestHelpers.swift in Sources */, diff --git a/BlackbookTests/SyncApplyTests.swift b/BlackbookTests/SyncApplyTests.swift new file mode 100644 index 0000000..e2f2909 --- /dev/null +++ b/BlackbookTests/SyncApplyTests.swift @@ -0,0 +1,244 @@ +import XCTest +import SwiftData +@testable import Blackbook + +/// Tests for the sync **apply / conflict-resolution** layer — the pure functions +/// `ContactSyncApply.applyRemoteContact` and `ModelSyncApply.applyRemoteInteraction`. +/// +/// This is the exact code surface behind the 2026-06-02 sync incidents (drift in #44, crash in +/// #43) and previously had **zero** unit coverage. These tests need no network or protocol seam: +/// the apply functions take a `[String: Any]` payload + a `ModelContext`. "Remote" payloads are +/// built with the real `contactToDict` / `interactionToDict` serializers so timestamp formatting +/// matches the parser exactly (`ISO8601DateFormatter`, whole-second precision — timestamps in +/// these tests are deliberately ≥1s apart). +/// +/// Network-path integration (URLSession injection into `LocalServerSyncService`) remains a +/// further step; see docs/CODE_REVIEW_2026-06-02.md finding #6. +@MainActor +final class SyncApplyTests: XCTestCase { + + private var container: ModelContainer! + private var context: ModelContext! + + private let t1 = Date(timeIntervalSince1970: 1_000_000) // older + private let t2 = Date(timeIntervalSince1970: 1_000_500) // newer (+500s) + + override func setUpWithError() throws { + container = try TestHelpers.makeContainer() + context = ModelContext(container) + } + + override func tearDown() { + context = nil + container = nil + } + + // MARK: - Builders + + /// A detached Contact serialized to a remote payload. Not inserted into `context`. + private func remoteContactDict( + id: UUID, + first: String, + last: String = "X", + updatedAt: Date, + isPriority: Bool = false, + emails: [String] = [] + ) -> [String: Any] { + let tmp = Contact(firstName: first, lastName: last) + tmp.id = id + tmp.updatedAt = updatedAt + tmp.isPriority = isPriority + tmp.emails = emails + return ContactSyncApply.contactToDict(tmp) + } + + private func fetchContact(_ id: UUID) throws -> Contact? { + try context.fetch(FetchDescriptor(predicate: #Predicate { $0.id == id })).first + } + + private func fetchInteraction(_ id: UUID) throws -> Interaction? { + try context.fetch(FetchDescriptor(predicate: #Predicate { $0.id == id })).first + } + + @discardableResult + private func insertLocalContact( + id: UUID = UUID(), + first: String, + updatedAt: Date, + status: SyncStatus, + isPriority: Bool = false + ) -> Contact { + let c = Contact(firstName: first, lastName: "X") + c.id = id + c.updatedAt = updatedAt + c.syncStatus = status.rawValue + c.isPriority = isPriority + context.insert(c) + return c + } + + // MARK: - applyRemoteContact: insert path + + func testRemoteContactInsertedWhenAbsent() throws { + let id = UUID() + let dict = remoteContactDict(id: id, first: "Ada", updatedAt: t2, isPriority: true, emails: ["ada@x.com"]) + try ContactSyncApply.applyRemoteContact(dict, to: context) + + let c = try XCTUnwrap(try fetchContact(id)) + XCTAssertEqual(c.firstName, "Ada") + XCTAssertTrue(c.isPriority) + XCTAssertEqual(c.emails, ["ada@x.com"]) + XCTAssertEqual(c.syncStatus, SyncStatus.synced.rawValue, "inserted remote record is marked synced") + } + + func testRoundTripPreservesCoreFields() throws { + let source = Contact(firstName: "Grace", lastName: "Hopper") + source.id = UUID() + source.updatedAt = t2 + source.emails = ["grace@navy.mil"] + source.phones = ["5550100"] + source.relationshipScore = 73 + source.isPriority = true + let dict = ContactSyncApply.contactToDict(source) + + try ContactSyncApply.applyRemoteContact(dict, to: context) + + let c = try XCTUnwrap(try fetchContact(source.id)) + XCTAssertEqual(c.firstName, "Grace") + XCTAssertEqual(c.lastName, "Hopper") + XCTAssertEqual(c.emails, ["grace@navy.mil"]) + XCTAssertEqual(c.phones, ["5550100"]) + XCTAssertEqual(c.relationshipScore, 73, accuracy: 0.0001) + XCTAssertTrue(c.isPriority) + } + + // MARK: - applyRemoteContact: conflict resolution + + func testRemoteNewerOverwritesLocal() throws { + let id = UUID() + let local = insertLocalContact(id: id, first: "OldName", updatedAt: t1, status: .synced) + try context.save() + + let dict = remoteContactDict(id: id, first: "NewName", updatedAt: t2, isPriority: true) + try ContactSyncApply.applyRemoteContact(dict, to: context) + + XCTAssertEqual(local.firstName, "NewName", "newer remote should win") + XCTAssertTrue(local.isPriority) + XCTAssertEqual(local.syncStatus, SyncStatus.synced.rawValue) + } + + func testLocalNewerAndPendingIsProtectedFromStaleRemote() throws { + // The key guard: an unsynced local edit that is NEWER than the incoming remote must NOT be + // clobbered. This is what keeps a fresh local edit from being lost on the next pull. + let id = UUID() + let local = insertLocalContact(id: id, first: "LocalEdit", updatedAt: t2, status: .pending, isPriority: true) + try context.save() + + let dict = remoteContactDict(id: id, first: "StaleRemote", updatedAt: t1, isPriority: false) + try ContactSyncApply.applyRemoteContact(dict, to: context) + + XCTAssertEqual(local.firstName, "LocalEdit", "newer pending local edit must survive") + XCTAssertTrue(local.isPriority) + XCTAssertEqual(local.syncStatus, SyncStatus.pending.rawValue, "still pending — not marked synced") + } + + func testLocalNewerButSyncedIsOverwrittenByStaleRemote() throws { + // Documents the subtle (and historically dangerous) branch: when local is NEWER but already + // marked `.synced`, the guard `local.syncStatus != .synced` is false, so the older remote is + // applied anyway. This is precisely why edits must flip syncStatus to `.pending` + // (markLocallyEdited) — a "synced but newer" record is treated as stale and clobbered. + // Regression guard for the #44 drift class. + let id = UUID() + let local = insertLocalContact(id: id, first: "NewerButSynced", updatedAt: t2, status: .synced, isPriority: true) + try context.save() + + let dict = remoteContactDict(id: id, first: "OlderRemote", updatedAt: t1, isPriority: false) + try ContactSyncApply.applyRemoteContact(dict, to: context) + + XCTAssertEqual(local.firstName, "OlderRemote", + "a 'synced' local is treated as not-locally-edited and yields to remote") + XCTAssertFalse(local.isPriority) + } + + // MARK: - applyRemoteContact: malformed payloads + + func testMissingIdIsIgnored() throws { + var dict = remoteContactDict(id: UUID(), first: "NoId", updatedAt: t2) + dict.removeValue(forKey: "id") + XCTAssertNoThrow(try ContactSyncApply.applyRemoteContact(dict, to: context)) + XCTAssertEqual(try context.fetch(FetchDescriptor()).count, 0) + } + + func testMissingUpdatedAtIsIgnored() throws { + var dict = remoteContactDict(id: UUID(), first: "NoTimestamp", updatedAt: t2) + dict.removeValue(forKey: "updatedAt") + XCTAssertNoThrow(try ContactSyncApply.applyRemoteContact(dict, to: context)) + XCTAssertEqual(try context.fetch(FetchDescriptor()).count, 0) + } + + func testIdempotentReapplyIsNoOp() throws { + let id = UUID() + let dict = remoteContactDict(id: id, first: "Once", updatedAt: t2) + try ContactSyncApply.applyRemoteContact(dict, to: context) + try ContactSyncApply.applyRemoteContact(dict, to: context) // re-apply same payload + XCTAssertEqual(try context.fetch(FetchDescriptor()).count, 1, "UUID upsert must not duplicate") + } + + // MARK: - applyRemoteInteraction + + func testRemoteInteractionLinksToExistingContact() throws { + let contact = TestHelpers.makeContact(firstName: "Linked", in: context) + try context.save() + + let interactionId = UUID() + let tmp = Interaction(contact: contact, type: .text, date: t1) + tmp.id = interactionId + tmp.updatedAt = t2 + tmp.summary = "hello" + let dict = ModelSyncApply.interactionToDict(tmp) + + try ModelSyncApply.applyRemoteInteraction(dict, to: context) + + let saved = try XCTUnwrap(try fetchInteraction(interactionId)) + XCTAssertEqual(saved.contact?.id, contact.id, "interaction should link to the resolved contact") + XCTAssertEqual(saved.summary, "hello") + XCTAssertEqual(saved.type, .text) + } + + func testRemoteInteractionWithUnknownContactIsInsertedWithNilContact() throws { + // A child record whose contactId isn't present locally must still persist (contact = nil), + // not crash or get dropped — it heals when the contact arrives on a later pull. + let interactionId = UUID() + let tmp = Interaction(contact: Contact(firstName: "Ghost", lastName: "X"), type: .call, date: t1) + tmp.id = interactionId + tmp.updatedAt = t2 + var dict = ModelSyncApply.interactionToDict(tmp) + dict["contactId"] = UUID().uuidString // a contact id that doesn't exist in this context + + try ModelSyncApply.applyRemoteInteraction(dict, to: context) + + let saved = try XCTUnwrap(try fetchInteraction(interactionId)) + XCTAssertNil(saved.contact, "unresolved contact id yields a nil-contact interaction, not a crash") + } + + func testRemoteInteractionConflictGuardProtectsNewerPendingLocal() throws { + let contact = TestHelpers.makeContact(firstName: "C", in: context) + let local = Interaction(contact: contact, type: .text, date: t1) + local.id = UUID() + local.summary = "local-newer" + local.updatedAt = t2 + local.syncStatus = SyncStatus.pending.rawValue + context.insert(local) + try context.save() + + let stale = Interaction(contact: contact, type: .text, date: t1) + stale.id = local.id + stale.summary = "stale-remote" + stale.updatedAt = t1 + let dict = ModelSyncApply.interactionToDict(stale) + + try ModelSyncApply.applyRemoteInteraction(dict, to: context) + + XCTAssertEqual(local.summary, "local-newer", "newer pending local interaction must survive a stale remote") + } +}