Skip to content

Commit

Permalink
[Incubator-kie-issues#1820] Resolve "Missing Conditional element" run…
Browse files Browse the repository at this point in the history
…time exception during model execution (apache#6257)

* [incubator-kie-issues#1820] Fix runtime exception during dmn model execution

* [incubator-kie-issues#1820] Update test cases

* [incubator-kie-issues#1820] Code refactoring

* [incubator-kie-issues#1820] Fix header for dmn file

* [incubator-kie-issues#1820] Fix test cases

* [incubator-kie-issues#1820] Implement recursive retrieval of node by id

* [incubator-kie-issues#1820] Code refactoring

* [incubator-kie-issues#1820] Fix review comments

* [incubator-kie-issues#1820] Fix test cases

---------

Co-authored-by: athira <[email protected]>
Co-authored-by: Gabriele-Cardosi <[email protected]>
  • Loading branch information
3 people authored and rgdoliveira committed Feb 24, 2025
1 parent 2fadf03 commit 0492aa9
Show file tree
Hide file tree
Showing 6 changed files with 408 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,11 @@
*/
package org.kie.dmn.core.ast;

import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Function;
import java.util.stream.Collectors;

import org.kie.dmn.api.core.DMNMessage;
import org.kie.dmn.api.core.DMNResult;
Expand All @@ -31,44 +34,88 @@
import org.kie.dmn.core.impl.DMNRuntimeEventManagerUtils;
import org.kie.dmn.core.util.Msg;
import org.kie.dmn.core.util.MsgUtil;
import org.kie.dmn.model.api.Conditional;
import org.kie.dmn.model.api.DMNElement;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

public class DMNConditionalEvaluator implements DMNExpressionEvaluator {

public enum EvaluatorType {
IF( "if" ),
THEN( "then" ),
ELSE( "else" );

public final String value;

EvaluatorType(String value) {
this.value = value;
}
public String getValue() {
return value;
}
}

public static class EvaluatorIdentifier {
final String id;
final EvaluatorType type;

public EvaluatorIdentifier (String id, EvaluatorType type) {
this.id = id;
this.type = type;
}

@Override
public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
EvaluatorIdentifier that = (EvaluatorIdentifier) o;
return Objects.equals(id, that.id) && type == that.type;
}

@Override
public int hashCode() {
return Objects.hash(id, type);
}
}

private static final Logger logger = LoggerFactory.getLogger(DMNConditionalEvaluator.class);

private DMNExpressionEvaluator ifEvaluator;
private DMNExpressionEvaluator thenEvaluator;
private DMNExpressionEvaluator elseEvaluator;
private DMNElement node;
private String name;
private final Map<DMNExpressionEvaluator, String> evaluatorIdMap = new HashMap<>();
private final DMNExpressionEvaluator ifEvaluator;
private final DMNExpressionEvaluator thenEvaluator;
private final DMNExpressionEvaluator elseEvaluator;
private final DMNElement node;
private final String name;
private final EvaluatorIdentifier ifEvaluatorIdentifier;
private final EvaluatorIdentifier thenEvaluatorIdentifier;
private final EvaluatorIdentifier elseEvaluatorIdentifier;

public DMNConditionalEvaluator(String name, DMNElement node, DMNExpressionEvaluator ifEvaluator, DMNExpressionEvaluator thenEvaluator, DMNExpressionEvaluator elseEvaluator) {
static Map<EvaluatorType, EvaluatorIdentifier> mapEvaluatorIdentifiers(Map<EvaluatorIdentifier, DMNExpressionEvaluator> evaluatorIdMap) {
return evaluatorIdMap.keySet().stream()
.collect(Collectors.toMap(identifier -> identifier.type, Function.identity()));
}

static EvaluatorIdentifier getEvaluatorIdentifier(Map<EvaluatorType, EvaluatorIdentifier> evaluatorIdentifierMap, EvaluatorType type) {
return Optional.ofNullable(evaluatorIdentifierMap.get(type))
.orElseThrow(() -> new RuntimeException("Missing " + type + " evaluator in evaluatorIdMap"));
}

public DMNConditionalEvaluator(String name, DMNElement node, Map <EvaluatorIdentifier, DMNExpressionEvaluator> evaluatorIdMap) {
this.name = name;
this.node = node;
this.ifEvaluator = ifEvaluator;
this.thenEvaluator = thenEvaluator;
this.elseEvaluator = elseEvaluator;
Conditional conditional = node.getChildren().stream()
.filter(c -> c instanceof Conditional)
.map(c -> (Conditional) c)
.findFirst()
.orElseThrow(() -> new RuntimeException("Missing Conditional element inside " + node));
evaluatorIdMap.put(ifEvaluator, conditional.getIf().getId());
evaluatorIdMap.put(thenEvaluator, conditional.getThen().getId());
evaluatorIdMap.put(elseEvaluator, conditional.getElse().getId());
Map<EvaluatorType, EvaluatorIdentifier> evaluatorIdentifierMap = mapEvaluatorIdentifiers(evaluatorIdMap);
this.ifEvaluatorIdentifier = getEvaluatorIdentifier(evaluatorIdentifierMap, EvaluatorType.IF);
this.thenEvaluatorIdentifier = getEvaluatorIdentifier(evaluatorIdentifierMap, EvaluatorType.THEN);
this.elseEvaluatorIdentifier = getEvaluatorIdentifier(evaluatorIdentifierMap, EvaluatorType.ELSE);
this.ifEvaluator = evaluatorIdMap.get(ifEvaluatorIdentifier);
this.thenEvaluator = evaluatorIdMap.get(thenEvaluatorIdentifier);
this.elseEvaluator = evaluatorIdMap.get(elseEvaluatorIdentifier);
}

@Override
public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult dmnr) {
DMNResultImpl result = (DMNResultImpl) dmnr;

EvaluatorResult ifEvaluation = ifEvaluator.evaluate(eventManager, result);
String executedId = evaluatorIdMap.get(ifEvaluator);
String executedId = ifEvaluatorIdentifier.id;
DMNRuntimeEventManagerUtils.fireAfterEvaluateConditional(eventManager, ifEvaluation, executedId);
if (ifEvaluation.getResultType().equals(ResultType.SUCCESS)) {
Object ifResult = ifEvaluation.getResult();
Expand All @@ -92,9 +139,8 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d

protected EvaluatorResult manageBooleanOrNullIfResult(Boolean booleanResult, DMNRuntimeEventManager eventManager, DMNResultImpl result) {
DMNExpressionEvaluator evaluatorToUse = booleanResult != null && booleanResult ? thenEvaluator : elseEvaluator;

EvaluatorResult toReturn = evaluatorToUse.evaluate(eventManager, result);
String executedId = evaluatorIdMap.get(evaluatorToUse);
String executedId = evaluatorToUse.equals(thenEvaluator) ? thenEvaluatorIdentifier.id : elseEvaluatorIdentifier.id;
DMNRuntimeEventManagerUtils.fireAfterConditionalEvaluation(eventManager, name, toReturn, executedId);
return toReturn;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,15 @@

import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Collectors;

import javax.swing.DefaultListSelectionModel;
import javax.xml.namespace.QName;

import org.kie.api.io.Resource;
Expand Down Expand Up @@ -124,6 +125,18 @@ public static DMNEvaluatorCompiler dmnEvaluatorCompilerFactory(DMNCompilerImpl d
}
}

static Map<DMNConditionalEvaluator.EvaluatorIdentifier, DMNExpressionEvaluator> getEvaluatorIdentifierMap(Conditional expression, DMNExpressionEvaluator ifEvaluator, DMNExpressionEvaluator thenEvaluator, DMNExpressionEvaluator elseEvaluator) {
Map<DMNConditionalEvaluator.EvaluatorIdentifier, DMNExpressionEvaluator> evaluatorIdMap = new HashMap<>();
evaluatorIdMap.put(getEvaluatorIdentifier(expression.getIf().getId(), DMNConditionalEvaluator.EvaluatorType.IF), ifEvaluator);
evaluatorIdMap.put(getEvaluatorIdentifier(expression.getThen().getId(), DMNConditionalEvaluator.EvaluatorType.THEN), thenEvaluator);
evaluatorIdMap.put(getEvaluatorIdentifier(expression.getElse().getId(), DMNConditionalEvaluator.EvaluatorType.ELSE), elseEvaluator);
return evaluatorIdMap;
}

static DMNConditionalEvaluator.EvaluatorIdentifier getEvaluatorIdentifier(String id, DMNConditionalEvaluator.EvaluatorType type) {
return new DMNConditionalEvaluator.EvaluatorIdentifier(id, type);
}

@Override
public DMNExpressionEvaluator compileExpression(DMNCompilerContext ctx, DMNModelImpl model, DMNBaseNode node,
String exprName, Expression expression) {
Expand Down Expand Up @@ -1035,37 +1048,39 @@ private java.util.List<UnaryTest> textToUnaryTestList(DMNCompilerContext ctx, St
return ctx.getFeelHelper().evaluateUnaryTests(ctx, text, model, element, errorMsg, msgParams);
}

private DMNExpressionEvaluator compileConditional(DMNCompilerContext ctx, DMNModelImpl model, DMNBaseNode node,
String exprName, Conditional expression) {
DMNExpressionEvaluator ifEvaluator = compileExpression(ctx, model, node, exprName + " [if]",
protected DMNExpressionEvaluator compileConditional(DMNCompilerContext ctx, DMNModelImpl model, DMNBaseNode node,
String exprName, Conditional expression) {
DMNExpressionEvaluator ifEvaluator = compileExpression(ctx, model, node, formatExpressionName(exprName, DMNConditionalEvaluator.EvaluatorType.IF),
expression.getIf().getExpression());
DMNExpressionEvaluator thenEvaluator = compileExpression(ctx, model, node, exprName + " [then]",
DMNExpressionEvaluator thenEvaluator = compileExpression(ctx, model, node, formatExpressionName(exprName, DMNConditionalEvaluator.EvaluatorType.THEN),
expression.getThen().getExpression());
DMNExpressionEvaluator elseEvaluator = compileExpression(ctx, model, node, exprName + " [else]",
DMNExpressionEvaluator elseEvaluator = compileExpression(ctx, model, node, formatExpressionName(exprName, DMNConditionalEvaluator.EvaluatorType.ELSE),
expression.getElse().getExpression());

if (ifEvaluator == null) {
MsgUtil.reportMessage(logger, DMNMessage.Severity.ERROR, node.getSource(), model, null, null,
Msg.MISSING_EXPRESSION_FOR_CONDITION, "if",
Msg.MISSING_EXPRESSION_FOR_CONDITION, DMNConditionalEvaluator.EvaluatorType.IF.getValue(),
node.getIdentifierString());
return null;
}

if (thenEvaluator == null) {
MsgUtil.reportMessage(logger, DMNMessage.Severity.ERROR, node.getSource(), model, null, null,
Msg.MISSING_EXPRESSION_FOR_CONDITION, "then",
Msg.MISSING_EXPRESSION_FOR_CONDITION, DMNConditionalEvaluator.EvaluatorType.THEN.getValue(),
node.getIdentifierString());
return null;
}

if (elseEvaluator == null) {
MsgUtil.reportMessage(logger, DMNMessage.Severity.ERROR, node.getSource(), model, null, null,
Msg.MISSING_EXPRESSION_FOR_CONDITION, "else",
Msg.MISSING_EXPRESSION_FOR_CONDITION, DMNConditionalEvaluator.EvaluatorType.ELSE.getValue(),
node.getIdentifierString());
return null;
}

return new DMNConditionalEvaluator(exprName, node.getSource(), ifEvaluator, thenEvaluator, elseEvaluator);
Map<DMNConditionalEvaluator.EvaluatorIdentifier, DMNExpressionEvaluator> evaluatorIdMap = getEvaluatorIdentifierMap(expression, ifEvaluator, thenEvaluator, elseEvaluator);

return new DMNConditionalEvaluator(exprName, node.getSource(), evaluatorIdMap);
}

private DMNExpressionEvaluator compileIterator(DMNCompilerContext ctx, DMNModelImpl model, DMNBaseNode node,
Expand Down Expand Up @@ -1180,4 +1195,8 @@ private DMNExpressionEvaluator compileFilter(DMNCompilerContext ctx, DMNModelImp

return new DMNFilterEvaluator(exprName, node.getSource(), inEvaluator, filterEvaluator);
}

private static String formatExpressionName(String exprName, DMNConditionalEvaluator.EvaluatorType type) {
return String.format("%s [%s]", exprName, type.getValue());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@
package org.kie.dmn.core.ast;

import java.util.Collections;
import java.util.List;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import org.junit.jupiter.api.BeforeAll;
Expand All @@ -32,14 +33,11 @@
import org.kie.dmn.api.core.event.DMNRuntimeEventManager;
import org.kie.dmn.core.api.DMNExpressionEvaluator;
import org.kie.dmn.core.impl.DMNResultImpl;
import org.kie.dmn.model.api.ChildExpression;
import org.kie.dmn.model.api.Conditional;
import org.kie.dmn.model.api.DMNElement;
import org.kie.dmn.model.api.DMNModelInstrumentedBase;
import org.kie.dmn.model.api.Expression;
import org.mockito.ArgumentCaptor;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatRuntimeException;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.reset;
import static org.mockito.Mockito.spy;
Expand All @@ -48,16 +46,20 @@

class DMNConditionalEvaluatorTest {

private static final String ID_ELEMENT_ID = "ID_ELEMENT_ID";
private static final String IF_ELEMENT_ID = "IF_ELEMENT_ID";
private static final String THEN_ELEMENT_ID = "THEN_ELEMENT_ID";
private static final String ELSE_ELEMENT_ID = "ELSE_ELEMENT_ID";
private static final Map<DMNConditionalEvaluator.EvaluatorIdentifier, DMNExpressionEvaluator> EVALUATOR_ID_MAP = new HashMap<>();
private static DMNConditionalEvaluator dmnConditionalEvaluator;
private static DMNConditionalEvaluator.EvaluatorIdentifier ifIdentifier;
private static DMNConditionalEvaluator.EvaluatorIdentifier thenIdentifier;
private static DMNConditionalEvaluator.EvaluatorIdentifier elseIdentifier;
private static DMNRuntimeEventManager eventManagerMock;
private static DMNRuntimeEventListener spiedListener;
private static DMNResultImpl dmnResultMock;
private static EvaluatorResult ifEvaluationMock;
private static EvaluatorResult thenEvaluationMock;
private static EvaluatorResult elseEvaluationMock;
private static DMNResultImpl dmnResultMock;
private static DMNConditionalEvaluator dmnConditionalEvaluator;

@BeforeAll
static void setUp() {
Expand All @@ -79,29 +81,17 @@ static void setUp() {
when(thenEvaluatorMock.evaluate(eventManagerMock, dmnResultMock)).thenReturn(thenEvaluationMock);
when(elseEvaluatorMock.evaluate(eventManagerMock, dmnResultMock)).thenReturn(elseEvaluationMock);

ChildExpression ifMock = mock(ChildExpression.class);
when(ifMock.getId()).thenReturn(ID_ELEMENT_ID);

ChildExpression thenMock = mock(ChildExpression.class);
when(thenMock.getId()).thenReturn(THEN_ELEMENT_ID);

ChildExpression elseMock = mock(ChildExpression.class);
when(elseMock.getId()).thenReturn(ELSE_ELEMENT_ID);
DMNElement nodeMock = mock(DMNElement.class);

Conditional conditionalMock = mock(Conditional.class);
when(conditionalMock.getIf()).thenReturn(ifMock);
when(conditionalMock.getThen()).thenReturn(thenMock);
when(conditionalMock.getElse()).thenReturn(elseMock);
ifIdentifier = new DMNConditionalEvaluator.EvaluatorIdentifier(IF_ELEMENT_ID, DMNConditionalEvaluator.EvaluatorType.IF);
thenIdentifier = new DMNConditionalEvaluator.EvaluatorIdentifier(THEN_ELEMENT_ID, DMNConditionalEvaluator.EvaluatorType.THEN);
elseIdentifier = new DMNConditionalEvaluator.EvaluatorIdentifier(ELSE_ELEMENT_ID, DMNConditionalEvaluator.EvaluatorType.ELSE);

List<DMNModelInstrumentedBase> nodeChildren = Collections.singletonList(conditionalMock);
DMNElement nodeMock = mock(DMNElement.class);
when(nodeMock.getChildren()).thenReturn(nodeChildren);
EVALUATOR_ID_MAP.put(ifIdentifier, ifEvaluatorMock);
EVALUATOR_ID_MAP.put(thenIdentifier, thenEvaluatorMock);
EVALUATOR_ID_MAP.put(elseIdentifier, elseEvaluatorMock);

dmnConditionalEvaluator = new DMNConditionalEvaluator("name",
nodeMock,
ifEvaluatorMock,
thenEvaluatorMock,
elseEvaluatorMock);
dmnConditionalEvaluator = new DMNConditionalEvaluator("name", nodeMock, EVALUATOR_ID_MAP);
}

@BeforeEach
Expand All @@ -121,7 +111,7 @@ void evaluateListenerInvocation() {
AfterEvaluateConditionalEvent evaluateConditionalEvent = evaluateConditionalEventArgumentCaptor.getValue();
assertThat(evaluateConditionalEvent).isNotNull();
assertThat(evaluateConditionalEvent.getEvaluatorResultResult()).isEqualTo(ifEvaluationMock);
assertThat(evaluateConditionalEvent.getExecutedId()).isEqualTo(ID_ELEMENT_ID);
assertThat(evaluateConditionalEvent.getExecutedId()).isEqualTo(IF_ELEMENT_ID);
}

@Test
Expand Down Expand Up @@ -172,4 +162,31 @@ void manageBooleanOrNullIfResultWithNull() {
assertThat(conditionalEvaluationEvent.getEvaluatorResultResult()).isEqualTo(elseEvaluationMock);
assertThat(conditionalEvaluationEvent.getExecutedId()).isEqualTo(ELSE_ELEMENT_ID);
}

@Test
void testMapEvaluatorIdentifiers() {
Map<DMNConditionalEvaluator.EvaluatorType, DMNConditionalEvaluator.EvaluatorIdentifier> mapEvaluatorIdentifiers = DMNConditionalEvaluator.mapEvaluatorIdentifiers(EVALUATOR_ID_MAP);

assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.IF)).isEqualTo(ifIdentifier);
assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.THEN)).isEqualTo(thenIdentifier);
assertThat(mapEvaluatorIdentifiers.get(DMNConditionalEvaluator.EvaluatorType.ELSE)).isEqualTo(elseIdentifier);
}

@Test
void testGetEvaluatorIdentifier() {
Map<DMNConditionalEvaluator.EvaluatorType, DMNConditionalEvaluator.EvaluatorIdentifier> evaluatorIdentifierMap = Map.of(
DMNConditionalEvaluator.EvaluatorType.IF, ifIdentifier,
DMNConditionalEvaluator.EvaluatorType.THEN, thenIdentifier,
DMNConditionalEvaluator.EvaluatorType.ELSE, elseIdentifier);
for (DMNConditionalEvaluator.EvaluatorType type : DMNConditionalEvaluator.EvaluatorType.values()) {
assertThat(DMNConditionalEvaluator.getEvaluatorIdentifier(evaluatorIdentifierMap, type)).isEqualTo(evaluatorIdentifierMap.get(type));
}
}

@Test
void testMissingEvaluatorIdentifier() {
Map<DMNConditionalEvaluator.EvaluatorType, DMNConditionalEvaluator.EvaluatorIdentifier> evaluatorIdentifierMap = new HashMap<>();
String errorMessage = "Missing THEN evaluator in evaluatorIdMap";
assertThatRuntimeException().isThrownBy(() -> DMNConditionalEvaluator.getEvaluatorIdentifier(evaluatorIdentifierMap, DMNConditionalEvaluator.EvaluatorType.THEN)).withMessage(errorMessage);
}
}
Loading

0 comments on commit 0492aa9

Please sign in to comment.