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 2011/09/22 11:27:49 UTC

svn commit: r1174016 - in /jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: ./ state/ version/

Author: jukka
Date: Thu Sep 22 09:27:48 2011
New Revision: 1174016

URL: http://svn.apache.org/viewvc?rev=1174016&view=rev
Log:
JCR-2272: Errors during concurrent session import of nodes with same UUIDs

Move node id generation into the createNew() method, and add a check for existing nodes when an existing node id is being used

Modified:
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
    jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java Thu Sep 22 09:27:48 2011
@@ -1098,9 +1098,6 @@ public class BatchedItemOperations exten
             NodeId errorId = parent.getChildNodeEntry(nodeName, 1).getId();
             throw new ItemExistsException(safeGetJCRPath(errorId));
         }
-        if (id == null) {
-            id = context.getNodeIdFactory().newNodeId();
-        }
         if (nodeTypeName == null) {
             // no primary node type specified,
             // try default primary type from definition
@@ -1119,7 +1116,7 @@ public class BatchedItemOperations exten
         }
 
         // now add new child node entry to parent
-        parent.addChildNodeEntry(nodeName, id);
+        parent.addChildNodeEntry(nodeName, node.getNodeId());
 
         EffectiveNodeType ent = getEffectiveNodeType(node);
 
@@ -1598,7 +1595,7 @@ public class BatchedItemOperations exten
 
         NodeState newState;
         try {
-            NodeId id;
+            NodeId id = null;
             EffectiveNodeType ent = getEffectiveNodeType(srcState);
             boolean referenceable = ent.includesNodeType(NameConstants.MIX_REFERENCEABLE);
             boolean versionable = ent.includesNodeType(NameConstants.MIX_SIMPLE_VERSIONABLE);
@@ -1616,17 +1613,10 @@ public class BatchedItemOperations exten
                         sharedState.addShare(destParentId);
                         return sharedState;
                     }
-                    // always create new node id
-                    id = context.getNodeIdFactory().newNodeId();
-                    if (referenceable) {
-                        // remember uuid mapping
-                        refTracker.mappedId(srcState.getNodeId(), id);
-                    }
                     break;
                 case CLONE:
                     if (!referenceable) {
                         // non-referenceable node: always create new node id
-                        id = context.getNodeIdFactory().newNodeId();
                         break;
                     }
                     // use same uuid as source node
@@ -1645,7 +1635,6 @@ public class BatchedItemOperations exten
                 case CLONE_REMOVE_EXISTING:
                     if (!referenceable) {
                         // non-referenceable node: always create new node id
-                        id = context.getNodeIdFactory().newNodeId();
                         break;
                     }
                     // use same uuid as source node
@@ -1682,6 +1671,11 @@ public class BatchedItemOperations exten
                             "unknown flag for copying node state: " + flag);
             }
             newState = stateMgr.createNew(id, srcState.getNodeTypeName(), destParentId);
