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

Improve Import Resolver logic #1383

Open
Tracked by #997
yesamer opened this issue Jul 10, 2024 · 2 comments
Open
Tracked by #997

Improve Import Resolver logic #1383

yesamer opened this issue Jul 10, 2024 · 2 comments
Labels
area:dmn Related to DMN area:engine Related to the runtime engines type:tech-debt Things that were left behind an may harm us in the future.

Comments

@yesamer
Copy link

yesamer commented Jul 10, 2024

Follow-up of #1150.

After the analysis of that part of the code, several improvements should be applied:

  • An old custom drools:modelName attribute is used in the logic. That attribute is a custom definition (i.e. not present in the DMN specs) no longer used in our implementation since DMN 1.1 version. Considering that the XSD that defines that attribute is no longer present in our codebase, the only reference of that attribute is present in the 1.1 TImport class, and our editors are not managing it at all, we agreed to remove any reference to that attribute in the logic
  • The new logic, should rely on the namespace only to resolve the imported DMN file. The namespace of the Definition metamodel doesn't ensure this attribute is unique over all DMNs of the entire project, so a collision may happen.
    In addition, the Import metamodel paragraph says: The namespace value should be globally unique, which in our interpretation doesn't force all the namespace to be unique but the required conditions to have a correct import resolution, without saying how to manage a possible collision.
    In case of collisions (i.e. multiple DMN files with the same namespace in the Definition metamodel), the idea is to rely on the locationURI attribute to correctly resolve the DMN model to be imported. The locationURI is optional, so the logic should manage such a scenario as well.
  • DMNResource and DMNModel DTOs have a very similar scope and fields. It seems the first one holds DMN files not yet compilated, while the latter is managing the already compilated ones. It could be possible to unify those DTOs - TBA
  • The overall logic that resolves and compiles the DMN assets and their dependencies with their Imported DMN could be improved. Multiple call to the Import resolver could be avoided. Some dmnModels are added from a source that seems no longer used (business-central) TBA
Screenshot 2024-05-03 at 11 29 57

@gitgabrio

@yesamer yesamer added area:dmn Related to DMN area:engine Related to the runtime engines type:tech-debt Things that were left behind an may harm us in the future. labels Jul 10, 2024
@jomarko
Copy link

jomarko commented Jul 11, 2024

What is the effect on the UI Side on the tooling @yesamer please? Do we need to remove/add some UI field in the popover for the importing model? Or should we remove some field form the UI Cards for displaying existing imports?

@gitgabrio
Copy link

@jomarko @tiagobento @yesamer
This is something that will be implemented during server-side validation.
It is up to tooling-team to decide if some preventive check should be made directly in UI, e.g. when a folder/project is imported, or when an import is defined

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:dmn Related to DMN area:engine Related to the runtime engines type:tech-debt Things that were left behind an may harm us in the future.
Projects
Status: 📋 Backlog
Development

No branches or pull requests

3 participants