Skip to content

Commit

Permalink
Copy default methods of a RestClient Reactive interface to the genera…
Browse files Browse the repository at this point in the history
…ted 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.
  • Loading branch information
Ladicek committed Nov 30, 2021
1 parent d8bae08 commit 6b19e71
Show file tree
Hide file tree
Showing 6 changed files with 98 additions and 19 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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<MethodInfo> restMethods = new ArrayList<>();

// search this class and its super interfaces for jaxrs methods
searchForJaxRsMethods(restMethods, jaxrsInterface, index);
if (restMethods.isEmpty()) {
List<MethodInfo> 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;
}

Expand Down Expand Up @@ -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));

Expand All @@ -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);
}
}
Expand Down
19 changes: 18 additions & 1 deletion integration-tests/rest-client-reactive/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
<name>Quarkus - Integration Tests - REST Client Reactive</name>

<!--todo add ssl tests-->
<!--todo test fault tolerance integration-->

<dependencies>
<!-- Client dependencies -->
Expand All @@ -26,6 +25,11 @@
<artifactId>quarkus-rest-client-reactive-jackson</artifactId>
</dependency>

<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-smallrye-fault-tolerance</artifactId>
</dependency>

<!-- Tracing dependency -->
<dependency>
<groupId>io.quarkus</groupId>
Expand Down Expand Up @@ -78,6 +82,19 @@
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-smallrye-fault-tolerance-deployment</artifactId>
<version>${project.version}</version>
<type>pom</type>
<scope>test</scope>
<exclusions>
<exclusion>
<groupId>*</groupId>
<artifactId>*</artifactId>
</exclusion>
</exclusions>
</dependency>
<dependency>
<groupId>io.quarkus</groupId>
<artifactId>quarkus-vertx-http-deployment</artifactId>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ public class ClientCallingResource {
@RestClient
ClientWithExceptionMapper clientWithExceptionMapper;

@RestClient
FaultToleranceClient faultToleranceClient;

@Inject
InMemorySpanExporter inMemorySpanExporter;

Expand Down Expand Up @@ -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<Void> success(RoutingContext rc, String body) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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!";
}
}
Original file line number Diff line number Diff line change
@@ -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}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 6b19e71

Please sign in to comment.