Copilot commented on code in PR #6556:
URL:
https://github.com/apache/incubator-kie-drools/pull/6556#discussion_r2707330935
##########
drools-scenario-simulation/drools-scenario-simulation-backend/src/main/java/org/drools/scenariosimulation/backend/expression/DMNFeelExpressionEvaluator.java:
##########
@@ -117,7 +118,7 @@ public Either<List<FEELEvent>, Boolean> apply(FEEL feel) {
final FEELEventListener utErrorListener = errorEvent ->
utEvalErrors.add(errorEvent);
EvaluationContext evaluationContext = ((FEELImpl)
feel).newEvaluationContext(List.of(utErrorListener),
Collections.singletonMap(UNARY_PARAMETER_IDENTIFIER,
-
resultValue));
+
resultValue), "RUNTIME_MODE_LENIENT");
Review Comment:
The string literal "RUNTIME_MODE_LENIENT" should use the constant
RUNTIME_MODE_LENIENT instead. The constant is defined on line 55 as "LENIENT",
but here it's being used as the string literal "RUNTIME_MODE_LENIENT" which
would be passed as the runtime mode value.
```suggestion
resultValue), RUNTIME_MODE_LENIENT);
```
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/decisiontables/DecisionTableImpl.java:
##########
@@ -229,13 +230,21 @@ private Either<FEELEvent, Object>
actualInputsMatchInputValues(EvaluationContext
}
if ( !satisfies ) {
- String values = input.getInputValuesText();
- return Either.ofLeft(new InvalidInputEvent(
FEELEvent.Severity.ERROR,
-
input.getInputExpression()+"='" + parameter + "' does not match any of the
valid values " + values + " for decision table '" + getName() + "'.",
- getName(),
- null,
- values )
- );
+ // Check if runtimeMode is "strict" before returning error
+ if (ctx instanceof EvaluationContextImpl) {
+ String runtimeMode = ((EvaluationContextImpl)
ctx).getRuntimeMode();
+ if (RUNTIME_MODE_STRICT.equalsIgnoreCase(runtimeMode))
{
Review Comment:
The runtime mode string comparison uses equalsIgnoreCase for "strict", but
the underlying RuntimeModeOption.MODE enum uses lowercase "strict" in its mode
field. Since the RuntimeModeOption.MODE.name() returns "STRICT" (uppercase) and
getMode() returns "strict" (lowercase), this creates potential for confusion.
Consider standardizing on either using the enum's getMode() method or comparing
against the enum directly rather than string comparisons.
```suggestion
if (RUNTIME_MODE_STRICT.equals(runtimeMode)) {
```
##########
kie-dmn/kie-dmn-feel/src/main/java/org/kie/dmn/feel/runtime/decisiontables/DecisionTableImpl.java:
##########
@@ -229,13 +230,21 @@ private Either<FEELEvent, Object>
actualInputsMatchInputValues(EvaluationContext
}
if ( !satisfies ) {
- String values = input.getInputValuesText();
- return Either.ofLeft(new InvalidInputEvent(
FEELEvent.Severity.ERROR,
-
input.getInputExpression()+"='" + parameter + "' does not match any of the
valid values " + values + " for decision table '" + getName() + "'.",
- getName(),
- null,
- values )
- );
+ // Check if runtimeMode is "strict" before returning error
+ if (ctx instanceof EvaluationContextImpl) {
+ String runtimeMode = ((EvaluationContextImpl)
ctx).getRuntimeMode();
+ if (RUNTIME_MODE_STRICT.equalsIgnoreCase(runtimeMode))
{
+ String values = input.getInputValuesText();
+ return Either.ofLeft(new InvalidInputEvent(
FEELEvent.Severity.ERROR,
+
input.getInputExpression()+"='" + parameter + "' does not match any of the
valid values " + values + " for decision table '" + getName() + "'.",
+ getName(),
+ null,
+ values )
+ );
+ } else {
+ params[i] = null;
+ }
Review Comment:
When the context is not an instance of EvaluationContextImpl or when
runtimeMode is null/not strict, the code falls through without returning, which
means it continues processing without handling the case where input validation
failed but wasn't in strict mode. While the current logic sets params[i] = null
in lenient mode, there's no explicit return statement or else clause handling
the case when ctx is not an EvaluationContextImpl instance, potentially
allowing processing to continue with invalid inputs.
```suggestion
} else {
// Lenient mode (including null runtimeMode):
treat invalid input as null
params[i] = null;
}
} else {
// Context is not an EvaluationContextImpl: fall
back to lenient behavior
params[i] = null;
```
##########
kie-dmn/kie-dmn-validation/src/main/java/org/kie/dmn/validation/dtanalysis/DMNDTAnalyserValueFromNodeVisitor.java:
##########
@@ -219,7 +220,7 @@ public Boolean visit(FunctionInvocationNode n) {
}
private Comparable<?> blankEvaluate(FunctionInvocationNode n) {
- return (Comparable<?>)
n.evaluate(FEEL.newEvaluationContext(Collections.emptyList(),
Collections.emptyMap()));
+ return (Comparable<?>)
n.evaluate(FEEL.newEvaluationContext(Collections.emptyList(),
Collections.emptyMap(), RUNTIME_MODE_LENIENT));
Review Comment:
There's an inconsistency in how runtime mode values are represented across
the codebase. The RuntimeModeOption.MODE enum uses lowercase values ("lenient",
"strict"), but this code uses "LENIENT". The DecisionTableImpl uses "strict"
(lowercase) for comparison. Consider using the enum's getMode() method or
calling .name() consistently across all call sites to maintain consistency.
```suggestion
return (Comparable<?>)
n.evaluate(FEEL.newEvaluationContext(Collections.emptyList(),
Collections.emptyMap(), FEELImpl.RuntimeModeOption.MODE.LENIENT.getMode()));
```
--
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]
