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/27 21:33:02 UTC
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...
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);