This is an automated email from the ASF dual-hosted git repository. stefanseifert pushed a commit to branch issue/SLING-13326-1.x in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 682da4c8bfff3389573f2cfc953f11f51aeac660 Author: Stefan Seifert <[email protected]> AuthorDate: Tue Oct 6 09:11:21 2026 +0200 SLING-13326 Fix caching for getParentResourceType (backport to 1.x) --- .../impl/ResourceResolverImpl.java | 23 ++-- .../impl/ResourceResolverImplTest.java | 122 +++++++++++++++++---- 2 files changed, 109 insertions(+), 36 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 cbd56019..ccf6d2de 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java @@ -97,8 +97,8 @@ public class ResourceResolverImpl extends SlingAdaptable implements ResourceReso protected final Map<ResourceTypeInformation, Boolean> resourceTypeLookupCache = new ConcurrentHashMap<>(); - // Store the resourceSupertype mapping (supertype can be null) - protected final Map<String, Optional<String>> parentResourceTypeMap = new ConcurrentHashMap<>(); + // Cache by resource type and declared supertype, including null results. + protected final Map<ResourceTypeInformation, Optional<String>> parentResourceTypeMap = new ConcurrentHashMap<>(); private Map<String, Object> propertyMap; @@ -1105,17 +1105,14 @@ public class ResourceResolverImpl extends SlingAdaptable implements ResourceReso @Override public String getParentResourceType(final Resource resource) { checkClosed(); - String resourceSuperType = null; - if (resource != null) { - if (parentResourceTypeMap.containsKey(resource.getPath())) { - resourceSuperType = - parentResourceTypeMap.get(resource.getPath()).orElse(null); - } else { - resourceSuperType = getParentResourceTypeInternal(resource); - parentResourceTypeMap.put(resource.getPath(), Optional.ofNullable(resourceSuperType)); - } + if (resource == null) { + return null; } - return resourceSuperType; + final ResourceTypeInformation key = + new ResourceTypeInformation(resource.getResourceType(), resource.getResourceSuperType(), null); + return parentResourceTypeMap + .computeIfAbsent(key, k -> Optional.ofNullable(getParentResourceTypeInternal(resource))) + .orElse(null); } String getParentResourceTypeInternal(final Resource resource) { @@ -1250,7 +1247,7 @@ public class ResourceResolverImpl extends SlingAdaptable implements ResourceReso return propertyMap; } - // Simple pojo acting as key for the resourceTypeLookupCache + // Simple pojo acting as key for the resource type caches public class ResourceTypeInformation { String s1; diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java index 29fe6dd6..c6856029 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java @@ -39,6 +39,7 @@ import org.apache.sling.api.resource.PersistenceException; import org.apache.sling.api.resource.Resource; import org.apache.sling.api.resource.ResourceResolver; import org.apache.sling.api.resource.ResourceResolverFactory; +import org.apache.sling.api.resource.ResourceWrapper; import org.apache.sling.api.resource.SyntheticResource; import org.apache.sling.api.security.ResourceAccessSecurity; import org.apache.sling.resourceresolver.impl.providers.ResourceProviderHandler; @@ -51,7 +52,6 @@ import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.junit.Before; import org.junit.Test; -import org.mockito.Mockito; import org.osgi.framework.Bundle; import static java.util.Arrays.asList; @@ -63,10 +63,11 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; public class ResourceResolverImplTest { @@ -563,7 +564,7 @@ public class ResourceResolverImplTest { // correct } // same RP supporting and allowing orderBefore - when(ras.canOrderChildren(Mockito.any())).thenReturn(true); + when(ras.canOrderChildren(any())).thenReturn(true); assertTrue(resResolver.orderBefore(root, "test", "test2")); } @@ -596,19 +597,19 @@ public class ResourceResolverImplTest { @Test public void testIsResourceTypeCached() throws Exception { - final PathBasedResourceResolverImpl resolver = Mockito.spy(getPathBasedResourceResolver()); + final PathBasedResourceResolverImpl resolver = spy(getPathBasedResourceResolver()); final Resource r1 = resolver.add(new SyntheticResource(resolver, "/a", "a:b")); final Resource r2 = resolver.add(new SyntheticResourceWithSupertype(resolver, "/b", "a:b", "c:d")); // 1st lookup needs to get through, 2nd will be taken from cache assertTrue(resolver.isResourceType(r1, "a:b")); - Mockito.verify(resolver, Mockito.times(1)).isResourceTypeInternal(eq(r1), eq("a:b")); + verify(resolver, times(1)).isResourceTypeInternal(r1, "a:b"); assertTrue(resolver.isResourceType(r1, "a:b")); - Mockito.verify(resolver, Mockito.times(1)).isResourceTypeInternal(eq(r1), eq("a:b")); + verify(resolver, times(1)).isResourceTypeInternal(r1, "a:b"); resolver.refresh(); assertTrue(resolver.isResourceType(r1, "a:b")); - Mockito.verify(resolver, Mockito.times(2)).isResourceTypeInternal(eq(r1), eq("a:b")); + verify(resolver, times(2)).isResourceTypeInternal(r1, "a:b"); // make sure that resources with the same resourceType but different resourceSuperType are // treated differently @@ -737,18 +738,11 @@ public class ResourceResolverImplTest { // use the propertyMap resolver = getPathBasedResourceResolver(); Object value1 = new String("value1"); - Closeable value2 = Mockito.spy(new Closeable() { - @Override - public void close() { - // do nothing - } - }); - Closeable valueWithException = Mockito.spy(new Closeable() { - @Override - public void close() { - throw new RuntimeException("RuntimeExceptions in close must be handled"); - } - }); + Closeable value2 = mock(Closeable.class); + Closeable valueWithException = mock(Closeable.class); + doThrow(new RuntimeException("RuntimeExceptions in close must be handled")) + .when(valueWithException) + .close(); assertNotNull(resolver.getPropertyMap()); resolver.getPropertyMap().put("key1", value1); resolver.getPropertyMap().put("key2", value2); @@ -757,13 +751,13 @@ public class ResourceResolverImplTest { resolver.close(); assertNotNull(resolver.getPropertyMap()); assertTrue(resolver.getPropertyMap().isEmpty()); - Mockito.verify(value2, Mockito.times(1)).close(); - Mockito.verify(valueWithException, Mockito.times(1)).close(); + verify(value2, times(1)).close(); + verify(valueWithException, times(1)).close(); } @Test public void testGetParentResourceType() { - final PathBasedResourceResolverImpl resolver = Mockito.spy(getPathBasedResourceResolver()); + final PathBasedResourceResolverImpl resolver = spy(getPathBasedResourceResolver()); resolver.add(new SyntheticResourceWithSupertype(resolver, "/types/1", "/types/component", "/types/2")); resolver.add(new SyntheticResourceWithSupertype(resolver, "/content/type1", "/types/1", null)); @@ -773,7 +767,89 @@ public class ResourceResolverImplTest { // Ensure that the next call will be served from the cache resolver.getParentResourceType(resolver.getResource("/types/1")); - Mockito.verify(resolver, times(2)).getParentResourceTypeInternal(any(Resource.class)); + verify(resolver, times(2)).getParentResourceTypeInternal(any(Resource.class)); + } + + /** + * @see <a href="https://issues.apache.org/jira/browse/SLING-13326";>SLING-13326</a> + */ + @Test + public void testGetParentResourceTypeWithOverriddenResourceType() { + final PathBasedResourceResolverImpl resolver = getPathBasedResourceResolver(); + + resolver.add(new SyntheticResourceWithSupertype( + resolver, "/foo/pages/special", "types/component", "foo/pages/base")); + resolver.add(new SyntheticResourceWithSupertype( + resolver, "/foo/pages/base", "types/component", "generic/pages/page")); + resolver.add(new SyntheticResource(resolver, "/generic/pages/page", "types/component")); + final Resource resource = resolver.add(new SyntheticResource(resolver, "/content/foo", "foo/pages/special")); + final Resource decorated = new ResourceWrapper(resource) { + @Override + public String getResourceType() { + return "foo/pages/base"; + } + }; + + assertEquals(resource.getPath(), decorated.getPath()); + assertEquals("foo/pages/base", resolver.getParentResourceType(resource)); + assertEquals( + "The cached parent type must reflect the wrapper's overridden resource type", + "generic/pages/page", + resolver.getParentResourceType(decorated)); + assertEquals("foo/pages/base", resolver.getParentResourceType(resource)); + } + + @Test + public void testGetParentResourceTypeWithOverriddenResourceSuperType() { + final PathBasedResourceResolverImpl resolver = getPathBasedResourceResolver(); + resolver.add(new SyntheticResourceWithSupertype(resolver, "/types/1", "/types/component", "/types/2")); + final Resource resource = resolver.add(new SyntheticResource(resolver, "/content/type1", "/types/1")); + final Resource decorated = new ResourceWrapper(resource) { + @Override + public String getResourceSuperType() { + return "/types/3"; + } + }; + + assertEquals("/types/2", resolver.getParentResourceType(resource)); + assertEquals("/types/3", resolver.getParentResourceType(decorated)); + assertEquals("/types/2", resolver.getParentResourceType(resource)); + } + + @Test + public void testGetParentResourceTypeWithCachedNull() { + final PathBasedResourceResolverImpl resolver = spy(getPathBasedResourceResolver()); + resolver.add(new SyntheticResourceWithSupertype(resolver, "/types/1", "/types/component", "/types/2")); + final Resource resource = resolver.add(new SyntheticResource(resolver, "/content/type1", "/types/unknown")); + final Resource decorated = new ResourceWrapper(resource) { + @Override + public String getResourceType() { + return "/types/1"; + } + }; + + assertNull(resolver.getParentResourceType((Resource) null)); + assertNull(resolver.getParentResourceType(resource)); + assertNull(resolver.getParentResourceType(resource)); + verify(resolver, times(1)).getParentResourceTypeInternal(resource); + assertEquals("/types/2", resolver.getParentResourceType(decorated)); + assertNull(resolver.getParentResourceType(resource)); + verify(resolver, times(1)).getParentResourceTypeInternal(resource); + } + + @Test + public void testGetParentResourceTypeCacheClearedOnRefresh() { + final PathBasedResourceResolverImpl resolver = spy(getPathBasedResourceResolver()); + final Resource resource = + resolver.add(new SyntheticResourceWithSupertype(resolver, "/content/type1", "/types/1", "/types/2")); + + assertEquals("/types/2", resolver.getParentResourceType(resource)); + assertEquals("/types/2", resolver.getParentResourceType(resource)); + verify(resolver, times(1)).getParentResourceTypeInternal(resource); + + resolver.refresh(); + assertEquals("/types/2", resolver.getParentResourceType(resource)); + verify(resolver, times(2)).getParentResourceTypeInternal(resource); } private PathBasedResourceResolverImpl getPathBasedResourceResolver() {
