From 283d23de91ce85e58121202fdcdcdd8cf61876fc Mon Sep 17 00:00:00 2001 From: jomarko Date: Mon, 14 Aug 2023 14:25:59 +0200 Subject: [PATCH] kie-issues#270: DMN Runner execution identifies wrongly missing requirements Closes: https://github.com/kiegroup/kie-issues/issues/270 The problem was not in the DMN Runner ui component but in the engine that executes the validation. The mechanism for storing messages includes filtering of messages. however this filtering inappropriately assumed messages bellow like equal: - Reqiured dependency 'A' not found on node 'N' - Required dependency 'B' not found on ndoe 'N' ``` @Override public DMNMessage addMessage(DMNMessage newMessage) { for( DMNMessage existingMessage : messages ) { if( isDuplicate( existingMessage, newMessage ) ) { return existingMessage; } } this.messages.add( newMessage ); return newMessage; } ``` `isDuplicate` didn't take into account all needed attributes of the `message` We decided to add option to turn off this filtering of messages on backround. As part of this PR we turn it of only for `DMNResultImpl`. We keep it unchanged on other places. --- .../kie/dmn/api/core/DMNMessageContainer.java | 12 ++ .../org/kie/dmn/core/impl/DMNResultImpl.java | 6 +- .../core/util/DefaultDMNMessagesManager.java | 14 +- .../java/org/kie/dmn/core/DMNRuntimeTest.java | 15 +++ .../org/kie/dmn/core/habitability.dmn | 124 ++++++++++++++++++ .../kie/dmn/validation/DMNValidatorImpl.java | 8 +- .../kie/dmn/validation/MessageReporter.java | 2 +- 7 files changed, 173 insertions(+), 8 deletions(-) create mode 100644 kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/habitability.dmn diff --git a/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/core/DMNMessageContainer.java b/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/core/DMNMessageContainer.java index 5c2ad05c7a7d..78d3b16d8c82 100644 --- a/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/core/DMNMessageContainer.java +++ b/kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/core/DMNMessageContainer.java @@ -7,6 +7,18 @@ * DMNResults and DMNModel */ public interface DMNMessageContainer { + + /** + * DMNMessageContainer may contain many similar DMNMessages like + * - Required Decision 'a' not found on node 'b' ... + * - Required dependency 'a' not found on node 'b' ... + * + * They are different in text but same by meaning, so by default, we try to keep single instance of similar message + */ + default boolean isDuplicateFilteringActive() { + return true; + } + /** * Returns a list of all the messages produced * during the DMN service invocation. diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNResultImpl.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNResultImpl.java index 25ae5c264427..305d5f0fc158 100644 --- a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNResultImpl.java +++ b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNResultImpl.java @@ -38,9 +38,13 @@ public class DMNResultImpl implements DMNResult, DMNMessageManager { private Map decisionResults; private final DMNModel model; + /** + * Please notice we setup `DefaultDMNMessagesManager` with `isDuplicateFilteringActive` as `false` + * It is because of https://github.com/kiegroup/kie-issues/issues/270 + */ public DMNResultImpl(DMNModel model) { this.model = model; - messages = new DefaultDMNMessagesManager(model != null ? model.getResource() : null); + messages = new DefaultDMNMessagesManager(model != null ? model.getResource() : null, false); decisionResults = new HashMap<>( ); } diff --git a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/DefaultDMNMessagesManager.java b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/DefaultDMNMessagesManager.java index 0c30dfaed304..77f8a4174dd2 100644 --- a/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/DefaultDMNMessagesManager.java +++ b/kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/DefaultDMNMessagesManager.java @@ -35,10 +35,21 @@ public class DefaultDMNMessagesManager // should we use a sorted set instead? private List messages; private String path; + private boolean isDuplicateFilteringActive; public DefaultDMNMessagesManager(Resource resource) { + new DefaultDMNMessagesManager(resource, true); + } + + public DefaultDMNMessagesManager(Resource resource, final boolean isDuplicateFilteringActive) { this.messages = new ArrayList<>(); this.path = resource != null ? resource.getSourcePath() : null; + this.isDuplicateFilteringActive = isDuplicateFilteringActive; + } + + @Override + public boolean isDuplicateFilteringActive() { + return isDuplicateFilteringActive; } @Override @@ -72,7 +83,7 @@ public void addAllUnfiltered(List messages) { @Override public DMNMessage addMessage(DMNMessage newMessage) { for( DMNMessage existingMessage : messages ) { - if( isDuplicate( existingMessage, newMessage ) ) { + if( isDuplicateFilteringActive() && isDuplicate( existingMessage, newMessage ) ) { return existingMessage; } } @@ -102,5 +113,4 @@ private boolean isDuplicate(DMNMessage existingMsg, DMNMessage newMessage) { return existingMsg.getMessageType().equals( newMessage.getMessageType() ) && existingMsg.getSourceReference() == newMessage.getSourceReference(); } - } diff --git a/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java b/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java index fe302231d7f5..3700ee8f5f89 100644 --- a/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java +++ b/kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java @@ -3172,4 +3172,19 @@ public void testSoundLevelAllowNullItemDef() { assertThat(dmnResult.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnResult.getMessages())).isFalse(); assertThat(dmnResult.getDecisionResultByName("Evaluation").getResult()).isEqualTo("Unknown"); } + + @Test + public void testKieIssue270() { + final DMNRuntime runtime = DMNRuntimeUtil.createRuntime("habitability.dmn", this.getClass() ); + final DMNModel dmnModel = runtime.getModel("https://kiegroup.org/dmn/_93836704-04E9-45B6-8D10-51409FEBDF25", "habitability" ); + assertThat(dmnModel).isNotNull(); + assertThat(dmnModel.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnModel.getMessages())).isFalse(); + + final DMNContext context = DMNFactory.newContext(); + + final DMNResult dmnResult = runtime.evaluateAll(dmnModel, context); + assertThat(dmnResult.hasErrors()).as(DMNRuntimeUtil.formatMessages(dmnResult.getMessages())).isTrue(); + assertThat(dmnResult.getMessages()).hasSize(2); + assertThat(dmnResult.getMessages()).extracting(DMNMessage::getText).contains("DMN: Required dependency 'temperature' not found on node 'habitability' (DMN id: _0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) ", "DMN: Required dependency 'oxygene' not found on node 'habitability' (DMN id: _0699341C-A1BE-4B6D-B8D5-3972D67FCA45, The referenced node was not found) "); + } } diff --git a/kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/habitability.dmn b/kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/habitability.dmn new file mode 100644 index 000000000000..c60143d047f9 --- /dev/null +++ b/kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/habitability.dmn @@ -0,0 +1,124 @@ + + + + + + + + + + + + + + + + + + + + + + + + oxygene + + + + + temperature + + + + + + + [10..100] + + + < 40 + + + "somehow doable" + + + + + + + + <10 + + + < 40 + + + "hardly doable" + + + + + + + + - + + + >= 40 + + + "too hot" + + + + + + + + + + + + + 50 + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file diff --git a/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/DMNValidatorImpl.java b/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/DMNValidatorImpl.java index 700b5aff3fc2..596f351e0f0d 100644 --- a/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/DMNValidatorImpl.java +++ b/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/DMNValidatorImpl.java @@ -208,7 +208,7 @@ public List theseModels(File... files) { @Override public List theseModels(Resource... resources) { - DMNMessageManager results = new DefaultDMNMessagesManager( null ); // this collector span multiple resources. + DMNMessageManager results = new DefaultDMNMessagesManager( null , true); // this collector span multiple resources. List models = new ArrayList<>(); for (Resource r : resources) { try { @@ -263,7 +263,7 @@ public List theseModels(Reader... readers) { @Override public List theseModels(Definitions... models) { - DMNMessageManager results = new DefaultDMNMessagesManager( null ); + DMNMessageManager results = new DefaultDMNMessagesManager( null, true ); if (flags.contains(VALIDATE_SCHEMA)) { MsgUtil.reportMessage(LOG, DMNMessage.Severity.ERROR, @@ -360,7 +360,7 @@ public List validate(Definitions dmnModel) { @Override public List validate(Definitions dmnModel, Validation... options) { - DMNMessageManager results = new DefaultDMNMessagesManager( null ); + DMNMessageManager results = new DefaultDMNMessagesManager( null, true ); EnumSet flags = EnumSet.copyOf( Arrays.asList( options ) ); if( flags.contains( VALIDATE_SCHEMA ) ) { MsgUtil.reportMessage( LOG, @@ -413,7 +413,7 @@ public List validate(Resource resource) { @Override public List validate(Resource resource, Validation... options) { - DMNMessageManager results = new DefaultDMNMessagesManager( resource ); + DMNMessageManager results = new DefaultDMNMessagesManager( resource, true ); EnumSet flags = EnumSet.copyOf( Arrays.asList( options ) ); try { // We get passed a Resource, which might be constructed from a Reader, so we have only 1-time opportunity to be sure to read it successfully, diff --git a/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/MessageReporter.java b/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/MessageReporter.java index 0414515291d6..c0c7cf1b1169 100644 --- a/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/MessageReporter.java +++ b/kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/MessageReporter.java @@ -35,7 +35,7 @@ public class MessageReporter { public MessageReporter(DMNResource dmnResource) { Resource resource = dmnResource != null ? dmnResource.getResAndConfig().getResource() : null; - this.messages = new DefaultDMNMessagesManager(resource); + this.messages = new DefaultDMNMessagesManager(resource, true); this.path = resource != null ? resource.getSourcePath() : null; }