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

[fedGuid] Handle cases when element was recreated instead of updated #93

Closed ViliusRuskys closed 1 year ago

ViliusRuskys commented 1 year ago

When an element gets updated by recreating it, the transformer sees 2 changes: 1 delete and 1 insert. Since provenance is tracked by fed guids the insert will not happen, and an element update will be triggered instead. When this happens we no longer need to invoke the delete operation on the updated element. We achieve this by keeping track of newly inserted elements in the target iModel and ignoring deletes on these elements.

ViliusRuskys commented 1 year ago

The problem with doing it in the IModelTransformer.remapDeletedSourceEntities method is that the exporter.sourceDbChanges member is not yet initialized at that point. It is only 'for sure' initialized only after we call exporter.exportChanges.

I'm thinking of moving the sourceDbChanges initialization to a public exporter.initializeChangeData(). That way we could initialize the exporter before running the remapDeletedSourceEntities.

What do you think about this approach?

MichaelBelousov commented 1 year ago

NOTE: discussed in private messages, I like this approach but by adding IModelExporter.initialize invoking initializeChangeData which is private

MichaelBelousov commented 1 year ago

There is an optimization I had to remove to merge with main, I can reintroduce that optimization after your PR, since your PR (I believe) will move the _sourceDbChanges initialization to before exportChanges, and I need it in IModelTransformer.processChanges before IModelExporter.exportChanges, and I need access to it that early to reintroduce the optimization. (which is to remap all non-deleted provenance-tracked elements ahead of time so that we can go back to the optimization of not exporting unchanged element in processChanges)

MichaelBelousov commented 1 year ago

Also note @ViliusRuskys I see a failing test on the synced copy of this branch (idk if you have local work):


  64 passing (1m)
  7 pending
  1 failing

  1) IModelCloneContext
       findTargetEntityId
         should return target relationship id:

      AssertionError: expected false to be true
      + expected - actual

      -false
      +true

      at Context.<anonymous> (src/test/standalone/IModelCloneContext.test.ts:124:47)
MichaelBelousov commented 1 year ago

Since you are out @ViliusRuskys I will make the changes I requested in a separate branch and merge them. Please feel free to debate me on some ofmy changes and we can consider reverting them.

MichaelBelousov commented 1 year ago

succeeded by https://github.com/iTwin/imodel-transformer/pull/99