Allow Consumer-style access to FluxExchangeResult within chain of calls [SPR-15959]

[spring-projects-issues]

Rob Winch opened SPR-15959 and commented

Currently ExchangeResult.assertWithDiagnostics accepts a Runnable which works fine for assertions within ExchangeResult because other methods have a reference to the ExchangeResult to create a Runnable which performs the assertion.

However, a Runnable does not work nicely for an external user who wants to perform an assertion. Currently they must do something like:

FluxExchangeResult<String> result = client.get()
	.exchange()
	.expectStatus()
	.isOk()
	.returnResult(String.class);
result.assertWithDiagnostics(() -> assertThat(result.getResponseCookies().isEmpty()));

It would be nice if assertWithDiagnostics accepted a Consuer<ExchangeResult> so that the code can be refactored to be something like:

client.get()
	.exchange()
	.expectStatus()
	.isOk()
	.returnResult(String.class)
	.assertWithDiagnostics( result -> assertThat(result.getResponseCookies().isEmpty()));

---

Affects: 5.0 RC4

Issue Links:

• Ensure that WebClient disposes the HTTP client connection once the client response is consumed [SPR-15920] #20474 Ensure that WebClient disposes the HTTP client connection once the client response is consumed

Referenced from: commits d742fc1

[spring-projects-issues]

Rossen Stoyanchev commented

We have both returnResult and consumeWith methods in the workflow of WebTestClient. The reason there is no consumeWith at the top ResponseSpec level is because we don't know yet how you want to treat/decode the body. We could turn it into FluxExchangeResult<DataBuffer> but the consumeWith methods allow further chaining and that would be in conflict with the workflow since you shouldn't be able to use any other body-related specs any more.

Note that you could do something like:

client.get()
	.exchange()
	.expectStatus()
	.isOk()
	.expectBody(String.class)
	.consumeWith(result -> assertThat(result.getResponseCookies().isEmpty()));

The difference is that the above would actually consume the body to a String. That may or may not be the intent but keep in mind that using returnResult at the top ResponseSpec level is for streaming cases, i.e. you should still consume the body or otherwise you may leave the connection hanging in a live integration test for example.

In addition we could add a close() method to WebTestClient.ResponseSpec mirroring the one recently added to ClientResponse in #20474 (i.e. I don't want to read the response, release the resources). We could also add a consumeWith method to ExchangeResult itself which I find strange to see, unless you understand the above example but I suppose we can provide an example in the Javadoc.

Either way I'm setting this for some kind of a fix since I think a close would be a good idea.

[spring-projects-issues]

Rossen Stoyanchev commented

I've added a consumeWith to FluxExchangeResult where it is actually needed. I have also added a no-arg returnResult() to the ResponseSpec returning Flux<DataBuffer> so you can do the following:

client.get()
      .uri("/persons")
      .accept(TEXT_EVENT_STREAM)
      .exchange()
      .expectStatus().isOk()
      .returnResult()
      .consumeWith(result -> assertThat(...);

[spring-projects-issues]

Rossen Stoyanchev commented

Rob Winch note that following this recent change #20567, I've made followup changes to the WebTestClient. In short for tests where you don't need to test the response content, or perhaps there is no response content (204, 4xx) you can use this:

client.get().put("/path")
        .exchange()
        .expectStatus().isOk()
        .expectBody(Void.class)
 	.consumeWith(result -> ... );

As you can see, there is not need to exit the chained API workflow. You're simply using Void.class to indicate you're not interested in the body whether it's empty or not.

I have already removed the top-level no-arg returnResult() method that was added (proactively on my part) for this ticket. I would also like to remove the consume method on FluxExchangeResult since that should no longer be needed.