jsedding commented on code in PR #236:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/236#discussion_r4249839837
##########
src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverImpl.java:
##########
@@ -1176,7 +1164,20 @@ String getParentResourceTypeInternal(final Resource
resource) {
@Override
public String getParentResourceType(final String resourceType) {
checkClosed();
- return this.control.getParentResourceType(this.factory, this,
resourceType);
+ if (resourceType == null) {
+ return null;
+ }
+ Optional<String> resourceSuperType =
parentResourceTypeMap.get(resourceType);
+ if (resourceSuperType == null) {
+ // Type resolution can re-enter this cache through providers or
resource wrappers.
+ resourceSuperType =
+
Optional.ofNullable(this.control.getParentResourceType(this.factory, this,
resourceType));
+ final Optional<String> cached =
parentResourceTypeMap.putIfAbsent(resourceType, resourceSuperType);
+ if (cached != null) {
+ resourceSuperType = cached;
+ }
+ }
+ return resourceSuperType.orElse(null);
Review Comment:
I don't think we have to make an effort to return the same `String` instance
in case two racing threads compute the same parent resource-type.
```suggestion
Optional<String> resourceSuperType =
parentResourceTypeMap.get(resourceType);
if (resourceSuperType == null) {
resourceSuperType =
Optional.ofNullable(this.control.getParentResourceType(this.factory, this,
resourceType))
// computeIfAbsent() might fail here, because type resolution
can re-enter this cache
// through providers or resource wrappers.
// In case of a race, the worst case is that multiple threads
compute the same super type.
parentResourceTypeMap.putIfAbsent(resourceType,
resourceSuperType);
}
return resourceSuperType.orElse(null);
```
--
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]
