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 3059b2237a0 [incubator-kie-issues#2419] fix(drools): collect facts,
not subnetwork tuples, in GenericCollectAccumulator (#7224)
3059b2237a0 is described below
commit 3059b2237a05af13884adeeb423455282e42b091
Author: Thebas <[email protected]>
AuthorDate: Mon Oct 26 07:38:57 2026 +0000
[incubator-kie-issues#2419] fix(drools): collect facts, not subnetwork
tuples, in GenericCollectAccumulator (#7224)
With the executable model, `from collect` into a concrete collection class
(ArrayList, LinkedList, HashSet, custom classes) goes through
GenericCollectAccumulator. When the collected pattern runs in a subnetwork
(an inner `from`, `from accumulate` or nested `from collect`), accumulate()
added the SubnetworkTuple itself to the collection instead of the matched
fact. Rules reading the elements broke, and marshalling the session failed
with NotSerializableException on AccumulateNode$AccumulateContext.
Unwrap the tuple through its fact handle, as the classic CollectAccumulator
does.
Co-authored-by: MARTINS FERRAZ Andre <[email protected]>
---
.../model/codegen/execmodel/AccumulateTest.java | 342 +++++++++++++++++++++
.../constraints/GenericCollectAccumulator.java | 5 +
.../compiler/integrationtests/AccumulateTest.java | 45 +++
3 files changed, 392 insertions(+)
diff --git
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
index 0ca109467f1..08ea148dcca 100644
---
a/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
+++
b/drools-model/drools-model-codegen/src/test/java/org/drools/model/codegen/execmodel/AccumulateTest.java
@@ -52,6 +52,7 @@ import org.drools.core.rule.consequence.InternalMatch;
import org.drools.model.functions.accumulate.GroupKey;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.MethodSource;
+import org.drools.model.codegen.execmodel.domain.Address;
import org.drools.model.codegen.execmodel.domain.Adult;
import org.drools.model.codegen.execmodel.domain.Child;
import org.drools.model.codegen.execmodel.domain.Customer;
@@ -4235,6 +4236,347 @@ public class AccumulateTest extends BaseModelTest {
}
}
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoArrayListFromSubnetwork(RUN_TYPE runType) {
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $list: ArrayList() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($list);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ // the collected list must hold the facts, not the subnetwork tuples
wrapping them
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+ }
+
+ // a custom concrete Collection target routed through
GenericCollectAccumulator;
+ // it needs a public no-arg constructor because the generic path uses
CustomType::new as a Supplier
+ public static class MyAddressCollection extends ArrayList<Object> {
+ public MyAddressCollection() {
+ super();
+ }
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoSetFromSubnetwork(RUN_TYPE runType) {
+ // Set target: routed through the collectSet path in FromCollectVisitor
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $set: java.util.Set() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($set);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ // the collected set must hold the facts, not the subnetwork tuples
wrapping them
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoHashSetFromSubnetwork(RUN_TYPE runType) {
+ // HashSet is a concrete type that is not List/Collection/Set, so it
is routed
+ // through GenericCollectAccumulator
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + HashSet.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $set: HashSet() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($set);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoLinkedListFromSubnetwork(RUN_TYPE runType) {
+ // LinkedList is a concrete type that is not List/Collection/Set, so
it is routed
+ // through GenericCollectAccumulator
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + java.util.LinkedList.class.getCanonicalName()
+ ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $list: LinkedList() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($list);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoCustomCollectionFromSubnetwork(RUN_TYPE
runType) {
+ // a user-defined concrete Collection goes through
GenericCollectAccumulator, which
+ // instantiates it via a no-arg constructor supplier (CustomType::new)
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + MyAddressCollection.class.getCanonicalName()
+ ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $coll: MyAddressCollection() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($coll);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoArrayListFromSubnetworkWithUpdate(RUN_TYPE
runType) {
+ // after removing an address and updating the Person through the
subnetwork, the
+ // collected ArrayList must reflect only the remaining address, with
no leaked
+ // SubnetworkTuple and no stale element
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $list: ArrayList() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.clear();\n" +
+ " result.addAll($list);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ FactHandle marioFH = ksession.insert(mario);
+
+ ksession.fireAllRules();
+
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+
+ // drop one address and re-evaluate through the subnetwork
+ mario.getAddresses().remove(milan);
+ ksession.update(marioFH, mario);
+ ksession.fireAllRules();
+
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactly(rome);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void testCollectIntoArrayListFromSubnetworkWithDelete(RUN_TYPE
runType) {
+ // deleting the Person must retract the whole subnetwork match; the
rule should no
+ // longer fire for it and the collected content must reflect the
deletion
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $list: ArrayList() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ " result.clear();\n" +
+ " result.addAll($list);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ Person mario = new Person("Mario", 40);
+ mario.addAddress(rome);
+ mario.addAddress(milan);
+ FactHandle marioFH = ksession.insert(mario);
+
+ int fired = ksession.fireAllRules();
+ assertThat(fired).isEqualTo(1);
+ assertThat(result).hasOnlyElementsOfType(Address.class);
+ assertThat(result).containsExactlyInAnyOrder(rome, milan);
+
+ // deleting the person removes the only match: the rule must not fire
again
+ ksession.delete(marioFH);
+ assertThat(ksession.fireAllRules()).isEqualTo(0);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void
testCollectIntoArrayListFromSubnetworkWithInnerAccumulate(RUN_TYPE runType) {
+ // the inner source of the collect is a pattern sourced from
accumulate, which also
+ // builds a subnetwork; ArrayList target exercises the
GenericCollectAccumulator path.
+ // count(...) yields a single Long equal to the number of Address
facts (2), so the
+ // outer collect gathers exactly that one Long - not the
SubnetworkTuple wrapping it.
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $list: ArrayList() from collect (\n" +
+ " Long() from accumulate ( $a: Address(); count($a)
)\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($list);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ ksession.insert(new Address("Rome"));
+ ksession.insert(new Address("Milan"));
+
+ ksession.fireAllRules();
+
+ // the collected list must hold the Long produced by the inner
accumulate,
+ // not the subnetwork tuple wrapping it
+ assertThat(result).hasOnlyElementsOfType(Long.class);
+ assertThat(result).containsExactly(2L);
+ }
+
+ @ParameterizedTest
+ @MethodSource("parameters")
+ public void
testCollectIntoArrayListFromSubnetworkWithNestedCollect(RUN_TYPE runType) {
+ // the inner source of the collect is itself a "from collect", which
builds a
+ // subnetwork too. The inner collect produces a single ArrayList
holding the Address
+ // facts; the outer ArrayList (GenericCollectAccumulator path) must
gather that one
+ // inner list as its single element - not the SubnetworkTuple wrapping
it.
+ String str = "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "global java.util.List result;\n" +
+ "rule R when\n" +
+ " $outer: ArrayList() from collect (\n" +
+ " ArrayList() from collect (\n" +
+ " Address()\n" +
+ " )\n" +
+ " )\n" +
+ "then\n" +
+ " result.addAll($outer);\n" +
+ "end";
+
+ KieSession ksession = getKieSession(runType, str);
+ List<Object> result = new ArrayList<>();
+ ksession.setGlobal("result", result);
+
+ Address rome = new Address("Rome");
+ Address milan = new Address("Milan");
+ ksession.insert(rome);
+ ksession.insert(milan);
+
+ ksession.fireAllRules();
+
+ // the outer list holds exactly one element: the ArrayList produced by
the inner
+ // collect, not the subnetwork tuple wrapping it
+ assertThat(result).hasOnlyElementsOfType(ArrayList.class);
+ assertThat(result).hasSize(1);
+ assertThat((ArrayList<Object>) result.get(0))
+ .hasOnlyElementsOfType(Address.class)
+ .containsExactlyInAnyOrder(rome, milan);
+ }
+
@ParameterizedTest
@MethodSource("parameters")
public void testExistsFromAccumulate(RUN_TYPE runType) {
diff --git
a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/constraints/GenericCollectAccumulator.java
b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/constraints/GenericCollectAccumulator.java
index 14497755e0b..ecdefcd95f2 100644
---
a/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/constraints/GenericCollectAccumulator.java
+++
b/drools-model/drools-model-compiler/src/main/java/org/drools/modelcompiler/constraints/GenericCollectAccumulator.java
@@ -22,6 +22,7 @@ import org.drools.base.base.ValueResolver;
import org.drools.base.reteoo.BaseTuple;
import org.drools.base.rule.Declaration;
import org.drools.base.rule.accessor.Accumulator;
+import org.drools.core.reteoo.SubnetworkTuple;
import org.kie.api.runtime.rule.FactHandle;
import java.util.Collection;
@@ -59,6 +60,10 @@ public class GenericCollectAccumulator implements
Accumulator {
Declaration[] innerDeclarations,
ValueResolver valueResolver) {
Object value = handle.getObject();
+ if (value instanceof SubnetworkTuple) {
+ // as in CollectAccumulator: collect the fact matched by the
subnetwork, not the tuple wrapping it
+ value = ( (SubnetworkTuple) value ).getFactHandle().getObject();
+ }
((Collection) context).add( value );
return value;
}
diff --git
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/AccumulateTest.java
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/AccumulateTest.java
index 011cb1753e1..ec4c831fd98 100644
---
a/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/AccumulateTest.java
+++
b/drools-test-coverage/test-compiler-integration/src/test/java/org/drools/compiler/integrationtests/AccumulateTest.java
@@ -36,9 +36,12 @@ import java.util.stream.Stream;
import org.drools.compiler.integrationtests.incrementalcompilation.TestUtil;
import org.drools.core.RuleSessionConfiguration;
+import org.drools.core.marshalling.ClassObjectMarshallingStrategyAcceptor;
+import org.drools.core.marshalling.SerializablePlaceholderResolverStrategy;
import org.drools.commands.runtime.rule.InsertElementsCommand;
import org.drools.kiesession.rulebase.InternalKnowledgeBase;
import org.drools.mvel.compiler.Primitives;
+import org.drools.testcoverage.common.model.Address;
import org.drools.testcoverage.common.model.Cheese;
import org.drools.testcoverage.common.model.Cheesery;
import org.drools.testcoverage.common.model.Order;
@@ -49,6 +52,7 @@ import org.drools.testcoverage.common.model.StockTick;
import org.drools.testcoverage.common.util.KieBaseTestConfiguration;
import org.drools.testcoverage.common.util.KieBaseUtil;
import org.drools.testcoverage.common.util.KieUtil;
+import org.drools.testcoverage.common.util.SerializationHelper;
import org.drools.testcoverage.common.util.TestParametersUtil2;
import org.junit.jupiter.api.Timeout;
import org.junit.jupiter.params.ParameterizedTest;
@@ -60,7 +64,9 @@ import org.kie.api.builder.Message;
import org.kie.api.builder.ReleaseId;
import org.kie.api.event.rule.AfterMatchFiredEvent;
import org.kie.api.event.rule.AgendaEventListener;
+import org.kie.api.marshalling.ObjectMarshallingStrategy;
import org.kie.api.runtime.KieContainer;
+import org.kie.api.runtime.EnvironmentName;
import org.kie.api.runtime.KieSession;
import org.kie.api.runtime.KieSessionConfiguration;
import org.kie.api.runtime.rule.AccumulateFunction;
@@ -4289,4 +4295,43 @@ public class AccumulateTest {
ksession.dispose();
}
}
+
+ @ParameterizedTest(name = "KieBase type={0}")
+ @MethodSource("parameters")
+ public void
testMarshallCollectIntoArrayListFromSubnetwork(KieBaseTestConfiguration
kieBaseTestConfiguration) throws Exception {
+ final String drl =
+ "import " + Person.class.getCanonicalName() + ";\n" +
+ "import " + Address.class.getCanonicalName() + ";\n" +
+ "import " + ArrayList.class.getCanonicalName() + ";\n" +
+ "rule R when\n" +
+ " $p: Person()\n" +
+ " $list: ArrayList() from collect (\n" +
+ " Address() from $p.addresses\n" +
+ " )\n" +
+ "then\n" +
+ "end";
+
+ final KieBase kbase =
KieBaseUtil.getKieBaseFromKieModuleFromDrl("accumulate-test",
kieBaseTestConfiguration, drl);
+ KieSession ksession = kbase.newKieSession();
+
System.setProperty(org.drools.core.util.KeyStoreConstants.PROP_ALLOWED_DESER_CLASS_PATTERNS,
+ "org.drools.testcoverage.common.model.Person;" +
+ "org.drools.testcoverage.common.model.Address");
+ try {
+
ksession.getEnvironment().set(EnvironmentName.OBJECT_MARSHALLING_STRATEGIES,
+ new ObjectMarshallingStrategy[]{new
SerializablePlaceholderResolverStrategy(ClassObjectMarshallingStrategyAcceptor.DEFAULT)});
+
+ final Person mario = new Person("Mario", 40);
+ mario.addAddress(new Address("Via Roma", 1, "Rome"));
+ mario.addAddress(new Address("Via Milano", 2, "Milan"));
+ ksession.insert(mario);
+ assertThat(ksession.fireAllRules()).isEqualTo(1);
+
+ ksession =
SerializationHelper.getSerialisedStatefulKnowledgeSession(ksession, true);
+
+ assertThat(ksession.fireAllRules()).isEqualTo(0);
+ } finally {
+
System.clearProperty(org.drools.core.util.KeyStoreConstants.PROP_ALLOWED_DESER_CLASS_PATTERNS);
+ ksession.dispose();
+ }
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
