Simpler solution to provide hints

[sdeleuze]

As proposed by @rstoyanchev, here is a draft PR that allow to provide hints without implementing Request/ResponseBodyAdvice. I have used an Object source parameter to allow using this mechanism for the HTTP client and the server functional programming model if need, without exposed MethodParameter in the codec/reader/writer API.

Any thoughts?

[sdeleuze]

This class will be removed from the commit before going to master, it was not intended to be here.

[rstoyanchev]

This looks simple enough but for JSONP and for HTTP ranges (very likely) we'll also need access to the request. At that point however it is no longer symmetric between client and server and only applies to the server.

Perhaps a variation on this PR could be a ServerHttpMessageWriter that extends HttpMessageWriter and has the extra method to resolve hints? On the implementation side it would be one or more hint resolving wrappers (e.g. JsonView, JSONP, etc) in the end delegating the actual writing to the target, underlying HttpMessageWriter (e.g. the one with the Jackson2JsonEncoder).

This would allow all places to still accept an HttpMessageWriter but at writing time also check if they're an instance of ServerHttpMessageWriter. We can automatically add the wrappers on startup in the WebReactiveConfiguration and there is no need for an additional configuration option.

[sdeleuze]

@rstoyanchev Good points, I will update the PR accordingly.

[sdeleuze]

@rstoyanchev I have pushed updated commits, could you please have a look?

[bclozel]

Because this class is in the spring-web-reactive module, I'm afraid SPR-14664 won't use this because ResourceHttpMessageWriter is in the spring-web module, org.springframework.http.codec package.

[rstoyanchev]

ServerHttpMessageWriter is generally useful for sever-side use. As such I see it as belonging in spring-web next to HttpMessageWriter where we also have other server-side only writers like the one for Server-Sent Events.

The ResourceHttpMessageWriter needs it and that's another confirmation it should be there. It would be needed for JSONP as well and anything else that needs access to the request to prepare hints.

As for the MethodParameter couldn't we make that a ResolvableType, call it streamType, and order it ahead of elementType in the method signature? That would be a little less assuming that there is a MethodParameter necessarily.

I would also de-emphasize that it's for controllers only in the Javadoc. It's for server-side use first and foremost, certainly in controllers. The Jackson implementations could also be in spring-web by the way as far as I can see so it's all together.

[rstoyanchev]

In addition to moving to spring-web I see two more things to improve.

There is no way to apply more than one of these (e.g. JsonView + JSONP). Perhaps instead of extending DecoderHttpMessageWriter we should have an AbstractServerHttpMessageWriter that delegates to another HttpMessageWriter. And if the delegate happens to be an ServerHttpMessageWriter it would allow the delegate to also add hints.

Two this doesn't yet cover the case of the HttpMessageWriterView which is a View that writes through an HttpMessageWriter. This would have been relatively simple to do until recently after the change to View to accept a Map rather than HandlerResult. @poutsma not sure what the answer is. On a first thought perhaps a HandlerResultView extension of View with a method that accepts HandlerResult instead of just a model? So the ViewResolutionResultHandler can then check and prefer to invoke the render with a HandlerResult if available. We need some way to pass the extra information when it is available.

[poutsma]

Two this doesn't yet cover the case of the HttpMessageWriterView which is a View that writes through an HttpMessageWriter. This would have been relatively simple to do until recently after the change to View to accept a Map rather than HandlerResult. @poutsma not sure what the answer is. On a first thought perhaps a HandlerResultView extension of View with a method that accepts HandlerResult instead of just a model? So the ViewResolutionResultHandler can then check and prefer to invoke the render with a HandlerResult if available. We need some way to pass the extra information when it is available.

To be honest, I never really understood the need for the HttpMessageWriterView in the first place, but perhaps I am missing something. What does it do that a @responsebody cannot do?

[sdeleuze]

Indeed + that allows to use that mechanism with the SSE reader/writer too.

I have updated the commit accordingly, is it ok for you (not sure if the remaining point you raised about HttpMessageWriterView impact this PR or not, I guess we will see depending on @poutsma feedback) ?

[rstoyanchev]

To be honest, I never really understood the need for the HttpMessageWriterView in the first place, but perhaps I am missing something. What does it do that a @responsebody cannot do?

Comparable to what MappingJackson2JsonView or MarshallingView do by delegating to any HttpMessageWriter. So if you have a controller method that typically renders an HTML template through view resolution, it can also get rendered as JSON or XML if the client requests it. Such a method would not be annotated with @ResponseBody.

[rstoyanchev]

Shouldn't this pre-implement resolveHints to do delegation to the next HttpMessageWriter if it is a ServerHttpMessageWriter and then invoke some resolveHintsInternal abstract protected method? I don't see that last bit right now that would allow decorating with more than one ServerHttpMessageWriter.

Same comment for the HttpMessageReader too.

[rstoyanchev]

This looks good now @sdeleuze. For the HttpMessageWriterView the compromise might use the model to pass the return type as we did previously.

[sdeleuze]

Merged