From 6b19e71a1815f26849ea8b6e69bb793a405a7085 Mon Sep 17 00:00:00 2001 From: Ladislav Thon Date: Thu, 25 Nov 2021 10:46:39 +0100 Subject: [PATCH] Copy default methods of a RestClient Reactive interface to the generated class RestClient Reactive generates a `...$$CDIWrapper` class for each RestClient interface, and creates an implementation of each RestClient method. This leaves non-RestClient methods (`default` methods) on the interface, which means that if they are annotated with an interceptor binding, the interceptor is not invoked. This is because interceptor bindings are not inherited from superinterfaces, only from superclasses (and while ArC has some support for intercepting `default` methods from superinterfaces, it doesn't extends that far). This PR doesn't attempt to support intercepting `default` methods in general, because that's a gray area. Instead, it fixes how RestClient Reactive generates the class for a RestClient interface (because RestClient interfaces are special in that they may define class-based beans if annotated `@RegisterRestClient`). In addition to RestClient methods, `default` methods from the RestClient interface are also copied to the generated class. (The "copy" delegates to the `default` method inherited from the superinterface, which had to be added to Gizmo, hence the Gizmo update.) That itself is enough for interceptors to start working. --- .../RestClientReactiveProcessor.java | 54 ++++++++++++------- .../rest-client-reactive/pom.xml | 19 ++++++- .../client/main/ClientCallingResource.java | 7 +++ .../client/main/FaultToleranceClient.java | 28 ++++++++++ .../src/main/resources/application.properties | 1 + .../io/quarkus/it/rest/client/BasicTest.java | 8 +++ 6 files changed, 98 insertions(+), 19 deletions(-) create mode 100644 integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/FaultToleranceClient.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 d3b16953987a8..c6069c19bb689 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 @@ -339,11 +339,21 @@ void addRestClientBeans(Capabilities capabilities, ClassInfo jaxrsInterface = registerRestClient.target().asClass(); // for each interface annotated with @RegisterRestClient, generate a $$CDIWrapper CDI bean that can be injected if (Modifier.isAbstract(jaxrsInterface.flags())) { - List restMethods = new ArrayList<>(); - - // search this class and its super interfaces for jaxrs methods - searchForJaxRsMethods(restMethods, jaxrsInterface, index); - if (restMethods.isEmpty()) { + List methodsToImplement = new ArrayList<>(); + + // search this interface and its super interfaces for jaxrs methods + searchForJaxRsMethods(methodsToImplement, jaxrsInterface, index); + // search this interface for default methods + // we could search for default methods in super interfaces too, + // but emitting the correct invokespecial instruction would become convoluted + // (as invokespecial may only reference a method from a _direct_ super interface) + for (MethodInfo method : jaxrsInterface.methods()) { + boolean isDefault = !Modifier.isAbstract(method.flags()); + if (isDefault) { + methodsToImplement.add(method); + } + } + if (methodsToImplement.isEmpty()) { continue; } @@ -392,11 +402,16 @@ void addRestClientBeans(Capabilities capabilities, constructor.returnValue(null); // METHODS: - for (MethodInfo method : restMethods) { + for (MethodInfo method : methodsToImplement) { // for each method that corresponds to making a rest call, create a method like: // public JsonArray get() { // return ((InterfaceClass)this.getDelegate()).get(); // } + // + // for each default method, create a method like: + // public JsonArray get() { + // return InterfaceClass.super.get(); + // } MethodCreator methodCreator = classCreator.getMethodCreator(MethodDescriptor.of(method)); methodCreator.setSignature(AsmUtil.getSignatureIfRequired(method)); @@ -409,22 +424,25 @@ void addRestClientBeans(Capabilities capabilities, } } - ResultHandle delegate = methodCreator.invokeVirtualMethod( - MethodDescriptor.ofMethod(RestClientReactiveCDIWrapperBase.class, "getDelegate", - Object.class), - methodCreator.getThis()); + ResultHandle result; int parameterCount = method.parameters().size(); - ResultHandle result; - if (parameterCount == 0) { - result = methodCreator.invokeInterfaceMethod(method, delegate); - } else { - ResultHandle[] params = new ResultHandle[parameterCount]; - for (int i = 0; i < parameterCount; i++) { - params[i] = methodCreator.getMethodParam(i); - } + ResultHandle[] params = new ResultHandle[parameterCount]; + for (int i = 0; i < parameterCount; i++) { + params[i] = methodCreator.getMethodParam(i); + } + + if (Modifier.isAbstract(method.flags())) { // RestClient method + ResultHandle delegate = methodCreator.invokeVirtualMethod( + MethodDescriptor.ofMethod(RestClientReactiveCDIWrapperBase.class, "getDelegate", + Object.class), + methodCreator.getThis()); + result = methodCreator.invokeInterfaceMethod(method, delegate, params); + } else { // default method + result = methodCreator.invokeSpecialInterfaceMethod(method, methodCreator.getThis(), params); } + methodCreator.returnValue(result); } } diff --git a/integration-tests/rest-client-reactive/pom.xml b/integration-tests/rest-client-reactive/pom.xml index 415a74cc99558..8271d622234ff 100644 --- a/integration-tests/rest-client-reactive/pom.xml +++ b/integration-tests/rest-client-reactive/pom.xml @@ -12,7 +12,6 @@ Quarkus - Integration Tests - REST Client Reactive - @@ -26,6 +25,11 @@ quarkus-rest-client-reactive-jackson + + io.quarkus + quarkus-smallrye-fault-tolerance + + io.quarkus @@ -78,6 +82,19 @@ + + io.quarkus + quarkus-smallrye-fault-tolerance-deployment + ${project.version} + pom + test + + + * + * + + + io.quarkus quarkus-vertx-http-deployment diff --git a/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/ClientCallingResource.java b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/ClientCallingResource.java index 2c67f67c4507c..0a9a432219503 100644 --- a/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/ClientCallingResource.java +++ b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/ClientCallingResource.java @@ -38,6 +38,9 @@ public class ClientCallingResource { @RestClient ClientWithExceptionMapper clientWithExceptionMapper; + @RestClient + FaultToleranceClient faultToleranceClient; + @Inject InMemorySpanExporter inMemorySpanExporter; @@ -132,6 +135,10 @@ void init(@Observes Router router) { .stream().filter(sd -> !sd.getName().contains("export")) .collect(Collectors.toList()))); }); + + router.route("/call-with-fault-tolerance").blockingHandler(rc -> { + rc.end(faultToleranceClient.helloWithFallback()); + }); } private Future success(RoutingContext rc, String body) { diff --git a/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/FaultToleranceClient.java b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/FaultToleranceClient.java new file mode 100644 index 0000000000000..ceab46764717b --- /dev/null +++ b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/FaultToleranceClient.java @@ -0,0 +1,28 @@ +package io.quarkus.it.rest.client.main; + +import javax.ws.rs.Consumes; +import javax.ws.rs.GET; +import javax.ws.rs.Path; +import javax.ws.rs.Produces; +import javax.ws.rs.core.MediaType; + +import org.eclipse.microprofile.faulttolerance.Fallback; +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; + +@Path("/unprocessable") +@RegisterRestClient(configKey = "w-fault-tolerance") +public interface FaultToleranceClient { + @GET + @Produces(MediaType.TEXT_PLAIN) + @Consumes(MediaType.TEXT_PLAIN) + String hello(); + + @Fallback(fallbackMethod = "fallback") + default String helloWithFallback() { + return hello(); + } + + default String fallback() { + return "Hello fallback!"; + } +} diff --git a/integration-tests/rest-client-reactive/src/main/resources/application.properties b/integration-tests/rest-client-reactive/src/main/resources/application.properties index c8425d08b6e93..dca0a78fb80fc 100644 --- a/integration-tests/rest-client-reactive/src/main/resources/application.properties +++ b/integration-tests/rest-client-reactive/src/main/resources/application.properties @@ -1,2 +1,3 @@ w-exception-mapper/mp-rest/url=${test.url} +w-fault-tolerance/mp-rest/url=${test.url} io.quarkus.it.rest.client.multipart.MultipartClient/mp-rest/url=${test.url} \ No newline at end of file diff --git a/integration-tests/rest-client-reactive/src/test/java/io/quarkus/it/rest/client/BasicTest.java b/integration-tests/rest-client-reactive/src/test/java/io/quarkus/it/rest/client/BasicTest.java index 82f666e6bad9e..0f1d890c01da6 100644 --- a/integration-tests/rest-client-reactive/src/test/java/io/quarkus/it/rest/client/BasicTest.java +++ b/integration-tests/rest-client-reactive/src/test/java/io/quarkus/it/rest/client/BasicTest.java @@ -84,6 +84,14 @@ void shouldMapExceptionCdi() { .statusCode(200); } + @Test + void shouldInterceptDefaultMethod() { + RestAssured.with().body(baseUrl).post("/call-with-fault-tolerance") + .then() + .statusCode(200) + .body(equalTo("Hello fallback!")); + } + @Test void shouldCreateClientSpans() { // Reset captured traces