You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by md...@apache.org on 2011/12/31 17:31:29 UTC

svn commit: r1226130 - in /jackrabbit/sandbox/jackrabbit-microkernel/src: main/java/org/apache/jackrabbit/state/ChangeTree.java test/java/org/apache/jackrabbit/state/TransientSpaceTest.java

Author: mduerig
Date: Sat Dec 31 16:31:29 2011
New Revision: 1226130

URL: http://svn.apache.org/viewvc?rev=1226130&view=rev
Log:
Microkernel based prototype of JCR implementation (WIP)
- fixed wrong order of move operations

Modified:
    jackrabbit/sandbox/jackrabbit-microkernel/src/main/java/org/apache/jackrabbit/state/ChangeTree.java
    jackrabbit/sandbox/jackrabbit-microkernel/src/test/java/org/apache/jackrabbit/state/TransientSpaceTest.java

Modified: jackrabbit/sandbox/jackrabbit-microkernel/src/main/java/org/apache/jackrabbit/state/ChangeTree.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/jackrabbit-microkernel/src/main/java/org/apache/jackrabbit/state/ChangeTree.java?rev=1226130&r1=1226129&r2=1226130&view=diff
==============================================================================
--- jackrabbit/sandbox/jackrabbit-microkernel/src/main/java/org/apache/jackrabbit/state/ChangeTree.java (original)
+++ jackrabbit/sandbox/jackrabbit-microkernel/src/main/java/org/apache/jackrabbit/state/ChangeTree.java Sat Dec 31 16:31:29 2011
@@ -30,9 +30,11 @@ import javax.jcr.ItemExistsException;
 import javax.jcr.ItemNotFoundException;
 import javax.jcr.PathNotFoundException;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Set;
 
 import static org.apache.commons.collections.map.ReferenceMap.HARD;
 import static org.apache.commons.collections.map.ReferenceMap.WEAK;
