From 819ec8186c847e73c10147388e45c33988d8a4b4 Mon Sep 17 00:00:00 2001 From: George Gastaldi Date: Mon, 26 Aug 2024 22:20:43 -0300 Subject: [PATCH] Fix null Response.getEntity in REST Reactive - Fixes #25496 - Fixes #41887 --- .../reactive/common/jaxrs/ResponseImpl.java | 9 ++-- .../client/main/ClientCallingResource.java | 21 ++++++++ .../rest/client/main/JAXRSResponseClient.java | 20 +++++++ .../io/quarkus/it/rest/client/BasicTest.java | 52 +++++++++++++------ 4 files changed, 80 insertions(+), 22 deletions(-) create mode 100644 integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/JAXRSResponseClient.java diff --git a/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/jaxrs/ResponseImpl.java b/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/jaxrs/ResponseImpl.java index 6d167482c9456a..6a4dccfb51363a 100644 --- a/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/jaxrs/ResponseImpl.java +++ b/independent-projects/resteasy-reactive/common/runtime/src/main/java/org/jboss/resteasy/reactive/common/jaxrs/ResponseImpl.java @@ -88,19 +88,18 @@ public Object getEntity() { checkClosed(); // this check seems very ugly, but it seems to be needed by the TCK // this will likely require a better solution - if (entity instanceof GenericEntity) { - GenericEntity genericEntity = (GenericEntity) entity; + if (entity instanceof GenericEntity genericEntity) { if (genericEntity.getRawType().equals(genericEntity.getType())) { return ((GenericEntity) entity).getEntity(); } } - return entity; + return entity == null ? entityStream : entity; } protected void setEntity(Object entity) { this.entity = entity; - if (entity instanceof InputStream) { - this.entityStream = (InputStream) entity; + if (entity instanceof InputStream inputStream) { + this.entityStream = inputStream; } } 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 11a803ea707411..7b0a611c8f37be 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 @@ -1,6 +1,8 @@ package io.quarkus.it.rest.client.main; import java.net.URI; +import java.time.Duration; +import java.time.temporal.ChronoUnit; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Function; @@ -10,6 +12,7 @@ import jakarta.enterprise.event.Observes; import jakarta.inject.Inject; import jakarta.ws.rs.core.MediaType; +import jakarta.ws.rs.core.Response; import org.eclipse.microprofile.rest.client.inject.RestClient; import org.jboss.resteasy.reactive.RestResponse; @@ -260,6 +263,24 @@ void init(@Observes Router router) { rc.response().setStatusCode(500).end(e.getCause().getClass().getSimpleName()); } }); + + router.post("/preserve-response-entity").blockingHandler(rc -> { + String url = rc.body().asString(); + JAXRSResponseClient client = QuarkusRestClientBuilder.newBuilder().baseUri(URI.create(url)) + .build(JAXRSResponseClient.class); + Response response = client.call(); + Response newResponse = Response.fromResponse(response).build(); + rc.response().end(newResponse.getEntity().toString()); + }); + + router.post("/preserve-response-entity-async").blockingHandler(rc -> { + String url = rc.body().asString(); + final JAXRSResponseClient client = QuarkusRestClientBuilder.newBuilder().baseUri(URI.create(url)) + .build(JAXRSResponseClient.class); + Response response = client.asyncCall().await().atMost(Duration.of(5, ChronoUnit.SECONDS)); + rc.response().end(response.getEntity().toString()); + }); + } private Future success(RoutingContext rc, String body) { diff --git a/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/JAXRSResponseClient.java b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/JAXRSResponseClient.java new file mode 100644 index 00000000000000..d066ddf03ce096 --- /dev/null +++ b/integration-tests/rest-client-reactive/src/main/java/io/quarkus/it/rest/client/main/JAXRSResponseClient.java @@ -0,0 +1,20 @@ +package io.quarkus.it.rest.client.main; + +import jakarta.ws.rs.GET; +import jakarta.ws.rs.Path; +import jakarta.ws.rs.core.Response; + +import org.eclipse.microprofile.rest.client.inject.RegisterRestClient; + +import io.smallrye.mutiny.Uni; + +@Path("/client-logger") +@RegisterRestClient(configKey = "w-client-logger") +public interface JAXRSResponseClient { + + @GET + Response call(); + + @GET + Uni asyncCall(); +} 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 0dc59698a24443..9b8fc080ee1a69 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 @@ -1,12 +1,13 @@ package io.quarkus.it.rest.client; import static io.restassured.RestAssured.get; -import static io.restassured.RestAssured.given; +import static io.restassured.RestAssured.with; import static java.util.stream.Collectors.counting; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.entry; -import static org.awaitility.Awaitility.await; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; import java.time.Duration; import java.util.List; @@ -40,26 +41,26 @@ public class BasicTest { @Test public void shouldMakeTextRequest() { - Response response = RestAssured.with().body(helloUrl).post("/call-hello-client"); + Response response = with().body(helloUrl).post("/call-hello-client"); assertThat(response.asString()).isEqualTo("Hello, JohnJohn"); } @Test public void shouldMakeJsonRequestAndGetTextResponse() { - Response response = RestAssured.with().body(helloUrl).post("/call-helloFromMessage-client"); + Response response = with().body(helloUrl).post("/call-helloFromMessage-client"); assertThat(response.asString()).isEqualTo("Hello world"); } @Test public void restResponseShouldWorkWithNonSuccessfulResponse() { - Response response = RestAssured.with().body(helloUrl).post("/rest-response"); + Response response = with().body(helloUrl).post("/rest-response"); assertThat(response.asString()).isEqualTo("405"); } @SuppressWarnings({ "rawtypes", "unchecked" }) @Test void shouldMakeJsonRequest() { - List results = RestAssured.with().body(appleUrl).post("/call-client") + List results = with().body(appleUrl).post("/call-client") .then() .statusCode(200) .contentType("application/json") @@ -73,7 +74,7 @@ void shouldMakeJsonRequest() { @Test void shouldRetryOnFailure() { - RestAssured.with().body(appleUrl).post("/call-client-retry") + with().body(appleUrl).post("/call-client-retry") .then() .statusCode(200) .body(equalTo("4")); @@ -81,42 +82,42 @@ void shouldRetryOnFailure() { @Test void shouldLogWithExplicitLogger() { - RestAssured.with().body(baseUrl).post("/call-client-with-explicit-client-logger") + with().body(baseUrl).post("/call-client-with-explicit-client-logger") .then() .statusCode(200); } @Test void shouldLogWithGlobalLogger() { - RestAssured.with().body(baseUrl).post("/call-client-with-global-client-logger") + with().body(baseUrl).post("/call-client-with-global-client-logger") .then() .statusCode(200); } @Test void shouldLogCdiWithGlobalLogger() { - RestAssured.with().body(baseUrl).post("/call-cdi-client-with-global-client-logger") + with().body(baseUrl).post("/call-cdi-client-with-global-client-logger") .then() .statusCode(200); } @Test void shouldMapException() { - RestAssured.with().body(baseUrl).post("/call-client-with-exception-mapper") + with().body(baseUrl).post("/call-client-with-exception-mapper") .then() .statusCode(200); } @Test void shouldMapExceptionCdi() { - RestAssured.with().body(baseUrl).post("/call-cdi-client-with-exception-mapper") + with().body(baseUrl).post("/call-cdi-client-with-exception-mapper") .then() .statusCode(200); } @Test void shouldInterceptDefaultMethod() { - RestAssured.with().body(baseUrl).post("/call-with-fault-tolerance") + with().body(baseUrl).post("/call-with-fault-tolerance") .then() .statusCode(200) .body(equalTo("Hello fallback!")); @@ -125,13 +126,13 @@ void shouldInterceptDefaultMethod() { @Test void shouldApplyInterfaceLevelInterceptorBinding() { for (int i = 0; i < 2; i++) { - RestAssured.with().body(baseUrl).post("/call-with-fault-tolerance-on-interface") + with().body(baseUrl).post("/call-with-fault-tolerance-on-interface") .then() .statusCode(200) .body(equalTo("ClientWebApplicationException")); } - RestAssured.with().body(baseUrl).post("/call-with-fault-tolerance-on-interface") + with().body(baseUrl).post("/call-with-fault-tolerance-on-interface") .then() .statusCode(200) .body(equalTo("CircuitBreakerOpenException")); @@ -142,7 +143,7 @@ void shouldCreateClientSpans() { // Reset captured traces RestAssured.given().when().get("/export-clear").then().statusCode(200); - Response response = RestAssured.with().body(helloUrl).post("/call-hello-client-trace"); + Response response = with().body(helloUrl).post("/call-hello-client-trace"); assertThat(response.asString()).isEqualTo("Hello, MaryMaryMary"); String serverSpanId = null; @@ -243,12 +244,29 @@ void shouldCreateClientSpans() { @Test public void shouldConvertParamFirstToOneUsingCustomConverter() { - RestAssured.with().body(paramsUrl).post("/call-params-client-with-param-first") + with().body(paramsUrl).post("/call-params-client-with-param-first") .then() .statusCode(200) .body(equalTo("1")); } + @Test + void shouldPreserveResponseEntity() { + with().body(baseUrl).post("/preserve-response-entity") + .then() + .statusCode(200) + .body(is(notNullValue())); + } + + @Test + void shouldPreserveResponseEntityAsync() { + with().body(baseUrl).post("/preserve-response-entity-async") + .then() + .log().all() + .statusCode(200) + .body(is(notNullValue())); + } + private List> getServerSpansFromPath(final String spanName, final String urlPath) { return get("/export").body().as(new TypeRef>>() { }).stream()