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 14 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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.Assertions.assertThatIllegalStateException;

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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateAll(dmnModel, context);
}).withMessage(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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Drive");
}).withMessage(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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateById(dmnModel, context, "_563E78C7-EFD1-4109-9F30-B14922EF68DF");
}).withMessage(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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Drive");
}).withMessage(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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Vote");
}).withMessage(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) ";
assertThatIllegalStateException().isThrownBy(() -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Vote?", "Can Drive?");
}).withMessage(errorMessage);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
<?xml version="1.0" encoding="UTF-8" ?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<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