You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by st...@apache.org on 2005/04/10 17:05:44 UTC

svn commit: r160781 - in incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core: WorkspaceImpl.java xml/WorkspaceImporter.java

Author: stefan
Date: Sun Apr 10 08:05:42 2005
New Revision: 160781

URL: http://svn.apache.org/viewcvs?view=rev&rev=160781
Log:
Workspace.clone(...,removeExisting=true) & Workspace.importXML(..., IMPORT_UUID_COLLISION_REMOVE_EXISTING) potentially leaves repository in inconsistent state

Modified:
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/WorkspaceImpl.java
    incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/xml/WorkspaceImporter.java

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/WorkspaceImpl.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/WorkspaceImpl.java?view=diff&r1=160780&r2=160781
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/WorkspaceImpl.java (original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/WorkspaceImpl.java Sun Apr 10 08:05:42 2005
@@ -82,6 +82,7 @@
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
+import java.util.List;
 
 /**
  * A <code>WorkspaceImpl</code> ...
@@ -480,8 +481,8 @@
     }
 
     /**
-     * Checks if removing the given target node is allowed in the current
-     * context.
+     * Checks if removing the given target node entirely (i.e. unlinking from
+     * all its parents) is allowed in the current context.
      *
      * @param targetState
      * @param options     bit-wise OR'ed flags specifying the checks that should be
@@ -511,13 +512,54 @@
             throws ConstraintViolationException, AccessDeniedException,
             VersionException, LockException, ItemNotFoundException,
             ReferentialIntegrityException, RepositoryException {
+        List parentUUIDs = targetState.getParentUUIDs();
+        Iterator iter = parentUUIDs.iterator();
+        while (iter.hasNext()) {
+            NodeId parentId = new NodeId((String) iter.next());
+            checkRemoveNode(targetState, parentId, options);
+        }
+    }
+
+    /**
+     * Checks if removing the given target node from the specifed parent
+     * is allowed in the current context.
+     *
+     * @param targetState
+     * @param parentId
+     * @param options     bit-wise OR'ed flags specifying the checks that should be
+     *                    performed; any combination of the following constants:
+     *                    <ul>
+     *                    <li><code>{@link #CHECK_ACCESS}</code>: make sure
+     *                    current session is granted read access on parent
+     *                    and remove privilege on target node</li>
+     *                    <li><code>{@link #CHECK_LOCK}</code>: make sure
+     *                    there's no foreign lock on parent node</li>
+     *                    <li><code>{@link #CHECK_VERSIONING}</code>: make sure
+     *                    parent node is checked-out</li>
+     *                    <li><code>{@link #CHECK_CONSTRAINTS}</code>:
+     *                    make sure no node type constraints would be violated</li>
+     *                    <li><code>{@link #CHECK_REFERENCES}</code>:
+     *                    make sure no references exist on target node</li>
+     *                    </ul>
+     * @throws ConstraintViolationException
+     * @throws AccessDeniedException
+     * @throws VersionException
+     * @throws LockException
+     * @throws ItemNotFoundException
+     * @throws ReferentialIntegrityException
+     * @throws RepositoryException
+     */
+    public void checkRemoveNode(NodeState targetState, NodeId parentId,
+                                int options)
+            throws ConstraintViolationException, AccessDeniedException,
+            VersionException, LockException, ItemNotFoundException,
+            ReferentialIntegrityException, RepositoryException {
 
         if (targetState.getParentUUID() == null) {
             // root or orphaned node
             throw new ConstraintViolationException("cannot remove root node");
         }
         NodeId targetId = (NodeId) targetState.getId();
-        NodeId parentId = new NodeId(targetState.getParentUUID());
         NodeState parentState = getNodeState(parentId);
         Path parentPath = hierMgr.getPath(parentId);
 
@@ -762,8 +804,53 @@
     }
 
     /**
-     * Recursively removes the specified node state including its properties and
-     * child nodes.
+     * Unlinks the specified node state from all its parents and recursively
+     * removes it including its properties and child nodes.
+     * <p/>
+     * <b>Precondition:</b> the state manager of this workspace needs to be in
+     * edit mode.
+     * todo duplicate code in WorkspaceImporter; consolidate in WorkspaceOperations class
+     *
+     * @param targetState
+     * @throws RepositoryException if an error occurs
+     */
+    private void removeNodeState(NodeState targetState)
+            throws RepositoryException {
+
+        // copy list to avoid ConcurrentModificationException
+        ArrayList parentUUIDs = new ArrayList(targetState.getParentUUIDs());
+        Iterator iter = parentUUIDs.iterator();
+        while (iter.hasNext()) {
+            String parentUUID = (String) iter.next();
+            NodeId parentId = new NodeId(parentUUID);
+
+            // unlink node state from this parent
+            unlinkNodeState(targetState, parentUUID);
+
+            // remove child node entries
+            NodeState parent = getNodeState(parentId);
+            // use temp array to avoid ConcurrentModificationException
+            ArrayList tmp =
+                    new ArrayList(parent.getChildNodeEntries(targetState.getUUID()));
+            // remove from tail to avoid problems with same-name siblings
+            for (int i = tmp.size() - 1; i >= 0; i--) {
+                NodeState.ChildNodeEntry entry = (NodeState.ChildNodeEntry) tmp.get(i);
+                parent.removeChildNodeEntry(entry.getName(), entry.getIndex());
+            }
+            // store parent
+            stateMgr.store(parent);
+        }
+    }
+
+    /**
+     * Unlinks the given node state from the specified parent i.e. removes
+     * <code>parentUUID</code> from its list of parents. If as a result
+     * the given node state would be orphaned it will be recursively removed
+     * including its properties and child nodes.
+     * <p/>
+     * Note that the child node entry refering to <code>targetState</code> is
+     * <b><i>not</i></b> automatically removed from <code>targetState</code>'s
+     * parent denoted by <code>parentUUID</code>.
      * <p/>
      * <b>Precondition:</b> the state manager of this workspace needs to be in
      * edit mode.
@@ -773,7 +860,7 @@
      * @param parentUUID
      * @throws RepositoryException if an error occurs
      */
-    private void removeNodeState(NodeState targetState, String parentUUID)
+    private void unlinkNodeState(NodeState targetState, String parentUUID)
             throws RepositoryException {
 
         // check if this node state would be orphaned after unlinking it from parent
@@ -791,11 +878,12 @@
                 NodeId nodeId = new NodeId(entry.getUUID());
                 try {
                     NodeState nodeState = (NodeState) stateMgr.getItemState(nodeId);
-                    // check if existing can be removed
-                    checkRemoveNode(nodeState, CHECK_ACCESS | CHECK_LOCK
-                            | CHECK_VERSIONING);
-                    // remove child node (recursive)
-                    removeNodeState(nodeState, targetState.getUUID());
+                    // check if child node can be removed
+                    // (access rights, locking & versioning status)
+                    checkRemoveNode(nodeState, (NodeId) targetState.getId(),
+                            CHECK_ACCESS | CHECK_LOCK | CHECK_VERSIONING);
+                    // unlink child node (recursive)
+                    unlinkNodeState(nodeState, targetState.getUUID());
                 } catch (ItemStateException ise) {
                     String msg = "internal error: failed to retrieve state of "
                             + nodeId;
@@ -815,7 +903,8 @@
                 PropertyId propId =
                         new PropertyId(targetState.getUUID(), entry.getName());
                 try {
-                    PropertyState propState = (PropertyState) stateMgr.getItemState(propId);
+                    PropertyState propState =
+                            (PropertyState) stateMgr.getItemState(propId);
                     // remove property entry
                     targetState.removePropertyEntry(propId.getName());
                     // destroy property state
@@ -911,11 +1000,30 @@
                     id = new NodeId(uuid);
                     if (stateMgr.hasItemState(id)) {
                         NodeState existingState = (NodeState) stateMgr.getItemState(id);
+                        // make sure existing node is not the parent
+                        // or an ancestor thereof
+                        NodeId newParentId = new NodeId(parentUUID);
+                        Path p0 = hierMgr.getPath(newParentId);
+                        Path p1 = hierMgr.getPath(id);
+                        try {
+                            if (p1.equals(p0) || p1.isAncestorOf(p0)) {
+                                String msg = "cannot remove ancestor node";
+                                log.debug(msg);
+                                throw new RepositoryException(msg);
+                            }
+                        } catch (MalformedPathException mpe) {
+                            // should never get here...
+                            String msg = "internal error: failed to determine degree of relationship";
+                            log.error(msg, mpe);
+                            throw new RepositoryException(msg, mpe);
+                        }
+
                         // check if existing can be removed
                         checkRemoveNode(existingState, CHECK_ACCESS | CHECK_LOCK
                                 | CHECK_VERSIONING | CHECK_CONSTRAINTS);
+
                         // do remove existing
-                        removeNodeState(existingState, existingState.getParentUUID());
+                        removeNodeState(existingState);
                     }
                     break;
                 default:
@@ -1390,8 +1498,8 @@
 
         // 2. check if target state can be removed from old/added to new parent
 
-        checkRemoveNode(targetState, CHECK_ACCESS | CHECK_LOCK
-                | CHECK_VERSIONING | CHECK_CONSTRAINTS);
+        checkRemoveNode(targetState, (NodeId) srcParentState.getId(),
+                CHECK_ACCESS | CHECK_LOCK | CHECK_VERSIONING | CHECK_CONSTRAINTS);
         checkAddNode(destParentState, destName.getName(),
                 targetState.getNodeTypeName(), CHECK_ACCESS | CHECK_LOCK
                 | CHECK_VERSIONING | CHECK_CONSTRAINTS);

Modified: incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/xml/WorkspaceImporter.java
URL: http://svn.apache.org/viewcvs/incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/xml/WorkspaceImporter.java?view=diff&r1=160780&r2=160781
==============================================================================
--- incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/xml/WorkspaceImporter.java (original)
+++ incubator/jackrabbit/trunk/src/java/org/apache/jackrabbit/core/xml/WorkspaceImporter.java Sun Apr 10 08:05:42 2005
@@ -372,17 +372,73 @@
     }
 
     /**
-     * Recursively removes the specified node state including its properties and
-     * child nodes.
+     * Unlinks the specified target node from all its parents and recursively
+     * removes it including its properties and child nodes.
      * <p/>
-     * todo duplicate code in WorkspaceImpl; consolidate in WorkspaceOperations class
+     * <b>Precondition:</b> the state manager of this workspace needs to be in
+     * edit mode.
+     * todo duplicate code in WorkspaceImporter; consolidate in WorkspaceOperations class
+     *
+     * @param target
+     * @throws RepositoryException if an error occurs
+     */
+    protected void removeNode(NodeState target)
+            throws RepositoryException {
+
+        // copy list to avoid ConcurrentModificationException
+        ArrayList parentUUIDs = new ArrayList(target.getParentUUIDs());
+        Iterator iter = parentUUIDs.iterator();
+        while (iter.hasNext()) {
+            String parentUUID = (String) iter.next();
+            NodeId parentId = new NodeId(parentUUID);
+
+            // unlink target node from this parent
+            unlinkNodeState(target, parentUUID);
+
+            // remove child node entries
+            NodeState parent;
+            try {
+                parent = (NodeState) stateMgr.getItemState(parentId);
+            } catch (ItemStateException ise) {
+                // should never get here...
+                String msg = "internal error: failed to retrieve parent state";
+                log.error(msg, ise);
+                throw new RepositoryException(msg, ise);
+            }
+            // use temp array to avoid ConcurrentModificationException
+            ArrayList tmp =
+                    new ArrayList(parent.getChildNodeEntries(target.getUUID()));
+            // remove from tail to avoid problems with same-name siblings
+            for (int i = tmp.size() - 1; i >= 0; i--) {
+                NodeState.ChildNodeEntry entry = (NodeState.ChildNodeEntry) tmp.get(i);
+                parent.removeChildNodeEntry(entry.getName(), entry.getIndex());
+            }
+            // store parent
+            stateMgr.store(parent);
+        }
+    }
+
+    /**
+     * Unlinks the given target node from the specified parent i.e. removes
+     * <code>parentUUID</code> from its list of parents. If as a result
+     * the target node would be orphaned it will be recursively removed
+     * including its properties and child nodes.
+     * <p/>
+     * Note that the child node entry refering to <code>target</code> is
+     * <b><i>not</i></b> automatically removed from <code>target</code>'s
+     * parent denoted by <code>parentUUID</code>.
+     * <p/>
+     * <b>Precondition:</b> the state manager of this workspace needs to be in
+     * edit mode.
+     * todo duplicate code in WorkspaceImporter; consolidate in WorkspaceOperations class
      *
      * @param target
      * @param parentUUID
      * @throws RepositoryException if an error occurs
      */
-    protected void removeNode(NodeState target, String parentUUID)
+    private void unlinkNodeState(NodeState target, String parentUUID)
             throws RepositoryException {
+
         // check if this node state would be orphaned after unlinking it from parent
         ArrayList parentUUIDs = new ArrayList(target.getParentUUIDs());
         parentUUIDs.remove(parentUUID);
@@ -397,14 +453,14 @@
                 NodeState.ChildNodeEntry entry = (NodeState.ChildNodeEntry) tmp.get(i);
                 NodeId nodeId = new NodeId(entry.getUUID());
                 try {
-                    NodeState node = (NodeState) stateMgr.getItemState(nodeId);
-                    // check if existing can be removed
+                    NodeState nodeState = (NodeState) stateMgr.getItemState(nodeId);
+                    // check if child node can be removed
                     // (access rights, locking & versioning status)
-                    wsp.checkRemoveNode(node,
+                    wsp.checkRemoveNode(nodeState, (NodeId) target.getId(),
                             WorkspaceImpl.CHECK_ACCESS | WorkspaceImpl.CHECK_LOCK
                             | WorkspaceImpl.CHECK_VERSIONING);
-                    // remove child node (recursive)
-                    removeNode(node, target.getUUID());
+                    // unlink child node (recursive)
+                    unlinkNodeState(nodeState, target.getUUID());
                 } catch (ItemStateException ise) {
                     String msg = "internal error: failed to retrieve state of "
                             + nodeId;
@@ -424,11 +480,11 @@
                 PropertyId propId =
                         new PropertyId(target.getUUID(), entry.getName());
                 try {
-                    PropertyState prop = (PropertyState) stateMgr.getItemState(propId);
+                    PropertyState propState = (PropertyState) stateMgr.getItemState(propId);
                     // remove property entry
                     target.removePropertyEntry(propId.getName());
                     // destroy property state
-                    stateMgr.destroy(prop);
+                    stateMgr.destroy(propState);
                 } catch (ItemStateException ise) {
                     String msg = "internal error: failed to retrieve state of "
                             + propId;
@@ -483,7 +539,7 @@
             Path p0 = hierMgr.getPath(importTarget.getId());
             Path p1 = hierMgr.getPath(conflicting.getId());
             try {
-                if (p0.equals(p1) || p0.isAncestorOf(p1)) {
+                if (p1.equals(p0) || p1.isAncestorOf(p0)) {
                     String msg = "cannot remove ancestor node";
                     log.debug(msg);
                     throw new RepositoryException(msg);
@@ -503,7 +559,8 @@
                     | WorkspaceImpl.CHECK_VERSIONING
                     | WorkspaceImpl.CHECK_CONSTRAINTS);
             // do remove conflicting (recursive)
-            removeNode(conflicting, conflicting.getParentUUID());
+            removeNode(conflicting);
+
             // create new with given uuid:
             // check if new node can be added (check access rights &
             // node type constraints only, assume locking & versioning status
@@ -540,7 +597,7 @@
                     | WorkspaceImpl.CHECK_VERSIONING
                     | WorkspaceImpl.CHECK_CONSTRAINTS);
             // do remove conflicting (recursive)
-            removeNode(conflicting, conflicting.getParentUUID());
+            removeNode(conflicting);
             // create new with given uuid at same location as conflicting:
             // check if new node can be added at other location
             // (access rights, node type constraints, locking & versioning status)