diff --git a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/ProxyInvocationHandler.java b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/ProxyInvocationHandler.java index 8be8808..40c9a8c 100644 --- a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/ProxyInvocationHandler.java +++ b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/ProxyInvocationHandler.java @@ -23,24 +23,12 @@ import java.lang.reflect.InvocationHandler; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.lang.reflect.ParameterizedType; import java.lang.reflect.Type; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Map; import java.util.Set; import java.util.concurrent.CompletionException; import java.util.concurrent.atomic.AtomicBoolean; -import java.util.stream.Collectors; -import jakarta.enterprise.context.spi.CreationalContext; -import jakarta.enterprise.inject.spi.BeanManager; -import jakarta.enterprise.inject.spi.CDI; -import jakarta.enterprise.inject.spi.InterceptionType; -import jakarta.enterprise.inject.spi.Interceptor; import jakarta.ws.rs.ProcessingException; import jakarta.ws.rs.client.ResponseProcessingException; import jakarta.ws.rs.ext.ParamConverter; @@ -59,29 +47,18 @@ public class ProxyInvocationHandler implements InvocationHandler { private final Set providerInstances; - private final Map> interceptorChains; - private final ResteasyClient client; - private final CreationalContext creationalContext; - private final AtomicBoolean closed; public ProxyInvocationHandler(final Class restClientInterface, final Object target, final Set providerInstances, - final ResteasyClient client, final BeanManager beanManager) { + final ResteasyClient client) { this.target = target; this.providerInstances = providerInstances; this.client = client; this.closed = new AtomicBoolean(); - if (beanManager != null) { - this.creationalContext = beanManager.createCreationalContext(null); - this.interceptorChains = initInterceptorChains(beanManager, creationalContext, restClientInterface); - } else { - this.creationalContext = null; - this.interceptorChains = Collections.emptyMap(); - } } @Override @@ -159,40 +136,34 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl args = argsReplacement; } - List chain = interceptorChains.get(method); - if (chain != null) { - // Invoke business method interceptors - return new InvocationContextImpl(target, method, args, chain).proceed(); - } else { - try { - return method.invoke(target, args); - } catch (InvocationTargetException e) { - Throwable cause = e.getCause(); - if (cause instanceof CompletionException) { - cause = cause.getCause(); + try { + return method.invoke(target, args); + } catch (InvocationTargetException e) { + Throwable cause = e.getCause(); + if (cause instanceof CompletionException) { + cause = cause.getCause(); + } + if (cause instanceof ExceptionMapping.HandlerException) { + ((ExceptionMapping.HandlerException) cause).mapException(method); + // no applicable exception mapper found or applicable mapper returned null + return null; + } + if (cause instanceof ResponseProcessingException) { + ResponseProcessingException rpe = (ResponseProcessingException) cause; + cause = rpe.getCause(); + if (cause instanceof RuntimeException) { + throw cause; } - if (cause instanceof ExceptionMapping.HandlerException) { - ((ExceptionMapping.HandlerException) cause).mapException(method); - // no applicable exception mapper found or applicable mapper returned null - return null; + } else { + if (cause instanceof ProcessingException && + cause.getCause() instanceof ClientHeaderFillingException) { + throw cause.getCause().getCause(); } - if (cause instanceof ResponseProcessingException) { - ResponseProcessingException rpe = (ResponseProcessingException) cause; - cause = rpe.getCause(); - if (cause instanceof RuntimeException) { - throw cause; - } - } else { - if (cause instanceof ProcessingException && - cause.getCause() instanceof ClientHeaderFillingException) { - throw cause.getCause().getCause(); - } - if (cause instanceof RuntimeException) { - throw cause; - } + if (cause instanceof RuntimeException) { + throw cause; } - throw e; } + throw e; } } @@ -210,9 +181,6 @@ private Object invokeRestClientProxyMethod(Object proxy, Method method, Object[] private void close() { if (closed.compareAndSet(false, true)) { - if (creationalContext != null) { - creationalContext.release(); - } client.close(); } } @@ -227,77 +195,4 @@ private Type[] getGenericTypes(Class aClass) { } return genericTypes; } - - private static List getBindings(Annotation[] annotations, BeanManager beanManager) { - if (annotations.length == 0) { - return Collections.emptyList(); - } - List bindings = new ArrayList<>(); - for (Annotation annotation : annotations) { - if (beanManager.isInterceptorBinding(annotation.annotationType())) { - bindings.add(annotation); - } - } - return bindings; - } - - private static BeanManager getBeanManager(Class restClientInterface) { - try { - CDI current = CDI.current(); - return current != null ? current.getBeanManager() : null; - } catch (IllegalStateException e) { - LOGGER.warnf("CDI container is not available - interceptor bindings declared on %s will be ignored", - restClientInterface.getSimpleName()); - return null; - } - } - - private static Map> initInterceptorChains( - BeanManager beanManager, CreationalContext creationalContext, Class restClientInterface) { - - Map> chains = new HashMap<>(); - // Interceptor as a key in a map is not entirely correct (custom interceptors) but should work in most cases - Map, Object> interceptorInstances = new HashMap<>(); - - List classLevelBindings = getBindings(restClientInterface.getAnnotations(), beanManager); - - for (Method method : restClientInterface.getMethods()) { - if (method.isDefault() || Modifier.isStatic(method.getModifiers())) { - continue; - } - List methodLevelBindings = getBindings(method.getAnnotations(), beanManager); - - if (!classLevelBindings.isEmpty() || !methodLevelBindings.isEmpty()) { - - Annotation[] interceptorBindings = merge(methodLevelBindings, classLevelBindings); - - List> interceptors = beanManager.resolveInterceptors(InterceptionType.AROUND_INVOKE, - interceptorBindings); - if (!interceptors.isEmpty()) { - List chain = new ArrayList<>(); - for (Interceptor interceptor : interceptors) { - chain.add(new InvocationContextImpl.InterceptorInvocation(interceptor, - interceptorInstances.computeIfAbsent(interceptor, - i -> beanManager.getReference(i, i.getBeanClass(), creationalContext)))); - } - chains.put(method, chain); - } - } - } - return chains.isEmpty() ? Collections.emptyMap() : chains; - } - - private static Annotation[] merge(List methodLevelBindings, List classLevelBindings) { - Set> types = methodLevelBindings.stream() - .map(a -> a.annotationType()) - .collect(Collectors.toSet()); - List merged = new ArrayList<>(methodLevelBindings); - for (Annotation annotation : classLevelBindings) { - if (!types.contains(annotation.annotationType())) { - merged.add(annotation); - } - } - return merged.toArray(new Annotation[] {}); - } - } diff --git a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java index 67a1199..6fd2906 100644 --- a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java +++ b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java @@ -385,7 +385,7 @@ public T build(Class aClass, ClientHttpEngine httpEngine) interfaces[2] = Closeable.class; T proxy = (T) Proxy.newProxyInstance(classLoader, interfaces, - new ProxyInvocationHandler(aClass, actualClient, getLocalProviderInstances(), client, beanManager)); + new ProxyInvocationHandler(aClass, actualClient, getLocalProviderInstances(), client)); ClientHeaderProviders.registerForClass(aClass, proxy, beanManager); return proxy; } diff --git a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientDelegateBean.java b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientDelegateBean.java index 41a0e8c..7e65b74 100644 --- a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientDelegateBean.java +++ b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientDelegateBean.java @@ -52,10 +52,10 @@ import jakarta.enterprise.context.Dependent; import jakarta.enterprise.context.spi.CreationalContext; import jakarta.enterprise.inject.Any; -import jakarta.enterprise.inject.Default; import jakarta.enterprise.inject.spi.Bean; import jakarta.enterprise.inject.spi.BeanManager; import jakarta.enterprise.inject.spi.InjectionPoint; +import jakarta.enterprise.inject.spi.InterceptionFactory; import jakarta.enterprise.inject.spi.PassivationCapable; import jakarta.enterprise.util.AnnotationLiteral; @@ -65,7 +65,7 @@ import org.eclipse.microprofile.rest.client.inject.RestClient; import org.jboss.logging.Logger; -public class RestClientDelegateBean implements Bean, PassivationCapable { +public class RestClientDelegateBean implements Bean, PassivationCapable { private static final Logger LOGGER = Logger.getLogger(RestClientDelegateBean.class); public static final String REST_URL_FORMAT = "%s/mp-rest/url"; @@ -96,7 +96,7 @@ public class RestClientDelegateBean implements Bean, PassivationCapable private static final String PROPERTY_PREFIX = "%s/property/"; - private final Class proxyType; + private final Class proxyType; private final Class scope; @@ -108,7 +108,7 @@ public class RestClientDelegateBean implements Bean, PassivationCapable private final Optional configKey; - RestClientDelegateBean(final Class proxyType, final BeanManager beanManager, final Optional baseUri, + RestClientDelegateBean(final Class proxyType, final BeanManager beanManager, final Optional baseUri, final Optional configKey) { this.proxyType = proxyType; this.beanManager = beanManager; @@ -134,7 +134,7 @@ public Set getInjectionPoints() { } @Override - public Object create(CreationalContext creationalContext) { + public T create(CreationalContext creationalContext) { RestClientBuilder builder; // This can be removed once the below issue is resolved. However, for now we can handle this safely here. // See https://github.com/eclipse/microprofile-rest-client/issues/353 @@ -153,7 +153,14 @@ public Object create(CreationalContext creationalContext) { configureSsl(builder); getConfigProperties().forEach(builder::property); - return builder.build(proxyType); + + // We want to use the interception factory to let CDI handle all interception + InterceptionFactory interceptionFactory = beanManager.createInterceptionFactory(creationalContext, proxyType); + // Weld takes the interceptor bindings from the class (proxyType) used to create InterceptionFactory + // NOTE: This is somewhat grey area, it might be safer (but way more complex) to properly look up all bindings and + // register them here via - interceptionFactory.configure().add(SomeBinding.Literal.INSTANCE) + // Finally, create the proxy type and feed it to the interception factory + return interceptionFactory.createInterceptedInstance(builder.build(proxyType)); } private void configureSsl(RestClientBuilder builder) { @@ -318,7 +325,7 @@ private static URI uriFromString(String uriString) { } @Override - public void destroy(Object instance, CreationalContext creationalContext) { + public void destroy(T instance, CreationalContext creationalContext) { if (instance instanceof AutoCloseable) { try { ((AutoCloseable) instance).close(); @@ -326,18 +333,21 @@ public void destroy(Object instance, CreationalContext creationalContext LOGGER.debugf(e, "Failed to close client %s", instance); } } + // release all possibly created dependent objects of this creational context + // this will most likely be a no-op + creationalContext.release(); } @Override public Set getTypes() { + // only add the interface type + // NOTE: if there is a hierarchy of interfaces, should all be added as bean types? return Collections.singleton(proxyType); } @Override public Set getQualifiers() { Set qualifiers = new HashSet(); - qualifiers.add(new AnnotationLiteral() { - }); qualifiers.add(new AnnotationLiteral() { }); qualifiers.add(RestClient.LITERAL); @@ -351,6 +361,7 @@ public Class getScope() { @Override public String getName() { + // NOTE: this is an EL bean name, chances are this could just be null? return proxyType.getName(); } diff --git a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientExtension.java b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientExtension.java index 4205396..08eed60 100644 --- a/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientExtension.java +++ b/rest-client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientExtension.java @@ -53,7 +53,7 @@ public void registerRestClient(@Observes @WithAnnotations(RegisterRestClient.cla Optional maybeConfigKey = extractConfigKey(annotation); proxyTypes.add(new RestClientData(javaClass, maybeUri, maybeConfigKey)); - type.veto(); + // no need to veto() these types because interfaces cannot become beans anyway } else { errors.add(new IllegalArgumentException("Rest client needs to be an interface " + javaClass)); } @@ -107,12 +107,12 @@ public static void clearBeanManager() { // nothing to do } - private static class RestClientData { - private final Class javaClass; + private static class RestClientData { + private final Class javaClass; private final Optional baseUri; private final Optional configKey; - private RestClientData(final Class javaClass, final Optional baseUri, + private RestClientData(final Class javaClass, final Optional baseUri, final Optional configKey) { this.javaClass = javaClass; this.baseUri = baseUri; diff --git a/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/client/integration/ContainerRestClientProxyTest.java b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/client/integration/ContainerRestClientProxyTest.java new file mode 100644 index 0000000..6658958 --- /dev/null +++ b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/client/integration/ContainerRestClientProxyTest.java @@ -0,0 +1,161 @@ +/* + * JBoss, Home of Professional Open Source. + * + * Copyright 2023 Red Hat, Inc., and individual contributors + * as indicated by the @author tags. + * + * Licensed 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.jboss.resteasy.microprofile.test.client.integration; + +import java.io.IOException; +import java.lang.annotation.ElementType; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; +import java.lang.annotation.Target; +import java.lang.reflect.ReflectPermission; +import java.util.PropertyPermission; + +import jakarta.annotation.Priority; +import jakarta.enterprise.context.RequestScoped; +import jakarta.enterprise.inject.Intercepted; +import jakarta.enterprise.inject.spi.Bean; +import jakarta.inject.Inject; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InterceptorBinding; +import jakarta.interceptor.InvocationContext; +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; + +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; +import org.eclipse.microprofile.rest.client.inject.RestClient; +import org.jboss.arquillian.container.test.api.Deployment; +import org.jboss.arquillian.junit.Arquillian; +import org.jboss.resteasy.microprofile.test.util.TestEnvironment; +import org.jboss.resteasy.utils.PermissionUtil; +import org.jboss.shrinkwrap.api.asset.EmptyAsset; +import org.jboss.shrinkwrap.api.spec.WebArchive; +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; + +/** + * @author James R. Perkins + */ +@RunWith(Arquillian.class) +public class ContainerRestClientProxyTest { + + @Deployment + public static WebArchive deployment() throws IOException { + return TestEnvironment.createWarWithConfigUrl(ContainerRestClientProxyTest.class, InterceptedClient.class, "test-app") + .addClasses( + InterceptedClient.class, + TestResource.class, + ClientInterceptor.class, + ClientInterceptorBinding.class, + ClientMethodInterceptor.class, + ClientMethodInterceptorBinding.class) + .addAsManifestResource(PermissionUtil.createPermissionsXmlAsset( + new PropertyPermission("arquillian.*", "read"), + new ReflectPermission("suppressAccessChecks"), + new RuntimePermission("accessDeclaredMembers")), + "permissions.xml") + .addAsWebInfResource(EmptyAsset.INSTANCE, "beans.xml"); + } + + @Inject + @RestClient + private InterceptedClient client; + + @Test + public void intercepted() { + Assert.assertNotNull(client); + Assert.assertFalse(ClientInterceptor.invoked); + Assert.assertFalse(ClientMethodInterceptor.invoked); + Assert.assertEquals("test", client.get()); + Assert.assertTrue(ClientInterceptor.invoked); + Assert.assertTrue(ClientMethodInterceptor.invoked); + } + + @RegisterRestClient + @RequestScoped + @ClientInterceptorBinding + @Path("/test") + public interface InterceptedClient { + + @GET + @Path("/test") + @ClientMethodInterceptorBinding + String get(); + + } + + @Path("/test") + public static class TestResource { + @GET + @Path("/test") + public String get() { + return "test"; + } + } + + @ClientInterceptorBinding + @Priority(1) + @Interceptor + public static class ClientInterceptor { + + public static boolean invoked = false; + + @Inject + @Intercepted + Bean bean; + + @AroundInvoke + public Object doSomething(InvocationContext ic) throws Exception { + invoked = true; + return ic.proceed(); + } + } + + @Priority(1) + @Interceptor + @ClientMethodInterceptorBinding + public static class ClientMethodInterceptor { + + public static boolean invoked = false; + + @Inject + @Intercepted + Bean bean; + + @AroundInvoke + public Object doSomething(InvocationContext ic) throws Exception { + invoked = true; + return ic.proceed(); + } + } + + @InterceptorBinding + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.TYPE, ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER }) + public @interface ClientInterceptorBinding { + } + + @InterceptorBinding + @Retention(RetentionPolicy.RUNTIME) + @Target({ ElementType.TYPE, ElementType.METHOD, ElementType.FIELD, ElementType.PARAMETER }) + public @interface ClientMethodInterceptorBinding { + } +} diff --git a/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/util/TestEnvironment.java b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/util/TestEnvironment.java index 8e7af24..81950fb 100644 --- a/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/util/TestEnvironment.java +++ b/testsuite/integration-tests/src/test/java/org/jboss/resteasy/microprofile/test/util/TestEnvironment.java @@ -60,7 +60,7 @@ public static WebArchive createWarWithConfigUrl(final Class test, final Class final String url = getHttpUrl() + test.getSimpleName() + (path == null ? "" : (path.charAt(0) == '/' ? path : "/" + path)); return addConfigProperties(createWar(test), - Collections.singletonMap(resource.getCanonicalName() + "/mp-rest/url", url)); + Collections.singletonMap(resource.getName() + "/mp-rest/url", url)); } public static WebArchive createWarWithConfigUrl(final String deploymentName, final Class resource,