diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift index aaf8568b..55ad5105 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudContainer.swift @@ -48,9 +48,9 @@ : sharedCloudDatabase let rootRecord: CKRecord? = database.state.withValue { - $0.storage[share.recordID.zoneID]?.records.values.first { record in - record.share?.recordID == share.recordID - } + $0.storage[share.recordID.zoneID]?.entries.values.first { entry in + entry.record.share?.recordID == share.recordID + }?.record } return ShareMetadata( diff --git a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift index ae093dd5..293b4326 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockCloudDatabase.swift @@ -18,6 +18,20 @@ lastRecordChangeTag += 1 return lastRecordChangeTag } + + mutating func saveRecord(_ record: CKRecord) { + guard var existingEntry = storage[record.recordID.zoneID]?.entries[record.recordID] + else { + storage[record.recordID.zoneID]?.entries[record.recordID] = + RecordEntry(record: record, history: [:]) + return + } + if let existingRecordChangeTag = existingEntry.record._recordChangeTag { + existingEntry.history[existingRecordChangeTag] = existingEntry.record.copy() as? CKRecord + } + existingEntry.record = record + storage[record.recordID.zoneID]?.entries[record.recordID] = existingEntry + } } struct AssetID: Hashable { @@ -27,7 +41,12 @@ package struct Zone { package var zone: CKRecordZone - package var records: [CKRecord.ID: CKRecord] = [:] + package var entries: [CKRecord.ID: RecordEntry] = [:] + } + + package struct RecordEntry { + package var record: CKRecord + package var history: [Int: CKRecord] } package init(databaseScope: CKDatabase.Scope) { @@ -49,7 +68,7 @@ let record = try state.withValue { state in guard let zone = state.storage[recordID.zoneID] else { throw CKError(.zoneNotFound) } - guard let record = zone.records[recordID] + guard let record = zone.entries[recordID]?.record else { throw CKError(.unknownItem) } guard let record = record.copy() as? CKRecord else { fatalError("Could not copy CKRecord.") } @@ -118,7 +137,7 @@ $0.share?.recordID == share.recordID }) let shareWasPreviouslySaved = - state.storage[share.recordID.zoneID]?.records[share.recordID] != nil + state.storage[share.recordID.zoneID]?.entries[share.recordID] != nil guard shareWasPreviouslySaved || isSavingRootRecord else { saveResults[recordToSave.recordID] = .failure(CKError(.invalidArguments)) @@ -141,14 +160,14 @@ continue } - let existingRecord = state.storage[recordToSave.recordID.zoneID]?.records[ + let existingRecord = state.storage[recordToSave.recordID.zoneID]?.entries[ recordToSave.recordID - ] + ]?.record func saveRecordToDatabase() { let hasReferenceViolation = recordToSave.parent.map { parent in - state.storage[parent.recordID.zoneID]?.records[parent.recordID] == nil + state.storage[parent.recordID.zoneID]?.entries[parent.recordID] == nil && !recordsToSave.contains { $0.recordID == parent.recordID } } ?? false @@ -161,12 +180,12 @@ func root(of record: CKRecord) -> CKRecord { guard let parent = record.parent else { return record } - return (state.storage[parent.recordID.zoneID]?.records[parent.recordID]).map( - root - ) ?? record + return (state.storage[parent.recordID.zoneID]?.entries[parent.recordID]?.record) + .map(root) ?? record } func share(for rootRecord: CKRecord) -> CKShare? { - for (_, record) in state.storage[rootRecord.recordID.zoneID]?.records ?? [:] { + for (_, entry) in state.storage[rootRecord.recordID.zoneID]?.entries ?? [:] { + let record = entry.record guard record.recordID == rootRecord.share?.recordID else { continue } return record as? CKShare @@ -199,7 +218,7 @@ } // TODO: This should merge copy's values to more accurately reflect reality - state.storage[recordToSave.recordID.zoneID]?.records[recordToSave.recordID] = copy + state.saveRecord(copy) saveResults[recordToSave.recordID] = .success(copy) // NB: "Touch" parent records when saving a child: @@ -207,12 +226,12 @@ // If the parent isn't also being saved in this batch. !recordsToSave.contains(where: { $0.recordID == parent.recordID }), // And if the parent is in the database. - let parentRecord = state.storage[parent.recordID.zoneID]?.records[parent.recordID]? - .copy() - as? CKRecord + let parentRecord = state.storage[parent.recordID.zoneID]?.entries[parent.recordID]? + .record + .copy() as? CKRecord { parentRecord._recordChangeTag = state.nextRecordChangeTag() - state.storage[parent.recordID.zoneID]?.records[parent.recordID] = parentRecord + state.saveRecord(parentRecord) } } @@ -225,12 +244,17 @@ precondition(existingRecord._recordChangeTag != nil) saveRecordToDatabase() } else { + let ancestorRecord = + state.storage[recordToSave.recordID.zoneID]?.entries[recordToSave.recordID]? + .history[recordToSaveChangeTag] + ?? (existingRecord.copy() as? CKRecord ?? existingRecord) saveResults[recordToSave.recordID] = .failure( CKError( .serverRecordChanged, userInfo: [ CKRecordChangedErrorServerRecordKey: existingRecord.copy() as Any, CKRecordChangedErrorClientRecordKey: recordToSave.copy(), + CKRecordChangedErrorAncestorRecordKey: ancestorRecord as Any, ] ) ) @@ -271,8 +295,8 @@ continue } let hasReferenceViolation = !Set( - state.storage[recordIDToDelete.zoneID]?.records.values - .compactMap { $0.parent?.recordID == recordIDToDelete ? $0.recordID : nil } + state.storage[recordIDToDelete.zoneID]?.entries.values + .compactMap { $0.record.parent?.recordID == recordIDToDelete ? $0.record.recordID : nil } ?? [] ) .subtracting(recordIDsToDelete) @@ -283,8 +307,9 @@ deleteResults[recordIDToDelete] = .failure(CKError(.referenceViolation)) continue } - let recordToDelete = state.storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] - state.storage[recordIDToDelete.zoneID]?.records[recordIDToDelete] = nil + let recordToDelete = state.storage[recordIDToDelete.zoneID]?.entries[recordIDToDelete]? + .record + state.storage[recordIDToDelete.zoneID]?.entries[recordIDToDelete] = nil deleteResults[recordIDToDelete] = .success(()) if let recordType = recordToDelete?.recordType { state.deletedRecords.append((recordIDToDelete, recordType)) @@ -297,18 +322,19 @@ shareToDelete.recordID.zoneID.ownerName == CKCurrentUserDefaultName { func deleteRecords(referencing recordID: CKRecord.ID) { - for recordToDelete in (state.storage[recordIDToDelete.zoneID]?.records ?? [:]).values + for entryToDelete in (state.storage[recordIDToDelete.zoneID]?.entries ?? [:]).values { + let record = entryToDelete.record guard - recordToDelete.share?.recordID == recordID - || recordToDelete.parent?.recordID == recordID + record.share?.recordID == recordID + || record.parent?.recordID == recordID else { continue } - state.storage[recordIDToDelete.zoneID]?.records[recordToDelete.recordID] = nil - deleteResults[recordToDelete.recordID] = .success(()) - state.deletedRecords.append((recordIDToDelete, recordToDelete.recordType)) - deleteRecords(referencing: recordToDelete.recordID) + state.storage[recordIDToDelete.zoneID]?.entries[record.recordID] = nil + deleteResults[record.recordID] = .success(()) + state.deletedRecords.append((recordIDToDelete, record.recordType)) + deleteRecords(referencing: record.recordID) } } deleteRecords(referencing: shareToDelete.recordID) @@ -348,7 +374,7 @@ deleteResults[deleteSuccessRecordID] = .failure(CKError(.batchRequestFailed)) } // All storage changes are reverted in zone. - state.storage[zoneID]?.records = previousStorage[zoneID]?.records ?? [:] + state.storage[zoneID]?.entries = previousStorage[zoneID]?.entries ?? [:] } return (saveResults: saveResults, deleteResults: deleteResults) } diff --git a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift index 9a2cc088..c75f3f3a 100644 --- a/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/Internal/MockSyncEngine.swift @@ -47,7 +47,7 @@ zoneIDs.reduce(into: [CKRecord]()) { accum, zoneID in - accum += ((state.storage[zoneID]?.records.values).map { Array($0) } ?? []) + accum += (state.storage[zoneID]?.entries.values.map(\.record) ?? []) .filter { precondition( $0._recordChangeTag != nil, @@ -201,7 +201,6 @@ extension SyncEngine { package struct SendRecordsCallback { fileprivate let operation: @Sendable () async -> Void - @discardableResult package func receive() async { await operation() } diff --git a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift index 9596a66e..f0e67dd1 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/ForeignKeyConstraintTests.swift @@ -672,9 +672,9 @@ """ } assertInlineSnapshot( - of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.records[ + of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.entries[ Reminder.recordID(for: 1) - ], + ]?.record, as: .customDump ) { """ @@ -767,9 +767,9 @@ """ } assertInlineSnapshot( - of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.records[ + of: syncEngine.private.database.state.storage[syncEngine.defaultZone.zoneID]?.entries[ Reminder.recordID(for: 1) - ], + ]?.record, as: .customDump ) { """ diff --git a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift index ecd50cc2..885ce137 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MockCloudDatabaseTests.swift @@ -650,6 +650,75 @@ } } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func serverRecordChangedIncludesAncestorRecord() async throws { + let recordID = CKRecord.ID(recordName: "1") + let record = CKRecord(recordType: "A", recordID: recordID) + record["value"] = 1 + + let (saveResults, _) = try syncEngine.private.database.modifyRecords(saving: [record]) + #expect(saveResults.values.count(where: { (try? $0.get()) != nil }) == 1) + + let clientRecord = try syncEngine.private.database.record(for: recordID) + + let serverRecord = try syncEngine.private.database.record(for: recordID) + serverRecord["value"] = 2 + _ = try syncEngine.private.database.modifyRecords(saving: [serverRecord]) + + clientRecord["value"] = 3 + let (conflictResults, _) = try syncEngine.private.database.modifyRecords( + saving: [clientRecord] + ) + let error = #expect(throws: CKError.self) { + try conflictResults[recordID]?.get() + } + #expect(error?.code == .serverRecordChanged) + + let ancestor = error?.userInfo[CKRecordChangedErrorAncestorRecordKey] as? CKRecord + let server = error?.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord + let client = error?.userInfo[CKRecordChangedErrorClientRecordKey] as? CKRecord + #expect(ancestor?["value"] as? Int == 1) + #expect(server?["value"] as? Int == 2) + #expect(client?["value"] as? Int == 3) + } + + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) + @Test func serverRecordChangedUsesChangeTagAncestor() async throws { + let recordID = CKRecord.ID(recordName: "1") + let record = CKRecord(recordType: "A", recordID: recordID) + record["value"] = 1 + + _ = try syncEngine.private.database.modifyRecords(saving: [record]) + + let clientRecord = try syncEngine.private.database.record(for: recordID) + let clientChangeTag = clientRecord._recordChangeTag + + let serverRecordV2 = try syncEngine.private.database.record(for: recordID) + serverRecordV2["value"] = 2 + _ = try syncEngine.private.database.modifyRecords(saving: [serverRecordV2]) + + let serverRecordV3 = try syncEngine.private.database.record(for: recordID) + serverRecordV3["value"] = 3 + _ = try syncEngine.private.database.modifyRecords(saving: [serverRecordV3]) + + clientRecord["value"] = 99 + let (conflictResults, _) = try syncEngine.private.database.modifyRecords( + saving: [clientRecord] + ) + let error = #expect(throws: CKError.self) { + try conflictResults[recordID]?.get() + } + #expect(error?.code == .serverRecordChanged) + + let ancestor = error?.userInfo[CKRecordChangedErrorAncestorRecordKey] as? CKRecord + let server = error?.userInfo[CKRecordChangedErrorServerRecordKey] as? CKRecord + let client = error?.userInfo[CKRecordChangedErrorClientRecordKey] as? CKRecord + #expect(ancestor?._recordChangeTag == clientChangeTag) + #expect(ancestor?["value"] as? Int == 1) + #expect(server?["value"] as? Int == 3) + #expect(client?["value"] as? Int == 99) + } + @Test func limitExceeded_modifyRecords() async throws { let remindersListRecord = CKRecord( recordType: RemindersList.tableName, diff --git a/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift b/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift index 338db9df..eee8dfd6 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKit+CustomDump.swift @@ -245,7 +245,7 @@ "storage": state .value .storage - .flatMap { _, value in value.records.values } + .flatMap { _, value in value.entries.values.map(\.record) } .sorted { ($0.recordType, $0.recordID.recordName) < ($1.recordType, $1.recordID.recordName) }, diff --git a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift index 4716be7b..f40231ba 100644 --- a/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift +++ b/Tests/SQLiteDataTests/Internal/CloudKitTestHelpers.swift @@ -77,7 +77,7 @@ extension SyncEngine { let recordsToDeleteByID = Dictionary( grouping: syncEngine.database.state.withValue { state in recordIDsToDelete.compactMap { - recordID in state.storage[recordID.zoneID]?.records[recordID] + recordID in state.storage[recordID.zoneID]?.entries[recordID]?.record } }, by: \.recordID