You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2020/01/30 23:00:38 UTC
[sling-org-apache-sling-resourceresolver] branch master updated:
SLING-9040 only update intersecting ResourceProviderHandler
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 <di...@netcentric.biz>
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