anchela commented on code in PR #49:
URL:
https://github.com/apache/sling-org-apache-sling-jcr-resource/pull/49#discussion_r2375784578
##########
src/test/java/org/apache/sling/jcr/resource/internal/JcrValueMapTest.java:
##########
@@ -58,6 +65,24 @@ public void testGetWithClass() {
assertEquals("test", vm.get("string",null));
}
-
+ /**
+ * Make sure cache is fully populated even
Review Comment:
even?
the comment seems truncated... and i wonder why the cache needs to be
populated eagerly
##########
src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java:
##########
@@ -300,10 +330,17 @@ public String getPath() {
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);
Review Comment:
the comment states that the primary type is used as fallback.
but looking at the code i don't see sling:resourceType being read. should
that be the first step and reading primary type only the fallback?
##########
src/main/java/org/apache/sling/jcr/resource/internal/JcrValueMap.java:
##########
@@ -392,6 +429,16 @@ void readFully() {
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);
Review Comment:
@kwin , similar as above. isn't this method here intended to make sure
primary type and resource type are always cached? in which case the primary
type property would only act as fallback?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
