This PR contains 2 commits intended to address SPR-15910 raised by @wilkinsona. The type resolution consistency change is a significant (but really needed IMO) one that I would like to validate with @wilkinsona, @rstoyanchev, @poutsma and @bclozel.

The details of this change are in the 2nd commit message log and reproduced bellow:

Before this commit, Spring WebFlux used both the declared return value type and the concrete object type in writers, in a similar fashion than Spring MVC.

But this can lead to inconsistent behaviors, for example between type resolution of a return value that requires an adapter (declared return value type is used) and one that does not (concrete return value type is used) or between ? and Object handling.

After this commit, only declared type (via return value type for annotation-based programming model or ParameterizedTypeReference for the functional API) is used in order to provide a consistent and predictable behavior.

The asynchronous nature of WebFlux, type erasure and providing error-prone SPI are also reasons that motivates this change.

Concretely that means that handler methods declaring an Object return value type and returning a String or Resource value won't be handled anymore by respectively by CharSequenceEncoder and ResourceEncoder. These handler methods need to declare explicitly the return value type.

For use cases that requires Spring MVC dynamic type resolution based on the concrete return value type, using the functional API which allows programmatic type resolution is the recommended way to use with WebFlux.

Thanks in advance for your feedbacks.

Comment From: wilkinsona

Concretely that means that handler methods declaring an Object return value type and returning a String or Resource value won't be handled anymore by respectively by CharSequenceEncoder and ResourceEncoder. These handler methods need to declare explicitly the return value type.

I don't have a better answer to suggest, but if I were to encounter this behaviour I'd be rather surprised and my instinct would be that it's a bug.

What is it in WebFlux that prevents the annotation-based model from behaving as it does in Spring MVC? I'd like to understand that difference as, without understanding it, I don't think I'll be able to do much other than feel surprised that it doesn't work as MVC does.

Comment From: poutsma

Concretely that means that handler methods declaring an Object return value type and returning a String or Resource value won't be handled anymore by respectively by CharSequenceEncoder and ResourceEncoder. These handler methods need to declare explicitly the return value type. I don't have a better answer to suggest, but if I were to encounter this behaviour I'd be rather surprised and my instinct would be that it's a bug.

I wouldn't call it a bug. It's different than MVC, that much is true. But that doesn't make it a bug.

What is it in WebFlux that prevents the annotation-based model from behaving as it does in Spring MVC? I'd like to understand that difference as, without understanding it, I don't think I'll be able to do much other than feel surprised that it doesn't work as MVC does.

I don't know the ins and outs of this particular issue, but one thing I know is that in MVC, we can easily inspect the value returned from the Controller with reflection. In WebFlux, we don't have that luxury, as the return value is typically some sort of Publisher, and its elements are coming in asynchronously. As we need to setup the response writing pipeline before that first element comes in, we cannot do so using reflection on the element instance; the method signature is the only source of information we have.

Comment From: sdeleuze

WebFlux is heavily based on Reactive wrappers (Publisher, Observable) subject to type erasure and the concrete value is async, so not known by design in most cases at writer resolution stage, so I am not sure sometimes leveraging it is a good idea.

ResponseEntity<Foo> handling is a special case where we only support single value processing where we do a flatmap operation that end up by calling the writer at a stage where the concrete value is available without a wrapper. But for the general purpose suche value is not available. And even if we modified the pipeline to wait it (which is unlikely), I would not be confortable by guessing the type of Flux<Object> using the first one.

Current behavior which sometimes use the concrete value type when available and otherwise use the declared one is confusing IMO since Foo and Single<Foo> will resolve diffrent type even if semantically very close.

I understand this difference of behavior between Spring MVC and Spring WebFlux can be first found confusing, but if we keep the current behavior with half baked fix to support the Resource use case, I am pretty sure we will faced a lot of others (I have already a few in mind).

If we accept this change (which lead to a more predicable behavior), I will document it in the reference documentation.

Comment From: wilkinsona

Thanks, @poutsma and @sdeleuze. I think I'm getting there now.

