You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by st...@apache.org on 2008/03/07 15:10:28 UTC
svn commit: r634681 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/
main/java/org/apache/jackrabbit/core/state/
test/java/org/apache/jackrabbit/core/integration/
Author: stefan
Date: Fri Mar 7 06:10:27 2008
New Revision: 634681
URL: http://svn.apache.org/viewvc?rev=634681&view=rev
Log:
JCR-1359: Adding nodes from concurrently running sessions cause exceptions
- added synchronization to NodeStateMerger.merge method
- fixed @todo issue in NodeStateMerger.merge method (checking node type sameNameSibling setting)
- enabling execution of ConcurrentNodeModificationTest
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.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/test/java/org/apache/jackrabbit/core/integration/MultiThreadingTest.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java?rev=634681&r1=634680&r2=634681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/SessionImpl.java Fri Mar 7 06:10:27 2008
@@ -270,7 +270,8 @@
* @return session item state manager
*/
protected SessionItemStateManager createSessionItemStateManager(LocalItemStateManager manager) {
- return new SessionItemStateManager(rep.getRootNodeId(), manager, this);
+ return new SessionItemStateManager(
+ rep.getRootNodeId(), manager, this, rep.getNodeTypeRegistry());
}
/**
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java?rev=634681&r1=634680&r2=634681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/state/NodeStateMerger.java Fri Mar 7 06:10:27 2008
@@ -18,6 +18,7 @@
import org.apache.jackrabbit.core.ItemId;
import org.apache.jackrabbit.core.PropertyId;
+import org.apache.jackrabbit.core.NodeId;
import org.apache.jackrabbit.spi.Name;
import java.util.ArrayList;
@@ -53,148 +54,154 @@
return false;
}
- /**
- * some examples for trivial non-conflicting changes:
- * - s1 added child node a, s2 removes child node b
- * - s1 adds child node a, s2 adds child node b
- * - s1 adds child node a, s2 adds mixin type
- *
- * conflicting changes causing staleness:
- * - s1 added non-sns child node or property a,
- * s2 added non-sns child node or property a => name clash
- * - either session reordered child nodes
- * (some combinations could possibly be merged)
- * - either session moved node
- */
-
- // compare current transient state with externally modified
- // overlayed state and determine what has been changed by whom
-
- // child node entries order
- if (!state.getReorderedChildNodeEntries().isEmpty()) {
- // for now we don't even try to merge the result of
- // a reorder operation
- return false;
- }
+ synchronized (overlayedState) {
+ synchronized (state) {
+ /**
+ * some examples for trivial non-conflicting changes:
+ * - s1 added child node a, s2 removes child node b
+ * - s1 adds child node a, s2 adds child node b
+ * - s1 adds child node a, s2 adds mixin type
+ *
+ * conflicting changes causing staleness:
+ * - s1 added non-sns child node or property a,
+ * s2 added non-sns child node or property a => name clash
+ * - either session reordered child nodes
+ * (some combinations could possibly be merged)
+ * - either session moved node
+ */
+
+ // compare current transient state with externally modified
+ // overlayed state and determine what has been changed by whom
+
+ // child node entries order
+ if (!state.getReorderedChildNodeEntries().isEmpty()) {
+ // for now we don't even try to merge the result of
+ // a reorder operation
+ return false;
+ }
- // mixin types
- if (!state.getMixinTypeNames().equals(overlayedState.getMixinTypeNames())) {
- // the mixins have been modified but by just looking at the diff we
- // can't determine where the change happened since the diffs of either
- // removing a mixin from the overlayed or adding a mixin to the
- // transient state would look identical...
- return false;
- }
+ // mixin types
+ if (!state.getMixinTypeNames().equals(overlayedState.getMixinTypeNames())) {
+ // the mixins have been modified but by just looking at the diff we
+ // can't determine where the change happened since the diffs of either
+ // removing a mixin from the overlayed or adding a mixin to the
+ // transient state would look identical...
+ return false;
+ }
- // parent id
- if (state.getParentId() != null
- && !state.getParentId().equals(overlayedState.getParentId())) {
- return false;
- }
+ // parent id
+ if (state.getParentId() != null
+ && !state.getParentId().equals(overlayedState.getParentId())) {
+ return false;
+ }
+
+ // child node entries
+ if (!state.getChildNodeEntries().equals(
+ overlayedState.getChildNodeEntries())) {
+ ArrayList added = new ArrayList();
+ ArrayList removed = new ArrayList();
+
+ for (Iterator iter = state.getAddedChildNodeEntries().iterator();
+ iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+
+ if (context.isAdded(cne.getId())) {
+ // a new child node entry has been added to this state;
+ // check for name collisions with other state
+ if (overlayedState.hasChildNodeEntry(cne.getName())) {
+ // conflicting names
+ if (cne.getIndex() < 2) {
+ // check if same-name siblings are allowed
+ if (!context.allowsSameNameSiblings(cne.getId())) {
+ return false;
+ }
+ }
+ // assume same-name siblings are allowed since index is >= 2
+ }
- // child node entries
- if (!state.getChildNodeEntries().equals(
- overlayedState.getChildNodeEntries())) {
- ArrayList added = new ArrayList();
- ArrayList removed = new ArrayList();
-
- for (Iterator iter = state.getAddedChildNodeEntries().iterator();
- iter.hasNext();) {
- NodeState.ChildNodeEntry cne =
- (NodeState.ChildNodeEntry) iter.next();
-
- if (context.isAdded(cne.getId())) {
- // a new child node entry has been added to this state;
- // check for name collisions with other state
- if (overlayedState.hasChildNodeEntry(cne.getName())) {
- // conflicting names
- if (cne.getIndex() < 2) {
- // todo check if same-name siblings are allowed
- return false;
+ added.add(cne);
}
- // assume same-name siblings are allowed since index is >= 2
}
- added.add(cne);
- }
- }
+ for (Iterator iter = state.getRemovedChildNodeEntries().iterator();
+ iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ if (context.isDeleted(cne.getId())) {
+ // a child node entry has been removed from this node state
+ removed.add(cne);
+ }
+ }
- for (Iterator iter = state.getRemovedChildNodeEntries().iterator();
- iter.hasNext();) {
- NodeState.ChildNodeEntry cne =
- (NodeState.ChildNodeEntry) iter.next();
- if (context.isDeleted(cne.getId())) {
- // a child node entry has been removed from this node state
- removed.add(cne);
+ // copy child node antries from other state and
+ // re-apply changes made on this state
+ state.setChildNodeEntries(overlayedState.getChildNodeEntries());
+ for (Iterator iter = added.iterator(); iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ state.addChildNodeEntry(cne.getName(), cne.getId());
+ }
+ for (Iterator iter = removed.iterator(); iter.hasNext();) {
+ NodeState.ChildNodeEntry cne =
+ (NodeState.ChildNodeEntry) iter.next();
+ state.removeChildNodeEntry(cne.getId());
+ }
}
- }
- // copy child node antries from other state and
- // re-apply changes made on this state
- state.setChildNodeEntries(overlayedState.getChildNodeEntries());
- for (Iterator iter = added.iterator(); iter.hasNext();) {
- NodeState.ChildNodeEntry cne =
- (NodeState.ChildNodeEntry) iter.next();
- state.addChildNodeEntry(cne.getName(), cne.getId());
- }
- for (Iterator iter = removed.iterator(); iter.hasNext();) {
- NodeState.ChildNodeEntry cne =
- (NodeState.ChildNodeEntry) iter.next();
- state.removeChildNodeEntry(cne.getId());
- }
- }
+ // property names
+ if (!state.getPropertyNames().equals(
+ overlayedState.getPropertyNames())) {
+ HashSet added = new HashSet();
+ HashSet removed = new HashSet();
+
+ for (Iterator iter = state.getAddedPropertyNames().iterator();
+ iter.hasNext();) {
+ Name name = (Name) iter.next();
+ PropertyId propId =
+ new PropertyId(state.getNodeId(), name);
+ if (context.isAdded(propId)) {
+ // a new property name has been added to this state;
+ // check for name collisions
+ if (overlayedState.hasPropertyName(name)
+ || overlayedState.hasChildNodeEntry(name)) {
+ // conflicting names
+ return false;
+ }
- // property names
- if (!state.getPropertyNames().equals(
- overlayedState.getPropertyNames())) {
- HashSet added = new HashSet();
- HashSet removed = new HashSet();
-
- for (Iterator iter = state.getAddedPropertyNames().iterator();
- iter.hasNext();) {
- Name name = (Name) iter.next();
- PropertyId propId =
- new PropertyId(state.getNodeId(), name);
- if (context.isAdded(propId)) {
- // a new property name has been added to this state;
- // check for name collisions
- if (overlayedState.hasPropertyName(name)
- || overlayedState.hasChildNodeEntry(name)) {
- // conflicting names
- return false;
+ added.add(name);
+ }
+ }
+ for (Iterator iter = state.getRemovedPropertyNames().iterator();
+ iter.hasNext();) {
+ Name name = (Name) iter.next();
+ PropertyId propId =
+ new PropertyId(state.getNodeId(), name);
+ if (context.isDeleted(propId)) {
+ // a property name has been removed from this state
+ removed.add(name);
+ }
}
- added.add(name);
- }
- }
- for (Iterator iter = state.getRemovedPropertyNames().iterator();
- iter.hasNext();) {
- Name name = (Name) iter.next();
- PropertyId propId =
- new PropertyId(state.getNodeId(), name);
- if (context.isDeleted(propId)) {
- // a property name has been removed from this state
- removed.add(name);
+ // copy property names from other and
+ // re-apply changes made on this state
+ state.setPropertyNames(overlayedState.getPropertyNames());
+ for (Iterator iter = added.iterator(); iter.hasNext();) {
+ Name name = (Name) iter.next();
+ state.addPropertyName(name);
+ }
+ for (Iterator iter = removed.iterator(); iter.hasNext();) {
+ Name name = (Name) iter.next();
+ state.removePropertyName(name);
+ }
}
- }
- // copy property names from other and
- // re-apply changes made on this state
- state.setPropertyNames(overlayedState.getPropertyNames());
- for (Iterator iter = added.iterator(); iter.hasNext();) {
- Name name = (Name) iter.next();
- state.addPropertyName(name);
- }
- for (Iterator iter = removed.iterator(); iter.hasNext();) {
- Name name = (Name) iter.next();
- state.removePropertyName(name);
+ // finally sync modification count
+ state.setModCount(overlayedState.getModCount());
+
+ return true;
}
}
-
- // finally sync modification count
- state.setModCount(overlayedState.getModCount());
-
- return true;
}
//-----------------------------------------------------< inner interfaces >
@@ -202,5 +209,6 @@
static interface MergeContext {
boolean isAdded(ItemId id);
boolean isDeleted(ItemId id);
+ boolean allowsSameNameSiblings(NodeId id);
}
}
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=634681&r1=634680&r2=634681&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 Fri Mar 7 06:10:27 2008
@@ -23,6 +23,8 @@
import org.apache.jackrabbit.core.NodeId;
import org.apache.jackrabbit.core.PropertyId;
import org.apache.jackrabbit.core.ZombieHierarchyManager;
+import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
+import org.apache.jackrabbit.core.nodetype.NodeDef;
import org.apache.jackrabbit.core.util.Dumpable;
import org.apache.jackrabbit.spi.commons.conversion.PathResolver;
import org.apache.jackrabbit.spi.Name;
@@ -76,6 +78,11 @@
private AtticItemStateManager attic;
/**
+ * Node Type Registry
+ */
+ private final NodeTypeRegistry ntReg;
+
+ /**
* State change dispatcher.
*/
private final transient StateChangeDispatcher dispatcher = new StateChangeDispatcher();
@@ -86,10 +93,12 @@
* @param rootNodeId the root node id
* @param stateMgr the local item state manager
* @param resolver path resolver for outputting user-friendly paths
+ * @param ntReg node type registry
*/
public SessionItemStateManager(NodeId rootNodeId,
LocalItemStateManager stateMgr,
- PathResolver resolver) {
+ PathResolver resolver,
+ NodeTypeRegistry ntReg) {
transientStore = new ItemStateMap();
atticStore = new ItemStateMap();
@@ -99,6 +108,8 @@
// create hierarchy manager that uses both transient and persistent state
hierMgr = new CachingHierarchyManager(rootNodeId, this, resolver);
addListener(hierMgr);
+
+ this.ntReg = ntReg;
}
/**
@@ -815,6 +826,17 @@
public boolean isDeleted(ItemId id) {
return atticStore.contains(id);
+ }
+
+ public boolean allowsSameNameSiblings(NodeId id) {
+ NodeState ns;
+ try {
+ ns = (NodeState) getItemState(id);
+ } catch (ItemStateException e) {
+ return false;
+ }
+ NodeDef def = ntReg.getNodeDef(ns.getDefinitionId());
+ return def != null ? def.allowsSameNameSiblings() : false;
}
};
if (NodeStateMerger.merge((NodeState) transientState, context)) {
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=634681&r1=634680&r2=634681&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 Fri Mar 7 06:10:27 2008
@@ -28,6 +28,7 @@
import org.apache.jackrabbit.core.nodetype.NodeTypeConflictException;
import org.apache.jackrabbit.core.nodetype.NodeTypeRegistry;
import org.apache.jackrabbit.core.nodetype.PropDef;
+import org.apache.jackrabbit.core.nodetype.NodeDef;
import org.apache.jackrabbit.core.observation.EventStateCollection;
import org.apache.jackrabbit.core.observation.EventStateCollectionFactory;
import org.apache.jackrabbit.core.util.Dumpable;
@@ -614,6 +615,21 @@
public boolean isDeleted(ItemId id) {
return local.deleted(id);
+ }
+
+ public boolean allowsSameNameSiblings(NodeId id) {
+ NodeState ns;
+ try {
+ if (local.has(id)) {
+ ns = (NodeState) local.get(id);
+ } else {
+ ns = (NodeState) getItemState(id);
+ }
+ } catch (ItemStateException e) {
+ return false;
+ }
+ NodeDef def = ntReg.getNodeDef(ns.getDefinitionId());
+ return def != null ? def.allowsSameNameSiblings() : false;
}
};
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/integration/MultiThreadingTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/integration/MultiThreadingTest.java?rev=634681&r1=634680&r2=634681&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/integration/MultiThreadingTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/integration/MultiThreadingTest.java Fri Mar 7 06:10:27 2008
@@ -41,7 +41,7 @@
suite.addTestSuite(ConcurrencyTest.class);
suite.addTestSuite(ConcurrentLoginTest.class);
- //suite.addTestSuite(ConcurrentNodeModificationTest.class);
+ suite.addTestSuite(ConcurrentNodeModificationTest.class);
suite.addTestSuite(ConcurrentReadWriteTest.class);
suite.addTestSuite(ConcurrentSaveTest.class);
suite.addTestSuite(ConcurrentVersioningTest.class);