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

Aditya Seth commented on SLING-10844:
-------------------------------------

[~mohiaror] Please correct me if I am wrong but I think API would still be 
backward incompatible [0] if we change the API annotation from NonNullable to 
Nullable as we leverage the overriding API to throw null results but consuming 
SDKs anticipated otherwise which makes it vulnerable to future NPEs. 
IMHO we should honour the contract as is and modify the overriding API 
accordingly hence I think we can remove one check from condition [1] such that 
even if "mappedPath" is empty it still gets added to "mappings" which leads to 
returning NonNull result in case of empty "resourcePath". This also makes check 
[2] and following line redundant so we. can remove them as well. WDYT?

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

[1]: 
[https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L166]

[2]:  
https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L74

> ResourceMapper.getMapping() returns null for empty path
> -------------------------------------------------------
>
>                 Key: SLING-10844
>                 URL: https://issues.apache.org/jira/browse/SLING-10844
>             Project: Sling
>          Issue Type: Bug
>          Components: ResourceResolver
>    Affects Versions: Resource Resolver 1.7.0
>            Reporter: Mohit Arora
>            Priority: Major
>             Fix For: Resource Resolver 1.7.12
>
>
> After the bug fix for SLING-9620, the behavior for 
> ResourceMapper.getAllMappings() was changed such that the mappings list 
> remains empty if the resourcePath provided is an empty string. Prior to this 
> bug fix, the mappings list contained a single entry for empty path.
> Since mappings list is empty, [ResourceMapper.getMapping() returns 
> null|https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L74-L75].
>  The [javadoc of 
> ResourceMapper.getMapping()|https://github.com/apache/sling-org-apache-sling-api/blob/master/src/main/java/org/apache/sling/api/resource/mapping/ResourceMapper.java#L67]
>  API mentions that it will return a non null value if the resourcepath is not 
> null.
> We need to decide on the expected behavior here. If mappings list should not 
> be updated in case of empty resourcePath then the API annotation will have to 
> be changed and it can potentially be a backwards incompatible issue in theory 
> as the consumers of the API may not have added a null check to the return 
> value.



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

Reply via email to