Additional headers and status change implementation changed; addition…

…al tests added;
quarkusio · Sep 18, 2021 · 1c65d6d · 1c65d6d
1 parent 69ebd69
commit 1c65d6d
Show file tree

Hide file tree

Showing 14 changed files with 189 additions and 42 deletions.
diff --git a/.../main/java/io/quarkus/resteasy/reactive/server/servlet/runtime/ServletRequestContext.java b/.../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<CharSequ
         return this;
     }
 
+    @Override
+    public void clearResponseHeaders() {
+        for (String header : response.getHeaderNames()){
+            response.setHeader(header, null);
+        }
+    }
+
     @Override
     public Iterable<Map.Entry<String, String>> getAllResponseHeaders() {
         List<Map.Entry<String, String>> ret = new ArrayList<>();

diff --git a/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java
@@ -193,7 +193,7 @@ public List<HandlerChainCustomizer> 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<HandlerChainCustomizer> 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/...test/java/io/quarkus/resteasy/reactive/server/test/responseheader/ResponseHeaderTest.java b/...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<String, String> 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<String, String> 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<String> 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<String> 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<String> throwExceptionUni() {
         public Multi<String> throwExceptionMulti() {
             return Multi.createFrom().failure(new IllegalArgumentException());
         }
+
+        @ResponseHeader(headers = {
+                @Header(name = "Access-Control-Allow-Origin", value = "*")
+        })
+        @Path("/exception_completion")
+        public CompletionStage<String> 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/...test/java/io/quarkus/resteasy/reactive/server/test/responsestatus/ResponseStatusTest.java b/...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<String> getTestMulti() {
             return Multi.createFrom().item("test");
         }
 
+        @Status(201)
+        @GET
+        @Path("/completion")
+        public CompletionStage<String> 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<String> throwExceptionMulti() {
             return Multi.createFrom().failure(new IllegalArgumentException());
         }
 
+        @Status(201)
+        @GET
+        @Path("/exception_completion")
+        public CompletionStage<String> throwExceptionCompletion() {
+            return CompletableFuture.failedFuture(new IllegalArgumentException());
+        }
+
+        @Status(201)
+        @GET
+        @Path("/exception_plain")
+        public String throwExceptionPlain() {
+            throw new IllegalArgumentException();
+        }
+
     }
 }
diff --git a/...ava/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java b/...ava/io/quarkus/resteasy/reactive/server/runtime/responseheader/ResponseHeaderHandler.java
@@ -19,7 +19,9 @@ public Map<String, String> getHeaders() {
     @Override
     public void handle(ResteasyReactiveRequestContext requestContext) throws Exception {
         if (headers != null) {
-            requestContext.setResponseHeaders(headers);
+            for (Map.Entry<String, String> header : headers.entrySet()) {
+                requestContext.serverResponse().setResponseHeader(header.getKey(), header.getValue());
+            }
         }
     }
 

diff --git a/...ava/io/quarkus/resteasy/reactive/server/runtime/responsestatus/ResponseStatusHandler.java b/...ava/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/...src/main/java/org/jboss/resteasy/reactive/server/core/ResteasyReactiveRequestContext.java b/...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<UriMatch> matchedURIs;
 
-    private Map<String, String> responseHeaders;
-    private Integer responseStatus;
-
     private AsyncResponseImpl asyncResponse;
     private SseEventSinkImpl sseEventSink;
     private List<PathSegment> pathSegments;
@@ -234,22 +230,6 @@ public void setMaxPathParams(int maxPathParams) {
         }
     }
 
-    public Map<String, String> getResponseHeaders() {
-        return responseHeaders;
-    }
-
-    public void setResponseHeaders(Map<String, String> 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/...e/server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/StreamingUtil.java b/...e/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<String, String> header : context.getResponseHeaders().entrySet()) {
-                    response.addResponseHeader(header.getKey(), header.getValue());
-                }
-            }
         }
     }
 }
diff --git a/...main/java/org/jboss/resteasy/reactive/server/handlers/CompletionStageResponseHandler.java b/...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/...r/runtime/src/main/java/org/jboss/resteasy/reactive/server/handlers/ExceptionHandler.java b/...r/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/...e/src/main/java/org/jboss/resteasy/reactive/server/handlers/PublisherResponseHandler.java b/...e/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 {