diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/AbstractNameMapper.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/AbstractNameMapper.java index 5c12d1b..e70a1f2 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/AbstractNameMapper.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/AbstractNameMapper.java @@ -57,7 +57,7 @@ public abstract class AbstractNameMapper implements NameMapper { String name = jcrName.substring(pos + 1); String oakPrefix = getOakPrefix(pref); if (oakPrefix == null) { - throw new IllegalArgumentException("prefix '" + pref + "' is not mapped" ); + return null; // not a mapped name } else { return oakPrefix + ':' + name; } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NameMapper.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NameMapper.java index adc3a1f..33ad5d8 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NameMapper.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NameMapper.java @@ -18,9 +18,26 @@ package org.apache.jackrabbit.oak.namepath; public interface NameMapper { - // TODO consider error handling + /** + * Returns the Oak name for the given JCR name, or {@code null} if no + * such mapping exists because the given JCR name contains an unknown + * namespace URI or prefix, or is otherwise invalid. + * + * @param jcrName JCR name + * @return Oak name, or {@code null} + */ String getOakName(String jcrName); - // TODO consider error handling + /** + * Returns the JCR name for the given Oak name. The given name is + * expected to have come from a valid Oak repository that contains + * only valid names with proper namespace mappings. If that's not + * the case, either a programming error or a repository corruption + * has occurred and an appropriate unchecked exception gets thrown. + * + * @param oakName Oak name + * @return JCR name + */ String getJcrName(String oakName); + } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java index 81ca4c5..28d52ed 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImpl.java @@ -108,11 +108,18 @@ public class NamePathMapperImpl implements NamePathMapper { @Override public void name(String name) { String p = nameMapper.getOakName(name); + if (p == null) { + error("Invalid name: " + name); + } elements.add(p); } }; - 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) { diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/PathMapper.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/PathMapper.java index 9e2ed20..6b9b166 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/PathMapper.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/namepath/PathMapper.java @@ -21,8 +21,27 @@ package org.apache.jackrabbit.oak.namepath; */ public interface PathMapper { + /** + * Returns the Oak path for the given JCR path, or {@code null} if no + * such mapping exists because the given JCR path contains a name element + * with an unknown namespace URI or prefix, or is otherwise invalid. + * + * @param jcrPath JCR path + * @return Oak path, or {@code null} + */ String getOakPath(String jcrPath); + /** + * Returns the JCR path for the given Oak path. The given path is + * expected to have come from a valid Oak repository that contains + * only valid paths whose name elements only use proper namespace + * mappings. If that's not the case, either a programming error or + * a repository corruption has occurred and an appropriate unchecked + * exception gets thrown. + * + * @param oakPath Oak path + * @return JCR path + */ String getJcrPath(String oakPath); } diff --git a/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java b/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java index 5e46c61..389d2f1 100644 --- a/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java +++ b/oak-core/src/test/java/org/apache/jackrabbit/oak/namepath/NamePathMapperImplTest.java @@ -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,22 +37,17 @@ public class NamePathMapperImplTest { @Test public void testValidIdentifierPath() { String idPath = '[' + UUID.randomUUID().toString()+ ']'; - npMapper.getOakPath(idPath); + assertNotNull(npMapper.getOakPath(idPath)); } @Test public void testInvalidIdentifierPath() { - List invalid = new ArrayList(); + List invalid = new ArrayList(); invalid.add('[' + UUID.randomUUID().toString()+ "]abc"); 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)); } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java index dfd3dfb..c3494f9 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/ItemImpl.java @@ -172,23 +172,6 @@ abstract class ItemImpl extends AbstractItem { return sessionDelegate.getValueFactory(); } - String toOakName(String jcrName) { - return sessionDelegate.getNamePathMapper().getOakName(jcrName); - } - - 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 toJcrName(String oakName) { - return sessionDelegate.getNamePathMapper().getJcrName(oakName); - } - String toJcrPath(String oakPath) { return sessionDelegate.getNamePathMapper().getJcrPath(oakPath); } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java index 5a2bdf3..b6000bd 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/NodeImpl.java @@ -24,6 +24,8 @@ import org.apache.jackrabbit.oak.api.Tree.Status; import org.apache.jackrabbit.oak.commons.PathUtils; import org.apache.jackrabbit.oak.jcr.util.LogUtil; import org.apache.jackrabbit.oak.jcr.value.ValueConverter; +import org.apache.jackrabbit.oak.namepath.NameMapper; +import org.apache.jackrabbit.oak.namepath.NamePathMapper; import org.apache.jackrabbit.oak.util.Function1; import org.apache.jackrabbit.oak.util.Iterators; import org.apache.jackrabbit.oak.util.Predicate; @@ -148,7 +150,7 @@ public class NodeImpl extends ItemImpl implements Node { 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) { @@ -211,8 +213,9 @@ public class NodeImpl extends ItemImpl implements Node { 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)); } } @@ -241,8 +244,9 @@ public class NodeImpl extends ItemImpl implements Node { p.remove(); return p; } else { + String oakName = sessionDelegate.getOakPathOrThrow(jcrName); List oakValue = ValueConverter.toCoreValues(targetValues, sessionDelegate); - return new PropertyImpl(dlg.setProperty(toOakPath(jcrName), oakValue)); + return new PropertyImpl(dlg.setProperty(oakName, oakValue)); } } @@ -361,7 +365,8 @@ public class NodeImpl extends ItemImpl implements Node { 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); } @@ -414,7 +419,8 @@ public class NodeImpl extends ItemImpl implements Node { 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 { @@ -545,14 +551,16 @@ public class NodeImpl extends ItemImpl implements Node { 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 @@ -609,7 +617,12 @@ public class NodeImpl extends ItemImpl implements Node { checkStatus(); // TODO: might be expanded, need a better way for this - String jcrName = toJcrName(toOakName(nodeTypeName)); + NameMapper mapper = sessionDelegate.getNamePathMapper(); + String oakName = mapper.getOakName(nodeTypeName); + if (oakName == null) { + return false; // An unknown name can't belong to a valid type + } + String jcrName = mapper.getJcrName(oakName); // TODO: figure out the right place for this check NodeTypeManager ntm = sessionDelegate.getNodeTypeManager(); @@ -643,8 +656,10 @@ public class NodeImpl extends ItemImpl implements Node { } // 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 diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java index abdf9d0..6afb545 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionDelegate.java @@ -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); } } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java index 77b0777..d08cf06 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/SessionImpl.java @@ -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 AbstractSession implements JackrabbitSession { @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 AbstractSession implements JackrabbitSession { 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 AbstractSession implements JackrabbitSession { @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 AbstractSession implements JackrabbitSession { throw new RepositoryException("This session has been closed."); } } + } \ No newline at end of file diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java index 36ee014..49bd080 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/WorkspaceImpl.java @@ -90,7 +90,9 @@ public class WorkspaceImpl implements JackrabbitWorkspace { 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 JackrabbitWorkspace { 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 diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/nodetype/NodeTypeManagerImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/nodetype/NodeTypeManagerImpl.java index ff9ac26..afe025b 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/nodetype/NodeTypeManagerImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/nodetype/NodeTypeManagerImpl.java @@ -92,13 +92,15 @@ public class NodeTypeManagerImpl implements NodeTypeManager { @Override public boolean hasNodeType(String name) throws RepositoryException { init(); - return typemap.containsKey(mapper.getOakName(name)); + String oakName = mapper.getOakName(name); // can be null, which is fine + return typemap.containsKey(oakName); } @Override public NodeType getNodeType(String name) throws RepositoryException { init(); - NodeType type = typemap.get(mapper.getOakName(name)); + String oakName = mapper.getOakName(name); // can be null, which is fine + NodeType type = typemap.get(oakName); if (type == null) { throw new NoSuchNodeTypeException("Unknown node type: " + name); } diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java index 63f8947..33a692c 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/security/user/UserManagerImpl.java @@ -388,6 +388,7 @@ public class UserManagerImpl implements UserManager { } private String getInternalPath(Node node) throws RepositoryException { - return sessionDelegate.getOakPath(node.getPath()); + return sessionDelegate.getOakPathOrThrow(node.getPath()); } + } \ No newline at end of file diff --git a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java index 0098fcb..f57d460 100644 --- a/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java +++ b/oak-jcr/src/main/java/org/apache/jackrabbit/oak/jcr/value/ValueFactoryImpl.java @@ -19,6 +19,7 @@ package org.apache.jackrabbit.oak.jcr.value; 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; @@ -149,9 +150,16 @@ public class ValueFactoryImpl implements ValueFactory { CoreValue cv; try { if (type == PropertyType.NAME) { - cv = factory.createValue(namePathMapper.getOakName(value), type); + String oakName = namePathMapper.getOakName(value); + if (oakName == null) { + throw new ValueFormatException("Invalid name: " + value); + } + 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) { @@ -163,13 +171,6 @@ public class ValueFactoryImpl implements ValueFactory { } } catch (NumberFormatException e) { throw new ValueFormatException("Invalid value " + value + " for type " + PropertyType.nameFromValue(type)); - } catch (IllegalArgumentException e) { - // TODO: review exception handling in path/name 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);