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
Changes from 1 commit
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
Prev Previous commit
Next Next commit
[incubator-kie-issues#1619] Code refactoring
  • Loading branch information
athira committed Dec 16, 2024
commit b51cb9369d5ce8b0119fd1b2bfcc342c2dce1c18
2 changes: 0 additions & 2 deletions kie-dmn/kie-dmn-core/pom.xml
Original file line number Diff line number Diff line change
@@ -37,7 +37,6 @@

<properties>
<java.module.name>org.kie.dmn.core</java.module.name>
<dependency-plugin.version>3.6.1</dependency-plugin.version>
</properties>

<dependencyManagement>
@@ -282,7 +281,6 @@
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-dependency-plugin</artifactId>
<version>${dependency-plugin.version}</version>
<executions>
<execution>
<id>unpack</id>
Original file line number Diff line number Diff line change
@@ -153,13 +153,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"));
}
List<DMNMessage> errorMessages = model.getMessages(DMNMessage.Severity.ERROR);
errorMessages.stream().filter(message -> Arrays.stream(decisionNames).anyMatch(decision -> message.getText().contains(decision)))
.findAny().ifPresent(message -> {
throw new IllegalStateException(message.getText());
});

System.out.println(errorMessages);
identifyDecisionErrors(model, decisionNames);
final DMNResultImpl result = createResult( model, context );
for (String name : decisionNames) {
evaluateByNameInternal( model, context, result, name );
@@ -196,11 +190,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"));
}
List<DMNMessage> errorMessages = model.getMessages(DMNMessage.Severity.ERROR);
errorMessages.stream().filter(message -> Arrays.stream(decisionIds).anyMatch(decision -> message.getText().contains(decision)))
.findAny().ifPresent(message -> {
throw new IllegalStateException(message.getText());
});
identifyDecisionErrors(model, decisionIds);
final DMNResultImpl result = createResult( model, context );
for ( String id : decisionIds ) {
evaluateByIdInternal( model, context, result, id );
@@ -777,6 +767,16 @@ private static void handleModelErrors(DMNModel model) {
throw new IllegalStateException(errorMessage);
}

private static void identifyDecisionErrors(DMNModel model, String... decisions) {
List<DMNMessage> errorMessages = model.getMessages(DMNMessage.Severity.ERROR);
List<String> identifiedErrors = errorMessages.stream()
.filter(message -> Arrays.stream(decisions).anyMatch(decision -> message.getText().contains(decision)))
.map(Message::getText).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();
Original file line number Diff line number Diff line change
@@ -34,8 +34,7 @@
import org.kie.internal.io.ResourceFactory;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.assertj.core.api.AssertionsForClassTypes.assertThatThrownBy;

class DMNRuntimeBuilderTest {

@@ -72,11 +71,10 @@ void fromDefaultsMultipleDecisionWithoutInputDataReference() {
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) ";
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> {
dmnRuntime.evaluateAll(dmnModel, context);
});
assertEquals(errorMessage, exception.getMessage());
}
assertThatThrownBy(() -> dmnRuntime.evaluateAll(dmnModel, context))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
void evaluateWrongDecisionWithoutInputDataReferencesByName() {
@@ -95,10 +93,9 @@ void evaluateWrongDecisionWithoutInputDataReferencesByName() {
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) ";
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Drive");
});
assertEquals(errorMessage, exception.getMessage());
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
@@ -138,10 +135,9 @@ void evaluateWrongDecisionWithoutInputDataReferencesById() {
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) ";
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> {
dmnRuntime.evaluateById(dmnModel, context, "_563E78C7-EFD1-4109-9F30-B14922EF68DF");
});
assertEquals(errorMessage, exception.getMessage());
assertThatThrownBy(() -> dmnRuntime.evaluateById(dmnModel, context, "_563E78C7-EFD1-4109-9F30-B14922EF68DF"))
.isInstanceOf(IllegalStateException.class)
.hasMessage(errorMessage);
}

