Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WebClient bodyToFlux(String.class) for string list doesn't separate individual values #22662

Closed
blabadi opened this issue Mar 25, 2019 · 3 comments
Assignees
Labels
status: declined A suggestion or change that we don't feel we should currently apply

Comments

@blabadi
Copy link

blabadi commented Mar 25, 2019

Affects: 5.1.5.RELEASE

Attached demo project for reproducing
bug-demo.zip

spring boot version: spring boot 2.1.3

if I have json response like this : ["v1", "v2", "v3"]
doing:
webclient.get().retrive().bodyToFlux(String.class)

will emit "["v1", "v2","v3"]" as single signal instead of 3 different signals.


@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Mar 25, 2019
@sdeleuze sdeleuze self-assigned this Mar 26, 2019
@sdeleuze
Copy link
Contributor

What happens here is that decoding JSON as String is always handled as low level processing since AbstractJackson2Decoder skips any CharSequence (raw, Mono or Flux) to let StringDecoder handling it. This is on purpose and unlikely to change.

That's said, I am wondering if we could make easier the required steps required to parse Flux<String> with Jackson as expected in this demo project since it seems a pretty reasonable need. Currently, it seems one has to create a new class extending Jackson2JsonDecoder and overriding canDecode to avoid the hardcoded skip for CharSequence which seems making the assumption that there will be a catch-all StringDecoder which is true by default, but not necessarily with custom configurations.

Maybe we could make AbstractJackson2Decoder configurable to allow users to make this skip for CharSequence configurable via a flag. That way it would only require to customize the list of decoders to customize the behavior. The principle seems to be conceptually close to what we do in StringDecoder#allMimeTypes and StringDecoder#textPlainOnly.

Another way to make this more flexible would be to leverage per usage hints but that would require passing hints to canDecode which is currently not the case.

Any thoughts @rstoyanchev @bclozel @poutsma?

@rstoyanchev
Copy link
Contributor

This is stated in the docs.

Note there is already some control. You can decode to List<String> which is exactly what the server is sending. If it needs to be a more continuous Flux then some form of streaming delineation is required anyway. For example if the server sent the strings with new lines, the client would get them one by one.

I am not in favor of adding more options to codecs configuration. That's not necessarily making things simpler.

@sdeleuze sdeleuze added status: declined A suggestion or change that we don't feel we should currently apply and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Mar 27, 2019
@fedotxxl
Copy link

fedotxxl commented Aug 6, 2019

Holy cow. This is a totally unexpected behavior... Good for you that is documented. Now bug becomes a feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

5 participants