jackrabbit-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1176546 - in /jackrabbit/trunk: ./ jackrabbit-core/ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/ jackrabbit-core/src/main/java/org/apache/jackrabbit/co...
Date Tue, 27 Sep 2011 19:33:02 GMT
Author: jukka
Date: Tue Sep 27 19:33:01 2011
New Revision: 1176546

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

Merge changes back to trunk.

Added:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java
Modified:
    jackrabbit/trunk/   (props changed)
    jackrabbit/trunk/jackrabbit-core/pom.xml
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.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/ItemManager.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/PropertyImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SharedItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentImportTest.java
    jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/TestAll.java

Propchange: jackrabbit/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Tue Sep 27 19:33:01 2011
@@ -1 +1,2 @@
+/jackrabbit/branches/JCR-2272:1173165-1176545
 /jackrabbit/sandbox/JCR-2415-lucene-3.0:1060860-1064038

Modified: jackrabbit/trunk/jackrabbit-core/pom.xml
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/pom.xml?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/pom.xml (original)
+++ jackrabbit/trunk/jackrabbit-core/pom.xml Tue Sep 27 19:33:01 2011
@@ -100,7 +100,6 @@
               <name>known.issues</name>
               <value>
 org.apache.jackrabbit.core.xml.DocumentViewTest#testMultiValue
-org.apache.jackrabbit.core.ConcurrentImportTest
 org.apache.jackrabbit.core.NodeImplTest#testReferentialIntegrityCorruptionGetPath
 org.apache.jackrabbit.core.integration.ConcurrentQueryTest#testConcurrentQueryWithDeletes
 org.apache.jackrabbit.test.api.ShareableNodeTest#testGetName

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=1176546&r1=1176545&r2=1176546&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
Tue Sep 27 19:33:01 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/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManagerImpl.java
Tue Sep 27 19:33:01 2011
@@ -464,7 +464,7 @@ public class HierarchyManagerImpl implem
         if (entry == null) {
             String msg = "failed to resolve name of " + id;
             log.debug(msg);
-            throw new RepositoryException(msg);
+            throw new ItemNotFoundException(msg);
         }
         return entry.getName();
     }

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=1176546&r1=1176545&r2=1176546&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
Tue Sep 27 19:33:01 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/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/ItemManager.java
Tue Sep 27 19:33:01 2011
@@ -1105,7 +1105,6 @@ public class ItemManager implements Item
             itemDestroyed(destroyed.getId(), data);
 
             data.setStatus(ItemImpl.STATUS_DESTROYED);
-            data.setState(null);
         }
     }
 

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=1176546&r1=1176545&r2=1176546&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
Tue Sep 27 19:33:01 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,29 +841,15 @@ 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;
         }
 
         NodeState transientState = data.getNodeState();
+        NodeState localState = stateMgr.makePersistent(transientState);
 
-        NodeState localState = (NodeState) transientState.getOverlayedState();
-        if (localState == null) {
-            // this node is 'new'
-            localState = stateMgr.createNew(transientState);
-        }
-
-        synchronized (localState) {
-            // copy state from transient state:
-            localState.copy(transientState, true);
-            // make state persistent
-            stateMgr.store(localState);
-        }
-
-        // tell state manager to disconnect item state
-        stateMgr.disconnectTransientItemState(transientState);
         // swap transient state with local state
         data.setState(localState);
         // reset status
@@ -902,16 +877,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/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/PropertyImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/PropertyImpl.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/PropertyImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/PropertyImpl.java
Tue Sep 27 19:33:01 2011
@@ -144,7 +144,11 @@ public class PropertyImpl extends ItemIm
         PropertyState persistentState = (PropertyState) transientState.getOverlayedState();
         if (persistentState == null) {
             // this property is 'new'
-            persistentState = stateMgr.createNew(transientState);
+            try {
+                persistentState = stateMgr.createNew(transientState);
+            } catch (ItemStateException e) {
+                throw new InvalidItemStateException(e);
+            }
         }
 
         synchronized (persistentState) {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/persistence/pool/BundleDbPersistenceManager.java
Tue Sep 27 19:33:01 2011
@@ -493,6 +493,10 @@ public class BundleDbPersistenceManager
                 } catch (SQLException e2) {
                     DbUtility.logException("rollback failed", e2);
                 }
