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 2012/01/02 13:30:51 UTC

svn commit: r1226401 - 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: Mon Jan  2 12:30:50 2012
New Revision: 1226401

URL: http://svn.apache.org/viewvc?rev=1226401&view=rev
Log:
Microkernel based prototype of JCR implementation (WIP)
- refactor Moved operation such that order of generated jsop statement is more coherent with actual order of operations
- fixed concurrent modification exception

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=1226401&r1=1226400&r2=1226401&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 Mon Jan  2 12:30:50 2012
@@ -33,7 +33,6 @@ import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
-import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
@@ -222,7 +221,7 @@ public class ChangeTree {
     }
 
     private class ChangeVisitor implements Visitor {
-        private final Set<Moved> visited = new HashSet<Moved>();
+        private final Set<NodeDelta> visited = new HashSet<NodeDelta>();
         private final Listener listener;
 
         public ChangeVisitor(Listener listener) {
@@ -231,33 +230,54 @@ public class ChangeTree {
 
         @Override
         public void visit(Existing delta) {
-            handleProperties(delta);
-            visitNodes(delta);
+            if (!visited.contains(delta)) {
+                visited.add(delta);
+                handleProperties(delta);
+                visitNodes(delta);
+            }
         }
 
         @Override
         public void visit(Added delta) {
-            listener.addNode(delta.getPath());
-            handleProperties(delta);
-            visitNodes(delta);
+            if (!visited.contains(delta)) {
+                listener.addNode(delta.getPath());
+                visited.add(delta);
+                handleProperties(delta);
+                visitNodes(delta);
+            }
         }
 
+        private void visitParent(NodeDelta delta) {
+            if (!visited.contains(delta.parent)) {
+                visitParent(delta.parent);
+                delta.parent.accept(this);
+            }
+        }
+        
         @Override
         public void visit(Removed delta) {
-            if (delta.isMoved()) {
-                delta.movedTo.accept(this);
-            }
-            else {
-                listener.removeNode(delta.getPath());
+            if (!visited.contains(delta)) {
+                if (delta.isMoved()) {
+                    // Visit parents of target before visiting target
+                    visitParent(delta.movedTo);
+                    visited.add(delta);
+                    delta.movedTo.accept(this);
+                }
+                else {
+                    listener.removeNode(delta.getPath());
+                    visited.add(delta);
+                }
             }
         }
 
         @Override
         public void visit(Moved delta) {
             if (!visited.contains(delta)) {
-                visited.add(delta);
                 delta.source.accept(this);
                 listener.moveNode(delta.getSourcePath(), delta.getPath());
+                visited.add(delta);
+                handleProperties(delta);
+                visitNodes(delta);
             }
         }
 
@@ -276,18 +296,7 @@ public class ChangeTree {
         }
 
         private void visitNodes(NodeDelta delta) {
-            List<NodeDelta> moved = new ArrayList<NodeDelta>();
-            // visit move operations after add and remove operations in order to make
-            // sure transiently added move targets are in place
             for (NodeDelta node : delta.childNodes()) {
-                if (node.isMoved()) {
-                    moved.add(node);
-                }
-                else {
-                    node.accept(this);
-                }
-            }
-            for (NodeDelta node : moved) {
                 node.accept(this);
             }
         }
@@ -526,13 +535,6 @@ public class ChangeTree {
         abstract NodeDelta remove() throws ItemNotFoundException;
         abstract void moveTo(Path parentPath, String name);
 
-        NodeDelta addChild(NodeDelta delta) {
-            childNodes.put(delta.name, delta);
-            delta.parent = this;
-            touch();
-            return delta;
-        }
-
         final void clear() {
             childNodes.clear();
             properties.clear();
@@ -550,6 +552,13 @@ public class ChangeTree {
             return childNodes.containsKey(name);
         }
 
+        final NodeDelta addChild(NodeDelta delta) {
+            childNodes.put(delta.name, delta);
+            delta.parent = this;
+            touch();
+            return delta;
+        }
+
         final NodeDelta removeChild(String name) {
             NodeDelta delta = childNodes.remove(name);
             if (delta != null) {
@@ -626,12 +635,16 @@ public class ChangeTree {
         NodeDelta remove() throws ItemNotFoundException {
             // Propagate remove down the tree in order to convert move operations
             // into this sub-tree to remove operations at the move's source node.
+            ArrayList<NodeDelta> r = new ArrayList<NodeDelta>();
             for (NodeDelta child : childNodes()) {
                 // Ignore removed and added nodes
                 if (!child.isRemoved() && (!child.isTransient() || child.isMoved())) {
-                    child.remove();
+                    r.add(child);
                 }
             }
+            for (NodeDelta child : r) {
+                child.remove();
+            }
             return parent.addChild(new Removed(parent, name, null));
         }
 
@@ -701,7 +714,7 @@ public class ChangeTree {
      * Represents a moved node.
      */
     public class Moved extends NodeDelta {
-        private final NodeDelta source;
+        final NodeDelta source;
 
         Moved(NodeDelta parent, String name, NodeDelta source) {
             super(parent, name);
@@ -747,29 +760,11 @@ public class ChangeTree {
 
         @Override
         public NodeDelta getNode(String name) {
-            return source.getNode(name);
-        }
-
-        @Override
-        public NodeDelta addNode(String name) throws ItemExistsException {
-            return source.addNode(name);
-        }
-
-        @Override
-        public NodeDelta removeNode(String name) throws ItemNotFoundException {
-            return source.removeNode(name);
-        }
-
-        @Override
-        public void moveNode(String name, Path destination) throws ItemNotFoundException, ItemExistsException,
-                PathNotFoundException {
-
-            source.moveNode(name, destination);
-        }
-
-        @Override
-        public void setValue(String name, JsonValue value) {
-            source.setValue(name, value);
+            NodeDelta delta = source.getNode(name);
+            if (delta == null) {
+                delta = getChild(name);
+            }
+            return delta;
         }
 
         @Override
@@ -788,11 +783,6 @@ public class ChangeTree {
         }
 
         @Override
-        NodeDelta addChild(NodeDelta delta) {
-            return source.addChild(delta);
-        }
-
-        @Override
         void moveTo(Path parentPath, String name) {
             parent.removeChild(this.name);
             NodeDelta destParent = ChangeTree.this.getNode(parentPath);
@@ -870,7 +860,7 @@ public class ChangeTree {
         @Override
         public String toString() {
             return "Removed[" + getPath() + ']'
-                    + (isMoved() ? "to " + movedTo : "");
+                    + (isMoved() ? " to " + movedTo : "");
         }
 
     }

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=1226401&r1=1226400&r2=1226401&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 Mon Jan  2 12:30:50 2012
@@ -851,7 +851,7 @@ public class TransientSpaceTest {
         
         root.addNode("target");
         root.moveNode("node", ROOT.concat("target/node-moved"));
-        
+
         assertNull(transientSpace.getNode(ROOT.concat("node")));
         assertNotNull(transientSpace.getNode(ROOT.concat("target/node-moved")));
 
@@ -862,6 +862,67 @@ public class TransientSpaceTest {
     }
 
     @Test
+    public void moveIntoSameBranch() throws RepositoryException {
+        NodeDelta root = transientSpace.getNode(ROOT);
+        root.addNode("a");
+        root.addNode("b");
+        transientSpace.save();
+        
+        root.moveNode("a", ROOT.concat("am"));
+        root.moveNode("b", ROOT.concat("am/bm"));
+
+        assertNull(root.getNode("a"));
+        assertNull(root.getNode("b"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("am/bm")));
+
+        transientSpace.save();
+
+        assertNull(root.getNode("a"));
+        assertNull(root.getNode("b"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("am/bm")));
+    }
+
+    @Test
+    public void moveIntoSameBranchIndirect() throws RepositoryException {
+        NodeDelta root = transientSpace.getNode(ROOT);
+        root.addNode("a");
+        root.addNode("b");
+        transientSpace.save();
+
+        root.moveNode("a", ROOT.concat("am"));
+        root.getNode("am").addNode("c");
+        root.moveNode("b", ROOT.concat("am/c/bm"));
+
+        assertNull(root.getNode("a"));
+        assertNull(root.getNode("b"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("am/c/bm")));
+
+        transientSpace.save();
+
+        assertNull(root.getNode("a"));
+        assertNull(root.getNode("b"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("am/c/bm")));
+    }
+
+    @Test
+    public void moveToReverse() throws RepositoryException {
+        NodeDelta root = transientSpace.getNode(ROOT);
+        root.addNode("a").addNode("b");
+        transientSpace.save();
+        
+        root.getNode("a").moveNode("b", ROOT.concat("bm"));
+        root.moveNode("a", ROOT.concat("bm/am"));
+        
+        assertNull(root.getNode("a"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("bm/am")));
+
+        transientSpace.save();
+
+        assertNull(root.getNode("a"));
+        assertNotNull(transientSpace.getNode(ROOT.concat("bm/am")));
+    }
+
+    @Test
     public void setPropertyTest() throws RepositoryException {
         // Set property
         NodeDelta root = transientSpace.getNode(ROOT);
@@ -1020,6 +1081,7 @@ public class TransientSpaceTest {
             @Override
             public void visit(Moved delta) {
                 System.out.println(ident + delta.getName() + ":M+");
+                visitChildren(delta.source);
                 visitChildren(delta);
             }