Current behavior which sometimes use the concrete value type when available and otherwise use the declared one is confusing IMO since Foo and Single will resolve diffrent type even if semantically very close.

I, too, think that's confusing. FWIW, I would definitely prioritise fixing this, and having consistent behaviour within WebFlux, over being somewhat consistent with MVC.

Comment From: sdeleuze

We will wait @rstoyanchev feedback before deciding. In any case thanks @wilkinsona for this issue report, you helped to raise a pretty serious design issue!

Comment From: rstoyanchev

I am taking a look. On a first impression I'm not sure I agree it is a bug. What puzzles me however is why wildcard behaves differently from Object.

Comment From: sdeleuze

It behaves differently because when ? is used, this condition behaves differently. With Object ResolvableType.forInstance(body) is used but with ? bodyType is used.

I have tried to remove the bodyClass == null part of the test which fixes Andy use case but break others like Jackson annotation support.

IMO even without @wilkinsona use case, I think we have a big consistency issue, hence my proposal.

Comment From: rstoyanchev

For use cases that requires Spring MVC dynamic type resolution based on the concrete return value type, using the functional API which allows programmatic type resolution is the recommended way to use with WebFlux.

@sdeleuze which functional API do you mean? I'm trying to picture how the Spring Boot 2.0 actuator endpoint infrastructure would do this.

Comment From: sdeleuze

@rstoyanchev I mean using WebFlux functional API, which is programmatic and give more control for this kind of dynamic behaviors compared to annotation-based programming model where return types are hard-coded. At some points we discussed the possibility of using the functional API with @snicoll for implementing Spring Boot 2.0 actuator but I think it was too much work to follow this path.

Another important thing to notice is that unlike Spring MVC GenericHttpMessageConverter which passes both declared and actual type, we provide a more focused and opinionated API WebFlux (and I think this is a good thing, and more error prone for writer implementers compared to Spring MVC SPI) with a single type passed (currently actual value type for ? and Object or the declared type for other types).

I have created a branch with additional WebFlux tests inspired from @wilkinsona ones.

Publisher<ResponseEntity<?>>, Object, Publisher<ResponseEntity<Object>>, ResponseEntity<?>, ResponseEntity<?> and ResponseEntity<Object> return values work as expected with Resource return value with @rstoyanchev fix.

But ResponseEntity<Publisher<Object>>, ResponseEntity<Publisher<?>>, Publisher<?> and Publisher<Object> return values don't work as expected with Resource return value.

It seems to me that we have the following choices: 1. Let master as it is regardless the inconsistencies listed above since @wilkinsona use case is fixed. 2. Modify ResponseBodyResultHandler to call the writer only when the value is known via a flatMap() operation for single values like Mono and Single. That will fix the behavior when single values will be used, and we will use declared type only for stream types like Flux or Observable (where I don't think we should use the first element type to guess the element type to use for other elements). 3. Make the algorithm more simple like in my proposal in this PR and only rely on the declared type.

I think I would be ok for solutions 2 or 3, but not for 1 which is what we have currently in master. Solution 3 is more simple, predictable and consistent, while solution 2 allows us to provide a behavior similar to Spring MVC, so I understand that we may ended up with it.

Any thoughts?

Comment From: rstoyanchev

At some points we discussed the possibility of using the functional API with @snicoll for implementing Spring Boot 2.0 actuator but I think it was too much work to follow this path.

From a pragmatic standpoint support for ResponseEntity with any Object is driven by a very specific requirement in Spring Boot so is withdrawing that capability doesn't seem like a real choice.

It shouldn't be hard to make those extra cases work. It would require changes to both ResponseBodyResultHandler and also ResponseEntityResultHandler.

Comment From: sdeleuze

I have made the required changes and it works but there is a blocking issue, the fact that we change the ResolvableType (to use the nested one) passed to the writer after the flatMap() operation break JsonView support which is provided at encoder level. Not sure yet how to fix that.

Comment From: sdeleuze

I deleted my fork branch for this outdated PR that is not relevant anymore.