search

iTwin / imodel-transformer

API for exporting an iModel's parts and also importing them into another iModel
MIT License
3 stars 2 forks source link

[fed guid optimization] Change processing does not work in reverse synchronization workflow #68

Closed ViliusRuskys closed 1 year ago

ViliusRuskys commented 1 year ago

Describe the bug The issue is related to the new rule added to the federation guid branch of imodel-transformer. This rule enforces that for change processing workflow the source iModel starting changeset index must be 1 higher over the changeset index used for the previous transformation. Right now we crate a Forked iModel by running the transformation in normal direction (Lets say source only has 1 changeset)

S1 -> T1, T2, T3 (During the initial transformation target iModel gets multiple changes pushed so target (T) is on changeset 3)    T4 (user pushes a few more changes to forked iModel) S2 <- T5 (we merge back with reverse synchronization workflow using start changeset with index 4 because data flow is reversed) The transformation errors out, because transformer expects changeset with index 2 to be used and isntead it got index 4.

Comparing these changesets does not make sense, because these changesets are from different iModels as well (changeset 1 is in source, changeset 4 is in target).

MichaelBelousov commented 1 year ago

@ViliusRuskys we may need to discuss this one more. I have added to the "big branch workflow test" a custom synchronizer that always performs multi-changeset synchronizations. But the tests are not failing with the error you say they would here. Is there anything in that implementation that looks wrong to you?

Here is the resulting change history of the test after those changes:

┌─────────┬───────────┬───────────┬───────────┐
│ (index) │  master   │  branch1  │  branch2  │
├─────────┼───────────┼───────────┼───────────┤
│    0    │   '0 '    │           │           │
│    1    │   '0 '    │ '1 2d730' │           │
│    2    │ '1 66678' │ '1 2d730' │           │
│    3    │ '1 66678' │ '1 2d730' │ '1 6f3c0' │
│    4    │ '1 66678' │ '2 d6b09' │ '1 6f3c0' │
│    5    │ '1 66678' │ '3 96a20' │ '1 6f3c0' │
│    6    │ '1 66678' │ '4 d25c9' │ '1 6f3c0' │
│    7    │ '1 66678' │ '5 36512' │ '1 6f3c0' │
│    8    │ '1 66678' │ '6 5101d' │ '1 6f3c0' │
│    9    │ '3 c6100' │ '6 5101d' │ '1 6f3c0' │
│   10    │ '3 c6100' │ '6 5101d' │ '1 6f3c0' │
│   11    │ '3 c6100' │ '6 5101d' │ '3 1d530' │
│   12    │ '3 c6100' │ '6 5101d' │ '3 1d530' │
│   13    │ '3 c6100' │ '6 5101d' │ '4 f1f24' │
│   14    │ '4 67ddd' │ '6 5101d' │ '4 f1f24' │
│   15    │ '6 4bac9' │ '6 5101d' │ '4 f1f24' │
│   16    │ '6 4bac9' │ '6 5101d' │ '4 f1f24' │
│   17    │ '7 ec9ee' │ '6 5101d' │ '4 f1f24' │
│   18    │ '8 6b6ea' │ '6 5101d' │ '4 f1f24' │
│   19    │ '8 6b6ea' │ '8 8bde9' │ '4 f1f24' │
│   20    │ '8 6b6ea' │ '8 8bde9' │ '4 f1f24' │
└─────────┴───────────┴───────────┴───────────┘

Note that master reverse-synchronizes both branch1 and branch2 which is where you see changeset index bumps by 2 instead of by 1, since the synchronization created more than 1 changeset in that step of the timeline, and only the ending changeset of each timeline step is recorded.

ViliusRuskys commented 1 year ago

To reproduce it you need to:

Pushed a test case to reproduce this to #78

MichaelBelousov commented 1 year ago

found the bug after @KyryloVolotovskyi provided a reproduction with a correct start changeset it. Good catch! Fixed and merged the new test.

I will publish a new fedguidopt prerelease version with the fix

MichaelBelousov commented 1 year ago

Reopen if you find it doesn't fix other cases.