[dna-commits] DNA SVN: r896 - trunk/dna-jcr/src/main/java/org/jboss/dna/jcr.

dna-commits at lists.jboss.org dna-commits at lists.jboss.org
Thu May 7 10:30:53 EDT 2009


Author: rhauch
Date: 2009-05-07 10:30:53 -0400 (Thu, 07 May 2009)
New Revision: 896

Modified:
   trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
Log:
DNA-398 RepositoryNodeTypeManager is Not Thread-Safe

Applied the patch that makes RepositoryNodeTypeManager thread safe.

Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java
===================================================================
--- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java	2009-05-07 14:30:44 UTC (rev 895)
+++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/RepositoryNodeTypeManager.java	2009-05-07 14:30:53 UTC (rev 896)
@@ -31,6 +31,8 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
 import javax.jcr.UnsupportedRepositoryOperationException;
@@ -40,7 +42,8 @@
 import javax.jcr.nodetype.NodeType;
 import javax.jcr.nodetype.PropertyDefinition;
 import javax.jcr.version.OnParentVersionAction;
-import net.jcip.annotations.Immutable;
+import net.jcip.annotations.GuardedBy;
+import net.jcip.annotations.ThreadSafe;
 import org.jboss.dna.common.text.TextEncoder;
 import org.jboss.dna.common.text.XmlNameEncoder;
 import org.jboss.dna.graph.ExecutionContext;
@@ -75,7 +78,7 @@
  * {@link javax.jcr.Session}'s transient mappings and then delegating node type lookups to the repository manager.
  * </p>
  */
