bncriju commented on code in PR #6267:
URL:
https://github.com/apache/incubator-kie-drools/pull/6267#discussion_r1991742886
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
+ // incubator-kie-issues#852: The idea is to not treat the
anonymous models as import, but to "merge" them
+ // with original one,
+ // because otherwise we would have to deal with clashing name
aliases, or similar issues
+ if (iAlias != null && !iAlias.isEmpty()) {
+ model.setImportAliasForNS(iAlias, located.getNamespace(),
located.getName());
+ importFromModel(model, located, iAlias);
+ } else {
+ toMerge.add(located);
+ }
+ }
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i,
Function<String, Reader> relativeResolver, DMNCompilerConfigurationImpl
dmnCompilerConfig) {
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNCompilerImpl.java:
##########
@@ -317,6 +282,7 @@ protected static Resource
resolveRelativeResource(ClassLoader classLoader, DMNMo
throw new UnsupportedOperationException("Unable to determine relative
Resource for import named: " + i.getName());
}
+
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
+ // incubator-kie-issues#852: The idea is to not treat the
anonymous models as import, but to "merge" them
+ // with original one,
+ // because otherwise we would have to deal with clashing name
aliases, or similar issues
+ if (iAlias != null && !iAlias.isEmpty()) {
+ model.setImportAliasForNS(iAlias, located.getNamespace(),
located.getName());
+ importFromModel(model, located, iAlias);
+ } else {
+ toMerge.add(located);
+ }
+ }
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i,
Function<String, Reader> relativeResolver, DMNCompilerConfigurationImpl
dmnCompilerConfig) {
+ ClassLoader rootClassLoader = dmnCompilerConfig.getRootClassLoader();
+ Resource relativeResource = resolveRelativeResource(rootClassLoader,
model, i, i, relativeResolver);
+ resolvePMMLImportType(model, i, relativeResource, dmnCompilerConfig);
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i, Resource
relativeResource, DMNCompilerConfigurationImpl dmnCompilerConfig) {
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
+ // incubator-kie-issues#852: The idea is to not treat the
anonymous models as import, but to "merge" them
+ // with original one,
+ // because otherwise we would have to deal with clashing name
aliases, or similar issues
+ if (iAlias != null && !iAlias.isEmpty()) {
+ model.setImportAliasForNS(iAlias, located.getNamespace(),
located.getName());
+ importFromModel(model, located, iAlias);
+ } else {
+ toMerge.add(located);
+ }
+ }
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i,
Function<String, Reader> relativeResolver, DMNCompilerConfigurationImpl
dmnCompilerConfig) {
+ ClassLoader rootClassLoader = dmnCompilerConfig.getRootClassLoader();
+ Resource relativeResource = resolveRelativeResource(rootClassLoader,
model, i, i, relativeResolver);
+ resolvePMMLImportType(model, i, relativeResource, dmnCompilerConfig);
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i, Resource
relativeResource, DMNCompilerConfigurationImpl dmnCompilerConfig) {
+ try (InputStream pmmlIS = relativeResource.getInputStream()) {
+ DMNImportPMMLInfo.from(pmmlIS, dmnCompilerConfig, model,
i).consume(new DMNCompilerImpl.PMMLImportErrConsumer(model, i),
+ model::addPMMLImportInfo);
+ } catch (IOException e) {
+ new DMNCompilerImpl.PMMLImportErrConsumer(model, i).accept(e);
+ }
+ }
+
+ static void logErrorMessage(DMNModelImpl model, String importType) {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ null,
+ model,
+ null,
+ null,
+ Msg.IMPORT_TYPE_UNKNOWN,
+ importType);
+ }
+
+ static void importFromModel(DMNModelImpl model, DMNModel m, String iAlias)
{
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
+ // incubator-kie-issues#852: The idea is to not treat the
anonymous models as import, but to "merge" them
+ // with original one,
+ // because otherwise we would have to deal with clashing name
aliases, or similar issues
+ if (iAlias != null && !iAlias.isEmpty()) {
+ model.setImportAliasForNS(iAlias, located.getNamespace(),
located.getName());
+ importFromModel(model, located, iAlias);
+ } else {
+ toMerge.add(located);
+ }
+ }
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i,
Function<String, Reader> relativeResolver, DMNCompilerConfigurationImpl
dmnCompilerConfig) {
+ ClassLoader rootClassLoader = dmnCompilerConfig.getRootClassLoader();
+ Resource relativeResource = resolveRelativeResource(rootClassLoader,
model, i, i, relativeResolver);
+ resolvePMMLImportType(model, i, relativeResource, dmnCompilerConfig);
+ }
+
+ static void resolvePMMLImportType(DMNModelImpl model, Import i, Resource
relativeResource, DMNCompilerConfigurationImpl dmnCompilerConfig) {
+ try (InputStream pmmlIS = relativeResource.getInputStream()) {
+ DMNImportPMMLInfo.from(pmmlIS, dmnCompilerConfig, model,
i).consume(new DMNCompilerImpl.PMMLImportErrConsumer(model, i),
+ model::addPMMLImportInfo);
+ } catch (IOException e) {
+ new DMNCompilerImpl.PMMLImportErrConsumer(model, i).accept(e);
+ }
+ }
+
+ static void logErrorMessage(DMNModelImpl model, String importType) {
Review Comment:
please add javadoc
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/assembler/DMNAssemblerService.java:
##########
@@ -43,13 +43,9 @@
import org.kie.dmn.api.core.DMNModel;
import org.kie.dmn.api.marshalling.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.DMNDecisionLogicCompilerFactory;
-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.RuntimeTypeCheckOption;
+import org.kie.dmn.core.compiler.*;
Review Comment:
Instead of asterisk and importing everything, import what exactly is required
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
+ // incubator-kie-issues#852: The idea is to not treat the
anonymous models as import, but to "merge" them
Review Comment:
The comment regarding the incubator-kie-issues#852 is valuable but could be
made clearer with a bit more context of the same
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/compiler/DMNImportsUtilTest.java:
##########
@@ -190,4 +197,96 @@ private Import makeImport(final String namespace, final
String name, final Strin
return i;
}
+ @Test
+ void checkLocatedDMNModel() {
+ List<DMNModel> toMerge = new ArrayList<>();
+ List<DMNModel> dmnModels = new ArrayList<>();
+ String nameSpace =
"http://www.trisotech.com/dmn/definitions/_f79aa7a4-f9a3-410a-ac95-bea496edabgc";;
+ Resource resource = new ClassPathResource(
"valid_models/DMNv1_5/Importing_Named_Model.dmn",
+ this.getClass());
+ DMNModel importingModel = dMNCompiler.compile(resource, dmnModels);
+ assertThat(importingModel).isNotNull();
+
assertThat(importingModel.getNamespace()).isNotNull().isEqualTo(nameSpace);
+
+ Import input = importingModel.getDefinitions().getImport().get(0);
+ DMNModelImpl model = new DMNModelImpl(importingModel.getDefinitions(),
resource);
+ DMNModel located = new DMNModelImpl(importingModel.getDefinitions(),
resource);
+ DMNImportsUtil.checkLocatedDMNModel(input, located, model, toMerge);
+ assertThat(importingModel).isNotNull();
+
assertThat(importingModel.getNamespace()).isNotNull().isEqualTo(nameSpace);
+ assertThat(toMerge).isEmpty();
+ }
+
+ @Test
+ void resolveDMNImportType() {
+ List<DMNModel> toMerge = new ArrayList<>();
+ List<DMNModel> dmnModels = new ArrayList<>();
+ Resource resource = new ClassPathResource(
"valid_models/DMNv1_5/Imported_Model_Unamed.dmn",
+ this.getClass());
+ DMNModel importedModel = dMNCompiler.compile( resource, dmnModels);
+ assertThat(importedModel).isNotNull();
+ dmnModels.add(importedModel);
+
+ //imported model - Importing_Named_Model.dmn
+ String nameSpace =
"http://www.trisotech.com/dmn/definitions/_f79aa7a4-f9a3-410a-ac95-bea496edabgc";;
+ resource = new ClassPathResource(
"valid_models/DMNv1_5/Importing_Named_Model.dmn",
+ this.getClass());
+ DMNModel importingModel = dMNCompiler.compile(resource, dmnModels);
+ assertThat(importingModel).isNotNull();
+
assertThat(importingModel.getNamespace()).isNotNull().isEqualTo(nameSpace);
+ assertThat(importingModel.getMessages()).isEmpty();
+
+ Import input = importingModel.getDefinitions().getImport().get(0);
+ DMNModelImpl model = new DMNModelImpl(importingModel.getDefinitions(),
resource);
+ DMNImportsUtil.resolveDMNImportType(input, dmnModels, model, toMerge);
+ assertThat(model.getMessages()).isEmpty();
+
assertThat(model.getImportAliasesForNS().entrySet().stream().findFirst())
+ .isPresent().get().extracting(Map.Entry::getValue)
+
.extracting(QName::getLocalPart).isNotNull().isEqualTo("Imported Model");
+
+ }
+
+
+
+ @Test
+ void checkLocatedDMNModelWithAliasNull() {
+ String
namespace="http://www.trisotech.com/dmn/definitions/_f79aa7a4-f9a3-410a-ac95-bea496edabgc";;
+ List<DMNModel> toMerge = new ArrayList<>();
+ List<DMNModel> dmnModels = new ArrayList<>();
+ Resource resource = new ClassPathResource(
"valid_models/DMNv1_5/Importing_EmptyNamed_Model_Without_Href_Namespace.dmn",
+ this.getClass());
+ DMNModel emptyNamedModel = dMNCompiler.compile( resource, dmnModels);
+ assertThat(emptyNamedModel).isNotNull();
+ dmnModels.add(emptyNamedModel);
+
+ Import input = emptyNamedModel.getDefinitions().getImport().get(0);
+ DMNModelImpl model = new
DMNModelImpl(emptyNamedModel.getDefinitions(), resource);
+ DMNModel located = new DMNModelImpl(emptyNamedModel.getDefinitions(),
resource);
+ DMNImportsUtil.checkLocatedDMNModel(input, located, model, toMerge);
+ assertThat(emptyNamedModel).isNotNull();
+ assertThat(toMerge).isNotEmpty();
+ assertThat(toMerge.size()).isEqualTo(1);
+
assertThat(toMerge.get(0).getNamespace()).isNotNull().isEqualTo(namespace);
+ }
+
+ @Test
+ void resolvePMMLImportType() {
+ List<DMNModel> dmnModels = new ArrayList<>();
+ Resource dmnResource = new ClassPathResource(
"../pmml/KiePMMLNewTree.dmn",
+ this.getClass());
+ DMNModel importingModel = dMNCompiler.compile( dmnResource, dmnModels);
+ assertThat(importingModel).isNotNull();
+
+
+ Import input = importingModel.getDefinitions().getImport().get(0);
+ DMNModelImpl model = new DMNModelImpl(importingModel.getDefinitions(),
dmnResource);
+
+ Resource relativeResource = new ClassPathResource(
"../pmml/test_tree_new.pmml",
+ this.getClass());
+ assertThat(model.getPmmlImportInfo()).isEmpty();
+ DMNCompilerConfigurationImpl dmnCompilerConfig =
(DMNCompilerConfigurationImpl)((DMNCompilerImpl)dMNCompiler).getDmnCompilerConfig();
+ DMNImportsUtil.resolvePMMLImportType(model, input, relativeResource,
dmnCompilerConfig);
+
assertThat(model.getPmmlImportInfo()).hasSize(1).containsOnlyKeys("test_tree");
+ }
+
Review Comment:
It might be beneficial to include edge cases, such as:
- An empty DMN file or one with invalid syntax.
- A case where the import model is valid but contains errors or
discrepancies in the expected structure.
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNImportsUtil.java:
##########
@@ -130,4 +141,84 @@ public static ImportType whichImportType(Import
importElement) {
return ImportType.UNKNOWN;
}
}
+
+ static void resolveDMNImportType(Import i, Collection<DMNModel> dmnModels,
DMNModelImpl model, List<DMNModel> toMerge) {
+ Either<String, DMNModel> resolvedResult =
DMNImportsUtil.resolveImportDMN(i, dmnModels, (DMNModel m) -> new
QName(m.getNamespace(), m.getName()));
+ DMNModel located = resolvedResult.cata(msg -> {
+ MsgUtil.reportMessage(LOGGER,
+ DMNMessage.Severity.ERROR,
+ i,
+ model,
+ null,
+ null,
+ Msg.IMPORT_NOT_FOUND_FOR_NODE,
+ msg,
+ i);
+ return null;
+ }, Function.identity());
+ checkLocatedDMNModel(i, located, model, toMerge);
+
+ }
+
+ static void checkLocatedDMNModel(Import i, DMNModel located, DMNModelImpl
model, List<DMNModel> toMerge) {
+ if (located != null) {
+ String iAlias =
Optional.ofNullable(i.getName()).orElse(located.getName());
Review Comment:
iAlias could be renamed to importAlias to improve readability.
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/DMNCompilerImpl.java:
##########
@@ -107,6 +98,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import static org.kie.dmn.core.compiler.DMNImportsUtil.*;
Review Comment:
Instead of asterisk and importing everything, import what exactly is required
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