+            id = newState.getNodeId();
+            if (flag == COPY && referenceable) {
+                // remember uuid mapping
+                refTracker.mappedId(srcState.getNodeId(), id);
+            }
             // copy node state
             newState.setMixinTypeNames(srcState.getMixinTypeNames());
             if (shareable) {

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemImpl.java Thu Sep 22 09:27:48 2011
@@ -124,7 +124,7 @@ public abstract class ItemImpl implement
 
     protected abstract ItemState getOrCreateTransientItemState() throws RepositoryException;
 
-    protected abstract void makePersistent() throws InvalidItemStateException;
+    protected abstract void makePersistent() throws RepositoryException;
 
     /**
      * Marks this instance as 'removed' and notifies its listeners.

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/NodeImpl.java Thu Sep 22 09:27:48 2011
@@ -500,19 +500,8 @@ public class NodeImpl extends ItemImpl i
                                                     NodeId id)
             throws RepositoryException {
         // create a new node state
-        NodeState nodeState;
-        try {
-            if (id == null) {
-                id = sessionContext.getNodeIdFactory().newNodeId();
-            }
-            nodeState =
-                    stateMgr.createTransientNodeState(id, nodeType.getQName(),
-                            getNodeId(), ItemState.STATUS_NEW);
-        } catch (ItemStateException ise) {
-            String msg = "failed to add child node " + name + " to " + this;
-            log.debug(msg);
-            throw new RepositoryException(msg, ise);
-        }
+        NodeState nodeState = stateMgr.createTransientNodeState(
+                id, nodeType.getQName(), getNodeId(), ItemState.STATUS_NEW);
 
         // create Node instance wrapping new node state
         NodeImpl node;
@@ -852,7 +841,7 @@ public class NodeImpl extends ItemImpl i
     }
 
     @Override
-    protected void makePersistent() throws InvalidItemStateException {
+    protected void makePersistent() throws RepositoryException {
         if (!isTransient()) {
             log.debug(this + " (" + id + "): there's no transient state to persist");
             return;
@@ -864,9 +853,13 @@ public class NodeImpl extends ItemImpl i
         if (localState == null) {
             // this node is 'new'
             try {
-                localState = stateMgr.createNew(transientState);
+                localState = stateMgr.createNew(
+                        transientState.getNodeId(),
+                        transientState.getNodeTypeName(),
+                        transientState.getParentId());
+                transientState.connect(localState);
             } catch (ItemStateException e) {
-                throw new InvalidItemStateException(e);
+                throw new RepositoryException(e);
             }
         }
 
@@ -906,16 +899,12 @@ public class NodeImpl extends ItemImpl i
             // JCR-2503: Re-create transient state in the state manager,
             // because it was removed
             synchronized (data) {
-                try {
-                    thisState = stateMgr.createTransientNodeState(
-                            (NodeId) transientState.getId(),
-                            transientState.getNodeTypeName(),
-                            transientState.getParentId(),
-                            NodeState.STATUS_NEW);
-                    data.setState(thisState);
-                } catch (ItemStateException e) {
-                    throw new RepositoryException(e);
-                }
+                thisState = stateMgr.createTransientNodeState(
+                        (NodeId) transientState.getId(),
+                        transientState.getNodeTypeName(),
+                        transientState.getParentId(),
+                        NodeState.STATUS_NEW);
+                data.setState(thisState);
             }
         }
 

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java Thu Sep 22 09:27:48 2011
@@ -176,7 +176,7 @@ public abstract class ItemState {
     /**
      * Connect this state to some underlying overlayed state.
      */
-    protected void connect(ItemState overlayedState)
+    public void connect(ItemState overlayedState)
             throws ItemStateException {
         if (this.overlayedState != null
                 && this.overlayedState != overlayedState) {

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java Thu Sep 22 09:27:48 2011
@@ -16,7 +16,9 @@
  */
 package org.apache.jackrabbit.core.state;
 
+import javax.jcr.InvalidItemStateException;
 import javax.jcr.ReferentialIntegrityException;
+import javax.jcr.RepositoryException;
 
 import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.id.NodeId;
@@ -252,17 +254,29 @@ public class LocalItemStateManager
     /**
      * {@inheritDoc}
      */
-    public NodeState createNew(NodeId id, Name nodeTypeName,
-                               NodeId parentId)
-            throws IllegalStateException {
+    public NodeState createNew(
+            NodeId id, Name nodeTypeName, NodeId parentId)
+            throws RepositoryException {
         if (!editMode) {
-            throw new IllegalStateException("Not in edit mode");
+            throw new RepositoryException("Not in edit mode");
+        }
+
+        boolean nonRandomId = true;
+        if (id == null) {
+            id = getNodeIdFactory().newNodeId();
+            nonRandomId = false;
         }
 
-        NodeState state = new NodeState(id, nodeTypeName, parentId,
-                ItemState.STATUS_NEW, false);
+        NodeState state = new NodeState(
+                id, nodeTypeName, parentId, ItemState.STATUS_NEW, false);
         changeLog.added(state);
         state.setContainer(this);
+
+        if (nonRandomId && sharedStateMgr.hasItemState(id)) {
+            throw new InvalidItemStateException(
+                    "Node " + id + " already exists");
+        }
+
         return state;
     }
 

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java Thu Sep 22 09:27:48 2011
@@ -37,7 +37,6 @@ import org.apache.jackrabbit.core.Hierar
 import org.apache.jackrabbit.core.ZombieHierarchyManager;
 import org.apache.jackrabbit.core.id.ItemId;
 import org.apache.jackrabbit.core.id.NodeId;
-import org.apache.jackrabbit.core.id.NodeIdFactory;
 import org.apache.jackrabbit.core.id.PropertyId;
 import org.apache.jackrabbit.spi.Name;
 import org.slf4j.Logger;
@@ -54,7 +53,7 @@ public class SessionItemStateManager
     /**
      * State manager that allows updates
      */
-    private final UpdatableItemStateManager stateMgr;
+    private final LocalItemStateManager stateMgr;
 
     /**
      * Hierarchy manager
@@ -218,26 +217,13 @@ public class SessionItemStateManager
     /**
      * {@inheritDoc}
      */
-    public NodeState createNew(NodeId id, Name nodeTypeName,
-                               NodeId parentId)
-            throws IllegalStateException {
+    public NodeState createNew(
+            NodeId id, Name nodeTypeName, NodeId parentId)
+            throws RepositoryException {
         return stateMgr.createNew(id, nodeTypeName, parentId);
     }
 
     /**
-     * Customized variant of {@link #createNew(NodeId, Name, NodeId)} that
-     * connects the newly created persistent state with the transient state.
-     */
-    public NodeState createNew(NodeState transientState)
-            throws ItemStateException {
-        NodeState persistentState = createNew(transientState.getNodeId(),
-                transientState.getNodeTypeName(),
-                transientState.getParentId());
-        transientState.connect(persistentState);
-        return persistentState;
-    }
-
-    /**
      * {@inheritDoc}
      */
     public PropertyState createNew(Name propName, NodeId parentId)
@@ -560,21 +546,25 @@ public class SessionItemStateManager
      * @param parentId
      * @param initialStatus
      * @return
-     * @throws ItemStateException
+     * @throws RepositoryException
      */
     public NodeState createTransientNodeState(NodeId id, Name nodeTypeName, NodeId parentId, int initialStatus)
-            throws ItemStateException {
-
+            throws RepositoryException {
         // check map; synchronized to ensure an entry is not created twice.
         synchronized (transientStore) {
-            if (transientStore.containsKey(id)) {
-                String msg = "there's already a node state instance with id " + id;
-                log.debug(msg);
-                throw new ItemStateException(msg);
+            if (id == null) {
+                id = stateMgr.getNodeIdFactory().newNodeId();
+            } else if (initialStatus == ItemState.STATUS_NEW
+                    && hasItemState(id)) {
+                throw new InvalidItemStateException(
+                        "Node " + id + " already exists");
+            } else if (transientStore.containsKey(id)) {
+                throw new RepositoryException(
+                        "There is already a transient state for node " + id);
             }
 
-            NodeState state = new NodeState(id, nodeTypeName, parentId,
-                    initialStatus, true);
+            NodeState state = new NodeState(
+                    id, nodeTypeName, parentId, initialStatus, true);
             // put transient state in the map
             transientStore.put(state.getId(), state);
             state.setContainer(this);
@@ -969,8 +959,4 @@ public class SessionItemStateManager
         }
     }
 
-    public NodeIdFactory getNodeIdFactory() {
-        return stateMgr.getNodeIdFactory();
-    }
-
 }

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java Thu Sep 22 09:27:48 2011
@@ -18,9 +18,9 @@ package org.apache.jackrabbit.core.state
 
 import org.apache.jackrabbit.spi.Name;
 import org.apache.jackrabbit.core.id.NodeId;
-import org.apache.jackrabbit.core.id.NodeIdFactory;
 
 import javax.jcr.ReferentialIntegrityException;
+import javax.jcr.RepositoryException;
 
 /**
  * Identifies an <code>ItemStateManager</code> that allows updating
@@ -53,14 +53,15 @@ public interface UpdatableItemStateManag
      * i.e. not yet existing state. Call {@link #store}
      * on the returned object to make it persistent.
      *
-     * @param id           the id of the node
+     * @param id the id of the node, or <code>null</code> for a new node id
      * @param nodeTypeName The node type name
      * @param parentId     parent node's id
      * @return a node state
-     * @throws IllegalStateException if the manager is not in edit mode.
+     * @throws RepositoryException if the node state can not be created
      */
-    NodeState createNew(NodeId id, Name nodeTypeName,
-                        NodeId parentId) throws IllegalStateException;
+    NodeState createNew(
+            NodeId id, Name nodeTypeName, NodeId parentId)
+            throws RepositoryException;
 
     /**
      * Creates a {@link PropertyState} instance representing new,
@@ -122,6 +123,4 @@ public interface UpdatableItemStateManag
      */
     void dispose();
 
-    NodeIdFactory getNodeIdFactory();
-
 }

Modified: jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
URL: http://svn.apache.org/viewvc/jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java?rev=1174016&r1=1174015&r2=1174016&view=diff
==============================================================================
--- jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java (original)
+++ jackrabbit/branches/JCR-2272/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java Thu Sep 22 09:27:48 2011
@@ -558,9 +558,6 @@ public class NodeStateEx {
             throws RepositoryException {
         NodeId parentId = nodeState.getNodeId();
         // create a new node state
-        if (id == null) {
-            id = stateMgr.getNodeIdFactory().newNodeId();
-        }
         NodeState state = stateMgr.createNew(id, nodeTypeName, parentId);
 
         // create Node instance wrapping new node state
@@ -568,7 +565,7 @@ public class NodeStateEx {
         node.setPropertyValue(NameConstants.JCR_PRIMARYTYPE, InternalValue.create(nodeTypeName));
 
         // add new child node entry
-        nodeState.addChildNodeEntry(name, id);
+        nodeState.addChildNodeEntry(name, state.getNodeId());
         if (nodeState.getStatus() == ItemState.STATUS_EXISTING) {
             nodeState.setStatus(ItemState.STATUS_EXISTING_MODIFIED);
         }