This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit de8614d7b28d68ceffabd5468e52bb44e3fc0324 Author: Robert Munteanu <[email protected]> AuthorDate: Mon Aug 17 15:16:01 2020 +0200 SLING-9623 - ResourceMapperImpl.getAllMappings is incomplete for nested alias Generate all possible nested aliases combinations. --- .../impl/mapping/PathGenerator.java | 10 +++-- .../impl/mapping/ResourceMapperImpl.java | 3 +- .../impl/mapping/PathGeneratorTest.java | 47 ++++++++++++++-------- .../impl/mapping/ResourceMapperImplTest.java | 2 - 4 files changed, 38 insertions(+), 24 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathGenerator.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathGenerator.java index 7cf86d1..0d61e1a 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathGenerator.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathGenerator.java @@ -22,6 +22,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.stream.Collectors; +import java.util.stream.Stream; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -34,7 +35,6 @@ import org.jetbrains.annotations.Nullable; * * <p>It generates all possible path combinations using a cartesian product that accummulates * using a {@link StringBuilder} instead of a set, to prevent intermediate object creation.</p> - * */ public class PathGenerator { @@ -76,12 +76,14 @@ public class PathGenerator { */ public void insertSegment(@NotNull List<String> alias, @NotNull String name) { - // TODO - can we avoid filtering here? - List<String> filtered = alias.stream() + List<String> filtered = Stream.concat(alias.stream(), Stream.of(name) ) .filter( e -> e != null && ! e.isEmpty() ) .collect(Collectors.toList()); - segments.add(!filtered.isEmpty() ? alias : Collections.singletonList(name)); + if ( filtered.isEmpty() ) + filtered = Collections.singletonList(""); + + segments.add(filtered); } /** diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java index 0cee1cd..c7eb908 100644 --- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImpl.java @@ -151,6 +151,8 @@ public class ResourceMapperImpl implements ResourceMapper { final Resource nonDecoratedResource = resolver.resolveInternal(parsed.getRawPath(), parsed.getParameters()); if (nonDecoratedResource != null) { List<String> aliases = loadAliasesIfApplicable(nonDecoratedResource); + // avoid duplicating the originally requested path + aliases.remove(mappedPath); // ensure that the first declared alias will be returned first Collections.reverse(aliases); @@ -213,7 +215,6 @@ public class ResourceMapperImpl implements ResourceMapper { // and then we have the mapped path to work on List<String> mappedPaths = pathBuilder.generatePaths(); - logger.debug("map: Alias mapping resolves to paths {}", mappedPaths); return mappedPaths; diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathGeneratorTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathGeneratorTest.java index fc498f1..36b8bb0 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathGeneratorTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathGeneratorTest.java @@ -23,6 +23,7 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; import static org.junit.Assert.*; +import java.util.Collections; import java.util.List; import org.hamcrest.Matchers; @@ -38,6 +39,17 @@ public class PathGeneratorTest { assertThat(paths, Matchers.hasSize(1)); assertThat(paths, Matchers.hasItem("/")); } + + @Test + public void emptyRootSegment() { + + PathGenerator builder = new PathGenerator(); + builder.insertSegment(Collections.emptyList(), ""); + List<String> paths = builder.generatePaths(); + + assertThat(paths, Matchers.hasSize(1)); + assertThat(paths, Matchers.hasItem("/")); + } @Test public void subPathWithMissingAliases() { @@ -59,8 +71,9 @@ public class PathGeneratorTest { builder.insertSegment(singletonList("super"), "foo"); List<String> paths = builder.generatePaths(); - assertThat(paths, Matchers.hasSize(1)); + assertThat(paths, Matchers.hasSize(2)); assertThat(paths, Matchers.hasItem("/super/bar")); + assertThat(paths, Matchers.hasItem("/foo/bar")); } @Test @@ -86,35 +99,35 @@ public class PathGeneratorTest { List<String> paths = builder.generatePaths(); - assertThat(paths, Matchers.hasSize(2)); - assertThat(paths, Matchers.hasItems("/alias1/bar", "/alias2/bar")); + assertThat(paths, Matchers.hasSize(3)); + assertThat(paths, Matchers.hasItems("/alias1/bar", "/alias2/bar", "/foo/bar")); } @Test public void subPathWithComplexAliasesSetup() { PathGenerator builder = new PathGenerator(); - builder.insertSegment(asList("4a", "4b", "4c"), "4"); + builder.insertSegment(asList("4a", "4b"), "4"); builder.insertSegment(emptyList(), "3"); - builder.insertSegment(asList("2a", "2b"), "2"); - builder.insertSegment(asList("1a", "1b"), "1"); + builder.insertSegment(asList("2a"), "2"); + builder.insertSegment(asList("1a"), "1"); List<String> paths = builder.generatePaths(); assertThat(paths, Matchers.hasSize(12)); assertThat(paths, Matchers.hasItems( + "/1/2/3/4a", + "/1/2/3/4b", + "/1/2/3/4", + "/1/2a/3/4", + "/1/2a/3/4a", + "/1/2a/3/4b", + "/1a/2/3/4", + "/1a/2/3/4a", + "/1a/2/3/4b", + "/1a/2a/3/4", "/1a/2a/3/4a", - "/1a/2a/3/4b", - "/1a/2a/3/4c", - "/1a/2b/3/4a", - "/1a/2b/3/4b", - "/1a/2b/3/4c", - "/1b/2a/3/4a", - "/1b/2a/3/4b", - "/1b/2a/3/4c", - "/1b/2b/3/4a", - "/1b/2b/3/4b", - "/1b/2b/3/4c" + "/1a/2a/3/4b" )); } } diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java index 5babfa8..3b8effa 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/ResourceMapperImplTest.java @@ -288,7 +288,6 @@ public class ResourceMapperImplTest { * @throws LoginException */ @Test - @Ignore public void mapResourceWithNestedAlias() { ExpectedMappings.existingResource("/parent/child") .singleMapping("/alias-parent/alias-child") @@ -304,7 +303,6 @@ public class ResourceMapperImplTest { * @throws LoginException */ @Test - @Ignore public void mapResourceWithNestedMultipleAlias() { ExpectedMappings.existingResource("/parent/child-multiple") .singleMapping("/alias-parent/alias-child-1")
