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-drools.git


The following commit(s) were added to refs/heads/main by this push:
     new dade33c2b2 [incubator-kie-drools-6420] modify or update in DRL causing 
rules wit… (#6430)
dade33c2b2 is described below

commit dade33c2b2168423fffdc4d242a67a4db06cce27
Author: Toshiya Kobayashi <[email protected]>
AuthorDate: Fri Sep 5 09:17:49 2025 +0900

    [incubator-kie-drools-6420] modify or update in DRL causing rules wit… 
(#6430)
    
    * [incubator-kie-drools-6420] modify or update in DRL causing rules with 
eval condition to fire even when pattern constraints are not matching
    - Test case only
    
    * - Add more tests
    - Revert disablePropertyReactivity (DROOLS-7255), so JoinNode before Eval 
is back to property reactive, considering it's relatively consistent. See PR 
6430 discussion
    
    * comment
---
 .../main/java/org/drools/core/reteoo/BetaNode.java |   7 -
 .../org/drools/core/reteoo/EvalConditionNode.java  |   8 -
 .../drools/model/codegen/execmodel/EvalTest.java   | 268 ++++++++++++++++++---
 3 files changed, 240 insertions(+), 43 deletions(-)

diff --git a/drools-core/src/main/java/org/drools/core/reteoo/BetaNode.java 
b/drools-core/src/main/java/org/drools/core/reteoo/BetaNode.java
index 863abfa1fe..df599cc26d 100644
--- a/drools-core/src/main/java/org/drools/core/reteoo/BetaNode.java
+++ b/drools-core/src/main/java/org/drools/core/reteoo/BetaNode.java
@@ -666,13 +666,6 @@ public abstract class BetaNode extends LeftTupleSource
         return rightInferredMask;
     }
 
-    void disablePropertyReactivity() {
-        rightInferredMask = AllSetBitMask.get();
-        if (NodeTypeEnums.isBetaNode(leftInput)) {
-            ((BetaNode)leftInput).disablePropertyReactivity();
-        }
-    }
-
     public BitMask getRightNegativeMask() {
         return rightNegativeMask;
     }
diff --git 
a/drools-core/src/main/java/org/drools/core/reteoo/EvalConditionNode.java 
b/drools-core/src/main/java/org/drools/core/reteoo/EvalConditionNode.java
index 3f807a3bf6..0a4ff2e5ca 100644
--- a/drools-core/src/main/java/org/drools/core/reteoo/EvalConditionNode.java
+++ b/drools-core/src/main/java/org/drools/core/reteoo/EvalConditionNode.java
@@ -86,14 +86,6 @@ public class EvalConditionNode extends LeftTupleSource
         this.leftInput.networkUpdated(updateContext);
     }
 
-    @Override
-    protected void initInferredMask(LeftTupleSource leftInput) {
-        super.initInferredMask( leftInput );
-        if (NodeTypeEnums.isBetaNode(leftInput)) {
-            ((BetaNode)leftInput).disablePropertyReactivity();
-        }
-    }
-
     // ------------------------------------------------------------
     // Instance methods
     // ------------------------------------------------------------
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 7200ba31c0..77fa035cbe 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
@@ -18,7 +18,9 @@
  */
 package org.drools.model.codegen.execmodel;
 
+import java.util.ArrayList;
 import java.util.Collection;
+import java.util.List;
 
 import org.drools.model.codegen.execmodel.domain.CalcFact;
 import org.drools.model.codegen.execmodel.domain.Overloaded;
