This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
The following commit(s) were added to refs/heads/master by this push:
new d9e90e4 SLING-11715 in the optimized code path avoid resource
resolution (#101)
d9e90e4 is described below
commit d9e90e455c0f71e84414bb09c83d7e678f1a788e
Author: Jörg Hoh <[email protected]>
AuthorDate: Thu Oct 5 14:43:45 2023 +0200
SLING-11715 in the optimized code path avoid resource resolution (#101)
Refactored the code path of the optimized alias resolution to not resolve
resources anymore. This refactoring does not change the logic at all, but
removes redundant repository access.
---
.../impl/mapping/ResourceMapperImpl.java | 122 +++++++++++++--------
.../impl/mapping/ResourceMapperImplTest.java | 54 ++++++++-
2 files changed, 131 insertions(+), 45 deletions(-)
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 3e02101..f1e3dcb 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
@@ -203,23 +203,7 @@ public class ResourceMapperImpl implements ResourceMapper {
// make sure to append resolutionPathInfo, if present
pathBuilder.setResolutionPathInfo(resolutionPathInfo);
- Resource current = res;
- String path = res.getPath();
- while (path != null) {
- List<String> aliases = Collections.emptyList();
- // read alias only if we can read the resources and it's not a
jcr:content leaf
- if (current != null &&
!path.endsWith(ResourceResolverImpl.JCR_CONTENT_LEAF)) {
- aliases = readAliases(path, current);
- }
- // build the path from the name segments or aliases
- pathBuilder.insertSegment(aliases, ResourceUtil.getName(path));
- path = ResourceUtil.getParent(path);
- if ("/".equals(path)) {
- path = null;
- } else if (path != null) {
- current = res.getResourceResolver().resolve(path);
- }
- }
+ resolveAliases(res, pathBuilder);
// and then we have the mapped path to work on
List<String> mappedPaths = pathBuilder.generatePaths();
@@ -232,34 +216,86 @@ public class ResourceMapperImpl implements ResourceMapper
{
return mappedPaths;
}
+
+ private void resolveAliases(Resource res, PathGenerator pathBuilder) {
+ Resource current = res;
+ String path = res.getPath();
+ if (this.mapEntries.isOptimizeAliasResolutionEnabled()) {
+ // this code path avoids any creation of Sling Resource objects
+ while (path != null) {
+ List<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 = readAliasesOptimized(path);
+ }
+ // build the path from the name segments or aliases
+ pathBuilder.insertSegment(aliases,
ResourceUtil.getName(path));
+ path = ResourceUtil.getParent(path);
+ if ("/".equals(path)) {
+ path = null;
+ }
+ }
+ }
+ else {
+ // while here there Resources are resolved
+ while (path != null) {
+ List<String> aliases = Collections.emptyList();
+ // read alias only if we can read the resources and it's
not a jcr:content leaf
+ if (current != null &&
!path.endsWith(ResourceResolverImpl.JCR_CONTENT_LEAF)) {
+ aliases = readAliases(path, current);
+ }
+ // build the path from the name segments or aliases
+ pathBuilder.insertSegment(aliases,
ResourceUtil.getName(path));
+ path = ResourceUtil.getParent(path);
+ if ("/".equals(path)) {
+ path = null;
+ } else if (path != null) {
+ current = resolver.resolve(path);
+ }
+ }
+ }
+ }
+ /**
+ * Resolve the aliases for the given resource by directly reading the
sling:alias property
+ * @param path the path of the resource
+ * @param current the resource
+ * @return
+ */
private List<String> readAliases(String path, Resource current) {
- if (this.mapEntries.isOptimizeAliasResolutionEnabled()) {
- logger.debug("map: Optimize Alias Resolution is Enabled");
- String parentPath = ResourceUtil.getParent(path);
-
- if ( parentPath == null )
- return Collections.emptyList();
-
- final Map<String, String> aliases =
mapEntries.getAliasMap(parentPath);
-
- if ( aliases == null || !aliases.containsValue(current.getName())
)
- return Collections.emptyList();
-
- return aliases.entrySet().stream()
- .filter( e -> current.getName().contentEquals(e.getValue()) )
- .map( Entry::getKey )
- .collect(Collectors.toList());
-
- } else {
- logger.debug("map: Optimize Alias Resolution is Disabled");
- String[] aliases = ResourceResolverControl.getProperty(current,
ResourceResolverImpl.PROP_ALIAS, String[].class);
- if ( aliases == null || aliases.length == 0 )
- return Collections.emptyList();
- if ( aliases.length == 1 )
- return Collections.singletonList(aliases[0]);
- return Arrays.asList(aliases);
- }
+ logger.debug("map: Optimize Alias Resolution is Disabled");
+ String[] aliases = ResourceResolverControl.getProperty(current,
ResourceResolverImpl.PROP_ALIAS, String[].class);
+ if ( aliases == null || aliases.length == 0 )
+ return Collections.emptyList();
+ if ( aliases.length == 1 )
+ return Collections.singletonList(aliases[0]);
+ return Arrays.asList(aliases);
+ }
+
+ /**
+ * Resolve teh aliases for the given resource by a lookup in the
mapEntries structure, avoiding
+ * any repository access
+ * @param path
+ * @return
+ */
+ private List<String> readAliasesOptimized(String path) {
+ logger.debug("map: Optimize Alias Resolution is Enabled");
+ String parentPath = ResourceUtil.getParent(path);
+ if ( parentPath == null ) {
+ return Collections.emptyList();
+ }
+ String name = ResourceUtil.getName(path);
+
+ final Map<String, String> aliases = mapEntries.getAliasMap(parentPath);
+
+ if ( aliases == null || !aliases.containsValue(name) )
+ return Collections.emptyList();
+
+ return aliases.entrySet().stream()
+ .filter( e -> name.contentEquals(e.getValue()) )
+ .map( Entry::getKey )
+ .collect(Collectors.toList());
+
}
private void populateMappingsFromMapEntries(List<String> mappings,
List<String> mappedPathList,
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java
index 4e57850..a1bd7d5 100644
---
a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java
@@ -24,10 +24,11 @@ import static
org.apache.sling.spi.resource.provider.ResourceProvider.PROPERTY_R
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertThat;
+import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
+import java.lang.reflect.Field;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashSet;
@@ -44,6 +45,7 @@ import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.mapping.ResourceMapper;
import org.apache.sling.resourceresolver.impl.ResourceAccessSecurityTracker;
import org.apache.sling.resourceresolver.impl.ResourceResolverFactoryActivator;
+import org.apache.sling.resourceresolver.impl.ResourceResolverImpl;
import org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl;
import org.apache.sling.spi.resource.provider.ResourceProvider;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
@@ -54,6 +56,8 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
+import org.mockito.Mockito;
+
import org.osgi.util.tracker.ServiceTracker;
/**
@@ -77,7 +81,7 @@ import org.osgi.util.tracker.ServiceTracker;
@RunWith(Parameterized.class)
public class ResourceMapperImplTest {
- @Parameters(name = "optimized alias resolution / paged query support ->
{0} / {1}")
+ @Parameters(name = "optimized alias resolution = {0} / paged query support
= {1}")
public static Collection<Object[]> data() {
return Arrays.asList(new Object[][] { { false, false }, { false, true
}, { true, false }, { true, true } });
}
@@ -112,6 +116,11 @@ public class ResourceMapperImplTest {
resourceProvider.putResource("/content1");
resourceProvider.putResource("/content1/jcr:content", PROP_ALIAS,
"jcr:content-alias"); // jcr:content resource
resourceProvider.putResource("/content");
+ resourceProvider.putResource("/content/very");
+ resourceProvider.putResource("/content/very/deep");
+ resourceProvider.putResource("/content/very/deep/path");
+ resourceProvider.putResource("/content/very/deep/path/with");
+ resourceProvider.putResource("/content/very/deep/path/with/resources");
resourceProvider.putResource("/content/virtual");
resourceProvider.putResource("/content/virtual/foo"); // matches
virtual.host.com.80 mapping entry
resourceProvider.putResource("/parent", PROP_ALIAS, "alias-parent");
// parent has alias
@@ -488,6 +497,47 @@ public class ResourceMapperImplTest {
.verify(resolver, req);
}
+ /**
+ * Validates that in case of the optimized lookup less repository access
is done
+ * @throws Exception
+ */
+ @Test
+ public void mapAliasLookupOptimization() throws Exception {
+ ResourceResolverImpl spyResolver = Mockito.spy((ResourceResolverImpl)
resolver);
+
+ // inject that spy into the mapper, so we can reason about the repo
access
+ ResourceMapperImpl mapper = (ResourceMapperImpl)
resolver.adaptTo(ResourceMapper.class);
+ Field internalResolver =
mapper.getClass().getDeclaredField("resolver");
+ internalResolver.setAccessible(true);
+ internalResolver.set(mapper,spyResolver);
+
+ assertResourceResolverAccess(spyResolver, mapper, "/parent/child"); //
alias on both parent and child
+ assertResourceResolverAccess(spyResolver, mapper,
"/alias-parent/alias-child"); // the path consists of 2 aliases
+ assertResourceResolverAccess(spyResolver, mapper,
"/content/very/deep/path/with/resources"); // deep path
+ }
+
+
+ /**
+ * validate the number of repository accesses by the previous operation
+ * @param spyResolver the resourceresolver
+ * @param mapper the ResourceMapper to use
+ * @param path the mapped path without trailing slash
+ */
+ private void assertResourceResolverAccess(ResourceResolverImpl
spyResolver, ResourceMapperImpl mapper, String path) {
+ int pathSegments = (int) path.chars().filter(c -> c == '/').count();
+ mapper.getMapping(path);
+ if (this.optimiseAliasResolution) {
+
Mockito.verify(spyResolver,Mockito.times(0)).resolve(Mockito.any(String.class));
+
Mockito.verify(spyResolver,Mockito.times(1)).resolveInternal(Mockito.any(String.class),Mockito.anyMap());
+ } else {
+
Mockito.verify(spyResolver,Mockito.times(pathSegments-1)).resolve(Mockito.any(String.class));
+
Mockito.verify(spyResolver,Mockito.times(pathSegments)).resolveInternal(Mockito.any(String.class),Mockito.anyMap());
+ }
+ Mockito.clearInvocations(spyResolver);
+ }
+
+
+
static class ExpectedMappings {
public static ExpectedMappings existingResource(String path) {
