From d7a29bbcef7d278a3ca9fd66ceb70c60fb283cc0 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 22 May 2020 07:32:16 +0100 Subject: [PATCH] DefaultClientResponseBuilder copies logPrefix Closes gh-25069 --- .../reactive/MockClientHttpRequest.java | 8 +++++- .../reactive/MockClientHttpResponse.java | 2 +- .../client/DefaultClientResponseBuilder.java | 18 +++++++------ .../DefaultClientResponseBuilderTests.java | 25 +++++++++++++------ 4 files changed, 36 insertions(+), 17 deletions(-) diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpRequest.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpRequest.java index 1fdd0edcd3..f21c01c195 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpRequest.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpRequest.java @@ -33,6 +33,7 @@ import org.springframework.core.io.buffer.DataBufferUtils; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpHeaders; import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; import org.springframework.http.client.reactive.AbstractClientHttpRequest; import org.springframework.http.client.reactive.ClientHttpRequest; import org.springframework.util.Assert; @@ -46,7 +47,7 @@ import org.springframework.web.util.UriComponentsBuilder; * @author Rossen Stoyanchev * @since 5.0 */ -public class MockClientHttpRequest extends AbstractClientHttpRequest { +public class MockClientHttpRequest extends AbstractClientHttpRequest implements HttpRequest { private final HttpMethod httpMethod; @@ -96,6 +97,11 @@ public class MockClientHttpRequest extends AbstractClientHttpRequest { return this.httpMethod; } + @Override + public String getMethodValue() { + return this.httpMethod.name(); + } + @Override public URI getURI() { return this.url; diff --git a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpResponse.java b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpResponse.java index 7ee7e6afb9..e5f014e871 100644 --- a/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpResponse.java +++ b/spring-web/src/testFixtures/java/org/springframework/web/testfixture/http/client/reactive/MockClientHttpResponse.java @@ -83,7 +83,7 @@ public class MockClientHttpResponse implements ClientHttpResponse { public HttpHeaders getHeaders() { if (!getCookies().isEmpty() && this.headers.get(HttpHeaders.SET_COOKIE) == null) { getCookies().values().stream().flatMap(Collection::stream) - .forEach(cookie -> getHeaders().add(HttpHeaders.SET_COOKIE, cookie.toString())); + .forEach(cookie -> this.headers.add(HttpHeaders.SET_COOKIE, cookie.toString())); } return this.headers; } diff --git a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java index 78751e00fc..b3520aae13 100644 --- a/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java +++ b/spring-webflux/src/main/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilder.java @@ -31,6 +31,7 @@ import org.springframework.http.HttpRequest; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; import org.springframework.http.client.reactive.ClientHttpResponse; +import org.springframework.lang.Nullable; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.LinkedMultiValueMap; @@ -75,6 +76,9 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { private Flux body = Flux.empty(); + @Nullable + private ClientResponse originalResponse; + private HttpRequest request; @@ -90,12 +94,9 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { this.statusCode = other.rawStatusCode(); this.headers.addAll(other.headers().asHttpHeaders()); this.cookies.addAll(other.cookies()); - if (other instanceof DefaultClientResponse) { - this.request = ((DefaultClientResponse) other).request(); - } - else { - this.request = EMPTY_REQUEST; - } + this.originalResponse = other; + this.request = (other instanceof DefaultClientResponse ? + ((DefaultClientResponse) other).request() : EMPTY_REQUEST); } @@ -178,7 +179,10 @@ final class DefaultClientResponseBuilder implements ClientResponse.Builder { // When building ClientResponse manually, the ClientRequest.logPrefix() has to be passed, // e.g. via ClientResponse.Builder, but this (builder) is not used currently. - return new DefaultClientResponse(httpResponse, this.strategies, "", "", () -> this.request); + return new DefaultClientResponse(httpResponse, this.strategies, + this.originalResponse != null ? this.originalResponse.logPrefix() : "", + this.request.getMethodValue() + " " + this.request.getURI(), + () -> this.request); } diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java index 8e6c01ef63..ab83781e39 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/function/client/DefaultClientResponseBuilderTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2020 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -26,8 +26,12 @@ import org.springframework.core.io.buffer.DataBuffer; import org.springframework.core.io.buffer.DataBufferFactory; import org.springframework.core.io.buffer.DefaultDataBufferFactory; import org.springframework.http.HttpHeaders; +import org.springframework.http.HttpMethod; +import org.springframework.http.HttpRequest; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseCookie; +import org.springframework.web.testfixture.http.client.reactive.MockClientHttpRequest; +import org.springframework.web.testfixture.http.client.reactive.MockClientHttpResponse; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -69,11 +73,16 @@ public class DefaultClientResponseBuilderTests { .map(s -> s.getBytes(StandardCharsets.UTF_8)) .map(dataBufferFactory::wrap); - ClientResponse other = ClientResponse.create(HttpStatus.BAD_REQUEST, ExchangeStrategies.withDefaults()) - .header("foo", "bar") - .cookie("baz", "qux") - .body(otherBody) - .build(); + HttpRequest mockClientHttpRequest = new MockClientHttpRequest(HttpMethod.GET, "/path"); + + MockClientHttpResponse httpResponse = new MockClientHttpResponse(HttpStatus.BAD_REQUEST); + httpResponse.getHeaders().add("foo", "bar"); + httpResponse.getCookies().add("baz", ResponseCookie.from("baz", "qux").build()); + httpResponse.setBody(otherBody); + + + DefaultClientResponse other = new DefaultClientResponse( + httpResponse, ExchangeStrategies.withDefaults(), "my-prefix", "", () -> mockClientHttpRequest); Flux body = Flux.just("baz") .map(s -> s.getBytes(StandardCharsets.UTF_8)) @@ -86,10 +95,11 @@ public class DefaultClientResponseBuilderTests { .build(); assertThat(result.statusCode()).isEqualTo(HttpStatus.BAD_REQUEST); - assertThat(result.headers().asHttpHeaders().size()).isEqualTo(1); + assertThat(result.headers().asHttpHeaders().size()).isEqualTo(2); assertThat(result.headers().asHttpHeaders().getFirst("foo")).isEqualTo("baar"); assertThat(result.cookies().size()).isEqualTo(1); assertThat(result.cookies().getFirst("baz").getValue()).isEqualTo("quux"); + assertThat(result.logPrefix()).isEqualTo("my-prefix"); StepVerifier.create(result.bodyToFlux(String.class)) .expectNext("baz") @@ -104,5 +114,4 @@ public class DefaultClientResponseBuilderTests { assertThat(result.rawStatusCode()).isEqualTo(499); assertThatIllegalArgumentException().isThrownBy(result::statusCode); } - }