From 1c65d6d439206285cc7d593de3cf877913cb60ca Mon Sep 17 00:00:00 2001 From: Yelzhas Suleimenov Date: Sat, 18 Sep 2021 14:41:57 +0600 Subject: [PATCH] Additional headers and status change implementation changed; additional tests added; --- .../runtime/ServletRequestContext.java | 12 +++ .../deployment/ResteasyReactiveProcessor.java | 4 +- .../responseheader/ResponseHeaderTest.java | 79 +++++++++++++++++++ .../responsestatus/ResponseStatusTest.java | 71 +++++++++++++++++ .../responseheader/ResponseHeaderHandler.java | 4 +- .../responsestatus/ResponseStatusHandler.java | 2 +- .../core/ResteasyReactiveRequestContext.java | 20 ----- .../reactive/server/core/StreamingUtil.java | 11 +-- .../CompletionStageResponseHandler.java | 1 + .../server/handlers/ExceptionHandler.java | 1 + .../handlers/PublisherResponseHandler.java | 1 + .../server/handlers/UniResponseHandler.java | 11 +-- .../server/spi/ServerHttpResponse.java | 4 + .../VertxResteasyReactiveRequestContext.java | 10 +++ 14 files changed, 189 insertions(+), 42 deletions(-) diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive-servlet/runtime/src/main/java/io/quarkus/resteasy/reactive/server/servlet/runtime/ServletRequestContext.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive-servlet/runtime/src/main/java/io/quarkus/resteasy/reactive/server/servlet/runtime/ServletRequestContext.java index f8f54895427da..8b72339ddeda5 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive-servlet/runtime/src/main/java/io/quarkus/resteasy/reactive/server/servlet/runtime/ServletRequestContext.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive-servlet/runtime/src/main/java/io/quarkus/resteasy/reactive/server/servlet/runtime/ServletRequestContext.java @@ -323,6 +323,11 @@ public ServerHttpResponse setStatusCode(int code) { return this; } + @Override + public int getStatusCode() { + return response.getStatus(); + } + @Override public ServerHttpResponse end() { try { @@ -388,6 +393,13 @@ public ServerHttpResponse setResponseHeader(CharSequence name, Iterable> getAllResponseHeaders() { List> ret = new ArrayList<>(); diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java index d5d2723f8e455..2cd4d8cd01ae5 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java @@ -193,7 +193,7 @@ public List scan(MethodInfo method, ClassInfo actualEndp ResponseStatusHandler handler = new ResponseStatusHandler(); handler.setStatus(responseStatusValue.asInt()); return Collections.singletonList(new FixedHandlerChainCustomizer(handler, - HandlerChainCustomizer.Phase.AFTER_METHOD_INVOKE)); + HandlerChainCustomizer.Phase.BEFORE_METHOD_INVOKE)); } }); } @@ -223,7 +223,7 @@ public List scan(MethodInfo method, ClassInfo actualEndp ResponseHeaderHandler handler = new ResponseHeaderHandler(); handler.setHeaders(headers); return Collections.singletonList(new FixedHandlerChainCustomizer(handler, - HandlerChainCustomizer.Phase.AFTER_METHOD_INVOKE)); + HandlerChainCustomizer.Phase.BEFORE_METHOD_INVOKE)); } }); } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responseheader/ResponseHeaderTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responseheader/ResponseHeaderTest.java index 42a079e99d66c..92ffc9a9dad52 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responseheader/ResponseHeaderTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responseheader/ResponseHeaderTest.java @@ -3,6 +3,8 @@ import static org.junit.jupiter.api.Assertions.*; import java.util.Map; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; import javax.ws.rs.GET; import javax.ws.rs.Path; @@ -52,6 +54,32 @@ public void should_return_added_headers_multi() { .headers(expectedHeaders); } + @Test + public void should_return_added_headers_completion() { + Map expectedHeaders = Map.of( + "Access-Control-Allow-Origin", "*", + "Keep-Alive", "timeout=5, max=997"); + RestAssured + .given() + .get("/test/completion") + .then() + .statusCode(200) + .headers(expectedHeaders); + } + + @Test + public void should_return_added_headers_plain() { + Map expectedHeaders = Map.of( + "Access-Control-Allow-Origin", "*", + "Keep-Alive", "timeout=5, max=997"); + RestAssured + .given() + .get("/test/plain") + .then() + .statusCode(200) + .headers(expectedHeaders); + } + @Test public void should_throw_exception_without_headers_uni() { Headers headers = RestAssured.given().get("/test/exception_uni") @@ -67,6 +95,20 @@ public void should_throw_exception_without_headers_multi() { assertFalse(headers.hasHeaderWithName("Access-Control-Allow-Origin")); } + @Test + public void should_throw_exception_without_headers_completion() { + Headers headers = RestAssured.given().get("/test/exception_completion") + .then().extract().headers(); + assertFalse(headers.hasHeaderWithName("Access-Control-Allow-Origin")); + } + + @Test + public void should_throw_exception_without_headers_plain() { + Headers headers = RestAssured.given().get("/test/exception_plain") + .then().extract().headers(); + assertFalse(headers.hasHeaderWithName("Access-Control-Allow-Origin")); + } + @Path("/test") public static class TestResource { @@ -90,6 +132,26 @@ public Multi getTestMulti() { return Multi.createFrom().item("test"); } + @ResponseHeader(headers = { + @Header(name = "Access-Control-Allow-Origin", value = "*"), + @Header(name = "Keep-Alive", value = "timeout=5, max=997"), + }) + @GET + @Path("/completion") + public CompletionStage getTestCompletion() { + return CompletableFuture.supplyAsync(() -> "test"); + } + + @ResponseHeader(headers = { + @Header(name = "Access-Control-Allow-Origin", value = "*"), + @Header(name = "Keep-Alive", value = "timeout=5, max=997"), + }) + @GET + @Path("/plain") + public String getTestPlain() { + return "test"; + } + @ResponseHeader(headers = { @Header(name = "Access-Control-Allow-Origin", value = "*") }) @@ -107,5 +169,22 @@ public Uni throwExceptionUni() { public Multi throwExceptionMulti() { return Multi.createFrom().failure(new IllegalArgumentException()); } + + @ResponseHeader(headers = { + @Header(name = "Access-Control-Allow-Origin", value = "*") + }) + @Path("/exception_completion") + public CompletionStage throwExceptionCompletion() { + return CompletableFuture.failedFuture(new IllegalArgumentException()); + } + + @ResponseHeader(headers = { + @Header(name = "Access-Control-Allow-Origin", value = "*") + }) + @GET + @Path("/exception_plain") + public String throwExceptionPlain() { + throw new IllegalArgumentException(); + } } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responsestatus/ResponseStatusTest.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responsestatus/ResponseStatusTest.java index 2eb7fc587e0fd..3389d9ce9c23a 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responsestatus/ResponseStatusTest.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/deployment/src/test/java/io/quarkus/resteasy/reactive/server/test/responsestatus/ResponseStatusTest.java @@ -1,5 +1,8 @@ package io.quarkus.resteasy.reactive.server.test.responsestatus; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.CompletionStage; + import javax.ws.rs.GET; import javax.ws.rs.Path; @@ -40,6 +43,26 @@ public void should_return_changed_status_multi() { .statusCode(expectedStatus); } + @Test + public void should_return_changed_status_completion() { + int expectedStatus = 201; + RestAssured + .given() + .get("/test/completion") + .then() + .statusCode(expectedStatus); + } + + @Test + public void should_return_changed_status_plain() { + int expectedStatus = 201; + RestAssured + .given() + .get("/test/plain") + .then() + .statusCode(expectedStatus); + } + @Test public void should_not_change_status_uni() { int expectedStatus = 500; @@ -60,6 +83,26 @@ public void should_not_change_status_multi() { .statusCode(expectedStatus); } + @Test + public void should_not_change_status_completion() { + int expectedStatus = 500; + RestAssured + .given() + .get("/test/exception_completion") + .then() + .statusCode(expectedStatus); + } + + @Test + public void should_not_change_status_plain() { + int expectedStatus = 500; + RestAssured + .given() + .get("/test/exception_plain") + .then() + .statusCode(expectedStatus); + } + @Path("/test") public static class TestResource { @@ -77,6 +120,20 @@ public Multi getTestMulti() { return Multi.createFrom().item("test"); } + @Status(201) + @GET + @Path("/completion") + public CompletionStage getTestCompletion() { + return CompletableFuture.supplyAsync(() -> "test"); + } + + @Status(201) + @GET + @Path("/plain") + public String getTestPlain() { + return "test"; + } + @Status(201) @GET @Path(("/exception_uni")) @@ -91,5 +148,19 @@ public Multi throwExceptionMulti() { return Multi.createFrom().failure(new IllegalArgumentException()); } + @Status(201) + @GET + @Path("/exception_completion") + public CompletionStage throwExceptionCompletion() { + return CompletableFuture.failedFuture(new IllegalArgumentException()); + } + + @Status(201) + @GET + @Path("/exception_plain") + public String throwExceptionPlain() { + throw new IllegalArgumentException(); + } + } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java index e0aa8fef30143..e734b32f92741 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java @@ -19,7 +19,9 @@ public Map getHeaders() { @Override public void handle(ResteasyReactiveRequestContext requestContext) throws Exception { if (headers != null) { - requestContext.setResponseHeaders(headers); + for (Map.Entry header : headers.entrySet()) { + requestContext.serverResponse().setResponseHeader(header.getKey(), header.getValue()); + } } } diff --git a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responsestatus/ResponseStatusHandler.java b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responsestatus/ResponseStatusHandler.java index b8d5f6c12695c..bbf672c17bf46 100644 --- a/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responsestatus/ResponseStatusHandler.java +++ b/extensions/resteasy-reactive/quarkus-resteasy-reactive/runtime/src/main/java/io/quarkus/resteasy/reactive/server/runtime/responsestatus/ResponseStatusHandler.java @@ -17,6 +17,6 @@ public int getStatus() { @Override public void handle(ResteasyReactiveRequestContext requestContext) throws Exception { - requestContext.setResponseStatus(status); + requestContext.serverResponse().setStatusCode(status); } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java index dc0ec7d651b2c..aaa97a3027348 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java @@ -14,7 +14,6 @@ import java.util.Deque; import java.util.LinkedList; import java.util.List; -import java.util.Map; import java.util.concurrent.Executor; import java.util.regex.Matcher; import javax.ws.rs.core.Cookie; @@ -133,9 +132,6 @@ public abstract class ResteasyReactiveRequestContext */ private List matchedURIs; - private Map responseHeaders; - private Integer responseStatus; - private AsyncResponseImpl asyncResponse; private SseEventSinkImpl sseEventSink; private List pathSegments; @@ -234,22 +230,6 @@ public void setMaxPathParams(int maxPathParams) { } } - public Map getResponseHeaders() { - return responseHeaders; - } - - public void setResponseHeaders(Map responseHeaders) { - this.responseHeaders = responseHeaders; - } - - public Integer getResponseStatus() { - return responseStatus; - } - - public void setResponseStatus(Integer responseStatus) { - this.responseStatus = responseStatus; - } - public String getPathParam(int index) { return doGetPathParam(index, pathParamValues); } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/StreamingUtil.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/StreamingUtil.java index 594ccea581648..a4983bcd42ddd 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/StreamingUtil.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/StreamingUtil.java @@ -4,7 +4,6 @@ import java.io.IOException; import java.lang.reflect.Type; import java.nio.charset.StandardCharsets; -import java.util.Map; import java.util.concurrent.CompletableFuture; import java.util.concurrent.CompletionStage; import javax.ws.rs.RuntimeType; @@ -77,15 +76,11 @@ public static void setHeaders(ResteasyReactiveRequestContext context, ServerHttp // FIXME: spec says we should flush the headers when first message is sent or when the resource method returns, whichever // happens first if (!response.headWritten()) { - response.setStatusCode( - context.getResponseStatus() != null ? context.getResponseStatus() : Response.Status.OK.getStatusCode()); + if (response.getStatusCode() == 0) { + response.setStatusCode(Response.Status.OK.getStatusCode()); + } response.setResponseHeader(HttpHeaders.CONTENT_TYPE, context.getResponseContentType().toString()); response.setChunked(true); - if (context.getResponseHeaders() != null) { - for (Map.Entry header : context.getResponseHeaders().entrySet()) { - response.addResponseHeader(header.getKey(), header.getValue()); - } - } } } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/CompletionStageResponseHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/CompletionStageResponseHandler.java index 78730e984c11d..9be1673bc28c0 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/CompletionStageResponseHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/CompletionStageResponseHandler.java @@ -15,6 +15,7 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti result.handle((v, t) -> { if (t != null) { + requestContext.serverResponse().clearResponseHeaders(); requestContext.handleException(t); } else { requestContext.setResult(v); diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ExceptionHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ExceptionHandler.java index dc6a06aa29c66..4791a382a26e0 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ExceptionHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ExceptionHandler.java @@ -10,6 +10,7 @@ public class ExceptionHandler implements ServerRestHandler { @Override public void handle(ResteasyReactiveRequestContext requestContext) throws Exception { + requestContext.serverResponse().clearResponseHeaders(); requestContext.mapExceptionIfPresent(); } } diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/PublisherResponseHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/PublisherResponseHandler.java index edb1625f17b8c..84c8d7bf6a726 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/PublisherResponseHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/PublisherResponseHandler.java @@ -147,6 +147,7 @@ public void onError(Throwable t) { protected void handleException(ResteasyReactiveRequestContext requestContext, Throwable t) { // in truth we can only send an exception if we haven't sent the headers yet, otherwise // it will appear to be an SSE value, which is incorrect, so we should only log it and close the connection + requestContext.serverResponse().clearResponseHeaders(); if (requestContext.serverResponse().headWritten()) { log.error("Exception in SSE server handling, impossible to send it to client", t); } else { diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/UniResponseHandler.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/UniResponseHandler.java index 9856ec79b3513..fb05884ba388d 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/UniResponseHandler.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/UniResponseHandler.java @@ -1,9 +1,7 @@ package org.jboss.resteasy.reactive.server.handlers; import io.smallrye.mutiny.Uni; -import java.util.Map; import java.util.function.Consumer; -import javax.ws.rs.core.Response; import org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext; import org.jboss.resteasy.reactive.server.spi.ServerRestHandler; @@ -20,19 +18,12 @@ public void handle(ResteasyReactiveRequestContext requestContext) throws Excepti @Override public void accept(Object v) { requestContext.setResult(v); - requestContext.serverResponse().setStatusCode( - requestContext.getResponseStatus() != null ? requestContext.getResponseStatus() - : Response.Status.OK.getStatusCode()); - if (requestContext.getResponseHeaders() != null) { - for (Map.Entry header : requestContext.getResponseHeaders().entrySet()) { - requestContext.serverResponse().addResponseHeader(header.getKey(), header.getValue()); - } - } requestContext.resume(); } }, new Consumer() { @Override public void accept(Throwable t) { + requestContext.serverResponse().clearResponseHeaders(); requestContext.resume(t, true); } }); diff --git a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ServerHttpResponse.java b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ServerHttpResponse.java index ae4f0b3b33d45..20b4c290eb934 100644 --- a/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ServerHttpResponse.java +++ b/independent-projects/resteasy-reactive/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/spi/ServerHttpResponse.java @@ -10,6 +10,8 @@ public interface ServerHttpResponse { ServerHttpResponse setStatusCode(int code); + int getStatusCode(); + ServerHttpResponse end(); boolean headWritten(); @@ -24,6 +26,8 @@ public interface ServerHttpResponse { ServerHttpResponse setResponseHeader(CharSequence name, Iterable values); + void clearResponseHeaders(); + Iterable> getAllResponseHeaders(); boolean closed(); diff --git a/independent-projects/resteasy-reactive/server/vertx/src/main/java/org/jboss/resteasy/reactive/server/vertx/VertxResteasyReactiveRequestContext.java b/independent-projects/resteasy-reactive/server/vertx/src/main/java/org/jboss/resteasy/reactive/server/vertx/VertxResteasyReactiveRequestContext.java index 25934998c399c..5fbafb14760b2 100644 --- a/independent-projects/resteasy-reactive/server/vertx/src/main/java/org/jboss/resteasy/reactive/server/vertx/VertxResteasyReactiveRequestContext.java +++ b/independent-projects/resteasy-reactive/server/vertx/src/main/java/org/jboss/resteasy/reactive/server/vertx/VertxResteasyReactiveRequestContext.java @@ -306,6 +306,11 @@ public ServerHttpResponse setStatusCode(int code) { return this; } + @Override + public int getStatusCode() { + return response.getStatusCode(); + } + @Override public ServerHttpResponse end() { if (!response.ended()) { @@ -349,6 +354,11 @@ public ServerHttpResponse setResponseHeader(CharSequence name, Iterable> getAllResponseHeaders() { return response.headers();