This is an automated email from the ASF dual-hosted git repository. reschke pushed a commit to branch reschke-tmp in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 05393961b9c2654670ca510707048e2b41278f9d Author: Julian Reschke <[email protected]> AuthorDate: Mon Jun 2 13:17:02 2025 +0100 test --- .../impl/ResourceResolverImpl.java | 6 + .../impl/mapping/AliasHandler.java | 37 ++--- .../impl/mapping/ResourceMapperImpl.java | 45 +++--- .../impl/MockedResourceResolverImplTest.java | 12 +- .../impl/ResourceDecorationTest.java | 2 +- .../impl/mapping/AliasMapEntriesTest.java | 162 +++++++++++++++++---- 6 files changed, 174 insertions(+), 90 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java index 5056d89a..2c196b2c 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java @@ -84,6 +84,12 @@ public class ResourceResolverImpl extends SlingAdaptable implements ResourceReso public static final String PROP_ALIAS = "sling:alias"; + // The suffix of a resource being a content node of some parent + // such as nt:file. The slash is included to prevent false + // positives for the String.endsWith check for names like + // "xyzjcr:content" + public static final String JCR_CONTENT_LEAF = "/jcr:content"; + protected static final String PARENT_RT_CACHEKEY = ResourceResolverImpl.class.getName() + ".PARENT_RT"; /** The factory which created this resource resolver. */ diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/AliasHandler.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/AliasHandler.java index da158284..fb2efff0 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/AliasHandler.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/AliasHandler.java @@ -32,6 +32,7 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.locks.ReentrantLock; +import java.util.stream.Collectors; import org.apache.sling.api.resource.LoginException; import org.apache.sling.api.resource.QuerySyntaxException; @@ -166,7 +167,7 @@ class AliasHandler { } boolean doAddAlias(@NotNull Resource resource) { - if (this.aliasMapsMap != UNITIALIZED_MAP) { + if (usesCache()) { return loadAlias(resource, this.aliasMapsMap, null, null); } else { return false; @@ -185,7 +186,7 @@ class AliasHandler { @NotNull String contentPath, @Nullable String path, @NotNull Runnable notifyOfChange) { - if (this.aliasMapsMap != UNITIALIZED_MAP) { + if (usesCache()) { return removeAliasInMap(resolver, contentPath, path, notifyOfChange); } else { return false; @@ -279,7 +280,7 @@ class AliasHandler { * @return {@code true} if any change */ boolean doUpdateAlias(@NotNull Resource resource) { - if (this.aliasMapsMap != UNITIALIZED_MAP) { + if (usesCache()) { return doUpdateAliasInMap(resource); } else { return false; @@ -321,16 +322,14 @@ class AliasHandler { } public @NotNull Map<String, Collection<String>> getAliasMap(@Nullable String parentPath) { - Map<String, Collection<String>> result = this.aliasMapsMap != UNITIALIZED_MAP - ? getAliasMapFromCache(parentPath) - : getAliasMapFromRepo(parentPath); + Map<String, Collection<String>> result = + usesCache() ? getAliasMapFromCache(parentPath) : getAliasMapFromRepo(parentPath); return result != null ? result : Collections.emptyMap(); } public @NotNull Map<String, Collection<String>> getAliasMap(@NotNull Resource parent) { - Map<String, Collection<String>> result = this.aliasMapsMap != UNITIALIZED_MAP - ? getAliasMapFromCache(parent.getPath()) - : getAliasMapFromRepo(parent); + Map<String, Collection<String>> result = + usesCache() ? getAliasMapFromCache(parent.getPath()) : getAliasMapFromRepo(parent); return result != null ? result : Collections.emptyMap(); } @@ -440,24 +439,14 @@ class AliasHandler { private String generateAliasQuery() { Set<String> allowedLocations = this.factory.getAllowedAliasLocations(); - StringBuilder baseQuery = new StringBuilder("SELECT [sling:alias] FROM [nt:base] WHERE"); + StringBuilder baseQuery = new StringBuilder("SELECT [sling:alias] FROM [nt:base] WHERE "); if (allowedLocations.isEmpty()) { - baseQuery.append(" ").append(QueryBuildHelper.excludeSystemPath()); + baseQuery.append(QueryBuildHelper.excludeSystemPath()); } else { - Iterator<String> pathIterator = allowedLocations.iterator(); - baseQuery.append(" ("); - String sep = ""; - while (pathIterator.hasNext()) { - String prefix = pathIterator.next(); - baseQuery - .append(sep) - .append("isdescendantnode('") - .append(QueryBuildHelper.escapeString(prefix)) - .append("')"); - sep = " OR "; - } - baseQuery.append(")"); + baseQuery.append(allowedLocations.stream() + .map(location -> "isdescendantnode('" + QueryBuildHelper.escapeString(location) + "')") + .collect(Collectors.joining(" OR ", "(", ")"))); } baseQuery.append(" AND [sling:alias] IS NOT NULL"); diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java index ec6bfc75..6a7576e6 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.function.UnaryOperator; import org.apache.sling.api.resource.Resource; +import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.api.resource.mapping.ResourceMapper; import org.apache.sling.resourceresolver.impl.JcrNamespaceMangler; import org.apache.sling.resourceresolver.impl.ResourceResolverImpl; @@ -35,7 +36,6 @@ import org.apache.sling.resourceresolver.impl.helper.ResourceDecoratorTracker; import org.apache.sling.resourceresolver.impl.helper.URI; import org.apache.sling.resourceresolver.impl.helper.URIException; import org.apache.sling.resourceresolver.impl.params.ParsedParameters; -import org.jetbrains.annotations.NotNull; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -211,44 +211,35 @@ public class ResourceMapperImpl implements ResourceMapper { return mappedPaths; } - /* - * Populate a {@linkplain PathGenerator} based on the aliases of this resource, plus all ancestors - * @param resource the resource from which to start - * @param pathGenerator path generator to populate - */ - private void resolveAliases(@NotNull Resource resource, @NotNull PathGenerator pathGenerator) { - Resource current = resource; - - while (current != null) { - String name = current.getName(); - - // read aliases only if it's not a jcr:content resource - Collection<String> aliases = name.equals("jcr:content") ? Collections.emptyList() : readAliases(current); + private void resolveAliases(Resource res, PathGenerator pathBuilder) { + String path = res.getPath(); + while (path != null) { + Collection<String> aliases = Collections.emptyList(); + // read alias only if we can read the resources and it's not a jcr:content leaf + if (!path.endsWith(ResourceResolverImpl.JCR_CONTENT_LEAF)) { + aliases = readAliases(path); + } // build the path from the name segments or aliases - pathGenerator.insertSegment(aliases, name); - - // traverse up - current = current.getParent(); - - // reached the root? -> stop traversing up - if (current != null && current.getParent() == null) { - current = null; + pathBuilder.insertSegment(aliases, ResourceUtil.getName(path)); + path = ResourceUtil.getParent(path); + if ("/".equals(path)) { + path = null; } } } /** * Resolve the aliases for the given resource by a lookup in MapEntries - * @param resource resource for which to lookup aliases + * @param path path for which to lookup aliases * @return collection of aliases for that resource */ - private @NotNull Collection<String> readAliases(@NotNull Resource resource) { - Resource parent = resource.getParent(); - if (parent == null) { + private Collection<String> readAliases(String path) { + String parentPath = ResourceUtil.getParent(path); + if (parentPath == null) { return Collections.emptyList(); } else { - return mapEntries.getAliasMap(parent).getOrDefault(resource.getName(), Collections.emptyList()); + return mapEntries.getAliasMap(parentPath).getOrDefault(ResourceUtil.getName(path), Collections.emptyList()); } } diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java index 57aad76e..0f9dfc6f 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java @@ -40,7 +40,6 @@ import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceMetadata; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.resource.ResourceResolverFactory; -import org.apache.sling.api.resource.ResourceUtil; import org.apache.sling.api.resource.ValueMap; import org.apache.sling.api.wrappers.ValueMapDecorator; import org.apache.sling.resourceresolver.impl.mapping.MapEntries; @@ -500,7 +499,7 @@ public class MockedResourceResolverImplTest { } /** - * Build a resource with parent, path, children and resource resolver. + * Build a resource with path, children and resource resolver. * @param fullpath * @param children * @param resourceResolver @@ -513,18 +512,9 @@ public class MockedResourceResolverImplTest { ResourceResolver resourceResolver, ResourceProvider<?> provider, String... properties) { - - // build a mocked parent resource so that getParent() can return something meaningful (it is null when we are - // already at root level) - Resource parentResource = fullpath == null || "/".equals(fullpath) - ? null - : buildResource( - ResourceUtil.getParent(fullpath), Collections.emptyList(), resourceResolver, provider, null); - Resource resource = mock(Resource.class); Mockito.when(resource.getName()).thenReturn(getResourceName(fullpath)); Mockito.when(resource.getPath()).thenReturn(fullpath); - Mockito.when(resource.getParent()).thenReturn(parentResource); ResourceMetadata resourceMetadata = new ResourceMetadata(); Mockito.when(resource.getResourceMetadata()).thenReturn(resourceMetadata); Mockito.when(resource.listChildren()).thenReturn(children.iterator()); diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecorationTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecorationTest.java index 11b7f660..c85ba4f5 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecorationTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceDecorationTest.java @@ -33,7 +33,7 @@ import static org.junit.Assert.assertTrue; public class ResourceDecorationTest extends ResourceDecoratorTestBase { private static final String DECORATED_NAME = "decorated"; - private static final String DECORATED_PATH = "/decorated"; + private static final String DECORATED_PATH = "/decoratedPath"; /** Wrap any resource so that its name is DECORATED_NAME */ protected Resource wrapResourceForTest(Resource resource) { diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java index 68f42126..53a8cc9d 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java @@ -18,12 +18,14 @@ */ package org.apache.sling.resourceresolver.impl.mapping; +import java.io.IOException; import java.lang.reflect.Field; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -31,7 +33,11 @@ import java.util.Optional; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; +import java.util.regex.Matcher; +import java.util.regex.Pattern; +import org.apache.sling.api.resource.LoginException; +import org.apache.sling.api.resource.QuerySyntaxException; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.resource.path.Path; @@ -170,23 +176,34 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { method.invoke(mapEntries, path, bool); } - private void internal_test_simple_alias_support(boolean onJcrContent) { - prepareMapEntriesForAlias(onJcrContent, false, "alias"); + private void internal_test_simple_alias_support(boolean onJcrContent, boolean cached) { + Resource parent = prepareMapEntriesForAlias(onJcrContent, false, !cached, false, "alias"); mapEntries.ah.initializeAliases(); - Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); - assertNotNull(aliasMap); - assertTrue(aliasMap.containsKey("child")); - assertEquals(List.of("alias"), aliasMap.get("child")); + + Map<String, Collection<String>> aliasMapString = mapEntries.getAliasMap("/parent"); + assertNotNull(aliasMapString); + assertTrue(aliasMapString.containsKey("child")); + assertEquals(List.of("alias"), aliasMapString.get("child")); + + Map<String, Collection<String>> aliasMapResource = mapEntries.getAliasMap(parent); + assertNotNull(aliasMapResource); + assertTrue(aliasMapResource.containsKey("child")); + assertEquals(List.of("alias"), aliasMapResource.get("child")); } @Test public void test_simple_alias_support() { - internal_test_simple_alias_support(false); + internal_test_simple_alias_support(false, true); + } + + @Test + public void test_simple_alias_support_uncached() { + internal_test_simple_alias_support(false, false); } @Test public void test_simple_alias_support_on_jcr_content() { - internal_test_simple_alias_support(true); + internal_test_simple_alias_support(true, true); } private void internal_test_simple_multi_alias_support(boolean onJcrContent) { @@ -200,11 +217,18 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { @Test public void internal_test_simple_alias_support_throwing_unsupported_operation_exception_exception() { - prepareMapEntriesForAlias(false, false, UnsupportedOperationException.class, "foo", "bar"); + prepareMapEntriesForAlias(false, false, true, false, "foo", "bar"); mapEntries.ah.initializeAliases(); assertFalse(mapEntries.ah.usesCache()); } + @Test + public void internal_test_simple_alias_support_throwing_query_syntax_exception_exception() { + prepareMapEntriesForAlias(false, false, false, true, "foo", "bar"); + mapEntries.ah.initializeAliases(); + assertTrue(mapEntries.ah.usesCache()); + } + @Test public void test_simple_multi_alias_support() { internal_test_simple_multi_alias_support(false); @@ -250,43 +274,69 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { } private void prepareMapEntriesForAlias(boolean onJcrContent, boolean withNullParent, String... aliases) { - prepareMapEntriesForAlias(onJcrContent, withNullParent, null, aliases); + prepareMapEntriesForAlias(onJcrContent, withNullParent, false, false, aliases); } - private void prepareMapEntriesForAlias( + private Resource prepareMapEntriesForAlias( boolean onJcrContent, boolean withNullParent, - Class<? extends Exception> queryThrowsWith, + boolean queryAlwaysThrows, + boolean pagedQueryThrows, String... aliases) { Resource parent = mock(Resource.class); - when(parent.getPath()).thenReturn("/parent"); + Resource result = mock(Resource.class); + Resource content = mock(Resource.class); - final Resource result = mock(Resource.class); - final Resource content = mock(Resource.class); - final Resource aliasResource = onJcrContent ? content : result; + when(parent.getChildren()).thenReturn(Set.of(result)); + when(result.getParent()).thenReturn(null); // should be root + when(parent.getPath()).thenReturn("/parent"); + when(parent.getName()).thenReturn("parent"); + when(result.getChildren()).thenReturn(Set.of(content)); when(result.getParent()).thenReturn(withNullParent && !onJcrContent ? null : parent); when(result.getPath()).thenReturn("/parent/child"); when(result.getName()).thenReturn("child"); + when(content.getChildren()).thenReturn(Set.of()); when(content.getParent()).thenReturn(withNullParent && onJcrContent ? null : result); when(content.getPath()).thenReturn("/parent/child/jcr:content"); when(content.getName()).thenReturn("jcr:content"); + Resource aliasResource = onJcrContent ? content : result; + when(aliasResource.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, aliases)); - if (queryThrowsWith == null) { - when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))) - .thenAnswer((Answer<Iterator<Resource>>) invocation -> { - if (invocation.getArguments()[0].toString().contains(ResourceResolverImpl.PROP_ALIAS)) { + when(resourceResolver.getResource(anyString())).thenAnswer(invocation -> { + String path = invocation.getArgument(0); + if (path.equals(parent.getPath())) { + return parent; + } else if (path.equals(result.getPath())) { + return result; + } else if (path.equals(content.getPath())) { + return content; + } else { + return null; + } + }); + + when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))) + .thenAnswer((Answer<Iterator<Resource>>) invocation -> { + String query = invocation.getArgument(0); + if (queryAlwaysThrows) { + throw new UnsupportedOperationException("test case configured to always throw: " + query); + } else { + if (pagedQueryThrows && matchesPagedQuery(query)) { + throw new QuerySyntaxException( + "test case configured to throw for paged queries", query, "JCR-SQL2"); + } else if (query.equals(AQ_SIMPLE) || matchesPagedQuery(query)) { return List.of(aliasResource).iterator(); } else { - return Collections.emptyIterator(); + throw new RuntimeException("unexpected query: " + query); } - }); - } else { - when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))).thenThrow(queryThrowsWith); - } + } + }); + + return parent; } @Test @@ -308,7 +358,8 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))) .thenAnswer((Answer<Iterator<Resource>>) invocation -> { - if (invocation.getArguments()[0].toString().contains(ResourceResolverImpl.PROP_ALIAS)) { + String query = invocation.getArgument(0); + if (query.equals(AQ_SIMPLE) || matchesPagedQuery(query)) { return Arrays.asList(result, secondResult).iterator(); } else { return Collections.emptyIterator(); @@ -345,7 +396,8 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))) .thenAnswer((Answer<Iterator<Resource>>) invocation -> { - if (invocation.getArguments()[0].toString().contains(ResourceResolverImpl.PROP_ALIAS)) { + String query = invocation.getArgument(0); + if (query.equals(AQ_SIMPLE) || matchesPagedQuery(query)) { return List.of(node, content).iterator(); } else { return Collections.emptyIterator(); @@ -380,6 +432,30 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { assertEquals("/content", actualContent); } + @Test + public void test_allowed_locations_query() throws LoginException, IOException { + when(resourceResolverFactory.getAllowedAliasLocations()).thenReturn(Set.of("/a", "/'b'")); + Set<String> queryMade = new HashSet<>(); + when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))) + .thenAnswer((Answer<Iterator<Resource>>) invocation -> { + String query = invocation.getArgument(0); + if (query.contains("alias")) { + queryMade.add(query); + } + return Collections.emptyIterator(); + }); + + new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics); + + assertTrue("seems no alias query was made", !queryMade.isEmpty()); + String match1 = "(isdescendantnode('/a') OR isdescendantnode('/''b'''))"; + String match2 = "(isdescendantnode('/''b''') OR isdescendantnode('/a'))"; + String actual = queryMade.iterator().next(); + assertTrue( + "query should contain '" + match1 + "' (or reversed), but was: '" + actual + "'", + actual.contains(match1) || actual.contains(match2)); + } + // SLING-3727 @Test public void test_doAddAliasAttributesWithDisableAliasOptimization() throws Exception { @@ -1228,4 +1304,36 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { ah.initializeAliases(); assertFalse("alias handler should not have set up cache", ah.usesCache()); } + + // utilities for testing alias queries + + // used for paged query of all + private static final String AQ_PAGED_START = + "SELECT [sling:alias] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:alias] IS NOT NULL AND FIRST([sling:alias]) >= '"; + private static final String AQ_PAGED_END = "' ORDER BY FIRST([sling:alias])"; + + private static final Pattern AQ_PAGED_PATTERN = + Pattern.compile(Pattern.quote(AQ_PAGED_START) + "(?<path>\\p{Alnum}*)" + Pattern.quote(AQ_PAGED_END)); + + // used when paged query not available + private static final String AQ_SIMPLE = + "SELECT [sling:alias] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:alias] IS NOT NULL"; + + // sanity test on matcher + @Test + public void testMatcher() { + assertTrue(AQ_PAGED_PATTERN.matcher(AQ_PAGED_START + AQ_PAGED_END).matches()); + assertTrue( + AQ_PAGED_PATTERN.matcher(AQ_PAGED_START + "xyz" + AQ_PAGED_END).matches()); + assertEquals( + 1, + AQ_PAGED_PATTERN.matcher(AQ_PAGED_START + "xyz" + AQ_PAGED_END).groupCount()); + Matcher m1 = AQ_PAGED_PATTERN.matcher(AQ_PAGED_START + "xyz" + AQ_PAGED_END); + assertTrue(m1.find()); + assertEquals("xyz", m1.group("path")); + } + + private boolean matchesPagedQuery(String query) { + return AQ_PAGED_PATTERN.matcher(query).matches(); + } }
