[GitHub] [sling-org-apache-sling-resourceresolver] Gabiesfat commented on a change in pull request #50: SLING-10844: ResourceMapper.getMapping() returns null for empty path

[GitBox <gi...@apache.org>]

Gabiesfat commented on a change in pull request #50:
URL: https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/50#discussion_r736197884



##########
File path: src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java
##########
@@ -72,7 +72,7 @@ public String getMapping(String resourcePath, HttpServletRequest request) {
         
         Collection<String> mappings = getAllMappings(resourcePath, request);
         if ( mappings.isEmpty() )
-            return null;
+            return "";

Review comment:
       This method depends on getAllMappings() for the mappings which only states that it returns a collection 
   of mappings. So theoretically _yes_ but practically _no_. Ideally this method should only be worried  about the contract getAllMappings shares (and intelligent enough to handle scenario based on that) rather than depending upon implementation beneath. However having said that after addressing the other comment mappings can be empty even practically too.

##########
File path: src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java
##########
@@ -154,7 +154,7 @@ public String getMapping(String resourcePath, HttpServletRequest request) {
         }
 
         // 5. add the requested path itself, if not already populated
-        if ( !mappedPath.isEmpty() && !mappings.contains(mappedPath))
+        if (!mappings.contains(mappedPath))

Review comment:
       Actually it was preventing the mappedPath from getting added to mappings which was leading to making it null however that case has been handled above and mappings can be empty so I believe the condition can be imposed again.




-- 
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: dev-unsubscribe@sling.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org