Skip to content

Commit

Permalink
[incubator-kie-issues#1326] Filter, Some and Every expression s…
Browse files Browse the repository at this point in the history
…hould fail when the matching function doesn't return a Boolean (#6000)

* Fix + Tests

* Fix tests

* Minor changes
  • Loading branch information
yesamer authored Jun 24, 2024
1 parent cf4ef68 commit f84a1be
Show file tree
Hide file tree
Showing 5 changed files with 55 additions and 42 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
package org.kie.dmn.core.ast;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -70,8 +69,8 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d
}
Object inObj = inResult.getResult();

if (inObj instanceof Range) {
inObj = new IterableRange((Range) inObj);
if (inObj instanceof Range range) {
inObj = new IterableRange(range);
} else if (!(inObj instanceof Iterable)) {
if (inObj == null) {
MsgUtil.reportMessage(logger,
Expand All @@ -85,7 +84,7 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d
return new EvaluatorResultImpl(null, ResultType.FAILURE);
}

// 10.3.2.9.4 Type conversions "to singleton list"
// 10.3.2.9.4 Type conversions "to singleton list"
inObj = Collections.singletonList(inObj);
}

Expand All @@ -96,45 +95,33 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d
try {
result.setContext(dmnContext);

boolean first = true;
for (Object item : (Iterable) inObj) {
for (Object item : (Iterable<?>) inObj) {

dmnContext.set("item", item);
if (item instanceof Map) {
Map<String, Object> complexItem = (Map<String, Object>) item;
complexItem.forEach((k, v) -> dmnContext.set(k, v));
complexItem.forEach(dmnContext::set);
}

EvaluatorResult evaluate = filterEvaluator.evaluate(eventManager, dmnr);
Object evalReturn = evaluate.getResult();

//If the evaluation is a boolean result, we add the item based on a return of true
if (evalReturn instanceof Boolean && ((Boolean) evalReturn).booleanValue() == true) {
returnList.add(item);
}

//If on the first evaluation, a number is returned, we are using an index instead of a boolean filter
if (first && evalReturn instanceof Number) {
List list = inObj instanceof List ? (List) inObj : List.of(inObj);
int i = ((Number) evalReturn).intValue();
if (i > 0 && i <= list.size()) {
return new EvaluatorResultImpl(list.get(i - 1), ResultType.SUCCESS);
} else if (i < 0 && Math.abs(i) <= list.size()) {
return new EvaluatorResultImpl(list.get(list.size() + i), ResultType.SUCCESS);
} else {
MsgUtil.reportMessage(logger,
DMNMessage.Severity.ERROR,
node,
result,
null,
null,
Msg.INDEX_OUT_OF_BOUND,
list.size(),
i);
return new EvaluatorResultImpl(null, ResultType.FAILURE);
if (evalReturn instanceof Boolean booleanResult) {
if (Boolean.TRUE.equals(booleanResult)) {
returnList.add(item);
}
} else {
MsgUtil.reportMessage(logger,
DMNMessage.Severity.ERROR,
node,
result,
null,
null,
Msg.FILTER_EXPRESSION_RESULT_NOT_BOOLEAN,
name);
return new EvaluatorResultImpl(null, ResultType.FAILURE);
}
first = false;
}

} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,8 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d
}
Object inObj = inResult.getResult();

if (inObj instanceof Range) {
inObj = new IterableRange((Range) inObj);
if (inObj instanceof Range range) {
inObj = new IterableRange(range);
} else if (!(inObj instanceof Iterable)) {
if (inObj == null) {
MsgUtil.reportMessage(logger,
Expand Down Expand Up @@ -112,16 +112,40 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager eventManager, DMNResult d

if (type instanceof Every) {
for (Object satisfies : returnList) {
if (!(satisfies instanceof Boolean) || ((Boolean) satisfies).booleanValue() == false) {
return new EvaluatorResultImpl(Boolean.FALSE, ResultType.SUCCESS);
if (satisfies instanceof Boolean satifiesBoolean) {
if (Boolean.FALSE.equals(satifiesBoolean)) {
return new EvaluatorResultImpl(Boolean.FALSE, ResultType.SUCCESS);
}
} else {
MsgUtil.reportMessage(logger,
DMNMessage.Severity.ERROR,
node,
result,
null,
null,
Msg.ITERATOR_EXPRESSION_RESULT_NOT_BOOLEAN,
name);
return new EvaluatorResultImpl(null, ResultType.FAILURE);
}
}
return new EvaluatorResultImpl(Boolean.TRUE, ResultType.SUCCESS);
}
if (type instanceof Some) {
for (Object satisfies : returnList) {
if (satisfies instanceof Boolean && ((Boolean) satisfies).booleanValue() == true) {
return new EvaluatorResultImpl(Boolean.TRUE, ResultType.SUCCESS);
if (satisfies instanceof Boolean satifiesBoolean) {
if (Boolean.TRUE.equals(satifiesBoolean)) {
return new EvaluatorResultImpl(Boolean.TRUE, ResultType.SUCCESS);
}
} else {
MsgUtil.reportMessage(logger,
DMNMessage.Severity.ERROR,
node,
result,
null,
null,
Msg.ITERATOR_EXPRESSION_RESULT_NOT_BOOLEAN,
name);
return new EvaluatorResultImpl(null, ResultType.FAILURE);
}
}
return new EvaluatorResultImpl(Boolean.FALSE, ResultType.SUCCESS);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ public final class Msg {
public static final Message2 MISSING_EXPRESSION_FOR_FILTER = new Message2( DMNMessageType.MISSING_EXPRESSION, "Missing %s expression for Filter node '%s'" );
public static final Message2 CONDITION_RESULT_NOT_BOOLEAN = new Message2( DMNMessageType.ERROR_EVAL_NODE, "The if condition on node %s returned a non boolean result: '%s'" );
public static final Message1 IN_RESULT_NULL = new Message1( DMNMessageType.ERROR_EVAL_NODE, "The in condition on node %s returned null.");
public static final Message1 FILTER_EXPRESSION_RESULT_NOT_BOOLEAN = new Message1( DMNMessageType.ERROR_EVAL_NODE, "The filter expression on node %s returned a non boolean result");
public static final Message1 ITERATOR_EXPRESSION_RESULT_NOT_BOOLEAN = new Message1( DMNMessageType.ERROR_EVAL_NODE, "The satisfy expression on node %s returned a non boolean result");
public static final Message2 INDEX_OUT_OF_BOUND = new Message2( DMNMessageType.ERROR_EVAL_NODE, "Index out of bound: list of %s elements, index %s; will evaluate as FEEL null");


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,14 @@ public void filterIndex(final BaseVariantTest.VariantTestConf conf) throws Throw
testConfig = conf;
setup();
DMNResult results = runtime.evaluateByName(model, new DMNContextImpl(Collections.singletonMap("Number Input", 5)), "Match by index");
assertThat(results.getDecisionResultByName("Match by index").getResult()).isEqualTo(new BigDecimal(5));
assertThat(results.getDecisionResultByName("Match by index").getResult()).asList().hasSize(1);
assertThat(results.getDecisionResultByName("Match by index").getResult()).asList().contains(new BigDecimal(5));

results = runtime.evaluateByName(model, new DMNContextImpl(Collections.singletonMap("Number Input", -2)), "Match by index");
assertThat(results.getDecisionResultByName("Match by index").getResult()).isEqualTo(new BigDecimal(9));
assertThat(results.getDecisionResultByName("Match by index").getResult()).asList().isEmpty();

results = runtime.evaluateByName(model, new DMNContextImpl(Collections.singletonMap("Number Input", 0)), "Match by index");
assertThat(results.getMessages()).hasSizeGreaterThan(0);

assertThat(results.getDecisionResultByName("Match by index").getResult()).asList().isEmpty();
}

@MethodSource("params")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@
</semantic:in>
<semantic:match id="_362153c8-76bd-4139-aee8-8ef75ebfe42a">
<semantic:literalExpression id="_2c32d1c1-d954-4422-80a4-36bc2ec63443" triso:descriptionVisible="false">
<semantic:text>Number Input</semantic:text>
<semantic:text>item = Number Input</semantic:text>
</semantic:literalExpression>
</semantic:match>
</semantic:filter>
Expand Down

0 comments on commit f84a1be

Please sign in to comment.