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 <[email protected]>
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) {
