Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

SLING-5668 - Leverage ServletRequestListener.requestDestroyed for cal… #1

Merged
merged 2 commits into from Dec 8, 2017
Merged

SLING-5668 - Leverage ServletRequestListener.requestDestroyed for cal… #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef36bed
SLING-5668 - Leverage ServletRequestListener.requestDestroyed for cal…
justinedelson Dec 7, 2017
e11c225
SLING-5668 - use DS for listener, OSGi constants from Whiteboard, and…
justinedelson Dec 7, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Expand Up @@ -15,3 +15,4 @@ maven-eclipse.xml
.DS_Store
jcr.log
atlassian-ide-plugin.xml
dependency-reduced-pom.xml
9 changes: 5 additions & 4 deletions pom.xml
View file
Open in desktop
Expand Up @@ -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>
Expand All @@ -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>
Expand Down Expand Up @@ -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>
Expand Down
68 changes: 60 additions & 8 deletions src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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",
Expand All @@ -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;
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -576,7 +588,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);

Expand All @@ -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();
Expand Down Expand Up @@ -637,7 +665,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);

Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -1256,4 +1296,16 @@ public <T> T getModelFromWrappedRequest(@Nonnull SlingHttpServletRequest request
scriptEngineFactory, bindingsValuesProvidersByContext).adaptTo(targetClass);
}

@Override
public void requestDestroyed(ServletRequestEvent sre) {
ServletRequest request = unwrapRequest(sre.getServletRequest());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also find it weird, that this event does still contained a wrapped servlet request. But unwrapping does not do any harm (in no case).

DisposalCallbackRegistryImpl registry = requestDisposalCallbacks.remove(request);
if (registry != null) {
registry.onDisposed();
}
}

@Override
public void requestInitialized(ServletRequestEvent sre) {
}
}
28 changes: 13 additions & 15 deletions src/main/java/org/apache/sling/models/impl/ModelPackageBundleListener.java
View file
Open in desktop
Expand Up @@ -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";
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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);

Expand Down
6 changes: 4 additions & 2 deletions src/test/java/org/apache/sling/models/impl/OSGiInjectionTest.java
View file
Open in desktop
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down