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

Allow shouldExportElement to take an elementId OR elementProps #160

Closed nick4598 closed 5 months ago

nick4598 commented 6 months ago

Transformation services queries the db before they run their transformation so they will already have access to a bunch of IDs that they should / should not export. We should support both options potentially (ID / props)

I should probably write more on this item after I take a look.

nick4598 commented 6 months ago

IModelExporter.exportElement->IModelExporter.shouldExportElement->IModelExporter.handler.shouldExportElement

IModelExporter.shouldExportElement does need the element loaded to support the functionality of excluding specific element classes from being exported.

handler.shouldExportElement will typically be the implementation of shouldExportElement defined by the given instance of IModelTransformer.

Looking through transformation-services, the majority of the implementations of shouldExportElement seem to rely on some form of instanceof on the element. Perhaps thats because the element was already loaded and available to them through the parameter passed.

I imagine there are scenarios where an implementation of shouldExportElement should only need the id to decide, but that seems that it can be exclusively handled by exporter.excludeElement(id64string).

Therefore I think it makes sense to continue loading the element, because if each individual implementation of shouldExportElement has to load the element because they are only given an id that would probably make the issue worse.

This did expose to me though that we load the element before first checking that the ID was excluded through excludeElement, which is a small and easy optimization to make. https://github.com/iTwin/imodel-transformer/pull/164

nick4598 commented 5 months ago

Closing this for now as I think the fix in #164 is sufficient.