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

Attempt to use CDI InterceptionFactory #181

Closed
wants to merge 2 commits into from
Closed

Attempt to use CDI InterceptionFactory #181

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
1bbbaeb
** Add test for proxy interface with interceptor.
jamezp Jun 14, 2023
406b146
Rework @RegisterRestClient to use CDI InterceptionFactory
manovotn Jun 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 25 additions & 130 deletions ...ent-base/src/main/java/org/jboss/resteasy/microprofile/client/ProxyInvocationHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -59,29 +47,18 @@ public class ProxyInvocationHandler implements InvocationHandler {

private final Set<Object> providerInstances;

private final Map<Method, List<InvocationContextImpl.InterceptorInvocation>> interceptorChains;

private final ResteasyClient client;

private final CreationalContext<?> creationalContext;

private final AtomicBoolean closed;

public ProxyInvocationHandler(final Class<?> restClientInterface,
final Object target,
final Set<Object> 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
Expand Down Expand Up @@ -159,40 +136,34 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
args = argsReplacement;
}

List<InvocationContextImpl.InterceptorInvocation> 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;
}
}

Expand All @@ -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();
}
}
Expand All @@ -227,77 +195,4 @@ private Type[] getGenericTypes(Class<?> aClass) {
}
return genericTypes;
}

private static List<Annotation> getBindings(Annotation[] annotations, BeanManager beanManager) {
if (annotations.length == 0) {
return Collections.emptyList();
}
List<Annotation> 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<Object> 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<Method, List<InvocationContextImpl.InterceptorInvocation>> initInterceptorChains(
BeanManager beanManager, CreationalContext<?> creationalContext, Class<?> restClientInterface) {

Map<Method, List<InvocationContextImpl.InterceptorInvocation>> chains = new HashMap<>();
// Interceptor as a key in a map is not entirely correct (custom interceptors) but should work in most cases
Map<Interceptor<?>, Object> interceptorInstances = new HashMap<>();

List<Annotation> classLevelBindings = getBindings(restClientInterface.getAnnotations(), beanManager);

for (Method method : restClientInterface.getMethods()) {
if (method.isDefault() || Modifier.isStatic(method.getModifiers())) {
continue;
}
List<Annotation> methodLevelBindings = getBindings(method.getAnnotations(), beanManager);

if (!classLevelBindings.isEmpty() || !methodLevelBindings.isEmpty()) {

Annotation[] interceptorBindings = merge(methodLevelBindings, classLevelBindings);

List<Interceptor<?>> interceptors = beanManager.resolveInterceptors(InterceptionType.AROUND_INVOKE,
interceptorBindings);
if (!interceptors.isEmpty()) {
List<InvocationContextImpl.InterceptorInvocation> 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<Annotation> methodLevelBindings, List<Annotation> classLevelBindings) {
Set<Class<? extends Annotation>> types = methodLevelBindings.stream()
.map(a -> a.annotationType())
.collect(Collectors.toSet());
List<Annotation> merged = new ArrayList<>(methodLevelBindings);
for (Annotation annotation : classLevelBindings) {
if (!types.contains(annotation.annotationType())) {
merged.add(annotation);
}
}
return merged.toArray(new Annotation[] {});
}

}
2 changes: 1 addition & 1 deletion ...ient-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientBuilderImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,7 @@ public <T> T build(Class<T> 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;
}
Expand Down
29 changes: 20 additions & 9 deletions ...ent-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientDelegateBean.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -65,7 +65,7 @@
import org.eclipse.microprofile.rest.client.inject.RestClient;
import org.jboss.logging.Logger;

public class RestClientDelegateBean implements Bean<Object>, PassivationCapable {
public class RestClientDelegateBean<T> implements Bean<T>, PassivationCapable {
private static final Logger LOGGER = Logger.getLogger(RestClientDelegateBean.class);

public static final String REST_URL_FORMAT = "%s/mp-rest/url";
Expand Down Expand Up @@ -96,7 +96,7 @@ public class RestClientDelegateBean implements Bean<Object>, PassivationCapable

private static final String PROPERTY_PREFIX = "%s/property/";

private final Class<?> proxyType;
private final Class<T> proxyType;

private final Class<? extends Annotation> scope;

Expand All @@ -108,7 +108,7 @@ public class RestClientDelegateBean implements Bean<Object>, PassivationCapable

private final Optional<String> configKey;

RestClientDelegateBean(final Class<?> proxyType, final BeanManager beanManager, final Optional<String> baseUri,
RestClientDelegateBean(final Class<T> proxyType, final BeanManager beanManager, final Optional<String> baseUri,
final Optional<String> configKey) {
this.proxyType = proxyType;
this.beanManager = beanManager;
Expand All @@ -134,7 +134,7 @@ public Set<InjectionPoint> getInjectionPoints() {
}

@Override
public Object create(CreationalContext<Object> creationalContext) {
public T create(CreationalContext<T> 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
Expand All @@ -153,7 +153,14 @@ public Object create(CreationalContext<Object> 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<T> 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) {
Expand Down Expand Up @@ -318,26 +325,29 @@ private static URI uriFromString(String uriString) {
}

@Override
public void destroy(Object instance, CreationalContext<Object> creationalContext) {
public void destroy(T instance, CreationalContext<T> creationalContext) {
if (instance instanceof AutoCloseable) {
try {
((AutoCloseable) instance).close();
} catch (Exception e) {
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<Type> 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<Annotation> getQualifiers() {
Set<Annotation> qualifiers = new HashSet<Annotation>();
qualifiers.add(new AnnotationLiteral<Default>() {
});
qualifiers.add(new AnnotationLiteral<Any>() {
});
qualifiers.add(RestClient.LITERAL);
Expand All @@ -351,6 +361,7 @@ public Class<? extends Annotation> getScope() {

@Override
public String getName() {
// NOTE: this is an EL bean name, chances are this could just be null?
return proxyType.getName();
}

Expand Down
8 changes: 4 additions & 4 deletions ...client-base/src/main/java/org/jboss/resteasy/microprofile/client/RestClientExtension.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ public void registerRestClient(@Observes @WithAnnotations(RegisterRestClient.cla
Optional<String> 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));
}
Expand Down Expand Up @@ -107,12 +107,12 @@ public static void clearBeanManager() {
// nothing to do
}

private static class RestClientData {
private final Class<?> javaClass;
private static class RestClientData<T> {
private final Class<T> javaClass;
private final Optional<String> baseUri;
private final Optional<String> configKey;

private RestClientData(final Class<?> javaClass, final Optional<String> baseUri,
private RestClientData(final Class<T> javaClass, final Optional<String> baseUri,
final Optional<String> configKey) {
this.javaClass = javaClass;
this.baseUri = baseUri;
Expand Down
Loading