sagarmiglani commented on code in PR #96:
URL:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/96#discussion_r1185836833
##########
src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java:
##########
@@ -397,8 +397,8 @@ public void mapResourceWithVanityPathsURLTargetNoPath() {
ExpectedMappings.existingResource("/vain-url-nopath")
.singleMapping("/vain-url-nopath")
.singleMappingWithRequest("/app/vain-url-nopath")
- .allMappings("/vain-url-nopath", "")
- .allMappingsWithRequest("/app/vain-url-nopath", "")
Review Comment:
I agree, this is changing the behaviour, but what would be the blast radius?
Could there be too many people using pathless url based vanity paths?
Also test you pointed is from "improved test coverage" \[0] (changes under
SLING-11757):
```
@Test
public void mapResourceWithVanityPathsURLTargetNoPath() {
ExpectedMappings.existingResource("/vain-url-nopath")
.singleMapping("/vain-url-nopath")
.singleMappingWithRequest("/app/vain-url-nopath")
.allMappings("/vain-url-nopath", "")
.allMappingsWithRequest("/app/vain-url-nopath", "")
.verify(resolver, req);
}
```
and it has the following expectation which itself seems wrong:
`allMappingsWithRequest("/app/vain-url-nopath", "")`
Does it look right to you? Shouldn't the there be "/app" ("" appended with
the request context path) instead of ""?
When I run this test`[main] WARN
org.apache.sling.resourceresolver.impl.mapping.ResourceMapperImpl - map: Unable
to mangle namespaces for returning unmangled`
[0]:
https://github.com/apache/sling-org-apache-sling-resourceresolver/pull/94/files#diff-f6a4a27e96e7b9934452dc38379974ed9583083294ebe795081b1963ac4b2c6fR401
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
