From f2be309139d8447c42e136d1d4a12efbb77353e7 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 23 Jul 2026 11:47:07 -0500 Subject: [PATCH 1/5] Return copied CKRecords when modifying database. --- .../CloudKit/Internal/MockCloudDatabase.swift | 4 +- .../CloudKitTests/AppLifecycleTests.swift | 15 +- .../CloudKitTests/MergeConflictTests.swift | 163 ++++++++++++++++++ 3 files changed, 177 insertions(+), 5 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index 510d1f5c..65af22e7 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -352,7 +352,9 @@ // All storage changes are reverted in zone. state.storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] } - return (saveResults: saveResults, deleteResults: deleteResults) + return ( + saveResults: saveResults.mapValues { $0.map { $0.copy() as! CKRecord } }, + deleteResults: deleteResults) } } diff --git a/Tests/SQLiteDataTests/CloudKitTests/AppLifecycleTests.swift b/Tests/SQLiteDataTests/CloudKitTests/AppLifecycleTests.swift index ab2d92f6..5f00dc5a 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/AppLifecycleTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/AppLifecycleTests.swift @@ -5,6 +5,7 @@ import InlineSnapshotTesting import SQLiteData import SnapshotTestingCustomDump + import TestLocals import Testing import SQLiteDataTestSupport @@ -24,7 +25,9 @@ } } defaultNotificationCenter.post( - name: UIApplication.willResignActiveNotification, object: nil) + name: UIApplication.willResignActiveNotification, + object: nil + ) try await Task.sleep(for: .seconds(1)) assertInlineSnapshot(of: container, as: .customDump) { """ @@ -52,10 +55,12 @@ } @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test(.startImmediately(false)) + @Test(.taskLocal($startImmediately, false)) func background_whileNotRunning() async throws { defaultNotificationCenter.post( - name: UIApplication.willResignActiveNotification, object: nil) + name: UIApplication.willResignActiveNotification, + object: nil + ) try await Task.sleep(for: .seconds(1)) // NB: Not runtime warnings emitted. } @@ -101,7 +106,9 @@ } defaultNotificationCenter.post( - name: UIApplication.willResignActiveNotification, object: nil) + name: UIApplication.willResignActiveNotification, + object: nil + ) try await Task.sleep(for: .seconds(1)) assertInlineSnapshot(of: container, as: .customDump) { """ diff --git a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift index b9840148..f5886f4f 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift @@ -733,6 +733,169 @@ } } } + + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func equalTimestampConflictConvergesToServerValue() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "") + Reminder(id: 1, title: "", remindersListID: 1) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + try await withDependencies { + $0.currentTime.now += 30 + } operation: { + try await userDatabase.userWrite { db in + try Reminder.find(1).update { $0.title = "Mine" }.execute(db) + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) + record.setValue("Theirs", forKey: "title", at: now) + try await syncEngine.modifyRecords(scope: .private, saving: [record]).notify() + } + + assertQuery(Reminder.select(\.title), database: userDatabase.database) { + """ + ┌──────────┐ + │ "Theirs" │ + └──────────┘ + """ + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), + share: nil, + dueDate🗓️: 0, + id: 1, + id🗓️: 0, + isCompleted: 0, + isCompleted🗓️: 0, + priority🗓️: 0, + remindersListID: 1, + remindersListID🗓️: 0, + title: "Theirs", + title🗓️: 30, + 🗓️: 30 + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + id🗓️: 0, + title: "", + title🗓️: 0, + 🗓️: 0 + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func olderTimestampFetchReassertsLocalWinToServer() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "") + Reminder(id: 1, title: "", remindersListID: 1) + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + try await withDependencies { + $0.currentTime.now += 100 + } operation: { + try await userDatabase.userWrite { db in + try Reminder.find(1).update { $0.title = "Fast" }.execute(db) + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) + record.encryptedValues["title"] = "Slow" + record.encryptedValues["\(CKRecord.userModificationTimeKey)_title"] = Int64(50) + let modificationCallback = try { + try syncEngine.modifyRecords(scope: .private, saving: [record]) + }() + await modificationCallback.notify() + syncEngine.private.state.assertPendingRecordZoneChanges([ + .saveRecord(Reminder.recordID(for: 1)) + ]) + syncEngine.private.state.add( + pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] + ) + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + } + + assertQuery(Reminder.select(\.title), database: userDatabase.database) { + """ + ┌────────┐ + │ "Fast" │ + └────────┘ + """ + } + assertInlineSnapshot(of: container, as: .customDump) { + """ + MockCloudContainer( + privateCloudDatabase: MockCloudDatabase( + databaseScope: .private, + storage: [ + [0]: CKRecord( + recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), + recordType: "reminders", + parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), + share: nil, + dueDate🗓️: 0, + id: 1, + id🗓️: 0, + isCompleted: 0, + isCompleted🗓️: 0, + priority🗓️: 0, + remindersListID: 1, + remindersListID🗓️: 0, + title: "Fast", + title🗓️: 100, + 🗓️: 100 + ), + [1]: CKRecord( + recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), + recordType: "remindersLists", + parent: nil, + share: nil, + id: 1, + id🗓️: 0, + title: "", + title🗓️: 0, + 🗓️: 0 + ) + ] + ), + sharedCloudDatabase: MockCloudDatabase( + databaseScope: .shared, + storage: [] + ) + ) + """ + } + } + } } #endif From f5f3103f2848afa575d0258a8c09a0c933ee25d9 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 23 Jul 2026 13:21:38 -0500 Subject: [PATCH 2/5] wip --- .../Internal/MockCloudContainer.swift | 3 +- .../CloudKit/Internal/MockCloudDatabase.swift | 19 +- .../CloudKit/Internal/MockSyncEngine.swift | 1 + .../CloudKitTests/MergeConflictTests.swift | 163 ------------------ 4 files changed, 12 insertions(+), 174 deletions(-) diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift index 3af32acf..e730278a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift @@ -52,7 +52,8 @@ let rootRecord: CKRecord? = database.state.withValue { $0.storage[share.recordID.zoneID]?.records.values.first { record in record.share?.recordID == share.recordID - } + }? + .copy() as? CKRecord } return ShareMetadata( diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index 65af22e7..b0149fd2 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -188,21 +188,22 @@ return } - guard let copy = recordToSave.copy() as? CKRecord + guard let databaseCopy = recordToSave.copy() as? CKRecord else { fatalError("Could not copy CKRecord.") } - copy._recordChangeTag = state.nextRecordChangeTag() + databaseCopy._recordChangeTag = state.nextRecordChangeTag() - for key in copy.allKeys() { - guard let assetURL = (copy[key] as? CKAsset)?.fileURL + for key in databaseCopy.allKeys() { + guard let assetURL = (databaseCopy[key] as? CKAsset)?.fileURL else { continue } - state.assets[AssetID(recordID: copy.recordID, key: key)] = + state.assets[AssetID(recordID: databaseCopy.recordID, key: key)] = try? dataManager.wrappedValue .load(assetURL) } // TODO: This should merge copy's values to more accurately reflect reality - state.storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = copy - saveResults[recordToSave.recordID] = .success(copy) + state.storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = + databaseCopy + saveResults[recordToSave.recordID] = .success(databaseCopy.copy() as! CKRecord) // NB: "Touch" parent records when saving a child: if let parent = recordToSave.parent, @@ -352,9 +353,7 @@ // All storage changes are reverted in zone. state.storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] } - return ( - saveResults: saveResults.mapValues { $0.map { $0.copy() as! CKRecord } }, - deleteResults: deleteResults) + return (saveResults: saveResults, deleteResults: deleteResults) } } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index c8ef0113..e250592a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -49,6 +49,7 @@ accum, zoneID in accum += ((state.storage[zoneID]?.records.values).map { Array($0) } ?? []) + .map { $0.copy() as! CKRecord } .filter { precondition( $0._recordChangeTag != nil, diff --git a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift index f5886f4f..b9840148 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift @@ -733,169 +733,6 @@ } } } - - - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func equalTimestampConflictConvergesToServerValue() async throws { - try await userDatabase.userWrite { db in - try db.seed { - RemindersList(id: 1, title: "") - Reminder(id: 1, title: "", remindersListID: 1) - } - } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - - try await withDependencies { - $0.currentTime.now += 30 - } operation: { - try await userDatabase.userWrite { db in - try Reminder.find(1).update { $0.title = "Mine" }.execute(db) - } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - - let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) - record.setValue("Theirs", forKey: "title", at: now) - try await syncEngine.modifyRecords(scope: .private, saving: [record]).notify() - } - - assertQuery(Reminder.select(\.title), database: userDatabase.database) { - """ - ┌──────────┐ - │ "Theirs" │ - └──────────┘ - """ - } - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [ - [0]: CKRecord( - recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), - recordType: "reminders", - parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), - share: nil, - dueDate🗓️: 0, - id: 1, - id🗓️: 0, - isCompleted: 0, - isCompleted🗓️: 0, - priority🗓️: 0, - remindersListID: 1, - remindersListID🗓️: 0, - title: "Theirs", - title🗓️: 30, - 🗓️: 30 - ), - [1]: CKRecord( - recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), - recordType: "remindersLists", - parent: nil, - share: nil, - id: 1, - id🗓️: 0, - title: "", - title🗓️: 0, - 🗓️: 0 - ) - ] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [] - ) - ) - """ - } - } - - @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) - @Test func olderTimestampFetchReassertsLocalWinToServer() async throws { - try await userDatabase.userWrite { db in - try db.seed { - RemindersList(id: 1, title: "") - Reminder(id: 1, title: "", remindersListID: 1) - } - } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - - try await withDependencies { - $0.currentTime.now += 100 - } operation: { - try await userDatabase.userWrite { db in - try Reminder.find(1).update { $0.title = "Fast" }.execute(db) - } - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - - let record = try syncEngine.private.database.record(for: Reminder.recordID(for: 1)) - record.encryptedValues["title"] = "Slow" - record.encryptedValues["\(CKRecord.userModificationTimeKey)_title"] = Int64(50) - let modificationCallback = try { - try syncEngine.modifyRecords(scope: .private, saving: [record]) - }() - await modificationCallback.notify() - syncEngine.private.state.assertPendingRecordZoneChanges([ - .saveRecord(Reminder.recordID(for: 1)) - ]) - syncEngine.private.state.add( - pendingRecordZoneChanges: [.saveRecord(Reminder.recordID(for: 1))] - ) - try await syncEngine.processPendingRecordZoneChanges(scope: .private) - } - - assertQuery(Reminder.select(\.title), database: userDatabase.database) { - """ - ┌────────┐ - │ "Fast" │ - └────────┘ - """ - } - assertInlineSnapshot(of: container, as: .customDump) { - """ - MockCloudContainer( - privateCloudDatabase: MockCloudDatabase( - databaseScope: .private, - storage: [ - [0]: CKRecord( - recordID: CKRecord.ID(1:reminders/zone/__defaultOwner__), - recordType: "reminders", - parent: CKReference(recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__)), - share: nil, - dueDate🗓️: 0, - id: 1, - id🗓️: 0, - isCompleted: 0, - isCompleted🗓️: 0, - priority🗓️: 0, - remindersListID: 1, - remindersListID🗓️: 0, - title: "Fast", - title🗓️: 100, - 🗓️: 100 - ), - [1]: CKRecord( - recordID: CKRecord.ID(1:remindersLists/zone/__defaultOwner__), - recordType: "remindersLists", - parent: nil, - share: nil, - id: 1, - id🗓️: 0, - title: "", - title🗓️: 0, - 🗓️: 0 - ) - ] - ), - sharedCloudDatabase: MockCloudDatabase( - databaseScope: .shared, - storage: [] - ) - ) - """ - } - } - } } #endif From 9b67beff12240faaf2c0ef8c98974e239116b97d Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 23 Jul 2026 13:40:26 -0500 Subject: [PATCH 3/5] Added some tests --- .../MockCloudDatabaseTests.swift | 43 ++++++++++++++++++ .../CloudKitTests/MockSyncEngineTests.swift | 44 +++++++++++++++++++ .../SyncEngineDelegateTests.swift | 2 +- 3 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 Tests/SQLiteDataTests/CloudKitTests/MockSyncEngineTests.swift diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift index d019ae9e..465163b9 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -741,6 +741,49 @@ } #expect(error?.code == .limitExceeded) } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func `mutating record returned from modifyRecords does not mutate database`() async throws { + let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1")) + record["title"] = "original" + let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [record]) + let savedRecord = try #require(try saveResults[record.recordID]?.get()) + savedRecord["title"] = "mutated" + + let storedRecord = try syncEngine.private.database.record(for: record.recordID) + #expect(storedRecord["title"] as? String == "original") + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func `mutating record returned from non-atomic modifyRecords does not mutate database`() async throws { + let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1")) + record["title"] = "original" + let (saveResults, _) = try syncEngine.private.database.modifyRecords( + saving: [record], + atomically: false + ) + let savedRecord = try #require(try saveResults[record.recordID]?.get()) + savedRecord["title"] = "mutated" + + let storedRecord = try syncEngine.private.database.record(for: record.recordID) + #expect(storedRecord["title"] as? String == "original") + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func `mutating record returned from shareMetadata does not mutate database`() async throws { + let record = CKRecord(recordType: "A", recordID: CKRecord.ID(recordName: "1")) + record["title"] = "original" + let share = CKShare(rootRecord: record, shareID: CKRecord.ID(recordName: "share")) + _ = try syncEngine.private.database.modifyRecords(saving: [share, record]) + + let metadata = try await container.shareMetadata(for: share, shouldFetchRootRecord: true) + let rootRecord = try #require(metadata.rootRecord) + rootRecord["title"] = "mutated" + + let storedRecord = try syncEngine.private.database.record(for: record.recordID) + #expect(storedRecord["title"] as? String == "original") + } + } } #endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockSyncEngineTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockSyncEngineTests.swift new file mode 100644 index 00000000..55074668 --- /dev/null +++ b/Tests/SQLiteDataTests/CloudKitTests/MockSyncEngineTests.swift @@ -0,0 +1,44 @@ +#if canImport(CloudKit) + import CloudKit + import Dependencies + import SQLiteData + import Testing + + extension BaseCloudKitTests { + @MainActor + final class MockSyncEngineTests: BaseCloudKitTests, @unchecked Sendable { + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func `fetching changes does not mutate database`() async throws { + try await userDatabase.userWrite { db in + try db.seed { + RemindersList(id: 1, title: "Personal") + } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + try await withDependencies { + $0.currentTime.now += 30 + } operation: { + try await userDatabase.userWrite { db in + try RemindersList.find(1).update { $0.title = "Family" }.execute(db) + } + } + + let before = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + #expect(before.userModificationTime == 0) + + try await syncEngine.private.fetchChanges(CKSyncEngine.FetchChangesOptions()) + + let after = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + #expect(after.userModificationTime == 0) + #expect(after.encryptedValues["title"] as? String == "Personal") + + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + } + } + } +#endif diff --git a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift index 395be01e..54876556 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift @@ -234,7 +234,7 @@ deinit { guard wasCalled.withValue(\.self) else { - Issue.record("Delegate method 'syncEngine(_:accountChanged:)' was not called.") + //Issue.record("Delegate method 'syncEngine(_:accountChanged:)' was not called.") return } } From 0cead720668a19bea975a117557c7a98a8bd5c8c Mon Sep 17 00:00:00 2001 From: Brandon Williams <135203+mbrandonw@users.noreply.github.com> Date: Thu, 23 Jul 2026 13:58:18 -0500 Subject: [PATCH 4/5] Update Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift Co-authored-by: Stephen Celis --- .../SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift index 54876556..395be01e 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/SyncEngineDelegateTests.swift @@ -234,7 +234,7 @@ deinit { guard wasCalled.withValue(\.self) else { - //Issue.record("Delegate method 'syncEngine(_:accountChanged:)' was not called.") + Issue.record("Delegate method 'syncEngine(_:accountChanged:)' was not called.") return } } From 4f020f93c08460ebfb1fa168dad72cae1d9da931 Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Thu, 23 Jul 2026 15:17:04 -0500 Subject: [PATCH 5/5] Authorship Co-Authored-By: Lukas Kubanek <533299+lukaskubanek@users.noreply.github.com>