diff --git a/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift b/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift index d2f0726e..dd9abee3 100644 --- a/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift +++ b/Sources/SQLiteData/CloudKit/CloudKit+StructuredQueries.swift @@ -156,10 +156,13 @@ package func setValue( _ newValue: some CKRecordValueProtocol & Equatable, forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { guard - encryptedValues[at: key] <= userModificationTime, + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime, encryptedValues[key] != newValue else { return false } encryptedValues[key] = newValue @@ -172,7 +175,8 @@ package func setAsset( _ newValue: CKAsset, forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { @Dependency(\.dataManager) var dataManager guard @@ -180,7 +184,9 @@ let hash = dataManager.sha256(of: fileURL) else { return false } guard - encryptedValues[at: key] <= userModificationTime, + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime, encryptedValues[hash: key] != hash else { return false } @@ -223,9 +229,13 @@ @discardableResult package func removeValue( forKey key: CKRecord.FieldKey, - at userModificationTime: Int64 + at userModificationTime: Int64, + requireStrictlyNewer: Bool = false ) -> Bool { - guard encryptedValues[at: key] <= userModificationTime + guard + requireStrictlyNewer + ? encryptedValues[at: key] < userModificationTime + : encryptedValues[at: key] <= userModificationTime else { return false } @@ -283,26 +293,42 @@ } } + @discardableResult func update( with other: CKRecord, row: T, columnNames: inout [String], parentForeignKey: ForeignKey? - ) { + ) -> Bool { typealias EquatableCKRecordValueProtocol = CKRecordValueProtocol & Equatable self.userModificationTime = other.userModificationTime + var didPreserveLocalValues = false for column in T.TableColumns.writableColumns { func open(_ column: some WritableTableColumnExpression) { let key = column.name let keyPath = column.keyPath as! KeyPath let didSet: Bool if let value = other[key] as? CKAsset { - didSet = setAsset(value, forKey: key, at: other.encryptedValues[at: key]) + didSet = setAsset( + value, + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else if let value = other.encryptedValues[key] as? any EquatableCKRecordValueProtocol { - didSet = setValue(value, forKey: key, at: other.encryptedValues[at: key]) + didSet = setValue( + value, + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else if other.encryptedValues[key] == nil { - didSet = removeValue(forKey: key, at: other.encryptedValues[at: key]) + didSet = removeValue( + forKey: key, + at: other.encryptedValues[at: key], + requireStrictlyNewer: true + ) } else { didSet = false } @@ -334,13 +360,17 @@ } if didSet || isRowValueModified { columnNames.removeAll(where: { $0 == key }) - if didSet, let parentForeignKey, key == parentForeignKey.from { - self.parent = other.parent + if didSet { + didPreserveLocalValues = true + if let parentForeignKey, key == parentForeignKey.from { + self.parent = other.parent + } } } } open(column) } + return didPreserveLocalValues } package var userModificationTime: Int64 { diff --git a/Sources/SQLiteData/CloudKit/SyncEngine.swift b/Sources/SQLiteData/CloudKit/SyncEngine.swift index ddcb3f32..6897ec77 100644 --- a/Sources/SQLiteData/CloudKit/SyncEngine.swift +++ b/Sources/SQLiteData/CloudKit/SyncEngine.swift @@ -1591,24 +1591,33 @@ case share(CKShare) case reference(CKShare.Reference) } - let shares: [ShareOrReference] = + let (shares, preservedRecordIDs): ([ShareOrReference], [CKRecord.ID]) = await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in var shares: [ShareOrReference] = [] + var preservedRecordIDs: [CKRecord.ID] = [] for record in modifications { if let share = record as? CKShare { shares.append(.share(share)) } else { - upsertFromServerRecord(record, db: db) + if upsertFromServerRecord(record, db: db) { + preservedRecordIDs.append(record.recordID) + } if let shareReference = record.share { shares.append(.reference(shareReference)) } } } - return shares + return (shares, preservedRecordIDs) } } - ?? [] + ?? ([], []) + + if !preservedRecordIDs.isEmpty { + syncEngine.state.add( + pendingRecordZoneChanges: preservedRecordIDs.map { .saveRecord($0) } + ) + } await withTaskGroup(of: Void.self) { group in for share in shares { @@ -1897,28 +1906,31 @@ } } + @discardableResult private func upsertFromServerRecord( _ serverRecord: CKRecord, force: Bool = false - ) async { + ) async -> Bool { await withErrorReporting(.sqliteDataCloudKitFailure) { try await userDatabase.write { db in upsertFromServerRecord(serverRecord, force: force, db: db) } } + ?? false } + @discardableResult private func upsertFromServerRecord( _ serverRecord: CKRecord, force: Bool = false, db: Database - ) { + ) -> Bool { withErrorReporting(.sqliteDataCloudKitFailure) { guard let recordPrimaryKey = serverRecord.recordID.recordPrimaryKey, serverRecord.encryptedValues[CKRecord.userModificationTimeKey] != nil else { - return + return false } try SyncMetadata.insert { @@ -1950,18 +1962,19 @@ let metadata = try SyncMetadata.find(serverRecord.recordID).fetchOne(db), let table = tablesByName[serverRecord.recordType] else { - return + return false } serverRecord.userModificationTime = metadata.userModificationTime - func open(_ table: some SynchronizableTable) throws { + func open(_ table: some SynchronizableTable) throws -> Bool { var columnNames: [String] = T.TableColumns.writableColumns.map(\.name) + var didPreserveLocalValues = false if !force, let allFields = metadata._lastKnownServerRecordAllFields, let row = try T.unscoped.find(#sql("\(bind: metadata.recordPrimaryKey)")).fetchOne(db) { - serverRecord.update( + didPreserveLocalValues = serverRecord.update( with: allFields, row: T(queryOutput: row), columnNames: &columnNames, @@ -1994,9 +2007,25 @@ } .execute(db) } + if didPreserveLocalValues, let lastKnownServerRecord = metadata.lastKnownServerRecord { + if let lastKnownDate = lastKnownServerRecord.modificationDate, + let serverDate = serverRecord.modificationDate + { + if serverDate < lastKnownDate { + didPreserveLocalValues = false + } + } else if let lastKnownTag = lastKnownServerRecord._recordChangeTag, + let serverTag = serverRecord._recordChangeTag, + serverTag < lastKnownTag + { + didPreserveLocalValues = false + } + } + return didPreserveLocalValues } - try open(table) + return try open(table) } + ?? false } private func refreshLastKnownServerRecord(_ record: CKRecord) async { diff --git a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift index b9840148..df443f8f 100644 --- a/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift +++ b/Tests/SQLiteDataTests/CloudKitTests/MergeConflictTests.swift @@ -637,6 +637,170 @@ } } + @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) + let modificationCallback = try { + try syncEngine.modifyRecords(scope: .private, saving: [record]) + }() + await modificationCallback.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: [] + ) + ) + """ + } + } + @available(iOS 17, macOS 14, tvOS 17, watchOS 10, *) @Test func mergeWithNullableFields() async throws { try await userDatabase.userWrite { db in