From ddb72c6751c6a89ea5ef187b93516303a3da7e5b Mon Sep 17 00:00:00 2001 From: Yeser Amer Date: Wed, 10 Jul 2024 17:19:29 +0200 Subject: [PATCH] [incubator-kie-issues#1150] Improve Import Resolver error messages to be more user friendly (#6014) * Improved Error Messages + Logs * dependabot.yml fixed * dependabot.yml fixed * Change Request * Minor change * Tests fixed --- .../core/compiler/ImportDMNResolverUtil.java | 80 +++++++++++++------ .../compiler/ImportDMNResolverUtilTest.java | 6 ++ 2 files changed, 61 insertions(+), 25 deletions(-) diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/ImportDMNResolverUtil.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/ImportDMNResolverUtil.java index 4a15ef4d199..be11db65f25 100644 --- a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/ImportDMNResolverUtil.java +++ b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/compiler/ImportDMNResolverUtil.java @@ -26,58 +26,88 @@ import javax.xml.namespace.QName; import org.kie.dmn.feel.util.Either; +import org.kie.dmn.model.api.Definitions; import org.kie.dmn.model.api.Import; import org.kie.dmn.model.api.NamespaceConsts; import org.kie.dmn.model.v1_1.TImport; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class ImportDMNResolverUtil { + private static final Logger LOGGER = LoggerFactory.getLogger(ImportDMNResolverUtil.class); + private ImportDMNResolverUtil() { // No constructor for util class. } - public static Either resolveImportDMN(Import _import, Collection all, Function idExtractor) { - final String iNamespace = _import.getNamespace(); - final String iName = _import.getName(); - final String iModelName = _import.getAdditionalAttributes().get(TImport.MODELNAME_QNAME); - List allInNS = all.stream() - .filter(m -> idExtractor.apply(m).getNamespaceURI().equals(iNamespace)) - .collect(Collectors.toList()); - if (allInNS.size() == 1) { - T located = allInNS.get(0); + public static Either resolveImportDMN(Import importElement, Collection dmns, Function idExtractor) { + final String importerDMNNamespace = ((Definitions) importElement.getParent()).getNamespace(); + final String importerDMNName = ((Definitions) importElement.getParent()).getName(); + final String importNamespace = importElement.getNamespace(); + final String importName = importElement.getName(); + final String importLocationURI = importElement.getLocationURI(); // This is optional + final String importModelName = importElement.getAdditionalAttributes().get(TImport.MODELNAME_QNAME); + + LOGGER.debug("Resolving an Import in DMN Model with name={} and namespace={}. " + + "Importing a DMN model with namespace={} name={} locationURI={}, modelName={}", + importerDMNNamespace, importerDMNName, importNamespace, importName, importLocationURI, importModelName); + + List matchingDMNList = dmns.stream() + .filter(m -> idExtractor.apply(m).getNamespaceURI().equals(importNamespace)) + .collect(Collectors.toList()); + if (matchingDMNList.size() == 1) { + T located = matchingDMNList.get(0); // Check if the located DMN Model in the NS, correspond for the import `drools:modelName`. - if (iModelName == null || idExtractor.apply(located).getLocalPart().equals(iModelName)) { + if (importModelName == null || idExtractor.apply(located).getLocalPart().equals(importModelName)) { + LOGGER.debug("DMN Model with name={} and namespace={} successfully imported a DMN " + + "with namespace={} name={} locationURI={}, modelName={}", + importerDMNNamespace, importerDMNName, importNamespace, importName, importLocationURI, importModelName); return Either.ofRight(located); } else { - return Either.ofLeft(String.format("While importing DMN for namespace: %s, name: %s, modelName: %s, located within namespace only %s but does not match for the actual name", - iNamespace, iName, iModelName, - idExtractor.apply(located))); + LOGGER.error("DMN Model with name={} and namespace={} can't import a DMN with namespace={}, name={}, modelName={}, " + + "located within namespace only {} but does not match for the actual modelName", + importerDMNNamespace, importerDMNName, importNamespace, importName, importModelName, idExtractor.apply(located)); + return Either.ofLeft(String.format( + "DMN Model with name=%s and namespace=%s can't import a DMN with namespace=%s, name=%s, modelName=%s, " + + "located within namespace only %s but does not match for the actual modelName", + importerDMNNamespace, importerDMNName, importNamespace, importName, importModelName, idExtractor.apply(located))); } } else { - List usingNSandName = allInNS.stream() - .filter(m -> idExtractor.apply(m).getLocalPart().equals(iModelName)) - .collect(Collectors.toList()); + List usingNSandName = matchingDMNList.stream() + .filter(dmn -> idExtractor.apply(dmn).getLocalPart().equals(importModelName)) + .toList(); if (usingNSandName.size() == 1) { + LOGGER.debug("DMN Model with name={} and namespace={} successfully imported a DMN " + + "with namespace={} name={} locationURI={}, modelName={}", + importerDMNNamespace, importerDMNName, importNamespace, importName, importLocationURI, importModelName); return Either.ofRight(usingNSandName.get(0)); - } else if (usingNSandName.size() == 0) { - return Either.ofLeft(String.format("Could not locate required dependency while importing DMN for namespace: %s, name: %s, modelName: %s.", - iNamespace, iName, iModelName)); + } else if (usingNSandName.isEmpty()) { + LOGGER.error("DMN Model with name={} and namespace={} failed to import a DMN with namespace={} name={} locationURI={}, modelName={}.", + importerDMNNamespace, importerDMNName, importNamespace, importName, importLocationURI, importModelName); + return Either.ofLeft(String.format( + "DMN Model with name=%s and namespace=%s failed to import a DMN with namespace=%s name=%s locationURI=%s, modelName=%s. ", + importerDMNNamespace, importerDMNName, importNamespace, importName, importLocationURI, importModelName)); } else { - return Either.ofLeft(String.format("While importing DMN for namespace: %s, name: %s, modelName: %s, could not locate required dependency within: %s.", - iNamespace, iName, iModelName, - allInNS.stream().map(idExtractor).collect(Collectors.toList()))); + LOGGER.error("DMN Model with name={} and namespace={} detected a collision ({} elements) trying to import a DMN with namespace={} name={} locationURI={}, modelName={}", + importerDMNNamespace, importerDMNName, usingNSandName.size(), importNamespace, importName, importLocationURI, importModelName); + return Either.ofLeft(String.format( + "DMN Model with name=%s and namespace=%s detected a collision trying to import a DMN with %s namespace, " + + "%s name and modelName %s. There are %s DMN files with the same namespace in your project. " + + "Please change the DMN namespaces and make them unique to fix this issue.", + importerDMNNamespace, importerDMNName, importNamespace, importName, importModelName, usingNSandName.size())); } } } - public static enum ImportType { + public enum ImportType { UNKNOWN, DMN, PMML; } - public static ImportType whichImportType(Import _import) { - switch (_import.getImportType()) { + public static ImportType whichImportType(Import importElement) { + switch (importElement.getImportType()) { case org.kie.dmn.model.v1_1.KieDMNModelInstrumentedBase.URI_DMN: case "http://www.omg.org/spec/DMN1-2Alpha/20160929/MODEL": case org.kie.dmn.model.v1_2.KieDMNModelInstrumentedBase.URI_DMN: diff --git a/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/compiler/ImportDMNResolverUtilTest.java b/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/compiler/ImportDMNResolverUtilTest.java index 87156dc70c5..6368dee3cc9 100644 --- a/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/compiler/ImportDMNResolverUtilTest.java +++ b/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/compiler/ImportDMNResolverUtilTest.java @@ -28,10 +28,12 @@ import org.junit.jupiter.api.Test; import org.kie.dmn.feel.util.Either; +import org.kie.dmn.model.api.Definitions; import org.kie.dmn.model.api.Import; import org.kie.dmn.model.v1_1.TImport; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.mock; class ImportDMNResolverUtilTest { @@ -174,6 +176,10 @@ private Import makeImport(final String namespace, final String name, final Strin addAttributes.put(TImport.MODELNAME_QNAME, modelName); } i.setAdditionalAttributes(addAttributes); + final Definitions definitions = mock(Definitions.class); + definitions.setNamespace("ParentDMNNamespace"); + definitions.setName("ParentDMN"); + i.setParent(definitions); return i; }