You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by md...@apache.org on 2013/11/22 13:50:45 UTC

svn commit: r1544516 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/plugins/memory/ main/java/org/apache/jackrabbit/oak/spi/state/ test/java/org/apache/jackrabbit/oak/kernel/ test/java/org/apache/jackrabbit/oak/plugins/mem...

Author: mduerig
Date: Fri Nov 22 12:50:44 2013
New Revision: 1544516

URL: http://svn.apache.org/r1544516
Log:
OAK-1114: Clarify NodeBuilder.moveTo() contract
- Adapted NodeBuilder.moveTo contract to cater for edge cases
- Updated test expectations accordingly
- Added Javadoc to MemoryNodeBuilder

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java?rev=1544516&r1=1544515&r2=1544516&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilder.java Fri Nov 22 12:50:44 2013
@@ -323,6 +323,20 @@ public class MemoryNodeBuilder implement
         }
     }
 
+    /**
+     * This implementation has the same semantics as adding this node
+     * with name {@code newName} as a new child of {@code newParent} followed
+     * by removing this node. As a consequence this implementation allows
+     * moving this node into the subtree rooted here, the result of which
+     * is the same as removing this node.
+     * <p>
+     * See also {@link NodeBuilder#moveTo(NodeBuilder, String) the general contract}
+     * for {@code MoveTo}.
+     *
+     * @param newParent  builder for the new parent.
+     * @param newName  name of this child at the new parent
+     * @return  {@code true} on success, {@code false} otherwise
+     */
     @Override
     public boolean moveTo(NodeBuilder newParent, String newName) {
         checkNotNull(newParent);
@@ -339,7 +353,6 @@ public class MemoryNodeBuilder implement
                 remove();
                 return true;
             } else {
-                // Move to descendant
                 return false;
             }
         }

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java?rev=1544516&r1=1544515&r2=1544516&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/NodeBuilder.java Fri Nov 22 12:50:44 2013
@@ -208,12 +208,20 @@ public interface NodeBuilder {
     boolean remove();
 
     /**
-     * Move this child to a new parent with a new name.
+     * Move this child to a new parent with a new name. When the move succeeded this
+     * builder has been moved to {@code newParent} as child {@code newName}. Otherwise neither
+     * this builder nor {@code newParent} are modified.
+     * <p>
      * The move succeeds if both, this builder and {@code newParent} exist, there is no child with
      * {@code newName} at {@code newParent} and {@code newParent} is not in the subtree of this
      * builder.
-     * After a successful move {@code exists()} on this builder returns {@code false}, otherwise
-     * {@code true}.
+     * <p>
+     * The move fails if the this builder or {@code newParent} does not exist or if there is
+     * already a child {@code newName} at {@code newParent}.
+     * <p>
+     * For all remaining cases (e.g. moving a builder into its own subtree) it is left
+     * to the implementation whether the move succeeds or fails as long as the state of the
+     * involved builder stays consistent.
      *
      * @param newParent  builder for the new parent.
      * @param newName  name of this child at the new parent

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java?rev=1544516&r1=1544515&r2=1544516&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/kernel/NodeStoreTest.java Fri Nov 22 12:50:44 2013
@@ -24,7 +24,6 @@ import static org.apache.jackrabbit.oak.
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assume.assumeTrue;
 import static org.junit.runners.Parameterized.Parameters;
 
 import java.util.ArrayList;
@@ -379,12 +378,17 @@ public class NodeStoreTest {
 
     @Test
     public void moveToDescendant() throws CommitFailedException {
-        assumeTrue(fixture != NodeStoreFixture.SEGMENT_MK);  // FIXME OAK-1114
         NodeBuilder test = store.getRoot().builder().getChildNode("test");
         NodeBuilder x = test.getChildNode("x");
-        assertFalse(x.moveTo(x, "xx"));
-        assertTrue(x.exists());
-        assertTrue(test.hasChildNode("x"));
+        if (fixture == NodeStoreFixture.SEGMENT_MK) {
+            assertTrue(x.moveTo(x, "xx"));
+            assertFalse(x.exists());
+            assertFalse(test.hasChildNode("x"));
+        } else {
+            assertFalse(x.moveTo(x, "xx"));
+            assertTrue(x.exists());
+            assertTrue(test.hasChildNode("x"));
+        }
     }
 
     @Test

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java?rev=1544516&r1=1544515&r2=1544516&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/plugins/memory/MemoryNodeBuilderTest.java Fri Nov 22 12:50:44 2013
@@ -32,7 +32,6 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
 import org.apache.jackrabbit.oak.spi.state.NodeState;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 public class MemoryNodeBuilderTest {
@@ -321,10 +320,10 @@ public class MemoryNodeBuilderTest {
     }
 
     @Test
-    @Ignore  // FIXME OAK-1114
     public void testMoveToDescendant() {
         NodeBuilder rootBuilder = base.builder();
-        assertFalse(rootBuilder.getChildNode("x").moveTo(rootBuilder.getChildNode("x"), "xx"));
+        assertTrue(rootBuilder.getChildNode("x").moveTo(rootBuilder.getChildNode("x"), "xx"));
+        assertFalse(rootBuilder.hasChildNode("x"));
     }
 
     @Test