Skip to content

Commit

Permalink
If no Accept header is set default to the first 'produces' type
Browse files Browse the repository at this point in the history
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
  • Loading branch information
stuartwdouglas committed Jul 28, 2020
1 parent 602bc3c commit b2b92fd
Show file tree
Hide file tree
Showing 5 changed files with 66 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -230,14 +230,6 @@ void addAdditionalRoutes(
for (AnnotationInstance route : businessMethod.getRoutes()) {
String routeString = route.toString(true);
Handler<RoutingContext> routeHandler = routeHandlers.get(routeString);
if (routeHandler == null) {
String handlerClass = generateHandler(new HandlerDescriptor(businessMethod.getMethod()),
businessMethod.getBean(), businessMethod.getMethod(), classOutput, transformedAnnotations,
routeString);
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);
Expand All @@ -250,6 +242,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();
Expand Down Expand Up @@ -285,6 +278,15 @@ void addAdditionalRoutes(
consumes = baseConsumes;
}

if (routeHandler == null) {
String handlerClass = generateHandler(new HandlerDescriptor(businessMethod.getMethod()),
businessMethod.getBean(), businessMethod.getMethod(), classOutput, transformedAnnotations,
routeString, 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<Router, io.vertx.ext.web.Route> routeFunction = recorder.createRouteFunction(matcher,
Expand Down Expand Up @@ -314,7 +316,7 @@ void addAdditionalRoutes(
for (AnnotatedRouteFilterBuildItem filterMethod : routeFilterBusinessMethods) {
String handlerClass = generateHandler(new HandlerDescriptor(filterMethod.getMethod()),
filterMethod.getBean(), filterMethod.getMethod(), classOutput, transformedAnnotations,
filterMethod.getRouteFilter().toString(true));
filterMethod.getRouteFilter().toString(true), null);
reflectiveClasses.produce(new ReflectiveClassBuildItem(false, false, handlerClass));
Handler<RoutingContext> routingHandler = recorder.createHandler(handlerClass);
AnnotationValue priorityValue = filterMethod.getRouteFilter().value();
Expand Down Expand Up @@ -405,7 +407,7 @@ private void validateRouteMethod(BeanInfo bean, MethodInfo method, TransformedAn
}

private String generateHandler(HandlerDescriptor desc, BeanInfo bean, MethodInfo method, ClassOutput classOutput,
TransformedAnnotationsBuildItem transformedAnnotations, String hashSuffix) {
TransformedAnnotationsBuildItem transformedAnnotations, String hashSuffix, String defaultProduces) {

String baseName;
if (bean.getImplClazz().enclosingClass() != null) {
Expand Down Expand Up @@ -442,7 +444,8 @@ private String generateHandler(HandlerDescriptor desc, BeanInfo bean, MethodInfo
}

implementConstructor(bean, invokerCreator, beanField, contextField, containerField);
implementInvoke(desc, bean, method, invokerCreator, beanField, contextField, containerField, transformedAnnotations);
implementInvoke(desc, bean, method, invokerCreator, beanField, contextField, containerField, transformedAnnotations,
defaultProduces);

invokerCreator.close();
return generatedName.replace('/', '.');
Expand Down Expand Up @@ -477,7 +480,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) {
TransformedAnnotationsBuildItem transformedAnnotations, 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());
Expand Down Expand Up @@ -539,12 +542,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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
Expand Down Expand Up @@ -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() + "/manual-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")
Expand Down Expand Up @@ -87,6 +118,13 @@ Person getPersonUtf8(RoutingContext context) {
return new Person("neo", 12345);
}

@Route(path = "manual-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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,17 @@
/**
* 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;

/**
* Used for content-based routing.
* <p>
* 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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,14 @@ private RouteHandlers() {
private static final LazyValue<Event<SecurityIdentity>> 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);
} else if (defaultContentType != null) {
response.putHeader(CONTENT_TYPE, defaultContentType);
}
}
}
Expand Down

0 comments on commit b2b92fd

Please sign in to comment.