From 5d9f3e2dbd4590df461207466de0df5c5ca2475d Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Thu, 14 Sep 2023 08:55:03 +0300 Subject: [PATCH] Fix use of multiple @ClientXX annotation in REST Client Reactive Fixes: #35884 --- .../RestClientReactiveProcessor.java | 101 ++++++++++-------- .../MultipleProvidersFromAnnotationTest.java | 83 ++++++++++++++ 2 files changed, 142 insertions(+), 42 deletions(-) create mode 100644 extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/redirect/MultipleProvidersFromAnnotationTest.java diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/main/java/io/quarkus/rest/client/reactive/deployment/RestClientReactiveProcessor.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/main/java/io/quarkus/rest/client/reactive/deployment/RestClientReactiveProcessor.java index e4483f6586bb7..04fd0a2d54494 100644 --- a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/main/java/io/quarkus/rest/client/reactive/deployment/RestClientReactiveProcessor.java +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/main/java/io/quarkus/rest/client/reactive/deployment/RestClientReactiveProcessor.java @@ -39,6 +39,7 @@ import jakarta.ws.rs.Priorities; import jakarta.ws.rs.RuntimeType; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.MultivaluedMap; import org.eclipse.microprofile.config.Config; import org.eclipse.microprofile.config.ConfigProvider; @@ -60,6 +61,7 @@ import org.jboss.resteasy.reactive.client.spi.MissingMessageBodyReaderErrorMessageContextualizer; import org.jboss.resteasy.reactive.common.processor.ResteasyReactiveDotNames; import org.jboss.resteasy.reactive.common.processor.transformation.AnnotationStore; +import org.jboss.resteasy.reactive.common.util.QuarkusMultivaluedHashMap; import io.quarkus.arc.deployment.AdditionalBeanBuildItem; import io.quarkus.arc.deployment.CustomScopeAnnotationsBuildItem; @@ -331,12 +333,15 @@ void registerProvidersFromAnnotations(CombinedIndexBuildItem indexBuildItem, } } - Map generatedProviders = new HashMap<>(); - populateClientExceptionMapperFromAnnotations(generatedClasses, reflectiveClasses, index, generatedProviders); - populateClientRedirectHandlerFromAnnotations(generatedClasses, reflectiveClasses, index, generatedProviders); + MultivaluedMap generatedProviders = new QuarkusMultivaluedHashMap<>(); + populateClientExceptionMapperFromAnnotations(generatedClasses, reflectiveClasses, index) + .forEach(generatedProviders::add); + populateClientRedirectHandlerFromAnnotations(generatedClasses, reflectiveClasses, index) + .forEach(generatedProviders::add); for (AnnotationToRegisterIntoClientContextBuildItem annotation : annotationsToRegisterIntoClientContext) { - populateClientProviderFromAnnotations(annotation, generatedClasses, reflectiveClasses, - index, generatedProviders); + populateClientProviderFromAnnotations(annotation, generatedClasses, reflectiveClasses, index) + .forEach(generatedProviders::add); + } addGeneratedProviders(index, constructor, annotationsByClassName, generatedProviders); @@ -551,77 +556,83 @@ && isImplementorOf(index, target.asClass(), RESPONSE_EXCEPTION_MAPPER, Set.of(AP } } - private void populateClientExceptionMapperFromAnnotations(BuildProducer generatedClasses, - BuildProducer reflectiveClasses, IndexView index, - Map generatedProviders) { + private Map populateClientExceptionMapperFromAnnotations( + BuildProducer generatedClasses, + BuildProducer reflectiveClasses, IndexView index) { + var result = new HashMap(); ClientExceptionMapperHandler clientExceptionMapperHandler = new ClientExceptionMapperHandler( new GeneratedClassGizmoAdaptor(generatedClasses, true)); for (AnnotationInstance instance : index.getAnnotations(CLIENT_EXCEPTION_MAPPER)) { - GeneratedClassResult result = clientExceptionMapperHandler.generateResponseExceptionMapper(instance); - if (result == null) { + GeneratedClassResult classResult = clientExceptionMapperHandler.generateResponseExceptionMapper(instance); + if (classResult == null) { continue; } - if (generatedProviders.containsKey(result.interfaceName)) { + if (result.containsKey(classResult.interfaceName)) { throw new IllegalStateException("Only a single instance of '" + CLIENT_EXCEPTION_MAPPER - + "' is allowed per REST Client interface. Offending class is '" + result.interfaceName + "'"); + + "' is allowed per REST Client interface. Offending class is '" + classResult.interfaceName + "'"); } - generatedProviders.put(result.interfaceName, result); - reflectiveClasses.produce(ReflectiveClassBuildItem.builder(result.generatedClassName) + result.put(classResult.interfaceName, classResult); + reflectiveClasses.produce(ReflectiveClassBuildItem.builder(classResult.generatedClassName) .serialization(false).build()); } + return result; } - private void populateClientRedirectHandlerFromAnnotations(BuildProducer generatedClasses, - BuildProducer reflectiveClasses, IndexView index, - Map generatedProviders) { + private Map populateClientRedirectHandlerFromAnnotations( + BuildProducer generatedClasses, + BuildProducer reflectiveClasses, IndexView index) { + var result = new HashMap(); ClientRedirectHandler clientHandler = new ClientRedirectHandler(new GeneratedClassGizmoAdaptor(generatedClasses, true)); for (AnnotationInstance instance : index.getAnnotations(CLIENT_REDIRECT_HANDLER)) { - GeneratedClassResult result = clientHandler.generateResponseExceptionMapper(instance); - if (result == null) { + GeneratedClassResult classResult = clientHandler.generateResponseExceptionMapper(instance); + if (classResult == null) { continue; } - GeneratedClassResult existing = generatedProviders.get(result.interfaceName); - if (existing != null && existing.priority == result.priority) { + GeneratedClassResult existing = result.get(classResult.interfaceName); + if (existing != null && existing.priority == classResult.priority) { throw new IllegalStateException("Only a single instance of '" + CLIENT_REDIRECT_HANDLER + "' with the same priority is allowed per REST Client interface. " - + "Offending class is '" + result.interfaceName + "'"); - } else if (existing == null || existing.priority < result.priority) { - generatedProviders.put(result.interfaceName, result); - reflectiveClasses.produce(ReflectiveClassBuildItem.builder(result.generatedClassName) + + "Offending class is '" + classResult.interfaceName + "'"); + } else if (existing == null || existing.priority < classResult.priority) { + result.put(classResult.interfaceName, classResult); + reflectiveClasses.produce(ReflectiveClassBuildItem.builder(classResult.generatedClassName) .serialization(false).build()); } } + return result; } - private void populateClientProviderFromAnnotations(AnnotationToRegisterIntoClientContextBuildItem annotationBuildItem, + private Map populateClientProviderFromAnnotations( + AnnotationToRegisterIntoClientContextBuildItem annotationBuildItem, BuildProducer generatedClasses, - BuildProducer reflectiveClasses, IndexView index, - Map generatedProviders) { + BuildProducer reflectiveClasses, IndexView index) { + var result = new HashMap(); ClientContextResolverHandler handler = new ClientContextResolverHandler(annotationBuildItem.getAnnotation(), annotationBuildItem.getExpectedReturnType(), new GeneratedClassGizmoAdaptor(generatedClasses, true)); for (AnnotationInstance instance : index.getAnnotations(annotationBuildItem.getAnnotation())) { - GeneratedClassResult result = handler.generateContextResolver(instance); - if (result == null) { + GeneratedClassResult classResult = handler.generateContextResolver(instance); + if (classResult == null) { continue; } - if (generatedProviders.containsKey(result.interfaceName)) { + if (result.containsKey(classResult.interfaceName)) { throw new IllegalStateException("Only a single instance of '" + annotationBuildItem.getAnnotation() - + "' is allowed per REST Client interface. Offending class is '" + result.interfaceName + "'"); + + "' is allowed per REST Client interface. Offending class is '" + classResult.interfaceName + "'"); } - generatedProviders.put(result.interfaceName, result); - reflectiveClasses.produce(ReflectiveClassBuildItem.builder(result.generatedClassName) + result.put(classResult.interfaceName, classResult); + reflectiveClasses.produce(ReflectiveClassBuildItem.builder(classResult.generatedClassName) .serialization(false).build()); } + return result; } private void addGeneratedProviders(IndexView index, MethodCreator constructor, Map> annotationsByClassName, - Map generatedProviders) { + Map> generatedProviders) { for (Map.Entry> annotationsForClass : annotationsByClassName.entrySet()) { ResultHandle map = constructor.newInstance(MethodDescriptor.ofConstructor(HashMap.class)); for (AnnotationInstance value : annotationsForClass.getValue()) { @@ -641,18 +652,24 @@ private void addGeneratedProviders(IndexView index, MethodCreator constructor, if (generatedProviders.containsKey(ifaceName)) { // remove the interface from the generated provider since it's going to be handled now // the remaining entries will be handled later - GeneratedClassResult result = generatedProviders.remove(ifaceName); - constructor.invokeInterfaceMethod(MAP_PUT, map, constructor.loadClass(result.generatedClassName), - constructor.load(result.priority)); + List providers = generatedProviders.remove(ifaceName); + for (GeneratedClassResult classResult : providers) { + constructor.invokeInterfaceMethod(MAP_PUT, map, constructor.loadClass(classResult.generatedClassName), + constructor.load(classResult.priority)); + } + } addProviders(constructor, ifaceName, map); } - for (Map.Entry entry : generatedProviders.entrySet()) { + for (Map.Entry> entry : generatedProviders.entrySet()) { ResultHandle map = constructor.newInstance(MethodDescriptor.ofConstructor(HashMap.class)); - constructor.invokeInterfaceMethod(MAP_PUT, map, constructor.loadClass(entry.getValue().generatedClassName), - constructor.load(entry.getValue().priority)); - addProviders(constructor, entry.getKey(), map); + for (GeneratedClassResult classResult : entry.getValue()) { + constructor.invokeInterfaceMethod(MAP_PUT, map, constructor.loadClass(classResult.generatedClassName), + constructor.load(classResult.priority)); + addProviders(constructor, entry.getKey(), map); + } + } } diff --git a/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/redirect/MultipleProvidersFromAnnotationTest.java b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/redirect/MultipleProvidersFromAnnotationTest.java new file mode 100644 index 0000000000000..237e3189a9ad1 --- /dev/null +++ b/extensions/resteasy-reactive/rest-client-reactive/deployment/src/test/java/io/quarkus/rest/client/reactive/redirect/MultipleProvidersFromAnnotationTest.java @@ -0,0 +1,83 @@ +package io.quarkus.rest.client.reactive.redirect; + +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import java.net.URI; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.QueryParam; +import jakarta.ws.rs.core.Response; + +import org.eclipse.microprofile.rest.client.RestClientBuilder; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; + +import io.quarkus.rest.client.reactive.ClientExceptionMapper; +import io.quarkus.rest.client.reactive.ClientRedirectHandler; +import io.quarkus.test.QuarkusUnitTest; +import io.quarkus.test.common.http.TestHTTPResource; + +public class MultipleProvidersFromAnnotationTest { + + @RegisterExtension + static final QuarkusUnitTest TEST = new QuarkusUnitTest() + .withApplicationRoot((jar) -> jar + .addClasses(Client.class, Resource.class)); + + @Test + void test() { + Client client = RestClientBuilder.newBuilder() + .baseUri(uri) + .followRedirects(true) + .build(Client.class); + assertThatThrownBy(() -> client.call(2)).hasMessage("dummy"); + } + + @TestHTTPResource + URI uri; + + @Path("test") + public interface Client { + + @GET + void call(@QueryParam("redirects") Integer numberOfRedirects); + + @ClientRedirectHandler + static URI redirectFor3xx(Response response) { + int status = response.getStatus(); + if (status > 300 && response.getStatus() < 400) { + return response.getLocation(); + } + + return null; + } + + @ClientExceptionMapper + static RuntimeException toException(Response response) { + if (response.getStatus() == 999) { + throw new RuntimeException("dummy") { + @Override + public synchronized Throwable fillInStackTrace() { + return this; + } + }; + } + return null; + } + } + + @Path("test") + public static class Resource { + + @GET + public Response redirectedResponse(@QueryParam("redirects") Integer number) { + if (number == null || 0 == number) { + return Response.status(999).build(); + } else { + return Response.status(Response.Status.FOUND).location(URI.create("/test?redirects=" + (number - 1))) + .build(); + } + } + } +}