DROOLS-1302 OOPath ReactiveList remove cases (#932)

* DROOLS-1302 OOPath ReactiveList remove cases Basic remove case now covered. * Extended remove case coverage.
apache · Sep 23, 2016 · 51c13f8 · 51c13f8
1 parent b08c1cc
commit 51c13f8
Show file tree

Hide file tree

Showing 7 changed files with 178 additions and 10 deletions.
diff --git a/drools-compiler/src/test/java/org/drools/compiler/xpath/Group.java b/drools-compiler/src/test/java/org/drools/compiler/xpath/Group.java
@@ -0,0 +1,31 @@
+package org.drools.compiler.xpath;
+
+import java.util.List;
+
+import org.drools.core.phreak.AbstractReactiveObject;
+import org.drools.core.phreak.ReactiveList;
+
+public class Group extends AbstractReactiveObject {
+    private final String name;
+    private final List<Person> members = new ReactiveList<Person>();
+
+    public Group(String name) {
+        this.name = name;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public List<Person> getMembers() {
+        return members;
+    }
+
+    public void addPerson(Person p) {
+        members.add(p);
+    }
+
+    public void removePerson(Person p) {
+        members.remove(p);
+    }
+}
diff --git a/drools-compiler/src/test/java/org/drools/compiler/xpath/School.java b/drools-compiler/src/test/java/org/drools/compiler/xpath/School.java
@@ -16,6 +16,7 @@
 package org.drools.compiler.xpath;
 
 import org.drools.core.phreak.AbstractReactiveObject;
+import org.drools.core.phreak.ReactiveList;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -24,7 +25,7 @@ public class School extends AbstractReactiveObject {
 
     private final String name;
 
-    private final List<Child> children = new ArrayList<Child>();
+    private final List<Child> children = new ReactiveList<Child>();
 
     public School(String name) {
         this.name = name;

diff --git a/drools-compiler/src/test/java/org/drools/compiler/xpath/XpathTest.java b/drools-compiler/src/test/java/org/drools/compiler/xpath/XpathTest.java
@@ -34,16 +34,20 @@
 import org.kie.api.io.ResourceType;
 import org.kie.api.runtime.KieSession;
 import org.kie.internal.utils.KieHelper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static java.util.Arrays.asList;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
 public class XpathTest {
-
+    public static final Logger LOG = LoggerFactory.getLogger(XpathTest.class);
+
     @Test
     public void testClassSimplestXpath() {
         String drl =
@@ -458,6 +462,108 @@ public void testReactiveDeleteOnLia() {
             assertTrue( obj == charlie );
         }
     }
+
+    @Test
+    public void testRemoveFromReactiveListBasic() {
+        String drl =
+                "import org.drools.compiler.xpath.*;\n" +
+                "\n" +
+                "rule R2 when\n" +
+                "  School( $child: /children{age >= 13 && age < 20} )\n" +
+                "then\n" +
+                "  System.out.println( $child );\n" +
+                "  insertLogical( $child );\n" +
+                "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        Child charlie = new Child( "Charles", 15 );
+        Child debbie = new Child( "Debbie", 19 );
+        School school = new School( "Da Vinci" );
+        school.addChild( charlie );
+
+        ksession.insert( school );
+        ksession.fireAllRules();
+        assertTrue(ksession.getObjects().contains(charlie));
+        assertFalse(ksession.getObjects().contains(debbie));
+
+        school.addChild( debbie );
+        ksession.fireAllRules();
+        assertTrue(ksession.getObjects().contains(charlie));
+        assertTrue(ksession.getObjects().contains(debbie));
+
+        school.getChildren().remove( debbie );
+        ksession.fireAllRules();
+        assertTrue(ksession.getObjects().contains(charlie));
+        assertFalse(ksession.getObjects().contains(debbie));
+
+        school.addChild( debbie );
+        ksession.fireAllRules();
+        assertTrue(ksession.getObjects().contains(charlie));
+        assertTrue(ksession.getObjects().contains(debbie));
+
+        debbie.setAge( 20 );        
+        ksession.fireAllRules();
+        assertTrue(ksession.getObjects().contains(charlie));
+        assertFalse(ksession.getObjects().contains(debbie));
+    }
+
+    @Test
+    public void testRemoveFromReactiveListExtended() {
+        String drl =
+                "import org.drools.compiler.xpath.*;\n" +
+                "\n" +
+                "rule R2 when\n" +
+                "  Group( $id: name, $p: /members{age >= 20} )\n" +
+                "then\n" +
+                "  System.out.println( $id + \".\" + $p.getName() );\n" +
+                "  insertLogical(      $id + \".\" + $p.getName() );\n" +
+                "end\n";
+
+        KieSession ksession = new KieHelper().addContent( drl, ResourceType.DRL )
+                                             .build()
+                                             .newKieSession();
+
+        Adult ada = new Adult("Ada", 19);
+        Adult bea = new Adult("Bea", 19);
+        Group x = new Group("X");
+        Group y = new Group("Y");
+        x.addPerson(ada);
+        x.addPerson(bea);
+        y.addPerson(ada);
+        y.addPerson(bea);
+        ksession.insert( x );
+        ksession.insert( y );
+        ksession.fireAllRules();
+        assertFalse (factsCollection(ksession).contains("X.Ada"));
+        assertFalse (factsCollection(ksession).contains("X.Bea"));
+        assertFalse (factsCollection(ksession).contains("Y.Ada"));
+        assertFalse (factsCollection(ksession).contains("Y.Bea"));
+
+        ada.setAge( 20 );        
+        ksession.fireAllRules();
+        ksession.getObjects().forEach(System.out::println);
+        assertTrue  (factsCollection(ksession).contains("X.Ada"));
+        assertFalse (factsCollection(ksession).contains("X.Bea"));
+        assertTrue  (factsCollection(ksession).contains("Y.Ada"));
+        assertFalse (factsCollection(ksession).contains("Y.Bea"));
+
+        y.removePerson(bea);
+        bea.setAge( 20 );        
+        ksession.fireAllRules();
+        assertTrue  (factsCollection(ksession).contains("X.Ada"));
+        assertTrue  (factsCollection(ksession).contains("X.Bea"));
+        assertTrue  (factsCollection(ksession).contains("Y.Ada"));
+        assertFalse (factsCollection(ksession).contains("Y.Bea"));
+    }
+
+    private List<?> factsCollection(KieSession ksession) {
+        List<Object> res = new ArrayList<>();
+        res.addAll(ksession.getObjects());
+        return res;
+    }
 
     @Test
     public void testReactiveOnBeta() {

diff --git a/drools-core/src/main/java/org/drools/core/phreak/AbstractReactiveObject.java b/drools-core/src/main/java/org/drools/core/phreak/AbstractReactiveObject.java
@@ -19,6 +19,7 @@
 import org.drools.core.spi.Tuple;
 
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 public class AbstractReactiveObject implements ReactiveObject {
@@ -32,10 +33,15 @@ public void addLeftTuple(Tuple leftTuple) {
     }
 
     public List<Tuple> getLeftTuples() {
-        return lts;
+        return lts != null ? lts : Collections.emptyList();
     }
 
     protected void notifyModification() {
         ReactiveObjectUtil.notifyModification(this);
     }
+
+    @Override
+    public void removeLeftTuple(Tuple leftTuple) {
+        lts.remove(leftTuple);
+    }
 }
diff --git a/drools-core/src/main/java/org/drools/core/phreak/ReactiveList.java b/drools-core/src/main/java/org/drools/core/phreak/ReactiveList.java
@@ -21,6 +21,9 @@
 import java.util.List;
 import java.util.ListIterator;
 
+import org.drools.core.phreak.ReactiveObjectUtil.ModificationType;
+import org.drools.core.spi.Tuple;
+
 public class ReactiveList<T> extends AbstractReactiveObject implements List<T> {
 
     private final List<T> list;
@@ -36,7 +39,12 @@ public ReactiveList(List<T> list) {
     @Override
     public boolean add(T t) {
         boolean result = list.add(t);
-        ReactiveObjectUtil.notifyModification(t, getLeftTuples());
+        ReactiveObjectUtil.notifyModification(t, getLeftTuples(), ModificationType.ADD);
+        if (t instanceof ReactiveObject) {
+            for (Tuple lts : getLeftTuples()) {
+                ((ReactiveObject) t).addLeftTuple(lts);
+            }
+        }
         return result;
     }
 
@@ -72,7 +80,16 @@ public <T1> T1[] toArray(T1[] a) {
 
     @Override
     public boolean remove(Object o) {
-        return list.remove(o);
+        boolean result = list.remove(o);
+        if (result) {
+            if (o instanceof ReactiveObject) {
+                for (Tuple lts : getLeftTuples()) {
+                    ((ReactiveObject) o).removeLeftTuple(lts);
+                }
+            }
+            ReactiveObjectUtil.notifyModification(o, getLeftTuples(), ModificationType.REMOVE);
+        }
+        return result;
     }
 
     @Override

diff --git a/drools-core/src/main/java/org/drools/core/phreak/ReactiveObject.java b/drools-core/src/main/java/org/drools/core/phreak/ReactiveObject.java
@@ -21,5 +21,6 @@
 
 public interface ReactiveObject {
     void addLeftTuple(Tuple leftTuple);
+    void removeLeftTuple(Tuple leftTuple);
     List<Tuple> getLeftTuples();
 }
diff --git a/drools-core/src/main/java/org/drools/core/phreak/ReactiveObjectUtil.java b/drools-core/src/main/java/org/drools/core/phreak/ReactiveObjectUtil.java
@@ -32,12 +32,16 @@
 import static org.drools.core.phreak.PhreakFromNode.*;
 
 public class ReactiveObjectUtil {
+
+    public enum ModificationType {
+        MODIFY, ADD, REMOVE
+    }
 
     public static void notifyModification(ReactiveObject reactiveObject) {
-        notifyModification( reactiveObject, reactiveObject.getLeftTuples() );
+        notifyModification( reactiveObject, reactiveObject.getLeftTuples(), ModificationType.MODIFY);
     }
 
-    public static void notifyModification(Object object, List<Tuple> leftTuples) {
+    public static void notifyModification(Object object, List<Tuple> leftTuples, ModificationType type) {
         if (leftTuples == null) {
             return;
         }
@@ -48,7 +52,7 @@ public static void notifyModification(Object object, List<Tuple> leftTuples) {
             LeftTupleSinkNode sink = node.getSinkPropagator().getFirstLeftTupleSink();
             InternalWorkingMemory wm = getInternalWorkingMemory(propagationContext);
 
-            wm.addPropagation(new ReactivePropagation(object, leftTuple, propagationContext, node, sink));
+            wm.addPropagation(new ReactivePropagation(object, leftTuple, propagationContext, node, sink, type));
         }
     }
 
@@ -64,21 +68,23 @@ static class ReactivePropagation extends PropagationEntry.AbstractPropagationEnt
         private final PropagationContext propagationContext;
         private final ReactiveFromNode node;
         private final LeftTupleSinkNode sink;
+        private final ModificationType type;
 
-        ReactivePropagation( Object object, Tuple leftTuple, PropagationContext propagationContext, ReactiveFromNode node, LeftTupleSinkNode sink ) {
+        ReactivePropagation( Object object, Tuple leftTuple, PropagationContext propagationContext, ReactiveFromNode node, LeftTupleSinkNode sink, ModificationType type ) {
             this.object = object;
             this.leftTuple = leftTuple;
             this.propagationContext = propagationContext;
             this.node = node;
             this.sink = sink;
+            this.type = type;
         }
 
         @Override
         public void execute( InternalWorkingMemory wm ) {
             ReactiveFromNode.ReactiveFromMemory mem = wm.getNodeMemory(node);
             InternalFactHandle factHandle = node.createFactHandle( leftTuple, propagationContext, wm, object );
 
-            if ( isAllowed( factHandle, node.getAlphaConstraints(), wm, mem ) ) {
+            if ( type != ModificationType.REMOVE && isAllowed( factHandle, node.getAlphaConstraints(), wm, mem ) ) {
                 ContextEntry[] context = mem.getBetaMemory().getContext();
                 BetaConstraints betaConstraints = node.getBetaConstraints();
                 betaConstraints.updateFromTuple( context,