Merge branch 'master' into feature/SLING-7768
diff --git a/bnd.bnd b/bnd.bnd
index 6e8ad2b..abe9d05 100644
--- a/bnd.bnd
+++ b/bnd.bnd
@@ -12,6 +12,10 @@
Require-Capability:\
osgi.service;filter:="(objectClass=org.osgi.service.event.EventHandler)";effective:=active;resolution:=optional
+-plugin:\
+ org.apache.sling.bnd.plugin.headers.parameters.remove.Plugin;\
+ 'Require-Capability'='osgi.service;filter:="(objectClass=org.osgi.service.event.EventHandler)";effective:=active;cardinality:=multiple'
+
-removeheaders:\
Include-Resource,\
Private-Package
diff --git a/pom.xml b/pom.xml
index cea7528..744001f 100644
--- a/pom.xml
+++ b/pom.xml
@@ -1,4 +1,4 @@
-<?xml version="1.0" encoding="ISO-8859-1"?>
+<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
@@ -18,16 +18,18 @@
under the License.
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
+
<modelVersion>4.0.0</modelVersion>
+
<parent>
<groupId>org.apache.sling</groupId>
- <artifactId>sling</artifactId>
- <version>34</version>
+ <artifactId>sling-bundle-parent</artifactId>
+ <version>35</version>
<relativePath />
</parent>
<artifactId>org.apache.sling.resourceresolver</artifactId>
- <version>1.6.13-SNAPSHOT</version>
+ <version>1.6.17-SNAPSHOT</version>
<name>Apache Sling Resource Resolver</name>
<description>
@@ -50,8 +52,7 @@
<plugins>
<plugin>
<groupId>org.apache.sling</groupId>
- <artifactId>maven-sling-plugin</artifactId>
- <version>2.3.8</version>
+ <artifactId>sling-maven-plugin</artifactId>
<executions>
<execution>
<id>generate-adapter-metadata</id>
@@ -91,6 +92,13 @@
<plugin>
<groupId>biz.aQute.bnd</groupId>
<artifactId>bnd-maven-plugin</artifactId>
+ <dependencies>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.bnd.plugin.headers.parameters.remove</artifactId>
+ <version>1.0.0</version>
+ </dependency>
+ </dependencies>
</plugin>
<plugin>
<groupId>biz.aQute.bnd</groupId>
@@ -124,13 +132,14 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.api</artifactId>
- <version>2.18.4</version>
+ <version>2.21.0</version>
<scope>provided</scope>
</dependency>
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.serviceusermapper</artifactId>
<version>1.3.4</version>
+ <scope>provided</scope>
</dependency>
<dependency>
<groupId>org.slf4j</groupId>
@@ -192,7 +201,7 @@
<dependency>
<groupId>org.apache.felix</groupId>
<artifactId>org.apache.felix.framework</artifactId>
- <version>5.4.0</version>
+ <version>6.0.3</version>
<scope>test</scope>
</dependency>
<dependency>
@@ -207,5 +216,11 @@
<version>2.3.10</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.osgi</groupId>
+ <artifactId>org.osgi.util.converter</artifactId>
+ <version>1.0.0</version>
+ <scope>test</scope>
+ </dependency>
</dependencies>
</project>
diff --git a/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControl.java b/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControl.java
index a5d591b..2eefd39 100644
--- a/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControl.java
+++ b/src/main/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControl.java
@@ -45,10 +45,12 @@
import org.apache.sling.api.resource.ValueMap;
import org.apache.sling.api.resource.path.PathBuilder;
import org.apache.sling.resourceresolver.impl.providers.ResourceProviderHandler;
+import org.apache.sling.resourceresolver.impl.providers.ResourceProviderInfo;
import org.apache.sling.resourceresolver.impl.providers.ResourceProviderStorage;
import org.apache.sling.resourceresolver.impl.providers.ResourceProviderStorageProvider;
import org.apache.sling.resourceresolver.impl.providers.stateful.AuthenticatedResourceProvider;
import org.apache.sling.resourceresolver.impl.providers.tree.Node;
+import org.apache.sling.resourceresolver.impl.providers.tree.PathTree;
import org.apache.sling.spi.resource.provider.ResourceProvider;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
@@ -499,71 +501,66 @@
return null;
}
- private AuthenticatedResourceProvider checkSourceAndDest(final ResourceResolverContext context,
+ private AuthenticatedResourceProvider checkExistence(final ResourceResolverContext context, final String path,
+ final String type) throws PersistenceException {
+ final Node<ResourceProviderHandler> node = getResourceProviderStorage().getTree().getBestMatchingNode(path);
+ if (node == null) {
+ throw new PersistenceException(type.concat(" resource does not exist."), null, path, null);
+ }
+ AuthenticatedResourceProvider provider = null;
+ try {
+ provider = context.getProviderManager().getOrCreateProvider(node.getValue(), this);
+ } catch (LoginException e) {
+ // ignore
+ }
+ if (provider == null) {
+ throw new PersistenceException(type.concat(" resource does not exist."), null, path, null);
+ }
+ final Resource rsrc = provider.getResource(path, null, null);
+ if (rsrc == null) {
+ throw new PersistenceException(type.concat(" resource does not exist."), null, path, null);
+ }
+ return provider;
+ }
+
+ public AuthenticatedResourceProvider checkSourceAndDest(final ResourceResolverContext context,
final String srcAbsPath, final String destAbsPath) throws PersistenceException {
// check source
- final Node<ResourceProviderHandler> srcNode = getResourceProviderStorage().getTree().getBestMatchingNode(srcAbsPath);
- if ( srcNode == null ) {
- throw new PersistenceException("Source resource does not exist.", null, srcAbsPath, null);
- }
- AuthenticatedResourceProvider srcProvider = null;
- try {
- srcProvider = context.getProviderManager().getOrCreateProvider(srcNode.getValue(), this);
- } catch (LoginException e) {
- // ignore
- }
- if ( srcProvider == null ) {
- throw new PersistenceException("Source resource does not exist.", null, srcAbsPath, null);
- }
- final Resource srcResource = srcProvider.getResource(srcAbsPath, null, null);
- if ( srcResource == null ) {
- throw new PersistenceException("Source resource does not exist.", null, srcAbsPath, null);
- }
-
+ final AuthenticatedResourceProvider srcProvider = checkExistence(context, srcAbsPath, "Source");
// check destination
- final Node<ResourceProviderHandler> destNode = getResourceProviderStorage().getTree().getBestMatchingNode(destAbsPath);
- if ( destNode == null ) {
- throw new PersistenceException("Destination resource does not exist.", null, destAbsPath, null);
- }
- AuthenticatedResourceProvider destProvider = null;
- try {
- destProvider = context.getProviderManager().getOrCreateProvider(destNode.getValue(), this);
- } catch (LoginException e) {
- // ignore
- }
- if ( destProvider == null ) {
- throw new PersistenceException("Destination resource does not exist.", null, destAbsPath, null);
- }
- final Resource destResource = destProvider.getResource(destAbsPath, null, null);
- if ( destResource == null ) {
- throw new PersistenceException("Destination resource does not exist.", null, destAbsPath, null);
- }
+ final AuthenticatedResourceProvider destProvider = checkExistence(context, destAbsPath, "Destination");
// check for sub providers of src and dest
- if ( srcProvider == destProvider && !collectProviders(context, srcNode) && !collectProviders(context, destNode) ) {
- return srcProvider;
+ if (srcProvider == destProvider) {
+ final PathTree<ResourceProviderHandler> providerTree = getResourceProviderStorage().getTree();
+ final boolean sourceHasProvider = hasSubProvider(context, providerTree.getNode(srcAbsPath));
+ final boolean destHasProvider = hasSubProvider(context, providerTree.getNode(destAbsPath));
+ if (!(sourceHasProvider || destHasProvider)) {
+ return srcProvider;
+ }
}
return null;
}
- private boolean collectProviders(final ResourceResolverContext context,
- final Node<ResourceProviderHandler> parent) {
- boolean hasMoreProviders = false;
- for (final Entry<String, Node<ResourceProviderHandler>> entry : parent.getChildren().entrySet()) {
- if ( entry.getValue().getValue() != null ) {
- try {
- context.getProviderManager().getOrCreateProvider(entry.getValue().getValue(), this);
- hasMoreProviders = true;
- } catch ( final LoginException ignore) {
- // ignore
+ private boolean hasSubProvider(ResourceResolverContext context, Node<ResourceProviderHandler> node) {
+ if (node != null) {
+ for (final Node<ResourceProviderHandler> childNode : node.getChildren().values()) {
+ final ResourceProviderHandler handler = childNode.getValue();
+ if (handler != null) {
+ try {
+ context.getProviderManager().getOrCreateProvider(handler, this);
+ return true;
+ } catch (final LoginException ignore) {
+ // ignore
+ }
}
- }
- if ( collectProviders(context, entry.getValue())) {
- hasMoreProviders = true;
+ if (hasSubProvider(context, childNode)) {
+ return true;
+ }
}
}
- return hasMoreProviders;
+ return false;
}
private void copy(final ResourceResolverContext context, final Resource src, final String dstPath, final List<Resource> newNodes) throws PersistenceException {
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 @@
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 @@
*/
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 @@
*/
private void deactivate(final ResourceProviderHandler handler) {
handler.deactivate();
+ synchronized (this.handlers) {
+ updateHandlers();
+ }
logger.debug("Deactivated resource provider {}", handler.getInfo());
}
@@ -485,6 +480,18 @@
}
}
+ 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/SimpleValueMapImpl.java b/src/test/java/org/apache/sling/resourceresolver/impl/SimpleValueMapImpl.java
index ef71aa7..9b4f1ac 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/SimpleValueMapImpl.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/SimpleValueMapImpl.java
@@ -23,6 +23,7 @@
import java.util.Set;
import org.apache.sling.api.resource.ValueMap;
+import org.jetbrains.annotations.NotNull;
public class SimpleValueMapImpl implements ValueMap {
@@ -81,7 +82,7 @@
}
@SuppressWarnings("unchecked")
- public <T> T get(String name, Class<T> type) {
+ public <T> T get(@NotNull String name, @NotNull Class<T> type) {
Object o = delegate.get(name);
if ( type.equals(String[].class) && ! ( o instanceof String[])) {
// According to ValueMap if the value cannot be converted it should return null
@@ -93,8 +94,9 @@
return (T) o;
}
+ @NotNull
@SuppressWarnings("unchecked")
- public <T> T get(String name, T defaultValue) {
+ public <T> T get(@NotNull String name, @NotNull T defaultValue) {
if ( delegate.containsKey(name)) {
return (T) delegate.get(name);
}
diff --git a/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControlTest.java b/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControlTest.java
index 45c3940..93957a2 100644
--- a/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControlTest.java
+++ b/src/test/java/org/apache/sling/resourceresolver/impl/helper/ResourceResolverControlTest.java
@@ -340,6 +340,23 @@
}
/**
+ * Checks that its correctly calculated whether a copy/move can be done with the
+ * same provider
+ *
+ * @throws PersistenceException
+ */
+ @Test
+ public void checkProvidersForCopyMove() throws PersistenceException {
+ // first check same provider at root
+ configureResourceAt(rootProvider, "/a");
+ configureResourceAt(rootProvider, "/b");
+ assertNotNull(crp.checkSourceAndDest(context, "/a", "/b"));
+
+ // second check different providers
+ assertNull(crp.checkSourceAndDest(context, "/some/path/object", "/"));
+ }
+
+ /**
* Verifies copying resources between the same ResourceProvider
*
* @throws PersistenceException persistence exception
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.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 @@
private EventAdmin eventAdmin;
private ResourceProviderInfo rp2Info;
private Fixture fixture;
-
+
@Before
public void prepare() throws Exception {
eventAdmin = context.getService(EventAdmin.class);
@@ -75,7 +83,7 @@
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 @@
@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 @@
@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 @@
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 @@
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 @@
}
}
- static final class SimpleObservationReporterGenerator implements ObservationReporterGenerator {
+ static class SimpleObservationReporterGenerator implements ObservationReporterGenerator {
private final ObservationReporter reporter;
SimpleObservationReporterGenerator(ObservationReporter reporter) {