+
+                // if we got here due to a constraint violation and we
+                // are running in test mode, we really want to stop
+                assert !isIntegrityConstraintViolation(e.getCause());
             }
             failures++;
             log.error("Failed to persist ChangeLog (stacktrace on DEBUG log level), blockOnConnectionLoss
= "
@@ -511,6 +515,15 @@ public class BundleDbPersistenceManager
         throw lastException;
     }
 
+    private boolean isIntegrityConstraintViolation(Throwable t) {
+        if (t instanceof SQLException) {
+            String state = ((SQLException) t).getSQLState();
+            return state != null && state.startsWith("23");
+        } else {
+            return false;
+        }
+    }
+
     /**
      * {@inheritDoc}
      */
@@ -1103,11 +1116,21 @@ public class BundleDbPersistenceManager
             Object[] params = createParams(bundle.getId(), out.toByteArray(), true);
             conHelper.update(sql, params);
         } catch (Exception e) {
-            String msg = "failed to write bundle: " + bundle.getId();
+            String msg;
+
+            if (isIntegrityConstraintViolation(e)) {
+                // we should never get an integrity constraint violation here
+                // other PMs may not be able to detect this and end up with
+                // corrupted data
+                msg = "FATAL error while writing the bundle: " + bundle.getId();
+            } else {
+                msg = "failed to write bundle: " + bundle.getId();
+            }
+
             log.error(msg, e);
             throw new ItemStateException(msg, e);
         }
-    }
+   }
 
     /**
      * {@inheritDoc}

Added: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java?rev=1176546&view=auto
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java
(added)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/DummyUpdateEventChannel.java
Tue Sep 27 19:33:01 2011
@@ -0,0 +1,39 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.jackrabbit.core.state;
+
+import org.apache.jackrabbit.core.cluster.Update;
+import org.apache.jackrabbit.core.cluster.UpdateEventChannel;
+import org.apache.jackrabbit.core.cluster.UpdateEventListener;
+
+/**
+ * Package-private utility class used as a sentinel by the
+ * {@link SharedItemStateManager} class.
+ */
+class DummyUpdateEventChannel implements UpdateEventChannel {
+
+    public void updatePrepared(Update update) {}
+
+    public void updateCreated(Update update) {}
+
+    public void updateCommitted(Update update, String path) {}
+
+    public void updateCancelled(Update update) {}
+
+    public void setListener(UpdateEventListener listener) {}
+
+}
\ No newline at end of file

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/ItemState.java
Tue Sep 27 19:33:01 2011
@@ -132,7 +132,7 @@ public abstract class ItemState {
                 throw new IllegalArgumentException(msg);
         }
         this.isTransient = isTransient;
-        connect(overlayedState);
+        this.overlayedState = overlayedState;
     }
 
     /**
@@ -176,11 +176,13 @@ public abstract class ItemState {
     /**
      * Connect this state to some underlying overlayed state.
      */
