jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r955229 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: BatchedItemOperations.java ItemImpl.java NodeImpl.java SessionImpl.java state/ChildNodeEntries.java state/SharedItemStateManager.java
Date Wed, 16 Jun 2010 13:52:31 GMT
Author: jukka
Date: Wed Jun 16 13:52:31 2010
New Revision: 955229

URL: http://svn.apache.org/viewvc?rev=955229&view=rev
Log:
JCR-2598: Saving concurrent sessions executing random operations causes a corrupt JCR

Patch by Stephan Huttenhuis

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
Wed Jun 16 13:52:31 2010
@@ -573,14 +573,14 @@ public class BatchedItemOperations exten
                 throw new UnsupportedRepositoryOperationException(msg);
             }
 
-            // remove child node entry from old parent
-            srcParent.removeChildNodeEntry(srcName.getName(), srcNameIndex);
-
-            // re-parent target node
-            target.setParentId(destParent.getNodeId());
-
-            // add child node entry to new parent
-            destParent.addChildNodeEntry(destName.getName(), target.getNodeId());
+            // do move:
+            // 1. remove child node entry from old parent
+            if (srcParent.removeChildNodeEntry(target.getNodeId())) {
+                // 2. re-parent target node
+                target.setParentId(destParent.getNodeId());
+                // 3. add child node entry to new parent
+                destParent.addChildNodeEntry(destName.getName(), target.getNodeId());
+            }
         }
 
         // store states

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
Wed Jun 16 13:52:31 2010
@@ -875,10 +875,10 @@ public abstract class ItemImpl implement
         }
 
         // delegate the removal of the child item to the parent node
-        Path.Element thisName = getPrimaryPath().getNameElement();
         if (isNode()) {
-            parentNode.removeChildNode(thisName.getName(), thisName.getIndex());
+            parentNode.removeChildNode((NodeId) getId());
         } else {
+            Path.Element thisName = getPrimaryPath().getNameElement();
             parentNode.removeChildProperty(thisName.getName());
         }
     }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
Wed Jun 16 13:52:31 2010
@@ -565,29 +565,24 @@ public class NodeImpl extends ItemImpl i
         itemMgr.getItem(propId).setRemoved();
     }
 
-    protected void removeChildNode(Name nodeName, int index)
-            throws RepositoryException {
+    protected void removeChildNode(NodeId childId) throws RepositoryException {
         // modify the state of 'this', i.e. the parent node
         NodeState thisState = (NodeState) getOrCreateTransientItemState();
-        if (index == 0) {
-            index = 1;
-        }
         ChildNodeEntry entry =
-                thisState.getChildNodeEntry(nodeName, index);
+                thisState.getChildNodeEntry(childId);
         if (entry == null) {
-            String msg = "failed to remove child " + nodeName + " of " + this;
+            String msg = "failed to remove child " + childId + " of " + this;
             log.debug(msg);
             throw new RepositoryException(msg);
         }
 
         // notify target of removal
-        NodeId childId = entry.getId();
         NodeImpl childNode = itemMgr.getNode(childId, getNodeId());
         childNode.onRemove(getNodeId());
 
         // remove the child node entry
-        if (!thisState.removeChildNodeEntry(nodeName, index)) {
-            String msg = "failed to remove child " + nodeName + " of " + this;
+        if (!thisState.removeChildNodeEntry(childId)) {
+            String msg = "failed to remove child " + childId + " of " + this;
             log.debug(msg);
             throw new RepositoryException(msg);
         }
@@ -635,7 +630,7 @@ public class NodeImpl extends ItemImpl i
                 NodeImpl childNode = itemMgr.getNode(childId, getNodeId());
                 childNode.onRemove(thisState.getNodeId());
                 // remove the child node entry
-                thisState.removeChildNodeEntry(entry.getName(), entry.getIndex());
+                thisState.removeChildNodeEntry(childId);
             }
         }
 
@@ -1146,7 +1141,7 @@ public class NodeImpl extends ItemImpl i
 
                 if (oldDef.isProtected()) {
                     // remove 'orphaned' protected child node immediately
-                    removeChildNode(entry.getName(), entry.getIndex());
+                    removeChildNode(entry.getId());
                     continue;
                 }
 
@@ -1161,7 +1156,7 @@ public class NodeImpl extends ItemImpl i
                 } catch (ConstraintViolationException cve) {
                     // no suitable definition found for this child node,
                     // remove it
-                    removeChildNode(entry.getName(), entry.getIndex());
+                    removeChildNode(entry.getId());
                 }
             }
             success = true;
