reta closed pull request #418: CXF-7742 handling scopes in CdiResourceProvider
URL: https://github.com/apache/cxf/pull/418
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/integration/cdi/src/main/java/org/apache/cxf/cdi/JAXRSCdiResourceExtension.java
b/integration/cdi/src/main/java/org/apache/cxf/cdi/JAXRSCdiResourceExtension.java
index d2b76dbd5cd..9c5b48c9e31 100644
---
a/integration/cdi/src/main/java/org/apache/cxf/cdi/JAXRSCdiResourceExtension.java
+++
b/integration/cdi/src/main/java/org/apache/cxf/cdi/JAXRSCdiResourceExtension.java
@@ -29,6 +29,7 @@
import java.util.ServiceLoader;
import java.util.Set;
+import javax.enterprise.context.Dependent;
import javax.enterprise.context.spi.CreationalContext;
import javax.enterprise.event.Observes;
import javax.enterprise.inject.spi.AfterBeanDiscovery;
@@ -45,6 +46,7 @@
import javax.enterprise.inject.spi.ProcessProducerField;
import javax.enterprise.inject.spi.ProcessProducerMethod;
import javax.enterprise.inject.spi.WithAnnotations;
+import javax.inject.Singleton;
import javax.ws.rs.ApplicationPath;
import javax.ws.rs.Path;
import javax.ws.rs.core.Application;
@@ -60,6 +62,7 @@
import org.apache.cxf.jaxrs.JAXRSServerFactoryBean;
import org.apache.cxf.jaxrs.ext.ContextClassProvider;
import org.apache.cxf.jaxrs.ext.JAXRSServerFactoryCustomizationExtension;
+import org.apache.cxf.jaxrs.lifecycle.ResourceProvider;
import org.apache.cxf.jaxrs.provider.ServerConfigurableFactory;
import org.apache.cxf.jaxrs.utils.InjectionUtils;
import org.apache.cxf.jaxrs.utils.JAXRSServerFactoryCustomizationUtils;
@@ -82,6 +85,8 @@
private final List< Bean< ? extends Feature > > featureBeans = new
ArrayList< Bean< ? extends Feature > >();
private final List< CreationalContext< ? > > disposableCreationalContexts =
new ArrayList< CreationalContext< ? > >();
+ private final List< Lifecycle > disposableLifecycles =
+ new ArrayList<>();
private final Set< Type > contextTypes = new LinkedHashSet<>();
private final Collection< String > existingStandardClasses = new
HashSet<>();
@@ -93,7 +98,7 @@
private static class ClassifiedClasses {
private List< Object > providers = new ArrayList<>();
private List< Feature > features = new ArrayList<>();
- private List< CdiResourceProvider > resourceProviders = new
ArrayList<>();
+ private List<ResourceProvider> resourceProviders = new ArrayList<>();
public void addProviders(final Collection< Object > others) {
this.providers.addAll(others);
@@ -103,7 +108,7 @@ public void addFeatures(final Collection< Feature > others)
{
this.features.addAll(others);
}
- public void addResourceProvider(final CdiResourceProvider other) {
+ public void addResourceProvider(final ResourceProvider other) {
this.resourceProviders.add(other);
}
@@ -115,7 +120,7 @@ public void addResourceProvider(final CdiResourceProvider
other) {
return features;
}
- public List<CdiResourceProvider> getResourceProviders() {
+ public List<ResourceProvider> getResourceProviders() {
return resourceProviders;
}
}
@@ -303,7 +308,11 @@ public void release(@Observes final BeforeShutdown event) {
for (final CreationalContext<?> disposableCreationalContext:
disposableCreationalContexts) {
disposableCreationalContext.release();
}
+ disposableCreationalContexts.clear();
}
+
+ disposableLifecycles.forEach(Lifecycle::destroy);
+ disposableLifecycles.clear();
}
private Class<?> toClass(String name) {
@@ -347,7 +356,7 @@ private JAXRSServerFactoryBean createFactoryInstance(final
Application applicati
instance.setProviders(classified.getProviders());
instance.getFeatures().addAll(classified.getFeatures());
- for (final CdiResourceProvider resourceProvider:
classified.getResourceProviders()) {
+ for (final ResourceProvider resourceProvider:
classified.getResourceProviders()) {
instance.setResourceProvider(resourceProvider.getResourceClass(),
resourceProvider);
}
@@ -372,7 +381,28 @@ private ClassifiedClasses classes2singletons(final
Application application, fina
for (final Bean< ? > bean: serviceBeans) {
if (classes.contains(bean.getBeanClass())) {
- classified.addResourceProvider(new
CdiResourceProvider(beanManager, bean));
+ // normal scoped beans will return us a proxy in
getInstance so it is singletons for us,
+ // @Singleton is indeed a singleton
+ // @Dependent should be a request scoped instance but for
backward compat we kept it a singleton
+ //
+ // other scopes are considered request scoped (for jaxrs)
+ // and are created per request
(getInstance/releaseInstance)
+ final ResourceProvider resourceProvider;
+ if (isCxfSingleton(beanManager, bean)) {
+ final Lifecycle lifecycle = new Lifecycle(beanManager,
bean);
+ resourceProvider = new
SingletonResourceProvider(lifecycle, bean.getBeanClass());
+
+ // if not a singleton we manage it per request
+ // if @Singleton the container handles it
+ // so we only need this case here
+ if (Dependent.class == bean.getScope()) {
+ disposableLifecycles.add(lifecycle);
+ }
+ } else {
+ resourceProvider = new PerRequestResourceProvider(
+ () -> new Lifecycle(beanManager, bean),
bean.getBeanClass());
+ }
+ classified.addResourceProvider(resourceProvider);
}
}
}
@@ -380,6 +410,15 @@ private ClassifiedClasses classes2singletons(final
Application application, fina
return classified;
}
+ boolean isCxfSingleton(final BeanManager beanManager, final Bean<?> bean) {
+ return beanManager.isNormalScope(bean.getScope()) ||
isConsideredSingleton(bean.getScope());
+ }
+
+ // warn: several impls use @Dependent == request so we should probably add
a flag
+ private boolean isConsideredSingleton(final Class<?> scope) {
+ return Singleton.class == scope || Dependent.class == scope;
+ }
+
/**
* Load external providers from service loader
* @return loaded external providers
diff --git
a/integration/cdi/src/main/java/org/apache/cxf/cdi/CdiResourceProvider.java
b/integration/cdi/src/main/java/org/apache/cxf/cdi/Lifecycle.java
similarity index 59%
rename from
integration/cdi/src/main/java/org/apache/cxf/cdi/CdiResourceProvider.java
rename to integration/cdi/src/main/java/org/apache/cxf/cdi/Lifecycle.java
index 68d3b2b75a9..089b25a94f3 100644
--- a/integration/cdi/src/main/java/org/apache/cxf/cdi/CdiResourceProvider.java
+++ b/integration/cdi/src/main/java/org/apache/cxf/cdi/Lifecycle.java
@@ -22,46 +22,28 @@
import javax.enterprise.inject.spi.Bean;
import javax.enterprise.inject.spi.BeanManager;
-import org.apache.cxf.jaxrs.lifecycle.ResourceProvider;
-import org.apache.cxf.message.Message;
-
-public class CdiResourceProvider implements ResourceProvider {
- private Object instance;
- private CreationalContext< ? > context;
-
+class Lifecycle {
private final BeanManager beanManager;
- private final Bean< ? > bean;
+ private final Bean<?> bean;
+ private Object instance;
+ private CreationalContext<?> context;
- CdiResourceProvider(final BeanManager beanManager, final Bean< ? > bean) {
+ Lifecycle(final BeanManager beanManager, final Bean<?> bean) {
this.beanManager = beanManager;
this.bean = bean;
}
- @Override
- public Object getInstance(Message m) {
- if (instance == null) {
- context = beanManager.createCreationalContext(bean);
- instance = beanManager.getReference(bean, bean.getBeanClass(),
context);
- }
-
+ Object create() {
+ context = beanManager.createCreationalContext(bean);
+ instance = beanManager.getReference(bean, bean.getBeanClass(),
context);
return instance;
}
- @Override
- public void releaseInstance(Message m, Object o) {
+ void destroy() {
if (context != null) {
context.release();
instance = null;
+ context = null;
}
}
-
- @Override
- public Class<?> getResourceClass() {
- return bean.getBeanClass();
- }
-
- @Override
- public boolean isSingleton() {
- return !beanManager.isNormalScope(bean.getScope());
- }
-}
+}
\ No newline at end of file
diff --git
a/integration/cdi/src/main/java/org/apache/cxf/cdi/PerRequestResourceProvider.java
b/integration/cdi/src/main/java/org/apache/cxf/cdi/PerRequestResourceProvider.java
new file mode 100644
index 00000000000..127bf3e102e
--- /dev/null
+++
b/integration/cdi/src/main/java/org/apache/cxf/cdi/PerRequestResourceProvider.java
@@ -0,0 +1,59 @@
+/**
+ * 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.cxf.cdi;
+
+import java.util.function.Supplier;
+
+import org.apache.cxf.jaxrs.lifecycle.ResourceProvider;
+import org.apache.cxf.message.Message;
+
+public class PerRequestResourceProvider implements ResourceProvider {
+ private final Supplier<Lifecycle> lifecycle;
+ private final Class<?> clazz;
+
+ PerRequestResourceProvider(final Supplier<Lifecycle> lifecycle, final
Class<?> clazz) {
+ this.lifecycle = lifecycle;
+ this.clazz = clazz;
+ }
+
+ @Override
+ public Object getInstance(final Message m) {
+ final Lifecycle currentLifecycle = this.lifecycle.get();
+ m.put(Lifecycle.class, currentLifecycle);
+ return currentLifecycle.create();
+ }
+
+ @Override
+ public void releaseInstance(final Message m, final Object o) {
+ final Lifecycle contextualLifecycle = m.get(Lifecycle.class);
+ if (contextualLifecycle != null) {
+ contextualLifecycle.destroy();
+ }
+ }
+
+ @Override
+ public Class<?> getResourceClass() {
+ return clazz;
+ }
+
+ @Override
+ public boolean isSingleton() {
+ return false;
+ }
+}
diff --git
a/integration/cdi/src/main/java/org/apache/cxf/cdi/SingletonResourceProvider.java
b/integration/cdi/src/main/java/org/apache/cxf/cdi/SingletonResourceProvider.java
new file mode 100644
index 00000000000..8b7e02f0fa3
--- /dev/null
+++
b/integration/cdi/src/main/java/org/apache/cxf/cdi/SingletonResourceProvider.java
@@ -0,0 +1,52 @@
+/**
+ * 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.cxf.cdi;
+
+import org.apache.cxf.jaxrs.lifecycle.ResourceProvider;
+import org.apache.cxf.message.Message;
+
+public class SingletonResourceProvider implements ResourceProvider {
+ private final Class<?> clazz;
+ private Object instance;
+
+ SingletonResourceProvider(final Lifecycle lifecycle, final Class<?> clazz)
{
+ this.instance = lifecycle.create();
+ this.clazz = clazz;
+ }
+
+ @Override
+ public Object getInstance(final Message m) {
+ return instance;
+ }
+
+ @Override
+ public void releaseInstance(final Message m, final Object o) {
+ // no-op
+ }
+
+ @Override
+ public Class<?> getResourceClass() {
+ return clazz;
+ }
+
+ @Override
+ public boolean isSingleton() {
+ return true;
+ }
+}
diff --git
a/integration/cdi/src/test/java/org/apache/cxf/cdi/CdiResourceProviderTest.java
b/integration/cdi/src/test/java/org/apache/cxf/cdi/CdiResourceProviderTest.java
new file mode 100644
index 00000000000..3c471980e35
--- /dev/null
+++
b/integration/cdi/src/test/java/org/apache/cxf/cdi/CdiResourceProviderTest.java
@@ -0,0 +1,125 @@
+/**
+ * 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.cxf.cdi;
+
+import java.lang.annotation.Documented;
+
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.context.NormalScope;
+import javax.enterprise.context.RequestScoped;
+import javax.enterprise.inject.spi.Bean;
+import javax.enterprise.inject.spi.BeanManager;
+import javax.inject.Singleton;
+
+import org.apache.cxf.jaxrs.lifecycle.ResourceProvider;
+import org.apache.cxf.message.MessageImpl;
+
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.mockito.Mock;
+import org.mockito.junit.MockitoJUnitRunner;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.when;
+
+@SuppressWarnings({"rawtypes", "unchecked"})
+@RunWith(MockitoJUnitRunner.class)
+public class CdiResourceProviderTest {
+ @Mock
+ private BeanManager beanManager;
+
+ @Mock
+ private Bean<?> bean;
+
+ @Before
+ public void setup() {
+ final Class beanClass = Object.class;
+ when(bean.getBeanClass()).thenReturn(beanClass);
+ when(beanManager.isNormalScope(any())).thenAnswer(invocationOnMock ->
+
Class.class.cast(invocationOnMock.getArguments()[0]).isAnnotationPresent(NormalScope.class));
+ when(beanManager.getReference(eq(bean), eq(beanClass), any()))
+ .thenAnswer(i -> new Object()); // ensure to create one
instance per call, this is what we test
+ }
+
+ @Test
+ public void normalScoped() {
+
when(bean.getScope()).thenReturn(Class.class.cast(ApplicationScoped.class));
+ assertSingleton();
+ }
+
+ @Test
+ public void singleton() {
+ when(bean.getScope()).thenReturn(Class.class.cast(Singleton.class));
+ assertSingleton();
+ }
+
+ @Test
+ public void dependent() {
+ when(bean.getScope()).thenReturn(Class.class.cast(Singleton.class));
+ assertSingleton();
+ }
+
+ @Test
+ public void requestScoped() {
+
when(bean.getScope()).thenReturn(Class.class.cast(RequestScoped.class));
+ assertSingleton(); // yes, this is a singleton since we look up the
proxy
+ }
+
+ @Test
+ public void perRequest() {
+ // not a scope so will be considered as a not singleton bean
+ when(bean.getScope()).thenReturn(Class.class.cast(Documented.class));
+ assertNotSingleton();
+ }
+
+ private void assertNotSingleton() {
+ final ResourceProvider provider = new PerRequestResourceProvider(
+ () -> new Lifecycle(beanManager, bean), Object.class);
+ assertFalse(new
JAXRSCdiResourceExtension().isCxfSingleton(beanManager, bean));
+ assertFalse(provider.isSingleton());
+
+ final MessageImpl message1 = new MessageImpl();
+ final MessageImpl message2 = new MessageImpl();
+ final Object instance = provider.getInstance(message1);
+ assertNotNull(instance);
+ assertNotEquals(provider.getInstance(message1),
provider.getInstance(message2));
+
+ // ensure we can close the lifecycle
+ final Lifecycle lifecycle1 = message1.get(Lifecycle.class);
+ assertNotNull(lifecycle1);
+ assertNotNull(message2.get(Lifecycle.class));
+ }
+
+ private void assertSingleton() {
+ final ResourceProvider provider = new SingletonResourceProvider(new
Lifecycle(beanManager, bean), Object.class);
+ assertTrue(new JAXRSCdiResourceExtension().isCxfSingleton(beanManager,
bean));
+ assertTrue(provider.isSingleton());
+
+ final Object instance = provider.getInstance(new MessageImpl());
+ assertNotNull(instance);
+ assertEquals(instance, provider.getInstance(new MessageImpl()));
+ }
+}
----------------------------------------------------------------
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:
[email protected]
With regards,
Apache Git Services
