You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by re...@apache.org on 2025/02/18 17:02:40 UTC

(sling-org-apache-sling-resourceresolver) branch OAK-12648b created (now b6b90e2)

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

reschke pushed a change to branch OAK-12648b
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git


      at b6b90e2  SLING-12648: MapEntries: factor out vanity path handling into inner class

This branch includes the following new commits:

     new b6b90e2  SLING-12648: MapEntries: factor out vanity path handling into inner class

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



(sling-org-apache-sling-resourceresolver) 01/01: SLING-12648: MapEntries: factor out vanity path handling into inner class

Posted by re...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

reschke pushed a commit to branch OAK-12648b
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git

commit b6b90e2120e697ede28a825d6a19be0f5ef3a2a2
Author: Julian Reschke <re...@apache.org>
AuthorDate: Tue Feb 18 18:02:23 2025 +0100

    SLING-12648: MapEntries: factor out vanity path handling into inner class
---
 .../resourceresolver/impl/mapping/MapEntries.java  | 127 ++++++++++++---------
 .../impl/mapping/VanityPathMapEntriesTest.java     |  55 ++++-----
 2 files changed, 97 insertions(+), 85 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 170376f..2c90c51 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
@@ -93,12 +93,6 @@ public class MapEntries implements
 
     public static final String PROP_REDIRECT_EXTERNAL_REDIRECT_STATUS = "sling:redirectStatus";
 
-    public static final String PROP_VANITY_PATH = "sling:vanityPath";
-
-    public static final String PROP_VANITY_ORDER = "sling:vanityOrder";
-
-    private static final int VANITY_BLOOM_FILTER_MAX_ENTRIES = 10000000;
-
     /** Key for the global list. */
     private static final String GLOBAL_LIST_KEY = "*";
 
@@ -128,16 +122,10 @@ public class MapEntries implements
 
     private final Map<String, List<MapEntry>> resolveMapsMap;
 
-    // Temporary cache for use while doing async vanity path query
-    private Map<String, List<MapEntry>> temporaryResolveMapsMap;
     private List<Map.Entry<String, ResourceChange.ChangeType>> resourceChangeQueue;
-    private AtomicLong temporaryResolveMapsMapHits = new AtomicLong();
-    private AtomicLong temporaryResolveMapsMapMisses = new AtomicLong();
 
     private Collection<MapEntry> mapMaps;
 
-    private Map <String,List <String>> vanityTargets;
-
     /**
      * The key of the map is the parent path, while the value is a map with the the resource name as key and the actual aliases as values)
      */
@@ -152,20 +140,13 @@ public class MapEntries implements
 
     private final ReentrantLock initializing = new ReentrantLock();
 
-    private final AtomicLong vanityCounter;
-    private final AtomicLong vanityResourcesOnStartup;
-    private final AtomicLong vanityPathLookups;
-    private final AtomicLong vanityPathBloomNegatives;
-    private final AtomicLong vanityPathBloomFalsePositives;
-
-    private byte[] vanityBloomFilter;
-
-    private AtomicBoolean vanityPathsProcessed = new AtomicBoolean(false);
-
     private final StringInterpolationProvider stringInterpolationProvider;
 
     private final boolean useOptimizeAliasResolution;
