SLING-5668 - Leverage ServletRequestListener.requestDestroyed for cal…

…ling DisposalCallback in case the model was created from a request

.gitignore

@@ -15,3 +15,4 @@ maven-eclipse.xml
 .DS_Store
 jcr.log
 atlassian-ide-plugin.xml
+dependency-reduced-pom.xml

src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java

@@ -41,6 +41,10 @@
 import javax.annotation.CheckForNull;
 import javax.annotation.Nonnull;
 import javax.annotation.PostConstruct;
+import javax.servlet.ServletRequest;
+import javax.servlet.ServletRequestEvent;
+import javax.servlet.ServletRequestListener;
+import javax.servlet.ServletRequestWrapper;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.felix.scr.annotations.Activate;
@@ -117,7 +121,7 @@
             policy = ReferencePolicy.DYNAMIC)
 })
 @SuppressWarnings("deprecation")
-public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFactory {
+public class ModelAdapterFactory implements AdapterFactory, Runnable, ModelFactory, ServletRequestListener {
 
     // hard code this value since we always know exactly how many there are
     private static final int VALUE_PREPARERS_COUNT = 2;
@@ -147,6 +151,8 @@ private void onDisposed() {
 
     private ConcurrentMap<java.lang.ref.Reference<Object>, DisposalCallbackRegistryImpl> disposalCallbacks;
 
+    private ConcurrentHashMap<ServletRequest, DisposalCallbackRegistryImpl> requestDisposalCallbacks;
+
     @Override
     public void run() {
         clearDisposalCallbackRegistryQueue();
@@ -216,6 +222,8 @@ private void clearDisposalCallbackRegistryQueue() {
 
     private ServiceRegistration configPrinterRegistration;
 
+    private ServiceRegistration servletRequestListenerRegistration;
+
     // Use threadlocal to count recursive invocations and break recursing if a max. limit is reached (to avoid cyclic dependencies)
     private ThreadLocal<ThreadInvocationCounter> invocationCountThreadLocal;
 
@@ -576,7 +584,11 @@ private <ModelType> Result<InvocationHandler> createInvocationHandler(final Obje
         MapBackedInvocationHandler handler = new MapBackedInvocationHandler(methods);
 
         DisposalCallbackRegistryImpl registry = new DisposalCallbackRegistryImpl();
-        registerCallbackRegistry(handler, registry);
+        if (adaptable instanceof ServletRequest) {
+            registerRequestCallbackRegistry((ServletRequest) adaptable, registry);
+        } else {
+            registerCallbackRegistry(handler, registry);
+        }
 
         final Map<ValuePreparer, Object> preparedValues = new HashMap<>(VALUE_PREPARERS_COUNT);
 
@@ -599,6 +611,18 @@ private void registerCallbackRegistry(Object object, DisposalCallbackRegistryImp
         disposalCallbacks.put(reference, registry);
     }
 
+    private void registerRequestCallbackRegistry(ServletRequest request, DisposalCallbackRegistryImpl registry) {
+        request = unwrapRequest(request);
+        requestDisposalCallbacks.put(request, registry);
+    }
+
+    private static ServletRequest unwrapRequest(ServletRequest request) {
+        while (request instanceof ServletRequestWrapper) {
+            request = ((ServletRequestWrapper) request).getRequest();
+        }
+        return request;
+    }
+
     private <ModelType> Result<ModelType> createObject(final Object adaptable, final ModelClass<ModelType> modelClass)
             throws InstantiationException, InvocationTargetException, IllegalAccessException {
         DisposalCallbackRegistryImpl registry = new DisposalCallbackRegistryImpl();
@@ -637,7 +661,11 @@ private <ModelType> Result<ModelType> createObject(final Object adaptable, final
             }
         }
 
-        registerCallbackRegistry(object, registry);
+        if (adaptable instanceof SlingHttpServletRequest) {
+            registerRequestCallbackRegistry((SlingHttpServletRequest) adaptable, registry);
+        } else {
+            registerCallbackRegistry(object, registry);
+        }
 
         InjectCallback callback = new SetFieldCallback(object);
 
@@ -1025,6 +1053,7 @@ protected ThreadInvocationCounter initialValue() {
         BundleContext bundleContext = ctx.getBundleContext();
         this.queue = new ReferenceQueue<>();
         this.disposalCallbacks = new ConcurrentHashMap<>();
+        this.requestDisposalCallbacks = new ConcurrentHashMap<>();
         Hashtable<Object, Object> properties = new Hashtable<>();
         properties.put(Constants.SERVICE_VENDOR, "Apache Software Foundation");
         properties.put(Constants.SERVICE_DESCRIPTION, "Sling Models OSGi Service Disposal Job");
@@ -1046,11 +1075,22 @@ protected ThreadInvocationCounter initialValue() {
 
         this.configPrinterRegistration = bundleContext.registerService(Object.class.getName(),
                 new ModelConfigurationPrinter(this, bundleContext, adapterImplementations), printerProps);
+
+        Hashtable<Object, Object> listenerProps = new Hashtable<>();
+        listenerProps.put("osgi.http.whiteboard.context.select", "(osgi.http.whiteboard.context.name=*)");
+        listenerProps.put("osgi.http.whiteboard.listener", "true");
+        this.servletRequestListenerRegistration = bundleContext.registerService(ServletRequestListener.class.getName(),
+                this, listenerProps);
+
     }
 
     @Deactivate
     protected void deactivate() {
         this.adapterCache = null;
+        for (final DisposalCallbackRegistryImpl requestRegistries : this.requestDisposalCallbacks.values()) {
+            requestRegistries.onDisposed();
+        }
+        this.requestDisposalCallbacks = null;
         this.clearDisposalCallbackRegistryQueue();
         this.listener.unregisterAll();
         this.adapterImplementations.removeAll();
@@ -1062,6 +1102,10 @@ protected void deactivate() {
             configPrinterRegistration.unregister();
             configPrinterRegistration = null;
         }
+        if (servletRequestListenerRegistration != null) {
+            servletRequestListenerRegistration.unregister();
+            servletRequestListenerRegistration = null;
+        }
     }
 
     protected void bindInjector(final Injector injector, final Map<String, Object> props) {
@@ -1256,4 +1300,16 @@ public <T> T getModelFromWrappedRequest(@Nonnull SlingHttpServletRequest request
                 scriptEngineFactory, bindingsValuesProvidersByContext).adaptTo(targetClass);
     }
 
+    @Override
+    public void requestDestroyed(ServletRequestEvent sre) {
+        ServletRequest request = unwrapRequest(sre.getServletRequest());
+        DisposalCallbackRegistryImpl registry = requestDisposalCallbacks.remove(request);
+        if (registry != null) {
+            registry.onDisposed();
+        }
+    }
+
+    @Override
+    public void requestInitialized(ServletRequestEvent sre) {
+    }
 }

src/test/java/org/apache/sling/models/impl/OSGiInjectionTest.java

@@ -55,6 +55,8 @@
 import org.osgi.framework.ServiceReference;
 import org.osgi.service.component.ComponentContext;
 
+import javax.servlet.ServletRequestListener;
+
 @RunWith(MockitoJUnitRunner.class)
 public class OSGiInjectionTest {
     private ModelAdapterFactory factory;
@@ -219,6 +221,7 @@ public void testSetOSGiModelField() throws Exception {
         verify(bundleContext).registerService(eq(Runnable.class.getName()), eq(factory), any(Dictionary.class));
         verify(bundleContext).addBundleListener(any(BundleListener.class));
         verify(bundleContext).registerService(eq(Object.class.getName()), any(Object.class), any(Dictionary.class));
+        verify(bundleContext).registerService(eq(ServletRequestListener.class.getName()), eq(factory), any(Dictionary.class));
         verify(bundleContext).getBundles();
         verify(bundleContext).getBundle();
         verifyNoMoreInteractions(res, bundleContext);