You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by ju...@apache.org on 2013/10/24 22:42:36 UTC
svn commit: r1535542 - in /jackrabbit/branches/2.6: ./
jackrabbit-core/src/main/java/org/apache/jackrabbit/core/
jackrabbit-core/src/test/java/org/apache/jackrabbit/core/
Author: jukka
Date: Thu Oct 24 20:42:36 2013
New Revision: 1535542
URL: http://svn.apache.org/r1535542
Log:
2.6: Merged revision 1535539 (JCR-3364)
Modified:
jackrabbit/branches/2.6/ (props changed)
jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java
Propchange: jackrabbit/branches/2.6/
------------------------------------------------------------------------------
Merged /jackrabbit/trunk:r1535539
Modified: jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java?rev=1535542&r1=1535541&r2=1535542&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java (original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java Thu Oct 24 20:42:36 2013
@@ -191,7 +191,8 @@ public class CachingHierarchyManager ext
* being used. If no mapping is found, the item is cached instead after
* the base implementation has been invoked.
*/
- protected void buildPath(PathBuilder builder, ItemState state)
+ protected void buildPath(
+ PathBuilder builder, ItemState state, CycleDetector detector)
throws ItemStateException, RepositoryException {
if (state.isNode()) {
@@ -211,7 +212,7 @@ public class CachingHierarchyManager ext
}
}
- super.buildPath(builder, state);
+ super.buildPath(builder, state, detector);
if (state.isNode()) {
try {
Modified: jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java?rev=1535542&r1=1535541&r2=1535542&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java (original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java Thu Oct 24 20:42:36 2013
@@ -17,9 +17,11 @@
package org.apache.jackrabbit.core;
import java.util.Collections;
+import java.util.HashSet;
import java.util.LinkedHashSet;
import java.util.Set;
+import javax.jcr.InvalidItemStateException;
import javax.jcr.ItemNotFoundException;
import javax.jcr.RepositoryException;
@@ -255,10 +257,12 @@ public class HierarchyManagerImpl implem
* either return cached responses or add response to cache. On exit,
* <code>builder</code> contains the path of <code>state</code>.
*
- * @param builder builder currently being used
- * @param state item to find path of
+ * @param builder builder currently being used
+ * @param state item to find path of
+ * @param detector path cycle detector
*/
- protected void buildPath(PathBuilder builder, ItemState state)
+ protected void buildPath(
+ PathBuilder builder, ItemState state, CycleDetector detector)
throws ItemStateException, RepositoryException {
// shortcut
@@ -273,11 +277,14 @@ public class HierarchyManagerImpl implem
+ ": orphaned item";
log.debug(msg);
throw new ItemNotFoundException(msg);
+ } else if (detector.checkCycle(parentId)) {
+ throw new InvalidItemStateException(
+ "Path cycle detected: " + parentId);
}
NodeState parent = (NodeState) getItemState(parentId);
// recursively build path of parent
- buildPath(builder, parent);
+ buildPath(builder, parent, detector);
if (state.isNode()) {
NodeState nodeState = (NodeState) state;
@@ -392,7 +399,7 @@ public class HierarchyManagerImpl implem
PathBuilder builder = new PathBuilder();
try {
- buildPath(builder, getItemState(id));
+ buildPath(builder, getItemState(id), new CycleDetector());
return builder.getPath();
} catch (NoSuchItemStateException nsise) {
String msg = "failed to build path of " + id;
@@ -644,4 +651,37 @@ public class HierarchyManagerImpl implem
throw new RepositoryException(msg, ise);
}
}
+
+ /**
+ * Utility class used to detect path cycles with as little overhead
+ * as possible. The {@link #checkCycle(ItemId)} method is called for
+ * each path element as the
+ * {@link HierarchyManagerImpl#buildPath(PathBuilder, ItemState, CycleDetector)}
+ * method walks up the hierarchy. At first, during the first fifteen
+ * path elements, the detector does nothing in order to avoid
+ * introducing any unnecessary overhead to normal paths that seldom
+ * are deeper than that. After that initial threshold all item
+ * identifiers along the path are tracked, and a cycle is reported
+ * if an identifier is encountered that already occurred along the
+ * same path.
+ */
+ protected static class CycleDetector {
+
+ private int count = 0;
+
+ private Set<ItemId> ids;
+
+ boolean checkCycle(ItemId id) throws InvalidItemStateException {
+ if (count++ >= 15) {
+ if (ids == null) {
+ ids = new HashSet<ItemId>();
+ } else {
+ return !ids.add(id);
+ }
+ }
+ return false;
+ }
+
+ }
+
}
Modified: jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java?rev=1535542&r1=1535541&r2=1535542&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java (original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java Thu Oct 24 20:42:36 2013
@@ -190,10 +190,6 @@ public class SessionMoveOperation implem
NodeState destParentState =
(NodeState) destParentNode.getOrCreateTransientItemState();
- // Create the transient parent nodes of the target node to prevent
- // conflicting and/or cyclic moves. See JCR-3921
- createTransientParentStates(destParentNode);
-
// do move:
// 1. remove child node entry from old parent
if (srcParentState.removeChildNodeEntry(targetId)) {
@@ -207,12 +203,6 @@ public class SessionMoveOperation implem
return this;
}
- private void createTransientParentStates(NodeImpl node) throws RepositoryException {
- while (node.getParentId() != null) {
- node.getOrCreateTransientItemState();
- node = (NodeImpl) node.getParent();
- }
- }
//--------------------------------------------------------------< Object >
Modified: jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java?rev=1535542&r1=1535541&r2=1535542&view=diff
==============================================================================
--- jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java (original)
+++ jackrabbit/branches/2.6/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java Thu Oct 24 20:42:36 2013
@@ -22,7 +22,6 @@ import javax.jcr.PathNotFoundException;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
-import org.apache.jackrabbit.core.state.StaleItemStateException;
import org.apache.jackrabbit.test.AbstractJCRTest;
public class ConcurrentCyclicMoveTest extends AbstractJCRTest {
@@ -45,24 +44,23 @@ public class ConcurrentCyclicMoveTest ex
// results in /b/a/aa
session1.move(testRootPath + "/a", testRootPath + "/b/a");
assertEquals(testRootPath + "/b/a/aa", session1.getNodeByIdentifier(aaId).getPath());
-
+
// results in a/aa/b
session2.move(testRootPath + "/b", testRootPath + "/a/aa/b");
assertEquals(testRootPath + "/a/aa/b", session2.getNodeByIdentifier(bId).getPath());
session1.save();
- // path should not have changed after save.
- assertEquals(testRootPath + "/a/aa/b", session2.getNodeByIdentifier(bId).getPath());
+ try {
+ session2.getNodeByIdentifier(bId).getPath();
+ fail("It should not be possible to access a cyclic path");
+ } catch (InvalidItemStateException expected) {
+ }
try {
session2.save();
fail("Save should have failed. Possible cyclic persistent path created.");
- } catch (InvalidItemStateException e) {
- // expect is a ex caused by a StaleItemStateException with "has been modified externally"
- if (!(e.getCause() instanceof StaleItemStateException)) {
- throw e;
- }
+ } catch (InvalidItemStateException expected) {
}
}