This is an automated email from the ASF dual-hosted git repository. angela pushed a commit to branch SLING-10011 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit c27e41c6547b734e1ac39501eeb352a2b118cfec Author: angela <[email protected]> AuthorDate: Tue May 10 12:20:59 2022 +0200 SLING-10011 : Use JackrabbitSession.getParentOrNull when resolving parent JCR node in JcrResourceProvider#getParent --- pom.xml | 8 ++-- .../helper/jcr/JcrItemResourceFactory.java | 21 ++++++++++ .../internal/helper/jcr/JcrResourceProvider.java | 31 ++++++-------- .../helper/jcr/JcrItemResourceFactoryTest.java | 47 +++++++++++++++++++++- 4 files changed, 84 insertions(+), 23 deletions(-) diff --git a/pom.xml b/pom.xml index 8aeb840..fd803b0 100644 --- a/pom.xml +++ b/pom.xml @@ -44,7 +44,9 @@ <properties> <site.jira.version.id>12314286</site.jira.version.id> <site.javadoc.exclude>**.internal.**</site.javadoc.exclude> - <oak.version>1.10.0</oak.version><!-- first version compatible with Jackrabbit API 2.18 (https://issues.apache.org/jira/browse/OAK-7943) --> + <!-- aok 1.10.0 was first version compatible with Jackrabbit API 2.18 (https://issues.apache.org/jira/browse/OAK-7943) --> + <!-- aok 1.42.0 first version to include JackrabbitSession.getParentOrNull in oak-jackrabbit-api (https://issues.apache.org/jira/browse/SLING-10011) --> + <oak.version>1.42.0</oak.version> <jackrabbit.version>2.18.0</jackrabbit.version><!-- required for direct binary access, https://issues.apache.org/jira/browse/JCR-4335 --> <project.build.outputTimestamp>1</project.build.outputTimestamp> </properties> @@ -112,8 +114,8 @@ <!-- Jackrabbit / Oak --> <dependency> <groupId>org.apache.jackrabbit</groupId> - <artifactId>jackrabbit-api</artifactId> - <version>${jackrabbit.version}</version> + <artifactId>oak-jackrabbit-api</artifactId> + <version>${oak.version}</version> <scope>provided</scope> </dependency> <dependency> diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java index 22b210e..d822209 100644 --- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java +++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactory.java @@ -18,6 +18,7 @@ */ package org.apache.sling.jcr.resource.internal.helper.jcr; +import java.lang.reflect.Method; import java.util.LinkedList; import java.util.Map; @@ -34,7 +35,10 @@ import org.apache.commons.lang3.StringUtils; import org.apache.jackrabbit.api.JackrabbitSession; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; +import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.jcr.resource.internal.HelperData; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -197,4 +201,21 @@ public class JcrItemResourceFactory { return item; } + @Nullable Node getParentOrNull(@NotNull Item child, @NotNull String parentPath) { + Node parent = null; + try { + // Use fast getParentOrNull if session is a JackrabbitSession + if (this.isJackrabbit) { + parent = ((JackrabbitSession) session).getParentOrNull(child); + } else if (session.nodeExists(parentPath)) { + // Fallback to slower nodeExists & getNode pattern + parent = session.getNode(parentPath); + } + } catch (RepositoryException e) { + log.debug("Unable to access node at " + parentPath + ", possibly invalid path", e); + } + + return parent; + } + } diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java index bdb7790..a6f380b 100644 --- a/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java +++ b/src/main/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrResourceProvider.java @@ -358,28 +358,21 @@ public class JcrResourceProvider extends ResourceProvider<JcrProviderState> { @Override public @Nullable Resource getParent(final @NotNull ResolveContext<JcrProviderState> ctx, final @NotNull Resource child) { if (child instanceof JcrItemResource<?>) { - try { - String version = null; - if (child.getResourceMetadata().getParameterMap() != null) { - version = child.getResourceMetadata().getParameterMap().get("v"); - } - if (version == null) { - String parentPath = ResourceUtil.getParent(child.getPath()); - if (parentPath != null) { - Item parentItem = ctx.getProviderState().getResourceFactory() - .getItemOrNull(parentPath); - if (parentItem != null && parentItem.isNode()) { - return new JcrNodeResource(ctx.getResourceResolver(), - parentPath, null, (Node)parentItem, - ctx.getProviderState().getHelperData()); - } + String version = null; + if (child.getResourceMetadata().getParameterMap() != null) { + version = child.getResourceMetadata().getParameterMap().get("v"); + } + if (version == null) { + String parentPath = ResourceUtil.getParent(child.getPath()); + if (parentPath != null) { + Item childItem = ((JcrItemResource) child).getItem(); + Node parentNode = ctx.getProviderState().getResourceFactory().getParentOrNull(childItem, parentPath); + if (parentNode != null) { + return new JcrNodeResource(ctx.getResourceResolver(), parentPath, null, parentNode, ctx.getProviderState().getHelperData()); } - return null; } - } catch (RepositoryException e) { - logger.warn("Can't get parent for {}", child, e); - return null; } + return null; } return super.getParent(ctx, child); } diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java index 3d65ebd..1f2e71c 100644 --- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java +++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrItemResourceFactoryTest.java @@ -23,12 +23,20 @@ import java.lang.reflect.Method; import java.lang.reflect.Proxy; import java.util.concurrent.atomic.AtomicReference; +import javax.jcr.GuestCredentials; import javax.jcr.Item; import javax.jcr.Node; import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.SimpleCredentials; +import javax.jcr.security.Privilege; +import org.apache.jackrabbit.JcrConstants; import org.apache.jackrabbit.commons.JcrUtils; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.apache.jackrabbit.oak.commons.PathUtils; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; +import org.apache.jackrabbit.oak.spi.security.user.UserConstants; import org.apache.sling.api.resource.external.URIProvider; import org.apache.sling.commons.classloader.DynamicClassLoaderManager; import org.apache.sling.jcr.resource.internal.HelperData; @@ -41,7 +49,7 @@ public class JcrItemResourceFactoryTest extends SlingRepositoryTestBase { private Node node; private Session nonJackrabbitSession; - + @Override protected void setUp() throws Exception { super.setUp(); @@ -58,6 +66,9 @@ public class JcrItemResourceFactoryTest extends SlingRepositoryTestBase { return method.invoke(session, args); } }); + + AccessControlUtils.allow(node, EveryonePrincipal.NAME, Privilege.JCR_READ); + session.save(); } @Override @@ -100,5 +111,39 @@ public class JcrItemResourceFactoryTest extends SlingRepositoryTestBase { assertEquals(expectedPath, item2.getPath()); } } + + public void testGetParentOrNullExistingNode() throws RepositoryException { + compareGetParentOrNull(session, EXISTING_NODE_PATH, false); + compareGetParentOrNull(nonJackrabbitSession, EXISTING_NODE_PATH, false); + } + + public void testGetParentOrNullExistingProperty() throws RepositoryException { + compareGetParentOrNull(session,EXISTING_NODE_PATH + "/" + JcrConstants.JCR_PRIMARYTYPE, false); + compareGetParentOrNull(nonJackrabbitSession,EXISTING_NODE_PATH + "/" + JcrConstants.JCR_PRIMARYTYPE, false); + } + + public void testGetParentOrNullNonAccessibleParent() throws RepositoryException { + Session guestSession = null; + try { + guestSession = session.getRepository().login(new GuestCredentials()); + compareGetParentOrNull(guestSession, EXISTING_NODE_PATH, true); + } finally { + if (guestSession != null) { + guestSession.logout(); + } + } + } + + private void compareGetParentOrNull(Session s, String path, boolean nullExpected) throws RepositoryException { + HelperData helper = new HelperData(new AtomicReference<DynamicClassLoaderManager>(), new AtomicReference<URIProvider[]>()); + String parentPath = PathUtils.getParentPath(path); + Node parent = new JcrItemResourceFactory(s, helper).getParentOrNull(s.getItem(path), parentPath); + if (nullExpected) { + assertNull(parent); + } else { + assertNotNull(parent); + assertEquals(parentPath, parent.getPath()); + } + } }
