sagarmiglani commented on a change in pull request #48:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/48#discussion_r667725412
##########
File path:
src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java
##########
@@ -150,7 +150,9 @@ public String getMapping(String resourcePath,
HttpServletRequest request) {
if (nonDecoratedResource != null) {
List<String> aliases =
loadAliasesIfApplicable(nonDecoratedResource);
// avoid duplicating the originally requested path
- aliases.remove(mappedPath);
+ if ( aliases.contains(mappedPath) ) {
Review comment:
Yes, this only influences the order of returned mappings, my analysis is
as follows: (please let me know if you think there is some issue with the same)
Lets say we have the following resource structure
- /parent -> with alias "parent-alias"
- /child -> with alias "child-alias"
For the following resource path requests, the primary mapping returned
should be:
| Requested resource path | Expected mapping |
| ------------- | ------------- |
| /parent/child | /parent-alias/child-alias |
| /parent/child-alias | /parent-alias/child-alias |
| /parent-alias/child-alias | /parent-alias/child-alias |
| /parent-alias/child-alias (this is our case of concern) |
/parent-alias/child-alias |
When the requested path is **/parent-alias/child-alias** (all segments are
aliases), our expectation is that the primary returned mapping should be
**/parent-alias/child-alias** (same as requested resource path).
With the above requested resource path, In aliases list
([ResourceMapperImpl.java:153](https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L153)),
/parent-alias/child-alias was present at 0 index and should have been returned
as the primary mapping.
But
[ResourceMapperImpl.java:153](https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L153),
we are removing the requested path from the aliases list itself, because we
have already added the requested resource path in the mapping at
[ResourceMapperImpl.java:143](https://github.com/apache/sling-org-apache-sling-resourceresolver/blob/master/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java#L143).
This was making the order of the returned mappings incorrect.
So, I thought that we should not be removing the requested resource path
from the aliases list, instead if aliases list contains the requested resource
path, we should remove it from mapping list.
But, If you think this has made the code harder to understand, I'll try to
come up with some other approach, but do you have any suggestions for the same?
--
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-unsubscr...@sling.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
