Merge branch 'master' into issue/SLING-7584
diff --git a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
index 736c344..7d93bbb 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
@@ -31,6 +31,7 @@
import java.util.Collections;
import java.util.Dictionary;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Hashtable;
import java.util.Iterator;
import java.util.List;
@@ -137,7 +138,7 @@
private static final Object REQUEST_MARKER_VALUE = new Object();
- private static class DisposalCallbackRegistryImpl implements DisposalCallbackRegistry {
+ private static class DisposalCallbackRegistryImpl implements DisposalCallbackRegistry, Disposable {
private List<DisposalCallback> callbacks = new ArrayList<>();
@@ -150,7 +151,9 @@
callbacks = Collections.unmodifiableList(callbacks);
}
- private void onDisposed() {
+
+ @Override
+ public void onDisposed() {
for (DisposalCallback callback : callbacks) {
callback.onDisposed();
}
@@ -158,11 +161,60 @@
}
+ private static class CombinedDisposable implements Disposable {
+ private final Collection<Disposable> delegates = Collections.synchronizedCollection(new HashSet<Disposable>());
+
+ private void add(Disposable disposable) {
+ delegates.add(disposable);
+ }
+
+ @Override
+ public void onDisposed() {
+ for (Disposable delegate : delegates) {
+ delegate.onDisposed();
+ }
+ }
+ }
+
+ private interface Disposable {
+ void onDisposed();
+ }
+
+ private static class RequestDisposalCallbacks {
+ private ConcurrentHashMap<ServletRequest, Disposable> callbacks = new ConcurrentHashMap<>();
+
+ public Collection<Disposable> values() {
+ return callbacks.values();
+ }
+
+ public Disposable remove(ServletRequest request) {
+ return callbacks.remove(request);
+ }
+
+ public void put(ServletRequest request, Disposable registry) {
+ synchronized (callbacks) {
+ CombinedDisposable combinedDisposable = null;
+ Disposable current = callbacks.get(request);
+ if (current == null) {
+ callbacks.put(request, registry);
+ return;
+ } else if (current instanceof CombinedDisposable) {
+ combinedDisposable = (CombinedDisposable) current;
+ } else {
+ combinedDisposable = new CombinedDisposable();
+ combinedDisposable.add(current);
+ callbacks.put(request, combinedDisposable);
+ }
+ combinedDisposable.add(registry);
+ }
+ }
+ }
+
private ReferenceQueue<Object> queue;
- private ConcurrentMap<java.lang.ref.Reference<Object>, DisposalCallbackRegistryImpl> disposalCallbacks;
+ private ConcurrentMap<java.lang.ref.Reference<Object>, Disposable> disposalCallbacks;
- private ConcurrentHashMap<ServletRequest, DisposalCallbackRegistryImpl> requestDisposalCallbacks;
+ private RequestDisposalCallbacks requestDisposalCallbacks;
@Override
public void run() {
@@ -173,7 +225,7 @@
java.lang.ref.Reference<?> ref = queue.poll();
while (ref != null) {
log.debug("calling disposal for {}.", ref.toString());
- DisposalCallbackRegistryImpl registry = disposalCallbacks.remove(ref);
+ Disposable registry = disposalCallbacks.remove(ref);
registry.onDisposed();
ref = queue.poll();
}
@@ -1072,7 +1124,7 @@
BundleContext bundleContext = ctx.getBundleContext();
this.queue = new ReferenceQueue<>();
this.disposalCallbacks = new ConcurrentHashMap<>();
- this.requestDisposalCallbacks = new ConcurrentHashMap<>();
+ this.requestDisposalCallbacks = new RequestDisposalCallbacks();
Hashtable<String, Object> properties = new Hashtable<>();
properties.put(Constants.SERVICE_VENDOR, "Apache Software Foundation");
properties.put(Constants.SERVICE_DESCRIPTION, "Sling Models OSGi Service Disposal Job");
@@ -1101,7 +1153,7 @@
protected void deactivate() {
this.adapterCache = null;
if (this.requestDisposalCallbacks != null) {
- for (final DisposalCallbackRegistryImpl requestRegistries : this.requestDisposalCallbacks.values()) {
+ for (final Disposable requestRegistries : this.requestDisposalCallbacks.values()) {
requestRegistries.onDisposed();
}
}
@@ -1314,7 +1366,7 @@
@Override
public void requestDestroyed(ServletRequestEvent sre) {
ServletRequest request = unwrapRequest(sre.getServletRequest());
- DisposalCallbackRegistryImpl registry = requestDisposalCallbacks.remove(request);
+ Disposable registry = requestDisposalCallbacks.remove(request);
if (registry != null) {
registry.onDisposed();
}
diff --git a/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
index 99ec6b5..bcf8e4a 100644
--- a/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
+++ b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
@@ -41,8 +41,10 @@
import java.lang.reflect.AnnotatedElement;
import java.lang.reflect.Type;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Hashtable;
import java.util.Map;
+import java.util.Set;
import static org.mockito.Mockito.*;
import static org.junit.Assert.*;
@@ -66,7 +68,7 @@
private ModelAdapterFactory factory;
- private TestDisposalCallback callback;
+ private Set<TestDisposalCallback> callbacks;
@Before
public void setup() {
@@ -96,8 +98,7 @@
return attributes.get(invocation.getArguments()[0]);
}
});
-
- callback = new TestDisposalCallback();
+ callbacks = new HashSet<>();
}
@Test
@@ -112,11 +113,33 @@
TestModel model = factory.getAdapter(adaptableRequest, TestModel.class);
assertEquals("teststring", model.testString);
- assertFalse(callback.isDisposed());
+ assertNoneDisposed();
factory.requestDestroyed(new ServletRequestEvent(servletContext, destroyedRequest));
- assertTrue(callback.isDisposed());
+ assertAllDisposed();
+ }
+
+ @Test
+ public void testTwoInstancesWithInitializedRequest() {
+ // destroy a wrapper of the root request
+ SlingHttpServletRequest destroyedRequest = new SlingHttpServletRequestWrapper(request);
+ factory.requestInitialized(new ServletRequestEvent(servletContext, destroyedRequest));
+
+ // but adapt from a wrapper of a wrapper of that wrapper
+ SlingHttpServletRequest adaptableRequest = new SlingHttpServletRequestWrapper(new SlingHttpServletRequestWrapper(destroyedRequest));
+
+ TestModel model = factory.getAdapter(adaptableRequest, TestModel.class);
+ assertEquals("teststring", model.testString);
+
+ TestModel model2 = factory.getAdapter(adaptableRequest, TestModel.class);
+ assertEquals("teststring", model2.testString);
+
+ assertNoneDisposed();
+
+ factory.requestDestroyed(new ServletRequestEvent(servletContext, destroyedRequest));
+
+ assertAllDisposed();
}
@Test
@@ -130,11 +153,23 @@
TestModel model = factory.getAdapter(adaptableRequest, TestModel.class);
assertEquals("teststring", model.testString);
- assertFalse(callback.isDisposed());
+ assertNoneDisposed();
factory.requestDestroyed(new ServletRequestEvent(servletContext, destroyedRequest));
- assertFalse(callback.isDisposed());
+ assertNoneDisposed();
+ }
+
+ private void assertNoneDisposed() {
+ for (TestDisposalCallback callback : callbacks) {
+ assertFalse(callback.isDisposed());
+ }
+ }
+
+ private void assertAllDisposed() {
+ for (TestDisposalCallback callback : callbacks) {
+ assertTrue(callback.isDisposed());
+ }
}
@Model(adaptables = SlingHttpServletRequest.class)
@@ -154,7 +189,9 @@
@Nullable
@Override
- public Object getValue(@NotNull Object o, String s, @NotNull Type type, @NotNull AnnotatedElement annotatedElement, @NotNull DisposalCallbackRegistry disposalCallbackRegistry) {
+ public Object getValue(@Nonnull Object o, String s, @Nonnull Type type, @Nonnull AnnotatedElement annotatedElement, @Nonnull DisposalCallbackRegistry disposalCallbackRegistry) {
+ TestDisposalCallback callback = new TestDisposalCallback();
+ callbacks.add(callback);
disposalCallbackRegistry.addDisposalCallback(callback);
return "teststring";
}
