Expose the WebTestClient assigned request id header as a public constant [SPR-15575]

[spring-projects-issues]

Andy Wilkinson opened SPR-15575 and commented

Assuming my understanding of the API is correct, I would expect this test to pass:

@Test
public void requestHeadersFromWebFilterAreAvailableFromExchangeResult() {
    RouterFunction<ServerResponse> route = RouterFunctions
            .route(RequestPredicates.GET("/"), (request) -> {
                Assert.notEmpty(request.headers().header("FOO"), "FOO is required");
                return ServerResponse.status(HttpStatus.OK).build();
            });
    WebTestClient client = WebTestClient.bindToRouterFunction(route)
            .webFilter((exchange, chain) -> {
                return chain.filter(exchange.mutate().request((builder) -> {
                    builder.header("FOO", "bar");
                }).build());
            }).build();

    client.get().uri("/").exchange().expectStatus().isOk();
    assertThat(client.get().uri("/").exchange().expectBody().returnResult()
            .getRequestHeaders().getFirst("FOO"), is(notNullValue()));
}

It fails (with both RC1 and the latest snapshot) on the assertion at the end which checks that the FOO header is present in the ExchangeResult's request headers.

---

Affects: 5.0 RC1

Referenced from: commits 204a9cf

[spring-projects-issues]

Andy Wilkinson commented

I've just noticed that this also fails:

@Test
public void requestHeadersFromRequestHeaderSpecAreAvailableFromExchangeResult() {
    RouterFunction<ServerResponse> route = RouterFunctions
            .route(RequestPredicates.GET("/"), (request) -> {
                Assert.notEmpty(request.headers().header("FOO"), "FOO is required");
                return ServerResponse.status(HttpStatus.OK).build();
            });
    WebTestClient client = WebTestClient.bindToRouterFunction(route).build();
    client.get().uri("/").header("FOO", "bar").exchange().expectStatus().isOk();
    assertThat(client.get().uri("/").exchange().expectBody().returnResult()
            .getRequestHeaders().getFirst("FOO"), is(notNullValue()));
}

Either my understanding is rather lacking, or the problem is broader than WebFilter.

[spring-projects-issues]

Rossen Stoyanchev commented

For the second example it should and it does work but the test is (erroneously?) executing two separate requests -- the first one sets the header, the second one does the assert. It probably should be:

@Test
public void requestHeadersFromRequestHeaderSpecAreAvailableFromExchangeResult() {
    // ...
    WebTestClient client = WebTestClient.bindToRouterFunction(route).build();
    assertThat(client.get().uri("/").header("FOO", "bar").exchange().expectBody().returnResult()
            .getRequestHeaders().getFirst("FOO"), is(notNullValue()));

The same extra line exists in the first example in the description too but I wouldn't expect that example to work. It's an interesting question whether it should work or not. What the WebTestClient does see currently in the ExchangeResult are the client-side request and response as seen at the connector level (WiretapConnector to be precise), i.e. on the line between client and server even if there is no actual socket. Should changes made to the request on the server side be reflected is what I'm not sure. I don't have a strong sense yet.

[spring-projects-issues]

Andy Wilkinson commented

Wow. Sorry, Rossen. That second example is clearly bogus.

To avoid further confusion, the first example should look like this:

@Test
public void requestHeadersFromWebFilterAreAvailableFromExchangeResult() {
    RouterFunction<ServerResponse> route = RouterFunctions
            .route(RequestPredicates.GET("/"), (request) -> {
                Assert.notEmpty(request.headers().header("FOO"), "FOO is required");
                return ServerResponse.status(HttpStatus.OK).build();
            });
    WebTestClient client = WebTestClient.bindToRouterFunction(route)
            .webFilter((exchange, chain) -> {
                return chain.filter(exchange.mutate().request((builder) -> {
                    builder.header("FOO", "bar");
                }).build());
            }).build();
    assertThat(client.get().uri("/").exchange().expectBody().returnResult()
            .getRequestHeaders().getFirst("FOO"), is(notNullValue()));
}

This use of a header came about, , as you may have guessed, following our discussion about using a global WebFilter to set a header that can be used to identify the request-response exchange and retrieve some state that is specific to it. In using a request header I was following the pattern of WiretapConnector and friends. I think I could use a response header for the same purpose and side-step the question about a "server-side" change being reflected on the client side as shown in this passing test:

@Test
public void responseHeadersFromWebFilterAreAvailableFromExchangeResult() {
    RouterFunction<ServerResponse> route = RouterFunctions
            .route(RequestPredicates.GET("/"), (request) -> {
                return ServerResponse.status(HttpStatus.OK).build();
            });
    WebTestClient client = WebTestClient.bindToRouterFunction(route)
            .webFilter((exchange, chain) -> {
                exchange.getResponse().getHeaders().add("FOO", "bar");
                return chain.filter(exchange);
            }).build();
    assertThat(client.get().uri("/").exchange().expectBody().returnResult()
            .getResponseHeaders().getFirst("FOO"), is("bar"));
}

[spring-projects-issues]

Rossen Stoyanchev commented

You would want to modify the request on the client side, using a client filter. This works:

WebTestClient client = WebTestClient.bindToRouterFunction(route).build()
		.filter((request, next) -> {
			request = ClientRequest.from(request).header("FOO", "bar").build();
			return next.exchange(request);
		});

That said we already stamp the request with a unique request id that you could use. Something like:

@Test
public void requestHeadersFromWebFilterAreAvailableFromExchangeResult() {

	String headerName = "webtestclient-request-id";
	Map<String, String> context = new HashMap<>();

	RouterFunction<ServerResponse> route = RouterFunctions
			.route(RequestPredicates.GET("/"), (request) -> {
				String id = request.headers().header(headerName).get(0);
				context.put(id, "foobar");
				return ServerResponse.status(HttpStatus.OK).build();
			});

	WebTestClient client = WebTestClient.bindToRouterFunction(route).build();

	EntityExchangeResult<byte[]> result = client.get().uri("/").exchange().expectBody().returnResult();
	String id = result.getRequestHeaders().getFirst(headerName);
	System.out.println(context.get(id));
}

[spring-projects-issues]

Rossen Stoyanchev commented

Perhaps what's missing is to promote "webtestclient-request-id" up to a public constant, perhaps on WebTestClient. The header is clearly visible anyway in the ExchangeResult and it's useful and required for the implementation, so it should be more generally useful.

[spring-projects-issues]

Rossen Stoyanchev commented

On second thought, it might make the most sense to add an ExchangeResult.getRequestId() method that returns the WebTestClient-assigned request id.

[spring-projects-issues]

Andy Wilkinson commented

I like the idea of a public constant for the header name. REST Docs will want to filter out the header when it's documenting a request so having a constant for its name would make filtering it out a little bit more robust.

[spring-projects-issues]

Rossen Stoyanchev commented

Sounds good, it will serve the purpose of advertising its existence as well.

[spring-projects-issues]

Rossen Stoyanchev commented

There is now a constant on WebTestClient. Also see related commits under #20129.