You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by ju...@apache.org on 2012/05/09 15:21:01 UTC
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...
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);