Open nick4598 opened 3 weeks ago
Maybe unrelated but Julija also reported this recently (just wanted to capture it somewhere):
FYI there is small bug in implementation, indices are duplicated if updateSynchronization is called more than once (e.g. we do it in our current implementation). So at first it is: { "id": "0x20000000001", "version": "26170edf46081dcdb7881980831a927f6de8a55c;1", "classFullName": "BisCore:ExternalSourceAspect", "element": { "id": "0x20000000002", "relClassName": "BisCore:ElementOwnsExternalSourceAspects" }, "scope": { "id": "0x1" }, "identifier": "ee72a974-9504-4a86-9f85-601d038be8f0", "kind": "Scope", "jsonProperties": { "pendingReverseSyncChangesetIndices": [1, 2], "pendingSyncChangesetIndices": [], "reverseSyncVersion": "9a3110da675cc82baee79e80e02416b1b48dc3a4;1" } }
On second call: { "id": "0x20000000001", "version": "26170edf46081dcdb7881980831a927f6de8a55c;1", "classFullName": "BisCore:ExternalSourceAspect", "element": { "id": "0x20000000002", "relClassName": "BisCore:ElementOwnsExternalSourceAspects" }, "scope": { "id": "0x1" }, "identifier": "ee72a974-9504-4a86-9f85-601d038be8f0", "kind": "Scope", "jsonProperties": { "pendingReverseSyncChangesetIndices": [1, 2, 1, 2], "pendingSyncChangesetIndices": [], "reverseSyncVersion": "9a3110da675cc82baee79e80e02416b1b48dc3a4;1" } }
What we do in core transformer:
PR where mike B. added support for reverseSyncVersion provenance changesets finding hteir way into pendingReverseSyncCHangesetIndices: https://github.com/iTwin/imodel-transformer/pull/108
Transformation Services bumps the reverseSyncVersion by one instead of adding to pendingreversesynchangesetindices. This is a change from a time before mike B's PR where we supported this at core level.
Thoughts
My gut reaction to this is that it feels wrong to bump the reverseSyncVersion as opposed to adding any provenance related changesets which may have been pushed to the source db in a reverse sync (the branch)..
I think the end result between the two approaches is the same in that the appropriate changesets get skipped: 1) Bump reversesyncversion further 2) Add to pendingReverseSyncChangesetIndices
Problem with Transformation services switching to approach 2 is that it is now inconsistent with their database. During a forward sync it is reliable to view the syncversion in their db as the starting changeset index. In approach 2, The reverseSyncVersion might actually be part of the pendignReverseSyncChangesetIndices making the true starting changesetIndex reverseSyncVersion + 1. This can be confusing if people don't make sure to cross reference with the pending indices.
Above could be seen as an argument for approach 1
Links
Svcs team's implementation of reversesyncversion