jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From mreut...@apache.org
Subject svn commit: r1433308 - in /jackrabbit/oak/trunk/oak-mongomk/src: main/java/org/apache/jackrabbit/mongomk/impl/command/ main/java/org/apache/jackrabbit/mongomk/impl/instruction/ main/java/org/apache/jackrabbit/mongomk/impl/model/ test/java/org/apache/ja...
Date Tue, 15 Jan 2013 08:12:47 GMT
Author: mreutegg
Date: Tue Jan 15 08:12:47 2013
New Revision: 1433308

URL: http://svn.apache.org/viewvc?rev=1433308&view=rev
Log:
OAK-535: MergeCommand reads complete tree into memory

Modified:
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
    jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java
    jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java
    jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java?rev=1433308&r1=1433307&r2=1433308&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java
Tue Jan 15 08:12:47 2013
@@ -65,7 +65,10 @@ public class NodeExistsCommand extends B
 
         Map<String, MongoNode> pathAndNodeMap = action.execute();
         node = pathAndNodeMap.get(this.path);
-        return node != null && !node.isDeleted();
+        if (node != null && node.isDeleted()) {
+            node = null;
+        }
+        return node != null;
     }
 
     /**

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java?rev=1433308&r1=1433307&r2=1433308&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java
Tue Jan 15 08:12:47 2013
@@ -16,6 +16,7 @@
  */
 package org.apache.jackrabbit.mongomk.impl.instruction;
 
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
@@ -28,7 +29,6 @@ import org.apache.jackrabbit.mongomk.api
 import org.apache.jackrabbit.mongomk.api.instruction.Instruction.SetPropertyInstruction;
 import org.apache.jackrabbit.mongomk.api.instruction.InstructionVisitor;
 import org.apache.jackrabbit.mongomk.impl.MongoNodeStore;
-import org.apache.jackrabbit.mongomk.impl.action.FetchNodesActionNew;
 import org.apache.jackrabbit.mongomk.impl.command.NodeExistsCommand;
 import org.apache.jackrabbit.mongomk.impl.exception.NotFoundException;
 import org.apache.jackrabbit.mongomk.impl.model.MongoCommit;
@@ -45,7 +45,6 @@ public class CommitCommandInstructionVis
     private final long baseRevisionId;
     private final MongoNodeStore nodeStore;
     private final Map<String, MongoNode> pathNodeMap;
-    private final List<MongoCommit> validCommits;
 
     private String branchId;
 
@@ -54,14 +53,12 @@ public class CommitCommandInstructionVis
      *
      * @param nodeStore Node store.
      * @param baseRevisionId the revision this commit is based on
-     * @param validCommits
      */
     public CommitCommandInstructionVisitor(MongoNodeStore nodeStore,
                                            long baseRevisionId,
                                            List<MongoCommit> validCommits) {
         this.nodeStore = nodeStore;
         this.baseRevisionId = baseRevisionId;
-        this.validCommits = validCommits;
         pathNodeMap = new HashMap<String, MongoNode>();
     }
 
@@ -157,29 +154,7 @@ public class CommitCommandInstructionVis
             throw new RuntimeException("Node already exists at copy destination path: " +
destPath);
         }
 
-        // First, copy existing nodes.
-        Map<String, MongoNode> nodesToCopy = fetchNodes(srcPath);
-        for (MongoNode srcNode : nodesToCopy.values()) {
-            String srcNodePath = srcNode.getPath();
-            MongoNode stagedSrcNode = pathNodeMap.get(srcNodePath);
-            if (stagedSrcNode != null && stagedSrcNode.isDeleted()) {
-                // Skip nodes that are staged to be deleted.
-                continue;
-            }
-            String destNodePath = PathUtils.concat(destPath, PathUtils.relativize(srcPath,
srcNodePath));
-            MongoNode destNode = srcNode.copy();
-            destNode.setPath(destNodePath);
-            destNode.removeField("_id");
-            if (stagedSrcNode != null) {
-                copyStagedChanges(stagedSrcNode, destNode, false);
-            }
-            pathNodeMap.put(destNodePath, destNode);
-        }
-
-        // Then, copy any staged changes.
-        MongoNode srcNode = getStagedNode(srcPath);
-        MongoNode destNode = getStagedNode(destPath);
-        copyStagedChanges(srcNode, destNode, false);
+        copy(getStoredNode(srcPath, false), destPath);
 
         // Finally, add to destParent.
         destParent.addChild(destNodeName);
