tkobayas commented on code in PR #6430: URL: https://github.com/apache/incubator-kie-drools/pull/6430#discussion_r2320532883
########## drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/EvalTest.java: ########## @@ -523,44 +525,254 @@ public void setT(String t) { @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); + + 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); Review Comment: This rule works for any Integer. Using a constant/variable sounds too much. Added a comment instead. -- 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: commits-unsubscr...@kie.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@kie.apache.org For additional commands, e-mail: commits-h...@kie.apache.org