[
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)
