DROOLS-1663 Kie DMN doesn't support IMPORT decisions between DMN files #1832
Conversation
|
[ as noted on JIRA, please notice these PRs are relevant only for importing ItemDefinition(s) and BKM(s) ] |
|
Jenkins test again |
| } | ||
| List<DMNResource> dmnResources = new ArrayList<>(); | ||
| for (ResourceWithConfiguration r : resources) { | ||
| Definitions definitions = dmnMarshaller.unmarshal(r.getResource().getReader()); |
There was a problem hiding this comment.
Nitpicking comment No.1: This could be extracted to a separate method. Something like unmarshallResources(...).
There was a problem hiding this comment.
Sorry not following: what should I extract? only this line 85?
There was a problem hiding this comment.
No, sorry for not being exact. I meant the whole logic block here. Same applies for all nitpicking comments.
There was a problem hiding this comment.
but why should I extract it? This part of code is not reused -unless I am missing something.
There was a problem hiding this comment.
Just for readability. But this is really just a nitpicking, so for me it is good as it is if you like it better as it is.
| } | ||
| // enrich with imports | ||
| for (DMNResource r : dmnResources) { | ||
| for (Import i : r.getDefinitions().getImport()) { |
There was a problem hiding this comment.
Nitpicking comment No.2: This could be extracted to a separate method. Something like resolveImports(...).
There was a problem hiding this comment.
Would do later, as currently import resolution is limited in functionality.
| DMNMarshaller dmnMarshaller = dmnCompiler.getMarshaller(); | ||
| if (resources.size() == 1) { | ||
| // quick path: | ||
| internalAddResource(kbuilderImpl, dmnCompiler, resources.iterator().next(), Collections.emptyList()); |
There was a problem hiding this comment.
Question: Will there be a possibility of an external import? I mean some external resource outside of the kjar or from URL. Asking because for this one file import is not resolved (and that is correct in current case).
There was a problem hiding this comment.
not as far as I know. @etirelli ?
There was a problem hiding this comment.
This might be necessary in the future, but support is not implemented yet. For now, the imported model has to be reachable by the kie-container classloader.
I don't think we need to worry at the moment about "external" imports as that would have additional concerns, like authentication and authorization, that are not covered by the spec at the moment.
| public String getModelNamespace() { | ||
| if (source != null) { | ||
| Object parent = source.getParent(); | ||
| while (!(parent instanceof Definitions)) { |
There was a problem hiding this comment.
Nitpicking comment No.3: This could be extracted to a separate method. E.g. findParent().
| public String getModelName() { | ||
| if (source != null) { | ||
| Object parent = source.getParent(); | ||
| while (!(parent instanceof Definitions)) { |
There was a problem hiding this comment.
See my previous comment. The extracted method can be reused here.
There was a problem hiding this comment.
for getModel() and getModelNamespace I agree will append commit asap
|
|
||
| DMNResult evaluateAll = runtime.evaluateAll(dmnModel, context); | ||
| for (DMNMessage message : evaluateAll.getMessages()) { | ||
| System.out.println(message); |
There was a problem hiding this comment.
Probably leftover from debugging. Please use asserts or logger instead if this is still needed here.
| for (DMNMessage message : evaluateAll.getMessages()) { | ||
| System.out.println(message); | ||
| } | ||
| System.out.println(evaluateAll); |
There was a problem hiding this comment.
Same as previous comment.
|
@tarilabs this looks good to go as the first version of the implementation (i.e., supports for data types and BKMs). |
642ff2a to
c85d849
Compare
Requires kiegroup/droolsjbpm-knowledge#300