-    public MapEntries(final MapConfigurationProvider factory, 
+
+    final VanityPathHandler vph;
+
+    public MapEntries(final MapConfigurationProvider factory,
             final BundleContext bundleContext, 
             final EventAdmin eventAdmin, 
             final StringInterpolationProvider stringInterpolationProvider, 
@@ -178,7 +159,6 @@ public class MapEntries implements
 
         this.resolveMapsMap = new ConcurrentHashMap<>(Map.of(GLOBAL_LIST_KEY, List.of()));
         this.mapMaps = Collections.<MapEntry> emptyList();
-        this.vanityTargets = Collections.<String,List <String>>emptyMap();
         this.aliasMapsMap = new ConcurrentHashMap<>();
         this.stringInterpolationProvider = stringInterpolationProvider;
 
@@ -190,21 +170,16 @@ public class MapEntries implements
 
         this.registration = registerResourceChangeListener(bundleContext);
 
-        this.vanityCounter = new AtomicLong(0);
-        this.vanityResourcesOnStartup = new AtomicLong(0);
-        this.vanityPathLookups = new AtomicLong(0);
-        this.vanityPathBloomNegatives = new AtomicLong(0);
-        this.vanityPathBloomFalsePositives = new AtomicLong(0);
-
-        initializeVanityPaths();
+        this.vph = new VanityPathHandler(this.factory, this.resolveMapsMap, this.initializing);
+        this.vph.initializeVanityPaths();
 
         this.metrics = metrics;
         if (metrics.isPresent()) {
-            this.metrics.get().setNumberOfVanityPathsSupplier(vanityCounter::get);
-            this.metrics.get().setNumberOfResourcesWithVanityPathsOnStartupSupplier(vanityResourcesOnStartup::get);
-            this.metrics.get().setNumberOfVanityPathLookupsSupplier(vanityPathLookups::get);
-            this.metrics.get().setNumberOfVanityPathBloomNegativesSupplier(vanityPathBloomNegatives::get);
-            this.metrics.get().setNumberOfVanityPathBloomFalsePositivesSupplier(vanityPathBloomFalsePositives::get);
+            this.metrics.get().setNumberOfVanityPathsSupplier(vph.vanityCounter::get);
+            this.metrics.get().setNumberOfResourcesWithVanityPathsOnStartupSupplier(vph.vanityResourcesOnStartup::get);
+            this.metrics.get().setNumberOfVanityPathLookupsSupplier(vph.vanityPathLookups::get);
+            this.metrics.get().setNumberOfVanityPathBloomNegativesSupplier(vph.vanityPathBloomNegatives::get);
+            this.metrics.get().setNumberOfVanityPathBloomFalsePositivesSupplier(vph.vanityPathBloomFalsePositives::get);
             this.metrics.get().setNumberOfResourcesWithAliasedChildrenSupplier(() -> (long) aliasMapsMap.size());
             this.metrics.get().setNumberOfResourcesWithAliasesOnStartupSupplier(aliasResourcesOnStartup::get);
             this.metrics.get().setNumberOfDetectedConflictingAliasesSupplier(detectedConflictingAliases::get);
@@ -295,7 +270,7 @@ public class MapEntries implements
             this.refreshResolverIfNecessary(resolverRefreshed);
             final Resource resource = this.resolver != null ? resolver.getResource(path) : null;
             if (resource != null) {
-                boolean changed = doAddVanity(resource);
+                boolean changed = vph.doAddVanity(resource);
                 if (this.useOptimizeAliasResolution && resource.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) {
                     changed |= doAddAlias(resource);
                 }
@@ -309,7 +284,7 @@ public class MapEntries implements
     }
 
     private boolean updateResource(final String path, final AtomicBoolean resolverRefreshed) {
-        final boolean isValidVanityPath =  this.isValidVanityPath(path);
+        final boolean isValidVanityPath = vph.isValidVanityPath(path);
         if ( this.useOptimizeAliasResolution || isValidVanityPath) {
             this.initializing.lock();
 
@@ -320,7 +295,7 @@ public class MapEntries implements
                     boolean changed = false;
                     if ( isValidVanityPath ) {
                         // we remove the old vanity path first
-                        changed |= doRemoveVanity(path);
+                        changed |= vph.doRemoveVanity(path);
 
                         // add back vanity path
                         Resource contentRsrc = null;
@@ -328,7 +303,7 @@ public class MapEntries implements
                             // there might be a JCR_CONTENT child resource
                             contentRsrc = resource.getChild(JCR_CONTENT);
                         }
-                        changed |= doAddVanity(contentRsrc != null ? contentRsrc : resource);
+                        changed |= vph.doAddVanity(contentRsrc != null ? contentRsrc : resource);
                     }
                     if (this.useOptimizeAliasResolution) {
                         changed |= doUpdateAlias(resource);
@@ -349,9 +324,9 @@ public class MapEntries implements
         final String actualContentPath = getActualContentPath(path);
         final String actualContentPathPrefix = actualContentPath + "/";
 
-        for (final String target : this.vanityTargets.keySet()) {
+        for (final String target : vph.getVanityPathMappings().keySet()) {
             if (target.startsWith(actualContentPathPrefix) || target.equals(actualContentPath)) {
-                changed |= removeVanityPath(target);
+                changed |= vph.removeVanityPath(target);
             }
         }
         if (this.useOptimizeAliasResolution) {
@@ -569,7 +544,7 @@ public class MapEntries implements
         }
 
         return new MapEntryIterator(key, resolveMapsMap.get(GLOBAL_LIST_KEY),
-                this::getCurrentMapEntryForVanityPath, this.factory.hasVanityPathPrecedence());
+                vph::getCurrentMapEntryForVanityPath, this.factory.hasVanityPathPrecedence());
     }
 
     @Override
@@ -589,12 +564,9 @@ public class MapEntries implements
 
     @Override
     public Map<String, List<String>> getVanityPathMappings() {
-        return Collections.unmodifiableMap(vanityTargets);
+        return vph.getVanityPathMappings();
     }
 
-    // special singleton entry for negative cache entries
-    private static final List<MapEntry> NO_MAP_ENTRIES = Collections.emptyList();
-
     /**
      * Refresh the resource resolver if not already done
      * @param resolverRefreshed Boolean flag containing the state if the resolver
@@ -653,7 +625,7 @@ public class MapEntries implements
     @Override
     public void onChange(final List<ResourceChange> changes) {
 
-        final boolean inStartup = !vanityPathsProcessed.get();
+        final boolean inStartup = !vph.isReady();
 
         final AtomicBoolean resolverRefreshed = new AtomicBoolean(false);
 
@@ -1163,8 +1135,56 @@ public class MapEntries implements
 
     // ---- vanity path handling ----
 
-    /**
-     * Actual vanity paths initializer. Guards itself against concurrent use by
+    public class VanityPathHandler {
+
+    public static final String PROP_VANITY_PATH = "sling:vanityPath";
+    public static final String PROP_VANITY_ORDER = "sling:vanityOrder";
+
+    private static final int VANITY_BLOOM_FILTER_MAX_ENTRIES = 10000000;
+
+    private final AtomicLong vanityCounter = new AtomicLong(0);
+
+    private final AtomicLong vanityResourcesOnStartup = new AtomicLong(0);
+    private final AtomicLong vanityPathLookups = new AtomicLong(0);
+    private final AtomicLong vanityPathBloomNegatives = new AtomicLong(0);
+    private final AtomicLong vanityPathBloomFalsePositives = new AtomicLong(0);
+    private final AtomicLong temporaryResolveMapsMapHits = new AtomicLong();
+    private final AtomicLong temporaryResolveMapsMapMisses = new AtomicLong();
+
+    private final AtomicBoolean vanityPathsProcessed = new AtomicBoolean(false);
+
+    private final Logger log = LoggerFactory.getLogger(VanityPathHandler.class);
+
+    private final MapConfigurationProvider factory;
+    private byte[] vanityBloomFilter;
+
+    private Map <String,List <String>> vanityTargets = Collections.emptyMap();
+    private final Map<String, List<MapEntry>> resolveMapsMap;
+
+    // special singleton entry for negative cache entries
+    private final List<MapEntry> noMapEntries = Collections.emptyList();
+
+    // Temporary cache for use while doing async vanity path query
+    private Map<String, List<MapEntry>> temporaryResolveMapsMap;
+
+    private final ReentrantLock initializing;
+
+    public VanityPathHandler(MapConfigurationProvider factory, Map<String, List<MapEntry>> resolveMapsMap, ReentrantLock initializing) {
+        this.factory = factory;
+        this.resolveMapsMap = resolveMapsMap;
+        this.initializing = initializing;
+    }
+
+    public boolean isReady() {
+        return this.vanityPathsProcessed.get();
+    }
+
+    public Map<String, List<String>> getVanityPathMappings() {
+         return Collections.unmodifiableMap(vanityTargets);
+    }
+
+   /**
+     * Actual vanity path initializer. Guards itself against concurrent use by
      * using a ReentrantLock. Does nothing if the resource resolver has already
      * been null-ed.
      *
@@ -1363,7 +1383,7 @@ public class MapEntries implements
                     mapEntries = mapEntry.get(vanityPath);
                     if (!initFinished && temporaryResolveMapsMap != null) {
                         log.trace("getMapEntryList: caching map entries for {} -> {}", vanityPath, mapEntries);
-                        temporaryResolveMapsMap.put(vanityPath, mapEntries == null ? NO_MAP_ENTRIES : mapEntries);
+                        temporaryResolveMapsMap.put(vanityPath, mapEntries == null ? noMapEntries : mapEntries);
                     }
                 }
             }
@@ -1373,7 +1393,7 @@ public class MapEntries implements
             }
         }
 
-        return mapEntries == NO_MAP_ENTRIES ? null : mapEntries;
+        return mapEntries == noMapEntries ? null : mapEntries;
     }
 
     private byte[] createVanityBloomFilter() throws IOException {
@@ -1719,4 +1739,7 @@ public class MapEntries implements
             return true;
         }
     }
+
+    }
+
 }
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java
index 9d893a4..8905f0c 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/VanityPathMapEntriesTest.java
@@ -151,20 +151,9 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
         waitForBgInit();
     }
 
-    // get internal flag that signals completion of background task
-    private AtomicBoolean getVanityPathsProcessed() {
-        try {
-            Field field = MapEntries.class.getDeclaredField("vanityPathsProcessed");
-            field.setAccessible(true);
-            return (AtomicBoolean) field.get(mapEntries);
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
-    }
-
     // wait for background thread to complete
     private void waitForBgInit() {
-        while (!getVanityPathsProcessed().get()) {
+        while (!mapEntries.vph.isReady()) {
             try {
                 Thread.sleep(10);
             } catch (InterruptedException e) {
@@ -175,21 +164,21 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
 
     // get vanity paths (after waiting for bg init to complete)
     private void initializeVanityPaths() throws IOException {
-        mapEntries.initializeVanityPaths();
+        mapEntries.vph.initializeVanityPaths();
         waitForBgInit();
     }
 
     private static AtomicLong getVanityCounter(MapEntries mapEntries) throws NoSuchFieldException, IllegalAccessException {
-        Field vanityCounter = MapEntries.class.getDeclaredField("vanityCounter");
+        Field vanityCounter = MapEntries.VanityPathHandler.class.getDeclaredField("vanityCounter");
         vanityCounter.setAccessible(true);
-        return (AtomicLong) vanityCounter.get(mapEntries);
+        return (AtomicLong) vanityCounter.get(mapEntries.vph);
     }
 
     @SuppressWarnings("unchecked")
     private static Map<String, List<String>> getVanityTargets(MapEntries mapEntries) throws NoSuchFieldException, IllegalAccessException {
-        Field field = MapEntries.class.getDeclaredField("vanityTargets");
+        Field field = MapEntries.VanityPathHandler.class.getDeclaredField("vanityTargets");
         field.setAccessible(true);
-        return (Map<String, List<String>>) field.get(mapEntries);
+        return (Map<String, List<String>>) field.get(mapEntries.vph);
     }
 
     @SuppressWarnings("unchecked")
@@ -200,9 +189,9 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
     }
 
     private static void getVanityPaths(MapEntries mapEntries, String path) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException {
-        Method method = MapEntries.class.getDeclaredMethod("getVanityPaths", String.class);
+        Method method = MapEntries.VanityPathHandler.class.getDeclaredMethod("getVanityPaths", String.class);
         method.setAccessible(true);
-        method.invoke(mapEntries, path);
+        method.invoke(mapEntries.vph, path);
     }
 
     private static void addResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException {
@@ -212,9 +201,9 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
     }
 
     private static void loadVanityPaths(MapEntries mapEntries, ResourceResolver resourceResolver) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException {
-        Method method = MapEntries.class.getDeclaredMethod("loadVanityPaths", ResourceResolver.class);
+        Method method = MapEntries.VanityPathHandler.class.getDeclaredMethod("loadVanityPaths", ResourceResolver.class);
         method.setAccessible(true);
-        method.invoke(mapEntries, resourceResolver);
+        method.invoke(mapEntries.vph, resourceResolver);
     }
 
     @Override
@@ -293,14 +282,14 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
         when(oneMore.getPath()).thenReturn("/" + containerName + "/" + additionalChildName);
         when(oneMore.getName()).thenReturn(additionalChildName);
 
-        when(oneMore.getValueMap()).thenReturn(buildValueMap(MapEntries.PROP_VANITY_PATH, vanityPath + "/onemore"));
+        when(oneMore.getValueMap()).thenReturn(buildValueMap(MapEntries.VanityPathHandler.PROP_VANITY_PATH, vanityPath + "/onemore"));
 
         final Resource vanityPropHolder = onJcrContent ? content : vanity;
 
-        when(vanityPropHolder.getValueMap()).thenReturn(buildValueMap(MapEntries.PROP_VANITY_PATH, vanityPath));
+        when(vanityPropHolder.getValueMap()).thenReturn(buildValueMap(MapEntries.VanityPathHandler.PROP_VANITY_PATH, vanityPath));
 
         when(resourceResolver.findResources(anyString(), eq("JCR-SQL2"))).thenAnswer((Answer<Iterator<Resource>>) invocation -> {
-            if (invocation.getArguments()[0].toString().contains(MapEntries.PROP_VANITY_PATH)) {
+            if (invocation.getArguments()[0].toString().contains(MapEntries.VanityPathHandler.PROP_VANITY_PATH)) {
                 return List.of(vanityPropHolder, oneMore).iterator();
             } else {
                 return Collections.emptyIterator();
@@ -777,7 +766,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
         Map<String, List<String>> vanityTargets = getVanityTargets(mapEntries);
         assertEquals(0, vanityTargets.size());
 
-        Method method1 = MapEntries.class.getDeclaredMethod("doRemoveVanity", String.class);
+        Method method1 = MapEntries.VanityPathHandler.class.getDeclaredMethod("doRemoveVanity", String.class);
         method1.setAccessible(true);
 
         Resource justVanityPath = mock(Resource.class, "justVanityPath");
@@ -796,7 +785,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
         assertEquals("/target/justVanityPath", vanityTargets.get("/justVanityPath").get(0));
 
         //remove vanity path
-        method1.invoke(mapEntries, "/justVanityPath");
+        method1.invoke(mapEntries.vph, "/justVanityPath");
 
         assertEquals(0, getVanityCounter(mapEntries).longValue());
 
@@ -825,7 +814,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
         assertEquals("/target/vanityPathOnJcrContent", vanityTargets.get("/vanityPathOnJcrContent").get(0));
 
         //remove vanity path
-        method1.invoke(mapEntries, "/vanityPathOnJcrContent/jcr:content");
+        method1.invoke(mapEntries.vph, "/vanityPathOnJcrContent/jcr:content");
 
         assertEquals(1, resolveMapsMap.size());
         assertEquals(0, vanityTargets.size());
@@ -899,12 +888,12 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
 
     @Test
     public void test_isValidVanityPath() throws Exception {
-        Method method = MapEntries.class.getDeclaredMethod("isValidVanityPath", String.class);
+        Method method = MapEntries.VanityPathHandler.class.getDeclaredMethod("isValidVanityPath", String.class);
         method.setAccessible(true);
 
-        assertFalse((Boolean)method.invoke(mapEntries, "/jcr:system/node"));
+        assertFalse((Boolean)method.invoke(mapEntries.vph, "/jcr:system/node"));
 
-        assertTrue((Boolean)method.invoke(mapEntries, "/justVanityPath"));
+        assertTrue((Boolean)method.invoke(mapEntries.vph, "/justVanityPath"));
     }
 
     @Test
@@ -1133,9 +1122,9 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
                 });
 
         Method method =
-                MapEntries.class.getDeclaredMethod("getMapEntryList", String.class);
+                MapEntries.VanityPathHandler.class.getDeclaredMethod("getMapEntryList", String.class);
         method.setAccessible(true);
-        method.invoke(mapEntries, "/target/justVanityPath");
+        method.invoke(mapEntries.vph, "/target/justVanityPath");
 
         final int expected = 2;
 
@@ -1144,7 +1133,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest {
 
         assertEquals(expected, getVanityCounter(mapEntries).longValue());
 
-        method.invoke(mapEntries, "/target/justVanityPath");
+        method.invoke(mapEntries.vph, "/target/justVanityPath");
 
         entries = mapEntries.getResolveMaps();
         assertEquals(expected, entries.size());