Deprecate WebClient#exchange() and provide safer alternatives

[rstoyanchev]

exchange() provides more flexibility than retrieve() but leaves the full handling of the response to the application and it remains difficult to use it correctly in order to avoid memory and connection leaks. retrieve() is safer because it wraps the handling of the response (and the framework can ensure correct release of resources) and since 5.2 with access to the response status and headers via ResponseEntity, retrieve() can cover all of common use cases.

The actual reasons for using exchange() are few. One example is the ability to suppress an error status and produce either a typed object from a successful response or a different object from an error response with the output consumed as Mono<Object>. However exchange() continues to be used much more widely than that.

It is time to provide a safer alternative to exchange() that retains the same flexibility but allows the WebClient to be involved in the handling of the response. For example:

<V> Mono<V> exchangeToMono(Function<ClientResponse, ? extends Mono<V>> responseHandler);

<V> Flux<V> exchangeToFlux(Function<ClientResponse, ? extends Flux<V>> responseHandler);

We can then deprecate exchange() and remove it in the next major version.

[rstoyanchev]

Re-opening in order to also consider if we need any updates related to Kotlin.

[sdeleuze]

I think we need exchangeToFlow and awaitExchange variants with a function parameter, I will provide that.

[mplain]

Hello!
I'm not sure how to proceed with this change, could you please take a look at my question?
https://stackoverflow.com/questions/64650820

[rstoyanchev]

@mplain as mentioned in the deprecation notice the preferred replacement is .retrieve(). Your use case is decoding the body to byte[] and preparing a ServerResponse. That should be a straight forward replacement.

[mplain]

@rstoyanchev I'm not sure I should decode the body to byte[], I just need to forward the response from the other server to my client. Is there a better way to do it, without decoding the body at my proxy server?

[rstoyanchev]

You can decode to Flux<DataBuffer> but I see now that we are missing an option under .retrieve() for a ResponseEntity with a Flux. So it should have been .retrieve().toEntityFlux(DataBuffer.class).map(entity -> ... ) but that's not possible right now. Can you create a new issue and describe your use case briefly?

[mplain]

@rstoyanchev meanwhile is there any way to combine .exchangeToMono() and Flux<DataBuffer>?

.exchangeToMono { response ->
   response.bodyToFlux<DataBuffer>().let(DataBufferUtils::join).flatMap { body ->
      ServerResponse.ok().body(body)
   }
}

I assume joining DataBuffers would be pretty much the same as using .bodyToMono<ByteArray>() in the first place?
So currently the only way to write the response body in a memory-efficient manner as Flux<DataBuffer> is via the deprecated .exchange() method, correct?

[rstoyanchev]

Correct.

[karthikairam]

As part of the unit testing, how can I mock the exchangeToMono(function) ? where I somehow want to execute the actual code in the function for code coverage.
Earlier when it was just exchange(), I just completely mocked and returned the expected response. But, now with exchangeToMono(function) I am not able to find any elegant way - except making the function gets prepared and returned from a public method. Then write a separate unit test case for that method :(. Which I don't feel like a good idea.
Any suggestions would be much appreciated. Thanks!

[karthikairam]

As part of the unit testing, how can I mock the exchangeToMono(function) ? where I somehow want to execute the actual code in the function for code coverage.
Earlier when it was just exchange(), I just completely mocked and returned the expected response. But, now with exchangeToMono(function) I am not able to find any elegant way - except making the function gets prepared and returned from a public method. Then write a separate unit test case for that method :(. Which I don't feel like a good idea.
Any suggestions would be much appreciated. Thanks!

I am able to achieve it using Mockito.doAnswer() option from mockito framework. Please ignore my above question. Thanks!

Working sample code for reference. Hope someone might find it helpful.

final ClientResponse mockedClientResponse = Mockito.mock(ClientResponse.class);

//set the required mocking behaviours here on the "mockedClientResponse" obj.

Mockito.doAnswer(invocationOnMock -> {
        return invocationOnMock
            .<Function<ClientResponse, Mono<SomeDTO>>>getArgument(0)
            .apply(mockedClientResponse);
    })
    .when(headerSpec)
    .exchangeToMono(ArgumentMatchers.<Function<ClientResponse, Mono<SomeDTO>>>any());

[rstoyanchev]

I'm not sure mocks are a good fit for mocking server responses. You can use something like OkHttp MockWebServer or WireMock.

[karthikairam]

I'm not sure mocks are a good fit for mocking server responses. You can use something like OkHttp MockWebServer or WireMock.

Thanks for your response. Yes we use WireMock for the Component Test. But for the Unit Testing, it will be bit heavy to have stub server. So we've used Mockito to cover that. And above code works just fine.
Cheers!