Refactor how hints (like JsonView) are passed to codecs [SPR-14557]

[spring-projects-issues]

Sébastien Deleuze opened SPR-14557 and commented

We need to define and standardize how context informations (like annotations for @JsonView) is passed to the codecs. In 5.0 M1, we temporarily use ResolvableType#getSource() but that does not work for wrapped return values like Mono<Foo> since the ResolvableType is not created from the MethodParameter + this is hardcoded and specific.

The original idea was to deal with that in a more flexible and maintainable way by using the Object... hints parameter of Encoder and Decoder. If we chose to go that way:

• HttpMessageWriter and HttpMessageReader API will have to be updated to pass this Object... hints parameter
• We should specify what should be in the Object parameter, or even perhaps use a more specific type like CodecContext ...
• We should implement a way to provide this context in a configurable way, maybe with a mechanism similar to RequestBodyAdvice / ResponseBodyAdvice.

The mechanism should allow to pass additional informations needed for serializing / deserializing without wrapping the information like in Spring MVC MappingJacksonValue to provide a more powerful, flexible and maintainable mechanism.

I plan to provide a PR and will request feedbacks and reviews based on that.

---

Affects: 5.0 M1

Issue Links:

• Add ResourceRegionHttpMessageWriter [SPR-14664] #19228 Add ResourceRegionHttpMessageWriter ("is depended on by")
• Provide annotated controller hints to codecs and JsonView support [SPR-14693] #19257 Provide annotated controller hints to codecs and JsonView support

[spring-projects-issues]

Sébastien Deleuze commented

Some example of the informations we need to pass to the codecs:

• Jackson JSON view Class<?>
• Jackson FilterProvider
• JSONP function name as a String
• Charset to use

These information can be provided for each invocation by the user, or determined from the handler method declaration (annotations).

[spring-projects-issues]

Sébastien Deleuze commented

Juergen Hoeller I have drafted an updated API that I would like to discuss with you (and anybody else interested of course).

[spring-projects-issues]

Sébastien Deleuze commented

Draft pull request created in order to get feedbacks.

[spring-projects-issues]

Rossen Stoyanchev commented

As I commented under the PR I think the Request/ResponseBodyAdvice and the JsonView implementation does not fit the stated scope of this ticket. They are indeed necessary to expose JsonView support at the controller level but I think we may not need them after all for the purpose here. Here is why.

While reviewing the PR I realized something is not entirely accurate in the original description above:

In 5.0 M1, we temporarily use ResolvableType#getSource() but that does not work for wrapped return values like Mono<Foo> since the ResolvableType is not created from the MethodParameter + this is hardcoded and specific.

HandlerResult was refactored before M1 to accept MethodParameter in its constructor. That means ResponseBodyResultHandler, for example, has the information it needs (i.e. method annotations). Of course with return values such as Mono<Foo> we do ResolvableType.forMethodParameter(bodyType).getGeneric(0) to get the element type and the information we need is effectively lost and not passed into the Encoder.

Currently Encoder and Decoder accept one ResolvableType that represents the element type of the input or output stream. That seems insufficient where we could and arguably should pass the stream type, the actual MethodParameter source if you will. Doing so would probably eliminate the need for request/response body advice for the purpose of JsonView support at least, which was the original idea and a much lighter solution overall.

[spring-projects-issues]

Sébastien Deleuze commented

Rossen Stoyanchev Thanks for your feedback! What you described is what I had initially in mind, but I see 2 issues by following that way.

The first one is that if we expose the original type (which indeed provides the actual MethodParameter source), we let Encoder and Decoder deal with Foo versus Flux<Foo> versus Observable<Foo>. Since we usually need the element type (like here), I believe this is an issue. The handling of these differences is currently at AbstractMessageReaderArgumentResolver and AbstractMessageWriterResultHandler level (using ReactiveAdapter), and I think that's a good thing to be able to deal with the element type in a consistent way in codecs.

The other reason is that this is maybe more clean to deal with hints like JsonView in a consistent way in codecs, regardless of the use case. Using MethodParameter is specific to the annotation based server side programming model, and we need JsonView support for the lambda based programming model introduced by #17546. That would be more maintainable to only deal with hints in codecs in a consistent way via the Map<String, Object> hints parameter, without using some MethodParameter specific code there.

Any thoughts?

[spring-projects-issues]

Rossen Stoyanchev commented

The other reason is that this is maybe more clean to deal with hints like JsonView in a consistent way in codecs, regardless of the use case.

Indeed although to be clear I don't mean for Encoder/Decoder to deal with different async wrapper types. All else remains the same, simply the Jackson Encoder checks if the {ResolvableType}} for the input stream is a MethodParameter source, just like it did originally.

Let's create a separate ticket for JsonView support for reactive @Controller's and consider the options including how the other hints above would work out. I can see that "jsonp" cannot be done internally within an HttpMessageWriter which only does not have access to the input message.

[spring-projects-issues]

Sébastien Deleuze commented

I have polished/merged the 2 first commits (#1, #2) and I have created #19257 to experiment and discuss about how to provide the hints to the codecs for the annotated controller use case.

[spring-projects-issues]

Sébastien Deleuze commented

I reopen this issue after the discussion we had at #1175.

[spring-projects-issues]

Sébastien Deleuze commented

I have merged these 3 additional commits : Do not provide hints for can*() methods anymore, Make ServerHttpMessageReader/Writer more powerful and flexible and Rename beforeRead/Write() to resolveRead/WriteHints().