This is an automated email from the ASF dual-hosted git repository.
rombert 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 272ea9a SLING-9040 only update intersecting ResourceProviderHandler
272ea9a is described below
commit 272ea9aa0a3bb4ce42e017c000e36e8f5bbec36f
Author: Dirk Rudolph <[email protected]>
AuthorDate: Thu Jan 30 16:18:15 2020 +0100
SLING-9040 only update intersecting ResourceProviderHandler
When a new ResourceProviderHandler gets activated or an existing one
deactivated currently all other registered ResourceProviderHandlers get
updated. To keep the exclude paths set for resource observation consistent
it is sufficient to only update those that are actually overlapping.
---
.../impl/providers/ResourceProviderTracker.java | 33 ++++++++++++++--
.../providers/ResourceProviderTrackerTest.java | 44 +++++++++++++++++++++-
2 files changed, 73 insertions(+), 4 deletions(-)
diff --git
a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
index f8f41b8..6726300 100644
---
a/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
+++
b/src/main/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTracker.java
@@ -19,6 +19,7 @@
package org.apache.sling.resourceresolver.impl.providers;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.Collections;
import java.util.Dictionary;
import java.util.HashMap;
@@ -31,6 +32,7 @@ import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import org.apache.sling.api.SlingConstants;
+import org.apache.sling.api.resource.ResourceUtil;
import org.apache.sling.api.resource.observation.ResourceChange;
import org.apache.sling.api.resource.observation.ResourceChange.ChangeType;
import org.apache.sling.api.resource.path.Path;
@@ -347,7 +349,7 @@ public class ResourceProviderTracker implements
ResourceProviderStorageProvider
*/
private boolean activate(final ResourceProviderHandler handler) {
synchronized (this.handlers) {
- updateHandlers();
+ updateHandlers(findShadowedHandlers(handler));
}
if ( !handler.activate() ) {
logger.warn("Activating resource provider {} failed",
handler.getInfo());
@@ -366,7 +368,7 @@ public class ResourceProviderTracker implements
ResourceProviderStorageProvider
private void deactivate(final ResourceProviderHandler handler) {
handler.deactivate();
synchronized (this.handlers) {
- updateHandlers();
+ updateHandlers(findShadowedHandlers(handler));
}
logger.debug("Deactivated resource provider {}", handler.getInfo());
}
@@ -480,8 +482,33 @@ public class ResourceProviderTracker implements
ResourceProviderStorageProvider
}
}
+ /**
+ * Returns a {@link Collection} of registered {@link
ResourceProviderHandler}s that are shadowed by the given {@link
ResourceProviderHandler}.
+ * This means that the path of each of the returned {@link
ResourceProviderHandler}s is a parent of the path of the given {@link
ResourceProviderHandler}
+ *
+ * @param handler
+ * @return
+ */
+ private Collection<List<ResourceProviderHandler>>
findShadowedHandlers(ResourceProviderHandler handler) {
+ Collection<List<ResourceProviderHandler>> shadowedHandlers = new
ArrayList<>(2);
+ String path = handler.getPath();
+ while(path != null) {
+ List<ResourceProviderHandler> list = handlers.get(path);
+ if (list != null && !list.isEmpty()) {
+ shadowedHandlers.add(list);
+ }
+ path = ResourceUtil.getParent(path);
+ }
+
+ return shadowedHandlers;
+ }
+
private void updateHandlers() {
- for (List<ResourceProviderHandler> list : handlers.values()) {
+ this.updateHandlers(this.handlers.values());
+ }
+
+ private void updateHandlers(Collection<List<ResourceProviderHandler>>
givenHandlers) {
+ for (List<ResourceProviderHandler> list : givenHandlers) {
if ( !list.isEmpty() ) {
final ResourceProviderHandler h = list.get(0);
if (h != null) {
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
b/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
index 21ead48..5d83705 100644
---
a/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/providers/ResourceProviderTrackerTest.java
@@ -26,12 +26,15 @@ import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
+import static org.mockito.Matchers.anyLong;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -291,17 +294,56 @@ public class ResourceProviderTrackerTest {
// register RP on /path, empty exclude set expected
info = fixture.registerResourceProvider(rp, "/path", AuthType.no);
assertNull(excludeSets.get("/"));
+ assertThat(excludeSets.get("/path"), hasSize(0));
// register RP on /, expect /path excluded
fixture.registerResourceProvider(rp, "/", AuthType.no);
assertThat(excludeSets.get("/"), hasSize(1));
assertThat(excludeSets.get("/"), contains("/path"));
+ assertThat(excludeSets.get("/path"), hasSize(0));
// unregister RP on /path, empty exclude set expected
fixture.unregisterResourceProvider(info);
assertThat(excludeSets.get("/"), hasSize(0));
+ assertThat(excludeSets.get("/path"), hasSize(0));
// register RP on /path again, expect /path excluded
fixture.registerResourceProvider(rp, "/path", AuthType.no);
assertThat(excludeSets.get("/"), hasSize(1));
assertThat(excludeSets.get("/"), contains("/path"));
+ assertThat(excludeSets.get("/path"), hasSize(0));
+ }
+
+ @Test
+ public void testUpdateOnlyOnIntersectingProviders() throws
InvalidSyntaxException {
+ final ResourceProviderTracker tracker = new ResourceProviderTracker();
+
+ tracker.activate(context.bundleContext(), eventAdmin, null);
+ tracker.setObservationReporterGenerator(new
SimpleObservationReporterGenerator(new DoNothingObservationReporter()));
+
+ ResourceProvider<?> rootRp = mock(ResourceProvider.class);
+ ResourceProvider<?> fooRp = mock(ResourceProvider.class);
+ ResourceProvider<?> barRp = mock(ResourceProvider.class);
+ ResourceProvider<?> foobarRp = mock(ResourceProvider.class);
+
+ ResourceProviderInfo info;
+
+ // register RPs and verify how often update() gets called
+ fixture.registerResourceProvider(rootRp, "/", AuthType.no);
+ verify(rootRp, never()).update(anyLong());
+ fixture.registerResourceProvider(fooRp, "/foo", AuthType.no);
+ verify(rootRp, times(1)).update(anyLong());
+ verify(fooRp, never()).update(anyLong());
+ info = fixture.registerResourceProvider(barRp, "/bar", AuthType.no);
+ verify(rootRp, times(2)).update(anyLong());
+ verify(fooRp, never()).update(anyLong());
+ verify(barRp, never()).update(anyLong());
+ fixture.unregisterResourceProvider(info);
+ verify(rootRp, times(3)).update(anyLong());
+ verify(fooRp, never()).update(anyLong());
+ verify(barRp, never()).update(anyLong());
+ fixture.registerResourceProvider(foobarRp, "/foo/bar", AuthType.no);
+ verify(rootRp, times(4)).update(anyLong());
+ verify(fooRp, times(1)).update(anyLong());
+ verify(barRp, never()).update(anyLong());
+ verify(foobarRp, never()).update(anyLong());
}
@Test