@@ -523,44 +525,254 @@ public class EvalTest extends BaseModelTest {
 
     @ParameterizedTest
        @MethodSource("parameters")
-    public void testModifyEvalAfterJoin(RUN_TYPE runType) {
+    public void 
testModifyEvalAfterJoinWithEmptyConstraint_withoutWatch_shouldNotFire(RUN_TYPE 
runType) {
+        testModifyEvalAfterJoinWithEmptyConstraint(runType, "", false);
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    public void 
testModifyEvalAfterJoinWithEmptyConstraint_withWatch_shouldFire(RUN_TYPE 
runType) {
+        testModifyEvalAfterJoinWithEmptyConstraint(runType, "@watch( t )", 
true);
+    }
+
+    private void testModifyEvalAfterJoinWithEmptyConstraint(RUN_TYPE runType, 
String watchAnnotation, boolean shouldFireR1) {
         // DROOLS-7255
         String str =
                 "import " + Dt1.class.getCanonicalName() + ";" +
-                "import " + Dt2.class.getCanonicalName() + ";" +
-                "\n" +
-                "rule \"b_1\"\n" +
-                "    when\n" +
-                "        dt1 : Dt1( )\n" +
-                "        dt2 : Dt2( )\n" +
-                "        eval(dt2.getT()==\"YES\")\n" +
-                "    then\n" +
-                "end\n" +
-                "\n" +
-                "rule \"b_2\"\n" +
-                "    when\n" +
-                "        dt2 : Dt2(  )\n" +
-                "        eval(dt2.getT()==\"YES\")\n" +
-                "    then\n" +
-                "end\n" +
-                "\n" +
-                "rule \"modify dt2 rule\"\n" +
-                "    when\n" +
-                "        dt1 : Dt1(a == 1)\n" +
-                "        dt2 : Dt2( )\n" +
-                "    then\n" +
-                "        modify( dt2 ) {\n" +
-                "            setT(\"YES\")\n" +
-                "        }\n" +
-                "end\n";
+                        "import " + Dt2.class.getCanonicalName() + ";" +
+                        "\n" +
+                        "global java.util.List list;\n" +
+                        "rule R1\n" +
+                        "    when\n" +
+                        "        dt1 : Dt1( )\n" +
+                        "        dt2 : Dt2( ) " + watchAnnotation + "\n" + // 
without watch, JoinNode doesn't proceed on modify
+                        "        eval(dt2.getT()==\"YES\")\n" +
+                        "    then\n" +
+                        "        list.add(\"R1\");\n" +
+                        "end\n" +
+                        "\n" +
+                        "rule R2\n" +
+                        "    when\n" +
+                        "        dt2 : Dt2(  )\n" +
+                        "        eval(dt2.getT()==\"YES\")\n" + // There is no 
JoinNode here, so this rule fires on modify
+                        "    then\n" +
+                        "        list.add(\"R2\");\n" +
+                        "end\n" +
+                        "\n" +
+                        "rule ModifyingRule\n" +
+                        "    when\n" +
+                        "        dt1 : Dt1(a == 1)\n" +
+                        "        dt2 : Dt2( )\n" +
+                        "    then\n" +
+                        "        list.add(\"ModifyingRule\");\n" +
+                        "        modify( dt2 ) {\n" +
+                        "            setT(\"YES\")\n" +
+                        "        }\n" +
+                        "end\n";
 
         KieSession ksession = getKieSession(runType, str);
+        List<String> list = new ArrayList<>();
+        ksession.setGlobal("list", list);
 
         Dt1 dt1 = new Dt1();
         dt1.setA(1);
         ksession.insert(dt1);
         ksession.insert(new Dt2());
 
-        assertThat(ksession.fireAllRules()).isEqualTo(3);
+        ksession.fireAllRules();
+
+        if (shouldFireR1) {
+            assertThat(list).as("All rules should 
fire").containsExactlyInAnyOrder("ModifyingRule", "R1", "R2");
+        } else {
+            assertThat(list).as("R1 should not 
fire").containsExactlyInAnyOrder("ModifyingRule", "R2");
+        }
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void testModifyEvalAfterJoinWithNonMatchingAlpha(RUN_TYPE runType) {
+        // incubator-kie-drools/issues/6420
+        String str =
+                "import " + Person.class.getCanonicalName() + ";\n" +
+                        "global java.util.List list;\n" +
+                        "rule R1\n" +
+                        "    when\n" +
+                        "        String( )\n" +
+                        "        $p : Person( name == \"John\")\n" + // 
obviously non-matching
+                        "        eval($p.getAge() == 10)\n" +
+                        "    then\n" +
+                        "        list.add(\"R1 : \" + $p.getName());\n" +
+                        "end\n" +
+                        "rule ModifyingRule\n" +
+                        "    when\n" +
+                        "        String( this == \"go\" )\n" +
+                        "        $p : Person( name == \"Paul\")\n" +
+                        "    then\n" +
+                        "        modify( $p ) {\n" +
+                        "            setAge(10)\n" +
+                        "        }\n" +
+                        "        list.add(\"ModifyingRule\");\n" +
+                        "end\n";
+
+        KieSession ksession = getKieSession(runType, str);
+
+        List<String> list = new ArrayList<>();
+        ksession.setGlobal("list", list);
+
+        Person paul = new Person("Paul", 20);
+        ksession.insert("go");
+        ksession.insert(paul);
+
+        ksession.fireAllRules();
+        assertThat(list).as("R1 should not 
fire").containsExactly("ModifyingRule");
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void 
testModifyEvalAfterJoinWithMatchingAlpha_withoutWatch_shouldNotFire(RUN_TYPE 
runType) {
+        testModifyEvalAfterJoinWithMatchingAlpha(runType, "", false);
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void 
testModifyEvalAfterJoinWithMatchingAlpha_withWatch_shouldFire(RUN_TYPE runType) 
{
+        testModifyEvalAfterJoinWithMatchingAlpha(runType, "@watch( age )", 
true);
+    }
+
+    private void testModifyEvalAfterJoinWithMatchingAlpha(RUN_TYPE runType, 
String watchAnnotation, boolean shouldFireR1) {
+        // incubator-kie-drools/issues/6420
+        // This test is already successful. Just for coverage.
+        String str =
+                "import " + Person.class.getCanonicalName() + ";\n" +
+                        "global java.util.List list;\n" +
+                        "rule R1\n" +
+                        "    when\n" +
+                        "        String( )\n" +
+                        "        $p : Person( name == \"Paul\") " + 
watchAnnotation + "\n" + // without watch, JoinNode doesn't proceed on modify
+                        "        eval($p.getAge() == 10)\n" +
+                        "    then\n" +
+                        "        list.add(\"R1 : \" + $p.getName());\n" +
+                        "end\n" +
+                        "rule ModifyingRule\n" +
+                        "    when\n" +
+                        "        String( this == \"go\" )\n" +
+                        "        $p : Person( name == \"Paul\")\n" +
+                        "    then\n" +
+                        "        modify( $p ) {\n" +
+                        "            setAge(10)\n" +
+                        "        }\n" +
+                        "        list.add(\"ModifyingRule\");\n" +
+                        "end\n";
+
+        KieSession ksession = getKieSession(runType, str);
+        List<String> list = new ArrayList<>();
+        ksession.setGlobal("list", list);
+
+        Person paul = new Person("Paul", 20);
+        ksession.insert("go");
+        ksession.insert(paul);
+
+        ksession.fireAllRules();
+
+        if (shouldFireR1) {
+            assertThat(list).as("Both rules should 
fire").containsExactly("ModifyingRule", "R1 : Paul");
+        } else {
+            assertThat(list).as("R1 should not 
fire").containsExactly("ModifyingRule");
+        }
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void testModifyJoinWithNonMatchingAlpha(RUN_TYPE runType) {
+        // This test doesn't include eval. But added here for coverage.
+        String str =
+                "import " + Person.class.getCanonicalName() + ";\n" +
+                        "global java.util.List list;\n" +
+
+                        "rule R1\n" +
+                        "    when\n" +
+                        "        String( )\n" +
+                        "        $p : Person( name == \"John\")\n" +
+                        "        Integer( )\n" +
+                        "    then\n" +
+                        "        list.add(\"R1 : \" + $p.getName());\n" +
+                        "end\n" +
+                        "rule ModifyingRule\n" +
+                        "    when\n" +
+                        "        String( this == \"go\" )\n" +
+                        "        $p : Person( name == \"Paul\" )\n" +
+                        "    then\n" +
+                        "        modify( $p ) {\n" +
+                        "            setAge(10)\n" +
+                        "        }\n" +
+                        "        list.add(\"ModifyingRule\");\n" +
+                        "end\n";
+
+        KieSession ksession = getKieSession(runType, str);
+        List<String> list = new ArrayList<>();
+        ksession.setGlobal("list", list);
+
+        Person paul = new Person("Paul", 20);
+        ksession.insert("go");
+        ksession.insert(paul);
+        ksession.insert(0); // any Integer matches
+
+        ksession.fireAllRules();
+        assertThat(list).as("R1 should not 
fire").containsExactly("ModifyingRule");
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void 
testModifyEvalAfterJoinWithMatchingAlphaSharingJoin_withoutWatch_shouldNotFire(RUN_TYPE
 runType) {
+        testModifyEvalAfterJoinWithMatchingAlphaSharingJoin(runType, "", 
false);
+    }
+
+    @ParameterizedTest
+    @MethodSource("parameters")
+    void 
testModifyEvalAfterJoinWithMatchingAlphaSharingJoin_withWatch_shouldFire(RUN_TYPE
 runType) {
+        testModifyEvalAfterJoinWithMatchingAlphaSharingJoin(runType, "@watch( 
age )", true);
+    }
+
+    private void testModifyEvalAfterJoinWithMatchingAlphaSharingJoin(RUN_TYPE 
runType, String watchAnnotation, boolean shouldFireR1) {
+        // This rule is similar to testModifyEvalAfterJoinWithMatchingAlpha, 
but sharing JoinNode
+        // If AlphaNode or JoinNode disables property reactive, this rule 
would cause an infinite loop
+        String str =
+                "import " + Person.class.getCanonicalName() + ";\n" +
+                        "global java.util.List list;\n" +
+                        "rule R1\n" +
+                        "    when\n" +
+                        "        String( this == \"go\" )\n" +
+                        "        $p : Person( name == \"Paul\") " + 
watchAnnotation + "\n" + // without watch, JoinNode doesn't proceed on modify
+                        "        eval($p.getAge() == 10) \n" +
+                        "    then\n" +
+                        "        list.add(\"R1 : \" + $p.getName());\n" +
+                        "end\n" +
+                        "rule ModifyingRule\n" +
+                        "    when\n" +
+                        "        String( this == \"go\" )\n" +
+                        "        $p : Person( name == \"Paul\")\n" +
+                        "    then\n" +
+                        "        modify( $p ) {\n" +
+                        "            setAge(10)\n" +
+                        "        }\n" +
+                        "        list.add(\"ModifyingRule\");\n" +
+                        "end\n";
+
+        KieSession ksession = getKieSession(runType, str);
+        List<String> list = new ArrayList<>();
+        ksession.setGlobal("list", list);
+
+        Person paul = new Person("Paul", 20);
+        ksession.insert("go");
+        ksession.insert(paul);
+        ksession.insert(0);
+
+        ksession.fireAllRules(10); // 10 doesn't affect the test execution, 
but just to avoid an unexpected loop for the future.
+
+        if (shouldFireR1) {
+            assertThat(list).as("Both rules should 
fire").containsExactly("ModifyingRule", "R1 : Paul");
+        } else {
+            assertThat(list).as("R1 should not 
fire").containsExactly("ModifyingRule");
+        }
     }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to