@@ -218,6 +220,7 @@ public class ChangeTree {
     }
 
     private class ChangeVisitor implements Visitor {
+        private final Set<Moved> visited = new HashSet<Moved>();
         private final Listener listener;
 
         public ChangeVisitor(Listener listener) {
@@ -239,15 +242,21 @@ public class ChangeTree {
 
         @Override
         public void visit(Removed delta) {
-            if (!delta.isMoved()) {
+            if (delta.isMoved()) {
+                delta.movedTo.accept(this);
+            }
+            else {
                 listener.removeNode(delta.getPath());
             }
         }
 
         @Override
         public void visit(Moved delta) {
-            delta.source.accept(this);
-            listener.moveNode(delta.getSourcePath(), delta.getPath());
+            if (!visited.contains(delta)) {
+                visited.add(delta);
+                delta.source.accept(this);
+                listener.moveNode(delta.getSourcePath(), delta.getPath());
+            }
         }
 
         private void handleProperties(NodeDelta delta) {
@@ -478,20 +487,28 @@ public class ChangeTree {
                 throw new PathNotFoundException(destParentPath.toJcrPath());
             }
 
-            if (source.isTransient()) {
+            Removed removed;
+            if (source.isTransient()) {  // fixme: can't this go into the moveTo methods?
+                removed = null;
                 removeChild(name);
             }
             else {
-                addChild(new Removed(this, name, true));
+                removed = new Removed(this, name);
+                addChild(removed);
             }
 
             // Resolve destination only *after* source has been removed in order
             // to make sure nodes on any common path prefix are already touched.
             NodeDelta destParent = ChangeTree.this.getNode(destParentPath);
             if (destParent == null) {
+                // fixme: throwing after tree modification leads to inconsistencies. check upfront!
+                // can this even be null here? there seems to be a check above already
                 throw new PathNotFoundException(destination.toJcrPath());
             }
-            source.moveTo(destParent, destination.getName());
+            NodeDelta movedTo = source.moveTo(destParent, destination.getName());
+            if (removed != null) {
+                removed.setMoved(movedTo);
+            }
         }
 
         /**
@@ -515,7 +532,7 @@ public class ChangeTree {
         void touch() { }
 
         abstract NodeDelta remove() throws ItemNotFoundException;
-        abstract void moveTo(NodeDelta parent, String name);
+        abstract NodeDelta moveTo(NodeDelta parent, String name);
 
         NodeDelta addChild(NodeDelta delta) {
             childNodes.put(delta.name, delta);
@@ -623,12 +640,12 @@ public class ChangeTree {
                     child.remove();
                 }
             }
-            return parent.addChild(new Removed(parent, name, false));
+            return parent.addChild(new Removed(parent, name));
         }
 
         @Override
-        void moveTo(NodeDelta parent, String name) {
-            parent.addChild(new Moved(parent, name, this));
+        NodeDelta moveTo(NodeDelta parent, String name) {
+            return parent.addChild(new Moved(parent, name, this));
         }
     }
 
@@ -676,9 +693,9 @@ public class ChangeTree {
         }
 
         @Override
-        void moveTo(NodeDelta parent, String name) {
+        NodeDelta moveTo(NodeDelta parent, String name) {
             this.name = name;
-            parent.addChild(this);
+            return parent.addChild(this);
         }
     }
 
@@ -778,8 +795,9 @@ public class ChangeTree {
         }
 
         @Override
-        void moveTo(NodeDelta parent, String name) {
-            parent.addChild(new Moved(parent, name, source));
+        NodeDelta moveTo(NodeDelta parent, String name) {
+            this.name = name;
+            return parent.addChild(this);
         }
     }
 
@@ -787,11 +805,14 @@ public class ChangeTree {
      * Represents a transiently removed node.
      */
     public class Removed extends NodeDelta {
-        private final boolean moved;
+        private NodeDelta movedTo;
 
-        Removed(NodeDelta parent, String name, boolean moved) {
+        Removed(NodeDelta parent, String name) {
             super(parent, name);
-            this.moved = moved;
+        }
+
+        public void setMoved(NodeDelta movedTo) {
+            this.movedTo = movedTo;
         }
 
         @Override
@@ -801,7 +822,7 @@ public class ChangeTree {
 
         @Override
         public boolean isMoved() {
-            return moved;
+            return movedTo != null;
         }
 
         @Override
@@ -845,7 +866,7 @@ public class ChangeTree {
         }
 
         @Override
-        void moveTo(NodeDelta parent, String name) {
+        NodeDelta moveTo(NodeDelta parent, String name) {
             throw new IllegalStateException("Removed");
         }
 

Modified: jackrabbit/sandbox/jackrabbit-microkernel/src/test/java/org/apache/jackrabbit/state/TransientSpaceTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/sandbox/jackrabbit-microkernel/src/test/java/org/apache/jackrabbit/state/TransientSpaceTest.java?rev=1226130&r1=1226129&r2=1226130&view=diff
==============================================================================
--- jackrabbit/sandbox/jackrabbit-microkernel/src/test/java/org/apache/jackrabbit/state/TransientSpaceTest.java (original)
+++ jackrabbit/sandbox/jackrabbit-microkernel/src/test/java/org/apache/jackrabbit/state/TransientSpaceTest.java Sat Dec 31 16:31:29 2011
@@ -449,7 +449,7 @@ public class TransientSpaceTest {
         root.addNode("target2");
         transientSpace.save();
 
-        NodeDelta sourceNode = transientSpace.getNode(ROOT.concat("source"));
+        NodeDelta sourceNode = root.getNode("source");
         sourceNode.moveNode("node", ROOT.concat("target1/moved1"));
         NodeDelta target1 = root.getNode("target1");
         target1.moveNode("moved1", ROOT.concat("target2/moved2"));
@@ -469,6 +469,34 @@ public class TransientSpaceTest {
     }
 
     @Test
+    public void moveMovedIndirect() throws RepositoryException {
+        NodeDelta root = transientSpace.getNode(ROOT);
+        root.addNode("source").addNode("node");
+        root.addNode("target1");
+        root.addNode("target2");
+        transientSpace.save();
+
+        NodeDelta sourceNode = root.getNode("source");
+        NodeDelta target1 = root.getNode("target1");
+        target1.addNode("child");
+        sourceNode.moveNode("node", ROOT.concat("target1/child/node-moved"));
+        target1.moveNode("child", ROOT.concat("target2/child-moved"));
+
+        assertNull(transientSpace.getNode(ROOT.concat("source/node")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("source")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("target1")));
+        assertNull(transientSpace.getNode(ROOT.concat("target1/child")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("target2/child-moved/node-moved")));
+        transientSpace.save();
+
+        assertNull(transientSpace.getNode(ROOT.concat("source/node")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("source")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("target1")));
+        assertNull(transientSpace.getNode(ROOT.concat("target1/child")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("target2/child-moved/node-moved")));
+    }
+
+    @Test
     public void moveMovedThenDelete() throws RepositoryException {
         NodeDelta root = transientSpace.getNode(ROOT);
         root.addNode("source").addNode("node");
@@ -660,6 +688,29 @@ public class TransientSpaceTest {
     }
 
     @Test
+    public void moveMove() throws RepositoryException {
+        NodeDelta root = transientSpace.getNode(ROOT);
+        root.addNode("a").addNode("b");
+        root.addNode("t1");
+        transientSpace.save();
+
+        root.moveNode("a", ROOT.concat("t1/am"));
+        root.getNode("t1").getNode("am").moveNode("b", ROOT.concat("bm"));
+        
+        assertNull(transientSpace.getNode(ROOT.concat("a")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("t1/am")));
+        assertNull(transientSpace.getNode(ROOT.concat("t1/am/b")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("bm")));
+
+        transientSpace.save();
+
+        assertNull(transientSpace.getNode(ROOT.concat("a")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("t1/am")));
+        assertNull(transientSpace.getNode(ROOT.concat("t1/am/b")));
+        assertNotNull(transientSpace.getNode(ROOT.concat("bm")));
+    }
+
+    @Test
     public void moveOutOfMoveTransient() throws RepositoryException {
         NodeDelta root = transientSpace.getNode(ROOT);
         root.addNode("source").addNode("node1").addNode("child");