This is an automated email from the ASF dual-hosted git repository. joerghoh pushed a commit to branch improvement/SLING-10968-reduce-events in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-resourceresolver.git
commit 7979beb631c37e2df6642ea1efab16939a6b4a13 Author: Joerg Hoh <[email protected]> AuthorDate: Thu Dec 9 09:25:29 2021 +0100 SLING-10958 send only 1 event even if multiple vanity path updates happen --- .../resourceresolver/impl/mapping/MapEntries.java | 9 +++- .../impl/mapping/MapEntriesTest.java | 63 +++++++++++++++++++++- 2 files changed, 69 insertions(+), 3 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 01c4df3..1c75f67 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 @@ -718,6 +718,9 @@ public class MapEntries implements public void onChange(final List<ResourceChange> changes) { final AtomicBoolean resolverRefreshed = new AtomicBoolean(false); + // send the change event only once + boolean sendEvent = false; + // the config needs to be reloaded only once final AtomicBoolean hasReloadedConfig = new AtomicBoolean(false); for(final ResourceChange rc : changes) { @@ -765,13 +768,15 @@ public class MapEntries implements changed |= updateResource(path, resolverRefreshed); } } - } if ( changed ) { - this.sendChangeEvent(); + sendEvent = true; } } + if (sendEvent) { + this.sendChangeEvent(); + } } // ---------- internal 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 ceda30a..9b97ea6 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 @@ -54,6 +54,7 @@ import org.junit.Test; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.MockitoAnnotations; +import org.mockito.internal.util.reflection.FieldSetter; import org.mockito.invocation.InvocationOnMock; import org.mockito.stubbing.Answer; import org.osgi.framework.Bundle; @@ -128,7 +129,7 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest { Optional<ResourceResolverMetrics> metrics = Optional.empty(); - mapEntries = new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics); + mapEntries = Mockito.spy(new MapEntries(resourceResolverFactory, bundleContext, eventAdmin, stringInterpolationProvider, metrics)); final Field aliasMapField = MapEntries.class.getDeclaredField("aliasMap"); aliasMapField.setAccessible(true); @@ -369,6 +370,66 @@ public class MapEntriesTest extends AbstractMappingMapEntriesTest { mapEntries.onChange(Arrays.asList(new ResourceChange(ChangeType.REMOVED, parent.getPath(), false))); assertTrue( mapEntries.getResolveMaps().isEmpty()); } + + @Test + public void test_vanity_path_updates_do_not_reload_multiple_times() throws IOException { + Resource parent = mock(Resource.class, "parent"); + when(parent.getPath()).thenReturn("/foo/parent"); + when(parent.getName()).thenReturn("parent"); + when(parent.getValueMap()).thenReturn(buildValueMap("sling:vanityPath", "/target/found1")); + when(resourceResolver.getResource(parent.getPath())).thenReturn(parent); + + Resource child = mock(Resource.class, "jcrcontent"); + when(child.getPath()).thenReturn("/foo/parent/jcr:content"); + when(child.getName()).thenReturn("jcr:content"); + when(child.getValueMap()).thenReturn(buildValueMap("sling:vanityPath", "/target/found2")); + when(child.getParent()).thenReturn(parent); + when(parent.getChild(child.getName())).thenReturn(child); + when(resourceResolver.getResource(child.getPath())).thenReturn(child); + + Resource child2 = mock(Resource.class, "child2"); + when(child2.getPath()).thenReturn("/foo/parent/child2"); + when(child2.getName()).thenReturn("child2"); + when(child2.getValueMap()).thenReturn(buildValueMap("sling:vanityPath", "/target/found3")); + when(child2.getParent()).thenReturn(parent); + when(parent.getChild(child2.getName())).thenReturn(child2); + when(resourceResolver.getResource(child2.getPath())).thenReturn(child2); + + when(resourceResolver.findResources(anyString(), eq("sql"))).thenAnswer(new Answer<Iterator<Resource>>() { + + @Override + public Iterator<Resource> answer(InvocationOnMock invocation) throws Throwable { + return Collections.<Resource> emptySet().iterator(); + } + }); + + mapEntries.doInit(); + mapEntries.initializeVanityPaths(); + + // map entries should have no alias atm + assertTrue( mapEntries.getResolveMaps().isEmpty()); + // till now we already have 2 events being sent + Mockito.verify(eventAdmin,Mockito.times(2)).postEvent(Mockito.anyObject()); + + // 2 updates at the same onChange call + mapEntries.onChange(Arrays.asList( + new ResourceChange(ChangeType.ADDED, parent.getPath(), false), + new ResourceChange(ChangeType.ADDED, child.getPath(), false), + new ResourceChange(ChangeType.ADDED, child2.getPath(), false) + )); + + // two entries for the vanity path + List<MapEntry> entries = mapEntries.getResolveMaps(); + assertEquals(6, entries.size()); + + assertTrue(entries.stream().anyMatch(e -> e.getPattern().contains("/target/found1"))); + assertTrue(entries.stream().anyMatch(e -> e.getPattern().contains("/target/found2"))); + assertTrue(entries.stream().anyMatch(e -> e.getPattern().contains("/target/found3"))); + + // a single event is sent for all 3 added vanity paths + Mockito.verify(eventAdmin,Mockito.times(3)).postEvent(Mockito.anyObject()); + + } @Test public void test_vanity_path_registration_include_exclude() throws IOException {