@Test
@@ -179,12 +175,34 @@ void evaluateDecisionWithInvalidFeelError() {
"DMN_33900B8B-73DD-4D1E-87E9-F6C3FE534B43");
assertThat(dmnModel).isNotNull();
DMNContext context = DMNFactory.newContext();
context.set( "Person Age", 24 );
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) ";
IllegalStateException exception = assertThrows(IllegalStateException.class, () -> {
dmnRuntime.evaluateByName(dmnModel, context, "Can Drive");
});
assertEquals(errorMessage, exception.getMessage());
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);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
<?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/_36ADF828-4BE5-41E1-8808-6245D13C6AB4" id="_6F7429DB-9A23-4584-861D-959D42F81C93" name="DMN_45A15AF7-9910-4EAD-B249-8AE218B3BF43" 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="Age" id="_3DF67640-214C-405C-B08E-DB9B21BB697C">
<variable name="Age" id="_8312D854-4604-46EC-AADF-E82FB52F9D28" typeRef="number" />
</inputData>
<decision name="Can Vote?" id="_B2773BA6-8A9C-45C7-A9DE-A4D2E771440B">
<variable id="_0E68422C-CA47-4D1D-8AA1-C2AB7F52ECBF" typeRef="boolean" name="Can Vote?" />
<informationRequirement id="_DFDE0845-C0D4-4646-AC72-1EA2CD31808D">
<requiredInput href="#_3DF67640-214C-405C-B08E-DB9B21BB697C" />
</informationRequirement>
<context id="_463A4AFA-C81B-4CD2-9F9D-57486F758458" typeRef="boolean" label="Can Vote?">
<contextEntry id="_808FC438-2162-4591-A015-0D864CBF7710">
<variable id="_E81D9B7C-05D8-4653-8F1A-B00E52424399" name="ContextEntry-1" />
<literalExpression id="_B7D17199-0568-40EE-94D0-FDFAB0E97868" label="ContextEntry-1">
<text>Age + 20.?&gt;</text>
</literalExpression>
</contextEntry>
<contextEntry id="_07CFB54F-605F-4737-A9A7-87AF905B9279">
<literalExpression id="_59E71393-14B3-405D-A0B4-3C1E6562823F" typeRef="boolean" label="Can Vote?">
<text>if Age &gt; 25 &quot;YES&quot; elsesss &quot;NO&quot;</text>
</literalExpression>
</contextEntry>
</context>
</decision>
<dmndi:DMNDI>
<dmndi:DMNDiagram id="_27B2598B-9F53-4B7E-8994-18F8A7AD723C" name="Default DRD" useAlternativeInputDataShape="false">
<di:extension>
<kie:ComponentsWidthsExtension>
<kie:ComponentWidths dmnElementRef="_463A4AFA-C81B-4CD2-9F9D-57486F758458">
<kie:width>120</kie:width>
</kie:ComponentWidths>
<kie:ComponentWidths dmnElementRef="_B7D17199-0568-40EE-94D0-FDFAB0E97868">
<kie:width>190</kie:width>
</kie:ComponentWidths>
<kie:ComponentWidths dmnElementRef="_59E71393-14B3-405D-A0B4-3C1E6562823F">
<kie:width>190</kie:width>
</kie:ComponentWidths>
</kie:ComponentsWidthsExtension>
</di:extension>
<dmndi:DMNShape id="_05C9DE98-88EB-4B10-B428-94D38804B882" dmnElementRef="_3DF67640-214C-405C-B08E-DB9B21BB697C" isCollapsed="false" isListedInputData="false">
<dc:Bounds x="100" y="300" width="160" height="80" />
</dmndi:DMNShape>
<dmndi:DMNShape id="_72B292E2-B32F-4DD2-AEE1-C2F8EB9C2824" dmnElementRef="_B2773BA6-8A9C-45C7-A9DE-A4D2E771440B" isCollapsed="false" isListedInputData="false">
<dc:Bounds x="100" y="100" width="160" height="80" />
</dmndi:DMNShape>
<dmndi:DMNEdge id="_741AC8E2-AC8D-4A04-BB47-FF7EC1203E39-AUTO-TARGET" dmnElementRef="_DFDE0845-C0D4-4646-AC72-1EA2CD31808D" sourceElement="_05C9DE98-88EB-4B10-B428-94D38804B882" targetElement="_72B292E2-B32F-4DD2-AEE1-C2F8EB9C2824">
<di:waypoint x="180" y="340" />
<di:waypoint x="180" y="140" />
</dmndi:DMNEdge>
</dmndi:DMNDiagram>
</dmndi:DMNDI>
</definitions>