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/08 14:34:07 UTC
[sling-org-apache-sling-resourceresolver] branch master updated:
SLING-8946 implement consistent shadowing of observation
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 4f48dee SLING-8946 implement consistent shadowing of observation
4f48dee is described below
commit 4f48deeddde060447cdcd00730263a9cb8e9ae6d
Author: Dirk Rudolph <di...@netcentric.biz>
AuthorDate: Fri Dec 20 19:16:20 2019 +0100
SLING-8946 implement consistent shadowing of observation
Make sure that the excludes PathSet used for shadowing observation of
overlapping ResourceProviders stays the same independenly from the order in
which they are registered as services.
---
.../impl/providers/ResourceProviderTracker.java | 27 +++++---
.../providers/ResourceProviderTrackerTest.java | 76 ++++++++++++++++++++--
2 files changed, 87 insertions(+), 16 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 98d60ff..f8f41b8 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
@@ -151,15 +151,7 @@ public class ResourceProviderTracker implements ResourceProviderStorageProvider
this.providerReporter = generator.createProviderReporter();
synchronized ( this.handlers ) {
this.reporterGenerator = generator;
- for (List<ResourceProviderHandler> list : handlers.values()) {
- if ( !list.isEmpty() ) {
- final ResourceProviderHandler h = list.get(0);
- if (h != null) {
- updateProviderContext(h);
- h.update();
- }
- }
- }
+ this.updateHandlers();
}
}
@@ -355,7 +347,7 @@ public class ResourceProviderTracker implements ResourceProviderStorageProvider
*/
private boolean activate(final ResourceProviderHandler handler) {
synchronized (this.handlers) {
- updateProviderContext(handler);
+ updateHandlers();
}
if ( !handler.activate() ) {
logger.warn("Activating resource provider {} failed", handler.getInfo());
@@ -373,6 +365,9 @@ public class ResourceProviderTracker implements ResourceProviderStorageProvider
*/
private void deactivate(final ResourceProviderHandler handler) {
handler.deactivate();
+ synchronized (this.handlers) {
+ updateHandlers();
+ }
logger.debug("Deactivated resource provider {}", handler.getInfo());
}
@@ -485,6 +480,18 @@ public class ResourceProviderTracker implements ResourceProviderStorageProvider
}
}
+ private void updateHandlers() {
+ for (List<ResourceProviderHandler> list : handlers.values()) {
+ if ( !list.isEmpty() ) {
+ final ResourceProviderHandler h = list.get(0);
+ if (h != null) {
+ updateProviderContext(h);
+ h.update();
+ }
+ }
+ }
+ }
+
private void updateProviderContext(final ResourceProviderHandler handler) {
final Set<String> excludedPaths = new HashSet<>();
final Path handlerPath = new Path(handler.getPath());
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 d0ed944..21ead48 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
@@ -19,15 +19,21 @@
package org.apache.sling.resourceresolver.impl.providers;
import static org.hamcrest.Matchers.arrayWithSize;
+import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
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.Mockito.mock;
+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;
import org.apache.sling.api.resource.observation.ResourceChange;
@@ -45,6 +51,8 @@ import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.InvalidSyntaxException;
import org.osgi.service.event.EventAdmin;
public class ResourceProviderTrackerTest {
@@ -55,7 +63,7 @@ public class ResourceProviderTrackerTest {
private EventAdmin eventAdmin;
private ResourceProviderInfo rp2Info;
private Fixture fixture;
-
+
@Before
public void prepare() throws Exception {
eventAdmin = context.getService(EventAdmin.class);
@@ -75,7 +83,7 @@ public class ResourceProviderTrackerTest {
fixture.registerResourceProvider(rp3, "invalid", AuthType.no);
ResourceProviderTracker tracker = new ResourceProviderTracker();
- tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new NoDothingObservationReporter()));
+ tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new DoNothingObservationReporter()));
tracker.activate(context.bundleContext(), eventAdmin, new DoNothingChangeListener());
return tracker;
}
@@ -105,7 +113,7 @@ public class ResourceProviderTrackerTest {
@Test
public void testActivationDeactivation() throws Exception {
final ResourceProviderTracker tracker = new ResourceProviderTracker();
- tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new NoDothingObservationReporter()));
+ tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new DoNothingObservationReporter()));
// create boolean markers for the listener
final AtomicBoolean addedCalled = new AtomicBoolean(false);
@@ -159,7 +167,7 @@ public class ResourceProviderTrackerTest {
@Test
public void testReactivation() throws Exception {
final ResourceProviderTracker tracker = new ResourceProviderTracker();
- tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new NoDothingObservationReporter()));
+ tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new DoNothingObservationReporter()));
// create boolean markers for the listener
final AtomicBoolean addedCalled = new AtomicBoolean(false);
@@ -240,6 +248,62 @@ public class ResourceProviderTrackerTest {
assertThat(tracker.getResourceProviderStorage().getAllHandlers().size(), equalTo(0));
}
+ /**
+ * This test verifies that shadowing of Resource observation is deterministic when ResourceProviders get registered and unregistered,
+ * meaning it is independent of the order in which those events happen.
+ * <p>
+ * It does so by
+ * 1) registering a ResourceProvider A on a deeper path then root (shadowing root)
+ * 2) registering a ResourceProvider B on root
+ * 3) unregistering the ResourceProvider A
+ * 4) and registering the ResoucreProvider A
+ * <p>
+ * This guarantees in both cases (A before B and B before A) the same excludes are applied in the ObservationReporter.
+ *
+ * @throws InvalidSyntaxException
+ */
+ @Test
+ public void testDeterministicObservationShadowing() throws InvalidSyntaxException {
+ final ResourceProviderTracker tracker = new ResourceProviderTracker();
+ final Map<String, List<String>> excludeSets = new HashMap<>();
+
+ tracker.activate(context.bundleContext(), eventAdmin, null);
+ tracker.setObservationReporterGenerator(new SimpleObservationReporterGenerator(new DoNothingObservationReporter()) {
+ @Override
+ public ObservationReporter create(Path path, PathSet excludes) {
+ List<String> excludeSetsPerPath = excludeSets.get(path.getPath());
+ if (excludeSetsPerPath == null) {
+ excludeSetsPerPath = new ArrayList<>(1);
+ excludeSets.put(path.getPath(), excludeSetsPerPath);
+ }
+
+ excludeSetsPerPath.clear();
+ for (Path exclude : excludes) {
+ excludeSetsPerPath.add(exclude.getPath());
+ }
+
+ return super.create(path, excludes);
+ }
+ });
+
+ ResourceProvider<?> rp = mock(ResourceProvider.class);
+ ResourceProviderInfo info;
+ // register RP on /path, empty exclude set expected
+ info = fixture.registerResourceProvider(rp, "/path", AuthType.no);
+ assertNull(excludeSets.get("/"));
+ // register RP on /, expect /path excluded
+ fixture.registerResourceProvider(rp, "/", AuthType.no);
+ assertThat(excludeSets.get("/"), hasSize(1));
+ assertThat(excludeSets.get("/"), contains("/path"));
+ // unregister RP on /path, empty exclude set expected
+ fixture.unregisterResourceProvider(info);
+ assertThat(excludeSets.get("/"), 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"));
+ }
+
@Test
public void fillDto() throws Exception {
ResourceProviderTracker tracker = registerDefaultResourceProviderTracker();
@@ -252,7 +316,7 @@ public class ResourceProviderTrackerTest {
assertThat( dto.failedProviders, arrayWithSize(1));
}
- static final class NoDothingObservationReporter implements ObservationReporter {
+ static class DoNothingObservationReporter implements ObservationReporter {
@Override
public void reportChanges(Iterable<ResourceChange> changes, boolean distribute) {
}
@@ -267,7 +331,7 @@ public class ResourceProviderTrackerTest {
}
}
- static final class SimpleObservationReporterGenerator implements ObservationReporterGenerator {
+ static class SimpleObservationReporterGenerator implements ObservationReporterGenerator {
private final ObservationReporter reporter;
SimpleObservationReporterGenerator(ObservationReporter reporter) {