jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1535543 - in /jackrabbit/branches/2.4: ./ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ jackrabbit-core/src/test/java/org/apache/jackrabbit/core/
Date Thu, 24 Oct 2013 20:42:57 GMT
Author: jukka
Date: Thu Oct 24 20:42:56 2013
New Revision: 1535543

URL: http://svn.apache.org/r1535543
Log:
2.4: Merged revision 1535539 (JCR-3364)

Modified:
    jackrabbit/branches/2.4/   (props changed)
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
    jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
    jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java

Propchange: jackrabbit/branches/2.4/
------------------------------------------------------------------------------
  Merged /jackrabbit/trunk:r1535539

Modified: jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java?rev=1535543&r1=1535542&r2=1535543&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
(original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
Thu Oct 24 20:42:56 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.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java?rev=1535543&r1=1535542&r2=1535543&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
(original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
Thu Oct 24 20:42:56 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.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java?rev=1535543&r1=1535542&r2=1535543&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
(original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionMoveOperation.java
Thu Oct 24 20:42:56 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.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java?rev=1535543&r1=1535542&r2=1535543&view=diff
==============================================================================
--- jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java
(original)
+++ jackrabbit/branches/2.4/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentCyclicMoveTest.java
Thu Oct 24 20:42:56 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) {
         }
     }
 



Mime
View raw message