@@ -3686,7 +3681,7 @@ public class NodeImpl extends ItemImpl i
                         NodeImpl node = (NodeImpl) itemMgr.getItem(nodeState.getId());
                         if (node.getDefinition().isProtected()) {
                             // remove 'orphaned' protected child node immediately
-                            removeChildNode(entry.getName(), entry.getIndex());
+                            removeChildNode(entry.getId());
                             continue;
                         }
                         NodeDefinitionImpl ndi = getApplicableChildNodeDefinition(
@@ -3699,7 +3694,7 @@ public class NodeImpl extends ItemImpl i
                     } catch (ConstraintViolationException cve) {
                         // no suitable definition found for this child node,
                         // remove it
-                        removeChildNode(entry.getName(), entry.getIndex());
+                        removeChildNode(entry.getId());
                     }
                 }
             } catch (ItemStateException ise) {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
Wed Jun 16 13:52:31 2010
@@ -1107,22 +1107,25 @@ public class SessionImpl extends Abstrac
                 log.debug(msg);
                 throw new UnsupportedRepositoryOperationException(msg);
             }
-              // change definition of target
+            // change definition of target
             targetNode.onRedefine(newTargetDef.unwrap());
 
-            // do move:
-            // 1. remove child node entry from old parent
+            // Get the transient states
             NodeState srcParentState =
                     (NodeState) srcParentNode.getOrCreateTransientItemState();
-            srcParentState.removeChildNodeEntry(srcName.getName(), index);
-            // 2. re-parent target node
             NodeState targetState =
                     (NodeState) targetNode.getOrCreateTransientItemState();
-            targetState.setParentId(destParentNode.getNodeId());
-            // 3. add child node entry to new parent
             NodeState destParentState =
                     (NodeState) destParentNode.getOrCreateTransientItemState();
-            destParentState.addChildNodeEntry(destName.getName(), targetId);
+
+            // do move:
+            // 1. remove child node entry from old parent
+            if (srcParentState.removeChildNodeEntry(targetId)) {
+                // 2. re-parent target node
+                targetState.setParentId(destParentNode.getNodeId());
+                // 3. add child node entry to new parent
+                destParentState.addChildNodeEntry(destName.getName(), targetId);
+            }
         }
     }
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ChildNodeEntries.java
Wed Jun 16 13:52:31 2010
@@ -153,6 +153,9 @@ class ChildNodeEntries implements List<C
         }
     }
 
