Affects: 5.2.0.RELEASE
The motivation for this enhancement is that WebTestClient API is mostly blocking (with the exception of .returnResult(Class<?>).getResponseBody() which exits the chained API), and when it's used with Spring WebFlux it might be very common to get an exception because reactor-tcp-nio-x is a non-blocking thread.
The main idea is to have another exchange method that is wrapped around a Mono publisher. This way the exchange can be executed within the reactive stream and the assertion API used in the assertNext(..) method of the StepVerifier.
This might also indirectly solve issues #21781 and especially #23390 since allowing the API to exchange within the stream will also allow the exchange to be in the same transaction. For example:
@Test
public void account_test() {
accountRepo.save( // Create and store an account using spring-data-r2dbc and r2dbc-postgresql
Account.builder()
.username("mock")
.password("1234")
.build()
)
.publishOn(Schedulers.elastic()) // need to publish on an elastic thread or the blocking exchange will throw an exception
.as(TestHelpers::withRollback) // a transaction wrapper using TransactionalOperator and configured as rollback only
.as(StepVerifier::create)
.assertNext(saved -> {
assertThat(saved.getId()).isGreaterThan(0); // just to verify the account was persited on db
WebTestClient.bindToApplicationContext(appContext)
.build()
.get()
.uri("/account/" + saved.getUsername()) // a simple WebFlux end-point that finds the account by username and returns it in the response body
.exchange()
.expectStatus()
.isOk();
})
.verifyComplete();
}
This test will always fail because is running within a transaction. As the exchange() method is blocking, it will be executed in another thread outside the transaction, so the saved account will never be found. To run the end-point test within the transaction we need a non-blocking exchange method. In addition, it'll also remove the need to publish on a blockable thread.
Thanks in advance for taking the time to review this PR 🙂
cc. @sbrannen @jhoeller @mp911de
Comment From: rstoyanchev
@JoseLion thanks for the suggestion. I don't think it is the right approach, but it has made me think things over which I do appreciate. Below is my thinking and some extra context.
The motivation for this enhancement is that WebTestClient API is mostly blocking
WebTestClient is built on WebClient and has non-blocking internals. However in the context of asserting the details of an HTTP response, there are only two options -- block and wait for the response details, or accumulate assertion declarations and block at the end, like the StepVerifier API does.
WebTestClient is a sort of hybrid. It does the former for the typical responses where the body is read in full, while for streaming, ResponseSpec#returnResult provides a way to continue with StepVerifier.
Note that neither of those promises to freely compose multiple HTTP calls together, nor participate in a chain of calls with other reactive clients. This is because asserting response details vs passing those response details down the execution chain are two competing goals that would be very hard to blend together in an API. It would be like trying to compose on multiple Publisher's each wrapped with a StepVerifier.
As for your suggestion, returning Mono<ClientResponse> is insufficient and doesn't provide an answer to all other branches of the assertion API. Specifically those ResponseSpec#responseBody methods that also block for the actual content. Overloading all of those with async variants would result in a very confusing API. Really the way to do that would have been some sort of StepVerifier like accumulation of assertion declarations with a verify at the end to trigger them but again that still requires a block at the end, it doesn't change anything with regards to chaining multiple remote calls, and it's tricky to begin with due to the number of assertion branches.
For the given test case one option is to use the WebClient. That would be my preference since the goal is to not to test an HTTP call but to verify something is in the the database.
However if you want to make it work with the WebTestClient then you'll need to make sure that call is in the Publisher that the transaction wraps, i.e. before and not after. Here is an example to make it more concrete, but please understand I'm a paraphrasing a code snippet here and not working with compiled code:
@Test
public void account_test() {
accountRepo.save( // Create and store an account using spring-data-r2dbc and r2dbc-postgresql
Account.builder()
.username("mock")
.password("1234")
.build()
)
.flatMap(saved -> {
assertThat(saved.getId()).isGreaterThan(0); // just to verify the account was persited on db
return Mono.fromRunnable(() ->
WebTestClient.bindToApplicationContext(appContext)
.build()
.get()
.uri("/account/" + saved.getUsername())
.exchange()
.expectStatus()
.isOk()
).subscribeOn(Schedulers.boundedElastic());
})
.as(TestHelpers::withRollback)
.block();
}
Comment From: rstoyanchev
I am closing for now to indicate no further plans at this time but the discussion can continue.
Comment From: JoseLion
@rstoyanchev thanks a lot for your response and for checking this PR. I really appreciate it 🙂
I understand your point, the last thing we want is to make the API confusing. Regarding your recommendations, I didn't use WebClient because the goal is indeed to test the HTTP call, I just didn't add it to the example to reduce its complexity 😅
For the given test case, I tried your suggestion. It makes sense to be sure the call is in the publisher the transaction wraps, so I tried your suggested code. The test still fails with 404 status code, so it seems the HTTP call of WebTestClient is not executed in the same transaction. However, if I remove the line .as(TestHelpers::withRollback) the test pass with no problem.
I'm not sure if I'm missing something, but my best guess was that the HTTP call cannot be included in the same transaction because of the blocking part of the call. So my first thought was to add the possibility to accumulate the ResponseSpec assertion to block it at the end with StepVerifier.
If this really isn't the best approach, what should be the best way to include the WebTestClient call in the same transaction?
Comment From: rstoyanchev
I think I better understand now. What you're trying to do is test the HTTP call by per-inserting data for the server to find, and then using a server-less test that you hope will see the data before the transaction is rolled back?
WebClient and server aren't meant to be in the same process typically, so I'm not sure it is reasonable to expect that testing through a (client-side) HTTP call will participate in a (server-side) DB transaction.
You would also have to at some point do a full end-to-end integration test and that would also require data in the database, and that certainly can't rely on rolling back transactions. So either data has to be inserted and committed or the data layer should be stubbed for more fine-grained tests.
Comment From: JoseLion
Yes, the idea is to do end-to-end tests using transactions to roll back any change in the database. My intention is for tests to be idempotent, to not depend on existing data in DB, and with high performance regarding IO.
I prefer not to stub/mock the data layer as it reduces the confidence and maintainability of the tests. My solution at the end was to remove the transaction and ensure a clean table for each test using a @BeforeEach. It works, but the data is actually written to DB and then cleared on each test, meaning lower performance. Using transactions means data is never actually written to DB but rolled back in the end (better performance per test).
So just to be sure, the problem is the WebClient and the server are not in the same process so they cannot participate in the same transaction, right?
Comment From: rstoyanchev
Yes that is correct.
Comment From: JoseLion
Great, thank you @rstoyanchev! This helped me better understand the nature of WebTestClient 🙂