You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by jo...@apache.org on 2022/11/03 17:04:13 UTC
[sling-org-apache-sling-resourceresolver] branch master updated: SLING-11581: use keyset pagination for vanity path query (#82)
This is an automated email from the ASF dual-hosted git repository.
joerghoh 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 51ed019 SLING-11581: use keyset pagination for vanity path query (#82)
51ed019 is described below
commit 51ed0197c143010dc3c75cacb5a56afc590cda18
Author: Julian Reschke <ju...@gmx.de>
AuthorDate: Thu Nov 3 18:04:08 2022 +0100
SLING-11581: use keyset pagination for vanity path query (#82)
* SLING-11581: use keyset pagination for vanity path query
---
.../resourceresolver/impl/mapping/MapEntries.java | 78 ++++++++++++++++++-
.../impl/mapping/InMemoryResourceProvider.java | 25 +++++-
.../impl/mapping/MapEntriesTest.java | 89 +++++++++++++++++++---
.../impl/mapping/ResourceMapperImplTest.java | 17 +++--
4 files changed, 185 insertions(+), 24 deletions(-)
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 178fdce..5107bdb 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
@@ -164,7 +164,7 @@ public class MapEntries implements
final Optional<ResourceResolverMetrics> metrics)
throws LoginException, IOException {
- this.resolver = factory.getServiceResourceResolver(factory.getServiceUserAuthenticationInfo("mapping"));
+ this.resolver = factory.getServiceResourceResolver(factory.getServiceUserAuthenticationInfo("mapping"));
this.factory = factory;
this.eventAdmin = eventAdmin;
@@ -1287,6 +1287,78 @@ public class MapEntries implements
return it;
}
+ private class PagedQueryIterator implements Iterator<Resource> {
+
+ private ResourceResolver resolver;
+ private String query;
+ private String lastPath = "";
+ private Iterator<Resource> it;
+ private int count = 0;
+ private int page = 0;
+ private int pageSize = Integer.getInteger("sling.vanityPath.pageSize", 2000);
+ private Resource next = null;
+
+ public PagedQueryIterator(ResourceResolver resolver, String query) {
+ this.resolver = resolver;
+ this.query = query;
+ nextPage();
+ }
+
+ private void nextPage() {
+ count = 0;
+ String tquery = String.format(query, queryLiteral(lastPath));
+ log.debug("start vanity path query (page {}): {}", page, tquery);
+ long queryStart = System.nanoTime();
+ this.it = resolver.findResources(tquery, "JCR-SQL2");
+ long queryElapsed = System.nanoTime() - queryStart;
+ log.debug("end vanity path query (page {}); elapsed {}ms", page, TimeUnit.NANOSECONDS.toMillis(queryElapsed));
+ page += 1;
+ }
+
+ private Resource getNext() throws NoSuchElementException {
+ Resource resource = it.next();
+ count += 1;
+ final String[] paths = resource.getValueMap().get(PROP_VANITY_PATH, new String[0]);
+ if (paths.length > 0) {
+ String p = paths[0];
+ if (p.compareTo(lastPath) < 0) {
+ String message = String.format(
+ "unexpected query result in page %d, vanity path of '%s' despite querying for > '%s'", (page - 1), p,
+ lastPath);
+ log.error(message);
+ throw new RuntimeException(message);
+ }
+ // start next page?
+ if (count > pageSize && !p.equals(lastPath)) {
+ lastPath = p;
+ nextPage();
+ }
+ }
+
+ return resource;
+ }
+
+ @Override
+ public boolean hasNext() {
+ if (next == null) {
+ try {
+ next = getNext();
+ } catch (NoSuchElementException ex) {
+ // there are no more
+ next = null;
+ }
+ }
+ return next != null;
+ }
+
+ @Override
+ public Resource next() throws NoSuchElementException {
+ Resource result = next != null ? next : getNext();
+ next = null;
+ return result;
+ }
+ }
+
/**
* Load vanity paths - search for all nodes (except under /jcr:system)
* having a sling:vanityPath property
@@ -1296,12 +1368,12 @@ public class MapEntries implements
final String baseQueryString = "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus]" + " FROM [nt:base]"
+ " WHERE NOT isdescendantnode('" + queryLiteral(JCR_SYSTEM_PATH) + "')"
+ " AND [sling:vanityPath] IS NOT NULL";
- final String queryStringWithSort = baseQueryString + " ORDER BY FIRST([sling:vanityPath]), [jcr:path]";
boolean supportsSort = true;
Iterator<Resource> it;
try {
- it = queryAllVanityPaths(queryStringWithSort);
+ final String queryStringWithSort = baseQueryString + " AND FIRST([sling:vanityPath]) > '%s' ORDER BY FIRST([sling:vanityPath])";
+ it = new PagedQueryIterator(resolver, queryStringWithSort);
} catch (QuerySyntaxException ex) {
log.debug("sort with first() not supported, falling back to base query");
supportsSort = false;
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
index 9a667e8..fe74988 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/InMemoryResourceProvider.java
@@ -26,6 +26,7 @@ import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
+import org.apache.sling.api.resource.QuerySyntaxException;
import org.apache.sling.api.resource.Resource;
import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.ValueMap;
@@ -41,6 +42,12 @@ public class InMemoryResourceProvider extends ResourceProvider<Void>{
private final Map<String, Map<String, Object>> resources = new HashMap<>();
+ private final boolean supportsPagedQuery;
+
+ public InMemoryResourceProvider(boolean supportsPagedQuery) {
+ this.supportsPagedQuery = supportsPagedQuery;
+ }
+
@Override
public Resource getResource(ResolveContext<Void> ctx, String path, ResourceContext resourceContext,
Resource parent) {
@@ -60,7 +67,7 @@ public class InMemoryResourceProvider extends ResourceProvider<Void>{
.map( e -> (Resource) new InMemoryResource(e.getKey(), ctx.getResourceResolver(), e.getValue()) )
.iterator();
}
-
+
public void putResource(String path) {
putResource(path, Collections.emptyMap());
}
@@ -103,9 +110,19 @@ public class InMemoryResourceProvider extends ResourceProvider<Void>{
.iterator();
}
- if ( "JCR-SQL2".equals(language) && "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:vanityPath] IS NOT NULL ORDER BY FIRST([sling:vanityPath]), [jcr:path]".equals(query) ) {
- return resourcesWithProperty(ctx, "sling:vanityPath")
- .iterator();
+ if ( "JCR-SQL2".equals(language) && "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:vanityPath] IS NOT NULL AND FIRST([sling:vanityPath]) > '' ORDER BY FIRST([sling:vanityPath])".equals(query) ) {
+ if (supportsPagedQuery) {
+ return resourcesWithProperty(ctx, "sling:vanityPath")
+ .iterator();
+ }
+ else {
+ throw new QuerySyntaxException("paged queries not supported", query, language);
+ }
+ }
+
+ if ( "JCR-SQL2".equals(language) && "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:vanityPath] IS NOT NULL".equals(query) ) {
+ return resourcesWithProperty(ctx, "sling:vanityPath")
+ .iterator();
}
throw new UnsupportedOperationException("Unsupported query: '" + query + "'");
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 8e71bc6..6a0b69c 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
@@ -30,9 +30,24 @@ import static org.mockito.Mockito.when;
import java.io.IOException;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
-import java.util.*;
-
-import java.util.concurrent.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.LinkedList;
+import java.util.List;
+import java.util.Map;
+import java.util.Optional;
+import java.util.Random;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Semaphore;
+import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicLong;
@@ -50,6 +65,9 @@ import org.apache.sling.resourceresolver.impl.mapping.MapConfigurationProvider.V
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameters;
import org.mockito.Mock;
import org.mockito.Mockito;
import org.mockito.MockitoAnnotations;
@@ -59,6 +77,7 @@ import org.osgi.framework.Bundle;
import org.osgi.framework.BundleContext;
import org.osgi.service.event.EventAdmin;
+@RunWith(Parameterized.class)
public class MapEntriesTest extends AbstractMappingMapEntriesTest {
private MapEntries mapEntries;
@@ -81,10 +100,25 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
private Map<String, Map<String, String>> aliasMap;
private int testSize = 5;
+ private int pageSize;
+ private int prevPageSize = 1000;
+
+ @Parameters(name = "page size -> {0}")
+ public static Collection<Object[]> data() {
+ return Arrays.asList(new Object[][] { { 1 }, { 1000 } });
+ }
+
+ public MapEntriesTest(int pageSize) {
+ this.pageSize = pageSize;
+ }
+
@Override
@SuppressWarnings({ "unchecked" })
@Before
public void setup() throws Exception {
+ prevPageSize = Integer.getInteger("sling.vanityPath.pageSize", 2000);
+ System.setProperty("sling.vanityPath.pageSize", Integer.toString(pageSize));
+
MockitoAnnotations.initMocks(this);
final List<VanityPathConfig> configs = new ArrayList<>();
@@ -122,7 +156,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
aliasPath.add("/parent"+i);
}
when(resourceResolverFactory.getAllowedAliasLocations()).thenReturn(aliasPath);
-
+
Optional<ResourceResolverMetrics> metrics = Optional.empty();
mapEntries = Mockito.spy(new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics));
@@ -135,6 +169,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@Override
@After
public void tearDown() throws Exception {
+ System.setProperty("sling.vanityPath.pageSize", Integer.toString(prevPageSize));
mapEntries.dispose();
}
@@ -229,7 +264,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
when(badVanityPath.getValueMap()).thenReturn(buildValueMap("sling:vanityPath", "/content/mypage/en-us-{132"));
resources.add(badVanityPath);
-
Resource redirectingVanityPath = mock(Resource.class, "redirectingVanityPath");
when(redirectingVanityPath.getPath()).thenReturn("/redirectingVanityPath");
when(redirectingVanityPath.getName()).thenReturn("redirectingVanityPath");
@@ -257,7 +291,12 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@Override
public Iterator<Resource> answer(InvocationOnMock invocation) throws Throwable {
- if (invocation.getArguments()[0].toString().contains("sling:vanityPath")) {
+ String query = invocation.getArguments()[0].toString();
+ if (matchesPagedQuery(query)) {
+ String path = extractStartPath(query);
+ Collections.sort(resources, vanityResourceComparator);
+ return resources.stream().filter(e -> getFirstVanityPath(e).compareTo(path) > 0).iterator();
+ } else if (query.contains("sling:vanityPath")) {
return resources.iterator();
} else {
return Collections.<Resource> emptySet().iterator();
@@ -269,6 +308,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
mapEntries.initializeVanityPaths();
List<MapEntry> entries = mapEntries.getResolveMaps();
+
assertEquals(8, entries.size());
for (MapEntry entry : entries) {
if (entry.getPattern().contains("/target/redirectingVanityPath301")) {
@@ -291,7 +331,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@SuppressWarnings("unchecked")
Map<String, List<String>> vanityTargets = (Map<String, List<String>>) field.get(mapEntries);
assertEquals(4, vanityTargets.size());
-
}
@Test
@@ -423,7 +462,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
// a single event is sent for all 3 added vanity paths
Mockito.verify(eventAdmin,Mockito.times(3)).postEvent(Mockito.anyObject());
-
}
@Test
@@ -444,7 +482,13 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
@Override
public Iterator<Resource> answer(InvocationOnMock invocation) throws Throwable {
- if (invocation.getArguments()[0].toString().contains("sling:vanityPath")) {
+ String query = invocation.getArguments()[0].toString();
+ if (matchesPagedQuery(query)) {
+ String path = extractStartPath(query);
+ Collections.sort(resources, vanityResourceComparator);
+ return resources.stream().filter(e -> getFirstVanityPath(e).compareTo(path) > 0).iterator();
+ } else
+ if (query.contains("sling:vanityPath")) {
return resources.iterator();
} else {
return Collections.<Resource> emptySet().iterator();
@@ -2259,4 +2303,31 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest {
mapEntries.doInit();
}
+ // utilities for testing vanity path queries
+
+ private static String VPQSTART = "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus] FROM [nt:base] WHERE NOT isdescendantnode('/jcr:system') AND [sling:vanityPath] IS NOT NULL AND FIRST([sling:vanityPath]) > '";
+ private static String VPQEND = "' ORDER BY FIRST([sling:vanityPath])";
+
+ private boolean matchesPagedQuery(String query) {
+ return query.startsWith(VPQSTART) && query.endsWith(VPQEND);
+ }
+
+ private String extractStartPath(String query) {
+ String remainder = query.substring(VPQSTART.length());
+ return remainder.substring(0, remainder.length() - VPQEND.length());
+ }
+
+ private String getFirstVanityPath(Resource r) {
+ String vp[] = r.getValueMap().get("sling:vanityPath", new String[0]);
+ return vp.length == 0 ? "": vp[0];
+ }
+
+ private Comparator<Resource> vanityResourceComparator = new Comparator<Resource>() {
+ @Override
+ public int compare(Resource o1, Resource o2) {
+ String s1 = getFirstVanityPath(o1);
+ String s2 = getFirstVanityPath(o2);
+ return s1.compareTo(s2);
+ }
+ };
}
\ No newline at end of file
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 9608601..33c3803 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
@@ -29,6 +29,7 @@ import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import java.util.Arrays;
+import java.util.Collection;
import java.util.HashSet;
import java.util.Set;
@@ -42,7 +43,6 @@ import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.mapping.ResourceMapper;
import org.apache.sling.resourceresolver.impl.ResourceAccessSecurityTracker;
import org.apache.sling.resourceresolver.impl.ResourceResolverFactoryActivator;
-import org.apache.sling.resourceresolver.impl.ResourceResolverMetrics;
import org.apache.sling.serviceusermapping.impl.ServiceUserMapperImpl;
import org.apache.sling.spi.resource.provider.ResourceProvider;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
@@ -53,7 +53,6 @@ import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
-import org.mockito.Mockito;
/**
* Validates that the {@link ResourceMapperImpl} correctly queries all sources of mappings
@@ -76,20 +75,22 @@ import org.mockito.Mockito;
@RunWith(Parameterized.class)
public class ResourceMapperImplTest {
- @Parameters(name="optimized alias resolution → {0}")
- public static Object[] data() {
- return new Object[] { false, true};
+ @Parameters(name = "optimized alias resolution / paged query support -> {0} / {1}")
+ public static Collection<Object[]> data() {
+ return Arrays.asList(new Object[][] { { false, false }, { false, true }, { true, false }, { true, true } });
}
-
+
@Rule
public final OsgiContext ctx = new OsgiContext();
private final boolean optimiseAliasResolution;
+ private final boolean pagedQuerySupport;
private HttpServletRequest req;
private ResourceResolver resolver;
- public ResourceMapperImplTest(boolean optimiseAliasResolution) {
+ public ResourceMapperImplTest(boolean optimiseAliasResolution, boolean pagedQuerySupport) {
this.optimiseAliasResolution = optimiseAliasResolution;
+ this.pagedQuerySupport = pagedQuerySupport;
}
@Before
@@ -99,7 +100,7 @@ public class ResourceMapperImplTest {
ctx.registerInjectActivateService(new ResourceAccessSecurityTracker());
ctx.registerInjectActivateService(new StringInterpolationProviderImpl());
- InMemoryResourceProvider resourceProvider = new InMemoryResourceProvider();
+ InMemoryResourceProvider resourceProvider = new InMemoryResourceProvider(pagedQuerySupport);
resourceProvider.putResource("/"); // root
resourceProvider.putResource("/here"); // regular page
resourceProvider.putResource("/there", PROP_ALIAS, "alias-value"); // with alias