Only changed elements should be passed to onExportElement() while processing changes

[JulijaRamoskiene]

When processing changes IModelExporter.exportElement() should skip elements that are not added to _sourceDbChanges.elements.insertedIds or _sourceDbChanges.elements.updatedIds, but instead it always tries to export such 'unchanged' elements. it is happening due to this:

    const isUpdate = this._sourceDbChanges?.element.insertIds.has(elementId)
      ? false
      : this._sourceDbChanges?.element.updateIds.has(elementId)
        ? true
        : undefined;

it should be handled the same way as for models instead. In models case method returns if model is not in change set:

 let isUpdate: boolean | undefined;
    if (this._sourceDbChanges !== undefined) {
      // is changeset information available?
      if (this._sourceDbChanges.model.insertIds.has(model.id)) {
        isUpdate = false;
      } else if (this._sourceDbChanges.model.updateIds.has(model.id)) {
        isUpdate = true;
      } else {
        return; // not in changeset, don't export
      }
    }

• Once this is fixed logic to populate IModelTransformer._hasElementChangedCache can be removed.
• IModelTransformer.hasElementChanged could be removed in favor of custom changes API. I see this is protected, so not sure if there are users that have custom implementations for this.

[nick4598]

So this bug has existed for a long time then, correct? Is this causing issues? The hasElementChanged function is currently saving us from this bug, right?

[JulijaRamoskiene]

I think it existed since Fed Guid branch was merged: 1100de5#diff-d07eab136b812db5c475ddb91cbc589d5fd6e5943638c6e2028994d6f58fe80b

During my investigation I learned that in previous implementation (before change set parsing was implemented), transformer was querying target element, and it's ESA, to check if element has changed.

Currently, it seems to work, and only issue seems to be performance, as target element is queried for each source element:

imodel-transformer/packages/transformer/src/IModelTransformer.ts

Line 1834 in d76e0f2

this.targetDb.elements.getElement(maybeTargetElementId);

However, it becomes a pain point in our custom changes PR:
https://github.com/iTwin/imodel-transformer/pull/172/files#r1920417272
I think this implementation is error prone, as there are 2 sources of truth keeping track of changed elements (_sourceDbChanges.element and _hasElementChangedCache). And those 2 lists need to be kept in sync.

@nick4598 Do you have concerns with removing _hasElementChangedCache. Am I missing some information, why it is needed and sensible to have?

[nick4598]

No I think you're right that it probably should be removed, was only asking those questions to make sure I fully understood