-
Notifications
You must be signed in to change notification settings - Fork 76
Three-way merge model for built-in conflict resolution for CloudKit sync #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
lukaskubanek
wants to merge
22
commits into
pointfreeco:main
Choose a base branch
from
structuredpath:prototype-conflict-resolution
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Three-way merge model for built-in conflict resolution for CloudKit sync #370
lukaskubanek
wants to merge
22
commits into
pointfreeco:main
from
structuredpath:prototype-conflict-resolution
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To align generics in RowVersion<Table> & FieldVersion<Value>…
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR explores how the current conflict resolution based on the “field-wise last edit wins” strategy can be expressed as a proper three-way merge model, as described in this document. It is the first step towards a broader effort to support customizable conflict resolution, originally pitched in #272.
With minor exceptions, this PR focuses on Stage 1 with the goal of refining the existing conflict resolution behavior into a clearer and more precise implementation and fixing a few correctness issues along the way. No public API changes are introduced and the current behavior remains effectively the same.
The prototype also lays the groundwork for Stage 2, where user-defined per-field merge policies could be added with minimal changes to the internals. The most promising direction here seems to be a declarative configuration via
@SyncedTable/@SyncedField(as discussed in #352), since macros can express per-field policies ergonomically and exhaustively. This would, however, require some additional infrastructure work around macro expansion.The prototype is not yet integrated into the CloudKit sync pipeline. It is intentionally kept in a standalone file with tests to validate the model and discuss details before further integration work.
Scope of this PR
This PR extracts conflict resolution into a small, self-contained model that performs a proper three-way merge between ancestor, client, and server versions of a row.
The prototype introduces a few supporting types to model conflicts explicitly:
MergeConflict<Table>bundles the ancestor, client, and server row versions for three-way merge.RowVersion<Table>wraps a decoded row together with per-field modification timestamps.FieldVersion<Value>represents a single field’s value paired with its modification timestamp.FieldMergePolicy<Value>provides an extensible mechanism for merging conflicting field values.When a conflict is detected, a
MergeConflictis constructed with three concrete row versions:CKRecorduserModificationTime, and the ancestor version (to obtain modification timestamps for unchanged fields)CKRecordThe choice to use value types is primarily for ease of reasoning and testing. It also leaves open the possibility of making some of these types public in Stage 2, so that users could resolve conflicts against Swift types rather than raw record data. Supporting custom per-field merge policies would at minimum require exposing
FieldVersionandFieldMergePolicy, and potentiallyRowVersionas well (see section on custom merge method below).Since table types cannot currently be constructed dynamically (other than via
T.init(decoder:)&QueryDecoder), the prototype decodes rows fromCKRecordinstances by performing a SQLSELECTquery with literal values, leveraging the existing decoding infrastructure.When a
MergeConflictis available, resolution proceeds by iterating over all fields and performing equality checks. Equality is determined viaQueryBinding, so field values do not need to conform toEquatable. Trivial cases such as no change or one-sided change are short-circuited. The field merge policy is consulted only when both the client and server diverge from the ancestor.The prototype comes with an abstraction for per-field merge policies and includes three concrete implementations:
.latest: Last edit wins, favoring the edited value with the newest modification timestamp..counter: Combines independent increments and decrements in integer fields from both edited sides..set: Merges set fields by preserving elements not deleted on either side and adding any new elements.In Stage 1, only
.latestis intended to be used, as it matches the current behavior in the library (but with clearer semantics). In particular, the merge logic avoids “reviving” ancestor values during conflict resolution, since.latestonly compares edited client and server versions.The additional policies serve primarily to show how the abstraction generalizes and what Stage 2 could unlock. All policies are exercised in the tests for the
Posttype, where.latest,.counter, and.setare applied to different fields.After resolving all fields, the merged values must be written back to the database. The prototype does this by generating an
UPDATEstatement that writes the merged field values directly into the existing row. No upsert is needed here, as a conflicting row is guaranteed to exist in the database.Earlier drafts explored producing a full
Tableinstance and then generating anUPDATEstatement from it, but that path does not seem viable in the current design (see section on custom merge methods below).For tests, there is a helper method that performs the whole roundtrip of inserting the client row, applying the merge update, fetching the resulting row, and asserting on its values.
Integration
As mentioned above, the prototype isn’t wired to the CloudKit sync pipeline yet. If this model were to be integrated in Stage 1, it would mean expressing the existing “last edit wins” semantics through the three-way merge model. Concretely, this would require a few targeted changes in the conflict handling paths:
.latestfield merge policy only for fields that diverge from the ancestor on both sides. One-sided edits and unchanged fields should bypass the policy.For Stage 2, where users could annotate individual fields with merge policies, a few additional pieces would be needed:
@SyncedTable/@SyncedFieldso that per-field merge policies can be specified declaratively. More on this in #352.Merge method returning
T?While ideating on the API design initially, the motivating idea was to express conflict resolution as a method that receives a
MergeConflictcontaining the three row versions and returns a merged row instance ofT(orT.Draft) to be written back to the database.This approach turned out not to be viable in Stage 1, as there does not seem to be a way to dynamically construct a row instance by inspecting its columns and assigning values to fields, other than going through a database statement and the
T.init(decoder:)initializer (as seen inCKRecorddecoding)1. For this reason, the prototype resolves conflicts by generating anUPDATEstatement that writes merged field values directly to the existing row, without constructing a new row instance.In Stage 2, with macros allowing users to specify per-field merge policies, the current approach of producing an
UPDATEstatement could be preserved. In that design, table types would expose the specified merge policies so that the conflict resolution logic can apply them field-by-field.Alternatively, the macro could generate a merge function in its expansion, iterating over all fields, computing the merged values, and constructing a new row instance. To fully deliver on the original motivation, the library could even allow users to override this with a custom declaration (e.g. via a
CustomMergeConflictResolvableprotocol).This direction hasn’t been explored in depth, but opening up a custom merge function comes with some downsides. It competes with macro-defined merge policies and introduces opportunities to omit fields or fall back to default values by accident. The macro-generated path therefore seems more promising.
The only use case for a custom merge function that comes to mind is when multiple fields need to be considered together, e.g.
startDateandendDatewhereendDatemust always be greater than or equal tostartDate. Such cases can be addressed by (a) grouping the data into a single structure and serializing it as JSON, or (b) installing a trigger that enforces the invariant before updating the row. Both approaches apply more generally, not only in the context of conflict resolution.It is not entirely clear where Stage 2 should ultimately land, but detecting conflicts at the row level while allowing the user to merge at the field level seems like a reasonable and pragmatic approach.
Open topics
CKRecord→RowVersiondecoding path. Asset-backed fields are routed through the prototype’s decoding path but are not exercised in tests yet.FieldMergePolicyAPI for such cases.Footnotes
Updating values on an existing instance, e.g. the client row, is not viable either, as this would require
WritableKeyPaths. It is also perfectly fine for a conflicting field to be represented as aletproperty in Swift, since the value in the database can still be mutated regardless of mutability in the Swift type. ↩