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]