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 2006/12/11 16:46:00 UTC

svn commit: r485720 - in /jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core: BatchedItemOperations.java CachingHierarchyManager.java HierarchyManager.java HierarchyManagerImpl.java ItemManager.java NodeImpl.java SessionImpl.java

Author: stefan
Date: Mon Dec 11 07:45:58 2006
New Revision: 485720

URL: http://svn.apache.org/viewvc?view=rev&rev=485720
Log:
performance tuning: avoid unneccessary throwing of PathNotFoundException

Modified:
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/BatchedItemOperations.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
    jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManager.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/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/SessionImpl.java

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?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- 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 Mon Dec 11 07:45:58 2006
@@ -1377,7 +1377,7 @@
             throws PathNotFoundException, RepositoryException {
         try {
             ItemId id = srcHierMgr.resolvePath(nodePath);
-            if (!id.denotesNode()) {
+            if (id == null || !id.denotesNode()) {
                 throw new PathNotFoundException(safeGetJCRPath(nodePath));
             }
             return (NodeState) getItemState(srcStateMgr, id);

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/CachingHierarchyManager.java Mon Dec 11 07:45:58 2006
@@ -112,7 +112,7 @@
      * Cache the intermediate item inside our cache.
      */
     protected ItemId resolvePath(Path path, ItemState state, int next)
-            throws PathNotFoundException, ItemStateException {
+            throws ItemStateException {
 
         if (state.isNode() && !isCached(state.getId())) {
             try {
@@ -175,8 +175,7 @@
      * <p/>
      * Check the path indicated inside our cache first.
      */
-    public ItemId resolvePath(Path path)
-            throws PathNotFoundException, RepositoryException {
+    public ItemId resolvePath(Path path) throws RepositoryException {
 
         // Run base class shortcut and sanity checks first
         if (path.denotesRoot()) {
@@ -614,7 +613,7 @@
      * @param path child path
      * @param id   node id
      *
-     * @throws PathNotFoundException if hte path was not found
+     * @throws PathNotFoundException if the path was not found
      */
     private void insert(Path path, ItemId id) throws PathNotFoundException {
         synchronized (cacheMonitor) {

Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManager.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManager.java?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManager.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/HierarchyManager.java Mon Dec 11 07:45:58 2006
@@ -30,13 +30,17 @@
 
     /**
      * Resolves a path into an item id.
-     * @param path
-     * @return
-     * @throws PathNotFoundException
-     * @throws RepositoryException
+     * <p/>
+     * Note that, for performance reasons, this method returns <code>null</code>
+     * rather than throwing a <code>PathNotFoundException</code> if there's no
+     * item to be found at <code>path</code>.
+     *  
+     * @param path path to resolve
+     * @return item id refered to by <code>path</code> or <code>null</code>
+     *         if there's no item at <code>path</code>.
+     * @throws RepositoryException if an error occurs
      */
-    ItemId resolvePath(Path path)
-            throws PathNotFoundException, RepositoryException;
+    ItemId resolvePath(Path path) throws RepositoryException;
 
     /**
      * Returns the path to the given item.

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?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- 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 Mon Dec 11 07:45:58 2006
@@ -32,7 +32,6 @@
 import org.slf4j.LoggerFactory;
 
 import javax.jcr.ItemNotFoundException;
-import javax.jcr.PathNotFoundException;
 import javax.jcr.RepositoryException;
 
 /**
@@ -221,10 +220,11 @@
      * @param path  full path of item to resolve
      * @param state intermediate state
      * @param next  next path element index to resolve
-     * @return the id of the item denoted by <code>path</code>
+     * @return the id of the item denoted by <code>path</code> or
+     *         <code>null</code> if no item exists at <code>path</code>.
      */
     protected ItemId resolvePath(Path path, ItemState state, int next)
-            throws PathNotFoundException, ItemStateException {
+            throws ItemStateException {
 
         Path.PathElement[] elements = path.getElements();
         if (elements.length == next) {
@@ -251,18 +251,18 @@
             // property
             if (index > 1) {
                 // properties can't have same name siblings
-                throw new PathNotFoundException(safeGetJCRPath(path));
+                return null;
 
             } else if (next < elements.length - 1) {
                 // property is not the last element in the path
-                throw new PathNotFoundException(safeGetJCRPath(path));
+                return null;
             }
 
             childId = new PropertyId(parentState.getNodeId(), name);
 
         } else {
             // no such item
-            throw new PathNotFoundException(safeGetJCRPath(path));
+            return null;
         }
         return resolvePath(path, getItemState(childId), next + 1);
     }
@@ -326,8 +326,7 @@
     /**
      * {@inheritDoc}
      */
-    public ItemId resolvePath(Path path)
-            throws PathNotFoundException, RepositoryException {
+    public ItemId resolvePath(Path path) throws RepositoryException {
         // shortcut
         if (path.denotesRoot()) {
             return rootNodeId;

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?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- 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 Mon Dec 11 07:45:58 2006
@@ -218,6 +218,9 @@
             session.sanityCheck();
 
             ItemId id = hierMgr.resolvePath(path);
+            if (id == null) {
+                return false;
+            }
 
             // check if state exists for the given item
             if (!itemStateProvider.hasItemState(id)) {
@@ -232,8 +235,6 @@
                 return false;
             }
             return true;
-        } catch (PathNotFoundException pnfe) {
-            return false;
         } catch (ItemNotFoundException infe) {
             return false;
         } catch (RepositoryException re) {
@@ -287,9 +288,12 @@
      * @throws AccessDeniedException
      * @throws RepositoryException
      */
-    public synchronized ItemImpl getItem(Path path)
+    public ItemImpl getItem(Path path)
             throws PathNotFoundException, AccessDeniedException, RepositoryException {
         ItemId id = hierMgr.resolvePath(path);
+        if (id == null) {
+            throw new PathNotFoundException(safeGetJCRPath(path));
+        }
         try {
             return getItem(id);
         } catch (ItemNotFoundException infe) {

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?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- 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 Mon Dec 11 07:45:58 2006
@@ -179,15 +179,15 @@
              */
             Path p = PathFormat.parse(getPrimaryPath(), relPath,
                     session.getNamespaceResolver()).getCanonicalPath();
-            try {
-                ItemId id = session.getHierarchyManager().resolvePath(p);
-                if (!id.denotesNode()) {
-                    return (PropertyId) id;
-                } else {
-                    // not a property
-                    return null;
-                }
-            } catch (PathNotFoundException pnfe) {
+            ItemId id = session.getHierarchyManager().resolvePath(p);
+            if (id == null) {
+                // path not found
+                return null;
+            }
+            if (!id.denotesNode()) {
+                return (PropertyId) id;
+            } else {
+                // not a property
                 return null;
             }
         } catch (NameException e) {
@@ -240,15 +240,15 @@
              * build and resolve absolute path
              */
             p = Path.create(getPrimaryPath(), p, true);
-            try {
-                ItemId id = session.getHierarchyManager().resolvePath(p);
-                if (id.denotesNode()) {
-                    return (NodeId) id;
-                } else {
-                    // not a node
-                    return null;
-                }
-            } catch (PathNotFoundException pnfe) {
+            ItemId id = session.getHierarchyManager().resolvePath(p);
+            if (id == null) {
+                // path not found
+                return null;
+            }
+            if (id.denotesNode()) {
+                return (NodeId) id;
+            } else {
+                // not a node
                 return null;
             }
         } catch (MalformedPathException e) {

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?view=diff&rev=485720&r1=485719&r2=485720
==============================================================================
--- 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 Mon Dec 11 07:45:58 2006
@@ -610,10 +610,11 @@
         if (set.contains(READ_ACTION)) {
             try {
                 targetId = hierMgr.resolvePath(targetPath);
+                if (targetId == null) {
+                    // target does not exist, throw exception
+                    throw new AccessControlException(READ_ACTION);
+                }
                 accessMgr.checkPermission(targetId, AccessManager.READ);
-            } catch (PathNotFoundException pnfe) {
-                // target does not exist, throw exception
-                throw new AccessControlException(READ_ACTION);
             } catch (AccessDeniedException re) {
                 // otherwise the RepositoryException catch clause will
                 // log a warn message, which is not appropriate in this case.
@@ -632,10 +633,11 @@
             try {
                 parentPath = targetPath.getAncestor(1);
                 parentId = hierMgr.resolvePath(parentPath);
+                if (parentId == null) {
+                    // parent does not exist (i.e. / was specified), throw exception
+                    throw new AccessControlException(ADD_NODE_ACTION);
+                }
                 accessMgr.checkPermission(parentId, AccessManager.WRITE);
-            } catch (PathNotFoundException pnfe) {
-                // parent does not exist (i.e. / was specified), throw exception
-                throw new AccessControlException(ADD_NODE_ACTION);
             } catch (AccessDeniedException re) {
                 // otherwise the RepositoryException catch clause will
                 // log a warn message, which is not appropriate in this case.
@@ -651,11 +653,12 @@
             try {
                 if (targetId == null) {
                     targetId = hierMgr.resolvePath(targetPath);
+                    if (targetId == null) {
+                        // parent does not exist, throw exception
+                        throw new AccessControlException(REMOVE_ACTION);
+                    }
                 }
                 accessMgr.checkPermission(targetId, AccessManager.REMOVE);
-            } catch (PathNotFoundException pnfe) {
-                // parent does not exist, throw exception
-                throw new AccessControlException(REMOVE_ACTION);
             } catch (AccessDeniedException re) {
                 // otherwise the RepositoryException catch clause will
                 // log a warn message, which is not appropriate in this case.
@@ -672,12 +675,8 @@
         if (set.contains(SET_PROPERTY_ACTION)) {
             try {
                 if (targetId == null) {
-                    try {
-                        targetId = hierMgr.resolvePath(targetPath);
-                        // property does already exist,
-                        // check WRITE permission on target
-                        accessMgr.checkPermission(targetId, AccessManager.WRITE);
-                    } catch (PathNotFoundException pnfe) {
+                    targetId = hierMgr.resolvePath(targetPath);
+                    if (targetId == null) {
                         // property does not exist yet,
                         // check WRITE permission on parent
                         if (parentPath == null) {
@@ -685,13 +684,18 @@
                         }
                         if (parentId == null) {
                             parentId = hierMgr.resolvePath(parentPath);
+                            if (parentId == null) {
+                                // parent does not exist, throw exception
+                                throw new AccessControlException(SET_PROPERTY_ACTION);
+                            }
                         }
                         accessMgr.checkPermission(parentId, AccessManager.WRITE);
+                    } else {
+                        // property does already exist,
+                        // check WRITE permission on target
+                        accessMgr.checkPermission(targetId, AccessManager.WRITE);
                     }
                 }
-            } catch (PathNotFoundException pnfe) {
-                // parent does not exist, throw exception
-                throw new AccessControlException(SET_PROPERTY_ACTION);
             } catch (AccessDeniedException re) {
                 // otherwise the RepositoryException catch clause will
                 // log a warn message, which is not appropriate in this case.