Author: mduerig
Date: Thu Dec 19 16:09:09 2013
New Revision: 1552323
URL: http://svn.apache.org/r1552323
Log:
OAK-1298: Improve MoveDetector to also include moves from transient locations
Correctly track moves of nodes whose parent was moved before
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/plugins/memory/MemoryNodeBuilder.java
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/MoveDetector.java
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/state/MoveDetectorTest.java
jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences.md
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
jackrabbit/oak/trunk/oak-parent/pom.xml
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=1552323&r1=1552322&r2=1552323&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 Dec 19 16:09:09 2013
@@ -75,11 +75,9 @@ public class KernelNodeBuilder extends M
if (newParent instanceof FastCopyMove) {
checkNotNull(newParent);
checkNotNull(newName);
+ annotateSourcePath();
boolean success = !isRoot() && exists() &&
!newParent.hasChildNode(newName) &&
((FastCopyMove) newParent).moveFrom(this, newName);
- if (success) {
- annotateSourcePath(newParent.getChildNode(newName), getPath());
- }
return success;
} else {
return super.moveTo(newParent, newName);
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=1552323&r1=1552322&r2=1552323&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 Dec 19 16:09:09 2013
@@ -29,10 +29,10 @@ import javax.annotation.Nonnull;
import com.google.common.base.Objects;
import com.google.common.io.ByteStreams;
-
import org.apache.jackrabbit.oak.api.Blob;
import org.apache.jackrabbit.oak.api.PropertyState;
import org.apache.jackrabbit.oak.api.Type;
+import org.apache.jackrabbit.oak.commons.PathUtils;
import org.apache.jackrabbit.oak.spi.state.MoveDetector;
import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
import org.apache.jackrabbit.oak.spi.state.NodeState;
@@ -357,9 +357,7 @@ public class MemoryNodeBuilder implement
return false;
} else {
if (newParent.exists()) {
- if (!isNew()) {
- annotateSourcePath(this, getPath());
- }
+ annotateSourcePath();
NodeState nodeState = getNodeState();
newParent.setChildNode(newName, nodeState);
remove();
@@ -370,12 +368,68 @@ public class MemoryNodeBuilder implement
}
}
- protected static void annotateSourcePath(NodeBuilder builder, String path)
{
+ /**
+ * Annotate this builder with its source path if this builder has not
+ * been transiently added. The source path is written to a property with
+ * then name {@link MoveDetector#SOURCE_PATH}.
+ * <p>
+ * The source path of a builder is its current path if its current
+ * source path is empty and all none of its parents has its source
+ * path set. Otherwise it is the source path of the first parent (or self)
+ * that has its source path set appended with the relative path from
+ * that parent to this builder.
+ * <p>
+ * This builder has been transiently added when there exists no a
+ * base node at its source path.
+ */
+ protected final void annotateSourcePath() {
+ String sourcePath = getSourcePath();
+ if (!isTransientlyAdded(sourcePath)) {
+ setProperty(MoveDetector.SOURCE_PATH, sourcePath);
+ }
+ }
+
+ private final String getSourcePath() {
+ // Traverse up the hierarchy until we encounter the first builder
+ // having a source path annotation or until we hit the root
+ MemoryNodeBuilder builder = this;
+ String sourcePath = getSourcePathAnnotation(builder);
+ while (sourcePath == null && builder.parent != null) {
+ builder = builder.parent;
+ sourcePath = getSourcePathAnnotation(builder);
+ }
+
+ if (sourcePath == null) {
+ // Neither self nor any parent has a source path annotation. The
source
+ // path is just the path of this builder
+ return getPath();
+ } else {
+ // The source path is the source path of the first parent having a
source
+ // path annotation with the relative path from this builder up to
that
+ // parent appended.
+ return PathUtils.concat(sourcePath,
+ PathUtils.relativize(builder.getPath(), getPath()));
+ }
+ }
+
+ private static String getSourcePathAnnotation(MemoryNodeBuilder builder) {
PropertyState base =
builder.getBaseState().getProperty(MoveDetector.SOURCE_PATH);
PropertyState head =
builder.getNodeState().getProperty(MoveDetector.SOURCE_PATH);
if (Objects.equal(base, head)) {
- builder.setProperty(MoveDetector.SOURCE_PATH, path);
+ // Both null: no source path annotation
+ // Both non null but equals: source path annotation is from a
previous commit
+ return null;
+ } else {
+ return head.getValue(Type.STRING);
+ }
+ }
+
+ private boolean isTransientlyAdded(String path) {
+ NodeState node = rootBuilder.getBaseState();
+ for (String name : PathUtils.elements(path)) {
+ node = node.getChildNode(name);
}
+ return !node.exists();
}
@Override
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/MoveDetector.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/MoveDetector.java?rev=1552323&r1=1552322&r2=1552323&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/MoveDetector.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/MoveDetector.java
Thu Dec 19 16:09:09 2013
@@ -43,8 +43,10 @@ import org.apache.jackrabbit.oak.spi.com
* target.</li>
* <li>Moving a transiently added node is not reported as a move operation
but as an
* add operation on the move target.</li>
- * <li>Moving a child node of a transiently moved node is not reported as
a move operation
- * but as an add operation on the move target.</li>
+ * <li>Removing a moved node is not reported as a move operation but as a
remove operation
+ * from the source of the move.</li>
+ * <li>Moving a child node of a transiently moved node is not reported
from the original
+ * source node, not from the intermediate one.</li>
* <li>Moving a node back and forth to its original location is not
reported at all.</li>
* </ul>
*/
Modified:
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/state/MoveDetectorTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/state/MoveDetectorTest.java?rev=1552323&r1=1552322&r2=1552323&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/state/MoveDetectorTest.java
(original)
+++
jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/spi/state/MoveDetectorTest.java
Thu Dec 19 16:09:09 2013
@@ -127,6 +127,26 @@ public class MoveDetectorTest {
move(rootBuilder, "/test/z", "/test/y/z");
NodeState moved = move(rootBuilder, "/test/y/z/zz",
"/test/x/zz").getNodeState();
MoveExpectation moveExpectation = new MoveExpectation(
+ ImmutableMap.of("/test/z", "/test/y/z", "/test/z/zz",
"/test/x/zz"));
+ MoveDetector moveDetector = new MoveDetector(moveExpectation);
+ CommitFailedException exception = EditorDiff.process(moveDetector,
root, moved);
+ if (exception != null) {
+ throw exception;
+ }
+ moveExpectation.assertAllFound();
+ }
+
+ /**
+ * Moving a transiently added node from a moved subtree doesn't generate a
move event.
+ * @throws CommitFailedException
+ */
+ @Test
+ public void moveAddedFromMovedSubtree() throws CommitFailedException {
+ NodeBuilder rootBuilder = root.builder();
+
rootBuilder.getChildNode("test").getChildNode("z").setChildNode("added");
+ move(rootBuilder, "/test/z", "/test/y/z");
+ NodeState moved = move(rootBuilder, "/test/y/z/added",
"/test/x/added").getNodeState();
+ MoveExpectation moveExpectation = new MoveExpectation(
ImmutableMap.of("/test/z", "/test/y/z"));
MoveDetector moveDetector = new MoveDetector(moveExpectation);
CommitFailedException exception = EditorDiff.process(moveDetector,
root, moved);
Modified: jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences.md
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences.md?rev=1552323&r1=1552322&r2=1552323&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences.md (original)
+++ jackrabbit/oak/trunk/oak-doc/src/site/markdown/differences.md Thu Dec 19
16:09:09 2013
@@ -119,12 +119,26 @@ Observation
* Limited support for `Event.NODE_MOVED`:
- + `NODE_MOVED` are only reported for nodes whose source location is not
transient. A source
- location is transient if it is transiently added or a child node of a
transiently moved
- tree.
+ + A node that is added and subsequently moved will not generate a
`Event.NODE_MOVED`
+ but a `Event.NODE_ADDED` for its final location.
- + Removing a node and adding a node with the same name at the same
parent will be reported as
- `NODE_MOVED` event as if it where caused by `Node.orderBefore()`.
+ + A node that is moved and subsequently removed will not generate a
`Event.NODE_MOVED`
+ but a `Event.NODE_REMOVED` for its initial location.
+
+ + A node that is moved and subsequently moved again will only generate a
single
+ `Event.NODE_MOVED` reporting its initial location as `srcAbsPath` and
its
+ final location as `destAbsPath`.
+
+ + A node whose parent was moved and that moved itself subsequently
reports its initial
+ location as `srcAbsPath` instead of the location it had under the
moved parent.
+
+ + A node that was moved and subsequently its parent is moved will report
its final
+ location as `destAbsPath` instead of the location it had before its
parent moved.
+
+ + Removing a node and adding a node with the same name at the same
parent will be
+ reported as `NODE_MOVED` event as if it where caused by
`Node.orderBefore()` if
+ the parent node is orderable and the sequence of operations caused a
change in
+ the order of the child nodes.
+ The exact sequence of `Node.orderBefore()` will not be reflected
through `NODE_MOVED`
events: given two child nodes `a` and `b`, ordering `a` after `b` may
be reported as
Modified:
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java?rev=1552323&r1=1552322&r2=1552323&view=diff
==============================================================================
---
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
(original)
+++
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/observation/ObservationTest.java
Thu Dec 19 16:09:09 2013
@@ -426,6 +426,37 @@ public class ObservationTest extends Abs
}
@Test
+ public void testMove() throws RepositoryException, ExecutionException,
InterruptedException {
+ Node testNode = getNode(TEST_PATH);
+ Session session = testNode.getSession();
+ Node nodeA = testNode.addNode("a");
+ Node nodeAA = nodeA.addNode("aa");
+ Node nodeT = testNode.addNode("t");
+ Node nodeS = testNode.addNode("s");
+ session.save();
+
+ ExpectationListener listener = new ExpectationListener();
+ observationManager.addEventListener(listener, NODE_MOVED, "/", true,
null, null, false);
+
+ String src1 = nodeA.getPath();
+ String dst1 = nodeT.getPath() + "/b";
+ session.move(src1, dst1);
+ listener.expectMove(src1, dst1);
+
+ String src2 = nodeT.getPath() + "/b/aa";
+ String dst2 = nodeS.getPath() + "/bb";
+ session.move(src2, dst2);
+ listener.expectMove(src1 + "/aa", dst2);
+
+ session.save();
+
+ List<Expectation> missing = listener.getMissing(2, TimeUnit.SECONDS);
+ assertTrue("Missing events: " + missing, missing.isEmpty());
+ List<Event> unexpected = listener.getUnexpected();
+ assertTrue("Unexpected events: " + unexpected, unexpected.isEmpty());
+ }
+
+ @Test
public void testReorder() throws RepositoryException,
InterruptedException, ExecutionException {
Node testNode = getNode(TEST_PATH);
Node nodeA = testNode.addNode("a", "nt:unstructured");
@@ -433,7 +464,7 @@ public class ObservationTest extends Abs
testNode.getSession().save();
ExpectationListener listener = new ExpectationListener();
- observationManager.addEventListener(listener, Event.NODE_MOVED, "/",
true, null, null, false);
+ observationManager.addEventListener(listener, NODE_MOVED, "/", true,
null, null, false);
listener.expect(new Expectation("orderBefore"){
@Override
public boolean onEvent(Event event) throws Exception {
@@ -677,6 +708,18 @@ public class ObservationTest extends Abs
return property;
}
+ public void expectMove(final String src, final String dst) {
+ expect(new Expectation(">" + src + ':' + dst){
+ @Override
+ public boolean onEvent(Event event) throws Exception {
+ return event.getType() == NODE_MOVED &&
+ equal(dst, event.getPath()) &&
+ equal(src, event.getInfo().get("srcAbsPath")) &&
+ equal(dst, event.getInfo().get("destAbsPath"));
+ }
+ });
+ }
+
public List<Expectation> getMissing(int time, TimeUnit timeUnit)
throws ExecutionException, InterruptedException {
List<Expectation> missing = Lists.newArrayList();
Modified: jackrabbit/oak/trunk/oak-parent/pom.xml
URL:
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-parent/pom.xml?rev=1552323&r1=1552322&r2=1552323&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-parent/pom.xml (original)
+++ jackrabbit/oak/trunk/oak-parent/pom.xml Thu Dec 19 16:09:09 2013
@@ -233,7 +233,7 @@
<dependency>
<groupId>org.apache.maven.doxia</groupId>
<artifactId>doxia-module-markdown</artifactId>
- <version>1.4</version>
+ <version>1.5</version>
</dependency>
</dependencies>
</plugin>
