[jira] [Updated] (SLING-10720) "nt:unstructured" is used in case of overwriting node with no sling:resourceType property defined (JCR Resource Merger)

["Henry Kuijpers (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/SLING-10720?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Henry Kuijpers updated SLING-10720:
-----------------------------------
    Description: 
https://github.com/apache/sling-org-apache-sling-resourcemerger/blob/c9d742c8080a0976b778c678747d99289c9efbb7/src/main/java/org/apache/sling/resourcemerger/impl/MergedResource.java#L75

In this constructor, I see the following logic:
1. Try to find the sling:resourceType property, set in any of the resources
2. If that property could not be found, try using the relative path

Then, even if a proper sling:resourceType property was found, it will overwrite that value with the result of Resource#getResourceType if it is not null, of the last resource that defines one. This could be entirely right (maybe this done because some people use wrappers, that override the getResourceType-method?).

However, in the JCR implementation of Resource (JcrNodeResource/JcrItemResource), it is rare for getResourceType to return null, because:
1. Try to return the defined sling:resourceType's value
2. If not found, try to return the value of jcr:primaryType

Generally, any node has a jcr:primaryType defined and thus returns it.

When the overridden resource does not mention a resource type, this effectively makes getResourceType of the merged resource return the node type of the overridden resource (in our case: nt:unstructured).

Easy case to reproduce:

/libs/test (@jcr:primaryType: nt:unstructured, @sling:resourceType=website/component, @prop=test)
/apps/test (@jcr:primaryType: nt:unstructured, @prop=test-overridden)

->

resource.getResourceType(): *nt:unstructured*
resource.getResourceSuperType(): *website/component*
resource.getValueMap().get("prop"): test-overridden
resource.getValueMap().get("sling:resourceType"): *website/component*

(Marked in the bold, the values I think are not correct in this case.)

  was:
https://github.com/apache/sling-org-apache-sling-resourcemerger/blob/c9d742c8080a0976b778c678747d99289c9efbb7/src/main/java/org/apache/sling/resourcemerger/impl/MergedResource.java#L75

In this file I see logic that works as follows:
1. Try to find the sling:resourceType property, if that is found, use it
2. If not found, use the relative path

So far, so good.

Then, there is no guard around the logic that uses the resource type of the last resource that defines one.

Unfortunately, in the JCR implementation, when a resource does not define a sling:resourceType-property, Resource::getResourceType() doesn't return null. It instead returns the node type of the node (in our case nt:unstructured).

This makes the MergedResource have:
* sling:resourceType = "nt:unstructured"
* sling:resourceSuperType = "website/component"

This then makes filters fail to invoke, since they require the resource to have a certain resource type.

Node config:
/libs/...../something:
@jcr:primaryType = nt:unstructured
@sling:resourceType = website/component
@label = test

/apps/..../something:
@jcr:primaryType = nt:unstructured
@label = test2

We then created a filter that is listening on the resource type "website/component", which fails to invoke because the Resource#getResourceType() yields "nt:unstructured".

When we put sling:resourceType = "website/component" in the node in /apps, it works. But we believe that is not the correct fix.


> "nt:unstructured" is used in case of overwriting node with no sling:resourceType property defined (JCR Resource Merger)
> -----------------------------------------------------------------------------------------------------------------------
>
>                 Key: SLING-10720
>                 URL: https://issues.apache.org/jira/browse/SLING-10720
>             Project: Sling
>          Issue Type: Improvement
>          Components: ResourceResolver
>    Affects Versions: Resource Merger 1.4.0
>            Reporter: Henry Kuijpers
>            Priority: Major
>
> https://github.com/apache/sling-org-apache-sling-resourcemerger/blob/c9d742c8080a0976b778c678747d99289c9efbb7/src/main/java/org/apache/sling/resourcemerger/impl/MergedResource.java#L75
> In this constructor, I see the following logic:
> 1. Try to find the sling:resourceType property, set in any of the resources
> 2. If that property could not be found, try using the relative path
> Then, even if a proper sling:resourceType property was found, it will overwrite that value with the result of Resource#getResourceType if it is not null, of the last resource that defines one. This could be entirely right (maybe this done because some people use wrappers, that override the getResourceType-method?).
> However, in the JCR implementation of Resource (JcrNodeResource/JcrItemResource), it is rare for getResourceType to return null, because:
> 1. Try to return the defined sling:resourceType's value
> 2. If not found, try to return the value of jcr:primaryType
> Generally, any node has a jcr:primaryType defined and thus returns it.
> When the overridden resource does not mention a resource type, this effectively makes getResourceType of the merged resource return the node type of the overridden resource (in our case: nt:unstructured).
> Easy case to reproduce:
> /libs/test (@jcr:primaryType: nt:unstructured, @sling:resourceType=website/component, @prop=test)
> /apps/test (@jcr:primaryType: nt:unstructured, @prop=test-overridden)
> ->
> resource.getResourceType(): *nt:unstructured*
> resource.getResourceSuperType(): *website/component*
> resource.getValueMap().get("prop"): test-overridden
> resource.getValueMap().get("sling:resourceType"): *website/component*
> (Marked in the bold, the values I think are not correct in this case.)



--
This message was sent by Atlassian Jira
(v8.3.4#803005)