This is an automated email from the ASF dual-hosted git repository. reschke pushed a commit to branch SLING-12894 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 69fdf154e020d5da17e5312fe1f06a344e72b34d Author: Julian Reschke <[email protected]> AuthorDate: Wed Aug 20 14:07:00 2025 +0100 SLING-12894: alias refactoring - support observation events while bg init not finished - wip --- .../resourceresolver/impl/mapping/MapEntries.java | 105 ++++++++++++--------- .../impl/mapping/AliasMapEntriesTest.java | 17 ++-- .../impl/mapping/MapEntriesTest.java | 5 +- .../impl/mapping/VanityPathMapEntriesTest.java | 13 +-- 4 files changed, 80 insertions(+), 60 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 c8e47673..7356058c 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 @@ -35,6 +35,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; import java.util.Optional; +import java.util.Set; import java.util.SortedMap; import java.util.TreeMap; import java.util.TreeSet; @@ -101,7 +102,8 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex private final Map<String, List<MapEntry>> resolveMapsMap; - private List<Map.Entry<String, ResourceChange.ChangeType>> resourceChangeQueue; + private List<Map.Entry<String, ResourceChange.ChangeType>> resourceChangeQueueForAliases; + private List<Map.Entry<String, ResourceChange.ChangeType>> resourceChangeQueueForVanityPaths; private Collection<MapEntry> mapMaps; @@ -133,7 +135,8 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex this.registration = registerResourceChangeListener(bundleContext); - this.vph = new VanityPathHandler(this.factory, this.resolveMapsMap, this.initializing, this::drainQueue); + this.vph = + new VanityPathHandler(this.factory, this.resolveMapsMap, this.initializing, this::drainVanityPathQueue); this.vph.initializeVanityPaths(); if (metrics.isPresent()) { @@ -164,19 +167,22 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex props.put(Constants.SERVICE_VENDOR, "The Apache Software Foundation"); log.info("Registering for {}", Arrays.toString(factory.getObservationPaths())); - this.resourceChangeQueue = Collections.synchronizedList(new LinkedList<>()); + this.resourceChangeQueueForAliases = Collections.synchronizedList(new LinkedList<>()); + this.resourceChangeQueueForVanityPaths = Collections.synchronizedList(new LinkedList<>()); + return bundleContext.registerService(ResourceChangeListener.class, this, props); } - private boolean addResource(final String path, final AtomicBoolean resolverRefreshed) { + private boolean addResource(String path, boolean forAlias, boolean forVanityPath, AtomicBoolean resolverRefreshed) { this.initializing.lock(); try { this.refreshResolverIfNecessary(resolverRefreshed); - final Resource resource = this.resolver != null ? resolver.getResource(path) : null; + + Resource resource = this.resolver != null ? resolver.getResource(path) : null; if (resource != null) { - boolean vanityPathAdded = vph.doAddVanity(resource); - boolean aliasAdded = ah.doAddAlias(resource); + boolean vanityPathAdded = forVanityPath && vph.doAddVanity(resource); + boolean aliasAdded = forAlias && ah.doAddAlias(resource); return vanityPathAdded || aliasAdded; } else { return false; @@ -186,22 +192,23 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex } } - private boolean updateResource(final String path, final AtomicBoolean resolverRefreshed) { + private boolean updateResource( + String path, boolean forAlias, boolean forVanityPath, AtomicBoolean resolverRefreshed) { this.initializing.lock(); try { this.refreshResolverIfNecessary(resolverRefreshed); - final Resource resource = this.resolver != null ? resolver.getResource(path) : null; + Resource resource = this.resolver != null ? resolver.getResource(path) : null; - final boolean isValidVanityPath = vph.isValidVanityPath(path); + boolean isValidVanityPath = vph.isValidVanityPath(path); if (resource != null) { boolean vanityPathChanged = false; - if (isValidVanityPath) { + if (forVanityPath && isValidVanityPath) { // we remove the old vanity path first vanityPathChanged |= vph.doRemoveVanity(path); @@ -214,7 +221,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex vanityPathChanged |= vph.doAddVanity(contentRsrc != null ? contentRsrc : resource); } - boolean aliasChanged = ah.doUpdateAlias(resource); + boolean aliasChanged = forAlias && ah.doUpdateAlias(resource); return vanityPathChanged || aliasChanged; } } finally { @@ -224,24 +231,32 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex return false; } - private boolean removeResource(final String path, final AtomicBoolean resolverRefreshed) { - final String actualContentPath = getActualContentPath(path); - final String actualContentPathPrefix = actualContentPath + "/"; + private boolean removeResource( + String path, boolean forAlias, boolean forVanityPath, AtomicBoolean resolverRefreshed) { boolean vanityPathChanged = false; boolean aliasChanged = false; - for (final String target : vph.getVanityPathMappings().keySet()) { - if (target.startsWith(actualContentPathPrefix) || target.equals(actualContentPath)) { - vanityPathChanged |= vph.removeVanityPath(target); + if (forVanityPath) { + String actualContentPath = getActualContentPath(path); + String actualContentPathPrefix = actualContentPath + "/"; + + for (String target : vph.getVanityPathMappings().keySet()) { + if (target.startsWith(actualContentPathPrefix) || target.equals(actualContentPath)) { + vanityPathChanged |= vph.removeVanityPath(target); + } } } - final String pathPrefix = path + "/"; - for (final String contentPath : ah.aliasMapsMap.keySet()) { - if (path.startsWith(contentPath + "/") || path.equals(contentPath) || contentPath.startsWith(pathPrefix)) { - aliasChanged |= ah.removeAlias( - resolver, contentPath, path, () -> this.refreshResolverIfNecessary(resolverRefreshed)); + if (forAlias) { + String pathPrefix = path + "/"; + for (String contentPath : ah.aliasMapsMap.keySet()) { + if (path.startsWith(contentPath + "/") + || path.equals(contentPath) + || contentPath.startsWith(pathPrefix)) { + aliasChanged |= ah.removeAlias( + resolver, contentPath, path, () -> this.refreshResolverIfNecessary(resolverRefreshed)); + } } } @@ -426,8 +441,11 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex // ---------- ResourceChangeListener interface + private final Set<ResourceChange.ChangeType> RELEVANT_CHANGE_TYPES = Set.of( + ResourceChange.ChangeType.ADDED, ResourceChange.ChangeType.CHANGED, ResourceChange.ChangeType.REMOVED); + /** - * Handles the change to any of the node properties relevant for vanity URL + * Handles the change to any of the node properties relevant for vanity paths or aliases * mappings. The {@link #MapEntries(MapConfigurationProvider, BundleContext, EventAdmin, StringInterpolationProvider, Optional)} * constructor makes sure the event listener is registered to only get * appropriate events. @@ -435,7 +453,8 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex @Override public void onChange(final List<ResourceChange> changes) { - final boolean inStartup = !vph.isReady(); + final boolean ahInStartup = !ah.isReady(); + final boolean vphInStartup = !vph.isReady(); final AtomicBoolean resolverRefreshed = new AtomicBoolean(false); @@ -450,7 +469,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex final ResourceChange.ChangeType type = rc.getType(); final String path = rc.getPath(); - log.debug("onChange, type={}, path={}", rc.getType(), path); + log.debug("onChange, type={}, path={}", type, path); // don't care for system area if (path.startsWith(JCR_SYSTEM_PREFIX)) { @@ -458,20 +477,14 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex } // during startup: just enqueue the events - if (inStartup) { - if (type == ResourceChange.ChangeType.REMOVED - || type == ResourceChange.ChangeType.ADDED - || type == ResourceChange.ChangeType.CHANGED) { + if (vphInStartup) { + if (RELEVANT_CHANGE_TYPES.contains(type)) { Map.Entry<String, ResourceChange.ChangeType> entry = new SimpleEntry<>(path, type); - log.trace("enqueue: {}", entry); - resourceChangeQueue.add(entry); + log.trace("enqueued for vanity paths {}", entry); + resourceChangeQueueForVanityPaths.add(entry); } } else { - boolean changed = handleResourceChange(type, path, resolverRefreshed, hasReloadedConfig); - - if (changed) { - sendEvent = true; - } + sendEvent |= handleResourceChange(type, path, true, true, resolverRefreshed, hasReloadedConfig); } } @@ -483,6 +496,8 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex private boolean handleResourceChange( ResourceChange.ChangeType type, String path, + boolean forAlias, + boolean forVanityPath, AtomicBoolean resolverRefreshed, AtomicBoolean hasReloadedConfig) { boolean changed = false; @@ -494,7 +509,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex if (result) { changed = true; } else { - changed |= removeResource(path, resolverRefreshed); + changed |= removeResource(path, forAlias, forVanityPath, resolverRefreshed); } } // session.move() is handled differently see also SLING-3713 and @@ -504,7 +519,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex if (result) { changed = true; } else { - changed |= addResource(path, resolverRefreshed); + changed |= addResource(path, forAlias, forVanityPath, resolverRefreshed); } } } else if (type == ResourceChange.ChangeType.CHANGED) { @@ -513,7 +528,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex if (result) { changed = true; } else { - changed |= updateResource(path, resolverRefreshed); + changed |= updateResource(path, forAlias, forVanityPath, resolverRefreshed); } } } @@ -717,7 +732,7 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex } } - private void drainQueue() { + private void drainVanityPathQueue() { final AtomicBoolean resolverRefreshed = new AtomicBoolean(false); // send the change event only once @@ -726,13 +741,13 @@ public class MapEntries implements MapEntriesHandler, ResourceChangeListener, Ex // the config needs to be reloaded only once final AtomicBoolean hasReloadedConfig = new AtomicBoolean(false); - while (!resourceChangeQueue.isEmpty()) { - Map.Entry<String, ResourceChange.ChangeType> entry = resourceChangeQueue.remove(0); + while (!resourceChangeQueueForVanityPaths.isEmpty()) { + Map.Entry<String, ResourceChange.ChangeType> entry = resourceChangeQueueForVanityPaths.remove(0); final ResourceChange.ChangeType type = entry.getValue(); final String path = entry.getKey(); - log.trace("drain type={}, path={}", type, path); - boolean changed = handleResourceChange(type, path, resolverRefreshed, hasReloadedConfig); + log.trace("drain vanity path queue - type={}, path={}", type, path); + boolean changed = handleResourceChange(type, path, false, true, resolverRefreshed, hasReloadedConfig); if (changed) { sendEvent = true; 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 b9ed9319..3090df85 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 @@ -178,16 +178,18 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { private static void addResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = MapEntries.class.getDeclaredMethod("addResource", String.class, AtomicBoolean.class); + Method method = MapEntries.class.getDeclaredMethod( + "addResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); method.setAccessible(true); - method.invoke(mapEntries, path, bool); + method.invoke(mapEntries, path, true, false, bool); } private static void removeResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = MapEntries.class.getDeclaredMethod("removeResource", String.class, AtomicBoolean.class); + Method method = MapEntries.class.getDeclaredMethod( + "removeResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); method.setAccessible(true); - method.invoke(mapEntries, path, bool); + method.invoke(mapEntries, path, true, false, bool); } private static void removeAlias( @@ -205,9 +207,10 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { private static void updateResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = MapEntries.class.getDeclaredMethod("updateResource", String.class, AtomicBoolean.class); + Method method = MapEntries.class.getDeclaredMethod( + "updateResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); method.setAccessible(true); - method.invoke(mapEntries, path, bool); + method.invoke(mapEntries, path, true, false, bool); } private void internal_test_simple_alias_support(boolean onJcrContent, boolean cached) { @@ -1272,7 +1275,7 @@ public class AliasMapEntriesTest extends AbstractMappingMapEntriesTest { Map<String, Collection<String>> aliasMapEntry = mapEntries.getAliasMap(top); assertTrue( "Alias Map for " + top.getPath() - + " should be empty due to removal event during background init, bug got: " + aliasMapEntry, + + " should be empty due to removal event during background init, but got: " + aliasMapEntry, aliasMapEntry.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 d5d37ed6..dbe7acf5 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 @@ -135,10 +135,11 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest { // tests SLING-6542 @Test public void sessionConcurrency() throws Exception { - final Method addResource = MapEntries.class.getDeclaredMethod("addResource", String.class, AtomicBoolean.class); + final Method addResource = MapEntries.class.getDeclaredMethod( + "addResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); addResource.setAccessible(true); final Method updateResource = - MapEntries.class.getDeclaredMethod("updateResource", String.class, AtomicBoolean.class); + MapEntries.class.getDeclaredMethod("updateResource", String.class, true, true, tomicBoolean.class); updateResource.setAccessible(true); final Method handleConfigurationUpdate = MapEntries.class.getDeclaredMethod( "handleConfigurationUpdate", String.class, AtomicBoolean.class, AtomicBoolean.class, boolean.class); 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 fa03f4ce..102a41ee 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 @@ -218,9 +218,10 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { private static void addResource(MapEntries mapEntries, String path, AtomicBoolean bool) throws IllegalAccessException, NoSuchMethodException, InvocationTargetException { - Method method = MapEntries.class.getDeclaredMethod("addResource", String.class, AtomicBoolean.class); + Method method = MapEntries.class.getDeclaredMethod( + "addResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); method.setAccessible(true); - method.invoke(mapEntries, path, bool); + method.invoke(mapEntries, path, false, true, bool); } private static void loadVanityPaths(MapEntries mapEntries, ResourceResolver resourceResolver) @@ -742,8 +743,8 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { Map<String, List<String>> vanityTargets = getVanityTargets(mapEntries); assertEquals(0, vanityTargets.size()); - final Method updateResource = - MapEntries.class.getDeclaredMethod("updateResource", String.class, AtomicBoolean.class); + final Method updateResource = MapEntries.class.getDeclaredMethod( + "updateResource", String.class, boolean.class, boolean.class, AtomicBoolean.class); updateResource.setAccessible(true); Resource justVanityPath = mock(Resource.class, "justVanityPath"); @@ -765,7 +766,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { // update vanity path when(justVanityPath.getValueMap()) .thenReturn(buildValueMap("sling:vanityPath", "/target/justVanityPathUpdated")); - updateResource.invoke(mapEntries, "/justVanityPath", new AtomicBoolean()); + updateResource.invoke(mapEntries, "/justVanityPath", false, true, new AtomicBoolean()); assertEquals(2, resolveMapsMap.size()); assertEquals(1, vanityTargets.size()); @@ -805,7 +806,7 @@ public class VanityPathMapEntriesTest extends AbstractMappingMapEntriesTest { // update vanity path when(vanityPathOnJcrContent.getValueMap()) .thenReturn(buildValueMap("sling:vanityPath", "/target/vanityPathOnJcrContentUpdated")); - updateResource.invoke(mapEntries, "/vanityPathOnJcrContent/jcr:content", new AtomicBoolean()); + updateResource.invoke(mapEntries, "/vanityPathOnJcrContent/jcr:content", false, true, new AtomicBoolean()); assertEquals(3, resolveMapsMap.size()); assertEquals(2, vanityTargets.size());
