Index: src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java =================================================================== --- src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java (revision 1432848) +++ src/main/java/org/apache/jackrabbit/mongomk/impl/command/NodeExistsCommand.java (working copy) @@ -65,7 +65,10 @@ Map pathAndNodeMap = action.execute(); node = pathAndNodeMap.get(this.path); - return node != null && !node.isDeleted(); + if (node != null && node.isDeleted()) { + node = null; + } + return node != null; } /** Index: src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java =================================================================== --- src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java (revision 1432848) +++ src/main/java/org/apache/jackrabbit/mongomk/impl/instruction/CommitCommandInstructionVisitor.java (working copy) @@ -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.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 @@ private final long baseRevisionId; private final MongoNodeStore nodeStore; private final Map pathNodeMap; - private final List validCommits; private String branchId; @@ -54,14 +53,12 @@ * * @param nodeStore Node store. * @param baseRevisionId the revision this commit is based on - * @param validCommits */ public CommitCommandInstructionVisitor(MongoNodeStore nodeStore, long baseRevisionId, List validCommits) { this.nodeStore = nodeStore; this.baseRevisionId = baseRevisionId; - this.validCommits = validCommits; pathNodeMap = new HashMap(); } @@ -157,30 +154,8 @@ throw new RuntimeException("Node already exists at copy destination path: " + destPath); } - // First, copy existing nodes. - Map 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); - } + copy(getStoredNode(srcPath, false), destPath); - // Then, copy any staged changes. - MongoNode srcNode = getStagedNode(srcPath); - MongoNode destNode = getStagedNode(destPath); - copyStagedChanges(srcNode, destNode, false); - // Finally, add to destParent. destParent.addChild(destNodeName); } @@ -189,61 +164,17 @@ 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); + if (destPath.startsWith(srcPath + "/")) { + throw new RuntimeException("Cannot move " + srcPath + " to " + destPath); } - MongoNode destParent = getStoredNode(destParentPath); - if (destParent.childExists(destNodeName)) { - throw new RuntimeException("Node already exists at move destination path: " + destPath); - } - - // First, copy existing nodes. - Map 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); - } - - // 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 @@ return node; } - private void copyStagedChanges(MongoNode srcNode, MongoNode destNode, boolean move) { - copyAddedNodes(srcNode, destNode, move); - copyRemovedNodes(srcNode, destNode); + /** + * Recursively copies nodes from srcNode to + * destPath. 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 addedChildren = srcNode.getAddedChildren(); - if (addedChildren == null || addedChildren.isEmpty()) { - return; + List children = new ArrayList(); + if (srcNode.getChildren() != null) { + children.addAll(srcNode.getChildren()); } - - for (String childName : addedChildren) { - getStagedNode(PathUtils.concat(destNode.getPath(), childName)); - destNode.addChild(childName); - if (move) { - pathNodeMap.remove(PathUtils.concat(srcNode.getPath(), childName)); + if (srcNode.getRemovedChildren() != null) { + for (String child : srcNode.getRemovedChildren()) { + destNode.removeChild(child); + children.remove(child); } } - } - - private void copyRemovedNodes(MongoNode srcNode, MongoNode destNode) { - List 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 @@ } private void markAsDeleted(String path) { - // Mark the path and all the children path with deleted flag. - Map 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 children = new ArrayList(); + if (node.getChildren() != null) { + children.addAll(node.getChildren()); } + if (node.getAddedChildren() != null) { + children.addAll(node.getAddedChildren()); + } + for (String child : children) { + markAsDeleted(PathUtils.concat(path, child)); + } + pathNodeMap.put(path, node); } - - private Map fetchNodes(String path) { - FetchNodesActionNew action = new FetchNodesActionNew(nodeStore, path, - FetchNodesActionNew.LIMITLESS_DEPTH, baseRevisionId); - action.setBranchId(branchId); - return action.execute(); - } } \ No newline at end of file Index: src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java =================================================================== --- src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java (revision 1432848) +++ src/main/java/org/apache/jackrabbit/mongomk/impl/model/MongoNode.java (working copy) @@ -106,8 +106,12 @@ 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() { Index: src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java (revision 1432848) +++ src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKBranchMergeTest.java (working copy) @@ -6,7 +6,6 @@ import static org.junit.Assert.fail; import org.apache.jackrabbit.mongomk.BaseMongoMicroKernelTest; -import org.junit.Ignore; import org.junit.Test; /** @@ -277,7 +276,6 @@ } @Test - @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change. public void addExistingRootInBranch() { addNodes(null, "/root"); assertNodesExist(null, "/root"); @@ -290,7 +288,6 @@ } @Test - @Ignore // FIXME - due to CommitCommandInstructionVisitor add node change. public void addExistingChildInBranch() { addNodes(null, "/root", "/root/child1"); assertNodesExist(null, "/root", "/root/child1"); @@ -329,6 +326,16 @@ } 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, ""); Index: src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java =================================================================== --- src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java (revision 1432848) +++ src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKCommitMoveTest.java (working copy) @@ -256,7 +256,6 @@ } @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 @@ } @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 @@ 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