You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by cz...@apache.org on 2022/09/05 12:36:10 UTC

[sling-org-apache-sling-resourceresolver] branch master updated: SLING-11541: vanity path query: attempt to query sorted by first vanity path, check results (#81)

This is an automated email from the ASF dual-hosted git repository.

cziegeler 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 ee2a0b2  SLING-11541: vanity path query: attempt to query sorted by first vanity path, check results (#81)
ee2a0b2 is described below

commit ee2a0b2317ec71c3af6e0655cf04487fc7273056
Author: Julian Reschke <ju...@gmx.de>
AuthorDate: Mon Sep 5 14:36:04 2022 +0200

    SLING-11541: vanity path query: attempt to query sorted by first vanity path, check results (#81)
    
    * SLING-11541: vanity path query: attempt to query sorted by first vanity path, check results
    
    * SLING-11541: vanity path query: attempt to query sorted by first vanity path - update tests
    
    * SLING-11541: vanity path query: attempt to query sorted by first vanity path: trace-level logging of query results
    
    * SLING-11541: vanity path query - use 'it' instead of 'i' as iterator name
---
 .../resourceresolver/impl/mapping/MapEntries.java  | 56 ++++++++++++++++------
 .../impl/mapping/InMemoryResourceProvider.java     |  2 +-
 2 files changed, 43 insertions(+), 15 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 dd9ee38..c23fc84 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
@@ -22,6 +22,7 @@ import org.apache.commons.collections4.map.LRUMap;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.sling.api.SlingConstants;
 import org.apache.sling.api.resource.LoginException;
+import org.apache.sling.api.resource.QuerySyntaxException;
 import org.apache.sling.api.resource.Resource;
 import org.apache.sling.api.resource.ResourceResolver;
 import org.apache.sling.api.resource.ResourceUtil;
@@ -533,7 +534,7 @@ public class MapEntries implements
         log.debug("doAddVanity getting {}", resource.getPath());
 
         boolean updateTheCache = isAllVanityPathEntriesCached() || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries();
-        return loadVanityPath(resource, resolveMapsMap, vanityTargets, updateTheCache);
+        return null != loadVanityPath(resource, resolveMapsMap, vanityTargets, updateTheCache);
     }
 
     private boolean doRemoveVanity(final String path) {
@@ -963,7 +964,7 @@ public class MapEntries implements
                         loadVanityPath(resource, resolveMapsMap, vanityTargets, true);
                         entryMap = resolveMapsMap;
                     } else {
-                        final Map <String, List<String>> targetPaths = new HashMap <>();
+                        final Map <String, List<String>> targetPaths = new HashMap<>();
                         loadVanityPath(resource, entryMap, targetPaths, true);
                     }
                 }
@@ -1275,35 +1276,56 @@ public class MapEntries implements
         return invalid;
     }
 
+    private Iterator<Resource> queryAllVanityPaths(String query) {
+        log.debug("start vanityPath query: {}", query);
+        long queryStart = System.nanoTime();
+        final Iterator<Resource> it = resolver.findResources(query, "JCR-SQL2");
+        long queryElapsed = System.nanoTime() - queryStart;
+        log.debug("end vanityPath query; elapsed {}ms", TimeUnit.NANOSECONDS.toMillis(queryElapsed));
+        return it;
+    }
+
     /**
      * Load vanity paths - search for all nodes (except under /jcr:system)
      * having a sling:vanityPath property
      */
     private Map<String, List<String>> loadVanityPaths(ResourceResolver resolver) {
         final Map<String, List<String>> targetPaths = new ConcurrentHashMap<>();
-        final String queryString = "SELECT [sling:vanityPath], [sling:redirect], [sling:redirectStatus]" + " FROM [nt:base]"
+        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]";
 
-        log.debug("start vanityPath query: {}", queryString);
-        long queryStart = System.nanoTime();
-        final Iterator<Resource> i = resolver.findResources(queryString, "JCR-SQL2");
-        long queryElapsed = System.nanoTime() - queryStart;
-        log.debug("end vanityPath query; elapsed {}ms", TimeUnit.NANOSECONDS.toMillis(queryElapsed));
+        boolean supportsSort = true;
+        Iterator<Resource> it;
+        try {
+            it = queryAllVanityPaths(queryStringWithSort);
+        } catch (QuerySyntaxException ex) {
+            log.debug("sort with first() not supported, falling back to base query");
+            supportsSort = false;
+            it = queryAllVanityPaths(baseQueryString);
+        }
 
         long count = 0;
         long countInScope = 0;
         long processStart = System.nanoTime();
+        String previousVanityPath = null;
 
-        while (i.hasNext()) {
+        while (it.hasNext()) {
             count += 1;
-            final Resource resource = i.next();
+            final Resource resource = it.next();
             final String resourcePath = resource.getPath();
             if (Stream.of(this.factory.getObservationPaths()).anyMatch(path -> path.matches(resourcePath))) {
                 countInScope += 1;
                 final boolean addToCache = isAllVanityPathEntriesCached()
                         || vanityCounter.longValue() < this.factory.getMaxCachedVanityPathEntries();
-                loadVanityPath(resource, resolveMapsMap, targetPaths, addToCache);
+                String firstVanityPath = loadVanityPath(resource, resolveMapsMap, targetPaths, addToCache);
+                if (supportsSort && firstVanityPath != null) {
+                    if (previousVanityPath != null && firstVanityPath.compareTo(previousVanityPath) < 0) {
+                        log.error("Sorting by first(vanityPath) does not appear to work; got " + firstVanityPath + " after " + previousVanityPath);
+                    }
+                    previousVanityPath = firstVanityPath;
+               }
             }
         }
         long processElapsed = System.nanoTime() - processStart;
@@ -1321,11 +1343,13 @@ public class MapEntries implements
 
     /**
      * Load vanity path given a resource
+     * 
+     * @return first vanity path or {@code null}
      */
-    private boolean loadVanityPath(final Resource resource, final Map<String, List<MapEntry>> entryMap, final Map <String, List<String>> targetPaths, boolean addToCache) {
+    private String loadVanityPath(final Resource resource, final Map<String, List<MapEntry>> entryMap, final Map <String, List<String>> targetPaths, boolean addToCache) {
 
         if (!isValidVanityPath(resource.getPath())) {
-            return false;
+            return null;
         }
 
         final ValueMap props = resource.getValueMap();
@@ -1335,6 +1359,10 @@ public class MapEntries implements
         // what is stored in the sling:vanityPath property
         boolean hasVanityPath = false;
         final String[] pVanityPaths = props.get(PROP_VANITY_PATH, new String[0]);
+        if (log.isTraceEnabled()) {
+            log.trace("vanity paths on {}: {}", resource.getPath(), Arrays.asList(pVanityPaths));
+        }
+
         for (final String pVanityPath : pVanityPaths) {
             final String[] result = this.getVanityPathDefinition(pVanityPath);
             if (result != null) {
@@ -1397,7 +1425,7 @@ public class MapEntries implements
                 }
             }
         }
-        return hasVanityPath;
+        return hasVanityPath ? pVanityPaths[0] : null;
     }
 
     private void updateTargetPaths(final Map<String, List<String>> targetPaths, final String key, final String entry) {
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 507837b..9a667e8 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
@@ -103,7 +103,7 @@ 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".equals(query) ) {
+                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();
                 }