I added that addToMap flag at some point to get Copy/Move nodes tests pass but with your recent changes, it didn't make a difference (i.e. Copy/Move nodes test pass with & without it) so I thought it's not needed but I didn't think about the source tree getting committed unnecessarily with a new revision id, so I think you're right. I'll add it back.
-Mete On 1/15/13 9:37 AM, "Marcel Reutegger" <[email protected]> wrote: >Hi, > >doesn't this also change behavior? getStoredNode() was used with both >true and false before. now the nodes are put into the map unconditionally. >doesn¹t that mean on copy also the source tree is put into the set of >nodes to commit? > >regards > marcel > >> -----Original Message----- >> From: [email protected] [mailto:[email protected]] >> Sent: Dienstag, 15. Januar 2013 09:27 >> To: [email protected] >> Subject: svn commit: r1433312 - in >>/jackrabbit/oak/trunk/oak-mongomk/src: >> main/java/org/apache/jackrabbit/mongomk/impl/instruction/ >> main/java/org/apache/jackrabbit/mongomk/impl/model/ >> test/java/org/apache/jackrabbit/mongomk/impl/ >> >> Author: meteatamel >> Date: Tue Jan 15 08:27:17 2013 >> New Revision: 1433312 >> >> URL: http://svn.apache.org/viewvc?rev=1433312&view=rev >> Log: >> Minor cleanup of unused code after recent changes >> >> Modified: >> 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/Mo >> ngoNode.java >> jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKB >> ranchTest.java >> jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitCopyTest.java >> jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitMoveTest.java >> >> 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=1433312&r1=1433311&r2=143 >> 3312&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:27:17 2013 >> @@ -145,7 +145,7 @@ public class CommitCommandInstructionVis >> String destParentPath = PathUtils.getParentPath(destPath); >> String destNodeName = PathUtils.getName(destPath); >> >> - MongoNode srcParent = getStoredNode(srcParentPath, false); >> + MongoNode srcParent = getStoredNode(srcParentPath); >> if (!srcParent.childExists(srcNodeName)) { >> throw new NotFoundException(srcPath); >> } >> @@ -154,7 +154,7 @@ public class CommitCommandInstructionVis >> throw new RuntimeException("Node already exists at copy >> destination path: " + destPath); >> } >> >> - copy(getStoredNode(srcPath, false), destPath); >> + copy(getStoredNode(srcPath), destPath); >> >> // Finally, add to destParent. >> destParent.addChild(destNodeName); >> @@ -194,10 +194,6 @@ public class CommitCommandInstructionVis >> } >> >> private MongoNode getStoredNode(String path) { >> - return getStoredNode(path, true); >> - } >> - >> - private MongoNode getStoredNode(String path, boolean addToMap) { >> MongoNode node = pathNodeMap.get(path); >> if (node != null) { >> return node; >> @@ -217,9 +213,8 @@ public class CommitCommandInstructionVis >> } >> node = existCommand.getNode(); >> node.removeField("_id"); >> - if (addToMap) { >> - pathNodeMap.put(path, node); >> - } >> + pathNodeMap.put(path, node); >> + >> return node; >> } >> >> @@ -257,7 +252,7 @@ public class CommitCommandInstructionVis >> for (String child : children) { >> String srcChildPath = PathUtils.concat(srcNode.getPath(), >>child); >> String destChildPath = PathUtils.concat(destPath, child); >> - copy(getStoredNode(srcChildPath, false), destChildPath); >> + copy(getStoredNode(srcChildPath), destChildPath); >> } >> } >> >> >> Modified: jackrabbit/oak/trunk/oak- >> mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/Mo >> ngoNode.java >> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak- >> mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/Mo >> ngoNode.java?rev=1433312&r1=1433311&r2=1433312&view=diff >> ========================================================== >> ==================== >> --- jackrabbit/oak/trunk/oak- >> mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/Mo >> ngoNode.java (original) >> +++ jackrabbit/oak/trunk/oak- >> mongomk/src/main/java/org/apache/jackrabbit/mongomk/impl/model/Mo >> ngoNode.java Tue Jan 15 08:27:17 2013 >> @@ -265,20 +265,4 @@ public class MongoNode extends BasicDBOb >> sb.append(" }"); >> return sb.toString(); >> } >> - >> - public boolean hasPendingChanges() { >> - if (addedChildren != null && !addedChildren.isEmpty()) { >> - return true; >> - } >> - if (removedChildren != null && !removedChildren.isEmpty()) { >> - return true; >> - } >> - if (addedProps != null && !addedProps.isEmpty()) { >> - return true; >> - } >> - if (removedProps != null && !removedProps.isEmpty()) { >> - return true; >> - } >> - return false; >> - } >> } >> >> Modified: jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKB >> ranchTest.java >> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKB >> ranchTest.java?rev=1433312&r1=1433311&r2=1433312&view=diff >> ========================================================== >> ==================== >> --- jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKB >> ranchTest.java (original) >> +++ jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKB >> ranchTest.java Tue Jan 15 08:27:17 2013 >> @@ -50,7 +50,7 @@ public class MongoMKBranchTest extends B >> String rev1 = mk.commit("", "+\"/child1\":{}", null, ""); >> >> String branchRev1 = mk.branch(rev1); >> - String branchRev11 = mk.commit("/child1", "^\"foo\":1", >>branchRev1, >> ""); >> + mk.commit("/child1", "^\"foo\":1", branchRev1, ""); >> >> String rev2 = mk.commit("", "+\"/child2\":{}", null, ""); >> >> >> Modified: jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitCopyTest.java >> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitCopyTest.java?rev=1433312&r1=1433311&r2=1433312&view=diff >> ========================================================== >> ==================== >> --- jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitCopyTest.java (original) >> +++ jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitCopyTest.java Tue Jan 15 08:27:17 2013 >> @@ -5,7 +5,6 @@ import static org.junit.Assert.assertTru >> import static org.junit.Assert.fail; >> >> import org.apache.jackrabbit.mongomk.BaseMongoMicroKernelTest; >> -import org.apache.jackrabbit.mongomk.impl.model.MongoNode; >> import org.json.simple.JSONObject; >> import org.junit.Ignore; >> import org.junit.Test; >> >> Modified: jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitMoveTest.java >> URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitMoveTest.java?rev=1433312&r1=1433311&r2=1433312&view=diff >> ========================================================== >> ==================== >> --- jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitMoveTest.java (original) >> +++ jackrabbit/oak/trunk/oak- >> mongomk/src/test/java/org/apache/jackrabbit/mongomk/impl/MongoMKC >> ommitMoveTest.java Tue Jan 15 08:27:17 2013 >> @@ -6,7 +6,6 @@ import static org.junit.Assert.fail; >> >> import org.apache.jackrabbit.mongomk.BaseMongoMicroKernelTest; >> import org.json.simple.JSONObject; >> -import org.junit.Ignore; >> import org.junit.Test; >> >> /** >> @@ -332,6 +331,13 @@ public class MongoMKCommitMoveTest exten >> } >> >> @Test >> + public void moveAndMoveBackWithAddedChildren() { >> + mk.commit("/", "+\"a\":{\"b\":{}}", null, null); >> + mk.commit("/", ">\"a\":\"x\"+\"x/c\":{}>\"x\":\"a\"", null, >>null); >> + assertNodesExist(null, "/a", "/a/b", "/a/c"); >> + } >> + >> + @Test >> public void moveAndMoveBackWithSetProperties() { >> mk.commit("/", "+\"a\":{\"b\":{}}", null, null); >> mk.commit("/", ">\"a\":\"x\"^\"x/p\":1>\"x\":\"a\"", null, >>null); >> >
