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 #32765
  • Loading branch information
Sgitario committed Apr 24, 2023
1 parent 4ca7cf2 commit 6565c22
Show file tree
Hide file tree
Showing 14 changed files with 496 additions and 160 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 @@ -74,7 +74,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 Down Expand Up @@ -117,7 +117,7 @@ interface Client {

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

@GET
@Path("/cookie")
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
package io.quarkus.rest.client.reactive.converter;

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;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.ext.ParamConverterProvider;

import org.eclipse.microprofile.rest.client.RestClientBuilder;
import org.eclipse.microprofile.rest.client.annotation.RegisterProvider;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.RegisterExtension;

import io.quarkus.test.QuarkusUnitTest;
import io.quarkus.test.common.http.TestHTTPResource;

public class ParamConverterReadAnnotationFromBeanTest {
@RegisterExtension
static final QuarkusUnitTest TEST = new QuarkusUnitTest();

@TestHTTPResource
URI baseUri;

@Test
void shouldConvertPathParam() {
Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class);
Bean bean = new Bean();
bean.param = Param.FIRST;
assertThat(client.get(bean)).isEqualTo("1");

bean = new Bean();
bean.param = Param.SECOND;
assertThat(client.sub().get(bean)).isEqualTo("2");
}

@Path("/echo")
@RegisterProvider(ParamConverter.class)
interface Client {
@Path("/sub")
SubClient sub();

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

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

public static class Bean {
@MyAnnotation("myValue")
@PathParam("param")
public Param param;
}

enum Param {
FIRST,
SECOND
}

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

public static class ParamConverter implements ParamConverterProvider {
@SuppressWarnings("unchecked")
@Override
public <T> jakarta.ws.rs.ext.ParamConverter<T> getConverter(Class<T> rawType, Type genericType,
Annotation[] annotations) {
if (annotations == null) {
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
public Param fromString(String value) {
return null;
}

@Override
public String toString(Param value) {
if (value == null) {
return null;
}
switch (value) {
case FIRST:
return "1";
case SECOND:
return "2";
default:
return "unexpected";
}
}
};
}
return null;
}
}

@Path("/echo")
public static class EchoEndpoint {
@Path("/param/{param}")
@GET
public String echoPath(@PathParam("param") String param) {
return param;
}

@Path("/sub/param/{param}")
@GET
public String echoSubPath(@PathParam("param") String param) {
return param;
}

@GET
@Path("/query")
public String get(@QueryParam("param") String param) {
return param;
}

@Path("/sub/query")
@GET
public String getSub(@QueryParam("param") String param) {
return param;
}

@GET
@Path("/header")
public String getHeader(@HeaderParam("param") String param) {
return param;
}

@Path("/sub/header")
@GET
public String getSubHeader(@HeaderParam("param") String param) {
return param;
}

@GET
@Path("/cookie")
public String getCookie(@CookieParam("cookie-param") String param) {
return param;
}

@Path("/sub/cookie")
@GET
public String getSubCookie(@CookieParam("cookie-param") String param) {
return param;
}
}
}
Loading

0 comments on commit 6565c22

Please sign in to comment.