Skip to content

Commit

Permalink
kie-issues#270: DMN Runner execution identifies wrongly missing requi…
Browse files Browse the repository at this point in the history 
…rements

Closes: apache/incubator-kie-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.
  • Loading branch information
@jomarko
jomarko committed Aug 24, 2023
1 parent 0722b59 commit 283d23d
Show file tree
Hide file tree
Showing 7 changed files with 173 additions and 8 deletions.
12 changes: 12 additions & 0 deletions kie-dmn/kie-dmn-api/src/main/java/org/kie/dmn/api/core/DMNMessageContainer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
6 changes: 5 additions & 1 deletion kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNResultImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,13 @@ public class DMNResultImpl implements DMNResult, DMNMessageManager {
private Map<String, DMNDecisionResult> 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<>( );
}

Expand Down
14 changes: 12 additions & 2 deletions kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/util/DefaultDMNMessagesManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,21 @@ public class DefaultDMNMessagesManager
// should we use a sorted set instead?
private List<DMNMessage> 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
Expand Down Expand Up @@ -72,7 +83,7 @@ public void addAllUnfiltered(List<? extends DMNMessage> messages) {
@Override
public DMNMessage addMessage(DMNMessage newMessage) {
for( DMNMessage existingMessage : messages ) {
if( isDuplicate( existingMessage, newMessage ) ) {
if( isDuplicateFilteringActive() && isDuplicate( existingMessage, newMessage ) ) {
return existingMessage;
}
}
Expand Down Expand Up @@ -102,5 +113,4 @@ private boolean isDuplicate(DMNMessage existingMsg, DMNMessage newMessage) {
return existingMsg.getMessageType().equals( newMessage.getMessageType() ) &&
existingMsg.getSourceReference() == newMessage.getSourceReference();
}

}
15 changes: 15 additions & 0 deletions kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNRuntimeTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) ");
}
}
124 changes: 124 additions & 0 deletions kie-dmn/kie-dmn-core/src/test/resources/org/kie/dmn/core/habitability.dmn
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,124 @@
<?xml version="1.0" encoding="UTF-8"?>
<dmn:definitions xmlns:dmn="http://www.omg.org/spec/DMN/20180521/MODEL/" xmlns="https://kiegroup.org/dmn/_93836704-04E9-45B6-8D10-51409FEBDF25" xmlns:feel="http://www.omg.org/spec/DMN/20180521/FEEL/" xmlns:kie="http://www.drools.org/kie/dmn/1.2" xmlns:dmndi="http://www.omg.org/spec/DMN/20180521/DMNDI/" xmlns:di="http://www.omg.org/spec/DMN/20180521/DI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" id="_769E1EEB-1FF5-486D-A01A-2AC8B41B56F1" name="habitability" typeLanguage="http://www.omg.org/spec/DMN/20180521/FEEL/" namespace="https://kiegroup.org/dmn/_93836704-04E9-45B6-8D10-51409FEBDF25">
<dmn:extensionElements/>
<dmn:inputData id="_91C73E47-B022-445F-8EB8-03C9D151ADAF" name="oxygene">
<dmn:extensionElements/>
<dmn:variable id="_134AD5DB-0EF6-4919-A1C8-D6DFD01F01E1" name="oxygene" typeRef="number"/>
</dmn:inputData>
<dmn:inputData id="_A69350B8-C033-4625-ACDD-BE3372DDA3A5" name="temperature">
<dmn:extensionElements/>
<dmn:variable id="_695F2636-2215-4D9C-B5CF-82A5EF087242" name="temperature" typeRef="number"/>
</dmn:inputData>
<dmn:decision id="_0699341C-A1BE-4B6D-B8D5-3972D67FCA45" name="habitability">
<dmn:extensionElements/>
<dmn:variable id="_5220852F-82F7-4796-B8DE-9F449B6CC816" name="habitability" typeRef="string"/>
<dmn:informationRequirement id="_A107332C-69A6-4645-945A-A96FA2D651B0">
<dmn:requiredInput href="#_A69350B8-C033-4625-ACDD-BE3372DDA3A5"/>
</dmn:informationRequirement>
<dmn:informationRequirement id="_787EB027-AB8A-4438-B731-D968F4CF79E5">
<dmn:requiredInput href="#_91C73E47-B022-445F-8EB8-03C9D151ADAF"/>
</dmn:informationRequirement>
<dmn:decisionTable id="_C5BB119B-AACF-401F-AF09-5BD330238822" hitPolicy="FIRST" preferredOrientation="Rule-as-Row">
<dmn:input id="_90FF4BF1-1EFA-4FC2-AB19-75E762FD6A73">
<dmn:inputExpression id="_9000F1E8-9B5B-47D2-B2A7-D61598581B5D" typeRef="number">
<dmn:text>oxygene</dmn:text>
</dmn:inputExpression>
</dmn:input>
<dmn:input id="_134AA7DE-CECC-4A11-BFC4-8C12A87257C6">
<dmn:inputExpression id="_9CD36547-D37A-4138-8CFA-42E6B7C09645" typeRef="number">
<dmn:text>temperature</dmn:text>
</dmn:inputExpression>
</dmn:input>
<dmn:output id="_E8EED071-672A-4177-BE50-9FC1CC8AED19"/>
<dmn:annotation name="annotation-1"/>
<dmn:rule id="_F935B705-FDF8-4334-8130-26A9A5635ECA">
<dmn:inputEntry id="_7C9B42E8-87B7-4230-B3D9-14A564ADD00A">
<dmn:text>[10..100]</dmn:text>
</dmn:inputEntry>
<dmn:inputEntry id="_3F897307-6E62-41E5-B841-F986DDE00A20">
<dmn:text>&lt; 40</dmn:text>
</dmn:inputEntry>
<dmn:outputEntry id="_EAB5858C-200C-4468-B5C2-D082B2E21028">
<dmn:text>"somehow doable"</dmn:text>
</dmn:outputEntry>
<dmn:annotationEntry>
<dmn:text/>
</dmn:annotationEntry>
</dmn:rule>
<dmn:rule id="_2A61D35A-722E-4EF0-8B3C-0ACB22EF7BBF">
<dmn:inputEntry id="_53C1F87C-9778-468F-ABB8-D09567F7A24E">
<dmn:text>&lt;10</dmn:text>
</dmn:inputEntry>
<dmn:inputEntry id="_8107798C-00D3-4F11-BDF0-9D509E3904E8">
<dmn:text>&lt; 40</dmn:text>
</dmn:inputEntry>
<dmn:outputEntry id="_570C9F94-13FD-4F98-90B0-3971173AA21D">
<dmn:text>"hardly doable"</dmn:text>
</dmn:outputEntry>
<dmn:annotationEntry>
<dmn:text/>
</dmn:annotationEntry>
</dmn:rule>
<dmn:rule id="_97632101-3E19-4BEF-B511-6E9523A48636">
<dmn:inputEntry id="_3110AE37-454D-4D46-BE90-EB9EBBDA32C9">
<dmn:text>-</dmn:text>
</dmn:inputEntry>
<dmn:inputEntry id="_C109EE05-38D4-468D-BB5C-73BC72BBB0CB">
<dmn:text>&gt;= 40</dmn:text>
</dmn:inputEntry>
<dmn:outputEntry id="_6764C731-E4D0-41FF-B432-F9A8FACB6E1C">
<dmn:text>"too hot"</dmn:text>
</dmn:outputEntry>
<dmn:annotationEntry>
<dmn:text/>
</dmn:annotationEntry>
</dmn:rule>
</dmn:decisionTable>
</dmn:decision>
<dmndi:DMNDI>
<dmndi:DMNDiagram id="_E7C6ED6C-30FF-428D-BBB3-29D7098060DF" name="DRG">
<di:extension>
<kie:ComponentsWidthsExtension>
<kie:ComponentWidths dmnElementRef="_C5BB119B-AACF-401F-AF09-5BD330238822">
<kie:width>50</kie:width>
</kie:ComponentWidths>
</kie:ComponentsWidthsExtension>
</di:extension>
<dmndi:DMNShape id="dmnshape-drg-_91C73E47-B022-445F-8EB8-03C9D151ADAF" dmnElementRef="_91C73E47-B022-445F-8EB8-03C9D151ADAF" isCollapsed="false">
<dmndi:DMNStyle>
<dmndi:FillColor red="255" green="255" blue="255"/>
<dmndi:StrokeColor red="0" green="0" blue="0"/>
<dmndi:FontColor red="0" green="0" blue="0"/>
</dmndi:DMNStyle>
<dc:Bounds x="82" y="96" width="100" height="50"/>
<dmndi:DMNLabel/>
</dmndi:DMNShape>
<dmndi:DMNShape id="dmnshape-drg-_A69350B8-C033-4625-ACDD-BE3372DDA3A5" dmnElementRef="_A69350B8-C033-4625-ACDD-BE3372DDA3A5" isCollapsed="false">
<dmndi:DMNStyle>
<dmndi:FillColor red="255" green="255" blue="255"/>
<dmndi:StrokeColor red="0" green="0" blue="0"/>
<dmndi:FontColor red="0" green="0" blue="0"/>
</dmndi:DMNStyle>
<dc:Bounds x="98" y="231" width="100" height="50"/>
<dmndi:DMNLabel/>
</dmndi:DMNShape>
<dmndi:DMNShape id="dmnshape-drg-_0699341C-A1BE-4B6D-B8D5-3972D67FCA45" dmnElementRef="_0699341C-A1BE-4B6D-B8D5-3972D67FCA45" isCollapsed="false">
<dmndi:DMNStyle>
<dmndi:FillColor red="255" green="255" blue="255"/>
<dmndi:StrokeColor red="0" green="0" blue="0"/>
<dmndi:FontColor red="0" green="0" blue="0"/>
</dmndi:DMNStyle>
<dc:Bounds x="429" y="155" width="100" height="50"/>
<dmndi:DMNLabel/>
</dmndi:DMNShape>
<dmndi:DMNEdge id="dmnedge-drg-_A107332C-69A6-4645-945A-A96FA2D651B0-AUTO-SOURCE-AUTO-TARGET" dmnElementRef="_A107332C-69A6-4645-945A-A96FA2D651B0">
<di:waypoint x="148" y="231"/>
<di:waypoint x="479" y="205"/>
</dmndi:DMNEdge>
<dmndi:DMNEdge id="dmnedge-drg-_787EB027-AB8A-4438-B731-D968F4CF79E5-AUTO-TARGET" dmnElementRef="_787EB027-AB8A-4438-B731-D968F4CF79E5">
<di:waypoint x="132" y="121"/>
<di:waypoint x="479" y="155"/>
</dmndi:DMNEdge>
</dmndi:DMNDiagram>
</dmndi:DMNDI>
</dmn:definitions>
8 changes: 4 additions & 4 deletions kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/DMNValidatorImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ public List<DMNMessage> theseModels(File... files) {

@Override
public List<DMNMessage> 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<DMNResource> models = new ArrayList<>();
for (Resource r : resources) {
try {
Expand Down Expand Up @@ -263,7 +263,7 @@ public List<DMNMessage> theseModels(Reader... readers) {

@Override
public List<DMNMessage> 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,
Expand Down Expand Up @@ -360,7 +360,7 @@ public List<DMNMessage> validate(Definitions dmnModel) {

@Override
public List<DMNMessage> validate(Definitions dmnModel, Validation... options) {
DMNMessageManager results = new DefaultDMNMessagesManager( null );
DMNMessageManager results = new DefaultDMNMessagesManager( null, true );
EnumSet<Validation> flags = EnumSet.copyOf( Arrays.asList( options ) );
if( flags.contains( VALIDATE_SCHEMA ) ) {
MsgUtil.reportMessage( LOG,
Expand Down Expand Up @@ -413,7 +413,7 @@ public List<DMNMessage> validate(Resource resource) {

@Override
public List<DMNMessage> validate(Resource resource, Validation... options) {
DMNMessageManager results = new DefaultDMNMessagesManager( resource );
DMNMessageManager results = new DefaultDMNMessagesManager( resource, true );
EnumSet<Validation> 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,
Expand Down
2 changes: 1 addition & 1 deletion kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/MessageReporter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down

0 comments on commit 283d23d

Please sign in to comment.