Skip to content

Commit

Permalink
Provide correct generic type and annotations in ParamConverterProvider
Browse files Browse the repository at this point in the history
These changes will provide the correct generic type and array of annotations when the JAX-RS annotation is present on a field or a method of a Bean Param class. 

The solution is similar to what was being done for the parameters of a REST Client method: it will load the metadata (generic type and annotations from fields and methods) of a BeanParam class using reflection only if there is a ParamConverterProvider present.

Fix quarkusio#32765
  • Loading branch information
Sgitario committed Apr 24, 2023
1 parent 485d870 commit a3d828e
Show file tree
Hide file tree
Showing 13 changed files with 347 additions and 167 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.gizmo.ResultHandle;
import io.quarkus.jaxrs.client.reactive.runtime.ParameterAnnotationsSupplier;
import io.quarkus.jaxrs.client.reactive.runtime.ParameterDescriptorFromClassSupplier;
import io.quarkus.jaxrs.client.reactive.runtime.ParameterGenericTypesSupplier;

class ClassRestClientContext implements AutoCloseable {
Expand All @@ -30,6 +31,9 @@ class ClassRestClientContext implements AutoCloseable {
public final Map<Integer, FieldDescriptor> methodStaticFields = new HashMap<>();
public final Map<Integer, FieldDescriptor> methodParamAnnotationsStaticFields = new HashMap<>();
public final Map<Integer, FieldDescriptor> methodGenericParametersStaticFields = new HashMap<>();
public final Map<String, FieldDescriptor> beanTypesParameterDescriptorsStaticFields = new HashMap<>();
public final Map<String, ResultHandle> classesMap = new HashMap<>();
private int beanParamIndex = 0;

public ClassRestClientContext(String name, BuildProducer<GeneratedClassBuildItem> generatedClasses,
String... interfaces) {
Expand All @@ -53,12 +57,12 @@ public void close() {
}

protected FieldDescriptor createJavaMethodField(ClassInfo interfaceClass, MethodInfo method, int methodIndex) {
ResultHandle interfaceClassHandle = clinit.loadClassFromTCCL(interfaceClass.toString());
ResultHandle interfaceClassHandle = loadClass(interfaceClass.toString());

ResultHandle parameterArray = clinit.newArray(Class.class, method.parametersCount());
for (int i = 0; i < method.parametersCount(); i++) {
String parameterClass = method.parameterType(i).name().toString();
clinit.writeArrayValue(parameterArray, i, clinit.loadClassFromTCCL(parameterClass));
clinit.writeArrayValue(parameterArray, i, loadClass(parameterClass));
}

ResultHandle javaMethodHandle = clinit.invokeVirtualMethod(
Expand Down Expand Up @@ -125,4 +129,43 @@ protected Supplier<FieldDescriptor> getLazyJavaMethodGenericParametersField(int
return javaMethodGenericParametersField;
};
}

/**
* Generates "Class.forName(beanClass)" to generate the parameter descriptors. This method will only be created if and only
* if the supplier is used in order to not have a penalty performance.
*/
protected Supplier<FieldDescriptor> getLazyBeanParameterDescriptors(String beanClass) {
return () -> {
FieldDescriptor field = beanTypesParameterDescriptorsStaticFields.get(beanClass);
if (field != null) {
return field;
}

ResultHandle clazz = loadClass(beanClass);

ResultHandle mapWithAnnotationsHandle = clinit.newInstance(MethodDescriptor.ofConstructor(
ParameterDescriptorFromClassSupplier.class, Class.class),
clazz);
field = FieldDescriptor.of(classCreator.getClassName(), "beanParamDescriptors" + beanParamIndex, Supplier.class);
classCreator.getFieldCreator(field).setModifiers(Modifier.FINAL | Modifier.STATIC); // needs to be package-private because it's used by subresources
clinit.writeStaticField(field, mapWithAnnotationsHandle);

beanTypesParameterDescriptorsStaticFields.put(beanClass, field);

beanParamIndex++;

return field;
};
}

private ResultHandle loadClass(String className) {
ResultHandle classType = classesMap.get(className);
if (classType != null) {
return classType;
}

ResultHandle classFromTCCL = clinit.loadClassFromTCCL(className);
classesMap.put(className, classFromTCCL);
return classFromTCCL;
}
}

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
package io.quarkus.jaxrs.client.reactive.runtime;

import java.lang.annotation.Annotation;
import java.lang.reflect.Field;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.util.HashMap;
import java.util.Map;
import java.util.function.Supplier;

public class ParameterDescriptorFromClassSupplier
implements Supplier<Map<String, ParameterDescriptorFromClassSupplier.ParameterDescriptor>> {

private final Class clazz;
private volatile Map<String, ParameterDescriptor> value;

public ParameterDescriptorFromClassSupplier(Class clazz) {
this.clazz = clazz;
}

@Override
public Map<String, ParameterDescriptor> get() {
if (value == null) {
value = new HashMap<>();
Class currentClass = clazz;
while (currentClass != null && currentClass != Object.class) {
for (Field field : currentClass.getDeclaredFields()) {
ParameterDescriptor descriptor = new ParameterDescriptor();
descriptor.annotations = field.getAnnotations();
descriptor.genericType = field.getGenericType();
value.put(field.getName(), descriptor);
}

for (Method method : currentClass.getDeclaredMethods()) {
ParameterDescriptor descriptor = new ParameterDescriptor();
descriptor.annotations = method.getAnnotations();
descriptor.genericType = method.getGenericReturnType();
value.put(method.getName(), descriptor);
}

currentClass = currentClass.getSuperclass();
}
}

return value;
}

public static class ParameterDescriptor {
public Annotation[] annotations;
public Type genericType;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Supplier;

import jakarta.ws.rs.ext.ParamConverter;
import jakarta.ws.rs.ext.ParamConverterProvider;
Expand Down Expand Up @@ -122,9 +121,8 @@ public RestClientBase(List<ParamConverterProvider> providers) {
}

@SuppressWarnings("unused") // used by generated code
public <T> Object[] convertParamArray(T[] value, Class<T> type, Supplier<Type[]> genericType,
Supplier<Annotation[][]> methodAnnotations, int paramIndex) {
ParamConverter<T> converter = getConverter(type, genericType, methodAnnotations, paramIndex);
public <T> Object[] convertParamArray(T[] value, Class<T> type, Type genericType, Annotation[] annotations) {
ParamConverter<T> converter = getConverter(type, genericType, annotations);

if (converter == null) {
return value;
Expand All @@ -139,10 +137,8 @@ public <T> Object[] convertParamArray(T[] value, Class<T> type, Supplier<Type[]>
}

@SuppressWarnings("unused") // used by generated code
public <T> Object convertParam(T value, Class<T> type, Supplier<Type[]> genericType,
Supplier<Annotation[][]> methodAnnotations,
int paramIndex) {
ParamConverter<T> converter = getConverter(type, genericType, methodAnnotations, paramIndex);
public <T> Object convertParam(T value, Class<T> type, Type genericType, Annotation[] annotations) {
ParamConverter<T> converter = getConverter(type, genericType, annotations);
if (converter != null) {
return converter.toString(value);
} else {
Expand All @@ -154,30 +150,26 @@ public <T> Object convertParam(T value, Class<T> type, Supplier<Type[]> genericT
}
}

private <T> ParamConverter<T> getConverter(Class<T> type, Supplier<Type[]> genericType,
Supplier<Annotation[][]> methodAnnotations,
int paramIndex) {
private <T> ParamConverter<T> getConverter(Class<T> type, Type genericType, Annotation[] annotations) {
ParamConverterProvider converterProvider = providerForClass.get(type);

if (converterProvider == null) {
for (ParamConverterProvider provider : paramConverterProviders) {
ParamConverter<T> converter = provider.getConverter(type, genericType.get()[paramIndex],
methodAnnotations.get()[paramIndex]);
ParamConverter<T> converter = provider.getConverter(type, genericType, annotations);
if (converter != null) {
providerForClass.put(type, provider);
return converter;
}
}
// FIXME: this should go in favour of generating them, so we can generate them only if used for dead-code elimination
ParamConverter<T> converter = DEFAULT_PROVIDER.getConverter(type, genericType.get()[paramIndex],
methodAnnotations.get()[paramIndex]);
ParamConverter<T> converter = DEFAULT_PROVIDER.getConverter(type, genericType, annotations);
if (converter != null) {
providerForClass.put(type, DEFAULT_PROVIDER);
return converter;
}
providerForClass.put(type, NO_PROVIDER);
} else if (converterProvider != NO_PROVIDER) {
return converterProvider.getConverter(type, genericType.get()[paramIndex], methodAnnotations.get()[paramIndex]);
return converterProvider.getConverter(type, genericType, annotations);
}
return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,16 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.lang.annotation.Annotation;
import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Type;
import java.net.URI;
import java.util.Arrays;

import jakarta.ws.rs.BeanParam;
import jakarta.ws.rs.CookieParam;
Expand Down Expand Up @@ -74,7 +80,7 @@ void shouldConvertHeaderParams() {
void shouldConvertCookieParams() {
Client client = RestClientBuilder.newBuilder().baseUri(baseUri)
.build(Client.class);
assertThat(client.getWithHeader(Param.FIRST)).isEqualTo("1");
assertThat(client.getWithCookie(Param.FIRST)).isEqualTo("1");
assertThat(client.sub().getWithCookie(Param.SECOND)).isEqualTo("2");

Bean bean = new Bean();
Expand All @@ -93,31 +99,31 @@ interface Client {

@GET
@Path("/param/{param}")
String get(@PathParam("param") Param param);
String get(@MyAnnotation("myValue") @PathParam("param") Param param);

@GET
@Path("/param/{param}")
String get(@BeanParam Bean beanParam);

@GET
@Path("/query")
String getWithQuery(@QueryParam("param") Param param);
String getWithQuery(@MyAnnotation("myValue") @QueryParam("param") Param param);

@GET
@Path("/query")
String getWithQuery(@BeanParam Bean beanParam);

@GET
@Path("/header")
String getWithHeader(@HeaderParam("param") Param param);
String getWithHeader(@MyAnnotation("myValue") @HeaderParam("param") Param param);

@GET
@Path("/header")
String getWithHeader(@BeanParam Bean beanParam);

@GET
@Path("/cookie")
String getWithCookie(@HeaderParam("cookie-param") Param param);
String getWithCookie(@MyAnnotation("myValue") @CookieParam("cookie-param") Param param);

@GET
@Path("/cookie")
Expand All @@ -127,28 +133,32 @@ interface Client {
interface SubClient {
@GET
@Path("/param/{param}")
String get(@PathParam("param") Param param);
String get(@MyAnnotation("myValue") @PathParam("param") Param param);

@GET
@Path("/query")
String getWithQuery(@QueryParam("param") Param param);
String getWithQuery(@MyAnnotation("myValue") @QueryParam("param") Param param);

@GET
@Path("/header")
String getWithHeader(@HeaderParam("param") Param param);
String getWithHeader(@MyAnnotation("myValue") @HeaderParam("param") Param param);

@GET
@Path("cookie")
String getWithCookie(@CookieParam("cookie-param") Param param);
String getWithCookie(@MyAnnotation("myValue") @CookieParam("cookie-param") Param param);
}

public static class Bean {
@MyAnnotation("myValue")
@PathParam("param")
public Param param;
@MyAnnotation("myValue")
@QueryParam("param")
public Param queryParam;
@MyAnnotation("myValue")
@HeaderParam("param")
public Param headerParam;
@MyAnnotation("myValue")
@CookieParam("cookie-param")
public Param cookieParam;
}
Expand All @@ -158,6 +168,12 @@ enum Param {
SECOND
}

@Target({ ElementType.FIELD, ElementType.PARAMETER })
@Retention(RetentionPolicy.RUNTIME)
public @interface MyAnnotation {
String value() default "";
}

public static class ParamConverter implements ParamConverterProvider {
@SuppressWarnings("unchecked")
@Override
Expand All @@ -171,6 +187,9 @@ public <T> jakarta.ws.rs.ext.ParamConverter<T> getConverter(Class<T> rawType, Ty
fail("Annotations cannot be null!");
}

assertTrue(Arrays.stream(annotations)
.anyMatch(a -> a instanceof MyAnnotation && ((MyAnnotation) a).value().equals("myValue")));

if (rawType == Param.class) {
return (jakarta.ws.rs.ext.ParamConverter<T>) new jakarta.ws.rs.ext.ParamConverter<Param>() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,17 @@ public class BeanParamItem extends Item {
private final List<Item> items;
private final String className;

public BeanParamItem(String fieldName, List<Item> items, String className, ValueExtractor extractor) {
super(fieldName, ItemType.BEAN_PARAM, extractor);
this.items = items;
this.className = className;
}

public String className() {
return className;
}

public List<Item> items() {
return items;
}

public BeanParamItem(List<Item> items, String className, ValueExtractor extractor) {
super(ItemType.BEAN_PARAM, extractor);
this.items = items;
this.className = className;
}
}
Loading

0 comments on commit a3d828e

Please sign in to comment.