This is an automated email from the ASF dual-hosted git repository.
tkobayas pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/incubator-kie.git
The following commit(s) were added to refs/heads/main by this push:
new abd77911788 [incubator-kie#7095] A ternary condition in an inline-eval
results in… (#7096)
abd77911788 is described below
commit abd7791178890662a9f890b990ed572a4c49bed7
Author: Toshiya Kobayashi <[email protected]>
AuthorDate: Fri Sep 11 15:10:24 2026 +0900
[incubator-kie#7095] A ternary condition in an inline-eval results in…
(#7096)
* [incubator-kie-7095] A ternary condition in an inline-eval results in a
wrong constraint and class-reactive
* copilot suggestion
* use DRL6 lexer to detect ternary instead of String manipulation
* improve comment
* removed meaningless test
---
.../compiler/rule/builder/PatternBuilder.java | 8 +-
.../java/org/drools/drl/parser/Drl6ExprParser.java | 22 +++
.../drools/drl/parser/ShouldPreserveEvalTest.java | 151 +++++++++++++++++++++
.../drools/model/codegen/execmodel/EvalTest.java | 85 ++++++++++++
4 files changed, 265 insertions(+), 1 deletion(-)
diff --git
a/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java
b/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java
index 4871ff8c924..f847b8eb98b 100644
---
a/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java
+++
b/drools-compiler/src/main/java/org/drools/compiler/rule/builder/PatternBuilder.java
@@ -98,6 +98,7 @@ import org.drools.drl.ast.descr.PredicateDescr;
import org.drools.drl.ast.descr.RelationalExprDescr;
import org.drools.drl.ast.descr.ReturnValueRestrictionDescr;
import org.drools.drl.ast.descr.RuleDescr;
+import org.drools.drl.parser.Drl6ExprParser;
import org.drools.drl.parser.DrlExprParser;
import org.drools.drl.parser.DrlExprParserFactory;
import org.drools.drl.parser.DroolsParserException;
@@ -1805,7 +1806,12 @@ public class PatternBuilder implements
RuleConditionBuilder<PatternDescr> {
final BaseDescr
original,
final String
expression) {
DrlExprParser parser =
DrlExprParserFactory.getDrlExprParser(context.getConfiguration().getOption(LanguageLevelOption.KEY));
- ConstraintConnectiveDescr result =
parser.parse(normalizeEval(expression));
+ String toParse = normalizeEval(expression);
+ if (!toParse.equals(expression) &&
Drl6ExprParser.shouldPreserveEval(toParse)) {
+ toParse = expression;
+ }
+ ConstraintConnectiveDescr result = parser.parse(toParse);
+
if (parser.hasErrors()) {
for (DroolsParserException error : parser.getErrors()) {
registerDescrBuildError(context, patternDescr,
diff --git
a/drools-drl/drools-drl-parser/src/main/java/org/drools/drl/parser/Drl6ExprParser.java
b/drools-drl/drools-drl-parser/src/main/java/org/drools/drl/parser/Drl6ExprParser.java
index 2376dd4f40f..6f9b68f06c8 100644
---
a/drools-drl/drools-drl-parser/src/main/java/org/drools/drl/parser/Drl6ExprParser.java
+++
b/drools-drl/drools-drl-parser/src/main/java/org/drools/drl/parser/Drl6ExprParser.java
@@ -25,8 +25,10 @@ import org.antlr.runtime.ANTLRStringStream;
import org.antlr.runtime.CommonTokenStream;
import org.antlr.runtime.RecognitionException;
import org.antlr.runtime.RecognizerSharedState;
+import org.antlr.runtime.Token;
import org.drools.drl.ast.descr.BaseDescr;
import org.drools.drl.ast.descr.ConstraintConnectiveDescr;
+import org.drools.drl.parser.lang.DRL6Lexer;
import org.drools.drl.parser.lang.DRLExpressions;
import org.drools.drl.parser.lang.DRLLexer;
import org.drools.drl.parser.lang.ParserHelper;
@@ -68,6 +70,26 @@ public class Drl6ExprParser implements DrlExprParser {
return constraint;
}
+ /**
+ * Returns whether to preserve the eval wrapper around the supplied
contents.
+ * The constraint parser enters at conditionalOrExpression, which is below
+ * ternaryExpression in the grammar, so a top-level ternary's branches are
+ * silently discarded. Question-mark tokens at any nesting depth may
indicate
+ * such a ternary. Strings and comments are ignored.
+ * Lexer errors also preserve the wrapper, leaving validation to
compilation.
+ * This is a conservative check, not validation of ternary syntax.
+ */
+ public static boolean shouldPreserveEval(String expression) {
+ DRL6Lexer lexer = new DRL6Lexer(new ANTLRStringStream(expression));
+ for (Token token = lexer.nextToken(); token.getType() != Token.EOF;
token = lexer.nextToken()) {
+ // QUESTION_DIV also covers a ternary immediately followed by a
comment: x?/*...*/y:z.
+ if (token.getType() == DRL6Lexer.QUESTION || token.getType() ==
DRL6Lexer.QUESTION_DIV) {
+ return true;
+ }
+ }
+ return !lexer.getErrors().isEmpty();
+ }
+
public String getLeftMostExpr() {
return helper != null ? helper.getLeftMostExpr() : null;
}
diff --git
a/drools-drl/drools-drl-parser/src/test/java/org/drools/drl/parser/ShouldPreserveEvalTest.java
b/drools-drl/drools-drl-parser/src/test/java/org/drools/drl/parser/ShouldPreserveEvalTest.java
new file mode 100644
index 00000000000..253962daf0f
--- /dev/null
+++
b/drools-drl/drools-drl-parser/src/test/java/org/drools/drl/parser/ShouldPreserveEvalTest.java
@@ -0,0 +1,151 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.drools.drl.parser;
+
+import org.junit.jupiter.api.Test;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+class ShouldPreserveEvalTest {
+
+ @Test
+ void ternaryOperator() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("\"foo\" == \"foo\" ?
\"foo\" == flag1 : \"foo\" == flag2")).isTrue();
+ }
+
+ @Test
+ void simpleTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x > 0 ? y :
z")).isTrue();
+ }
+
+ @Test
+ void noTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("age > 10")).isFalse();
+ }
+
+ @Test
+ void equalityExpression() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("length == 4")).isFalse();
+ }
+
+ @Test
+ void questionMarkInsideStringLiteral() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("name ==
\"what?\"")).isFalse();
+ }
+
+ @Test
+ void escapedQuoteBeforeTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("\"val\\\"ue\" == flag ?
x : y")).isTrue();
+ }
+
+ @Test
+ void questionMarkInsideStringWithEscapedQuote() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("name ==
\"is\\\"this?real\"")).isFalse();
+ }
+
+ @Test
+ void singleQuotedStringWithQuestionMark() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("name ==
'what?'")).isFalse();
+ }
+
+ @Test
+ void questionMarkWithoutColon() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x?")).isTrue();
+ }
+
+ @Test
+ void colonInsideStringAfterQuestionMark() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x ? \":\"")).isTrue();
+ }
+
+ @Test
+ void emptyExpression() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("")).isFalse();
+ }
+
+ @Test
+ void complexTernaryWithLogicalOperators() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("a > 0 && b < 10 ? c :
d")).isTrue();
+ }
+
+ @Test
+ void parenthesizedTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("(x ? y : z)")).isTrue();
+ }
+
+ @Test
+ void parenthesizedTernaryInEquality() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("(x ? y : z) ==
true")).isTrue();
+ }
+
+ @Test
+ void nestedTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x ? y ? a : b :
z")).isTrue();
+ }
+
+ @Test
+ void parenthesizedComparisonTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("((x > 0 ? y : z)) ==
true")).isTrue();
+ }
+
+ @Test
+ void ternaryInMethodArgument() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("check(x > 0 ? y :
z)")).isTrue();
+ }
+
+ @Test
+ void singleQuotedTernary() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("'foo' == flag ? 'yes' :
'no'")).isTrue();
+ }
+
+ @Test
+ void ternaryImmediatelyFollowedByComment() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x?/* comment
*/y:z")).isTrue();
+ }
+
+ @Test
+ void questionMarkInsideBlockComment() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("length /* ?: */ ==
4")).isFalse();
+ }
+
+ @Test
+ void questionMarkInsideLineComment() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("length == 4 // ?:
comment")).isFalse();
+ }
+
+ @Test
+ void drlNullSafeOperatorWithoutQuestionMark() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("address!.city ==
'London'")).isFalse();
+ }
+
+ @Test
+ void ternaryWithTrailingTokens() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("x ? y : z
garbage")).isTrue();
+ }
+
+ @Test
+ void unterminatedString() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("name ==
'unterminated")).isTrue();
+ }
+
+ @Test
+ void unterminatedComment() {
+ assertThat(Drl6ExprParser.shouldPreserveEval("length == 4 /*
unterminated")).isTrue();
+ }
+}
diff --git
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/EvalTest.java
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/EvalTest.java
index 77fa035cbe5..04715d46022 100644
---
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/EvalTest.java
+++
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/EvalTest.java
@@ -775,4 +775,89 @@ public class EvalTest extends BaseModelTest {
assertThat(list).as("R1 should not
fire").containsExactly("ModifyingRule");
}
}
+
+ @ParameterizedTest
+ @MethodSource("parametersStandardOnly") // standard only: inline eval with
ternary is not supported in exec-model.
+ // See
https://github.com/apache/incubator-kie/issues/7097
+ void testTernaryEvalInsidePattern(RUN_TYPE runType) {
+ String str =
+ "import " + MyPerson.class.getCanonicalName() + ";\n" +
+ "rule \"TernaryEvalLoop\"\n" +
+ "dialect \"mvel\"\n" +
+ "when\n" +
+ " String(this == \"go\")\n" +
+ " $p : MyPerson( eval(\"foo\" == \"foo\" ? \"foo\" ==
flag1 : \"foo\" == flag2) )\n" +
+ "then\n" +
+ " modify($p) {\n" +
+ " setOtherAttribute(\"done\")\n" +
+ " }\n" +
+ "end";
+
+ //-- 1st round
+
+ KieSession ksession = getKieSession(runType, str);
+ try {
+ MyPerson person = new MyPerson();
+ person.setFlag1("foo"); // matches the rule
+ person.setFlag2("bar");
+
+ ksession.insert("go");
+ ksession.insert(person);
+ int fired = ksession.fireAllRules(10);
+
+ assertThat(fired).isEqualTo(1);
+ assertThat(person.getOtherAttribute()).isEqualTo("done");
+ } finally {
+ ksession.dispose();
+ }
+
+ //-- 2nd round
+
+ ksession = getKieSession(runType, str);
+ try {
+ MyPerson person = new MyPerson();
+ person.setFlag1("bar"); // doesn't match the rule
+ person.setFlag2("bar");
+
+ ksession.insert("go");
+ ksession.insert(person);
+ int fired = ksession.fireAllRules(10); // do not fire
+
+ assertThat(fired).isZero();
+ assertThat(person.getOtherAttribute()).isNull();
+ } finally {
+ ksession.dispose();
+ }
+ }
+
+ public static class MyPerson {
+ private String flag1;
+ private String flag2;
+ private String otherAttribute;
+
+ public String getFlag1() {
+ return flag1;
+ }
+
+ public void setFlag1(String flag1) {
+ this.flag1 = flag1;
+ }
+
+ public String getFlag2() {
+ return flag2;
+ }
+
+ public void setFlag2(String flag2) {
+ this.flag2 = flag2;
+ }
+
+ public String getOtherAttribute() {
+ return otherAttribute;
+ }
+
+ public void setOtherAttribute(String otherAttribute) {
+ this.otherAttribute = otherAttribute;
+ }
+ }
+
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]