From e28b6125c680f5c8443026d324f399fefa71e308 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Tue, 28 Jul 2020 12:42:56 +1000 Subject: [PATCH] If no Accept header is set default to the first 'produces' type Currently if the request does not include an Accept header the method will still be invoked but the content type will not always be automatically set. Fixes #10960 --- .../quarkus/vertx/web/deployment/Methods.java | 2 +- .../web/deployment/VertxWebProcessor.java | 34 +++++++++-------- .../vertx/web/mutiny/SyncRouteTest.java | 38 +++++++++++++++++++ .../main/java/io/quarkus/vertx/web/Route.java | 5 ++- .../vertx/web/runtime/RouteHandlers.java | 21 +++++++--- 5 files changed, 77 insertions(+), 23 deletions(-) diff --git a/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/Methods.java b/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/Methods.java index ed63d6c1eb430..24530fad50ffb 100644 --- a/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/Methods.java +++ b/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/Methods.java @@ -163,7 +163,7 @@ class Methods { static final MethodDescriptor OBJECT_CONSTRUCTOR = MethodDescriptor.ofConstructor(Object.class); static final MethodDescriptor ROUTE_HANDLERS_SET_CONTENT_TYPE = MethodDescriptor - .ofMethod(RouteHandlers.class, "setContentType", void.class, RoutingContext.class); + .ofMethod(RouteHandlers.class, "setContentType", void.class, RoutingContext.class, String.class); static final MethodDescriptor OPTIONAL_OF_NULLABLE = MethodDescriptor .ofMethod(Optional.class, "ofNullable", Optional.class, Object.class); diff --git a/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/VertxWebProcessor.java b/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/VertxWebProcessor.java index e5bf83a03fa66..404c6040a210e 100644 --- a/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/VertxWebProcessor.java +++ b/extensions/vertx-web/deployment/src/main/java/io/quarkus/vertx/web/deployment/VertxWebProcessor.java @@ -232,14 +232,6 @@ void addAdditionalRoutes( for (AnnotationInstance route : businessMethod.getRoutes()) { String routeString = route.toString(true); Handler routeHandler = routeHandlers.get(routeString); - if (routeHandler == null) { - String handlerClass = generateHandler(new HandlerDescriptor(businessMethod.getMethod()), - businessMethod.getBean(), businessMethod.getMethod(), classOutput, transformedAnnotations, - routeString, reflectiveHierarchy); - reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, handlerClass)); - routeHandler = recorder.createHandler(handlerClass); - routeHandlers.put(routeString, routeHandler); - } AnnotationValue regexValue = route.value(VALUE_REGEX); AnnotationValue pathValue = route.value(VALUE_PATH); @@ -252,6 +244,7 @@ void addAdditionalRoutes( String regex = null; String[] produces = producesValue.asStringArray(); String[] consumes = consumesValue.asStringArray(); + HttpMethod[] methods = Arrays.stream(methodsValue.asEnumArray()).map(HttpMethod::valueOf) .toArray(HttpMethod[]::new); Integer order = orderValue.asInt(); @@ -287,6 +280,15 @@ void addAdditionalRoutes( consumes = baseConsumes; } + if (routeHandler == null) { + String handlerClass = generateHandler(new HandlerDescriptor(businessMethod.getMethod()), + businessMethod.getBean(), businessMethod.getMethod(), classOutput, transformedAnnotations, + routeString, reflectiveHierarchy, produces.length > 0 ? produces[0] : null); + reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, handlerClass)); + routeHandler = recorder.createHandler(handlerClass); + routeHandlers.put(routeString, routeHandler); + } + RouteMatcher matcher = new RouteMatcher(path, regex, produces, consumes, methods, order); matchers.put(matcher, businessMethod.getMethod()); Function routeFunction = recorder.createRouteFunction(matcher, @@ -316,7 +318,7 @@ void addAdditionalRoutes( for (AnnotatedRouteFilterBuildItem filterMethod : routeFilterBusinessMethods) { String handlerClass = generateHandler(new HandlerDescriptor(filterMethod.getMethod()), filterMethod.getBean(), filterMethod.getMethod(), classOutput, transformedAnnotations, - filterMethod.getRouteFilter().toString(true), reflectiveHierarchy); + filterMethod.getRouteFilter().toString(true), reflectiveHierarchy, null); reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, handlerClass)); Handler routingHandler = recorder.createHandler(handlerClass); AnnotationValue priorityValue = filterMethod.getRouteFilter().value(); @@ -408,7 +410,7 @@ private void validateRouteMethod(BeanInfo bean, MethodInfo method, TransformedAn private String generateHandler(HandlerDescriptor desc, BeanInfo bean, MethodInfo method, ClassOutput classOutput, TransformedAnnotationsBuildItem transformedAnnotations, String hashSuffix, - BuildProducer reflectiveHierarchy) { + BuildProducer reflectiveHierarchy, String defaultProduces) { String baseName; if (bean.getImplClazz().enclosingClass() != null) { @@ -446,7 +448,7 @@ private String generateHandler(HandlerDescriptor desc, BeanInfo bean, MethodInfo implementConstructor(bean, invokerCreator, beanField, contextField, containerField); implementInvoke(desc, bean, method, invokerCreator, beanField, contextField, containerField, transformedAnnotations, - reflectiveHierarchy); + reflectiveHierarchy, defaultProduces); invokerCreator.close(); return generatedName.replace('/', '.'); @@ -482,7 +484,7 @@ void implementConstructor(BeanInfo bean, ClassCreator invokerCreator, FieldCreat void implementInvoke(HandlerDescriptor descriptor, BeanInfo bean, MethodInfo method, ClassCreator invokerCreator, FieldCreator beanField, FieldCreator contextField, FieldCreator containerField, TransformedAnnotationsBuildItem transformedAnnotations, - BuildProducer reflectiveHierarchy) { + BuildProducer reflectiveHierarchy, String defaultProduces) { // The descriptor is: void invoke(RoutingContext rc) MethodCreator invoke = invokerCreator.getMethodCreator("invoke", void.class, RoutingContext.class); ResultHandle beanHandle = invoke.readInstanceField(beanField.getFieldDescriptor(), invoke.getThis()); @@ -544,12 +546,14 @@ void implementInvoke(HandlerDescriptor descriptor, BeanInfo bean, MethodInfo met MethodDescriptor methodDescriptor = MethodDescriptor .ofMethod(bean.getImplClazz().name().toString(), method.name(), returnType, parameterTypes); + // If no content-type header is set then try to use the most acceptable content type + // the business method can override this manually if required + invoke.invokeStaticMethod(Methods.ROUTE_HANDLERS_SET_CONTENT_TYPE, routingContext, + defaultProduces == null ? invoke.loadNull() : invoke.load(defaultProduces)); + // Invoke the business method handler ResultHandle res = invoke.invokeVirtualMethod(methodDescriptor, beanInstanceHandle, paramHandles); - // If no content-type header is set then try to use the most acceptable content type - invoke.invokeStaticMethod(Methods.ROUTE_HANDLERS_SET_CONTENT_TYPE, routingContext); - // Get the response: HttpServerResponse response = rc.response() MethodDescriptor end = Methods.getEndMethodForContentType(descriptor); if (descriptor.isReturningUni()) { diff --git a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SyncRouteTest.java b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SyncRouteTest.java index 8357a73d2ed6c..de9c6dae80c94 100644 --- a/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SyncRouteTest.java +++ b/extensions/vertx-web/deployment/src/test/java/io/quarkus/vertx/web/mutiny/SyncRouteTest.java @@ -3,13 +3,26 @@ import static io.restassured.RestAssured.when; import static org.hamcrest.Matchers.*; +import java.net.URL; + +import javax.json.Json; +import javax.json.JsonObject; +import javax.json.stream.JsonParser; + +import org.apache.http.client.methods.CloseableHttpResponse; +import org.apache.http.client.methods.HttpGet; +import org.apache.http.impl.client.CloseableHttpClient; +import org.apache.http.impl.client.HttpClientBuilder; import org.jboss.shrinkwrap.api.ShrinkWrap; import org.jboss.shrinkwrap.api.spec.JavaArchive; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; import io.quarkus.test.QuarkusUnitTest; +import io.quarkus.test.common.http.TestHTTPResource; import io.quarkus.vertx.web.Route; +import io.quarkus.vertx.web.RoutingExchange; import io.vertx.core.buffer.Buffer; import io.vertx.ext.web.RoutingContext; @@ -19,6 +32,9 @@ public class SyncRouteTest { static final QuarkusUnitTest config = new QuarkusUnitTest() .setArchiveProducer(() -> ShrinkWrap.create(JavaArchive.class).addClasses(SimpleBean.class)); + @TestHTTPResource + URL url; + @Test public void testSynchronousRoute() { when().get("hello-sync").then().statusCode(200) @@ -49,6 +65,21 @@ public void testSynchronousRoute() { .body(containsString("boom")); } + //https://github.com/quarkusio/quarkus/issues/10960 + @Test + public void testNoAcceptHeaderContentType() throws Exception { + //RESTAssured always sets an Accept header + try (CloseableHttpClient client = HttpClientBuilder.create().build()) { + CloseableHttpResponse res = client.execute(new HttpGet(this.url.toExternalForm() + "/default-content-type")); + JsonParser parser = Json.createParser(res.getEntity().getContent()); + parser.next(); + JsonObject obj = parser.getObject(); + Assertions.assertEquals("neo", obj.getString("name")); + Assertions.assertEquals(12345, obj.getInt("id")); + Assertions.assertEquals("application/json", res.getFirstHeader("content-type").getValue()); + } + } + static class SimpleBean { @Route(path = "hello-sync") @@ -87,6 +118,13 @@ Person getPersonUtf8(RoutingContext context) { return new Person("neo", 12345); } + @Route(path = "default-content-type", produces = "application/json") + void hi(RoutingExchange routing) { + routing.ok(new io.vertx.core.json.JsonObject() + .put("name", "neo") + .put("id", 12345).encode()); + } + } static class Person { diff --git a/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/Route.java b/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/Route.java index 634e29dc11065..19a460b6e5f1e 100644 --- a/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/Route.java +++ b/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/Route.java @@ -110,7 +110,7 @@ /** * If set to a positive number, it indicates the place of the route in the chain. * - * @see io.vertx.ext.web.Route#order() + * @see io.vertx.ext.web.Route#order(int) */ int order() default 0; @@ -118,6 +118,9 @@ * Used for content-based routing. *

