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 73e8766ac0ed6f28e4198bf79ddec36aed1f20f4 Author: Robert Munteanu <[email protected]> AuthorDate: Thu Aug 6 16:46:43 2020 +0200 SLING-9620 - ResourceMapperImpl.getAllMappings does not respect multi-valued sling:alias Add a cartesianJoin utility method to help with calculating paths for multiple aliases. --- .../resourceresolver/impl/mapping/PathBuilder.java | 100 +++++++++++++++++ .../impl/mapping/ResourceMapperImpl.java | 120 ++++++++++----------- .../impl/mapping/PathBuilderTest.java | 94 ++++++++++++++++ 3 files changed, 248 insertions(+), 66 deletions(-) diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilder.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilder.java new file mode 100644 index 0000000..bc37a86 --- /dev/null +++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilder.java @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.resourceresolver.impl.mapping; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; + +/** + * Utility to construct paths from segments, starting with the leaf-most ones + * + */ +public class PathBuilder { + + // visible for testing + static List<String> cartesianJoin(List<List<String>> segments) { + + return cartesianJoin0(0, segments).stream() + .map( StringBuilder::toString ) + .collect( Collectors.toList() ); + } + + private static List<StringBuilder> cartesianJoin0(int index, List<List<String>> segments) { + List<StringBuilder> out = new ArrayList<>(); + if ( index == segments.size() ) { + out.add(new StringBuilder()); + } else { + for ( String segment : segments.get(index) ) { + for (StringBuilder sb : cartesianJoin0(index + 1, segments) ) { + // TODO - this is sub-optimal, as we are copying the array for each move + sb.insert(0, '/' + segment); + out.add(sb); + } + } + } + + return out; + } + + private List<String> segments = new ArrayList<>(); + private String toAppend; + + /** + * Inserts a new segment as the parent of the existing ones + * + * @param alias the alias, ignored if null or empty + * @param name the name + */ + public void insertSegment(@Nullable String alias, @NotNull String name) { + segments.add(alias != null && alias.length() != 0 ? alias : name); + } + + /** + * Sets a new value to append, typically the resolution info + * + * @param toAppend the parameters to append, ignored if null or empty + */ + public void setResolutionPathInfo(@Nullable String toAppend) { + this.toAppend = toAppend; + } + + /** + * Constructs a new path from the provided information + * + * @return a path in string form + */ + public String toPath() { + StringBuilder sb = new StringBuilder(); + sb.append('/'); + for ( int i = segments.size() - 1 ; i >= 0 ; i-- ) { + sb.append(segments.get(i)); + if ( i == 1 ) + sb.append('/'); + } + + if ( toAppend != null ) + sb.append(toAppend); + + return sb.toString(); + } +} 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 0d78a8c..6fdd883 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 @@ -22,7 +22,6 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.LinkedHashSet; -import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.function.UnaryOperator; @@ -128,8 +127,8 @@ public class ResourceMapperImpl implements ResourceMapper { if (fragmentQueryMark >= 0) { fragmentQuery = resourcePath.substring(fragmentQueryMark); mappedPath = resourcePath.substring(0, fragmentQueryMark); - logger.debug("map: Splitting resource path '{}' into '{}' and '{}'", new Object[] { resourcePath, mappedPath, - fragmentQuery }); + logger.debug("map: Splitting resource path '{}' into '{}' and '{}'", resourcePath, mappedPath, + fragmentQuery); } else { fragmentQuery = null; mappedPath = resourcePath; @@ -161,17 +160,12 @@ public class ResourceMapperImpl implements ResourceMapper { // 7. set back the fragment query if needed if ( fragmentQuery != null ) { - mappings.replaceAll(new UnaryOperator<String>() { - @Override - public String apply(String mappedPath) { - return mappedPath.concat(fragmentQuery); - } - }); + mappings.replaceAll(path -> path.concat(fragmentQuery)); } - mappings.forEach( path -> { - logger.debug("map: Returning URL {} as mapping for path {}", path, resourcePath); - }); + mappings.forEach( path -> + logger.debug("map: Returning URL {} as mapping for path {}", path, resourcePath) + ); Collections.reverse(mappings); @@ -180,7 +174,7 @@ public class ResourceMapperImpl implements ResourceMapper { private String loadAliasIfApplicable(final Resource nonDecoratedResource) { //Invoke the decorator for the resolved resource - Resource res = resourceDecorator.decorate(nonDecoratedResource); + Resource res = resourceDecorator.decorate(nonDecoratedResource); // keep, what we might have cut off in internal resolution final String resolutionPathInfo = res.getResourceMetadata().getResolutionPathInfo(); @@ -190,36 +184,21 @@ public class ResourceMapperImpl implements ResourceMapper { // find aliases for segments. we can't walk the parent chain // since the request session might not have permissions to // read all parents SLING-2093 - final LinkedList<String> names = new LinkedList<>(); + PathBuilder pathBuilder = new PathBuilder(); + // make sure to append resolutionPathInfo, if present + pathBuilder.setResolutionPathInfo(resolutionPathInfo); + Resource current = res; String path = res.getPath(); while (path != null) { String alias = null; + // read alias only if we can read the resources and it's not a jcr:content leaf if (current != null && !path.endsWith(ResourceResolverImpl.JCR_CONTENT_LEAF)) { - if (optimizedAliasResolutionEnabled) { - logger.debug("map: Optimize Alias Resolution is Enabled"); - String parentPath = ResourceUtil.getParent(path); - if (parentPath != null) { - final Map<String, String> aliases = mapEntries.getAliasMap(parentPath); - if (aliases!= null && aliases.containsValue(current.getName())) { - for (String key:aliases.keySet()) { - if (current.getName().equals(aliases.get(key))) { - alias = key; - break; - } - } - } - } - } else { - logger.debug("map: Optimize Alias Resolution is Disabled"); - alias = ResourceResolverControl.getProperty(current, ResourceResolverImpl.PROP_ALIAS); - } - } - if (alias == null || alias.length() == 0) { - alias = ResourceUtil.getName(path); + alias = readAlias(path, current); } - names.add(alias); + // build the path from the name segments or aliases + pathBuilder.insertSegment(alias, ResourceUtil.getName(path)); path = ResourceUtil.getParent(path); if ("/".equals(path)) { path = null; @@ -227,32 +206,41 @@ public class ResourceMapperImpl implements ResourceMapper { current = res.getResourceResolver().resolve(path); } } - - // build path from segment names - final StringBuilder buf = new StringBuilder(); - - // construct the path from the segments (or root if none) - if (names.isEmpty()) { - buf.append('/'); - } else { - while (!names.isEmpty()) { - buf.append('/'); - buf.append(names.removeLast()); - } - } - - // reappend the resolutionPathInfo - if (resolutionPathInfo != null) { - buf.append(resolutionPathInfo); - } - + // and then we have the mapped path to work on - String mappedPath = buf.toString(); + String mappedPath = pathBuilder.toPath(); logger.debug("map: Alias mapping resolves to path {}", mappedPath); return mappedPath; } + + private String readAlias(String path, Resource current) { + if (optimizedAliasResolutionEnabled) { + logger.debug("map: Optimize Alias Resolution is Enabled"); + String parentPath = ResourceUtil.getParent(path); + + if ( parentPath == null ) + return null; + + final Map<String, String> aliases = mapEntries.getAliasMap(parentPath); + + if ( aliases == null ) + return null; + + if ( aliases.containsValue(current.getName()) ) { + for ( Map.Entry<String,String> entry : aliases.entrySet() ) { + if (current.getName().equals(entry.getValue())) { + return entry.getKey(); + } + } + } + return null; + } else { + logger.debug("map: Optimize Alias Resolution is Disabled"); + return ResourceResolverControl.getProperty(current, ResourceResolverImpl.PROP_ALIAS); + } + } private void populateMappingsFromMapEntries(List<String> mappings, String mappedPath, final RequestContext requestContext) { @@ -300,14 +288,6 @@ public class ResourceMapperImpl implements ResourceMapper { } } - private String mangleNamespaces(String absPath) { - if ( absPath != null && namespaceMangler != null && namespaceMangler instanceof JcrNamespaceMangler ) { - absPath = ((JcrNamespaceMangler) namespaceMangler).mangleNamespaces(resolver, logger, absPath); - } - - return absPath; - } - private class RequestContext { private final String uri; @@ -317,8 +297,8 @@ public class ResourceMapperImpl implements ResourceMapper { if ( request != null ) { this.uri = MapEntry.getURI(request.getScheme(), request.getServerName(), request.getServerPort(), "/"); this.schemeWithPrefix = request.getScheme().concat("://"); - logger.debug("map: Mapping path {} for {} (at least with scheme prefix {})", new Object[] { resourcePath, - uri, schemeWithPrefix }); + logger.debug("map: Mapping path {} for {} (at least with scheme prefix {})", resourcePath, + uri, schemeWithPrefix ); } else { this.uri = null; this.schemeWithPrefix = null; @@ -378,5 +358,13 @@ public class ResourceMapperImpl implements ResourceMapper { return mappedPath; } + + private String mangleNamespaces(String absPath) { + if ( absPath != null && namespaceMangler != null && namespaceMangler instanceof JcrNamespaceMangler ) { + absPath = ((JcrNamespaceMangler) namespaceMangler).mangleNamespaces(resolver, logger, absPath); + } + + return absPath; + } } } diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilderTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilderTest.java new file mode 100644 index 0000000..6f4ba78 --- /dev/null +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/PathBuilderTest.java @@ -0,0 +1,94 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sling.resourceresolver.impl.mapping; + +import static org.junit.Assert.*; + +import java.util.Arrays; +import java.util.List; + +import org.hamcrest.Matchers; +import org.junit.Test; + +public class PathBuilderTest { + + @Test + public void buildRootPath() { + assertThat(new PathBuilder().toPath(), Matchers.equalTo("/")); + } + + @Test + public void buildSubPathWithMissingAliases() { + PathBuilder builder = new PathBuilder(); + builder.insertSegment(null, "bar"); + builder.insertSegment("", "foo"); + assertThat(builder.toPath(), Matchers.equalTo("/foo/bar")); + } + + @Test + public void buildSubPathWithMixedAliases() { + PathBuilder builder = new PathBuilder(); + builder.insertSegment(null, "bar"); + builder.insertSegment("super", "foo"); + assertThat(builder.toPath(), Matchers.equalTo("/super/bar")); + } + + @Test + public void buildSubPathWithResolutionInfo() { + PathBuilder builder = new PathBuilder(); + builder.insertSegment(null, "bar"); + builder.insertSegment(null, "foo"); + builder.setResolutionPathInfo("/baz"); + assertThat(builder.toPath(), Matchers.equalTo("/foo/bar/baz")); + } + + @Test + public void cartesianJoin_simple() { + List<String> paths = PathBuilder.cartesianJoin(Arrays.asList( Arrays.asList("1"), Arrays.asList("2") )); + assertThat(paths, Matchers.hasSize(1)); + assertThat(paths, Matchers.hasItem("/1/2")); + } + + @Test + public void cartesianJoin_multiple() { + List<String> paths = PathBuilder.cartesianJoin(Arrays.asList( Arrays.asList("1a", "1b"), Arrays.asList("2") )); + assertThat(paths, Matchers.hasSize(2)); + assertThat(paths, Matchers.hasItems("/1a/2", "/1b/2")); + } + + @Test + public void cartesianJoin_complex() { + List<String> paths = PathBuilder.cartesianJoin(Arrays.asList( Arrays.asList("1a", "1b"), Arrays.asList("2a", "2b"), Arrays.asList("3"), Arrays.asList("4a", "4b", "4c") )); + assertThat(paths, Matchers.hasSize(12)); + assertThat(paths, Matchers.hasItems( + "/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" + )); + } +}
