tkobayas commented on PR #7135: URL: https://github.com/apache/incubator-kie/pull/7135#issuecomment-6034210067
Hi @Rikkola , Regarding "Fact retraction", I think we need to re-confirm the expected behavior. In https://github.com/apache/incubator-kie/issues/7134 , ``` // Package B not ( $c : Container() and String( this == "blocked" ) from $c.items ) // Package C (triggers the segment split) not ( $c : Container() and String( this == "special" ) from $c.items ) 1. Build a KieBase from both packages. 2. Insert new Container("blocked") → only package C's rule fires. 3. Retract the Container → expected: both rules fire. Actual: package C's rule never fires again. ``` I think `3. Retract the Container → expected:` should be `only package B's rule fires`, because retracting the fact doesn't change the truth value of C's rule (TRUE -> TRUE). I have sent a PR to your branch -> https://github.com/Rikkola/drools/pull/13 I enhanced `NotFromListCrossPkgTest` with more test cases and added `NotFromListSamePkgTest` for "single rule" DRL and "multi rules in the same package" DRL. Key point is `NotFromListSamePkgTest.singleRule_trueTrue_multipleCycles` which demonstrates that retraction doesn't trigger the same rule firing again. Inserting another fact doesn't yet trigger the rule. The test passes in the main branch without the PR fix, so we may consider it's the expected behavior. But I'd like @mariofusco to double-check. This is the expected behavior? ``` private static final String DRL_SINGLE = "package repro.single;\n" + "import " + Container.class.getCanonicalName() + ";\n" + "global java.util.List results;\n" + "rule \"sibling\"\n" + "when\n" + " not ( $c : Container() and String( this == \"special\" ) from $c.items )\n" + "then\n" + " results.add(\"sibling\");\n" + "end\n"; public void singleRule_trueTrue_multipleCycles(KieBaseTestConfiguration cfg) { KieBase kbase = KieBaseUtil.getKieBaseFromKieModuleFromDrl("single-tt", cfg, DRL_SINGLE); KieSession ks = kbase.newKieSession(); try { List<String> results = new ArrayList<>(); ks.setGlobal("results", results); FactHandle fh1 = ks.insert(new Container("blocked")); // this is not "special", so fire ks.fireAllRules(); assertThat(results).containsExactly("sibling"); results.clear(); ks.delete(fh1); int fired1 = ks.fireAllRules(); assertThat(fired1).as("TRUE→TRUE: no re-fire on retract").isZero(); results.clear(); FactHandle fh2 = ks.insert(new Container("blocked")); int fired2 = ks.fireAllRules(); assertThat(fired2).as("TRUE→TRUE: no re-fire on cycle2 insert").isZero(); results.clear(); ks.delete(fh2); int fired3 = ks.fireAllRules(); assertThat(fired3).as("TRUE→TRUE: no re-fire on cycle2 retract").isZero(); } finally { ks.dispose(); } } ``` My PR has more tests, but we need to agree on the expected behavior first. WDYT? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