* If no {@code Content-Type} header is set then try to use the most acceptable content type. + * + * If the request does not contain an 'Accept' header and no content type is explicitly set in the + * handler then the content type will be set to the first content type in the array. * * @see io.vertx.ext.web.Route#produces(String) * @see RoutingContext#getAcceptableContentType() diff --git a/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/runtime/RouteHandlers.java b/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/runtime/RouteHandlers.java index a2f37cac5b35d..7bc476142e66b 100644 --- a/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/runtime/RouteHandlers.java +++ b/extensions/vertx-web/runtime/src/main/java/io/quarkus/vertx/web/runtime/RouteHandlers.java @@ -5,6 +5,7 @@ import io.quarkus.arc.Arc; import io.quarkus.arc.impl.LazyValue; import io.quarkus.security.identity.SecurityIdentity; +import io.vertx.core.Handler; import io.vertx.core.http.HttpServerResponse; import io.vertx.ext.web.RoutingContext; @@ -18,14 +19,22 @@ private RouteHandlers() { private static final LazyValue> SECURITY_IDENTITY_EVENT = new LazyValue<>( RouteHandlers::createEvent); - public static void setContentType(RoutingContext context) { + public static void setContentType(RoutingContext context, String defaultContentType) { HttpServerResponse response = context.response(); - if (response.headers().get(CONTENT_TYPE) == null) { - String acceptableContentType = context.getAcceptableContentType(); - if (acceptableContentType != null) { - response.putHeader(CONTENT_TYPE, acceptableContentType); + context.addHeadersEndHandler(new Handler() { + @Override + public void handle(Void aVoid) { + //use a listener to set the content type if it has not been set + if (response.headers().get(CONTENT_TYPE) == null) { + String acceptableContentType = context.getAcceptableContentType(); + if (acceptableContentType != null) { + response.putHeader(CONTENT_TYPE, acceptableContentType); + } else if (defaultContentType != null) { + response.putHeader(CONTENT_TYPE, defaultContentType); + } + } } - } + }); } static void fireSecurityIdentity(SecurityIdentity identity) {