If no Accept header is set default to the first 'produces' type #11014
Conversation
| @@ -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, | |||
There was a problem hiding this comment.
I think that we should only set the content-type header if the business method did not set it manually, i.e. after it was executed. If we set it before then multiple content-type headers may be set to the response.
| @@ -87,6 +118,13 @@ Person getPersonUtf8(RoutingContext context) { | |||
| return new Person("neo", 12345); | |||
| } | |||
|
|
|||
| @Route(path = "manual-content-type", produces = "application/json") | |||
There was a problem hiding this comment.
manual-content-type is not a good path for this route. Maybe default-content-type?
| String acceptableContentType = context.getAcceptableContentType(); | ||
| if (acceptableContentType != null) { | ||
| response.putHeader(CONTENT_TYPE, acceptableContentType); | ||
| response.headersEndHandler(new Handler<Void>() { |
There was a problem hiding this comment.
Shouldn't we use io.vertx.ext.web.RoutingContext.addHeadersEndHandler(Handler<Void>) instead?
Also from perf POV this means one more Handler instance per each request. It's probably negligible but I wonder if the previous approach "set the content type after the route method is executed" represents some serious issue?
There was a problem hiding this comment.
The previous approach would fail if the method used the RoutingContext to send data, as the headers would have already been written.
There was a problem hiding this comment.
Ah, you're right! What about the RoutingContext.addHeadersEndHandler()?
There was a problem hiding this comment.
Oops, forgot to push it.
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 quarkusio#10960
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