This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch bugfix/fallback-for-primary-type-retrieval in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-resource.git
commit 45a0c9fe53c01290e576915792f12f7466762816 Author: Konrad Windszus <[email protected]> AuthorDate: Mon Sep 22 21:50:09 2025 +0200 SLING-12944 Retrieve primary type as fallback via Node.getPrimaryNodeType() The permission evaluation is skipped for this call. Also make sure to populate the cache with entries for both "sling:resourceType" and "jcr:primaryType" with readFully() --- pom.xml | 2 +- .../sling/jcr/resource/internal/JcrValueMap.java | 55 ++++++++++++++++++++-- .../jcr/resource/internal/JcrValueMapTest.java | 27 ++++++++++- .../internal/helper/jcr/JcrNodeResourceTest.java | 2 + 4 files changed, 80 insertions(+), 6 deletions(-) diff --git a/pom.xml b/pom.xml index 05772f6..a6c2066 100644 --- a/pom.xml +++ b/pom.xml @@ -213,7 +213,7 @@ <dependency> <groupId>org.hamcrest</groupId> <artifactId>hamcrest</artifactId> - <version>2.2</version> + <version>3.0</version> <scope>test</scope> </dependency> <dependency> diff --git a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java index 7384046..d549d2c 100644 --- a/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java +++ b/src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java @@ -18,6 +18,7 @@ */ package org.apache.sling.jcr.resource.internal; +import java.io.IOException; import java.util.Calendar; import java.util.Collection; import java.util.Collections; @@ -277,6 +278,35 @@ public class JcrValueMap implements ValueMap { } } + private @NotNull JcrPropertyMapCacheEntry cacheProperty(final @NotNull String key, final @NotNull Object value, final @NotNull Node node) { + try { + JcrPropertyMapCacheEntry entry = cache.get(key); + if (entry == null) { + entry = new JcrPropertyMapCacheEntry(value, node); + cache.put(key, entry); + valueCache.put(key, entry.getPropertyValue()); + } + return entry; + } catch (final RepositoryException|IOException re) { + throw new IllegalArgumentException(re); + } + } + + /** + * Reads the primary type of the current node via {@link Node#getPrimaryNodeType()} and caches it. + * That way regular permission evaluation is bypassed (see <a href="https://issues.apache.org/jira/browse/OAK-2441";>OAK-2441</a>). + * Should only be used as fallback if regular access via {@link Node#getProperty(String)} fails as + * calculating the NodeType is expensive. + * @return the cache entry for the primary type + */ + private JcrPropertyMapCacheEntry readPrimaryType() { + try { + String primaryType = node.getPrimaryNodeType().getName(); + return cacheProperty(JcrConstants.JCR_PRIMARYTYPE, primaryType, node); + } catch (final RepositoryException re) { + throw new IllegalArgumentException(re); + } + } /** * Read a single property. * @throws IllegalArgumentException if a repository exception occurs @@ -300,10 +330,17 @@ public class JcrValueMap implements ValueMap { try { final String key = escapeKeyName(name); Property property = NodeUtil.getPropertyOrNull(node,key); - if (property == null && name.equals(ResourceResolver.PROPERTY_RESOURCE_TYPE)) { - // special handling for the resource type property which according to the API must always be exposed via property sling:resourceType - // use value of jcr:primaryType if sling:resourceType is not set - property = NodeUtil.getPropertyOrNull(node, JcrConstants.JCR_PRIMARYTYPE); + if (property == null) { + if (name.equals(ResourceResolver.PROPERTY_RESOURCE_TYPE)) { + // special handling for the resource type property which according to the API must always be exposed via property sling:resourceType + // use value of jcr:primaryType if sling:resourceType is not set + JcrPropertyMapCacheEntry entry = read(JcrConstants.JCR_PRIMARYTYPE); + if (entry != null) { + return cacheProperty(name, entry.getPropertyValue(), node); + } + } else if (name.equals(JcrConstants.JCR_PRIMARYTYPE)) { + return readPrimaryType(); + } } if (property != null) { return cacheProperty(property); @@ -392,6 +429,16 @@ public class JcrValueMap implements ValueMap { final Property prop = pi.nextProperty(); this.cacheProperty(prop); } + // make sure primary type is in cache + if (!this.cache.containsKey(JcrConstants.JCR_PRIMARYTYPE)) { + readPrimaryType(); + } + // make sure sling:resourceType is in cache + if (!this.cache.containsKey(ResourceResolver.PROPERTY_RESOURCE_TYPE)) { + // special handling for the resource type property which according to the API must always be exposed via property sling:resourceType + // use value of jcr:primaryType if sling:resourceType is not set + this.cacheProperty(ResourceResolver.PROPERTY_RESOURCE_TYPE, valueCache.get(JcrConstants.JCR_PRIMARYTYPE), node); + } fullyRead = true; } catch (final RepositoryException re) { throw new IllegalArgumentException(re); diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/JcrValueMapTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/JcrValueMapTest.java index 07d5f57..7e334b7 100644 --- a/src/test/java/org/apache/sling/jcr/resource/internal/JcrValueMapTest.java +++ b/src/test/java/org/apache/sling/jcr/resource/internal/JcrValueMapTest.java @@ -18,9 +18,16 @@ */ package org.apache.sling.jcr.resource.internal; +import static org.mockito.Mockito.times; +import static org.hamcrest.MatcherAssert.assertThat; + +import java.util.AbstractMap.SimpleEntry; + import javax.jcr.Node; +import javax.jcr.RepositoryException; import org.apache.sling.jcr.resource.internal.helper.jcr.SlingRepositoryTestBase; +import org.hamcrest.Matchers; import org.junit.Test; import org.mockito.Mockito; @@ -58,6 +65,24 @@ public class JcrValueMapTest extends SlingRepositoryTestBase { assertEquals("test", vm.get("string",null)); } - + /** + * Make sure cache is fully populated even + * @throws RepositoryException + */ + @Test + public void testGetEntrySet() throws RepositoryException { + Node node = Mockito.spy(rootNode); + JcrValueMap vm = new JcrValueMap(node, helperData); + assertThat( vm.entrySet(), Matchers.hasSize(3) ); + assertThat( vm.keySet(), Matchers.hasSize(3) ); + assertThat( vm.values(), Matchers.hasSize(3) ); + assertThat( vm.entrySet(), Matchers.containsInAnyOrder( + new SimpleEntry<String, Object>("jcr:primaryType", "nt:unstructured"), + new SimpleEntry<String, Object>("sling:resourceType", "nt:unstructured"), + new SimpleEntry<String, Object>("string", "test") + )); + // cache should only be populated once + Mockito.verify(node, times(1)).getProperties(); + } } diff --git a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java index 3fa404e..73de47a 100644 --- a/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java +++ b/src/test/java/org/apache/sling/jcr/resource/internal/helper/jcr/JcrNodeResourceTest.java @@ -34,6 +34,7 @@ import org.apache.jackrabbit.JcrConstants; import org.apache.sling.api.resource.ModifiableValueMap; 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.ValueMap; import org.apache.sling.jcr.resource.api.JcrResourceConstants; @@ -220,6 +221,7 @@ public class JcrNodeResourceTest extends JcrItemResourceTestBase { existingKeys.add(JcrConstants.JCR_ENCODING); existingKeys.add(JcrConstants.JCR_DATA); existingKeys.add(JcrConstants.JCR_PRIMARYTYPE); + existingKeys.add(ResourceResolver.PROPERTY_RESOURCE_TYPE); final Set<Object> crossCheck = new HashSet<>(props.keySet()); crossCheck.removeAll(existingKeys); assertTrue(crossCheck.isEmpty());
