Spring MVC @ResponseBody handling of Flux<?> inconsistent when type of result is unknown [SPR-15456]

[spring-projects-issues]

Dave Syer opened SPR-15456 and commented

I would expect this:

@GetMapping("/empty")
public Flux<Object> empty() {
     return Flux.fromIterable(Collections.emptyList());
}

to produce an identical result to this:

@GetMapping("/empty")
public Flux<String> empty() {
     return Flux.fromIterable(Collections.emptyList());
}

But it doesn't (the former is "null" and the latter is "[]"). I guess maybe the "null" is a bug?

If the Flux is not empty, then both signatures should also probably produce the same result, but doesn't (the strings get concatented and the objects get converted)? Or maybe I'm missing something.

---

Affects: 5.0 RC1

Issue Links:

• WebFlux should support Flux<?> and Flux<Object> rendering [SPR-15464] #20024 WebFlux should support Flux<?> and Flux rendering

  Referenced from: commits abe3cfd, d3b178a, a936984

[spring-projects-issues]

Dave Syer commented

Here's another inconsistency (not sure if related). These two methods should be identical as far as HTTP request-response goes:

		@PostMapping("/updates")
		@ResponseStatus(HttpStatus.ACCEPTED)
		public Flux<String> updates(@RequestBody List<String> list) {
			Flux<String> flux = Flux.fromIterable(list);
			flux = flux.cache();
			flux.subscribe(value -> this.list.add(value));
			return flux;
		}

		@PostMapping("/updatesEntity")
		public ResponseEntity<Flux<String>> updatesEntity(@RequestBody List<String> list) {
			Flux<String> flux = Flux.fromIterable(list);
			flux = flux.cache();
			flux.subscribe(value -> this.list.add(value));
			return ResponseEntity.accepted().body(flux);
		}

But the ResponseEntity version sends back a 500.

[spring-projects-issues]

Rossen Stoyanchev commented

hi Dave, thanks for the report. Spring MVC I presume? And with "Accept: application/json" present? It's the only way I can get null vs "[]".

Given "Accept: application/json" if the Flux produces a single item it's rendered as-is through Jackson; for multiple items as a List (i.e. JSON array); for 0 items we have the inconsistency to fix. I would actually argue that for Flux we should always render a JSON array regardless of the number of items. Does that make sense?

The second issue is that Flux<String> should not render as JSON. That may seem strange at first but today if a controller returns String it is rendered with StringHttpMessageConverter for any media type including "application/json". The same principle should apply to Flux<String> for consistency, i.e. simply appending each String treating it as text regardless of the media type.

There is an inherent ambiguity whether String + "application/json" means "render a String containing JSON" or a "render a String as JSON". We've run into this much more on the WebFlux side where the encoding mechanism does not even differentiate between encoding a single String vs Publisher<String> (it's always the latter). So in WebFlux we already explicitly exclude String from Jackson encoding/decoding and resolve the ambiguity in favor of assuming that String is better treated as text regardless of the media type. So this is also about having Spring MVC be consistent with WebFlux.

I will look into the ResponseEntity issue separately. I suspect that's a related but different issue.

[spring-projects-issues]

Rossen Stoyanchev commented

I've created #20024 to fix a related issue with Flux<?> handling on the WebFlux side.

[spring-projects-issues]

Rossen Stoyanchev commented

For the ResponseEntity<Flux<String>> case I confirm this is a bug with the proper determination of the (double)-nested generic type. I will fix it.

[spring-projects-issues]

Dave Syer commented

I would actually argue that for Flux we should always render a JSON array regardless of the number of items. Does that make sense?

Yes. I would argue that is more intuitive. If I could try that out in a snapshot it would be awesome.

Does it create a problem for WebFlux when the Flux is potentially unbounded? I would argue that the content negotiation should explicitly require the client to ask for an unbounded media type (e.g. EVENT_STREAM) or else you would expect to get an array still.

[spring-projects-issues]

Dave Syer commented

That may seem strange at first but today if a controller returns String it is rendered with StringHttpMessageConverter for any media type including "application/json"

I guess that seems inconsistent to me. I wouldn't mind if that was the behaviour for Accept: text/plain, but I'd expect a Flux<String> to be an array in JSON, really.

[spring-projects-issues]

Rossen Stoyanchev commented

A snapshot is ready with Spring MVC fixes for Flux<T> to produce a JSON array consistently and for the NPE fix with ResponseEntity<Flux<T>>. Also WebFlux related fixes for #20024.

If a Flux is unbounded the controller method should be marked accordingly (e.g. with produces="text/event-stream") or otherwise it would be an issue since we assume collection semantics for the Flux by default.

Regarding Flux<String> producing a JSON array causes more problems than it solves IMO and also requires more complex and inconsistent behavior to support. Consider the following inconsistencies it creates:

• Rendering one String vs Flux<String> - suppose you're writing serialized JSON (as in Fix writing serialized JSON with WebClient and WebTestClient [SPR-15443] #20004) and then decide to do the same with Flux<String>, in streaming fashion, perhaps even in combination with application/stream+json. It's inconsistent, you have to learn we do one thing for a single String but another for Flux<String>.
• Rendering Flux<String> with "application/json" vs all other media types -- having String written with StringHttpMessageConverter for all media types (including "application/json") is long standing behavior that mostly continues for Flux<String> except for JSON but then again only for Flux<String> and not for String or Mono<String>.
• Rendering Flux<String> with Spring MVC vs WebFlux -- the Encoder in WebFlux deals with a Publisher<T> and is not equipped at present to differentiate between rendering a single vs many items. Unless we make changes at that level we cannot have consistency between Spring MVC and WebFlux and turning Flux<String> into a JSON array doesn't seem like a strong enough case to me.

No matter what we decide to do, there will be some inconsistency depending on your perspective and use case. So I would weigh the use cases and I'd argue that rendering String as serialized text always is consistent with what we've always done and enables more useful use cases (e.g. rendering serialized JSON including in streaming fashion) vs the ability to render Flux<String> as a JSON array which can still be accomplished if desired from the controller by using Flux.collectToList() and hence returning Mono<List<String>>.

[spring-projects-issues]

Dave Syer commented

I know that String is a little bit of a special case (and an artificial one probably since JSON array of string is probably the least likely data type to be returned from an HTTP endpoint in production), but I can't shake the feeling that we have painted ourselves into a corner here. Surely there should be some way to use content negotiation to extract a JSON array? That's my position still, but I don't want to chew up valuable time on it, so we can agree to disagree if you want.

[spring-projects-issues]

Rossen Stoyanchev commented

After consulting others on the team we'll go with treating Flux<String> + "application/json" the same way we do String, i.e. as serialized text to be rendered out as is. This is a less of a technical issue of how to actually render a JSON array and more a question of consistent treatment of String for encoding purposes.