Author: mduerig
Date: Thu Oct 24 13:02:00 2013
New Revision: 1535368
URL: http://svn.apache.org/r1535368
Log:
OAK-1114: Clarify NodeBuilder.moveTo() contract
The move succeeds if both, this builder and the new parent exist, there is no
child with the given name at the new parent and the new parent is not in the
subtree of this the source. After a successful move exists() of the moved
builder returns false, otherwise it returns true.
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeBuilder.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/NodeStoreKernel.java
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
jackrabbit/oak/trunk/oak-it/mk/src/main/java/org/apache/jackrabbit/mk/test/MicroKernelIT.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeBuilder.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeBuilder.java?rev=1535368&r1=1535367&r2=1535368&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeBuilder.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeBuilder.java
Thu Oct 24 13:02:00 2013
@@ -66,7 +66,10 @@ public class KernelNodeBuilder extends M
@Override
public boolean moveTo(NodeBuilder newParent, String newName) {
if (newParent instanceof FastCopyMove) {
- return ((FastCopyMove) newParent).moveFrom(this, newName);
+ checkNotNull(newParent);
+ checkNotNull(newName);
+ return !isRoot() && exists() && !newParent.hasChildNode(newName) &&
+ ((FastCopyMove) newParent).moveFrom(this, newName);
} else {
return super.moveTo(newParent, newName);
}
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/NodeStoreKernel.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/NodeStoreKernel.java?rev=1535368&r1=1535367&r2=1535368&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/NodeStoreKernel.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/NodeStoreKernel.java
Thu Oct 24 13:02:00 2013
@@ -35,6 +35,10 @@ import java.util.zip.Checksum;
import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
+import com.google.common.hash.HashCode;
+import com.google.common.hash.Hasher;
+import com.google.common.hash.Hashing;
+import com.google.common.io.ByteStreams;
import org.apache.jackrabbit.mk.api.MicroKernel;
import org.apache.jackrabbit.mk.api.MicroKernelException;
import org.apache.jackrabbit.mk.json.JsopBuilder;
@@ -46,7 +50,6 @@ import org.apache.jackrabbit.oak.api.Typ
import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.plugins.memory.AbstractBlob;
import org.apache.jackrabbit.oak.spi.commit.CommitHook;
-import org.apache.jackrabbit.oak.spi.commit.CommitInfo;
import org.apache.jackrabbit.oak.spi.commit.DefaultValidator;
import org.apache.jackrabbit.oak.spi.commit.EditorHook;
import org.apache.jackrabbit.oak.spi.commit.Validator;
@@ -55,11 +58,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.apache.jackrabbit.oak.spi.state.NodeState;
import org.apache.jackrabbit.oak.spi.state.NodeStore;
-import com.google.common.hash.HashCode;
-import com.google.common.hash.Hasher;
-import com.google.common.hash.Hashing;
-import com.google.common.io.ByteStreams;
-
/**
* This is a simple {@link NodeStore}-based {@link MicroKernel} implementation.
*/
@@ -209,6 +207,10 @@ public class NodeStoreKernel implements
NodeBuilder targetParent =
getNode(builder, getParentPath(targetPath));
String targetName = getName(targetPath);
+ if (path.equals(targetPath) || PathUtils.isAncestor(path,
targetPath)) {
+ throw new MicroKernelException(
+ "Target path must not be the same or a descendant
of the source path: " + targetPath);
+ }
if (targetParent.hasChildNode(targetName)) {
throw new MicroKernelException(
"Target node exists: " + targetPath);
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=1535368&r1=1535367&r2=1535368&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
Thu Oct 24 13:02:00 2013
@@ -158,7 +158,7 @@ public class MemoryNodeBuilder implement
/**
* @return {@code true} iff this is the root builder
*/
- private boolean isRoot() {
+ protected final boolean isRoot() {
return this == rootBuilder;
}
@@ -320,13 +320,15 @@ public class MemoryNodeBuilder implement
@Override
public boolean moveTo(NodeBuilder newParent, String newName) {
- if (isRoot()) {
+ checkNotNull(newParent);
+ checkNotNull(newName);
+ if (isRoot() || !exists() || newParent.hasChildNode(newName)) {
return false;
} else {
- NodeState nodeState = getNodeState();
- remove();
if (newParent.exists()) {
- checkNotNull(newParent).setChildNode(checkNotNull(newName),
nodeState);
+ NodeState nodeState = getNodeState();
+ newParent.setChildNode(newName, nodeState);
+ remove();
return true;
} else {
// Move to descendant
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=1535368&r1=1535367&r2=1535368&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
Thu Oct 24 13:02:00 2013
@@ -208,6 +208,12 @@ public interface NodeBuilder {
/**
* Move this child to a new parent with a new name.
+ * 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}.
+ *
* @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
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=1535368&r1=1535367&r2=1535368&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
Thu Oct 24 13:02:00 2013
@@ -45,7 +45,6 @@ import org.apache.jackrabbit.oak.spi.sta
import org.junit.After;
import org.junit.Assume;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -286,14 +285,13 @@ public class NodeStoreTest {
assertEquals("child-moved", diff.removed.get(0));
}
- @Ignore // FIXME OAK-1114
@Test
public void move() throws CommitFailedException {
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder x = test.getChildNode("x");
NodeBuilder y = test.getChildNode("y");
assertTrue(x.moveTo(y, "xx"));
- assertTrue(x.exists());
+ assertFalse(x.exists());
assertTrue(y.exists());
assertFalse(test.hasChildNode("x"));
assertTrue(y.hasChildNode("xx"));
@@ -301,7 +299,6 @@ public class NodeStoreTest {
@Test
public void moveNonExisting() throws CommitFailedException {
- Assume.assumeTrue(fixture != NodeStoreFixture.SEGMENT_MK); // FIXME
OAK-1114
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder any = test.getChildNode("any");
NodeBuilder y = test.getChildNode("y");
@@ -313,7 +310,6 @@ public class NodeStoreTest {
@Test
public void moveToExisting() throws CommitFailedException {
- Assume.assumeTrue(fixture != NodeStoreFixture.SEGMENT_MK); // FIXME
OAK-1114
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder x = test.getChildNode("x");
assertFalse(x.moveTo(test, "y"));
@@ -322,20 +318,18 @@ public class NodeStoreTest {
assertTrue(test.hasChildNode("y"));
}
- @Ignore // FIXME OAK-1114
@Test
public void rename() throws CommitFailedException {
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder x = test.getChildNode("x");
assertTrue(x.moveTo(test, "xx"));
- assertTrue(x.exists());
+ assertFalse(x.exists());
assertFalse(test.hasChildNode("x"));
assertTrue(test.hasChildNode("xx"));
}
@Test
public void renameNonExisting() throws CommitFailedException {
- Assume.assumeTrue(fixture != NodeStoreFixture.SEGMENT_MK); // FIXME
OAK-1114
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder any = test.getChildNode("any");
assertFalse(any.moveTo(test, "xx"));
@@ -345,7 +339,6 @@ public class NodeStoreTest {
@Test
public void renameToExisting() throws CommitFailedException {
- Assume.assumeTrue(fixture != NodeStoreFixture.SEGMENT_MK); // FIXME
OAK-1114
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder x = test.getChildNode("x");
assertFalse(x.moveTo(test, "y"));
@@ -358,12 +351,11 @@ public class NodeStoreTest {
public void moveToSelf() throws CommitFailedException {
NodeBuilder test = store.getRoot().builder().getChildNode("test");
NodeBuilder x = test.getChildNode("x");
- assertTrue(x.moveTo(test, "x"));
+ assertFalse(x.moveTo(test, "x"));
assertTrue(x.exists());
assertTrue(test.hasChildNode("x"));
}
- @Ignore // FIXME OAK-1114
@Test
public void moveToSelfNonExisting() throws CommitFailedException {
NodeBuilder test = store.getRoot().builder().getChildNode("test");
@@ -374,6 +366,16 @@ public class NodeStoreTest {
}
@Test
+ public void moveToDescendant() throws CommitFailedException {
+ Assume.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"));
+ }
+
+ @Test
public void oak965() throws CommitFailedException {
NodeStore store1 = init(fixture.createNodeStore());
NodeStore store2 = init(fixture.createNodeStore());
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=1535368&r1=1535367&r2=1535368&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
Thu Oct 24 13:02:00 2013
@@ -32,6 +32,7 @@ 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 {
@@ -313,13 +314,14 @@ public class MemoryNodeBuilderTest {
@Test
public void testMoveToSelf() {
NodeBuilder rootBuilder = base.builder();
- assertTrue(rootBuilder.getChildNode("y").moveTo(rootBuilder, "y"));
+ assertFalse(rootBuilder.getChildNode("y").moveTo(rootBuilder, "y"));
NodeState newRoot = rootBuilder.getNodeState();
assertTrue(newRoot.hasChildNode("y"));
}
@Test
+ @Ignore // FIXME OAK-1114
public void testMoveToDescendant() {
NodeBuilder rootBuilder = base.builder();
assertFalse(rootBuilder.getChildNode("x").moveTo(rootBuilder.getChildNode("x"),
"xx"));
Modified:
jackrabbit/oak/trunk/oak-it/mk/src/main/java/org/apache/jackrabbit/mk/test/MicroKernelIT.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-it/mk/src/main/java/org/apache/jackrabbit/mk/test/MicroKernelIT.java?rev=1535368&r1=1535367&r2=1535368&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-it/mk/src/main/java/org/apache/jackrabbit/mk/test/MicroKernelIT.java
(original)
+++
jackrabbit/oak/trunk/oak-it/mk/src/main/java/org/apache/jackrabbit/mk/test/MicroKernelIT.java
Thu Oct 24 13:02:00 2013
@@ -16,6 +16,13 @@
*/
package org.apache.jackrabbit.mk.test;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
import java.io.BufferedInputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -34,13 +41,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
/**
* Integration tests for verifying that a {@code MicroKernel} implementation
* obeys the contract of the {@code MicroKernel} API.
@@ -937,7 +937,7 @@ public class MicroKernelIT extends Abstr
mk.commit("", "+\"/test/sub\":{}", null, "");
try {
// try to move /test to /test/sub/test
- mk.commit("/", "> \"test\": \"/test/sub/test\"", null, "");
+ mk.commit("/", "> \"/test\": \"/test/sub/test\"", null, "");
fail();
} catch (Exception e) {
// expected
