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: dev-unsubscr...@sling.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org