Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Schema Merger behavior with multiple constraint classes and improve error codes #997

Open
4 of 5 tasks
c-macdonald opened this issue Feb 10, 2025 · 1 comment
Open
4 of 5 tasks
Assignees
Labels

Comments

@c-macdonald
Copy link

c-macdonald commented Feb 10, 2025

2 Sub-Tasks:

  • Make SchemaMerger able to work without validation. At the moment, its CopySchema() and CopyClass() calls do not perform validation, they never call validate() bit there is one exception from that: RelationshipClass has a m_validate flag, which if true, prevents you from composing a relationship class with multiple constraint classes without an abstract constraint or common base class. CopyClass() does not copy that flag to the copy, which it should.
    • Talk with Carole, compose tests to cover this scenario, make sure that it's the reason for the fail.
    • Add another test which merges schemas so the result ends up having multiple constraint classes. It's a slightly different scenario which should work as well.
    • If the assumption is correct, make CopyClass() copy the m_validate flags.
  • For better error diagnosis Carole wants detailed return codes instead of only BentleyStatus. This can be done and committed separately.

Email from Carole:
In a nutshell, the IFC4 schema has a relationship class with 3 target classes. These classes don't share a common base class (in fact, none of them have a base class). This means that EC 3.0 can't create an abstract constraint class. We have 2 different versions of the IfcApplicationDomain schema. They each reference the IFC4 schema. The first thing the new merger does is make a copy of the input schema and all of its references. It fails to copy the IFC4 referenced schema because of the illegal relationship class.

So, technically, not a problem with the merger. The ironic part is that we don't care at all about the targets. Once the schemas are consolidated, then we BISify them. And one of the steps of bisification is to remove all target constraints and just add the base Bis:Element class. Since we aren't creating new relationships, we don't need to actually constrain them. We just need to be able to read existing relationships.

I can work around this by pre-processing the schema and removing extra constraint classes before merging. But that is potentially a lot of overhead that isn’t necessary in most cases. All of this leads up to my actual question: can we change the SchemaMerger::MergeSchemas to return an actual status instead of just BentleyStatus? If it returned that the actual ECObjectsStatus, then I can try to fix certain cases. I’d start with this one and look for RelationshipConstraintsNotCompatible. If so, I’d then run through the schema and its references and remove the extra constraint classes.

@ColinKerr
Copy link
Member

We can investigate modifying SchemaMerger::MergeSchemas to return the full ECObjectsStatus

@rschili rschili changed the title Have schema merger return a real status Fix Schema Merger behavior with multiple constraint classes and improve error codes Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants