This is an automated email from the ASF dual-hosted git repository. reschke pushed a commit to branch SLING-12701 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 5f508471504505b2e43b87796faf4d30fcc32775 Author: Julian Reschke <[email protected]> AuthorDate: Thu Mar 6 19:12:13 2025 +0100 SLING-12701: MapEntries - move alias handling into inner class - minimal changes --- .../resourceresolver/impl/mapping/MapEntries.java | 71 +++++++++++++--------- .../impl/mapping/AliasMapEntriesTest.java | 18 +++--- .../impl/mapping/EtcMappingMapEntriesTest.java | 12 ++-- .../impl/mapping/MapEntriesTest.java | 2 +- .../mapping/StringInterpolationMapEntriesTest.java | 4 +- .../impl/mapping/VanityPathMapEntriesTest.java | 2 +- 6 files changed, 62 insertions(+), 47 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 3be2e3ef..b4632b0a 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 @@ -136,6 +136,7 @@ public class MapEntries implements private final boolean useOptimizeAliasResolution; + final AliasHandler ah; final VanityPathHandler vph; public MapEntries(final MapConfigurationProvider factory, @@ -158,7 +159,9 @@ public class MapEntries implements this.detectedConflictingAliases = new AtomicLong(0); this.detectedInvalidAliases = new AtomicLong(0); - this.useOptimizeAliasResolution = initializeAliases(); + this.ah = new AliasHandler(); + + this.useOptimizeAliasResolution = ah.initializeAliases(); this.registration = registerResourceChangeListener(bundleContext); @@ -206,7 +209,7 @@ public class MapEntries implements if (resource != null) { boolean changed = vph.doAddVanity(resource); if (this.useOptimizeAliasResolution && resource.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { - changed |= doAddAlias(resource); + changed |= ah.doAddAlias(resource); } return changed; } @@ -240,7 +243,7 @@ public class MapEntries implements changed |= vph.doAddVanity(contentRsrc != null ? contentRsrc : resource); } if (this.useOptimizeAliasResolution) { - changed |= doUpdateAlias(resource); + changed |= ah.doUpdateAlias(resource); } return changed; @@ -268,7 +271,7 @@ public class MapEntries implements for (final String contentPath : this.aliasMapsMap.keySet()) { if (path.startsWith(contentPath + "/") || path.equals(contentPath) || contentPath.startsWith(pathPrefix)) { - changed |= removeAlias(contentPath, path, resolverRefreshed); + changed |= ah.removeAlias(contentPath, path, resolverRefreshed); } } } @@ -379,6 +382,17 @@ public class MapEntries implements return this.useOptimizeAliasResolution; } + @Override + public Map<String, List<String>> getVanityPathMappings() { + return vph.getVanityPathMappings(); + } + + @Override + public @NotNull Map<String, Collection<String>> getAliasMap(final String parentPath) { + Map<String, Collection<String>> aliasMapForParent = aliasMapsMap.get(parentPath); + return aliasMapForParent != null ? aliasMapForParent : Collections.emptyMap(); + } + /** * Refresh the resource resolver if not already done * @param resolverRefreshed Boolean flag containing the state if the resolver @@ -701,6 +715,11 @@ public class MapEntries implements } } + @Override + public void logDisableAliasOptimization() { + this.ah.logDisableAliasOptimization(null); + } + private MapEntry getMapEntry(final String url, final int status, final String... redirect) { return getMapEntry(url, status, 0, redirect); } @@ -746,6 +765,8 @@ public class MapEntries implements // Alias handling code + class AliasHandler { + /** * Actual initializer. Guards itself against concurrent use by using a * ReentrantLock. Does nothing if the resource resolver has already been @@ -754,24 +775,24 @@ public class MapEntries implements */ protected boolean initializeAliases() { - this.initializing.lock(); + MapEntries.this.initializing.lock(); try { - final ResourceResolver resolver = this.resolver; - final MapConfigurationProvider factory = this.factory; + final ResourceResolver resolver = MapEntries.this.resolver; + final MapConfigurationProvider factory = MapEntries.this.factory; if (resolver == null || factory == null) { - return this.factory.isOptimizeAliasResolutionEnabled(); + return MapEntries.this.factory.isOptimizeAliasResolutionEnabled(); } List<String> conflictingAliases = new ArrayList<>(); List<String> invalidAliases = new ArrayList<>(); - boolean isOptimizeAliasResolutionEnabled = this.factory.isOptimizeAliasResolutionEnabled(); + boolean isOptimizeAliasResolutionEnabled = MapEntries.this.factory.isOptimizeAliasResolutionEnabled(); //optimization made in SLING-2521 if (isOptimizeAliasResolutionEnabled) { try { final Map<String, Map<String, Collection<String>>> loadedMap = this.loadAliases(resolver, conflictingAliases, invalidAliases); - this.aliasMapsMap = loadedMap; + MapEntries.this.aliasMapsMap = loadedMap; // warn if there are more than a few defunct aliases if (conflictingAliases.size() >= MAX_REPORT_DEFUNCT_ALIASES) { @@ -801,13 +822,13 @@ public class MapEntries implements } finally { - this.initializing.unlock(); + MapEntries.this.initializing.unlock(); } } private boolean doAddAlias(final Resource resource) { - return loadAlias(resource, this.aliasMapsMap, null, null); + return loadAlias(resource, MapEntries.this.aliasMapsMap, null, null); } /** @@ -847,18 +868,18 @@ public class MapEntries implements return false; } - this.initializing.lock(); + MapEntries.this.initializing.lock(); try { final Map<String, Collection<String>> aliasMapEntry = aliasMapsMap.get(contentPath); if (aliasMapEntry != null) { - this.refreshResolverIfNecessary(resolverRefreshed); + MapEntries.this.refreshResolverIfNecessary(resolverRefreshed); String prefix = contentPath.endsWith("/") ? contentPath : contentPath + "/"; if (aliasMapEntry.entrySet().removeIf(e -> (prefix + e.getKey()).startsWith(resourcePath)) && (aliasMapEntry.isEmpty())) { - this.aliasMapsMap.remove(contentPath); + MapEntries.this.aliasMapsMap.remove(contentPath); } - Resource containingResource = this.resolver != null ? this.resolver.getResource(resourcePath) : null; + Resource containingResource = MapEntries.this.resolver != null ? MapEntries.this.resolver.getResource(resourcePath) : null; if (containingResource != null) { if (containingResource.getValueMap().containsKey(ResourceResolverImpl.PROP_ALIAS)) { @@ -872,7 +893,7 @@ public class MapEntries implements } return aliasMapEntry != null; } finally { - this.initializing.unlock(); + MapEntries.this.initializing.unlock(); } } @@ -894,7 +915,7 @@ public class MapEntries implements if (aliasMapEntry != null) { aliasMapEntry.remove(containingResourceName); if (aliasMapEntry.isEmpty()) { - this.aliasMapsMap.remove(parentPath); + MapEntries.this.aliasMapsMap.remove(parentPath); } } @@ -916,17 +937,11 @@ public class MapEntries implements return false; } - @Override public @NotNull Map<String, Collection<String>> getAliasMap(final String parentPath) { Map<String, Collection<String>> aliasMapForParent = aliasMapsMap.get(parentPath); return aliasMapForParent != null ? aliasMapForParent : Collections.emptyMap(); } - @Override - public Map<String, List<String>> getVanityPathMappings() { - return vph.getVanityPathMappings(); - } - /** * Load aliases - Search for all nodes (except under /jcr:system) below * configured alias locations having the sling:alias property @@ -973,7 +988,7 @@ public class MapEntries implements log.info("alias initialization - completed, processed {} resources with sling:alias properties in {}ms (~{} resource/s){}", count, TimeUnit.NANOSECONDS.toMillis(processElapsed), resourcePerSecond, diagnostics); - this.aliasResourcesOnStartup.set(count); + MapEntries.this.aliasResourcesOnStartup.set(count); return map; } @@ -982,7 +997,7 @@ public class MapEntries implements * generate alias query based on configured alias locations */ private String generateAliasQuery() { - final Set<String> allowedLocations = this.factory.getAllowedAliasLocations(); + final Set<String> allowedLocations = MapEntries.this.factory.getAllowedAliasLocations(); StringBuilder baseQuery = new StringBuilder("SELECT [sling:alias] FROM [nt:base] WHERE"); @@ -1096,7 +1111,7 @@ public class MapEntries implements /** * Check alias syntax */ - private static boolean isAliasInvalid(String alias) { + private boolean isAliasInvalid(String alias) { boolean invalid = alias.equals("..") || alias.equals(".") || alias.isEmpty(); if (!invalid) { for (final char c : alias.toCharArray()) { @@ -1124,7 +1139,6 @@ public class MapEntries implements private final long LOGGING_ERROR_PERIOD = 1000 * 60 * 5; - @Override public void logDisableAliasOptimization() { this.logDisableAliasOptimization(null); } @@ -1140,3 +1154,4 @@ public class MapEntries implements } } } +} diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java index 53e85c2b..218064cb 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/AliasMapEntriesTest.java @@ -147,9 +147,9 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { } private static void removeAlias(MapEntries mapEntries, String contentPath, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = MapEntries.class.getDeclaredMethod("removeAlias", String.class, String.class, AtomicBoolean.class); + Method method = MapEntries.AliasHandler.class.getDeclaredMethod("removeAlias", String.class, String.class, AtomicBoolean.class); method.setAccessible(true); - method.invoke(mapEntries, contentPath, path, bool); + method.invoke(mapEntries.ah, contentPath, path, bool); } private static void updateResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { @@ -160,7 +160,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { private void internal_test_simple_alias_support(boolean onJcrContent) { prepareMapEntriesForAlias(onJcrContent, false, "alias"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); assertTrue(aliasMap.containsKey("child")); @@ -179,7 +179,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { private void internal_test_simple_multi_alias_support(boolean onJcrContent) { prepareMapEntriesForAlias(onJcrContent, false, "foo", "bar"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); assertTrue(aliasMap.containsKey("child")); @@ -189,7 +189,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { @Test public void internal_test_simple_alias_support_throwing_unsupported_operation_exception_exception() { prepareMapEntriesForAlias(false, false, UnsupportedOperationException.class, "foo", "bar"); - assertFalse(mapEntries.initializeAliases()); + assertFalse(mapEntries.ah.initializeAliases()); } @Test @@ -206,7 +206,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { public void test_simple_multi_alias_support_with_null_parent() { // see SLING-12383 prepareMapEntriesForAlias(true, true, "foo", "bar"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); assertFalse(aliasMap.containsKey("child")); @@ -216,7 +216,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { public void test_simple_multi_alias_support_with_blank_and_invalid() { // invalid aliases filtered out prepareMapEntriesForAlias(false, false, "", "foo", ".", "bar", "x/y", "qux", " "); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); assertTrue(aliasMap.containsKey("child")); @@ -229,7 +229,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { List<String> invalidAliases = List.of(".", "..", "foo/bar", "# foo", ""); for (String invalidAlias : invalidAliases) { prepareMapEntriesForAlias(false, false, invalidAlias); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertEquals(Collections.emptyMap(), aliasMap); } @@ -297,7 +297,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { } }); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); Map<String, Collection<String>> aliasMap = mapEntries.getAliasMap("/parent"); assertNotNull(aliasMap); diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/EtcMappingMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/EtcMappingMapEntriesTest.java index 912649b9..7549bf5d 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/EtcMappingMapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/EtcMappingMapEntriesTest.java @@ -64,7 +64,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { public void root_node_to_content_mapping() throws Exception { setupEtcMapResource("localhost.8080", http,PROP_REDIRECT_EXTERNAL, "/content/simple-node"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping("^http/localhost.8080/", "/content/simple-node/"); expectedEtcMapping.assertEtcMap("Etc Mapping for simple node", mapEntries.getResolveMaps()); } @@ -76,7 +76,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { PROP_REDIRECT_EXTERNAL, "/content/simple-match/" ); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping("^http/localhost.8080/", "/content/simple-match/"); expectedEtcMapping.assertEtcMap("Etc Mapping for simple match", mapEntries.getResolveMaps()); } @@ -87,7 +87,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { public void internal_to_external_node_mapping() throws Exception { setupEtcMapResource("example.com.80", http,PROP_REDIRECT_EXTERNAL, "http://www.example.com/";); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping("^http/example.com.80/", "http://www.example.com/";); expectedEtcMapping.assertEtcMap("Etc Mapping for internal to external based on node", mapEntries.getResolveMaps()); } @@ -96,7 +96,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { public void internal_root_to_content_node_mapping() throws Exception { setupEtcMapResource("www.example.com.80", http,PROP_REDIRECT_INTERNAL, "/example"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping().addEtcMapEntry("^http/www.example.com.80/", true, "/example/"); expectedEtcMapping.assertEtcMap("Etc Mapping for internal root to content", mapEntries.getResolveMaps()); } @@ -108,7 +108,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { PROP_REDIRECT_EXTERNAL, "http://www.example.com/"; ); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping().addEtcMapEntry("^http/.+\\.example\\.com\\.80", false, "http://www.example.com/";); expectedEtcMapping.assertEtcMap("Etc Mapping for host redirect match mapping", mapEntries.getResolveMaps()); } @@ -123,7 +123,7 @@ public class EtcMappingMapEntriesTest extends AbstractMappingMapEntriesTest { setupEtcMapResource("gateway", localhost, PROP_REDIRECT_INTERNAL, "http://gbiv.com";); setupEtcMapResource("(stories)", localhost, PROP_REDIRECT_INTERNAL, "/anecdotes/$1"); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping() .addEtcMapEntry("^http/localhost\\.\\d*", true, "/content") .addEtcMapEntry("^http/localhost\\.\\d*/cgi-bin/", true, "/scripts/") 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 b72a2011..3e5058a0 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 @@ -205,6 +205,6 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest { return Collections.emptyIterator(); }); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); } } \ No newline at end of file diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java index eaf51752..1b9f13c5 100644 --- a/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java +++ b/src/test/java/org/apache/sling/resourceresolver/impl/mapping/StringInterpolationMapEntriesTest.java @@ -35,7 +35,7 @@ public class StringInterpolationMapEntriesTest extends AbstractMappingMapEntries Resource sivOne = setupEtcMapResource("$[config:siv.one]", http,PROP_REDIRECT_EXTERNAL, "/content/simple-node"); setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-node"}); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping("^http/test-simple-node/", "/content/simple-node/"); expectedEtcMapping.assertEtcMap("String Interpolation for simple match", mapEntries.getResolveMaps()); } @@ -49,7 +49,7 @@ public class StringInterpolationMapEntriesTest extends AbstractMappingMapEntries ); setupStringInterpolationProvider(stringInterpolationProvider, stringInterpolationProviderConfiguration, new String[] {"siv.one=test-simple-match"}); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); ExpectedEtcMapping expectedEtcMapping = new ExpectedEtcMapping("^http/test-simple-match/", "/content/simple-match/"); expectedEtcMapping.assertEtcMap("String Interpolation for simple match", mapEntries.getResolveMaps()); } 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 af6fb4ce..c319eaa6 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 @@ -1214,7 +1214,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { mapEntries = new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics); - mapEntries.initializeAliases(); + mapEntries.ah.initializeAliases(); } @Test
