This is an automated email from the ASF dual-hosted git repository.
joerghoh pushed a commit to branch SLING-11558-return-resources
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-servlets-resolver.git
The following commit(s) were added to refs/heads/SLING-11558-return-resources
by this push:
new dcf9d93 Use the cacheMap
dcf9d93 is described below
commit dcf9d93bbc98e391fd322e5b6d81b5c7b8722c69
Author: Jörg Hoh <[email protected]>
AuthorDate: Sat Jun 10 17:54:17 2023 +0200
Use the cacheMap
before resolving a resource check if it has not yet done before. This
map shares the lifetime of the ResourceResolver, so a single instance
can be used on many servlet resolutions.
---
pom.xml | 8 +-
.../internal/helper/LocationCollector.java | 110 +++++++++++++++------
.../internal/helper/LocationCollectorTest.java | 90 +++++++++++++----
3 files changed, 157 insertions(+), 51 deletions(-)
diff --git a/pom.xml b/pom.xml
index b77ab75..0989ffb 100644
--- a/pom.xml
+++ b/pom.xml
@@ -316,10 +316,16 @@
<version>3.4.10</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+
<artifactId>org.apache.sling.testing.resourceresolver-mock</artifactId>
+ <version>1.4.2</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.testing.resourceresolver-mock</artifactId>
- <version>1.2.2</version>
+ <version>1.4.2</version>
<scope>test</scope>
</dependency>
<dependency>
diff --git
a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java
b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java
index ebf2478..284ba38 100644
---
a/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java
+++
b/src/main/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollector.java
@@ -19,8 +19,10 @@
package org.apache.sling.servlets.resolver.internal.helper;
import java.util.ArrayList;
+import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
+import java.util.Map;
import java.util.Set;
import org.apache.commons.lang3.StringUtils;
@@ -54,29 +56,13 @@ import org.slf4j.LoggerFactory;
class LocationCollector {
- static @NotNull List<Resource> getLocations(String resourceType, String
resourceSuperType, String baseResourceType,
- ResourceResolver
resolver) {
- LocationCollector collector = new LocationCollector(resourceType,
resourceSuperType, baseResourceType, resolver);
- List<Resource> result = new ArrayList<>();
- collector.getResolvedLocations().forEach((location) -> {
- // get the location resource, use a synthetic resource if there
- // is no real location. There may still be children at this
- // location
- final String path;
- if ( location.endsWith("/") ) {
- path = location.substring(0, location.length() - 1);
- } else {
- path = location;
- }
- final Resource locationRes = getResource(resolver, path);
- result.add(locationRes);
- });
- return result;
- }
+ private static final String CACHE_KEY =
LocationCollector.class.getName() + ".CacheKey";
// The search path of the resource resolver
private final String[] searchPath;
+ private Map<String,Resource> cacheMap;
+
private final ResourceResolver resolver;
private final String baseResourceType;
private final String resourceType;
@@ -88,12 +74,13 @@ class LocationCollector {
private final List<String> result = new ArrayList<>();
private LocationCollector(String resourceType, String resourceSuperType,
String baseResourceType,
- ResourceResolver resolver) {
+ ResourceResolver resolver, Map<String,Resource>
cacheMap) {
this.resourceType = resourceType;
this.resourceSuperType = resourceSuperType;
this.baseResourceType = baseResourceType;
this.resolver = resolver;
+ this.cacheMap = cacheMap;
String[] tmpPath = resolver.getSearchPath();
if (tmpPath.length == 0) {
@@ -182,7 +169,7 @@ class LocationCollector {
&& this.resourceSuperType != null ) {
superType = this.resourceSuperType;
} else {
- superType = getResourceSuperType(resolver, resourceType);
+ superType = getResourceSuperTypeInternal(resourceType);
}
// detect circular dependency
@@ -198,15 +185,14 @@ class LocationCollector {
}
// this method is largely duplicated from ResourceUtil
- private @Nullable String getResourceSuperType(final @NotNull
ResourceResolver resourceResolver,
- final @NotNull String
resourceType) {
+ private @Nullable String getResourceSuperTypeInternal(final @NotNull
String resourceType) {
// normalize resource type to a path string
final String rtPath = ResourceUtil.resourceTypeToPath(resourceType);
// get the resource type resource and check its super type
String rst = null;
// if the path is absolute, use it directly
if (rtPath.startsWith("/")) {
- final Resource rtResource = resourceResolver.getResource(rtPath);
+ final Resource rtResource = resolveResource(rtPath);
if (rtResource != null) {
rst = rtResource.getResourceSuperType();
}
@@ -214,7 +200,7 @@ class LocationCollector {
// if the path is relative we use the search paths
for (final String path : this.searchPath) {
final String candidatePath = path + rtPath;
- final Resource rtResource =
resourceResolver.getResource(candidatePath);
+ final Resource rtResource = resolveResource(candidatePath);
if (rtResource != null && rtResource.getResourceSuperType() !=
null) {
rst = rtResource.getResourceSuperType();
break;
@@ -224,14 +210,74 @@ class LocationCollector {
return rst;
}
- protected static @NotNull Resource getResource(final ResourceResolver
resolver, String path) {
- Resource res = resolver.getResource(path);
-
- if (res == null) {
- res = new SyntheticResource(resolver, path,
"$synthetic$");
+ /**
+ * Resolve a path to a resource; the cacheMap is used for it.
+ * @param path the path
+ * @return the resource for it or null
+ */
+ private @Nullable Resource resolveResource (@NotNull String path) {
+ if (cacheMap.containsKey(path)) {
+ return cacheMap.get(path);
+ } else {
+ Resource r = resolver.getResource(path);
+ cacheMap.put(path, r);
+ return r;
+ }
+ }
+
+
+
+ // ---- static helpers ---
+
+ static @NotNull List<Resource> getLocations(String resourceType, String
resourceSuperType, String baseResourceType,
+ ResourceResolver resolver) {
+
+ @SuppressWarnings("unchecked")
+ Map<String,Resource> m = (Map<String,Resource>)
resolver.getPropertyMap().get(CACHE_KEY);
+ if (m == null) {
+ m = new HashMap<>();
+ resolver.getPropertyMap().put(CACHE_KEY, m);
+ }
+ final Map<String,Resource> cacheMap = m;
+
+ LocationCollector collector = new
LocationCollector(resourceType, resourceSuperType, baseResourceType,
+ resolver, cacheMap);
+ List<Resource> result = new ArrayList<>();
+ collector.getResolvedLocations().forEach((location) -> {
+ // get the location resource, use a synthetic resource
if there
+ // is no real location. There may still be children at
this
+ // location
+ final String path;
+ if (location.endsWith("/")) {
+ path = location.substring(0, location.length()
- 1);
+ } else {
+ path = location;
+ }
+ final Resource locationRes = getResource(resolver,
path, cacheMap);
+ result.add(locationRes);
+ });
+ return result;
+ }
+
+ /**
+ * Resolve a path to a resource
+ * @param resolver
+ * @param path
+ * @return a synthetic or "real" resource
+ */
+ protected static @NotNull Resource getResource(final @NotNull
ResourceResolver resolver,
+ @NotNull String path, @NotNull Map<String,Resource>
cacheMap) {
+
+ if (cacheMap.containsKey(path) && cacheMap.get(path) != null) {
+ return cacheMap.get(path);
+ } else {
+ Resource res = resolver.getResource(path);
+ if (res == null) {
+ res = new SyntheticResource(resolver, path,
"$synthetic$");
+ }
+ cacheMap.put(path, res);
+ return res;
}
-
- return res;
}
}
diff --git
a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollectorTest.java
b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollectorTest.java
index 39962ee..0512793 100644
---
a/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollectorTest.java
+++
b/src/test/java/org/apache/sling/servlets/resolver/internal/helper/LocationCollectorTest.java
@@ -86,21 +86,6 @@ public class LocationCollectorTest {
request.setResource(resource);
}
-
- List<Resource> getLocations(final String resourceType,
- final String resourceSuperType) {
- return getLocations(resourceType, resourceSuperType,
DEFAULT_RESOURCE_TYPE);
- }
-
- List<Resource> getLocations( final String resourceType,
- final String resourceSuperType,
- final String baseResourceType) {
-
- return LocationCollector.getLocations(resourceType,
- resourceSuperType,
- baseResourceType,
- resolver);
- }
@Test
public void testSearchPathEmpty() {
@@ -134,6 +119,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath2Elements() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -175,6 +161,7 @@ public class LocationCollectorTest {
request.setResource(r);
}
+ @Test
public void testSearchPathEmptyAbsoluteType() {
// expect path gets { "/" }
searchPathOptions.setSearchPaths(null);
@@ -194,6 +181,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath1ElementAbsoluteType() {
String root0 = "/apps/";
searchPathOptions.setSearchPaths(new String[] {
@@ -216,6 +204,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath2ElementsAbsoluteType() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -240,6 +229,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPathEmptyWithSuper() {
// expect path gets { "/" }
searchPathOptions.setSearchPaths(null);
@@ -260,6 +250,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath1ElementWithSuper() {
String root0 = "/apps/";
searchPathOptions.setSearchPaths(new String[] {
@@ -282,6 +273,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath2ElementsWithSuper() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -309,6 +301,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPathEmptyAbsoluteTypeWithSuper() {
// expect path gets { "/" }
searchPathOptions.setSearchPaths(null);
@@ -333,6 +326,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath1ElementAbsoluteTypeWithSuper() {
String root0 = "/apps/";
searchPathOptions.setSearchPaths(new String[] {
@@ -359,6 +353,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testSearchPath2ElementsAbsoluteTypeWithSuper() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -389,6 +384,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testScriptNameWithoutResourceType() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -405,6 +401,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testScriptNameWithResourceType() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -422,6 +419,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testScriptNameWithResourceTypeAndSuperType() {
String root0 = "/apps/";
String root1 = "/libs/";
@@ -442,6 +440,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+ @Test
public void testCircularResourceTypeHierarchy() {
final String root1 = "/libs/";
searchPathOptions.setSearchPaths(new String[] {
@@ -487,9 +486,14 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
-
+ @Test
public void testResolveDefaultResourceType() {
+ searchPathOptions.setSearchPaths(new String[] {
+ "/apps/",
+ "/libs/"
+ });
+
List<Resource> loc = getLocations(DEFAULT_RESOURCE_TYPE,
resourceSuperType);
List<Resource> expected = Arrays.asList(
@@ -500,7 +504,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
-
+ @Test
public void testAbsoluteResourceSuperType() throws Exception {
final String root = "/apps/";
searchPathOptions.setSearchPaths(new String[] {
@@ -533,7 +537,7 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
-
+ @Test
public void testNoSuperType() throws Exception {
final String root = "/apps/";
searchPathOptions.setSearchPaths(new String[] {
@@ -559,11 +563,61 @@ public class LocationCollectorTest {
assertThat(loc,isSameResourceList(expected));
}
+
+ @Test
+ public void checkThatTheCacheIsUsed() {
+
+ // The basic test setup is copied from testSearchPath2ElementsWithSuper
+ String root0 = "/apps/";
+ String root1 = "/libs/";
+ searchPathOptions.setSearchPaths(new String[] {
+ root0,
+ root1
+ });
+
+ // set resource super type
+ resourceSuperType = "foo:superBar";
+ resourceSuperTypePath =
ResourceUtil.resourceTypeToPath(resourceSuperType);
+ replaceResource(null, resourceSuperType);
+
+ final Resource r = request.getResource();
+
+ // Execute the same call twice and expect that on 2nd time the
ResourceResolver
+ // is never used, because all is taken from the cache
+ getLocations(r.getResourceType(),
+ r.getResourceSuperType());
+
+ Mockito.clearInvocations(resolver);
+
+ getLocations(r.getResourceType(),
+ r.getResourceSuperType());
+
+ Mockito.verify(resolver,
Mockito.never()).getResource(Mockito.anyString());
+ }
+
+
+ // --- helper ---
+
private Resource r (String path) {
return new SyntheticResource(resolver, path, "resourcetype");
}
+ List<Resource> getLocations(final String resourceType,
+ final String resourceSuperType) {
+ return getLocations(resourceType, resourceSuperType,
DEFAULT_RESOURCE_TYPE);
+ }
+
+ List<Resource> getLocations( final String resourceType,
+ final String resourceSuperType,
+ final String baseResourceType) {
+
+ return LocationCollector.getLocations(resourceType,
+ resourceSuperType,
+ baseResourceType,
+ resolver);
+ }
+
// Mimic the searchpath semantic of the ResourceResolverFactory
public class SearchPathOptions {
