Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Incubator-kie-issues#1619] Correctly manage execution of invalid models #6200

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions kie-dmn/kie-dmn-core/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -276,5 +276,35 @@
<filtering>true</filtering>
</testResource>
</testResources>
<plugins>
<!-- Unpack DMN resources from from org.kie:kie-dmn-test-resources -->
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<executions>
<execution>
<id>unpack</id>
<phase>generate-test-resources</phase>
<goals>
<goal>unpack</goal>
</goals>
<configuration>
<artifactItems>
<artifactItem>
<groupId>org.kie</groupId>
<artifactId>kie-dmn-test-resources</artifactId>
<version>${project.version}</version>
<type>jar</type>
<classifier>tests</classifier>
<overWrite>true</overWrite>
<outputDirectory>${project.build.directory}/test-classes/org/kie/dmn/core</outputDirectory>
<includes>**/*.dmn</includes>
</artifactItem>
</artifactItems>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@
*/
package org.kie.dmn.core.impl;

import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
Expand All @@ -30,6 +32,7 @@
import javax.xml.namespace.QName;

import org.drools.kiesession.rulebase.InternalKnowledgeBase;
import org.kie.api.builder.Message;
import org.kie.dmn.api.core.DMNContext;
import org.kie.dmn.api.core.DMNDecisionResult;
import org.kie.dmn.api.core.DMNMessage;
Expand Down Expand Up @@ -111,6 +114,7 @@ public DMNResult evaluateAll(DMNModel model, DMNContext context) {
Objects.requireNonNull(model, () -> MsgUtil.createMessage(Msg.PARAM_CANNOT_BE_NULL, "model"));
Objects.requireNonNull(context, () -> MsgUtil.createMessage(Msg.PARAM_CANNOT_BE_NULL, "context"));
boolean performRuntimeTypeCheck = performRuntimeTypeCheck(model);
identifyDecisionErrors(model);
DMNResultImpl result = createResult( model, context );
DMNRuntimeEventManagerUtils.fireBeforeEvaluateAll( eventManager, model, result );
// the engine should evaluate all Decisions belonging to the "local" model namespace, not imported decision explicitly.
Expand Down Expand Up @@ -148,6 +152,7 @@ public DMNResult evaluateByName( DMNModel model, DMNContext context, String... d
if (decisionNames.length == 0) {
throw new IllegalArgumentException(MsgUtil.createMessage(Msg.PARAM_CANNOT_BE_EMPTY, "decisionNames"));
}
identifyDecisionErrors(model, decisionNames);
final DMNResultImpl result = createResult( model, context );
for (String name : decisionNames) {
evaluateByNameInternal( model, context, result, name );
Expand Down Expand Up @@ -184,6 +189,7 @@ public DMNResult evaluateById( DMNModel model, DMNContext context, String... dec
if (decisionIds.length == 0) {
throw new IllegalArgumentException(MsgUtil.createMessage(Msg.PARAM_CANNOT_BE_EMPTY, "decisionIds"));
}
identifyDecisionErrors(model, decisionIds);
final DMNResultImpl result = createResult( model, context );
for ( String id : decisionIds ) {
evaluateByIdInternal( model, context, result, id );
Expand Down Expand Up @@ -754,6 +760,15 @@ private static String getDependencyIdentifier(DMNNode callerNode, DMNNode node)

}

private static void identifyDecisionErrors(DMNModel model, String... decisions) {
List<DMNMessage> errorMessages = model.getMessages(DMNMessage.Severity.ERROR);
List<String> identifiedErrors = errorMessages.stream().map(Message::getText)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HI @AthiraHari77
Many thanks for the PR.
I'm only slightly concerned about this assertion.
The logic here is that

  1. decisions could be decisionsId or decisionName (IINW)
  2. if an error message contains one of them, then the decisions it relates to is "wrong"

I'm afraid there could be some cases where an error message contains the decisionId or decisionName, but that the error is not in the decision itself (e.g. another element that refer that decision)

.filter(messages -> decisions.length == 0 || Arrays.stream(decisions).anyMatch(messages::contains)).collect(Collectors.toList());
if (!identifiedErrors.isEmpty()) {
throw new IllegalStateException(String.join(", ", identifiedErrors));
}
}

public boolean performRuntimeTypeCheck(DMNModel model) {
Objects.requireNonNull(model, () -> MsgUtil.createMessage(Msg.PARAM_CANNOT_BE_NULL, "model"));
return overrideRuntimeTypeCheck || ((DMNModelImpl) model).isRuntimeTypeCheck();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,13 +18,23 @@
*/
package org.kie.dmn.core.internal.utils;

import java.io.File;
import java.util.Collections;

import org.drools.util.FileUtils;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.kie.api.io.Resource;
import org.kie.dmn.api.core.DMNContext;
import org.kie.dmn.api.core.DMNModel;
import org.kie.dmn.api.core.DMNResult;
import org.kie.dmn.api.core.DMNRuntime;
import org.kie.dmn.core.api.DMNFactory;
import org.kie.dmn.core.impl.DMNRuntimeImpl;
import org.kie.internal.io.ResourceFactory;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

class DMNRuntimeBuilderTest {

Expand All @@ -43,4 +53,198 @@ void buildFromConfiguration() {
.fromResources(Collections.emptyList()).getOrElseThrow(RuntimeException::new);
assertThat(retrieved).isNotNull();
}

@Test
void fromDefaultsMultipleDecisionWithoutInputDataReference() {
File modelFile = FileUtils.getFile("Invalid_decisions_model.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_BDC29BCF-B5DC-4AD7-8A5F-43DC08780F97";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_1A4BD262-7672-4887-9F25-986EE5277D16");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >= 18' for name 'Can Drive?' on node 'Can Drive?': syntax error near 'Age' (DMN id: _563E78C7-EFD1-4109-9F30-B14922EF68DF, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateAll(dmnModel, context))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
void evaluateMultipleDecisionModel() {
File modelFile = FileUtils.getFile("MultipleDecision.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_CADD03FC-4ABD-46D2-B631-E7FDE384D6D7";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_54AA2CFA-2374-4FCE-8F16-B594DFF87EBE");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
DMNResult dmnResult = dmnRuntime.evaluateAll(dmnModel, context);
assertThat(dmnResult).isNotNull();
}

@Test
void evaluateWrongDecisionWithoutInputDataReferencesByName() {
File modelFile = FileUtils.getFile("Invalid_decisions_model.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_BDC29BCF-B5DC-4AD7-8A5F-43DC08780F97";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_1A4BD262-7672-4887-9F25-986EE5277D16");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >= 18' for name 'Can Drive?' on node 'Can Drive?': syntax error near 'Age' (DMN id: _563E78C7-EFD1-4109-9F30-B14922EF68DF, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateByName(dmnModel, context, "Can Drive"))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would go for the more readable assertThatIllegalStateExceptionIsThrownBy(.... Consider this also in the other tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pibizza Updated the test cases as specified

.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
void evaluateRightDecisionWithoutInputDataReferencesByName() {
File modelFile = FileUtils.getFile("Invalid_decisions_model.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_BDC29BCF-B5DC-4AD7-8A5F-43DC08780F97";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_1A4BD262-7672-4887-9F25-986EE5277D16");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
DMNResult dmnResult = dmnRuntime.evaluateByName(dmnModel, context, "Can Vote?");
assertThat(dmnResult).isNotNull();
}

@Test
void evaluateWrongDecisionWithoutInputDataReferencesById() {
File modelFile = FileUtils.getFile("Invalid_decisions_model.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_BDC29BCF-B5DC-4AD7-8A5F-43DC08780F97";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_1A4BD262-7672-4887-9F25-986EE5277D16");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >= 18' for name 'Can Drive?' on node 'Can Drive?': syntax error near 'Age' (DMN id: _563E78C7-EFD1-4109-9F30-B14922EF68DF, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateById(dmnModel, context, "_563E78C7-EFD1-4109-9F30-B14922EF68DF"))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
void evaluateRightDecisionWithoutInputDataReferencesById() {
File modelFile = FileUtils.getFile("Invalid_decisions_model.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_BDC29BCF-B5DC-4AD7-8A5F-43DC08780F97";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_1A4BD262-7672-4887-9F25-986EE5277D16");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
DMNResult dmnResult = dmnRuntime.evaluateById(dmnModel, context, "_7ACCB8BC-A382-4530-B8EE-AD32D187FD8B");
assertThat(dmnResult).isNotNull();
}

@Test
void evaluateDecisionWithInvalidFeelError() {
File modelFile = FileUtils.getFile("InvalidFeel.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_9DF86C49-C80A-4744-9F50-BCE65A89C98C";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_33900B8B-73DD-4D1E-87E9-F6C3FE534B43");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set("Person Age", 24);
String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >?= 18' for name 'Can Drive' on node 'Can Drive': Unknown variable '?' (DMN id: _F477B6E0-C617-4087-9648-DE25A711C5F9, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateByName(dmnModel, context, "Can Drive"))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);

}

@Test
void evaluateMultipleErrorDecision() {
File modelFile = FileUtils.getFile("MultipleErrorDecision.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_36ADF828-4BE5-41E1-8808-6245D13C6AB4";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_45A15AF7-9910-4EAD-B249-8AE218B3BF43");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
String errorMessage = "DMN: Error compiling FEEL expression 'Age + 20.?>' for name 'ContextEntry-1' on node 'Can Vote?': syntax error near '+' (DMN id: _B7D17199-0568-40EE-94D0-FDFAB0E97868, Error compiling the referenced FEEL expression) , DMN: Error compiling FEEL expression 'if Age > 25 \"YES\" elsesss \"NO\"' for name 'Can Vote?' on node 'Can Vote?': syntax error near '\"YES\"' (DMN id: _59E71393-14B3-405D-A0B4-3C1E6562823F, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateByName(dmnModel, context, "Can Vote"))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
void evaluateMultipleErrorModel() {
File modelFile = FileUtils.getFile("MultipleError.dmn");
assertThat(modelFile).isNotNull().exists();
Resource modelResource = ResourceFactory.newFileResource(modelFile);
DMNRuntime dmnRuntime = DMNRuntimeBuilder.fromDefaults().buildConfiguration()
.fromResources(Collections.singletonList(modelResource)).getOrElseThrow(RuntimeException::new);
assertThat(dmnRuntime).isNotNull();
String nameSpace = "https://kie.org/dmn/_231A34DE-33C6-4787-A51F-228C910D5EAF";

final DMNModel dmnModel = dmnRuntime.getModel(
nameSpace,
"DMN_DC99A8C4-4524-407D-B3D1-577442AED995");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
String errorMessage = "DMN: Error compiling FEEL expression 'Person Age >= 18' for name 'Can Vote?' on node 'Can Vote?': syntax error near 'Age' (DMN id: _E3EF0CCA-0F1E-42B1-8C65-124D77C07E38, Error compiling the referenced FEEL expression) , DMN: Error compiling FEEL expression 'Person Age >=18' for name 'Can Drive?' on node 'Can Drive?': syntax error near 'Age' (DMN id: _B2F31CDD-29D1-4C20-93B8-8FB8E11E1FFC, Error compiling the referenced FEEL expression) ";
assertThatThrownBy(() -> dmnRuntime.evaluateByName(dmnModel, context, "Can Vote?", "Can Drive?"))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
<?xml version="1.0" encoding="UTF-8" ?>
<definitions xmlns="https://www.omg.org/spec/DMN/20230324/MODEL/" expressionLanguage="https://www.omg.org/spec/DMN/20230324/FEEL/" namespace="https://kie.org/dmn/_9DF86C49-C80A-4744-9F50-BCE65A89C98C" id="_A3750F9C-1684-4E92-9829-C4E7448E618B" name="DMN_33900B8B-73DD-4D1E-87E9-F6C3FE534B43" xmlns:dmndi="https://www.omg.org/spec/DMN/20230324/DMNDI/" xmlns:dc="http://www.omg.org/spec/DMN/20180521/DC/" xmlns:di="http://www.omg.org/spec/DMN/20180521/DI/" xmlns:kie="https://kie.org/dmn/extensions/1.0">
<inputData name="Person Age" id="_66BAE7BF-A7DA-410F-B3D9-C0497CC8383A">
<variable name="Person Age" id="_B1BE45B0-A2DE-45A9-932D-B6ED19BE1115" typeRef="number" />
</inputData>
<decision name="Can Drive" id="_FCD824F2-E680-4D08-86AA-A6B38D12FEDF">
<variable id="_B6333FE5-44EC-4E30-AAF2-B888FBCBA9A4" typeRef="boolean" name="Can Drive" />
<informationRequirement id="_A1D1397A-3D2C-43F1-96CB-BFC7373B81DA">
<requiredInput href="#_66BAE7BF-A7DA-410F-B3D9-C0497CC8383A" />
</informationRequirement>
<literalExpression id="_F477B6E0-C617-4087-9648-DE25A711C5F9" typeRef="boolean" label="Can Drive">
<text>Person Age &gt;?= 18</text>
</literalExpression>
</decision>
<dmndi:DMNDI>
<dmndi:DMNDiagram id="_A36FB014-BA47-4B2E-AAE3-079E6A4CD418" name="Default DRD" useAlternativeInputDataShape="false">
<di:extension>
<kie:ComponentsWidthsExtension>
<kie:ComponentWidths dmnElementRef="_F477B6E0-C617-4087-9648-DE25A711C5F9">
<kie:width>190</kie:width>
</kie:ComponentWidths>
</kie:ComponentsWidthsExtension>
</di:extension>
<dmndi:DMNShape id="_7BEF4150-375E-4747-915B-1BDE22711237" dmnElementRef="_66BAE7BF-A7DA-410F-B3D9-C0497CC8383A" isCollapsed="false" isListedInputData="false">
<dc:Bounds x="100" y="300" width="160" height="80" />
</dmndi:DMNShape>
<dmndi:DMNShape id="_B901E181-D6EA-4735-9286-B3B51A10534F" dmnElementRef="_FCD824F2-E680-4D08-86AA-A6B38D12FEDF" isCollapsed="false" isListedInputData="false">
<dc:Bounds x="100" y="100" width="160" height="80" />
</dmndi:DMNShape>
<dmndi:DMNEdge id="_0D5B4E0B-BDD4-4FB8-BC32-E12FF9A049D0-AUTO-TARGET" dmnElementRef="_A1D1397A-3D2C-43F1-96CB-BFC7373B81DA" sourceElement="_7BEF4150-375E-4747-915B-1BDE22711237" targetElement="_B901E181-D6EA-4735-9286-B3B51A10534F">
<di:waypoint x="180" y="340" />
<di:waypoint x="180" y="140" />
</dmndi:DMNEdge>
</dmndi:DMNDiagram>
</dmndi:DMNDI>
</definitions>
Loading
Loading