justinedelson closed pull request #1: SLING-5668 - Leverage
ServletRequestListener.requestDestroyed for cal?
URL: https://github.com/apache/sling-org-apache-sling-models-impl/pull/1
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/.gitignore b/.gitignore
index 5b783ed..12746f7 100644
--- a/.gitignore
+++ b/.gitignore
@@ -15,3 +15,4 @@ maven-eclipse.xml
.DS_Store
jcr.log
atlassian-ide-plugin.xml
+dependency-reduced-pom.xml
diff --git a/pom.xml b/pom.xml
index 63270e0..97a29f2 100644
--- a/pom.xml
+++ b/pom.xml
@@ -142,8 +142,8 @@
</dependency>
<dependency>
<groupId>org.osgi</groupId>
- <artifactId>org.osgi.compendium</artifactId>
- <version>4.2.0</version>
+ <artifactId>osgi.cmpn</artifactId>
+ <version>6.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -154,7 +154,8 @@
</dependency>
<dependency>
<groupId>org.osgi</groupId>
- <artifactId>org.osgi.core</artifactId>
+ <artifactId>osgi.core</artifactId>
+ <version>6.0.0</version>
<scope>provided</scope>
</dependency>
<dependency>
@@ -188,7 +189,7 @@
<dependency>
<groupId>org.apache.sling</groupId>
<artifactId>org.apache.sling.testing.osgi-mock</artifactId>
- <version>1.5.0</version>
+ <version>2.3.2</version>
<scope>test</scope>
</dependency>
<dependency>
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 9f21435..1897a51 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
@@ -41,11 +41,16 @@
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;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
+import org.apache.felix.scr.annotations.Properties;
import org.apache.felix.scr.annotations.Property;
import org.apache.felix.scr.annotations.Reference;
import org.apache.felix.scr.annotations.ReferenceCardinality;
@@ -98,12 +103,17 @@
import org.osgi.framework.Constants;
import org.osgi.framework.ServiceRegistration;
import org.osgi.service.component.ComponentContext;
+import org.osgi.service.http.whiteboard.HttpWhiteboardConstants;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@Component(metatype = true, immediate = true,
label = "Apache Sling Model Adapter Factory")
-@Service(value = ModelFactory.class)
+@Service(value = { ModelFactory.class, ServletRequestListener.class })
+@Properties({
+ @Property(name = HttpWhiteboardConstants.HTTP_WHITEBOARD_LISTENER, value =
"true"),
+ @Property(name = HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_SELECT,
value = "(" + HttpWhiteboardConstants.HTTP_WHITEBOARD_CONTEXT_NAME + "=*)")
+})
@References({
@Reference(
name = "injector",
@@ -117,7 +127,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 +157,8 @@ private void onDisposed() {
private ConcurrentMap<java.lang.ref.Reference<Object>,
DisposalCallbackRegistryImpl> disposalCallbacks;
+ private ConcurrentHashMap<ServletRequest, DisposalCallbackRegistryImpl>
requestDisposalCallbacks;
+
@Override
public void run() {
clearDisposalCallbackRegistryQueue();
@@ -576,7 +588,11 @@ RuntimeException injectElement(final InjectableElement
element, final Object ada
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 +615,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 +665,11 @@ private void registerCallbackRegistry(Object object,
DisposalCallbackRegistryImp
}
}
- registerCallbackRegistry(object, registry);
+ if (adaptable instanceof SlingHttpServletRequest) {
+ registerRequestCallbackRegistry((SlingHttpServletRequest)
adaptable, registry);
+ } else {
+ registerCallbackRegistry(object, registry);
+ }
InjectCallback callback = new SetFieldCallback(object);
@@ -1025,32 +1057,40 @@ protected ThreadInvocationCounter initialValue() {
BundleContext bundleContext = ctx.getBundleContext();
this.queue = new ReferenceQueue<>();
this.disposalCallbacks = new ConcurrentHashMap<>();
- Hashtable<Object, Object> properties = new Hashtable<>();
+ this.requestDisposalCallbacks = new ConcurrentHashMap<>();
+ 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");
properties.put("scheduler.name", "Sling Models OSGi Service Disposal
Job");
properties.put("scheduler.concurrent", false);
properties.put("scheduler.period",
PropertiesUtil.toLong(props.get(PROP_CLEANUP_JOB_PERIOD),
DEFAULT_CLEANUP_JOB_PERIOD));
- this.jobRegistration =
bundleContext.registerService(Runnable.class.getName(), this, properties);
+ this.jobRegistration = bundleContext.registerService(Runnable.class,
this, properties);
this.scriptEngineFactory = new
SlingModelsScriptEngineFactory(bundleContext.getBundle());
this.listener = new ModelPackageBundleListener(ctx.getBundleContext(),
this, this.adapterImplementations, bindingsValuesProvidersByContext,
scriptEngineFactory);
- Hashtable<Object, Object> printerProps = new Hashtable<>();
+ Hashtable<String, Object> printerProps = new Hashtable<>();
printerProps.put(Constants.SERVICE_VENDOR, "Apache Software
Foundation");
printerProps.put(Constants.SERVICE_DESCRIPTION, "Sling Models
Configuration Printer");
printerProps.put("felix.webconsole.label", "slingmodels");
printerProps.put("felix.webconsole.title", "Sling Models");
printerProps.put("felix.webconsole.configprinter.modes", "always");
- this.configPrinterRegistration =
bundleContext.registerService(Object.class.getName(),
+ this.configPrinterRegistration =
bundleContext.registerService(Object.class,
new ModelConfigurationPrinter(this, bundleContext,
adapterImplementations), printerProps);
+
}
@Deactivate
protected void deactivate() {
this.adapterCache = null;
+ if (this.requestDisposalCallbacks != null) {
+ for (final DisposalCallbackRegistryImpl requestRegistries :
this.requestDisposalCallbacks.values()) {
+ requestRegistries.onDisposed();
+ }
+ }
+ this.requestDisposalCallbacks = null;
this.clearDisposalCallbackRegistryQueue();
this.listener.unregisterAll();
this.adapterImplementations.removeAll();
@@ -1256,4 +1296,16 @@ private Object handleBoundModelResult(Result<?> result) {
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) {
+ }
}
diff --git
a/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
b/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
index 28dcc16..0c285f5 100644
--- a/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
+++ b/src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
@@ -49,7 +49,7 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class ModelPackageBundleListener implements BundleTrackerCustomizer {
+public class ModelPackageBundleListener implements
BundleTrackerCustomizer<ServiceRegistration[]> {
static final String PACKAGE_HEADER = "Sling-Model-Packages";
static final String CLASSES_HEADER = "Sling-Model-Classes";
@@ -96,12 +96,12 @@ public ModelPackageBundleListener(BundleContext
bundleContext,
this.adapterImplementations = adapterImplementations;
this.bindingsValuesProvidersByContext =
bindingsValuesProvidersByContext;
this.scriptEngineFactory = scriptEngineFactory;
- this.bundleTracker = new BundleTracker(bundleContext, Bundle.ACTIVE,
this);
+ this.bundleTracker = new BundleTracker<>(bundleContext, Bundle.ACTIVE,
this);
this.bundleTracker.open();
}
@Override
- public Object addingBundle(Bundle bundle, BundleEvent event) {
+ public ServiceRegistration[] addingBundle(Bundle bundle, BundleEvent
event) {
List<ServiceRegistration> regs = new ArrayList<>();
Dictionary<?, ?> headers = bundle.getHeaders();
@@ -193,23 +193,21 @@ private void analyzeClass(Bundle bundle, String
className, List<ServiceRegistrat
}
@Override
- public void modifiedBundle(Bundle bundle, BundleEvent event, Object
object) {
+ public void modifiedBundle(Bundle bundle, BundleEvent event,
ServiceRegistration[] object) {
}
@Override
- public void removedBundle(Bundle bundle, BundleEvent event, Object object)
{
- if (object instanceof ServiceRegistration[]) {
- for (ServiceRegistration reg : (ServiceRegistration[]) object) {
- ServiceReference ref = reg.getReference();
- String[] adapterTypeNames =
PropertiesUtil.toStringArray(ref.getProperty(AdapterFactory.ADAPTER_CLASSES));
- if (adapterTypeNames != null) {
- String implTypeName =
PropertiesUtil.toString(ref.getProperty(PROP_IMPLEMENTATION_CLASS), null);
- for (String adapterTypeName : adapterTypeNames) {
- adapterImplementations.remove(adapterTypeName,
implTypeName);
- }
+ public void removedBundle(Bundle bundle, BundleEvent event,
ServiceRegistration[] object) {
+ for (ServiceRegistration reg : object) {
+ ServiceReference ref = reg.getReference();
+ String[] adapterTypeNames =
PropertiesUtil.toStringArray(ref.getProperty(AdapterFactory.ADAPTER_CLASSES));
+ if (adapterTypeNames != null) {
+ String implTypeName =
PropertiesUtil.toString(ref.getProperty(PROP_IMPLEMENTATION_CLASS), null);
+ for (String adapterTypeName : adapterTypeNames) {
+ adapterImplementations.remove(adapterTypeName,
implTypeName);
}
- reg.unregister();
}
+ reg.unregister();
}
adapterImplementations.removeResourceTypeBindings(bundle);
diff --git a/src/test/java/org/apache/sling/models/impl/OSGiInjectionTest.java
b/src/test/java/org/apache/sling/models/impl/OSGiInjectionTest.java
index 166545f..ec04a57 100644
--- a/src/test/java/org/apache/sling/models/impl/OSGiInjectionTest.java
+++ b/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;
@@ -216,9 +218,9 @@ public void testSetOSGiModelField() throws Exception {
SetOSGiModel model = factory.getAdapter(res, SetOSGiModel.class);
assertNull(model);
- verify(bundleContext).registerService(eq(Runnable.class.getName()),
eq(factory), any(Dictionary.class));
+ verify(bundleContext).registerService(eq(Runnable.class), 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(Object.class),
any(Object.class), any(Dictionary.class));
verify(bundleContext).getBundles();
verify(bundleContext).getBundle();
verifyNoMoreInteractions(res, bundleContext);
diff --git
a/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
new file mode 100644
index 0000000..c163798
--- /dev/null
+++ b/src/test/java/org/apache/sling/models/impl/RequestDisposalTest.java
@@ -0,0 +1,136 @@
+/*
+ * 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.models.impl;
+
+import org.apache.sling.api.SlingHttpServletRequest;
+import org.apache.sling.api.resource.Resource;
+import org.apache.sling.api.wrappers.SlingHttpServletRequestWrapper;
+import org.apache.sling.models.annotations.Model;
+import org.apache.sling.models.spi.DisposalCallback;
+import org.apache.sling.models.spi.DisposalCallbackRegistry;
+import org.apache.sling.models.spi.Injector;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.runners.MockitoJUnitRunner;
+import org.osgi.framework.BundleContext;
+import org.osgi.service.component.ComponentContext;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.inject.Inject;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletRequestEvent;
+import java.lang.reflect.AnnotatedElement;
+import java.lang.reflect.Type;
+import java.util.Hashtable;
+
+import static org.mockito.Mockito.*;
+import static org.junit.Assert.*;
+
+@RunWith(MockitoJUnitRunner.class)
+public class RequestDisposalTest {
+ @Mock
+ private ComponentContext componentCtx;
+
+ @Mock
+ private BundleContext bundleContext;
+
+ @Mock
+ private Resource resource;
+
+ @Mock
+ private SlingHttpServletRequest request;
+
+ @Mock
+ private ServletContext servletContext;
+
+ private ModelAdapterFactory factory;
+
+ private TestDisposalCallback callback;
+
+ @Before
+ public void setup() {
+ when(componentCtx.getBundleContext()).thenReturn(bundleContext);
+ when(componentCtx.getProperties()).thenReturn(new Hashtable<String,
Object>());
+
+ factory = new ModelAdapterFactory();
+ factory.activate(componentCtx);
+ factory.bindInjector(new DisposedInjector(), new
ServicePropertiesMap(0, 0));
+
factory.adapterImplementations.addClassesAsAdapterAndImplementation(TestModel.class);
+
+ callback = new TestDisposalCallback();
+
+ }
+
+ @Test
+ public void test() {
+ // destroy a wrapper of the root request
+ SlingHttpServletRequest destroyedRequest = new
SlingHttpServletRequestWrapper(request);
+
+ // 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);
+
+ assertFalse(callback.isDisposed());
+
+ factory.requestDestroyed(new ServletRequestEvent(servletContext,
destroyedRequest));
+
+ assertTrue(callback.isDisposed());
+ }
+
+ @Model(adaptables = SlingHttpServletRequest.class)
+ public static class TestModel {
+
+ @Inject
+ public String testString;
+
+ }
+
+ private class DisposedInjector implements Injector {
+ @Nonnull
+ @Override
+ public String getName() {
+ return "disposed";
+ }
+
+ @CheckForNull
+ @Override
+ public Object getValue(@Nonnull Object o, String s, @Nonnull Type
type, @Nonnull AnnotatedElement annotatedElement, @Nonnull
DisposalCallbackRegistry disposalCallbackRegistry) {
+ disposalCallbackRegistry.addDisposalCallback(callback);
+ return "teststring";
+ }
+ }
+
+ private class TestDisposalCallback implements DisposalCallback {
+ private boolean disposed = false;
+
+ @Override
+ public void onDisposed() {
+ disposed = true;
+ }
+
+ public boolean isDisposed() {
+ return disposed;
+ }
+ }
+
+
+}
diff --git
a/src/test/java/org/apache/sling/models/testutil/ModelAdapterFactoryUtil.java
b/src/test/java/org/apache/sling/models/testutil/ModelAdapterFactoryUtil.java
index cca5baa..bd3a133 100644
---
a/src/test/java/org/apache/sling/models/testutil/ModelAdapterFactoryUtil.java
+++
b/src/test/java/org/apache/sling/models/testutil/ModelAdapterFactoryUtil.java
@@ -18,13 +18,17 @@
*/
package org.apache.sling.models.testutil;
+import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.net.MalformedURLException;
import java.net.URL;
+import java.security.cert.X509Certificate;
import java.util.Dictionary;
import java.util.Enumeration;
import java.util.Hashtable;
+import java.util.List;
+import java.util.Map;
import java.util.Vector;
import org.apache.sling.testing.mock.osgi.MockOsgi;
@@ -33,6 +37,7 @@
import org.osgi.framework.BundleEvent;
import org.osgi.framework.BundleException;
import org.osgi.framework.ServiceReference;
+import org.osgi.framework.Version;
/**
* Helper methods for simulating sling models bundle events
@@ -47,7 +52,6 @@ private ModelAdapterFactoryUtil() {
/**
* Scan classpaths for given package name (and sub packages) to scan for
and
* register all classes with @Model annotation.
- * @param packageName Java package name
*/
public static void addModelsForPackage(BundleContext bundleContext,
Class... classes) {
Bundle bundle = new ModelsPackageBundle(classes, Bundle.ACTIVE,
bundleContext);
@@ -81,7 +85,7 @@ public Dictionary getHeaders() {
@Override
public Enumeration findEntries(String path, String filePattern,
boolean recurse) {
- Vector<URL> urls = new Vector<URL>(); // NOPMD
+ Vector<URL> urls = new Vector<>(); // NOPMD
for (int i = 0; i < classes.length; i++) {
try {
urls.add(new URL("file:/" +
classes[i].getName().replace('.', '/') + ".class"));
@@ -198,6 +202,30 @@ public long getLastModified() {
return 0;
}
+ @Override
+ public File getDataFile(String filename) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public <A> A adapt(Class<A> type) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Map<X509Certificate, List<X509Certificate>>
getSignerCertificates(int signersType) {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public Version getVersion() {
+ throw new UnsupportedOperationException();
+ }
+
+ @Override
+ public int compareTo(Bundle o) {
+ throw new UnsupportedOperationException();
+ }
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
With regards,
Apache Git Services
