You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2021/01/12 09:34:10 UTC
[sling-org-apache-sling-resourceresolver] branch master updated:
SLING-9535_-Improve performance of sling:alias
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
The following commit(s) were added to refs/heads/master by this push:
new 5e1b550 SLING-9535_-Improve performance of sling:alias
5e1b550 is described below
commit 5e1b550b740619c20eab9cb389bcf6dc637636c4
Author: akankshajain18 <cs...@gmail.com>
AuthorDate: Tue Nov 3 17:19:46 2020 +0530
SLING-9535_-Improve performance of sling:alias
---
.../impl/CommonResourceResolverFactoryImpl.java | 14 ++-
.../impl/ResourceResolverFactoryActivator.java | 40 ++++++--
.../impl/ResourceResolverFactoryConfig.java | 7 ++
.../impl/mapping/MapConfigurationProvider.java | 8 ++
.../resourceresolver/impl/mapping/MapEntries.java | 110 ++++++++++++---------
.../impl/MockedResourceResolverImplTest.java | 5 +
.../impl/ResourceResolverFactoryTest.java | 23 ++++-
.../impl/mapping/MapEntriesTest.java | 43 +++++---
8 files changed, 167 insertions(+), 83 deletions(-)
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java b/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java
index b925e65..d9b6c08 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/CommonResourceResolverFactoryImpl.java
@@ -20,14 +20,7 @@ package org.apache.sling.resourceresolver.impl;
import java.lang.ref.ReferenceQueue;
import java.lang.ref.WeakReference;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Stack;
+import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -472,6 +465,11 @@ public class CommonResourceResolverFactoryImpl implements ResourceResolverFactor
return configs;
}
+ @Override
+ public Set<String> getAllowedAliasLocations() {
+ return this.activator.getAllowedAliasLocations();
+ }
+
/**
* Is this factory still alive?
*/
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
index 646a077..94dd37d 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
@@ -21,14 +21,8 @@ package org.apache.sling.resourceresolver.impl;
import java.lang.reflect.InvocationHandler;
import java.lang.reflect.Method;
import java.lang.reflect.Proxy;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Dictionary;
-import java.util.HashSet;
-import java.util.Hashtable;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
+import java.util.*;
+
import org.apache.commons.collections4.BidiMap;
import org.apache.commons.collections4.bidimap.TreeBidiMap;
@@ -128,6 +122,9 @@ public class ResourceResolverFactoryActivator {
private volatile ResourceResolverFactoryConfig config = DEFAULT_CONFIG;
+ @SuppressWarnings("java:S3077")
+ private volatile Set<String> allowedAliasLocations = Collections.emptySet();
+
/** Vanity path whitelist */
private volatile String[] vanityPathWhiteList;
@@ -205,6 +202,10 @@ public class ResourceResolverFactoryActivator {
return this.config.resource_resolver_optimize_alias_resolution();
}
+ public Set<String> getAllowedAliasLocations(){
+ return this.allowedAliasLocations;
+ }
+
public boolean isLogUnclosedResourceResolvers() {
return this.config.resource_resolver_log_unclosed();
}
@@ -304,6 +305,29 @@ public class ResourceResolverFactoryActivator {
this.observationPaths[i] = new Path(paths[i]);
}
+ // optimize alias path allow list
+ String[] aliasLocationsPrefix = config.resource_resolver_allowed_alias_locations();
+ if ( aliasLocationsPrefix != null ) {
+ final Set<String> prefixSet = new TreeSet<>();
+ for(final String prefix : aliasLocationsPrefix) {
+ String value = prefix.trim();
+ if (!value.isEmpty()) {
+ if (value.startsWith("/")) { // absolute path should be given
+ if (value.endsWith("/")) {
+ prefixSet.add(value);
+ } else {
+ prefixSet.add(value + "/");
+ }
+ }else{
+ logger.warn("Path [{}] is ignored. As only absolute paths are allowed for alias optimization", value);
+ }
+ }
+ }
+ if ( !prefixSet.isEmpty()) {
+ this.allowedAliasLocations = Collections.unmodifiableSet(prefixSet);
+ }
+ }
+
// vanity path white list
this.vanityPathWhiteList = null;
String[] vanityPathPrefixes = config.resource_resolver_vanitypath_whitelist();
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java
index a0aa53f..0e4537f 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryConfig.java
@@ -149,6 +149,13 @@ public @interface ResourceResolverFactoryConfig {
" and on the alias update time if the number of aliases is huge (over 10000).")
boolean resource_resolver_optimize_alias_resolution() default true;
+ @AttributeDefinition(name = "Allowed Optimized Alias Locations",
+ description = "This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
+ "such a list is configured, for alias optimization, only paths from resources starting with this prefix " +
+ "are considered. If the list is empty, all paths are used.)")
+ String[] resource_resolver_allowed_alias_locations();
+
+
@AttributeDefinition(name = "Allowed Vanity Path Location",
description ="This setting can contain a list of path prefixes, e.g. /libs/, /content/. If " +
"such a list is configured, only vanity paths from resources starting with this prefix " +
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java
index aeb437c..c8733a9 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapConfigurationProvider.java
@@ -18,6 +18,7 @@ package org.apache.sling.resourceresolver.impl.mapping;
import java.util.List;
import java.util.Map;
+import java.util.Set;
import org.apache.sling.api.resource.LoginException;
import org.apache.sling.api.resource.ResourceResolverFactory;
@@ -78,4 +79,11 @@ public interface MapConfigurationProvider extends ResourceResolverFactory {
* If <code>null</code> is returned, all paths are allowed.
*/
List<VanityPathConfig> getVanityPathConfig();
+
+ /**
+ * A set of allow prefixes all ending with a slash.
+ * If empty set is returned, all paths are allowed.
+ * @return
+ */
+ Set<String> getAllowedAliasLocations();
}
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
index 95f174b..fe3db2f 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/mapping/MapEntries.java
@@ -18,47 +18,8 @@
*/
package org.apache.sling.resourceresolver.impl.mapping;
-import java.io.DataInputStream;
-import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
-import java.io.IOException;
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Dictionary;
-import java.util.HashMap;
-import java.util.Hashtable;
-import java.util.Iterator;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
-import java.util.Map.Entry;
-import java.util.NoSuchElementException;
-import java.util.SortedMap;
-import java.util.Timer;
-import java.util.TimerTask;
-import java.util.TreeMap;
-import java.util.TreeSet;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
-import java.util.concurrent.atomic.AtomicLong;
-import java.util.concurrent.locks.ReentrantLock;
-import java.util.function.Supplier;
-import java.util.stream.Stream;
-
-import javax.servlet.http.HttpServletResponse;
-
import org.apache.sling.api.SlingConstants;
-import org.apache.sling.api.resource.LoginException;
-import org.apache.sling.api.resource.Resource;
-import org.apache.sling.api.resource.ResourceResolver;
-import org.apache.sling.api.resource.ResourceUtil;
-import org.apache.sling.api.resource.ValueMap;
+import org.apache.sling.api.resource.*;
import org.apache.sling.api.resource.observation.ExternalResourceChangeListener;
import org.apache.sling.api.resource.observation.ResourceChange;
import org.apache.sling.api.resource.observation.ResourceChangeListener;
@@ -73,6 +34,21 @@ import org.osgi.service.event.EventAdmin;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import javax.servlet.http.HttpServletResponse;
+import java.io.*;
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.util.*;
+import java.util.Map.Entry;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicLong;
+import java.util.concurrent.locks.ReentrantLock;
+import java.util.function.Supplier;
+import java.util.stream.Stream;
+
+
public class MapEntries implements
MapEntriesHandler,
ResourceChangeListener,
@@ -100,6 +76,8 @@ public class MapEntries implements
private static final int VANITY_BLOOM_FILTER_MAX_ENTRIES = 10000000;
+ private final Logger logger = LoggerFactory.getLogger(MapEntries.class);
+
/** Key for the global list. */
private static final String GLOBAL_LIST_KEY = "*";
@@ -109,7 +87,7 @@ public class MapEntries implements
private static final String JCR_SYSTEM_PREFIX = "/jcr:system/";
- static final String ALIAS_QUERY_DEFAULT = "SELECT sling:alias FROM nt:base WHERE sling:alias IS NOT NULL";
+ static final String ALIAS_BASE_QUERY_DEFAULT = "SELECT sling:alias FROM nt:base AS page";
static final String ANY_SCHEME_HOST = "[^/]+/[^/]+";
@@ -1038,23 +1016,58 @@ public class MapEntries implements
*/
private Map<String, Map<String, String>> loadAliases(final ResourceResolver resolver) {
final Map<String, Map<String, String>> map = new ConcurrentHashMap<>();
- final String queryString = ALIAS_QUERY_DEFAULT;
- final Iterator<Resource> i = resolver.findResources(queryString, "sql");
- while (i.hasNext()) {
+ final String queryString = updateAliasQuery();
+ final Iterator<Resource> i = resolver.findResources(queryString, "sql");
+ while (i.hasNext()) {
final Resource resource = i.next();
loadAlias(resource, map);
}
return map;
}
- /**
+
+ /*
+ * Update alias query based on configured alias locations
+ */
+ private String updateAliasQuery(){
+ final Set<String> allowedLocations = this.factory.getAllowedAliasLocations();
+
+ StringBuilder baseQuery = new StringBuilder(ALIAS_BASE_QUERY_DEFAULT);
+ baseQuery.append(" ").append("WHERE");
+
+ if(allowedLocations.isEmpty()){
+ baseQuery.append(" ").append("(").append("NOT ISDESCENDANTNODE(page,")
+ .append("\"").append(JCR_SYSTEM_PREFIX).append("\"").append("))");
+
+ }else{
+ Iterator<String> pathIterator = allowedLocations.iterator();
+ baseQuery.append("(");
+ while(pathIterator.hasNext()){
+ String prefix = pathIterator.next();
+ baseQuery.append(" ").append("ISDESCENDANTNODE(page,")
+ .append("\"").append(prefix).append("\"")
+ .append(")").append(" ").append("OR");
+ }
+ //Remove last "OR" keyword
+ int orLastIndex = baseQuery.lastIndexOf("OR");
+ baseQuery.delete(orLastIndex,baseQuery.length());
+ baseQuery.append(")");
+ }
+
+ baseQuery.append(" AND sling:alias IS NOT NULL ");
+ String aliasQuery = baseQuery.toString();
+ logger.debug("Query to fetch alias [{}] ", aliasQuery);
+
+ return aliasQuery;
+ }
+
+ /**
* Load alias given a resource
*/
private boolean loadAlias(final Resource resource, Map<String, Map<String, String>> map) {
// ignore system tree
- if (resource.getPath().startsWith(JCR_SYSTEM_PREFIX)) {
- log.debug("loadAliases: Ignoring {}", resource);
- return false;
+ if(resource.getPath() == null){
+ throw new IllegalArgumentException("Unexpected null path");
}
final String resourceName;
@@ -1091,6 +1104,7 @@ public class MapEntries implements
final String[] aliasArray = props.get(ResourceResolverImpl.PROP_ALIAS, String[].class);
if ( aliasArray != null ) {
+ logger.debug("Found alias, total size {}", aliasArray.length);
Map<String, String> parentMap = map.get(parentPath);
for (final String alias : aliasArray) {
if (parentMap != null && parentMap.containsKey(alias)) {
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
index b960a82..a9f42ae 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/MockedResourceResolverImplTest.java
@@ -236,6 +236,11 @@ public class MockedResourceResolverImplTest {
}
@Override
+ public String[] resource_resolver_allowed_alias_locations() {
+ return new String[]{"/apps/", "/libs/", "/content/"};
+ }
+
+ @Override
public String[] resource_resolver_mapping() {
return new String[] { "/:/",
"/content/:/", "/system/docroot/:/", "/content.html-/$" };
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryTest.java
index 57234f0..b44d713 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryTest.java
@@ -18,21 +18,26 @@
*/
package org.apache.sling.resourceresolver.impl;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNull;
-
import org.apache.sling.api.resource.ResourceResolver;
import org.apache.sling.resourceresolver.impl.providers.ResourceProviderTracker;
import org.junit.Before;
import org.junit.Test;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.TreeSet;
+
+import static org.apache.sling.resourceresolver.util.MockTestUtil.setInaccessibleField;
+import static org.junit.Assert.*;
+
public class ResourceResolverFactoryTest {
private CommonResourceResolverFactoryImpl commonFactory;
+ private ResourceResolverFactoryActivator activator;
+
@Before public void setup() {
- ResourceResolverFactoryActivator activator = new ResourceResolverFactoryActivator();
+ activator = new ResourceResolverFactoryActivator();
activator.resourceProviderTracker = new ResourceProviderTracker();
commonFactory = new CommonResourceResolverFactoryImpl(activator);
}
@@ -99,4 +104,12 @@ public class ResourceResolverFactoryTest {
admin.close();
assertNull(this.commonFactory.getThreadResourceResolver());
}
+
+
+ @Test public void testGetAllowedAliasPaths() throws NoSuchMethodException {
+ assertTrue(this.commonFactory.getAllowedAliasLocations().isEmpty());
+ String[] allowPaths = {"/parent", "/parent0"};
+ setInaccessibleField("allowedAliasLocations", activator, Collections.unmodifiableSet(new TreeSet<>(Arrays.asList(allowPaths))));
+ assertTrue(!this.commonFactory.getAllowedAliasLocations().isEmpty());
+ }
}
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
index fc6691c..eef569a 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/MapEntriesTest.java
@@ -31,20 +31,9 @@ import java.io.File;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.LinkedList;
-import java.util.List;
-import java.util.Map;
-import java.util.Random;
-import java.util.Set;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Semaphore;
-import java.util.concurrent.TimeUnit;
+import java.util.*;
+
+import java.util.concurrent.*;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
@@ -91,6 +80,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
private EventAdmin eventAdmin;
private Map<String, Map<String, String>> aliasMap;
+ private int testSize = 5;
@SuppressWarnings({ "unchecked" })
@Before
@@ -124,6 +114,14 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
when(resourceResolverFactory.isMaxCachedVanityPathEntriesStartup()).thenReturn(true);
when(resourceResolver.findResources(anyString(), eq("sql"))).thenReturn(
Collections.<Resource> emptySet().iterator());
+ //when(resourceResolverFactory.getAliasPath()).thenReturn(Arrays.asList("/child"));
+
+ Set<String> aliasPath = new TreeSet<>();
+ aliasPath.add("/parent");
+ for(int i = 1; i < testSize; i++){
+ aliasPath.add("/parent"+i);
+ }
+ when(resourceResolverFactory.getAllowedAliasLocations()).thenReturn(aliasPath);
mapEntries = new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider);
final Field aliasMapField = MapEntries.class.getDeclaredField("aliasMap");
@@ -1005,6 +1003,23 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
assertEquals("parent", aliasMapEntry.get("aliasJcrContent"));
assertEquals(1, aliasMap.size());
+
+ //trying to add invalid alias path
+ final Resource invalidResourcePath = mock(Resource.class);
+ when(resourceResolver.getResource("/notallowedparent")).thenReturn(invalidResourcePath);
+ when(invalidResourcePath.getParent()).thenReturn(parent);
+ when(invalidResourcePath.getPath()).thenReturn("/notallowedparent");
+ when(invalidResourcePath.getName()).thenReturn("notallowedparent");
+ when(invalidResourcePath.getValueMap()).thenReturn(buildValueMap(ResourceResolverImpl.PROP_ALIAS, "alias"));
+
+ addResource.invoke(mapEntries, "/notallowedparent", new AtomicBoolean());
+
+ aliasMapEntry = mapEntries.getAliasMap("/");
+ assertNotNull(aliasMapEntry);
+ assertTrue(aliasMapEntry.containsKey("alias"));
+ assertEquals("parent", aliasMapEntry.get("alias"));
+ assertEquals(1, aliasMap.size());
+
}
@Test