Copilot commented on code in PR #6556:
URL:
https://github.com/apache/incubator-kie-drools/pull/6556#discussion_r2720636089
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/lang/impl/EvaluationContextImpl.java:
##########
@@ -47,6 +47,7 @@ public class EvaluationContextImpl implements
EvaluationContext {
private ClassLoader rootClassLoader;
private final FEELDialect feelDialect;
private final DMNVersion dmnVersion;
+ private boolean isLenient = true;
Review Comment:
`EvaluationContextImpl.current()` clones the context but does not propagate
the new `isLenient` flag. This means any code using `ctx.current()` (e.g.,
decision table evaluation) will silently revert to the default lenient behavior
even when strict mode was requested, so strict-mode invalid-input checks won’t
fire. Copy `isLenient` into the cloned context (and any other cloning/copy
constructors) to keep mode consistent.
##########
kie-dmn/kie-dmn-core/src/main/java/org/kie/dmn/core/ast/DMNDTExpressionEvaluator.java:
##########
@@ -78,7 +77,8 @@ public EvaluatorResult evaluate(DMNRuntimeEventManager dmrem,
DMNResult dmnr) {
DMNRuntimeEventManagerUtils.fireBeforeEvaluateDecisionTable(
dmrem, node.getName(), dt.getName(), dtNodeId, result );
List<String> paramNames =
dt.getParameters().get(0).stream().map(Param::getName).toList();
Object[] params = new Object[paramNames.size()];
- EvaluationContextImpl ctx =
feel.newEvaluationContext(List.of(events::add), Collections.emptyMap());
+ boolean isLenient =
RuntimeModeOption.MODE.LENIENT.name().equals(((DMNRuntimeImpl)
dmrem.getRuntime()).getRuntimeModeOption().name());
+ EvaluationContext ctx =
feel.newEvaluationContext(List.of(events::add), Collections.emptyMap(),
isLenient);
ctx.setPerformRuntimeTypeCheck(((DMNRuntimeImpl)
dmrem.getRuntime()).performRuntimeTypeCheck(result.getModel()));
Review Comment:
`isLenient` detection is doing a `name().equals(other.name())` string
comparison. This is harder to read and more error-prone than directly comparing
the enum values (or checking for `STRICT`). Consider using a direct enum
comparison and storing the `(DMNRuntimeImpl) dmrem.getRuntime()` cast in a
local variable to avoid repeating it.
##########
kie-dmn/kie-dmn-legacy-tests/src/test/java/org/kie/dmn/legacy/tests/core/v1_1/DMNDecisionTableRuntimeTest.java:
##########
@@ -163,7 +163,24 @@ void decisionTableInvalidInputErrorMessage(VariantTestConf
conf) {
context.set( "Branches dispersion", "Province" );
context.set( "Number of Branches", BigDecimal.valueOf( 10 ) );
- testDecisionTableInvalidInput( context );
+ final DMNRuntime runtime = DMNRuntimeUtil.createRuntime(
"InvalidInput.dmn", this.getClass() );
+ final DMNModel dmnModel = runtime.getModel(
"http://www.trisotech.com/dmn/definitions/_cdf29af2-959b-4004-8271-82a9f5a62147";,
"Dessin 1" );
+ assertThat(dmnModel).isNotNull();
+
+ final DMNResult dmnResult = runtime.evaluateAll( dmnModel, context );
+ // In lenient mode, invalid input value is set to null and evaluation
continues
+ assertThat( dmnResult.hasErrors()).isFalse();
+
+ final DMNContext result = dmnResult.getContext();
+
+ // Verify that the invalid input "Province" (without leading space)
was coerced to null
+ // The decision table should receive null for "Branches dispersion"
parameter
+ // Since Number of Branches = 10 (<10) and Branches dispersion = null
(doesn't match any specific rule),
Review Comment:
The explanation comment says `Number of Branches = 10 (<10)`, but `10` is
not `< 10`. Please correct this (or align it with the actual rule condition) to
avoid misleading future readers of the test.
```suggestion
// Since Number of Branches = 10 (not <10) and Branches dispersion =
null (doesn't match any specific rule),
```
##########
kie-dmn/kie-dmn-core/src/test/java/org/kie/dmn/core/DMNDecisionTableRuntimeTest.java:
##########
@@ -208,18 +208,51 @@ void decisionTableNonexistingInputErrorMessage(boolean
useExecModelCompiler) {
context.set( "Not exists", "Province" );
context.set( "Number of Branches", BigDecimal.valueOf( 10 ) );
- testDecisionTableInvalidInput( context );
+ testDecisionTableMissingDependency( context );
}
- private void testDecisionTableInvalidInput(final DMNContext inputContext) {
+ private void testDecisionTableInvalidInputLenient(final DMNContext
inputContext) {
final DMNRuntime runtime = DMNRuntimeUtil.createRuntime(
"InvalidInput.dmn", this.getClass() );
final DMNModel dmnModel = runtime.getModel(
"http://www.trisotech.com/dmn/definitions/_cdf29af2-959b-4004-8271-82a9f5a62147";,
"Dessin 1" );
assertThat(dmnModel).isNotNull();
final DMNResult dmnResult = runtime.evaluateAll( dmnModel,
inputContext );
+ // In lenient mode (default), invalid input values don't generate
errors
+ // The invalid value is set to null and evaluation continues
+ assertThat( dmnResult.hasErrors()).isFalse();
+
+ final DMNContext result = dmnResult.getContext();
+ // Result is defined - evaluation continues with null for invalid input
+ assertThat( result.isDefined( "Branches distribution"
)).isEqualTo(Boolean.TRUE);
+ }
Review Comment:
This test now asserts lenient-mode behavior for an invalid constrained
value, but there’s no corresponding strict-mode coverage in this class to
ensure the previous error behavior still occurs when strict mode is enabled.
Please add a strict-mode variant that sets strict mode and verifies the invalid
value produces an error/undefined result.
##########
kie-dmn/kie-dmn-legacy-tests/src/test/java/org/kie/dmn/legacy/tests/core/v1_1/DMNDecisionTableHitPolicyTest.java:
##########
@@ -66,9 +67,36 @@ void
simpleDecisionTableHitPolicyUniqueSatisfies(VariantTestConf conf) {
final DMNContext context =
getSimpleTableContext(BigDecimal.valueOf(18), "ASD", false);
final DMNResult dmnResult = runtime.evaluateAll(dmnModel, context);
final DMNContext result = dmnResult.getContext();
+
+ // In lenient mode, invalid input value "ASD" is set to null and
evaluation continues
+ // Rule 4 matches: Age=any(-), RiskCategory=any(-), isAffordable=false
-> "Declined"
+ assertThat(result.get("Approval Status")).isEqualTo("Declined");
- assertThat(result.get("Approval Status")).isNull();
- assertThat(dmnResult.getMessages()).hasSizeGreaterThan(0);
+ assertThat(dmnResult.getDecisionResults()).hasSize(1);
+
assertThat(dmnResult.getDecisionResultByName("_0004-simpletable-U")).isNotNull();
+
assertThat(dmnResult.getDecisionResultByName("_0004-simpletable-U").getResult()).isEqualTo("Declined");
+
assertThat(dmnResult.getDecisionResultByName("_0004-simpletable-U").getEvaluationStatus())
+
.isEqualTo(org.kie.dmn.api.core.DMNDecisionResult.DecisionEvaluationStatus.SUCCEEDED);
+ }
+
+ @ParameterizedTest(name = "{0}")
+ @MethodSource("params")
+ void simpleDecisionTableHitPolicyUniqueSatisfiesStrictMode(VariantTestConf
conf) {
+ testConfig = conf;
+ System.setProperty(RuntimeModeOption.PROPERTY_NAME,
RuntimeModeOption.MODE.STRICT.getMode());
+ try {
+ final DMNRuntime runtime =
DMNRuntimeUtil.createRuntime("0004-simpletable-U.dmn", this.getClass());
+ final DMNModel dmnModel =
runtime.getModel("https://github.com/kiegroup/kie-dmn";, "0004-simpletable-U");
+ assertThat(dmnModel).isNotNull();
+
+ final DMNContext context =
getSimpleTableContext(BigDecimal.valueOf(18), "Medium", true);
+ final DMNResult dmnResult = runtime.evaluateAll(dmnModel, context);
+
+ assertThat(dmnResult.hasErrors()).isFalse();
+ assertThat(dmnResult.getContext().get("Approval
Status")).isEqualTo("Approved");
Review Comment:
Strict-mode behavior for *invalid* constrained input values isn’t covered
here: the new strict-mode test only exercises a valid input path. Please add an
assertion (or a separate test) that, under strict mode, an invalid value like
`"ASD"` produces an error and does not get coerced to `null`/SUCCEEDED, so the
strict/lenient split is verified.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
