jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1336185 - in /jackrabbit/oak/trunk: oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/ oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/ oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ oak-jcr/src/main/java/org/apache...
Date Wed, 09 May 2012 13:21:01 GMT
Author: jukka
Date: Wed May  9 13:21:01 2012
New Revision: 1336185

URL: http://svn.apache.org/viewvc?rev=1336185&view=rev
Log:
OAK-89: Improve exception handling

Proper handling of null return values from getOakPath()

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java
    jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java
Wed May  9 13:21:01 2012
@@ -115,7 +115,11 @@ public class NamePathMapperImpl implemen
             }
         };
 
-        JcrPathParser.parse(jcrPath, listener);
+        try {
+            JcrPathParser.parse(jcrPath, listener);
+        } catch (RuntimeException e) {
+            return null; // TODO Avoid exceptions for control flow
+        }
 
         StringBuilder oakPath = new StringBuilder();
         for (String element : elements) {

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java
Wed May  9 13:21:01 2012
@@ -25,6 +25,8 @@ import java.util.Map;
 import java.util.UUID;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.fail;
 
 public class NamePathMapperImplTest {
@@ -35,7 +37,7 @@ public class NamePathMapperImplTest {
     @Test
     public void testValidIdentifierPath() {
         String idPath = '[' + UUID.randomUUID().toString()+ ']';
-        npMapper.getOakPath(idPath);
+        assertNotNull(npMapper.getOakPath(idPath));
     }
 
     @Test
@@ -45,12 +47,7 @@ public class NamePathMapperImplTest {
         invalid.add('[' + UUID.randomUUID().toString()+ "]/a/b/c");
 
         for (String jcrPath : invalid) {
-            try {
-                npMapper.getOakPath(jcrPath);
-                fail("Invalid identifier path");
-            } catch (Exception e) {
-                // success
-            }
+            assertNull(npMapper.getOakPath(jcrPath));
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java
Wed May  9 13:21:01 2012
@@ -172,15 +172,6 @@ abstract class ItemImpl extends Abstract
         return sessionDelegate.getValueFactory();
     }
 
-    String toOakPath(String jcrPath) throws RepositoryException {
-        try {
-            return sessionDelegate.getOakPath(jcrPath);
-        } catch (IllegalArgumentException ex) {
-            // TODO we shouldn't have to catch this one
-            throw new RepositoryException(ex);
-        }
-    }
-
     String toJcrPath(String oakPath) {
         return sessionDelegate.getNamePathMapper().getJcrPath(oakPath);
     }

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java
Wed May  9 13:21:01 2012
@@ -150,7 +150,7 @@ public class NodeImpl extends ItemImpl i
     public Node addNode(String relPath, String primaryNodeTypeName) throws RepositoryException
{
         checkStatus();
 
-        String oakPath = toOakPath(relPath);
+        String oakPath = sessionDelegate.getOakPathOrThrow(relPath);
         String oakName = PathUtils.getName(oakPath);
         NodeDelegate parent = dlg.getChild(PathUtils.getParentPath(oakPath));
         if (parent == null) {
@@ -213,8 +213,9 @@ public class NodeImpl extends ItemImpl i
             p.remove();
             return p;
         } else {
+            String oakName = sessionDelegate.getOakPathOrThrow(jcrName);
             CoreValue oakValue = ValueConverter.toCoreValue(targetValue, sessionDelegate);
-            return new PropertyImpl(dlg.setProperty(toOakPath(jcrName), oakValue));
+            return new PropertyImpl(dlg.setProperty(oakName, oakValue));
         }
     }
 
@@ -243,8 +244,9 @@ public class NodeImpl extends ItemImpl i
             p.remove();
             return p;
         } else {
+            String oakName = sessionDelegate.getOakPathOrThrow(jcrName);
             List<CoreValue> oakValue = ValueConverter.toCoreValues(targetValues, sessionDelegate);
-            return new PropertyImpl(dlg.setProperty(toOakPath(jcrName), oakValue));
+            return new PropertyImpl(dlg.setProperty(oakName, oakValue));
         }
     }
 
@@ -363,7 +365,8 @@ public class NodeImpl extends ItemImpl i
     public Node getNode(String relPath) throws RepositoryException {
         checkStatus();
 
-        NodeDelegate nd = dlg.getChild(toOakPath(relPath));
+        String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
+        NodeDelegate nd = dlg.getChild(oakPath);
         if (nd == null) {
             throw new PathNotFoundException(relPath);
         }
@@ -416,7 +419,8 @@ public class NodeImpl extends ItemImpl i
     public Property getProperty(String relPath) throws RepositoryException {
         checkStatus();
 
-        PropertyDelegate pd = dlg.getProperty(toOakPath(relPath));
+        String oakPath = sessionDelegate.getOakPathOrThrowNotFound(relPath);
+        PropertyDelegate pd = dlg.getProperty(oakPath);
         if (pd == null) {
             throw new PathNotFoundException(relPath + " not found on " + getPath());
         } else {
@@ -547,14 +551,16 @@ public class NodeImpl extends ItemImpl i
     public boolean hasNode(String relPath) throws RepositoryException {
         checkStatus();
 
-        return dlg.getChild(toOakPath(relPath)) != null;
+        String oakPath = sessionDelegate.getOakPathOrNull(relPath);
+        return oakPath != null && dlg.getChild(oakPath) != null;
     }
 
     @Override
     public boolean hasProperty(String relPath) throws RepositoryException {
         checkStatus();
 
-        return dlg.getProperty(toOakPath(relPath)) != null;
+        String oakPath = sessionDelegate.getOakPathOrNull(relPath);
+        return oakPath != null && dlg.getProperty(oakPath) != null;
     }
 
     @Override
@@ -650,8 +656,10 @@ public class NodeImpl extends ItemImpl i
         }
         // TODO: END
 
+        String jcrPrimaryType =
+                sessionDelegate.getOakPathOrThrow(Property.JCR_PRIMARY_TYPE);
         CoreValue cv = ValueConverter.toCoreValue(nodeTypeName, PropertyType.NAME, sessionDelegate);
-        dlg.setProperty(toOakPath(Property.JCR_PRIMARY_TYPE), cv);
+        dlg.setProperty(jcrPrimaryType, cv);
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java
Wed May  9 13:21:01 2012
@@ -145,16 +145,46 @@ public class SessionDelegate {
     /**
      * Shortcut for {@code SessionDelegate.getNamePathMapper().getOakPath(jcrPath)}.
      *
-     * @param jcrPath
-     * @return
-     * @throws RepositoryException
+     * @param jcrPath JCR path
+     * @return Oak path, or {@code null}
      */
-    public String getOakPath(String jcrPath) throws RepositoryException {
-        try {
-            return getNamePathMapper().getOakPath(jcrPath);
-        } catch (IllegalArgumentException ex) {
-            // TODO we shouldn't have to catch this one
-            throw new RepositoryException(ex);
+    public String getOakPathOrNull(String jcrPath) {
+        return getNamePathMapper().getOakPath(jcrPath);
+    }
+
+    /**
+     * Returns the Oak path for the given JCR path, or throws a
+     * {@link PathNotFoundException} if the path can not be mapped.
+     *
+     * @param jcrPath JCR path
+     * @return Oak path
+     * @throws PathNotFoundException if the path can not be mapped
+     */
+    public String getOakPathOrThrowNotFound(String jcrPath)
+            throws PathNotFoundException {
+        String oakPath = getOakPathOrNull(jcrPath);
+        if (oakPath != null) {
+            return oakPath;
+        } else {
+            throw new PathNotFoundException(jcrPath);
+        }
+    }
+
+    /**
+     * Returns the Oak path for the given JCR path, or throws a
+     * {@link RepositoryException} if the path can not be mapped.
+     *
+     * @param jcrPath JCR path
+     * @return Oak path
+     * @throws RepositoryException if the path can not be mapped
+     */
+    public String getOakPathOrThrow(String jcrPath)
+            throws RepositoryException {
+        String oakPath = getOakPathOrNull(jcrPath);
+        if (oakPath != null) {
+            return oakPath;
+        } else {
+            throw new RepositoryException("Invalid path: " + jcrPath);
         }
     }
 

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java
Wed May  9 13:21:01 2012
@@ -26,6 +26,7 @@ import org.xml.sax.ContentHandler;
 
 import javax.jcr.Credentials;
 import javax.jcr.Node;
+import javax.jcr.PathNotFoundException;
 import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
@@ -120,7 +121,10 @@ public class SessionImpl extends Abstrac
     @Override
     public void move(String srcAbsPath, String destAbsPath) throws RepositoryException {
         ensureIsAlive();
-        dlg.move(dlg.getOakPath(srcAbsPath), dlg.getOakPath(destAbsPath), true);
+        dlg.move(
+                dlg.getOakPathOrThrowNotFound(srcAbsPath),
+                dlg.getOakPathOrThrowNotFound(destAbsPath),
+                true);
     }
 
     @Override
@@ -156,8 +160,9 @@ public class SessionImpl extends Abstrac
     public ContentHandler getImportContentHandler(String parentAbsPath, int uuidBehavior)
throws RepositoryException {
         ensureIsAlive();
 
-        // TODO
-        String internalPath = dlg.getOakPath(parentAbsPath);
+        @SuppressWarnings("unused")
+        String oakPath = dlg.getOakPathOrThrowNotFound(parentAbsPath);
+
         throw new UnsupportedRepositoryOperationException("TODO: Session.getImportContentHandler");
     }
 
@@ -200,7 +205,11 @@ public class SessionImpl extends Abstrac
     @Override
     public boolean hasPermission(String absPath, String actions) throws RepositoryException
{
         ensureIsAlive();
-        String internalPath = dlg.getOakPath(absPath);
+
+        String oakPath = dlg.getOakPathOrNull(absPath);
+        if (oakPath == null) {
+            return false; // TODO should we throw an exception here?
+        }
 
         // TODO
         return false;
@@ -266,4 +275,5 @@ public class SessionImpl extends Abstrac
             throw new RepositoryException("This session has been closed.");
         }
     }
+
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java
Wed May  9 13:21:01 2012
@@ -90,7 +90,9 @@ public class WorkspaceImpl implements Ja
             throw new UnsupportedRepositoryOperationException("Not implemented.");
         }
 
-        sessionDelegate.copy(sessionDelegate.getOakPath(srcAbsPath), sessionDelegate.getOakPath(destAbsPath));
+        sessionDelegate.copy(
+                sessionDelegate.getOakPathOrThrowNotFound(srcAbsPath),
+                sessionDelegate.getOakPathOrThrowNotFound(destAbsPath));
     }
 
     @SuppressWarnings("deprecation")
@@ -108,7 +110,10 @@ public class WorkspaceImpl implements Ja
         ensureSupportedOption(Repository.LEVEL_2_SUPPORTED);
         ensureIsAlive();
 
-        sessionDelegate.move(sessionDelegate.getOakPath(srcAbsPath), sessionDelegate.getOakPath(destAbsPath),
false);
+        sessionDelegate.move(
+                sessionDelegate.getOakPathOrThrowNotFound(srcAbsPath),
+                sessionDelegate.getOakPathOrThrowNotFound(destAbsPath),
+                false);
     }
 
     @Override

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java
Wed May  9 13:21:01 2012
@@ -388,6 +388,7 @@ public class UserManagerImpl implements 
     }
 
     private String getInternalPath(Node node) throws RepositoryException {
-        return sessionDelegate.getOakPath(node.getPath());
+        return sessionDelegate.getOakPathOrThrow(node.getPath());
     }
+
 }
\ No newline at end of file

Modified: jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java?rev=1336185&r1=1336184&r2=1336185&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java
Wed May  9 13:21:01 2012
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.jcr.va
 import org.apache.commons.io.IOUtils;
 import org.apache.jackrabbit.oak.api.CoreValue;
 import org.apache.jackrabbit.oak.api.CoreValueFactory;
+import org.apache.jackrabbit.oak.jcr.SessionDelegate;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
 import org.apache.jackrabbit.util.ISO8601;
 import org.slf4j.Logger;
@@ -156,6 +157,9 @@ public class ValueFactoryImpl implements
                 cv = factory.createValue(oakName, type);
             } else if (type == PropertyType.PATH) {
                 String oakPath = namePathMapper.getOakPath(value);
+                if (oakPath == null) {
+                    throw new ValueFormatException("Invalid path: " + value);
+                }
                 cv = factory.createValue(oakPath, type);
             } else if (type == PropertyType.DATE) {
                 if (ISO8601.parse(value) == null) {
@@ -167,13 +171,6 @@ public class ValueFactoryImpl implements
             }
         } catch (NumberFormatException e) {
             throw new ValueFormatException("Invalid value " + value + " for type " + PropertyType.nameFromValue(type));
-        } catch (IllegalArgumentException e) {
-            // TODO: review exception handling in path resolution again
-            throw new ValueFormatException("Invalid value " + value + " for type " + PropertyType.nameFromValue(type));
-        } catch (Exception e) {
-            // TODO: review exception handling in path/name resolution again
-            // TODO: throws RuntimeException which is pretty ugly
-            throw new ValueFormatException("Invalid value " + value + " for type " + PropertyType.nameFromValue(type));
         }
 
         return new ValueImpl(cv, namePathMapper);



Mime
View raw message