+    // The index may have changed because of changes by another session. Use remove(NodeId
id)
+    // instead    
+    @Deprecated
     @SuppressWarnings("unchecked")
     public ChildNodeEntry remove(Name nodeName, int index) {
         if (index < 1) {
@@ -233,7 +236,7 @@ class ChildNodeEntries implements List<C
      * @return the removed entry or <code>null</code> if there is no such entry.
      */
     public ChildNodeEntry remove(ChildNodeEntry entry) {
-        return remove(entry.getName(), entry.getIndex());
+        return remove(entry.getId());
     }
 
     /**

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java?rev=955229&r1=955228&r2=955229&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
Wed Jun 16 13:52:31 2010
@@ -118,6 +118,13 @@ public class SharedItemStateManager
     private static Logger log = LoggerFactory.getLogger(SharedItemStateManager.class);
 
     /**
+     * Flag for enabling hierarchy validation.
+     * @see <a href="https://issues.apache.org/jira/browse/JCR-2598">JCR-2598</a>
+     */
+    private static final boolean VALIDATE_HIERARCHY =
+        Boolean.getBoolean("org.apache.jackrabbit.core.state.validatehierarchy");
+
+    /**
      * cache of weak references to ItemState objects issued by this
      * ItemStateManager
      */
@@ -706,6 +713,17 @@ public class SharedItemStateManager
                     eventChannel.updatePrepared(this);
                 }
 
+                if (VALIDATE_HIERARCHY) {
+                    log.info("Validating change-set hierarchy");
+                    try {
+                        validateHierarchy(local);
+                    } catch (ItemStateException e) {
+                        throw e;
+                    } catch (RepositoryException e) {
+                        throw new ItemStateException("Invalid hierarchy", e);
+                    }
+                }
+
                 /* Push all changes from the local items to the shared items */
                 local.push();
 
@@ -1096,7 +1114,334 @@ public class SharedItemStateManager
         }
 
     }
