[jira] [Commented] (SLING-9620) ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias

["Mohit Arora (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/SLING-9620?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17420389#comment-17420389 ] 

Mohit Arora commented on SLING-9620:
------------------------------------

[~rombert] It seems that after this bug fix, the behavior has changed for ResourceMapperImpl.getMapping() when the resourcePath is an empty String. This is happening because of the addition of [this check|https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L157] for mappedPath not being empty before adding it to mappings list. Earlier, we used to add the mappedPath to mappings list irrespective of it being empty. Because of that, for empty path, [getMapping function used to return a non null value|https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L74] but now it is returning null.

I agree that this was a bug with the earlier implementation and the callers of getMapping API should check for null values being returned, but the [API specifically annotates|https://github.com/apache/sling-org-apache-sling-api/blob/master/src/main/java/org/apache/sling/api/resource/mapping/ResourceMapper.java#L96] that for nonNull resourcePath, the return value will be nonNull. So in "theory" this looks like a backwards incompatible change. Although checking the implementation of ResourceMapperImpl the behavior is same from the beginning. It seems the API was incorrectly annotated?

What should be done to address this? Should we correct the API annotation and put Nullable as return value? I think that's something that needs to be done anyway. Apart from that do you think we need to try to maintain backwards compatibility here by even adding the empty resourcePath to the mappings list?

> ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias
> ---------------------------------------------------------------------------
>
>                 Key: SLING-9620
>                 URL: https://issues.apache.org/jira/browse/SLING-9620
>             Project: Sling
>          Issue Type: Bug
>          Components: ResourceResolver
>    Affects Versions: Resource Resolver 1.6.16
>            Reporter: Angela Schreiber
>            Assignee: Robert Munteanu
>            Priority: Major
>             Fix For: Resource Resolver 1.7.0
>
>         Attachments: SLING-9620-test.patch
>
>          Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> while investigating an issue involving {{sling:alias}}, i ended up manually adding the property using JCR API calls. this involved first adding the {{sling:ResourceAlias}} and i noticed that {{sling:alias}} can be both single or multi-valued according to the node type definition:
> {code}
> / Mixin node type to enable setting an alias on a resource
> [sling:ResourceAlias]
>     mixin
>   
>     // alias name(s) for the node (single or multi-value)
>   - sling:alias (string)
>   - sling:alias (string) multiple
> {code}
> when setting multiple values for the {{sling:alias}} property, i found that {{ResourceMapper.getAllMappings}} only returns the first alias.
> looking at the implementation in {{ResourceMapperImpl.loadAliasIfApplicable}}, it seems that line 216 ({{String alias = ResourceResolverControl.getProperty(current, ResourceResolverImpl.PROP_ALIAS);}}), is the culprit as call will in any case just return a single string (it calls {{getProperty(res, propName, String.class)}}).
> as a consequence consumers of the {{ResourceMapper.getAllMappings}} method will not get a complete list of all aliases available.



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