- at Immutable
+ at ThreadSafe
 class RepositoryNodeTypeManager {
 
     private static final Map<String, Integer> PROPERTY_TYPE_VALUES_FROM_NAME;
@@ -100,11 +103,16 @@
     private static final TextEncoder NAME_ENCODER = new XmlNameEncoder();
 
     private final ExecutionContext context;
+
+    @GuardedBy("nodeTypeManagerLock")
     private final Map<Name, JcrNodeType> nodeTypes;
+    @GuardedBy("nodeTypeManagerLock")
     private final Map<PropertyDefinitionId, JcrPropertyDefinition> propertyDefinitions;
+    @GuardedBy("nodeTypeManagerLock")
     private final Map<NodeDefinitionId, JcrNodeDefinition> childNodeDefinitions;
     private final PropertyFactory propertyFactory;
     private final PathFactory pathFactory;
+    private final ReadWriteLock nodeTypeManagerLock = new ReentrantReadWriteLock();
 
     /**
      * List of ways to filter the returned property definitions
@@ -139,39 +147,70 @@
     }
 
     public Collection<JcrNodeType> getAllNodeTypes() {
-        return nodeTypes.values();
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            return nodeTypes.values();
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
+        }
     }
 
     public Collection<JcrNodeType> getMixinNodeTypes() {
-        List<JcrNodeType> types = new ArrayList<JcrNodeType>(nodeTypes.size());
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            
+            List<JcrNodeType> types = new ArrayList<JcrNodeType>(nodeTypes.size());
 
-        for (JcrNodeType nodeType : nodeTypes.values()) {
-            if (nodeType.isMixin()) types.add(nodeType);
+            for (JcrNodeType nodeType : nodeTypes.values()) {
+                if (nodeType.isMixin()) types.add(nodeType);
+            }
+
+            return types;
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
         }
-
-        return types;
     }
 
     public Collection<JcrNodeType> getPrimaryNodeTypes() {
-        List<JcrNodeType> types = new ArrayList<JcrNodeType>(nodeTypes.size());
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            List<JcrNodeType> types = new ArrayList<JcrNodeType>(nodeTypes.size());
 
-        for (JcrNodeType nodeType : nodeTypes.values()) {
-            if (!nodeType.isMixin()) types.add(nodeType);
+            for (JcrNodeType nodeType : nodeTypes.values()) {
+                if (!nodeType.isMixin()) types.add(nodeType);
+            }
+
+            return types;
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
         }
-
-        return types;
     }
 
     public JcrPropertyDefinition getPropertyDefinition( PropertyDefinitionId id ) {
-        return propertyDefinitions.get(id);
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            return propertyDefinitions.get(id);
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
+        }
     }
 
     public JcrNodeDefinition getChildNodeDefinition( NodeDefinitionId id ) {
-        return childNodeDefinitions.get(id);
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            return childNodeDefinitions.get(id);
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
+        }
     }
 
     JcrNodeType getNodeType( Name nodeTypeName ) {
-        return nodeTypes.get(nodeTypeName);
+        try {
+            nodeTypeManagerLock.readLock().lock();
+            return nodeTypes.get(nodeTypeName);
+        } finally {
+            nodeTypeManagerLock.readLock().unlock();
+        }
     }
 
     /**
@@ -517,6 +556,7 @@
                                                                  PropertyCardinality typeToCheck,
                                                                  List<JcrNodeType> pendingTypes ) {
         assert typeNamesToCheck != null;
+
         Collection<JcrPropertyDefinition> propDefs = null;
         List<JcrPropertyDefinition> matchingDefs = new ArrayList<JcrPropertyDefinition>();
 
@@ -790,7 +830,7 @@
 
         Graph.Batch batch = graph.batch();
 
-        for (JcrNodeType nodeType : nodeTypes.values()) {
+        for (JcrNodeType nodeType : getAllNodeTypes()) {
             projectNodeTypeOnto(nodeType, parentOfTypeNodes, batch);
         }
 
@@ -975,8 +1015,8 @@
      *         type name that is already registered
      * @throws RepositoryException if another error occurs
      */
-    public JcrNodeType registerNodeType( NodeTypeDefinition ntd,
-                                         boolean allowUpdates )
+    JcrNodeType registerNodeType( NodeTypeDefinition ntd,
+                                  boolean allowUpdates )
         throws InvalidNodeTypeDefinitionException, NodeTypeExistsException, RepositoryException {
         assert ntd != null;
         assert ntd instanceof JcrNodeTypeTemplate;
@@ -1063,8 +1103,8 @@
      *         type name that is already registered
      * @throws RepositoryException if another error occurs
      */
-    public List<JcrNodeType> registerNodeTypes( Collection<NodeTypeDefinition> nodeTypeBatch,
-                                                boolean allowUpdates )
+    List<JcrNodeType> registerNodeTypes( Collection<NodeTypeDefinition> nodeTypeBatch,
+                                         boolean allowUpdates )
         throws InvalidNodeTypeDefinitionException, NodeTypeExistsException, RepositoryException {
 
         if (nodeTypeBatch.isEmpty()) {
@@ -1163,76 +1203,82 @@
         List<Location> nodeTypeLocations = nodeTypeBatch.getChildren().of("/");
         List<JcrNodeType> typesPendingRegistration = new ArrayList<JcrNodeType>(nodeTypeLocations.size());
 
-        for (Location location : nodeTypeLocations) {
-            Node nodeTypeNode = nodeTypeBatch.getNodeAt(location);
-            assert location.getPath() != null;
+        try {
+            nodeTypeManagerLock.writeLock().lock();
+            for (Location location : nodeTypeLocations) {
+                Node nodeTypeNode = nodeTypeBatch.getNodeAt(location);
+                assert location.getPath() != null;
 
-            Name internalName = location.getPath().getLastSegment().getName();
-            if (internalName == null || internalName.getLocalName().length() == 0) {
-                throw new InvalidNodeTypeDefinitionException(JcrI18n.invalidNodeTypeName.text());
-            }
+                Name internalName = location.getPath().getLastSegment().getName();
+                if (internalName == null || internalName.getLocalName().length() == 0) {
+                    throw new InvalidNodeTypeDefinitionException(JcrI18n.invalidNodeTypeName.text());
+                }
 
-            if (nodeTypes.containsKey(internalName)) {
-                throw new NodeTypeExistsException(internalName,
-                                                  JcrI18n.nodeTypeAlreadyExists.text(internalName.getString(namespaces)));
-            }
+                if (nodeTypes.containsKey(internalName)) {
+                    throw new NodeTypeExistsException(internalName,
+                                                      JcrI18n.nodeTypeAlreadyExists.text(internalName.getString(namespaces)));
+                }
 
-            List<JcrNodeType> supertypes = supertypesFor(nodeTypeNode, typesPendingRegistration);
-            // No need to re-parse the supertypes
-            JcrNodeType nodeType = nodeTypeFrom(nodeTypeBatch.getSubgraphOfDepth(2).at(location), supertypes);
+                List<JcrNodeType> supertypes = supertypesFor(nodeTypeNode, typesPendingRegistration);
+                // No need to re-parse the supertypes
+                JcrNodeType nodeType = nodeTypeFrom(nodeTypeBatch.getSubgraphOfDepth(2).at(location), supertypes);
 
-            validate(nodeType, supertypes, typesPendingRegistration);
+                validate(nodeType, supertypes, typesPendingRegistration);
 
-            List<JcrPropertyDefinition> propertyDefs = new ArrayList<JcrPropertyDefinition>(
-                                                                                            nodeType.getDeclaredPropertyDefinitions().length);
+                List<JcrPropertyDefinition> propertyDefs = new ArrayList<JcrPropertyDefinition>(
+                                                                                                nodeType.getDeclaredPropertyDefinitions().length);
 
-            for (JcrPropertyDefinition propertyDef : nodeType.getDeclaredPropertyDefinitions()) {
-                propertyDefs.add(propertyDef.with(this.context));
-            }
+                for (JcrPropertyDefinition propertyDef : nodeType.getDeclaredPropertyDefinitions()) {
+                    propertyDefs.add(propertyDef.with(this.context));
+                }
 
-            List<JcrNodeDefinition> nodeDefs = new ArrayList<JcrNodeDefinition>(nodeType.getDeclaredChildNodeDefinitions().length);
+                List<JcrNodeDefinition> nodeDefs = new ArrayList<JcrNodeDefinition>(
+                                                                                    nodeType.getDeclaredChildNodeDefinitions().length);
 
-            for (JcrNodeDefinition nodeDef : nodeType.getDeclaredChildNodeDefinitions()) {
-                JcrNodeType[] requiredPrimaryTypes = new JcrNodeType[nodeDef.requiredPrimaryTypeNames().length];
+                for (JcrNodeDefinition nodeDef : nodeType.getDeclaredChildNodeDefinitions()) {
+                    JcrNodeType[] requiredPrimaryTypes = new JcrNodeType[nodeDef.requiredPrimaryTypeNames().length];
 
-                int i = 0;
-                for (Name primaryTypeName : nodeDef.requiredPrimaryTypeNames()) {
-                    requiredPrimaryTypes[i] = findTypeInMapOrList(primaryTypeName, typesPendingRegistration);
+                    int i = 0;
+                    for (Name primaryTypeName : nodeDef.requiredPrimaryTypeNames()) {
+                        requiredPrimaryTypes[i] = findTypeInMapOrList(primaryTypeName, typesPendingRegistration);
 
-                    if (requiredPrimaryTypes[i] == null) {
-                        throw new RepositoryException(JcrI18n.invalidPrimaryTypeName.text(primaryTypeName, nodeType.getName()));
+                        if (requiredPrimaryTypes[i] == null) {
+                            throw new RepositoryException(
+                                                          JcrI18n.invalidPrimaryTypeName.text(primaryTypeName, nodeType.getName()));
+                        }
+                        i++;
                     }
-                    i++;
+
+                    nodeDefs.add(nodeDef.with(this.context).with(this));
                 }
 
-                nodeDefs.add(nodeDef.with(this.context).with(this));
+                // Create a new node type that also has the correct property and child node definitions associated
+                JcrNodeType newNodeType = new JcrNodeType(this.context, this, nodeType.getInternalName(), supertypes,
+                                                          nodeType.getInternalPrimaryItemName(), nodeDefs, propertyDefs,
+                                                          nodeType.isMixin(), nodeType.hasOrderableChildNodes());
+                typesPendingRegistration.add(newNodeType);
             }
 
-            // Create a new node type that also has the correct property and child node definitions associated
-            JcrNodeType newNodeType = new JcrNodeType(this.context, this, nodeType.getInternalName(), supertypes,
-                                                      nodeType.getInternalPrimaryItemName(), nodeDefs, propertyDefs,
-                                                      nodeType.isMixin(), nodeType.hasOrderableChildNodes());
-            typesPendingRegistration.add(newNodeType);
-        }
+            // Graph.Batch batch = graph.batch();
+            for (JcrNodeType nodeType : typesPendingRegistration) {
+                /*
+                 * See comment in constructor.  Using a ConcurrentHashMap seems to be to weak of a
+                 * solution (even it were also used for childNodeDefinitions and propertyDefinitions).
+                 * Probably need to block all read access to these maps during this phase of registration.
+                 */
+                nodeTypes.put(nodeType.getInternalName(), nodeType);
+                for (JcrNodeDefinition childDefinition : nodeType.childNodeDefinitions()) {
+                    childNodeDefinitions.put(childDefinition.getId(), childDefinition);
+                }
+                for (JcrPropertyDefinition propertyDefinition : nodeType.propertyDefinitions()) {
+                    propertyDefinitions.put(propertyDefinition.getId(), propertyDefinition);
+                }
 
-        // Graph.Batch batch = graph.batch();
-        for (JcrNodeType nodeType : typesPendingRegistration) {
-            /*
-             * See comment in constructor.  Using a ConcurrentHashMap seems to be to weak of a
-             * solution (even it were also used for childNodeDefinitions and propertyDefinitions).
-             * Probably need to block all read access to these maps during this phase of registration.
-             */
-            nodeTypes.put(nodeType.getInternalName(), nodeType);
-            for (JcrNodeDefinition childDefinition : nodeType.childNodeDefinitions()) {
-                childNodeDefinitions.put(childDefinition.getId(), childDefinition);
+                // projectNodeTypeOnto(nodeType, parentOfTypeNodes, batch);
             }
-            for (JcrPropertyDefinition propertyDefinition : nodeType.propertyDefinitions()) {
-                propertyDefinitions.put(propertyDefinition.getId(), propertyDefinition);
-            }
-
-            // projectNodeTypeOnto(nodeType, parentOfTypeNodes, batch);
+        } finally {
+            nodeTypeManagerLock.writeLock().unlock();
         }
-
         // batch.execute();
 
         return typesPendingRegistration;




More information about the dna-commits mailing list