-    protected void connect(ItemState overlayedState) {
-        if (this.overlayedState != null) {
-            if (this.overlayedState != overlayedState) {
-                throw new IllegalStateException("Item state already connected to another
underlying state: " + this);
-            }
+    public void connect(ItemState overlayedState)
+            throws ItemStateException {
+        if (this.overlayedState != null
+                && this.overlayedState != overlayedState) {
+            throw new ItemStateException(
+                    "Item state already connected to another"
+                            + " underlying state: " + this);
         }
         this.overlayedState = overlayedState;
     }
@@ -189,9 +191,9 @@ public abstract class ItemState {
      * Reconnect this state to the overlayed state that it has been
      * disconnected from earlier.
      */
-    protected void reconnect() {
+    protected void reconnect() throws ItemStateException {
         if (this.overlayedState == null) {
-            throw new IllegalStateException(
+            throw new ItemStateException(
                     "Item state cannot be reconnected because there's no"
                     + " underlying state to reconnect to: " + this);
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/LocalItemStateManager.java
Tue Sep 27 19:33:01 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,21 +254,66 @@ 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 && !changeLog.deleted(id)
+                && sharedStateMgr.hasItemState(id)) {
+            throw new InvalidItemStateException(
+                    "Node " + id + " already exists");
+        }
+
         return state;
     }
 
     /**
+     * Returns the local node state below the given transient one. If given
+     * a fresh new node state, then a new local state is created and added
+     * to the change log.
+     *
+     * @param transientState transient state
+     * @return local node state
+     * @throws RepositoryException if the local state could not be created
+     */
+    public NodeState getOrCreateLocalState(NodeState transientState)
+            throws RepositoryException {
+        NodeState localState = (NodeState) transientState.getOverlayedState();
+        if (localState == null) {
+            // The transient node state is new, create a new local state
+            localState = new NodeState(
+                    transientState.getNodeId(),
+                    transientState.getNodeTypeName(),
+                    transientState.getParentId(),
+                    ItemState.STATUS_NEW,
+                    false);
+            changeLog.added(localState);
+            localState.setContainer(this);
+            try {
+                transientState.connect(localState);
+            } catch (ItemStateException e) {
+                // should never happen
+                throw new RepositoryException(e);
+            }
+        }
+        return localState;
+    }
+
+    /**
      * {@inheritDoc}
      */
     public PropertyState createNew(Name propName, NodeId parentId)
@@ -408,10 +455,20 @@ public class LocalItemStateManager
             try {
                 local = changeLog.get(created.getId());
                 if (local != null) {
-                    // underlying state has been permanently created
-                    local.pull();
-                    local.setStatus(ItemState.STATUS_EXISTING);
-                    cache.cache(local);
+                    if (local.isNode() && local.getOverlayedState() != created) {
+                        // mid-air collision of concurrent node state creation
+                        // with same id (JCR-2272)
+                        if (local.getStatus() == ItemState.STATUS_NEW) {
+                            local.setStatus(ItemState.STATUS_UNDEFINED); // we need a state
that is != NEW
+                        }
+                    } else {
+                        if (local.getOverlayedState() == created) {
+                            // underlying state has been permanently created
+                            local.pull();
+                            local.setStatus(ItemState.STATUS_EXISTING);
+                            cache.cache(local);
+                        }
+                    }
                 }
             } catch (NoSuchItemStateException e) {
                 /* ignore */

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/SessionItemStateManager.java
Tue Sep 27 19:33:01 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,27 +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 IllegalStateException {
-
-        NodeState persistentState = createNew(transientState.getNodeId(),
-                transientState.getNodeTypeName(),
-                transientState.getParentId());
-        transientState.connect(persistentState);
-        return persistentState;
-    }
-
-    /**
      * {@inheritDoc}
      */
     public PropertyState createNew(Name propName, NodeId parentId)
@@ -251,8 +236,7 @@ public class SessionItemStateManager
      * connects the newly created persistent state with the transient state.
      */
     public PropertyState createNew(PropertyState transientState)
-            throws IllegalStateException {
-
+            throws ItemStateException {
         PropertyState persistentState = createNew(transientState.getName(),
                 transientState.getParentId());
         transientState.connect(persistentState);
@@ -565,20 +549,27 @@ 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 {
+        if (initialStatus == ItemState.STATUS_NEW && id != null
+                && hasItemState(id)) {
+            throw new InvalidItemStateException(
+                    "Node " + id + " already exists");
+        }
+
         // 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 (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);
@@ -973,8 +964,31 @@ public class SessionItemStateManager
         }
     }
 
-    public NodeIdFactory getNodeIdFactory() {
-        return stateMgr.getNodeIdFactory();
+    /**
+     * Pushes the given transient state to the change log so it'll be
+     * persisted when the change log is committed. The transient state
+     * is replaced with the local state that has been pushed to the
+     * change log.
+     *
+     * @param transientState transient state
+     * @return the local state to be persisted
+     * @throws RepositoryException if the transiet state can not be persisted
+     */
+    public NodeState makePersistent(NodeState transientState)
+            throws RepositoryException {
+        NodeState localState = stateMgr.getOrCreateLocalState(transientState);
+
+        synchronized (localState) {
+            // copy state from transient state:
+            localState.copy(transientState, true);
+            // make state persistent
+            store(localState);
+        }
+
+        // disconnect the transient item state
+        disconnectTransientItemState(transientState);
+
+        return localState;
     }
 
 }

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=1176546&r1=1176545&r2=1176546&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
Tue Sep 27 19:33:01 2011
@@ -178,9 +178,13 @@ public class SharedItemStateManager
     private ISMLocking ismLocking;
 
     /**
-     * Update event channel.
+     * Update event channel. By default this is a dummy channel that simply
+     * ignores all events (so we don't need to check for null all the time),
+     * but in clustered environments the
+     * {@link #setEventChannel(UpdateEventChannel)} method should be called
+     * during initialization to connect this SISM instance with the cluster.
      */
-    private UpdateEventChannel eventChannel;
+    private UpdateEventChannel eventChannel = new DummyUpdateEventChannel();
 
     private final NodeIdFactory nodeIdFactory;
 
@@ -557,15 +561,13 @@ public class SharedItemStateManager
 
             virtualNodeReferences = new ChangeLog[virtualProviders.length];
 
-            /* let listener know about change */
-            if (eventChannel != null) {
-                eventChannel.updateCreated(this);
-            }
+            // let listener know about change
+            eventChannel.updateCreated(this);
 
             try {
                 writeLock = acquireWriteLock(local);
             } finally {
-                if (writeLock == null && eventChannel != null) {
+                if (writeLock == null) {
                     eventChannel.updateCancelled(this);
                 }
             }
@@ -683,6 +685,16 @@ public class SharedItemStateManager
                     shared.deleted(state.getOverlayedState());
                 }
                 for (ItemState state : local.addedStates()) {
+                    if (state.isNode() && state.getStatus() != ItemState.STATUS_NEW)
{
+                        // another node with same id had been created
+                        // in the meantime, probably caused by mid-air collision
+                        // of concurrent versioning operations (JCR-2272)
+                        String msg = state.getId()
+                                + " has been created externally  (status "
+                                + state.getStatus() + ")";
+                        log.debug(msg);
+                        throw new StaleItemStateException(msg);
+                    }
                     state.connect(createInstance(state));
                     shared.added(state.getOverlayedState());
                 }
@@ -716,10 +728,8 @@ public class SharedItemStateManager
                 /* create event states */
                 events.createEventStates(rootNodeId, local, SharedItemStateManager.this);
 
-                /* let listener know about change */
-                if (eventChannel != null) {
-                    eventChannel.updatePrepared(this);
-                }
+                // let listener know about change
+                eventChannel.updatePrepared(this);
 
                 if (VALIDATE_HIERARCHY) {
                     log.info("Validating change-set hierarchy");
@@ -789,13 +799,15 @@ public class SharedItemStateManager
 
                 /* dispatch the events */
                 events.dispatch();
-
-                /* let listener know about finished operation */
-                if (eventChannel != null) {
-                    String path = events.getSession().getUserID() + "@" + events.getCommonPath();
-                    eventChannel.updateCommitted(this, path);
-                }
             } finally {
+                // Let listener know about finished operation. This needs
+                // to happen in the finally block so that the cluster lock
+                // always gets released, even if a post-store() exception
+                // is thrown from the code above. See also JCR-2272.
+                String path = events.getSession().getUserID()
+                        + "@" + events.getCommonPath();
+                eventChannel.updateCommitted(this, path);
+
                 if (writeLock != null) {
                     // exception occurred before downgrading lock
                     writeLock.release();
@@ -812,10 +824,8 @@ public class SharedItemStateManager
          */
         public void cancel() {
             try {
-                /* let listener know about canceled operation */
-                if (eventChannel != null) {
-                    eventChannel.updateCancelled(this);
-                }
+                // let listener know about canceled operation
+                eventChannel.updateCancelled(this);
 
                 local.disconnect();
 

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/UpdatableItemStateManager.java
Tue Sep 27 19:33:01 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/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/InternalVersionManagerImpl.java
Tue Sep 27 19:33:01 2011
@@ -22,6 +22,7 @@ import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
+import javax.jcr.InvalidItemStateException;
 import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
@@ -258,7 +259,8 @@ public class InternalVersionManagerImpl 
         });
 
         if (state == null) {
-            throw new VersionException("History already exists for node " + node.getNodeId());
+            throw new InvalidItemStateException(
+                    "History already exists for node " + node.getNodeId());
         }
         Name root = NameConstants.JCR_ROOTVERSION;
         return new VersionHistoryInfo(

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/version/NodeStateEx.java
Tue Sep 27 19:33:01 2011
@@ -427,7 +427,9 @@ public class NodeStateEx {
                 ItemState state = stateMgr.getItemState(propId);
                 stateMgr.destroy(state);
                 nodeState.removePropertyName(name);
-                nodeState.setStatus(ItemState.STATUS_EXISTING_MODIFIED);
+                if (nodeState.getStatus() != ItemState.STATUS_NEW) {
+                    nodeState.setStatus(ItemState.STATUS_EXISTING_MODIFIED);
+                }
                 return true;
             }
         } catch (ItemStateException e) {
@@ -556,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
@@ -566,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);
         }

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentImportTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentImportTest.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentImportTest.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/ConcurrentImportTest.java
Tue Sep 27 19:33:01 2011
@@ -20,18 +20,18 @@ import java.util.UUID;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
-import javax.jcr.RepositoryException;
-import javax.jcr.Session;
-import javax.jcr.Node;
 import javax.jcr.ImportUUIDBehavior;
 import javax.jcr.InvalidItemStateException;
+import javax.jcr.Node;
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
 
+import org.apache.jackrabbit.JcrConstants;
+import org.apache.jackrabbit.spi.Name;
+import org.xml.sax.Attributes;
 import org.xml.sax.ContentHandler;
 import org.xml.sax.SAXException;
-import org.xml.sax.Attributes;
 import org.xml.sax.helpers.AttributesImpl;
-import org.apache.jackrabbit.spi.Name;
-import org.apache.jackrabbit.JcrConstants;
 
 /**
  * <code>ConcurrentVersioningTest</code> contains test cases that run version
@@ -87,10 +87,13 @@ public class ConcurrentImportTest extend
                         }
                         try {
                             session.refresh(false);
-                            addNode(test, uuid, JcrConstants.NT_UNSTRUCTURED, uuid, mixins);
                             try {
+                                addNode(test, uuid,
+                                        JcrConstants.NT_UNSTRUCTURED, uuid,
+                                        mixins);
                                 session.save();
-                                log.println("Added " + test.getPath() + "/" + uuid);
+                                log.println("Added " + test.getPath() + "/"
+                                        + uuid);
                                 log.flush();
                             } catch (InvalidItemStateException e) {
                                 log.println("Ignoring expected error: " + e.toString());

Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/TestAll.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/TestAll.java?rev=1176546&r1=1176545&r2=1176546&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/TestAll.java
(original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/TestAll.java
Tue Sep 27 19:33:01 2011
@@ -48,8 +48,7 @@ public class TestAll extends TestCase {
         suite.addTestSuite(ReplaceTest.class);
 
         // test related to NodeStateMerger
-        // temporarily disabled see JCR-2272 and JCR-2295
-        // suite.addTestSuite(ConcurrentImportTest.class);
+        suite.addTestSuite(ConcurrentImportTest.class);
         suite.addTestSuite(ConcurrentAddRemoveMoveTest.class);
         suite.addTestSuite(ConcurrentAddRemovePropertyTest.class);
         suite.addTestSuite(ConcurrentMixinModificationTest.class);



Mime
View raw message