(sling-org-apache-sling-resourceresolver) 02/02: SLING-13326 Fix caching for getParentResourceType

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joerghoh pushed a commit to branch SLING-13326
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git

commit 960381f9f8be5a35d94d47177d13df188786687c
Author: Joerg Hoh <jo...@apache.org>
AuthorDate: Sun Oct 4 13:06:10 2026 +0200

    SLING-13326 Fix caching for getParentResourceType
    
    Use the resource type and declared resource supertype as the cache key for the parentResourceTypeMap, rather than the resource path. This handles cases where a ResourceWrapper exposes an existing resource with a different resource type or supertype while retaining the same path.
---
 .../impl/ResourceResolverImpl.java                 | 23 ++++------
 .../impl/ResourceResolverImplTest.java             | 53 ++++++++++++++++++++++
 2 files changed, 63 insertions(+), 13 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 9f7b0248..cb1a3cfd 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java
@@ -96,8 +96,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;
 
@@ -1148,17 +1148,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) {
@@ -1293,7 +1290,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 46ef768d..36834226 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverImplTest.java
@@ -806,6 +806,59 @@ public class ResourceResolverImplTest {
         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 = Mockito.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));
+        Mockito.verify(resolver, times(1)).getParentResourceTypeInternal(resource);
+        assertEquals("/types/2", resolver.getParentResourceType(decorated));
+        assertNull(resolver.getParentResourceType(resource));
+        Mockito.verify(resolver, times(1)).getParentResourceTypeInternal(resource);
+    }
+
+    @Test
+    public void testGetParentResourceTypeCacheClearedOnRefresh() {
+        final PathBasedResourceResolverImpl resolver = Mockito.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));
+        Mockito.verify(resolver, times(1)).getParentResourceTypeInternal(resource);
+
+        resolver.refresh();
+        assertEquals("/types/2", resolver.getParentResourceType(resource));
+        Mockito.verify(resolver, times(2)).getParentResourceTypeInternal(resource);
+    }
+
     private PathBasedResourceResolverImpl getPathBasedResourceResolver() {
         return getPathBasedResourceResolver(new String[] {""});
     }