Skip to content

Add toEntityFlux with BodyExtractor to WebClient.ResponseSpec #26099

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

Closed
HaloFour opened this issue Nov 16, 2020 · 7 comments
Closed

Add toEntityFlux with BodyExtractor to WebClient.ResponseSpec #26099

HaloFour opened this issue Nov 16, 2020 · 7 comments
Assignees
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement

Comments

@HaloFour
Copy link
Contributor

HaloFour commented Nov 16, 2020

Affects: Spring WebFlux 5.3.1


I started a conversation over on #26023 (comment) but it feels more appropriate to open a new issue.

If the plan is to remove the WebClient#exchange method entirely (as described here) how can we solve for those cases that aren't currently solved for using the alternative methods?

I have two scenarios in my case base which seem to require WebClient#exchange:

The first scenario is a series of endpoints which more or less act as a proxy to an upstream service. We use Flux<DataBuffer> to stream the raw data to/from the client. In this case retrieve().entityToFlux(DataBuffer.class) looks like it comes close to satisfying this use case, but I don't see a way with ResponseSpec to disable the default status code handling entirely, which is what I need.

The second scenario are some endpoints where we make an upstream service request and get back a large JSON response containing an array nested in a root object, e.g.:

{
    "response": {
        "entities": [ ]
    }
}

We're currently using exchange with a custom BodyExtractor<Flux<T>> which can scan into the JSON response to the array and stream the individual elements which we return in a ResponseEntity<Flux<T>> so that we can also return the response headers and status code. We then flow this Flux<T> back to the controller where it is mapped to transform each entity and stream it down to the client. See #24951 for more details.

I've not fully evaluated where we're using exchange and whether or not they all fit into these two cases so I might be missing some scenarios.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Nov 16, 2020
@HaloFour
Copy link
Contributor Author

HaloFour commented Nov 16, 2020

I'm not 100% sure but I think my use cases might be solved with a few additional APIs to ResponseSpec which would enable us to use retrieve instead of exchange. Any names suggested here are just for illustration.

  1. Allow us to completely disable the status code mapping to an error. I can add a StatusHandler to the chain but if they match they have to return a Mono<Throwable> and if none of them match then it will always fallback to treating >400 as an error.

    ResponseSpec withoutStatusHandlers();
  2. Offer APIs to decode the response body into a ResponseEntity<T> using an arbitrary BodyExtractor<T> or even an arbitrary transformation method:

    Mono<ResponseEntity<T>> toEntity(BodyExtractor<T> extractor);
    Mono<ResponseEntity<T>> toEntity(Function<ClientResponse, Mono<T>> function);
    Mono<ResponseEntity<Flux<T>>> toEntityFlux(Function<ClientResponse, Flux<T>> function);

@rstoyanchev
Copy link
Contributor

Thanks for the extra details.

For your first scenario, have you seen the Javadoc for onStatus where you can can return Mono.empty() to bypass error status handling and treat the response as normal.

For extracting to Mono:

Mono<ResponseEntity<T>> toEntity(BodyExtractor<T> extractor);
Mono<ResponseEntity<T>> toEntity(Function<ClientResponse, Mono<T>> function);

For those you could use exchangeToMono:

Mono<ResponseWrapper<Foo>> result = webClient.get().uri("/foo/1")
		.exchangeToMono(response -> response.body(fooExtractor).map(foo -> new ResponseWrapper(...)));

That leaves BodyExtractor for Flux for which we could add:

<T> Mono<ResponseEntity<Flux<T>>> toEntityFlux(BodyExtractor<T, ClientHttpResponse> extractor);

@HaloFour
Copy link
Contributor Author

@rstoyanchev

For your first scenario, have you seen the Javadoc for onStatus where you can can return Mono.empty() to bypass error status handling and treat the response as normal.

Thanks for that. I'll give it a try. I tried to follow the code path for that case but I must've misinterpreted it. Empty Mono's are weird beasts.

@HaloFour
Copy link
Contributor Author

The following approach does work, thank you for that:

.retrieve()
.onRawStatus(status -> true, () -> Mono.empty())
.toEntityFlux(DataBuffer.class)

I do think that onRawStatus/onStatus call is a little weird, though, and I have been putting comments on every use to ensure that other developers understand that the purpose is to disable the default error handling on WebClient so that we can interpret 4xx/5xx status codes. Named singleton instances of the handler would help but maybe an overload would be clearer?

@rstoyanchev
Copy link
Contributor

You can create a static handler with an appropriate name and a note if you want to make it more obvious:

/** Handler that suppresses default status error handling */
final static Function<ClientResponse, Mono<? extends Throwable>> NOOP_ERROR_HANDLER = () -> Mono.empty();

and then:

.retrieve()
.onRawStatus(status -> true, NOOP_ERROR_HANDLER)
.toEntityFlux(DataBuffer.class)

@HaloFour
Copy link
Contributor Author

@rstoyanchev

Thanks for your help, I was able to port over almost all of our uses of exchange to either exchangeToMono or retrieve, save for the case described above with the custom BodyExtractor. Would a community contribution for ResponseSpec#toEntityFlux(BodyExtractor) be welcomed?

@rstoyanchev
Copy link
Contributor

Sure, go ahead and submit.

@rstoyanchev rstoyanchev self-assigned this Nov 18, 2020
@rstoyanchev rstoyanchev added in: web Issues in web modules (web, webmvc, webflux, websocket) type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Nov 18, 2020
@rstoyanchev rstoyanchev added this to the 5.3.2 milestone Nov 18, 2020
@rstoyanchev rstoyanchev changed the title Gaps with Deprecation of WebClient#exchange Add toEntityFlux with BodyExtractor to WebClient.ResponseSpec Nov 18, 2020
@rstoyanchev rstoyanchev removed this from the 5.3.2 milestone Nov 18, 2020
@rstoyanchev rstoyanchev added the status: superseded An issue that has been superseded by another label Nov 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: web Issues in web modules (web, webmvc, webflux, websocket) status: superseded An issue that has been superseded by another type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants