-
Notifications
You must be signed in to change notification settings - Fork 2.6k
DROOLS-1663 Kie DMN doesn't support IMPORT decisions between DMN files #1832
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
Changes from all commits
9faed48
c2acde9
2976428
99dc867
bd7a6a3
00e947f
edca7c6
181ae4a
69831c8
06f7746
f5f8a40
e5b0338
8fcad77
ea7fc20
c85d849
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,4 +22,8 @@ public interface DMNNode { | |
|
|
||
| String getName(); | ||
|
|
||
| String getModelNamespace(); | ||
|
|
||
| String getModelName(); | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,11 +18,14 @@ | |
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
| import java.util.HashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
|
|
||
| import javax.xml.namespace.QName; | ||
|
|
||
| import org.drools.compiler.builder.impl.KnowledgeBuilderImpl; | ||
| import org.drools.compiler.compiler.PackageRegistry; | ||
| import org.drools.compiler.lang.descr.PackageDescr; | ||
|
|
@@ -32,15 +35,23 @@ | |
| import org.kie.api.io.Resource; | ||
| import org.kie.api.io.ResourceConfiguration; | ||
| import org.kie.api.io.ResourceType; | ||
| import org.kie.api.io.ResourceWithConfiguration; | ||
| import org.kie.dmn.api.core.DMNCompiler; | ||
| import org.kie.dmn.api.core.DMNCompilerConfiguration; | ||
| import org.kie.dmn.api.core.DMNModel; | ||
| import org.kie.dmn.api.marshalling.v1_1.DMNMarshaller; | ||
| import org.kie.dmn.core.api.DMNFactory; | ||
| import org.kie.dmn.core.compiler.DMNCompilerConfigurationImpl; | ||
| import org.kie.dmn.core.compiler.DMNCompilerImpl; | ||
| import org.kie.dmn.core.compiler.DMNProfile; | ||
| import org.kie.dmn.core.compiler.ImportDMNResolverUtil; | ||
| import org.kie.dmn.core.compiler.ImportDMNResolverUtil.ImportType; | ||
| import org.kie.dmn.core.compiler.profiles.ExtendedDMNProfile; | ||
| import org.kie.dmn.core.impl.DMNKnowledgeBuilderError; | ||
| import org.kie.dmn.core.impl.DMNPackageImpl; | ||
| import org.kie.dmn.feel.util.Either; | ||
| import org.kie.dmn.model.v1_1.Definitions; | ||
| import org.kie.dmn.model.v1_1.Import; | ||
| import org.kie.internal.builder.ResultSeverity; | ||
| import org.kie.internal.utils.ChainedProperties; | ||
| import org.slf4j.Logger; | ||
|
|
@@ -61,12 +72,68 @@ public ResourceType getResourceType() { | |
| } | ||
|
|
||
| @Override | ||
| public void addResource(Object kbuilder, Resource resource, ResourceType type, ResourceConfiguration configuration) | ||
| throws Exception { | ||
| public void addResources(Object kbuilder, Collection<ResourceWithConfiguration> resources, ResourceType type) throws Exception { | ||
| KnowledgeBuilderImpl kbuilderImpl = (KnowledgeBuilderImpl) kbuilder; | ||
| DMNCompilerImpl dmnCompiler = (DMNCompilerImpl) kbuilderImpl.getCachedOrCreate(DMN_COMPILER_CACHE_KEY, () -> getCompiler(kbuilderImpl)); | ||
| DMNMarshaller dmnMarshaller = dmnCompiler.getMarshaller(); | ||
| if (resources.size() == 1) { | ||
| // quick path: | ||
| internalAddResource(kbuilderImpl, dmnCompiler, resources.iterator().next(), Collections.emptyList()); | ||
| return; | ||
| } | ||
| List<DMNResource> dmnResources = new ArrayList<>(); | ||
| for (ResourceWithConfiguration r : resources) { | ||
| Definitions definitions = dmnMarshaller.unmarshal(r.getResource().getReader()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking comment No.1: This could be extracted to a separate method. Something like unmarshallResources(...).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry not following: what should I extract? only this line 85?
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, sorry for not being exact. I meant the whole logic block here. Same applies for all nitpicking comments.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but why should I extract it? This part of code is not reused -unless I am missing something.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| QName modelID = new QName(definitions.getNamespace(), definitions.getName()); | ||
| DMNResource dmnResource = new DMNResource(modelID, r, definitions); | ||
| dmnResources.add(dmnResource); | ||
| } | ||
| // enrich with imports | ||
| for (DMNResource r : dmnResources) { | ||
| for (Import i : r.getDefinitions().getImport()) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpicking comment No.2: This could be extracted to a separate method. Something like resolveImports(...).
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would do later, as currently import resolution is limited in functionality. |
||
| if (ImportDMNResolverUtil.whichImportType(i) == ImportType.DMN) { | ||
| Either<String, DMNResource> resolvedResult = ImportDMNResolverUtil.resolveImportDMN(i, dmnResources, DMNResource::getModelID); | ||
| DMNResource located = resolvedResult.getOrElseThrow(RuntimeException::new); | ||
| r.addDependency(located.getModelID()); | ||
| } | ||
| } | ||
| } | ||
| List<DMNResource> sortedDmnResources = DMNResourceDependenciesSorter.sort(dmnResources); | ||
| Collection<DMNModel> dmnModels = new ArrayList<>(); | ||
|
|
||
| for (DMNResource dmnRes : sortedDmnResources) { | ||
| DMNModel dmnModel = internalAddResource(kbuilderImpl, dmnCompiler, dmnRes.getResAndConfig(), dmnModels); | ||
| dmnModels.add(dmnModel); | ||
| } | ||
| } | ||
|
|
||
| private DMNModel internalAddResource(KnowledgeBuilderImpl kbuilder, DMNCompiler dmnCompiler, ResourceWithConfiguration r, Collection<DMNModel> dmnModels) throws Exception { | ||
| r.getBeforeAdd().accept(kbuilder); | ||
| DMNModel dmnModel = compileResourceToModel(kbuilder, dmnCompiler, r.getResource(), dmnModels); | ||
| r.getAfterAdd().accept(kbuilder); | ||
| return dmnModel; | ||
| } | ||
|
|
||
| @Override | ||
| public void addResource(Object kbuilder, Resource resource, ResourceType type, ResourceConfiguration configuration) throws Exception { | ||
| logger.warn("invoked legacy addResource (no control on the order of the assembler compilation): " + resource.getSourcePath()); | ||
| KnowledgeBuilderImpl kbuilderImpl = (KnowledgeBuilderImpl) kbuilder; | ||
| DMNCompiler dmnCompiler = kbuilderImpl.getCachedOrCreate( DMN_COMPILER_CACHE_KEY, () -> getCompiler( kbuilderImpl ) ); | ||
|
|
||
| DMNModel model = dmnCompiler.compile(resource); | ||
| Collection<DMNModel> dmnModels = new ArrayList<>(); | ||
| for (PackageRegistry pr : kbuilderImpl.getPackageRegistry().values()) { | ||
| ResourceTypePackage resourceTypePackage = pr.getPackage().getResourceTypePackages().get(ResourceType.DMN); | ||
| if (resourceTypePackage != null) { | ||
| DMNPackageImpl dmnpkg = (DMNPackageImpl) resourceTypePackage; | ||
| dmnModels.addAll(dmnpkg.getAllModels().values()); | ||
| } | ||
| } | ||
|
|
||
| compileResourceToModel(kbuilderImpl, dmnCompiler, resource, dmnModels); | ||
| } | ||
|
|
||
| private DMNModel compileResourceToModel(KnowledgeBuilderImpl kbuilderImpl, DMNCompiler dmnCompiler, Resource resource, Collection<DMNModel> dmnModels) { | ||
| DMNModel model = dmnCompiler.compile(resource, dmnModels); | ||
| if( model != null ) { | ||
| String namespace = model.getNamespace(); | ||
|
|
||
|
|
@@ -92,6 +159,7 @@ public void addResource(Object kbuilder, Resource resource, ResourceType type, R | |
| kbuilderImpl.addBuilderResult(new DMNKnowledgeBuilderError(ResultSeverity.ERROR, resource, "Unable to compile DMN model for the resource")); | ||
| logger.error( "Unable to compile DMN model for resource {}", resource.getSourcePath() ); | ||
| } | ||
| return model; | ||
| } | ||
|
|
||
| private List<DMNProfile> getDMNProfiles(KnowledgeBuilderImpl kbuilderImpl) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| package org.kie.dmn.core.assembler; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| import javax.xml.namespace.QName; | ||
|
|
||
| import org.kie.api.io.ResourceWithConfiguration; | ||
| import org.kie.dmn.model.v1_1.Definitions; | ||
|
|
||
| public class DMNResource { | ||
|
|
||
| private final QName modelID; | ||
| private final ResourceWithConfiguration resAndConfig; | ||
| private final Definitions definitions; | ||
| private final List<QName> dependencies = new ArrayList<>(); | ||
|
|
||
| public DMNResource(QName modelID, ResourceWithConfiguration resAndConfig, Definitions definitions) { | ||
| this.modelID = modelID; | ||
| this.resAndConfig = resAndConfig; | ||
| this.definitions = definitions; | ||
| } | ||
|
|
||
| public QName getModelID() { | ||
| return modelID; | ||
| } | ||
|
|
||
| public ResourceWithConfiguration getResAndConfig() { | ||
| return resAndConfig; | ||
| } | ||
|
|
||
| public Definitions getDefinitions() { | ||
| return definitions; | ||
| } | ||
|
|
||
| public void addDependency(QName dep) { | ||
| this.dependencies.add(dep); | ||
| } | ||
|
|
||
| public void addDependencies(List<QName> deps) { | ||
| this.dependencies.addAll(deps); | ||
| } | ||
|
|
||
| public List<QName> getDependencies() { | ||
| return dependencies; | ||
| } | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return "DMNResource [modelID=" + modelID + ", resource=" + resAndConfig.getResource().getSourcePath() + "]"; | ||
| } | ||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,49 @@ | ||
| package org.kie.dmn.core.assembler; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class DMNResourceDependenciesSorter { | ||
|
|
||
| /** | ||
| * Return a new list of DMNResource sorted by dependencies (required dependencies comes first) | ||
| */ | ||
| public static List<DMNResource> sort(List<DMNResource> ins) { | ||
| // In a graph A -> B -> {C, D} | ||
| // showing that A requires B, and B requires C,D | ||
| // then a depth-first visit would satisfy required ordering, for example a valid depth first visit is also a valid sort here: C, D, B, A. | ||
| Collection<DMNResource> visited = new ArrayList<>(ins.size()); | ||
| List<DMNResource> dfv = new ArrayList<>(ins.size()); | ||
|
|
||
| for (DMNResource node : ins) { | ||
| if (!visited.contains(node)) { | ||
| dfVisit(node, ins, visited, dfv); | ||
| } | ||
| } | ||
|
|
||
| return dfv; | ||
| } | ||
|
|
||
| /** | ||
| * Performs a depth first visit, but keeping a separate reference of visited/visiting nodes, _also_ to avoid potential issues of circularities. | ||
| */ | ||
| private static void dfVisit(DMNResource node, List<DMNResource> allNodes, Collection<DMNResource> visited, List<DMNResource> dfv) { | ||
| if (visited.contains(node)) { | ||
| throw new RuntimeException("Circular dependency detected: " + visited + " , and again to: " + node); | ||
| } | ||
| visited.add(node); | ||
|
|
||
| List<DMNResource> neighbours = node.getDependencies().stream() | ||
| .flatMap(dep -> allNodes.stream().filter(r -> r.getModelID().equals(dep))) | ||
| .collect(Collectors.toList()); | ||
| for (DMNResource n : neighbours) { | ||
| if (!visited.contains(n)) { | ||
| dfVisit(n, allNodes, visited, dfv); | ||
| } | ||
| } | ||
|
|
||
| dfv.add(node); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not as far as I know. @etirelli ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.