@@ -189,61 +164,17 @@ public class CommitCommandInstructionVis
     public void visit(MoveNodeInstruction instruction) {
         String srcPath = instruction.getSourcePath();
         String destPath = instruction.getDestPath();
-        if (PathUtils.isAncestor(srcPath, destPath)) {
-            throw new RuntimeException("Target path cannot be descendant of source path:
"
-                    + destPath);
-        }
-
-        String srcParentPath = PathUtils.getParentPath(srcPath);
-        String srcNodeName = PathUtils.getName(srcPath);
-
-        String destParentPath = PathUtils.getParentPath(destPath);
-        String destNodeName = PathUtils.getName(destPath);
 
-        MongoNode srcParent = getStoredNode(srcParentPath);
-        if (!srcParent.childExists(srcNodeName)) {
-            throw new NotFoundException(srcPath);
-        }
-
-        MongoNode destParent = getStoredNode(destParentPath);
-        if (destParent.childExists(destNodeName)) {
-            throw new RuntimeException("Node already exists at move destination path: " +
destPath);
-        }
-
-        // First, copy existing nodes.
-        Map<String, MongoNode> nodesToCopy = fetchNodes(srcPath);
-        for (MongoNode srcNode : nodesToCopy.values()) {
-            String srcNodePath = srcNode.getPath();
-            MongoNode stagedSrcNode = pathNodeMap.get(srcNodePath);
-            if (stagedSrcNode != null && stagedSrcNode.isDeleted()) {
-                // Skip nodes that are staged to be deleted.
-                continue;
-            }
-            String destNodePath = PathUtils.concat(destPath, PathUtils.relativize(srcPath,
srcNodePath));
-            MongoNode destNode = srcNode.copy();
-            destNode.setPath(destNodePath);
-            destNode.removeField("_id");
-            if (stagedSrcNode != null) {
-                copyStagedChanges(stagedSrcNode, destNode, true);
-            }
-            pathNodeMap.put(destNodePath, destNode);
+        if (destPath.startsWith(srcPath + "/")) {
+            throw new RuntimeException("Cannot move " + srcPath + " to " + destPath);
         }
 
-        // Then, copy any staged changes.
-        MongoNode srcNode = getStagedNode(srcPath);
-        MongoNode destNode = getStagedNode(destPath);
-        copyStagedChanges(srcNode, destNode, true);
-
-        // Finally, add to destParent and remove from srcParent.
-        destParent.addChild(destNodeName);
-        srcParent.removeChild(srcNodeName);
-
-        if (!srcParent.hasPendingChanges()) {
-            pathNodeMap.remove(srcPath);
-            pathNodeMap.remove(srcParentPath);
-        } else {
-            markAsDeleted(srcPath);
-        }
+        // copy source to destination
+        visit(new CopyNodeInstructionImpl(instruction.getPath(),
+                srcPath, instruction.getDestPath()));
+        // delete source tree
+        visit(new RemoveNodeInstructionImpl(PathUtils.getParentPath(srcPath),
+                PathUtils.getName(srcPath)));
     }
 
     private void checkAbsolutePath(String srcPath) {
@@ -292,36 +223,41 @@ public class CommitCommandInstructionVis
         return node;
     }
 
-    private void copyStagedChanges(MongoNode srcNode, MongoNode destNode, boolean move) {
-        copyAddedNodes(srcNode, destNode, move);
-        copyRemovedNodes(srcNode, destNode);
+    /**
+     * Recursively copies nodes from <code>srcNode</code> to
+     * <code>destPath</code>. This method takes existing nodes as well as
+     * staged nodes into account.
+     *
+     * @param srcNode the source node.
+     */
+    private void copy(MongoNode srcNode, String destPath) {
+        MongoNode destNode = srcNode.copy();
+        destNode.setPath(destPath);
+        destNode.removeField("_id");
         copyAddedProperties(srcNode, destNode);
         copyRemovedProperties(srcNode, destNode);
-    }
+        pathNodeMap.put(destPath, destNode);
 
-    private void copyAddedNodes(MongoNode srcNode, MongoNode destNode, boolean move) {
-        List<String> addedChildren = srcNode.getAddedChildren();
-        if (addedChildren == null || addedChildren.isEmpty()) {
-            return;
-        }
-
-        for (String childName : addedChildren) {
-            getStagedNode(PathUtils.concat(destNode.getPath(), childName));
-            destNode.addChild(childName);
-            if (move) {
-                pathNodeMap.remove(PathUtils.concat(srcNode.getPath(), childName));
+        List<String> children = new ArrayList<String>();
+        if (srcNode.getChildren() != null) {
+            children.addAll(srcNode.getChildren());
+        }
+        if (srcNode.getRemovedChildren() != null) {
+            for (String child : srcNode.getRemovedChildren()) {
+                destNode.removeChild(child);
+                children.remove(child);
             }
         }
-    }
-
-    private void copyRemovedNodes(MongoNode srcNode, MongoNode destNode) {
-        List<String> removedChildren = srcNode.getRemovedChildren();
-        if (removedChildren == null || removedChildren.isEmpty()) {
-            return;
+        if (srcNode.getAddedChildren() != null) {
+            for (String child : srcNode.getAddedChildren()) {
+                destNode.addChild(child);
+                children.add(child);
+            }
         }
-
-        for (String child : removedChildren) {
-            destNode.removeChild(child);
+        for (String child : children) {
+            String srcChildPath = PathUtils.concat(srcNode.getPath(), child);
+            String destChildPath = PathUtils.concat(destPath, child);
+            copy(getStoredNode(srcChildPath, false), destChildPath);
         }
     }
 
@@ -348,19 +284,18 @@ public class CommitCommandInstructionVis
     }
 
     private void markAsDeleted(String path) {
-        // Mark the path and all the children path with deleted flag.
-        Map<String, MongoNode> nodes = fetchNodes(path);
-        for (MongoNode nodeMongo : nodes.values()) {
-            nodeMongo.setDeleted();
-            nodeMongo.removeField("_id");
-            pathNodeMap.put(nodeMongo.getPath(), nodeMongo);
+        MongoNode node = getStoredNode(path);
+        node.setDeleted(true);
+        List<String> children = new ArrayList<String>();
+        if (node.getChildren() != null) {
+            children.addAll(node.getChildren());
         }
-    }
-
-    private Map<String, MongoNode> fetchNodes(String path) {
-        FetchNodesActionNew action = new FetchNodesActionNew(nodeStore, path,
-                FetchNodesActionNew.LIMITLESS_DEPTH, baseRevisionId);
-        action.setBranchId(branchId);
-        return action.execute();
+        if (node.getAddedChildren() != null) {
+            children.addAll(node.getAddedChildren());
+        }
+        for (String child : children) {
+            markAsDeleted(PathUtils.concat(path, child));
+        }
+        pathNodeMap.put(path, node);
     }
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java?rev=1433308&r1=1433307&r2=1433308&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java
Tue Jan 15 08:12:47 2013
@@ -106,8 +106,12 @@ public class MongoNode extends BasicDBOb
         return getBoolean(KEY_DELETED);
     }
 
-    public void setDeleted() {
-        put(KEY_DELETED, Boolean.TRUE);
+    public void setDeleted(boolean deleted) {
+        if (deleted) {
+            put(KEY_DELETED, Boolean.TRUE);
+        } else {
+            remove(KEY_DELETED);
+        }
     }
 
     public String getPath() {
@@ -154,8 +158,7 @@ public class MongoNode extends BasicDBOb
     //--------------------------------------------------------------------------
 
     public void addChild(String childName) {
-        if (removedChildren != null && removedChildren.contains(childName)) {
-            removedChildren.remove(childName);
+        if (removedChildren != null && removedChildren.remove(childName)) {
             return;
         }
 
@@ -173,8 +176,7 @@ public class MongoNode extends BasicDBOb
     }
 
     public void removeChild(String childName) {
-        if (addedChildren != null && addedChildren.contains(childName)) {
-            addedChildren.remove(childName);
+        if (addedChildren != null && addedChildren.remove(childName)) {
             return;
         }
 

Modified: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java?rev=1433308&r1=1433307&r2=1433308&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java
Tue Jan 15 08:12:47 2013
@@ -277,7 +277,6 @@ public class MongoMKBranchMergeTest exte
     }
 
     @Test
-    @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change.
     public void addExistingRootInBranch() {
         addNodes(null, "/root");
         assertNodesExist(null, "/root");
@@ -290,7 +289,6 @@ public class MongoMKBranchMergeTest exte
     }
 
     @Test
-    @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change.
     public void addExistingChildInBranch() {
         addNodes(null, "/root", "/root/child1");
         assertNodesExist(null, "/root", "/root/child1");
@@ -306,6 +304,7 @@ public class MongoMKBranchMergeTest exte
     }
 
     @Test
+    @Ignore("Implementation specific behavior")
     public void emptyMergeCausesNoChange() {
         String rev1 = mk.commit("", "+\"/child1\":{}", null, "");
 
@@ -329,6 +328,16 @@ public class MongoMKBranchMergeTest exte
         } catch (Exception expected) {}
     }
 
+    @Test
+    public void movesInBranch() {
+        String rev = mk.commit("/", "+\"a\":{\"b\":{}}", null, null);
+        String branchRev = mk.branch(rev);
+        branchRev = mk.commit("/", ">\"a\":\"x\"^\"x/b/p\":1>\"x\":\"a\"", branchRev,
null);
+        rev = mk.merge(branchRev, null);
+        assertNodesExist(rev, "/a", "/a/b");
+        assertPropExists(rev, "/a/b", "p");
+    }
+
     private String addNodes(String rev, String...nodes) {
         for (String node : nodes) {
             rev = mk.commit("", "+\"" + node + "\":{}", rev, "");

Modified: jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java?rev=1433308&r1=1433307&r2=1433308&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java
(original)
+++ jackrabbit/oak/trunk/oak-mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java
Tue Jan 15 08:12:47 2013
@@ -256,7 +256,6 @@ public class MongoMKCommitMoveTest exten
     }
 
     @Test
-    @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change.
     public void modifyParentAddPropertyAndMove() {
         mk.commit("/", "+\"a\":{}", null, null);
         mk.commit("/", "+\"b\" : {}\n"
@@ -303,7 +302,6 @@ public class MongoMKCommitMoveTest exten
     }
 
     @Test
-    @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change.
     public void modifyParentRemovePropertyAndMove() {
         mk.commit("/", "+\"a\":{ \"key1\" : \"value1\"}", null, null);
         mk.commit("/", "+\"b\" : {}\n"
@@ -318,4 +316,26 @@ public class MongoMKCommitMoveTest exten
         JSONObject obj = parseJSONObject(nodes);
         assertPropertyNotExists(obj, "c/key1");
     }
+
+    @Test
+    public void moveAndMoveBack() {
+        mk.commit("/", "+\"a\":{}", null, null);
+        mk.commit("/", ">\"a\":\"x\">\"x\":\"a\"", null, null);
+        assertNodesExist(null, "/a");
+    }
+
+    @Test
+    public void moveAndMoveBackWithChildren() {
+        mk.commit("/", "+\"a\":{\"b\":{}}", null, null);
+        mk.commit("/", ">\"a\":\"x\">\"x\":\"a\"", null, null);
+        assertNodesExist(null, "/a", "/a/b");
+    }
+
+    @Test
+    public void moveAndMoveBackWithSetProperties() {
+        mk.commit("/", "+\"a\":{\"b\":{}}", null, null);
+        mk.commit("/", ">\"a\":\"x\"^\"x/p\":1>\"x\":\"a\"", null, null);
+        assertNodesExist(null, "/a", "/a/b");
+        assertPropExists(null, "/a", "p");
+    }
 }
\ No newline at end of file



Mime
View raw message