This is an automated email from the ASF dual-hosted git repository.
jsedding 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 170d3a7 SLING-12019 - Avoid duplicate ResourceResolverFactory
registrations (#104)
170d3a7 is described below
commit 170d3a7b415a3c147b2381c8919fd9dfa7eb427c
Author: Julian Sedding <jsedd...@apache.org>
AuthorDate: Mon Sep 18 13:04:50 2023 +0200
SLING-12019 - Avoid duplicate ResourceResolverFactory registrations (#104)
- reworked synchronization model from relying on worker queue
to locks for "configuration" and an AtomicReference for the
factoryRegistration
- removed unnecessary null checks for factoryRegistration, simplifying the
code
- increased test coverage, particularly for concurrent scenarios
- switched to junit-platform to allow mixing junit4 and junit5 tests
- enabled parallel test execution for junit-platform
---
pom.xml | 36 +++-
.../impl/FactoryRegistrationHandler.java | 127 ++++++++----
.../impl/ResourceResolverFactoryActivator.java | 16 +-
.../impl/FactoryRegistrationHandlerTest.java | 230 ++++++++++++---------
.../resourceresolver/util/CustomMatchers.java | 123 +++++++++++
.../util/events/AbstractAwaitingListener.java | 75 +++++++
.../util/events/RecordingListener.java | 99 +++++++++
.../util/events/ServiceEventUtil.java | 105 ++++++++++
src/test/resources/junit-platform.properties | 19 ++
9 files changed, 683 insertions(+), 147 deletions(-)
diff --git a/pom.xml b/pom.xml
index 75d9d34..302d3de 100644
--- a/pom.xml
+++ b/pom.xml
@@ -188,20 +188,32 @@
<!-- Testing -->
<dependency>
- <groupId>org.hamcrest</groupId>
- <artifactId>hamcrest-library</artifactId>
- <version>1.3</version>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-api</artifactId>
+ <version>5.9.2</version>
<scope>test</scope>
</dependency>
<dependency>
- <groupId>org.slf4j</groupId>
- <artifactId>slf4j-simple</artifactId>
+ <groupId>org.junit.jupiter</groupId>
+ <artifactId>junit-jupiter-engine</artifactId>
+ <version>5.9.2</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.junit.vintage</groupId>
+ <artifactId>junit-vintage-engine</artifactId>
+ <version>5.9.2</version>
+ <scope>test</scope>
+ </dependency>
+ <dependency>
+ <groupId>org.hamcrest</groupId>
+ <artifactId>hamcrest</artifactId>
+ <version>2.2</version>
<scope>test</scope>
</dependency>
<dependency>
- <groupId>junit-addons</groupId>
- <artifactId>junit-addons</artifactId>
- <version>1.4</version>
+ <groupId>org.slf4j</groupId>
+ <artifactId>slf4j-simple</artifactId>
<scope>test</scope>
</dependency>
<dependency>
@@ -216,10 +228,16 @@
<version>5.5.0</version>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.sling</groupId>
+ <artifactId>org.apache.sling.testing.osgi-mock.junit5</artifactId>
+ <version>3.3.8</version>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.testing.osgi-mock.junit4</artifactId>
- <version>3.2.2</version>
+ <version>3.3.8</version>
<scope>test</scope>
</dependency>
<dependency>
diff --git
a/src/main/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandler.java
b/src/main/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandler.java
index 94d8002..1227f3c 100644
---
a/src/main/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandler.java
+++
b/src/main/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandler.java
@@ -30,45 +30,78 @@ import org.slf4j.LoggerFactory;
import java.util.Dictionary;
import java.util.Hashtable;
+import java.util.Objects;
+import java.util.Optional;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantLock;
public class FactoryRegistrationHandler implements AutoCloseable {
private static final Logger LOG =
LoggerFactory.getLogger(FactoryRegistrationHandler.class);
- private final ResourceResolverFactoryActivator activator;
+ private final ExecutorService factoryRegistrationWorker;
+
+ /**
+ * The configurationLock serializes access to the fields {@code activator}
+ * and {@code factoryPreconditions}. Each access to these fields must be
+ * guarded by this lock.
+ */
+ private final ReentrantLock configurationLock = new ReentrantLock();
- private final FactoryPreconditions factoryPreconditions;
+ /** Access only when holding configurationLock */
+ private ResourceResolverFactoryActivator activator;
- private final ExecutorService factoryRegistrationWorker;
+ /** Access only when holding configurationLock */
+ private FactoryPreconditions factoryPreconditions;
- @SuppressWarnings("java:S3077") // The field is only ever set to null or
to a new FactoryRegistration instance, which is safe.
- private volatile FactoryRegistration factoryRegistration;
+ private final AtomicReference<FactoryRegistration> factoryRegistration =
new AtomicReference<>(null);
- public FactoryRegistrationHandler(
- ResourceResolverFactoryActivator activator, FactoryPreconditions
factoryPreconditions) {
- this.factoryPreconditions = factoryPreconditions;
- this.activator = activator;
+ public FactoryRegistrationHandler() {
this.factoryRegistrationWorker = Executors.newSingleThreadExecutor(
r -> new Thread(r,
ResourceResolverFactory.class.getSimpleName() + "
registration/deregistration"));
}
+ public void configure(ResourceResolverFactoryActivator activator,
FactoryPreconditions factoryPreconditions) {
+ checkClosed();
+
+ boolean reRegister;
+ try {
+ configurationLock.lock();
+ reRegister = this.activator != activator ||
!Objects.equals(this.factoryPreconditions, factoryPreconditions);
+ LOG.debug("activator differs = {}, factoryPreconditions differ =
{}", this.activator != activator, !Objects.equals(this.factoryPreconditions,
factoryPreconditions));
+ LOG.debug("factoryPreconditions {} vs {}",
this.factoryPreconditions, factoryPreconditions);
+ this.factoryPreconditions = factoryPreconditions;
+ this.activator = activator;
+ } finally {
+ configurationLock.unlock();
+ }
+
+ if (reRegister) {
+ unregisterFactory();
+ maybeRegisterFactory(null, null);
+ }
+ }
+
+ private void checkClosed() {
+ if (factoryRegistrationWorker.isShutdown()) {
+ throw new IllegalStateException("FactoryRegistrationHandler is
already closed");
+ }
+ }
+
@Override
public void close() {
- unregisterFactory();
factoryRegistrationWorker.shutdown();
try {
- if (!factoryRegistrationWorker.awaitTermination(5,
TimeUnit.SECONDS)) {
+ if (!factoryRegistrationWorker.awaitTermination(1,
TimeUnit.MINUTES)) {
factoryRegistrationWorker.shutdownNow();
- // make sure everything is unregistered, even if
- // the factoryRegistrationWorker did not complete
- doUnregisterFactory();
}
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
+ runWithThreadName("deregistration on close",
this::doUnregisterFactory);
}
/**
@@ -77,15 +110,23 @@ public class FactoryRegistrationHandler implements
AutoCloseable {
void maybeRegisterFactory(final String unavailableName, final String
unavailableServicePid) {
if (!factoryRegistrationWorker.isShutdown()) {
LOG.debug("submitting maybeRegisterFactory");
- factoryRegistrationWorker.submit(() -> {
- final boolean preconditionsOk =
factoryPreconditions.checkPreconditions(unavailableName, unavailableServicePid);
- if ( preconditionsOk && this.factoryRegistration == null ) {
+ factoryRegistrationWorker.execute(() -> {
+ final boolean preconditionsOk;
+ final ResourceResolverFactoryActivator localActivator;
+ try {
+ configurationLock.lock();
+ preconditionsOk =
factoryPreconditions.checkPreconditions(unavailableName, unavailableServicePid);
+ localActivator = activator;
+ } finally {
+ configurationLock.unlock();
+ }
+ if (preconditionsOk) {
+ final Bundle systemBundle =
localActivator.getBundleContext().getBundle(Constants.SYSTEM_BUNDLE_LOCATION);
// check system bundle state - if stopping, don't register
new factory
- final Bundle systemBundle =
activator.getBundleContext().getBundle(Constants.SYSTEM_BUNDLE_LOCATION);
- if ( systemBundle != null && systemBundle.getState() !=
Bundle.STOPPING ) {
- runWithThreadName("registration",
this::doRegisterFactory);
+ if (systemBundle != null && systemBundle.getState() !=
Bundle.STOPPING) {
+ runWithThreadName("registration", () ->
this.doRegisterFactory(localActivator));
}
- } else if ( !preconditionsOk && this.factoryRegistration !=
null ) {
+ } else {
LOG.debug("performing unregisterFactory via
maybeRegisterFactory");
runWithThreadName("deregistration",
this::doUnregisterFactory);
}
@@ -94,35 +135,40 @@ public class FactoryRegistrationHandler implements
AutoCloseable {
}
void unregisterFactory() {
- LOG.debug("submitting unregisterFactory");
- factoryRegistrationWorker.submit(() ->
runWithThreadName("deregistration",
- this::doUnregisterFactory));
+ if (!factoryRegistrationWorker.isShutdown()) {
+ LOG.debug("submitting unregisterFactory");
+ factoryRegistrationWorker.execute(
+ () -> runWithThreadName("deregistration",
this::doUnregisterFactory));
+ }
}
/**
* Register the factory.
*/
- private void doRegisterFactory() {
- LOG.debug("performing registerFactory, factoryRegistration == {}",
factoryRegistration);
- if (this.factoryRegistration == null) {
- this.factoryRegistration = new
FactoryRegistration(activator.getBundleContext(), activator);
- }
- LOG.debug("finished performing registerFactory, factoryRegistration ==
{}", factoryRegistration);
+ private void doRegisterFactory(ResourceResolverFactoryActivator activator)
{
+ final FactoryRegistration newRegistration =
factoryRegistration.updateAndGet(oldRegistration -> {
+ LOG.debug("performing registerFactory, factoryRegistration == {}",
oldRegistration);
+ return Objects.requireNonNullElseGet(oldRegistration,
+ () -> new
FactoryRegistration(activator.getBundleContext(), activator));
+ });
+ LOG.debug("finished performing registerFactory, factoryRegistration ==
{}", newRegistration);
}
/**
* Unregister the factory (if registered).
*/
private void doUnregisterFactory() {
- LOG.debug("performing unregisterFactory, factoryRegistration == {}",
factoryRegistration);
- if (this.factoryRegistration != null) {
- this.factoryRegistration.unregister();
- this.factoryRegistration = null;
- }
- LOG.debug("finished performing unregisterFactory, factoryRegistration
== {}", factoryRegistration);
+ final FactoryRegistration newRegistration =
factoryRegistration.updateAndGet(oldRegistration -> {
+ LOG.debug("performing unregisterFactory, factoryRegistration ==
{}", factoryRegistration);
+ Optional.ofNullable(oldRegistration)
+ .ifPresent(FactoryRegistration::unregister);
+ LOG.debug("setting factoryRegistration = null");
+ return null;
+ });
+ LOG.debug("finished performing unregisterFactory, factoryRegistration
== {}", newRegistration);
}
- private static void runWithThreadName(String threadNameSuffix, Runnable
task) {
+ private void runWithThreadName(String threadNameSuffix, Runnable task) {
final String name = Thread.currentThread().getName();
try {
Thread.currentThread().setName(ResourceResolverFactory.class.getSimpleName() +
" " + threadNameSuffix);
@@ -168,9 +214,16 @@ public class FactoryRegistrationHandler implements
AutoCloseable {
}
void unregister() {
+ LOG.debug("Unregister runtimeRegistration");
runtimeRegistration.unregister();
+
+ LOG.debug("Unregister factoryRegistration");
factoryRegistration.unregister();
+
+ LOG.debug("Unregister commonFactory");
commonFactory.deactivate();
+
+ LOG.debug("Unregister completed");
}
}
}
diff --git
a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
index 1651e06..39bc17c 100644
---
a/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
+++
b/src/main/java/org/apache/sling/resourceresolver/impl/ResourceResolverFactoryActivator.java
@@ -124,8 +124,7 @@ public class ResourceResolverFactoryActivator {
/** Observation paths */
private volatile Path[] observationPaths;
- @SuppressWarnings("java:S3077")
- private volatile FactoryRegistrationHandler factoryRegistrationHandler;
+ private final FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler();
private final VanityPathConfigurer vanityPathConfigurer = new
VanityPathConfigurer();
{
@@ -313,8 +312,7 @@ public class ResourceResolverFactoryActivator {
resourceProviderTracker,
requiredResourceProviderNames,
requiredResourceProvidersLegacy);
- factoryRegistrationHandler = new FactoryRegistrationHandler(this,
factoryPreconditions);
- factoryRegistrationHandler.maybeRegisterFactory(null, null);
+ factoryRegistrationHandler.configure(this, factoryPreconditions);
if (!hasPreRegisteredResourceProviderTracker) {
this.resourceProviderTracker.activate(this.bundleContext,
this.eventAdmin,
@@ -343,7 +341,7 @@ public class ResourceResolverFactoryActivator {
protected void modified(final BundleContext bundleContext,
final ResourceResolverFactoryConfig config,
final VanityPathConfigurer.DeprecatedVanityConfig
deprecatedVanityConfig) {
- this.deactivate();
+ this.deactivateInternal();
this.activate(bundleContext, config, deprecatedVanityConfig);
}
@@ -352,11 +350,13 @@ public class ResourceResolverFactoryActivator {
*/
@Deactivate
protected void deactivate() {
- this.factoryRegistrationHandler.close();
- this.factoryRegistrationHandler = null;
-
// factoryRegistrationHandler must be closed before bundleContext is
set to null
+ this.factoryRegistrationHandler.close();
this.bundleContext = null;
+ deactivateInternal();
+ }
+
+ private void deactivateInternal() {
this.config = DEFAULT_CONFIG;
this.vanityPathConfigurer.setConfiguration(DEFAULT_CONFIG, null);
this.changeListenerWhiteboard.deactivate();
diff --git
a/src/test/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandlerTest.java
b/src/test/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandlerTest.java
index 0f66b7f..04fb408 100644
---
a/src/test/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandlerTest.java
+++
b/src/test/java/org/apache/sling/resourceresolver/impl/FactoryRegistrationHandlerTest.java
@@ -22,160 +22,204 @@ import
org.apache.commons.collections4.bidimap.TreeBidiMap;
import org.apache.sling.api.resource.ResourceResolverFactory;
import
org.apache.sling.resourceresolver.impl.providers.ResourceProviderStorage;
import
org.apache.sling.resourceresolver.impl.providers.ResourceProviderTracker;
+import org.apache.sling.resourceresolver.util.events.RecordingListener;
+import
org.apache.sling.resourceresolver.util.events.ServiceEventUtil.ServiceEventDTO;
import org.apache.sling.serviceusermapping.ServiceUserMapper;
import org.apache.sling.testing.mock.osgi.junit.OsgiContext;
-import org.junit.Before;
-import org.junit.Rule;
-import org.junit.Test;
+import org.apache.sling.testing.mock.osgi.junit5.OsgiContextExtension;
+import org.hamcrest.Matcher;
+import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.RepeatedTest;
+import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mockito;
import org.mockito.internal.stubbing.defaultanswers.ReturnsSmartNulls;
import org.osgi.framework.BundleContext;
-import org.osgi.framework.ServiceEvent;
-import org.osgi.framework.ServiceListener;
-import org.osgi.framework.ServiceReference;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-import java.util.concurrent.CountDownLatch;
-import java.util.concurrent.TimeUnit;
-import java.util.function.Predicate;
-
-import static org.junit.Assert.assertTrue;
+import static org.apache.sling.resourceresolver.util.CustomMatchers.allOf;
+import static org.apache.sling.resourceresolver.util.CustomMatchers.hasItem;
+import static
org.apache.sling.resourceresolver.util.events.ServiceEventUtil.registration;
+import static
org.apache.sling.resourceresolver.util.events.ServiceEventUtil.unregistration;
+import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.hasSize;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.mockito.ArgumentMatchers.isNull;
+import static org.mockito.Mockito.doCallRealMethod;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.when;
-public class FactoryRegistrationHandlerTest {
+@ExtendWith(OsgiContextExtension.class)
+class FactoryRegistrationHandlerTest {
+
+ private static final int DEFAULT_TEST_ITERATIONS = 20;
+
+ private static final @NotNull Matcher<Iterable<? extends ServiceEventDTO>>
RRF_REGISTRATION = allOf(
+ hasSize(4),
+ hasItem(registration(ResourceResolverFactory.class))
+ );
- private static final Logger LOG =
LoggerFactory.getLogger(FactoryRegistrationHandlerTest.class);
+ private static final @NotNull Matcher<Iterable<? extends ServiceEventDTO>>
RRF_UNREGISTRATION = allOf(
+ hasSize(4),
+ hasItem(unregistration(ResourceResolverFactory.class))
+ );
- public static final ReturnsSmartNulls DEFAULT_ANSWER = new
ReturnsSmartNulls();
+ private static final @NotNull Matcher<Iterable<? extends ServiceEventDTO>>
RRF_REREGISTRATION = allOf(
+ hasSize(8),
+ hasItem(unregistration(ResourceResolverFactory.class)),
+ hasItem(registration(ResourceResolverFactory.class))
+ );
- @Rule
- public OsgiContext osgi = new OsgiContext();
+ OsgiContext osgi = new OsgiContext();
private ResourceResolverFactoryActivator activator;
- @Before
- public void setUp() {
+ @BeforeEach
+ void setUp() {
final ResourceProviderTracker resourceProviderTracker =
mock(ResourceProviderTracker.class);
doReturn(mock(ResourceProviderStorage.class)).when(resourceProviderTracker).getResourceProviderStorage();
+ final VanityPathConfigurer vanityPathConfigurer =
mock(VanityPathConfigurer.class);
+ doReturn(false).when(vanityPathConfigurer).isVanityPathEnabled();
+
activator = mock(ResourceResolverFactoryActivator.class);
doReturn(osgi.bundleContext()).when(activator).getBundleContext();
doReturn(resourceProviderTracker).when(activator).getResourceProviderTracker();
doReturn(mock(ServiceUserMapper.class)).when(activator).getServiceUserMapper();
doReturn(new TreeBidiMap<>()).when(activator).getVirtualURLMap();
doReturn(null).when(activator).getEventAdmin();
-
+
doReturn(vanityPathConfigurer).when(activator).getVanityPathConfigurer();
+ doCallRealMethod().when(activator).getRuntimeService();
}
private static <T> T mock(Class<T> classToMock) {
- return Mockito.mock(classToMock, DEFAULT_ANSWER);
+ return Mockito.mock(classToMock, new ReturnsSmartNulls());
}
- @Test
- public void testFactoryRegistration() throws InterruptedException {
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testFactoryRegistrationDeregistration() throws InterruptedException {
+ final BundleContext bundleContext = osgi.bundleContext();
final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
- try (AwaitingListener unregistration =
- AwaitingListener.unregistration(osgi.bundleContext(),
ResourceResolverFactory.class)) {
- try (FactoryRegistrationHandler factoryRegistrationHandler =
- new FactoryRegistrationHandler(activator,
preconditions);
- AwaitingListener registration =
AwaitingListener.registration(osgi.bundleContext(),
ResourceResolverFactory.class)) {
+ try (FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler()) {
+
+ try (RecordingListener listener =
RecordingListener.of(bundleContext)) {
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
+ }
+
+ try (RecordingListener listener =
RecordingListener.of(bundleContext)) {
+ factoryRegistrationHandler.unregisterFactory();
+ listener.assertRecorded(RRF_UNREGISTRATION);
+ }
+
+ try (RecordingListener listener =
RecordingListener.of(bundleContext)) {
factoryRegistrationHandler.maybeRegisterFactory(null, null);
- assertTrue("Expected ResourceResolverFactory service to be
registered",
- registration.await(5, TimeUnit.SECONDS));
+ listener.assertRecorded(RRF_REGISTRATION);
}
- assertTrue("Expected ResourceResolverFactory service to be
unregistered",
- unregistration.await(5, TimeUnit.SECONDS));
}
}
- @Test
- public void testFactoryDeregistrationWhenConditionsUnsatisfied() throws
InterruptedException {
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testConditionChangeLeadingToUnregistration() throws
InterruptedException {
final BundleContext ctx = osgi.bundleContext();
final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
- try (FactoryRegistrationHandler factoryRegistrationHandler =
- new FactoryRegistrationHandler(activator, preconditions);
- AwaitingListener registration =
AwaitingListener.registration(ctx, ResourceResolverFactory.class);
- AwaitingListener unregistration =
AwaitingListener.unregistration(ctx, ResourceResolverFactory.class)) {
- when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
- factoryRegistrationHandler.maybeRegisterFactory(null, null);
- assertTrue("Expected ResourceResolverFactory service to be
registered",
- registration.await(5, TimeUnit.SECONDS));
+ try (FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler()) {
+ try (final RecordingListener listener = RecordingListener.of(ctx))
{
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
+ }
- when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(false);
- factoryRegistrationHandler.maybeRegisterFactory(null, null);
- assertTrue("Expected ResourceResolverFactory service to be
unregistered",
- unregistration.await(5, TimeUnit.SECONDS));
+ try (final RecordingListener listener = RecordingListener.of(ctx))
{
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(false);
+ factoryRegistrationHandler.maybeRegisterFactory(null, null);
+ listener.assertRecorded(RRF_UNREGISTRATION);
+ }
}
}
- private static class AwaitingListener implements ServiceListener,
AutoCloseable {
- private final BundleContext bundleContext;
-
- private final Predicate<ServiceEvent> expectedEventPredicate;
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testReconfigurationLeadingToUnregsitration() throws
InterruptedException {
+ final BundleContext ctx = osgi.bundleContext();
+ final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
- private final CountDownLatch latch;
+ try (final FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler()) {
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
+ }
- public AwaitingListener(BundleContext ctx, Predicate<ServiceEvent>
expectedEventPredicate) {
- this(ctx, expectedEventPredicate, 1);
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ final FactoryPreconditions failingPreconditions =
mock(FactoryPreconditions.class); // new instance
+ when(failingPreconditions.checkPreconditions(isNull(),
isNull())).thenReturn(false);
+ factoryRegistrationHandler.configure(activator,
failingPreconditions);
+ listener.assertRecorded(RRF_UNREGISTRATION);
+ }
}
+ }
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testReconfigurationWithNoChanges() throws InterruptedException {
+ final BundleContext ctx = osgi.bundleContext();
+ final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
- public AwaitingListener(BundleContext ctx, Predicate<ServiceEvent>
expectedEventPredicate, int count) {
- this.bundleContext = ctx;
- this.expectedEventPredicate = expectedEventPredicate;
- this.latch = new CountDownLatch(count);
- this.bundleContext.addServiceListener(this);
- }
+ try (final FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler()) {
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
+ }
- public static AwaitingListener registration(BundleContext
bundleContext, Class<?> clazz) {
- return new AwaitingListener(bundleContext, serviceEvent -> {
- if (serviceEvent.getType() == ServiceEvent.REGISTERED) {
- return isInstance(bundleContext,
serviceEvent.getServiceReference(), clazz);
- }
- return false;
- });
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(empty());
+ }
}
+ }
- public static AwaitingListener unregistration(BundleContext
bundleContext, Class<?> clazz) {
- return new AwaitingListener(bundleContext, serviceEvent -> {
- if (serviceEvent.getType() == ServiceEvent.UNREGISTERING) {
- return isInstance(bundleContext,
serviceEvent.getServiceReference(), clazz);
- }
- return false;
- });
- }
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testReconfigurationLeadingToReregistration() throws
InterruptedException {
+ final BundleContext ctx = osgi.bundleContext();
- private static boolean isInstance(BundleContext bundleContext,
ServiceReference<?> ref, Class<?> clazz) {
- try {
- final Object service = bundleContext.getService(ref);
- final boolean isInstance = clazz.isInstance(service);
- LOG.info("{} == isInstance({}, {})", isInstance,
service.getClass(), clazz);
- return isInstance;
- } finally {
- bundleContext.ungetService(ref);
+ try (final FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler()) {
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
}
- }
- @Override
- public void serviceChanged(ServiceEvent event) {
- if (expectedEventPredicate.test(event)) {
- latch.countDown();
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(allOf(RRF_REREGISTRATION));
}
}
+ }
- public boolean await(long timeout, TimeUnit timeUnit) throws
InterruptedException {
- return latch.await(timeout, timeUnit);
+ @RepeatedTest(DEFAULT_TEST_ITERATIONS)
+ void testUnregisterOnClose() throws InterruptedException {
+ final BundleContext ctx = osgi.bundleContext();
+ final FactoryPreconditions preconditions =
mock(FactoryPreconditions.class);
+ final FactoryRegistrationHandler factoryRegistrationHandler = new
FactoryRegistrationHandler();
+
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(true);
+ factoryRegistrationHandler.configure(activator, preconditions);
+ listener.assertRecorded(RRF_REGISTRATION);
}
- @Override
- public void close() {
- bundleContext.removeServiceListener(this);
+ try (RecordingListener listener = RecordingListener.of(ctx)) {
+ when(preconditions.checkPreconditions(isNull(),
isNull())).thenReturn(false);
+ factoryRegistrationHandler.close();
+ listener.assertRecorded(allOf(RRF_UNREGISTRATION));
+
+ assertThrows(IllegalStateException.class, () ->
factoryRegistrationHandler.configure(activator, preconditions),
+ "Reconfiguration is no longer possible after a
FactoryRegistrationHandler is closed.");
}
}
}
\ No newline at end of file
diff --git
a/src/test/java/org/apache/sling/resourceresolver/util/CustomMatchers.java
b/src/test/java/org/apache/sling/resourceresolver/util/CustomMatchers.java
new file mode 100644
index 0000000..1f20694
--- /dev/null
+++ b/src/test/java/org/apache/sling/resourceresolver/util/CustomMatchers.java
@@ -0,0 +1,123 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.util;
+
+import org.hamcrest.Description;
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+import org.hamcrest.StringDescription;
+import org.hamcrest.TypeSafeDiagnosingMatcher;
+import org.jetbrains.annotations.NotNull;
+
+import java.util.Collection;
+import java.util.List;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+import java.util.stream.StreamSupport;
+
+/**
+ * Custom version of Hamcrest matchers that fix the generics to make them
usable.
+ */
+public class CustomMatchers {
+
+
+ @SafeVarargs
+ @NotNull
+ public static <T> Matcher<T> allOf(Matcher<? extends T> first, Matcher<?
extends T>... more) {
+ final List<Matcher<? extends T>> matchers =
Stream.concat(Stream.of(first), Stream.of(more)).collect(Collectors.toList());
+ return new AllOfMatcher<>(matchers);
+ }
+
+ public static <T> Matcher<Iterable<? extends T>> hasItem(T item) {
+ return hasItem(Matchers.equalTo(item));
+ }
+
+ public static <T> Matcher<Iterable<? extends T>> hasItem(Matcher<? extends
T> elementMatcher) {
+ return new HasItemMatcher<>(elementMatcher);
+ }
+
+
+ private static <T> Stream<? extends T> toStream(Iterable<? extends T>
iterable) {
+ return StreamSupport.stream(iterable.spliterator(), false);
+ };
+
+ private static class AllOfMatcher<T> extends TypeSafeDiagnosingMatcher<T> {
+ private final Collection<Matcher<? extends T>> matchers;
+
+ AllOfMatcher(Collection<Matcher<? extends T>> matchers) {
+ this.matchers = matchers;
+ }
+
+ @Override
+ protected boolean matchesSafely(T item, Description
mismatchDescription) {
+ return matchers.stream()
+ .filter(matcher -> !matcher.matches(item))
+ .peek(matcher -> {
+
mismatchDescription.appendDescriptionOf(matcher).appendText(" ");
+ matcher.describeMismatch(item, mismatchDescription);
+ })
+ .findFirst()
+ .isEmpty();
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description.appendList("(", " " + "and" + " ", ")", matchers);
+ }
+ }
+
+ private static class HasItemMatcher<T> extends
TypeSafeDiagnosingMatcher<Iterable<? extends T>> {
+
+ private final Matcher<? extends T> elementMatcher;
+
+ public HasItemMatcher(Matcher<? extends T> elementMatcher) {
+ this.elementMatcher = elementMatcher;
+ }
+
+ @Override
+ protected boolean matchesSafely(Iterable<? extends T> iterable,
Description mismatchDescription) {
+ if (toStream(iterable).limit(1).count() == 0) {
+ mismatchDescription.appendText("was empty");
+ return false;
+ }
+
+ if (toStream(iterable).anyMatch(elementMatcher::matches)) {
+ return true;
+ }
+
+ mismatchDescription.appendText(
+ toStream(iterable)
+ .map(item -> describeItemMismatch(elementMatcher,
item))
+ .collect(Collectors.joining(", ", "mismatches
were: [", "]")));
+ return false;
+ }
+
+ @NotNull
+ private String describeItemMismatch(Matcher<? extends T>
elementMatcher1, T item) {
+ final StringDescription desc = new StringDescription();
+ elementMatcher1.describeMismatch(item, desc);
+ return desc.toString();
+ }
+
+ @Override
+ public void describeTo(Description description) {
+ description.appendText("an iterable containing
").appendDescriptionOf(elementMatcher);
+ }
+ }
+}
diff --git
a/src/test/java/org/apache/sling/resourceresolver/util/events/AbstractAwaitingListener.java
b/src/test/java/org/apache/sling/resourceresolver/util/events/AbstractAwaitingListener.java
new file mode 100644
index 0000000..6a03afb
--- /dev/null
+++
b/src/test/java/org/apache/sling/resourceresolver/util/events/AbstractAwaitingListener.java
@@ -0,0 +1,75 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.util.events;
+
+import
org.apache.sling.resourceresolver.util.events.ServiceEventUtil.ServiceEventDTO;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceEvent;
+import org.osgi.framework.ServiceListener;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+abstract class AbstractAwaitingListener implements ServiceListener,
AutoCloseable {
+
+ private static final Logger LOG =
LoggerFactory.getLogger(AbstractAwaitingListener.class);
+
+ private final BundleContext bundleContext;
+
+ private final CountDownLatch latch;
+
+ public AbstractAwaitingListener(BundleContext ctx, int count) {
+ this.bundleContext = ctx;
+ this.latch = new CountDownLatch(count);
+ }
+
+ protected final void startListening() {
+ this.bundleContext.addServiceListener(this);
+ LOG.info("addServiceListener({})", this);
+ }
+
+ @Override
+ public void serviceChanged(ServiceEvent event) {
+ final ServiceEventDTO eventDTO = ServiceEventDTO.create(event);
+ final boolean matchingServiceEvent = isMatchingServiceEvent(event);
+ LOG.info("serviceChanged({}, {}) => {} | {}", eventDTO.getEventType(),
eventDTO.getClasses(), matchingServiceEvent, this);
+ if (matchingServiceEvent) {
+ latch.countDown();
+ }
+ }
+
+ protected abstract boolean isMatchingServiceEvent(ServiceEvent
serviceEvent);
+
+ protected boolean await(long timeout, TimeUnit timeUnit) throws
InterruptedException {
+ return latch.await(timeout, timeUnit);
+ }
+
+ @Override
+ public void close() {
+ stopListening();
+ }
+
+ protected void stopListening() {
+ bundleContext.removeServiceListener(this);
+ LOG.info("removeServiceListener({})", this);
+ }
+
+}
diff --git
a/src/test/java/org/apache/sling/resourceresolver/util/events/RecordingListener.java
b/src/test/java/org/apache/sling/resourceresolver/util/events/RecordingListener.java
new file mode 100644
index 0000000..dbeb60f
--- /dev/null
+++
b/src/test/java/org/apache/sling/resourceresolver/util/events/RecordingListener.java
@@ -0,0 +1,99 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.util.events;
+
+import
org.apache.sling.resourceresolver.util.events.ServiceEventUtil.ServiceEventDTO;
+import org.hamcrest.Matcher;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceEvent;
+import org.osgi.framework.ServiceRegistration;
+
+import java.util.Collection;
+import java.util.Hashtable;
+import java.util.Map;
+import java.util.Objects;
+import java.util.concurrent.ConcurrentLinkedQueue;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class RecordingListener extends AbstractAwaitingListener {
+
+ private final Collection<ServiceEventDTO> serviceEvents = new
ConcurrentLinkedQueue<ServiceEventDTO>();
+
+ private ServiceRegistration<RecordingListener> signalRegistration;
+
+ private final Predicate<ServiceEvent> recordingFilter;
+
+ public static RecordingListener of(BundleContext ctx) {
+ final RecordingListener recordingListener = new RecordingListener(ctx,
serviceEvent -> true);
+ recordingListener.startListening();
+ return recordingListener;
+ }
+
+ public static RecordingListener of(BundleContext ctx,
Predicate<ServiceEvent> recordingFilter) {
+ final RecordingListener recordingListener = new RecordingListener(ctx,
recordingFilter);
+ recordingListener.startListening();
+ return recordingListener;
+ }
+
+ private RecordingListener(BundleContext ctx, Predicate<ServiceEvent>
recordingFilter) {
+ super(ctx, 1);
+ this.recordingFilter = recordingFilter;
+ this.signalRegistration = ctx.registerService(RecordingListener.class,
this,
+ new Hashtable<>(Map.of("::class::", RecordingListener.class)));
+ }
+
+ @Override
+ protected boolean isMatchingServiceEvent(ServiceEvent serviceEvent) {
+ return signalRegistration != null // is null when registering "this"
+ && serviceEvent.getServiceReference() ==
signalRegistration.getReference();
+ }
+
+ @Override
+ public void serviceChanged(ServiceEvent serviceEvent) {
+ super.serviceChanged(serviceEvent);
+ if (!isInternalEvent(serviceEvent) &&
recordingFilter.test(serviceEvent)) {
+ serviceEvents.add(ServiceEventDTO.create(serviceEvent));
+ }
+ }
+
+ private boolean isInternalEvent(ServiceEvent serviceEvent) {
+ return
Objects.equals(serviceEvent.getServiceReference().getProperty("::class::"),
RecordingListener.class);
+ }
+
+ public void assertRecorded(Matcher<? super Collection<? extends
ServiceEventDTO>> serviceEventDTOMatcher) throws InterruptedException {
+ if (signalRegistration != null) {
+ final long deadline = System.nanoTime() +
TimeUnit.SECONDS.toNanos(1);
+ while (!serviceEventDTOMatcher.matches(serviceEvents) &&
System.nanoTime() < deadline) {
+ // give other threads a chance
+ Thread.yield();
+ }
+ signalRegistration.unregister();
+ if (!await(1, TimeUnit.SECONDS)) {
+ throw new IllegalStateException("Unregistration event not
observed within 1 second.");
+ }
+ signalRegistration = null;
+ stopListening();
+ }
+
+ assertThat("Expected ServiceEvents", serviceEvents,
serviceEventDTOMatcher);
+ }
+}
diff --git
a/src/test/java/org/apache/sling/resourceresolver/util/events/ServiceEventUtil.java
b/src/test/java/org/apache/sling/resourceresolver/util/events/ServiceEventUtil.java
new file mode 100644
index 0000000..16b6262
--- /dev/null
+++
b/src/test/java/org/apache/sling/resourceresolver/util/events/ServiceEventUtil.java
@@ -0,0 +1,105 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.resourceresolver.util.events;
+
+import org.hamcrest.Matcher;
+import org.hamcrest.Matchers;
+import org.osgi.framework.ServiceEvent;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Objects;
+import java.util.Set;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
+
+public class ServiceEventUtil {
+
+ public static Matcher<ServiceEventDTO> registration(Class<?> clazz) {
+ return
Matchers.equalTo(ServiceEventDTO.create(ServiceEvent.REGISTERED, clazz));
+ }
+
+ public static Matcher<ServiceEventDTO> unregistration(Class<?> clazz) {
+ return
Matchers.equalTo(ServiceEventDTO.create(ServiceEvent.UNREGISTERING, clazz));
+ }
+
+ public static class ServiceEventDTO {
+
+ private static final String[] SERVICE_EVENT_TYPES = new String[9];
+ static {
+ SERVICE_EVENT_TYPES[ServiceEvent.REGISTERED] = "REGISTERED";
+ SERVICE_EVENT_TYPES[ServiceEvent.UNREGISTERING] = "UNREGISTERING";
+ SERVICE_EVENT_TYPES[ServiceEvent.MODIFIED] = "MODIFIED";
+ SERVICE_EVENT_TYPES[ServiceEvent.MODIFIED_ENDMATCH] =
"MODIFIED_ENDMATCH";
+ }
+
+ private final int eventType;
+
+ private final Set<String> classNames;
+
+ private ServiceEventDTO(int eventType, Set<String> classNames) {
+ this.eventType = eventType;
+ this.classNames = Collections.unmodifiableSet(classNames);
+ }
+
+ public static ServiceEventDTO create(int eventType, Class<?>...
classes) {
+ final Set<String> classNames =
Stream.of(classes).map(Class::getName).collect(Collectors.toCollection(TreeSet::new));
+ return new ServiceEventDTO(eventType, classNames);
+ }
+
+ public static ServiceEventDTO create(ServiceEvent event) {
+ final int eventType = event.getType();
+ final Object objectClass =
event.getServiceReference().getProperty("objectClass");
+ final Set<String> classNames;
+ if (objectClass instanceof String[]) {
+ classNames = new TreeSet<>(Arrays.asList((String[])
objectClass));
+ } else {
+ classNames = Collections.emptySet();
+ }
+ return new ServiceEventDTO(eventType, classNames);
+ }
+
+ public String getEventType() {
+ return SERVICE_EVENT_TYPES[eventType];
+ }
+
+ public Set<String> getClasses() {
+ return classNames;
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) return true;
+ if (o == null || getClass() != o.getClass()) return false;
+ ServiceEventDTO that = (ServiceEventDTO) o;
+ return eventType == that.eventType && Objects.equals(classNames,
that.classNames);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(eventType, classNames);
+ }
+
+ @Override
+ public String toString() {
+ return String.format("ServiceEvent(%s, %s)", getEventType(),
classNames);
+ }
+ }
+}
diff --git a/src/test/resources/junit-platform.properties
b/src/test/resources/junit-platform.properties
new file mode 100644
index 0000000..07cb911
--- /dev/null
+++ b/src/test/resources/junit-platform.properties
@@ -0,0 +1,19 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+junit.jupiter.execution.parallel.enabled = true
+junit.jupiter.execution.parallel.mode.default = concurrent
+junit.jupiter.execution.parallel.mode.classes.default = concurrent
\ No newline at end of file
