-
Notifications
You must be signed in to change notification settings - Fork 76
Description
Description
I’ve been trying to wrap my head around the implementation of the built-in “field-wise last edit wins” strategy, and I’m honestly having a hard time. To better understand what’s going on, I started documenting the current behavior in this document. While I still don’t have all the answers, I ran into some unexpected behavior that felt worth reporting, as it might help clarify the intended design.
It traces back to the purpose of the didSet flag in CKRecord.update(with:row:columnNames:parentForeignKey:), which is used during upserts. As I understand it, this logic attempts to restore values from the last-known server record onto an incoming server record. In my mental model, this should be superfluous (and really a no-op), as I was assuming the last-known server record represents the most recent record that was either accepted by or fetched from the server. In the three-way merge terms, this would be the ancestor.
However, while digging deeper, I found two spots in the codebase where a record is saved as the last-known server record before it has been accepted by the server. I went into more detail on these in the warning sections of the linked document. In particular, the premature save during send looked worrying to me, as it seems to break the mental model of the last-known server record.
When testing this with the sample apps, I noticed that the record was not actually being saved during send. Inspecting SyncEngine.refreshLastKnownServerRecord(_:) revealed that this was due to the check of the modificationDate between the saved last-known server record and the given record, which happened to be equal, as the given record was based off of the last-known server record.
In tests, however, the record is being saved. The reason appears to be that the mock server does not populate modificationDate on records it reports back to the sync engine. While CKRecord.modificationDate can’t be set directly, I was able to swizzle it and have the mock server simulate a save timestamp. With that change in place, behavior became closer to production but two MergeConflictTests started producing different results, as they had previously been operating on a polluted last-known server record:
clientRecordUpdatedBeforeServerRecord(): the final assertion changed fromisCompleted = 1 @ t=30toisCompleted = 1 @ t=60serverRecordEditedAfterClientAndProcessedAfterClient()(added in #353): the title changed from “Buy milk” to “Get milk”, which is incorrect, but at least aligned with the other test preferring local edits regardless of timestamps (as described in #354)
I might be wrong, but I don’t think the reconciliation from the last-known server record that doesn’t reflect the actual server state is an intended behavior specifically built for tests. Having different results between the production and test environment feels wrong, even though the broader question of the desired semantics still stands.
The most straightforward way to align both environments seems to be to have the mock server populate modificationDate on “saved” records. From there, we could iterate further. Is this how things are meant to work? Do you want me to submit a PR that allows the mock server to set the modification date?
Checklist
- I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
- I have determined whether this bug is also reproducible in a vanilla GRDB project.
- If possible, I've reproduced the issue using the
mainbranch of this package. - This issue hasn't been addressed in an existing GitHub issue or discussion.
SQLiteData version information
1.4.1
Sharing version information
2.7.4
GRDB version information
7.8.0
Destination operating system
No response
Xcode version information
Version 26.2 (17C52)
Swift Compiler version information
swift-driver version: 1.127.14.1 Apple Swift version 6.2.3 (swiftlang-6.2.3.3.21 clang-1700.6.3.2)
Target: arm64-apple-macosx15.0