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

IModelTransformer is not handling unresolved elements in finalizeTransformation() #134

Closed JulijaRamoskiene closed 5 months ago

JulijaRamoskiene commented 7 months ago

In IModelTransformer:finalizeTransformation() unresolved elements are not logged and forceComplete () is not called for them.

After running transformation I see this warning: 

Warning | core-backend.IModelTransformer | The following elements were never fully resolved:

This indicates that either some references were excluded from the transformation
or the source has dangling references. {}

Additional context This issue is happening because BigMap does not have keys () and values() implementation that would correctly collect keys and values. https://github.com/iTwin/imodel-transformer/blob/main/packages/transformer/src/BigMap.ts

https://github.com/iTwin/imodel-transformer/blob/main/packages/transformer/src/IModelTransformer.ts#L1122C6-L1122C6 private finalizeTransformation() { if (this._partiallyCommittedEntities.size > 0) { Logger.logWarning( loggerCategory, [ "The following elements were never fully resolved:", [...this._partiallyCommittedEntities.keys()].join(","), "This indicates that either some references were excluded from the transformation", "or the source has dangling references.", ].join("\n") ); for (const partiallyCommittedElem of this._partiallyCommittedEntities.values()) { partiallyCommittedElem.forceComplete(); } }

MichaelBelousov commented 7 months ago

Since you identified the problem, would you be willing to submit a PR fixing this @JulijaRamoskiene?

JulijaRamoskiene commented 7 months ago

Not at the moment, as I am a bit occupied with my current task. I will let you know if I have time in the future (to check if someone else started working on it).

MichaelBelousov commented 7 months ago

no problem. Upon further analysis, the reason this doesn't throw a type error is because we are extends'ing Map when we should be implements'ing it. All unoverridden methods now default to super class Map which is empty, which is very bad.