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

Support providing the reversesync and sync versions when unsafe-migrate is set. #179

Closed nick4598 closed 4 months ago

nick4598 commented 4 months ago

Support providing the reversesync and sync versions when unsafe-migrate is set. This is to support svcs team and their use cases so they no longer have to have code surrounding this. A change we made to make syncDirection determined automatically by the transformer has made some of their patches no longer work.

I also ran into a bug when trying to test this which had 2 issues, the first being the hasElementChangedCache was getting aspect ids added to it. I believe it was ALWAYS supposed to be only element ids, but I made that mistake when converting to use Affan's changeset reader. The query that used to be used for the hasElementChangedCache had a where clause: ic.ChangedInstance.ClassId IS (ONLY BisCore.Element)

The second part of the bug: I believe we should no longer be checking if Id64.isValid(targetElementId) when deciding if we should return early because an element has not changed. This seems to be leftover from a time when you needed the targetElementId to find the ESA which described the relationship between the source and target element. I believe at the time of Mike making his change to no longer search for ESAs he didn't want to remove targetElementId (so he changed it to _targetElementId), because that COULD be a breaking change if someone was overriding the hasElementChanged function.

The services team does not appear to be overriding the hasElementChanged function, and this change will be part of a 1.0.0 major release.

nick4598 commented 4 months ago

https://github.com/iTwin/itwinjs-core/pull/6643/files

Test is failing on 4.6.0 due to the above change