From 7bb4bbcc125cb1cd93b635472334793f8cfda45a Mon Sep 17 00:00:00 2001 From: Lukas Kubanek Date: Fri, 24 Jul 2026 11:25:12 +0200 Subject: [PATCH 1/3] Deliver latest record state in ModifyRecordsCallback test helper --- .../ModifyRecordsCallbackTests.swift | 68 +++++++++++++++++++ .../Internal/CloudKitTestHelpers.swift | 18 +++-- 2 files changed, 80 insertions(+), 6 deletions(-) create mode 100644 Tests/SQLiteDataTests/CloudKitTests/ModifyRecordsCallbackTests.swift diff --git a/Tests/SQLiteDataTests/CloudKitTests/ModifyRecordsCallbackTests.swift b/Tests/SQLiteDataTests/CloudKitTests/ModifyRecordsCallbackTests.swift new file mode 100644 index 00000000..8f34118b --- /dev/null +++ b/Tests/SQLiteDataTests/CloudKitTests/ModifyRecordsCallbackTests.swift @@ -0,0 +1,68 @@ +#if canImport(CloudKit) + import CloudKit + import SQLiteData + import Testing + + extension BaseCloudKitTests { + @MainActor + final class ModifyRecordsCallbackTests: BaseCloudKitTests, @unchecked Sendable { + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func deferredCallbackDeliversLatestRecords() async throws { + try await userDatabase.userWrite { db in + try db.seed { RemindersList(id: 1, title: "Original") } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let firstRecord = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + firstRecord.setValue("First", forKey: "title", at: 30) + let callback = try syncEngine.modifyRecords(scope: .private, saving: [firstRecord]) + + let secondRecord = try syncEngine.private.database.record( + for: RemindersList.recordID(for: 1) + ) + secondRecord.setValue("Second", forKey: "title", at: 60) + _ = try syncEngine.modifyRecords(scope: .private, saving: [secondRecord]) + + await callback.notify() + + let title = try await userDatabase.database.read { db in + try RemindersList.find(1).select(\.title).fetchOne(db) + } + #expect(title == "Second") + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func deferredCallbackSkipsDeletionOfReSavedRecord() async throws { + try await userDatabase.userWrite { db in + try db.seed { RemindersList(id: 1, title: "Original") } + } + try await syncEngine.processPendingRecordZoneChanges(scope: .private) + + let recordID = RemindersList.recordID(for: 1) + + let callback = try syncEngine.modifyRecords( + scope: .private, + deleting: [recordID] + ) + + let revivedRecord = CKRecord( + recordType: RemindersList.tableName, + recordID: recordID + ) + revivedRecord.setValue("Revived", forKey: "title", at: 30) + _ = try syncEngine.modifyRecords(scope: .private, saving: [revivedRecord]) + + await callback.notify() + + let title = try await userDatabase.database.read { db in + try RemindersList.find(1).select(\.title).fetchOne(db) + } + // NB: The deletion is skipped and the revival's callback isn't notified at this point, + // so the local record stays untouched. + #expect(title == "Original") + } + } + } +#endif diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index 4716be7b..a71ad3e2 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -93,13 +93,19 @@ extension SyncEngine { return ModifyRecordsCallback { await syncEngine.parentSyncEngine.handleEvent( .fetchedRecordZoneChanges( - modifications: saveResults.values.compactMap { try? $0.get() }, + modifications: saveResults.compactMap { recordID, result in + guard case .success = result else { return nil } + // NB: Read the latest version from the database in case the record has been + // modified between the original save and when this callback is invoked. + return try? syncEngine.database.record(for: recordID) + }, deletions: deleteResults.compactMap { recordID, result in - (recordsToDeleteByID[recordID]?.recordType).flatMap { recordType in - (try? result.get()) != nil - ? (recordID, recordType) - : nil - } + guard case .success = result else { return nil } + // NB: Skip if the record has been re-saved between the original delete and + // when this callback is invoked. + guard (try? syncEngine.database.record(for: recordID)) == nil else { return nil } + guard let record = recordsToDeleteByID[recordID] else { return nil } + return (recordID, record.recordType) } ), syncEngine: syncEngine From 6001f5f5a4e3309fdcc691a47250d24225ba6f1b Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 24 Jul 2026 11:30:05 -0500 Subject: [PATCH 2/3] Remove the ReturnValue generic from ModifyRecordsCallback. It's not used anywhere. --- .../Internal/CloudKitTestHelpers.swift | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index a71ad3e2..fac238b7 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -20,10 +20,9 @@ extension PrimaryKeyedTable where PrimaryKey.QueryOutput: IdentifierStringConver @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) extension SyncEngine { - struct ModifyRecordsCallback { - fileprivate let operation: @Sendable () async -> ReturnValue - @discardableResult - func notify() async -> ReturnValue { + struct ModifyRecordsCallback { + fileprivate let operation: @Sendable () async -> Void + func notify() async { await operation() } } @@ -32,12 +31,7 @@ extension SyncEngine { scope: CKDatabase.Scope, saving recordZonesToSave: [CKRecordZone] = [], deleting recordZoneIDsToDelete: [CKRecordZone.ID] = [] - ) throws -> ModifyRecordsCallback< - ( - saveResults: [CKRecordZone.ID: Result], - deleteResults: [CKRecordZone.ID: Result] - ) - > { + ) throws -> ModifyRecordsCallback { let syncEngine = syncEngine(for: scope) let (saveResults, deleteResults) = try syncEngine.database.modifyRecordZones( @@ -58,7 +52,6 @@ extension SyncEngine { ), syncEngine: syncEngine ) - return (saveResults, deleteResults) } } @@ -67,12 +60,7 @@ extension SyncEngine { saving recordsToSave: [CKRecord] = [], deleting recordIDsToDelete: [CKRecord.ID] = [], atomically: Bool = true - ) throws -> ModifyRecordsCallback< - ( - saveResults: [CKRecord.ID: Result], - deleteResults: [CKRecord.ID: Result] - ) - > { + ) throws -> ModifyRecordsCallback { let syncEngine = syncEngine(for: scope) let recordsToDeleteByID = Dictionary( grouping: syncEngine.database.state.withValue { state in @@ -110,7 +98,6 @@ extension SyncEngine { ), syncEngine: syncEngine ) - return (saveResults, deleteResults) } } } From ab9c78486a194534b1d29f9f259facc9a0e9ce0e Mon Sep 17 00:00:00 2001 From: Brandon Williams Date: Fri, 24 Jul 2026 13:38:07 -0500 Subject: [PATCH 3/3] Fetch all records at once instead of individually. --- .../Internal/CloudKitTestHelpers.swift | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index fac238b7..9228bfe2 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -79,19 +79,24 @@ extension SyncEngine { ) return ModifyRecordsCallback { + let savedRecordIDs = saveResults.compactMap { recordID, result in + (try? result.get()) != nil ? recordID : nil + } + let deletedRecordIDs = deleteResults.compactMap { recordID, result in + (try? result.get()) != nil ? recordID : nil + } + let freshRecords = + (try? syncEngine.database.records( + for: savedRecordIDs + deletedRecordIDs, + desiredKeys: nil + )) ?? [:] await syncEngine.parentSyncEngine.handleEvent( .fetchedRecordZoneChanges( - modifications: saveResults.compactMap { recordID, result in - guard case .success = result else { return nil } - // NB: Read the latest version from the database in case the record has been - // modified between the original save and when this callback is invoked. - return try? syncEngine.database.record(for: recordID) + modifications: savedRecordIDs.compactMap { recordID in + try? freshRecords[recordID]?.get() }, - deletions: deleteResults.compactMap { recordID, result in - guard case .success = result else { return nil } - // NB: Skip if the record has been re-saved between the original delete and - // when this callback is invoked. - guard (try? syncEngine.database.record(for: recordID)) == nil else { return nil } + deletions: deletedRecordIDs.compactMap { recordID in + guard (try? freshRecords[recordID]?.get()) == nil else { return nil } guard let record = recordsToDeleteByID[recordID] else { return nil } return (recordID, record.recordType) }