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]

Reply via email to