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 Runne rui component but in the engine that executes the validation. The mechanism for storing messages contained a bug.
```
    @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`
  • Loading branch information
@jomarko
jomarko committed Aug 23, 2023
1 parent 29502db commit 1088ca1
Show file tree
Hide file tree
Showing 8 changed files with 173 additions and 12 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: 3 additions & 3 deletions kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/impl/DMNModelImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,13 +106,13 @@ public DMNModelImpl(Definitions definitions) {
this.definitions = definitions;
wireTypeRegistry(definitions);
importChain = new ImportChain(this);
messages = new DefaultDMNMessagesManager(null);
messages = new DefaultDMNMessagesManager(null, true);
}

public DMNModelImpl(Definitions dmndefs, Resource resource) {
this(dmndefs);
this.setResource(resource);
messages = new DefaultDMNMessagesManager(resource);
messages = new DefaultDMNMessagesManager(resource, true);
}

private void wireTypeRegistry(Definitions definitions) {
Expand Down Expand Up @@ -436,7 +436,7 @@ public void writeExternal(ObjectOutput out) throws IOException {
public void readExternal(ObjectInput in) throws IOException, ClassNotFoundException {
this.serializedAs = (SerializationFormat) in.readObject();
this.resource = (Resource) in.readObject();
this.messages = new DefaultDMNMessagesManager(this.resource);
this.messages = new DefaultDMNMessagesManager(this.resource, true);
String xml = (String) in.readObject();

if ( !(in instanceof DroolsObjectInputStream) ) {
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
12 changes: 9 additions & 3 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,17 @@ public class DefaultDMNMessagesManager
// should we use a sorted set instead?
private List<DMNMessage> messages;
private String path;
private boolean isDuplicateFilteringActive;

public DefaultDMNMessagesManager(Resource resource) {
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 +79,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 +109,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 1088ca1

Please sign in to comment.