You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@cxf.apache.org by am...@apache.org on 2020/01/16 19:46:49 UTC
[cxf] 03/03: CXF-8188: Ensure CreationalContexts are released
This is an automated email from the ASF dual-hosted git repository.
amccright pushed a commit to branch 3.3.x-fixes
in repository https://gitbox.apache.org/repos/asf/cxf.git
commit 90f626ca32946511583975d5ea44ba8e852a71ff
Author: Andy McCright <j....@gmail.com>
AuthorDate: Tue Jan 14 16:46:56 2020 -0600
CXF-8188: Ensure CreationalContexts are released
Code review comment - releasing contexts when the client
proxy is closed.
Signed-off-by: Andy McCright <j....@gmail.com>
---
.../cxf/microprofile/client/cdi/CDIFacade.java | 6 +-
.../cxf/microprofile/client/cdi/CDIUtils.java | 21 +++---
.../cxf/microprofile/client/cdi/Instance.java | 39 ++++++++++++
.../client/proxy/MicroProfileClientProxyImpl.java | 21 +++++-
.../microprofile/client/cdi/RestClientCdiTest.java | 74 ++++++++++++++++++++++
5 files changed, 146 insertions(+), 15 deletions(-)
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIFacade.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIFacade.java
index dfa3cbb..0f18678 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIFacade.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIFacade.java
@@ -56,17 +56,17 @@ public final class CDIFacade {
}
}
- public static <T> Optional<T> getInstanceFromCDI(Class<T> clazz, Bus b) {
+ public static <T> Optional<Instance<T>> getInstanceFromCDI(Class<T> clazz, Bus b) {
return nullableOptional(() -> CDIUtils.getInstanceFromCDI(clazz, b));
}
- public static <T> Optional<T> getInstanceFromCDI(Class<T> clazz) {
+ public static <T> Optional<Instance<T>> getInstanceFromCDI(Class<T> clazz) {
return nullableOptional(() -> CDIUtils.getInstanceFromCDI(clazz));
}
private static <T> Optional<T> nullableOptional(Callable<T> callable) {
if (!CDI_AVAILABLE) {
- return Optional.ofNullable(null);
+ return Optional.empty();
}
T t;
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIUtils.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIUtils.java
index 733bc83..095fbe6 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIUtils.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/CDIUtils.java
@@ -48,30 +48,31 @@ public final class CDIUtils {
}
- static <T> T getInstanceFromCDI(Class<T> clazz) {
+ static <T> Instance<T> getInstanceFromCDI(Class<T> clazz) {
return getInstanceFromCDI(clazz, null);
}
-
- static <T> T getInstanceFromCDI(Class<T> clazz, Bus bus) {
- T t;
+
+ static <T> Instance<T> getInstanceFromCDI(Class<T> clazz, Bus bus) {
+ Instance<T> instance;
try {
- t = findBean(clazz, bus);
+ instance = findBean(clazz, bus);
} catch (ExceptionInInitializerError | NoClassDefFoundError | IllegalStateException ex) {
// expected if no CDI implementation is available
- t = null;
+ instance = null;
} catch (NoSuchElementException ex) {
// expected if ClientHeadersFactory is not managed by CDI
- t = null;
+ instance = null;
}
- return t;
+ return instance;
}
@SuppressWarnings("unchecked")
- private static <T> T findBean(Class<T> clazz, Bus bus) {
+ private static <T> Instance<T> findBean(Class<T> clazz, Bus bus) {
BeanManager beanManager = bus == null ? getCurrentBeanManager() : getCurrentBeanManager(bus);
Bean<?> bean = beanManager.getBeans(clazz).iterator().next();
CreationalContext<?> ctx = beanManager.createCreationalContext(bean);
- T instance = (T) beanManager.getReference(bean, clazz, ctx);
+ Instance<T> instance = new Instance<>((T) beanManager.getReference(bean, clazz, ctx),
+ beanManager.isNormalScope(bean.getScope()) ? () -> { } : ctx::release);
return instance;
}
}
\ No newline at end of file
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/Instance.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/Instance.java
new file mode 100644
index 0000000..b083517
--- /dev/null
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/cdi/Instance.java
@@ -0,0 +1,39 @@
+/**
+ * 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.microprofile.client.cdi;
+
+public class Instance<T> {
+
+ private final T value;
+ private final Runnable releaseMethod;
+
+ Instance(T value, Runnable releaseMethod) {
+ this.value = value;
+ this.releaseMethod = releaseMethod;
+ }
+
+ public T getValue() {
+ return value;
+ }
+
+ public void release() {
+ releaseMethod.run();
+ }
+}
\ No newline at end of file
diff --git a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/proxy/MicroProfileClientProxyImpl.java b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/proxy/MicroProfileClientProxyImpl.java
index b53e34d..f9ba974 100644
--- a/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/proxy/MicroProfileClientProxyImpl.java
+++ b/rt/rs/microprofile-client/src/main/java/org/apache/cxf/microprofile/client/proxy/MicroProfileClientProxyImpl.java
@@ -25,6 +25,7 @@ import java.lang.reflect.Type;
import java.net.URI;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.LinkedList;
import java.util.List;
import java.util.Map;
import java.util.WeakHashMap;
@@ -62,6 +63,7 @@ import org.apache.cxf.microprofile.client.MPRestClientCallback;
import org.apache.cxf.microprofile.client.MicroProfileClientProviderFactory;
import org.apache.cxf.microprofile.client.cdi.CDIFacade;
import org.apache.cxf.microprofile.client.cdi.CDIInterceptorWrapper;
+import org.apache.cxf.microprofile.client.cdi.Instance;
import org.eclipse.microprofile.rest.client.annotation.ClientHeaderParam;
import org.eclipse.microprofile.rest.client.annotation.RegisterClientHeaders;
import org.eclipse.microprofile.rest.client.ext.ClientHeadersFactory;
@@ -102,6 +104,7 @@ public class MicroProfileClientProxyImpl extends ClientProxyImpl {
private Object objectInstance;
private Map<Class<ClientHeadersFactory>, ProviderInfo<ClientHeadersFactory>> clientHeaderFactories =
new WeakHashMap<>();
+ private List<Instance<?>> cdiInstances = new LinkedList<>();
//CHECKSTYLE:OFF
public MicroProfileClientProxyImpl(URI baseURI, ClassLoader loader, ClassResourceInfo cri,
@@ -425,6 +428,11 @@ public class MicroProfileClientProxyImpl extends ClientProxyImpl {
}
}
+ private ClientHeadersFactory mapClientHeadersInstance(Instance<ClientHeadersFactory> instance) {
+ cdiInstances.add(instance);
+ return instance.getValue();
+ }
+
private void mergeHeaders(Class<ClientHeadersFactory> factoryCls, MultivaluedMap<String, String> existingHeaders) {
try {
@@ -435,13 +443,16 @@ public class MicroProfileClientProxyImpl extends ClientProxyImpl {
if (m != null) {
factory = CDIFacade.getInstanceFromCDI(factoryCls, m.getExchange().getBus())
- .orElse(factoryCls.newInstance());
+ .map(this::mapClientHeadersInstance)
+ .orElse(factoryCls.newInstance());
ProviderInfo<ClientHeadersFactory> pi = clientHeaderFactories.computeIfAbsent(factoryCls, k -> {
return new ProviderInfo<ClientHeadersFactory>(factory, m.getExchange().getBus(), true);
});
InjectionUtils.injectContexts(factory, pi, m);
} else {
- factory = CDIFacade.getInstanceFromCDI(factoryCls).orElse(factoryCls.newInstance());
+ factory = CDIFacade.getInstanceFromCDI(factoryCls)
+ .map(this::mapClientHeadersInstance)
+ .orElse(factoryCls.newInstance());
}
MultivaluedMap<String, String> updatedHeaders = factory.update(getJaxrsHeaders(m), existingHeaders);
@@ -512,4 +523,10 @@ public class MicroProfileClientProxyImpl extends ClientProxyImpl {
}
return headers;
}
+
+ @Override
+ public void close() {
+ cdiInstances.forEach(Instance::release);
+ super.close();
+ }
}
diff --git a/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/cdi/RestClientCdiTest.java b/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/cdi/RestClientCdiTest.java
index e2126d4..019f894 100644
--- a/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/cdi/RestClientCdiTest.java
+++ b/rt/rs/microprofile-client/src/test/java/org/apache/cxf/microprofile/client/cdi/RestClientCdiTest.java
@@ -18,16 +18,24 @@
*/
package org.apache.cxf.microprofile.client.cdi;
+import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import javax.enterprise.context.ApplicationScoped;
+import javax.enterprise.context.NormalScope;
+import javax.enterprise.context.spi.CreationalContext;
+import javax.enterprise.inject.spi.Bean;
import javax.enterprise.inject.spi.BeanManager;
import javax.ws.rs.Consumes;
import javax.ws.rs.Path;
import javax.ws.rs.Priorities;
import javax.ws.rs.Produces;
+import org.apache.cxf.Bus;
+import org.apache.cxf.bus.extension.ExtensionManagerBus;
import org.apache.cxf.microprofile.client.mock.HighPriorityClientReqFilter;
import org.apache.cxf.microprofile.client.mock.InvokedMethodClientRequestFilter;
import org.apache.cxf.microprofile.client.mock.LowPriorityClientReqFilter;
@@ -77,5 +85,71 @@ public class RestClientCdiTest {
assertEquals(3, (int) priorities.get(LowPriorityClientReqFilter.class));
assertEquals(10, (int) priorities.get(HighPriorityClientReqFilter.class));
assertEquals(Priorities.USER, (int) priorities.get(InvokedMethodClientRequestFilter.class));
+
+ control.verify();
+ }
+
+ @Test
+ @SuppressWarnings({"unchecked", "rawTypes"})
+ public void testCreationalContextsReleasedOnClientClose() throws Exception {
+ IMocksControl control = EasyMock.createStrictControl();
+ BeanManager mockedBeanMgr = control.createMock(BeanManager.class);
+ CreationalContext<?> mockedCreationalCtx = control.createMock(CreationalContext.class);
+ Bean<?> mockedBean = control.createMock(Bean.class);
+ List<String> stringList = new ArrayList<>(Collections.singleton("abc"));
+
+ EasyMock.expect(mockedBeanMgr.getBeans(List.class))
+ .andReturn(Collections.singleton(mockedBean));
+ EasyMock.expect(mockedBeanMgr.createCreationalContext(mockedBean))
+ .andReturn((CreationalContext) mockedCreationalCtx);
+ EasyMock.expect(mockedBeanMgr.getReference(mockedBean, List.class, mockedCreationalCtx))
+ .andReturn(stringList);
+ EasyMock.expect(mockedBean.getScope())
+ .andReturn((Class) ApplicationScoped.class);
+ EasyMock.expect(mockedBeanMgr.isNormalScope(ApplicationScoped.class))
+ .andReturn(false);
+ mockedCreationalCtx.release();
+ EasyMock.expectLastCall().once();
+ control.replay();
+
+ Bus bus = new ExtensionManagerBus();
+ bus.setExtension(mockedBeanMgr, BeanManager.class);
+
+ Instance<List> i = CDIUtils.getInstanceFromCDI(List.class, bus);
+ assertEquals(stringList, i.getValue());
+ i.release();
+
+ control.verify();
+ }
+
+ @Test
+ @SuppressWarnings({"unchecked", "rawTypes"})
+ public void testCreationalContextsNotReleasedOnClientCloseUsingNormalScope() throws Exception {
+ IMocksControl control = EasyMock.createStrictControl();
+ BeanManager mockedBeanMgr = control.createMock(BeanManager.class);
+ CreationalContext<?> mockedCreationalCtx = control.createMock(CreationalContext.class);
+ Bean<?> mockedBean = control.createMock(Bean.class);
+ List<String> stringList = new ArrayList<>(Collections.singleton("xyz"));
+
+ EasyMock.expect(mockedBeanMgr.getBeans(List.class))
+ .andReturn(Collections.singleton(mockedBean));
+ EasyMock.expect(mockedBeanMgr.createCreationalContext(mockedBean))
+ .andReturn((CreationalContext) mockedCreationalCtx);
+ EasyMock.expect(mockedBeanMgr.getReference(mockedBean, List.class, mockedCreationalCtx))
+ .andReturn(stringList);
+ EasyMock.expect(mockedBean.getScope())
+ .andReturn((Class) NormalScope.class);
+ EasyMock.expect(mockedBeanMgr.isNormalScope(NormalScope.class))
+ .andReturn(true);
+ control.replay();
+
+ Bus bus = new ExtensionManagerBus();
+ bus.setExtension(mockedBeanMgr, BeanManager.class);
+
+ Instance<List> i = CDIUtils.getInstanceFromCDI(List.class, bus);
+
+ i.release();
+
+ control.verify();
}
}
\ No newline at end of file