+    
+    /**
+     * Validates the hierarchy consistency of the changes in the changelog.
+     * 
+     * @param changeLog
+     *            The local changelog the should be validated
+     * @throws ItemStateException
+     *             If the hierarchy changes are inconsistent.
+     * @throws RepositoryException
+     *             If the consistency could not be validated
+     * 
+     */
+    private void validateHierarchy(ChangeLog changeLog) throws ItemStateException, RepositoryException
{
+
+        // Check the deleted node states
+        validateDeleted(changeLog);
+
+        // Check the added node states
+        validateAdded(changeLog);
+
+        // Check the modified node states
+        validateModified(changeLog);
+    }
+
+    /**
+     * Checks the parents and children of all deleted node states in the changelog.
+     * 
+     * @param changeLog
+     *            The local changelog the should be validated
+     * @throws ItemStateException
+     *             If the hierarchy changes are inconsistent.
+     */
+    private void validateDeleted(ChangeLog changeLog) throws ItemStateException {
+
+        // Check each deleted nodestate
+        for (ItemState removedState : changeLog.deletedStates()) {
+            if (removedState instanceof NodeState) {
+
+                // Get the next state
+                NodeState removedNodeState = (NodeState) removedState;
+                NodeId id = removedNodeState.getNodeId();
+
+                // Get and check the corresponding overlayed state
+                NodeState overlayedState = (NodeState) removedState.getOverlayedState();
+                if (overlayedState == null) {
+                    String message = "Unable to load persistent state for removed node "
+ id;
+                    overlayedState = (NodeState) SharedItemStateManager.this.getItemState(id);
+                    if (overlayedState == null) {
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+                }
+
+                // Check whether an version of this node has been restored
+                boolean addedAndRemoved = changeLog.has(removedNodeState.getId());
+                if (!addedAndRemoved) {
+
+                    // Check the old parent
+                    NodeId oldParentId = overlayedState.getParentId();
+                    if (changeLog.deleted(oldParentId)) {
+                        // parent has been deleted aswell
+                    } else if (changeLog.isModified(oldParentId)) {
+                        // the modified state will be check later on
+                    } else {
+                        String message = "Node with id " + id
+                                + " has been removed, but the parent node isn't part of the
changelog " + oldParentId;
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+
+                    // Get the original list of child ids
+                    for (ChildNodeEntry entry : overlayedState.getChildNodeEntries()) {
+
+                        // Check the next child
+                        NodeId childId = entry.getId();
+
+                        if (changeLog.deleted(childId)) {
+                            // child has been deleted aswell
+                        } else if (changeLog.isModified(childId)) {
+
+                            // the modified state will be check later on
+                        } else {
+                            String message = "Node with id " + id
+                                    + " has been removed, but the old child node isn't part
of the changelog "
+                                    + childId;
+                            log.error(message);
+                            throw new ItemStateException(message);
+                        }
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Checks the parents and children of all added node states in the changelog.
+     * 
+     * @param changeLog
+     *            The local changelog the should be validated
+     * @throws ItemStateException
+     *             If the hierarchy changes are inconsistent.
+     */
+    private void validateAdded(ChangeLog changeLog) throws ItemStateException {
+
+        // Check each added node
+        for (ItemState state : changeLog.addedStates()) {
+            if (state instanceof NodeState) {
+
+                // Get the next added node
+                NodeState addedNodeState = (NodeState) state;
+                NodeId id = addedNodeState.getNodeId();
+
+                // Check the parent
+                NodeId parentId = addedNodeState.getParentId();
+                if (changeLog.has(parentId)) { // Added or modified
+                    // the modified state will be check later on
+                    checkParent(changeLog, addedNodeState, parentId);
+                } else {
+                    String message = "Node with id " + id
+                            + " has been added, but the parent node isn't part of the changelog
" + parentId;
+                    log.error(message);
+                    throw new ItemStateException(message);
+                }
+
+                // Check the children
+                for (ChildNodeEntry entry : addedNodeState.getChildNodeEntries()) {
+
+                    // Get the next child
+                    NodeId childId = entry.getId();
+
+                    if (changeLog.has(childId)) {
+                        NodeState childState = (NodeState) changeLog.get(childId);
+                        checkParent(changeLog, childState, id);
+                        // the child state will be check later on
+
+                    } else {
+                        String message = "Node with id " + id
+                                + " has been added, but the child node isn't part of the
changelog " + childId;
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Checks the parents and children of all modified node states in the changelog.
+     * 
+     * @param changeLog
+     *            The local changelog the should be validated
+     * @throws ItemStateException
+     *             If the hierarchy changes are inconsistent.
+     */
+    private void validateModified(ChangeLog changeLog) throws ItemStateException, RepositoryException
{
+
+        // Check all modified nodes
+        for (ItemState state : changeLog.modifiedStates()) {
+            if (state instanceof NodeState) {
+
+                // Check the next node
+                NodeState modifiedNodeState = (NodeState) state;
+                NodeId id = modifiedNodeState.getNodeId();
+
+                // Check whether to overlayed state is present for determining diffs
+                NodeState overlayedState = (NodeState) modifiedNodeState.getOverlayedState();
+                if (overlayedState == null) {
+                    String message = "Unable to load persistent state for modified node "
+ id;
+                    log.error(message);
+                    throw new ItemStateException(message);
+                }
+
+                // Check the parent
+                NodeId parentId = modifiedNodeState.getParentId();
+                NodeId oldParentId = overlayedState.getParentId();
+
+                // The parent should not be deleted
+                if (parentId != null && changeLog.deleted(parentId)) {
+                    String message = "Parent of node with id " + id + " has been deleted";
+                    log.error(message);
+                    throw new ItemStateException(message);
+                }
+
+                if (parentId != null && changeLog.has(parentId)) {
+                    checkParent(changeLog, modifiedNodeState, parentId);
+                }
+
+                // Check whether this node is the root node
+                if (parentId == null && oldParentId == null) {
+                    // The root node can be ignored
+
+                } else if (!parentId.equals(oldParentId)) {
+
+                    // This node has been moved, check whether the parent has been modified
aswell
+                    if (changeLog.has(parentId)) {
+                        checkParent(changeLog, modifiedNodeState, parentId);
+                    } else if (!isShareable(modifiedNodeState)) {
+                        String message = "New parent of node " + id + " is not present in
the changelog " + id;
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+
+                    // The old parent must be modified or deleted
+                    if (!changeLog.isModified(oldParentId) && !changeLog.deleted(oldParentId))
{
+                        String message = "Node with id " + id
+                                + " has been move, but the original parent is not part of
the changelog: "
+                                + oldParentId;
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+                }
+
+                // Check all assigned children
+                for (ChildNodeEntry entry : modifiedNodeState.getChildNodeEntries()) {
+
+                    NodeId childId = entry.getId();
+
+                    // Check whether this node has a deleted childid
+                    if (changeLog.deleted(childId) && !changeLog.has(childId)) {
// Versionable
+                        String message = "Node with id " + id + " has a deleted childid:
" + childId;
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+
+                    if (changeLog.has(childId)) {
+                        NodeState childState = (NodeState) changeLog.get(childId);
+                        checkParent(changeLog, childState, id);
+                    }
+                }
+
+                // Check all children the have been added
+                for (ChildNodeEntry entry : modifiedNodeState.getAddedChildNodeEntries())
{
+                    NodeId childId = entry.getId();
+                    if (!changeLog.has(childId)) {
+                        String message = "ChildId " + childId + " has been added to parent
" + id
+                                + ", but is not present in the changelog";
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+                }
+
+                // Check all children the have been moved or removed
+                for (ChildNodeEntry entry : modifiedNodeState.getRemovedChildNodeEntries())
{
+                    NodeId childId = entry.getId();
+                    if (!changeLog.isModified(childId) && !changeLog.deleted(childId))
{
+                        String message = "Child node entry with id " + childId
+                                + " has been removed, but is not present in the changelog";
+                        log.error(message);
+                        throw new ItemStateException(message);
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Check the consistency of a parent/child relationship.
+     * 
+     * @param changeLog
+     *            The changelog to check
+     * @param childState
+     *            The id of the node for which the parent/child relationship should be validated.
+     * @param expectedParent
+     *            The expected parent id of the child node.
+     * @throws ItemStateException
+     *             If a inconsistency has been detected.
+     */
+    void checkParent(ChangeLog changeLog, NodeState childState, NodeId expectedParent) throws
ItemStateException {
+
+        // Check whether the the changelog contains an entry for the parent aswell.
+        NodeId parentId = childState.getParentId();
+        if (!parentId.equals(expectedParent)) {
+            Set sharedSet = childState.getSharedSet();
+            if (sharedSet.contains(expectedParent)) {
+                return;
+            }
+            String message = "Child node has another parent id " + parentId + ", expected
" + expectedParent;
+            log.error(message);
+            throw new ItemStateException(message);
+        }
+
+        if (!changeLog.has(parentId)) {
+            String message = "Parent not part of changelog";
+            log.error(message);
+            throw new ItemStateException(message);
+        }
+
+        // Get the parent from the changelog
+        NodeState parent = (NodeState) changeLog.get(parentId);
+
+        // Get and check the child node entry from the parent
+        NodeId childId = childState.getNodeId();
+        ChildNodeEntry childNodeEntry = parent.getChildNodeEntry(childId);
+        if (childNodeEntry == null) {
+            String message = "Child not present in parent";
+            log.error(message);
+            throw new ItemStateException(message);
+        }
+    }
+
+    /**
+     * Determines whether the specified node is <i>shareable</i>, i.e. whether
the mixin type <code>mix:shareable</code>
+     * is either directly assigned or indirectly inherited.
+     * 
+     * @param state
+     *            node state to check
+     * @return true if the specified node is <i>shareable</i>, false otherwise.
+     * @throws RepositoryException
+     *             if an error occurs
+     */
+    private boolean isShareable(NodeState state) throws RepositoryException {
+        // shortcut: check some wellknown built-in types first
+        Name primary = state.getNodeTypeName();
+        Set mixins = state.getMixinTypeNames();
+        if (mixins.contains(NameConstants.MIX_SHAREABLE)) {
+            return true;
+        }
 
+        try {
+            EffectiveNodeType type = ntReg.getEffectiveNodeType(primary, mixins);
+            return type.includesNodeType(NameConstants.MIX_SHAREABLE);
+        } catch (NodeTypeConflictException ntce) {
+            String msg = "internal error: failed to build effective node type for node "
+ state.getNodeId();
+            log.debug(msg);
+            throw new RepositoryException(msg, ntce);
+        }
+    }
+    
     /**
      * Begin update operation. This will return an object that can itself be
      * ended/canceled.



Mime
View raw message