From 9f06514b895a0962839a3de6ac838e84166bd9ca Mon Sep 17 00:00:00 2001 From: kevinstrijbos Date: Sat, 25 May 2019 18:23:02 +0200 Subject: [PATCH 1/2] Make it easier to set bufferRequestBody Update `RestTemplateBuilder` to make it easier to set the `bufferRequestBody` value on a underlying `ClientHttpRequestFactory` See gh-16972 --- .../boot/web/client/RestTemplateBuilder.java | 76 ++++++++++++++++++- .../web/client/RestTemplateBuilderTests.java | 64 ++++++++++++++++ 2 files changed, 136 insertions(+), 4 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java index 784aacbb0c..27eae51380 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java @@ -60,6 +60,7 @@ import org.springframework.web.util.UriTemplateHandler; * @author Andy Wilkinson * @author Brian Clozel * @author Dmytro Nosan + * @author Kevin Strijbos * @since 1.4.0 */ public class RestTemplateBuilder { @@ -506,6 +507,22 @@ public class RestTemplateBuilder { this.interceptors); } + /** + * Sets the bufferrequestbody value on the underlying + * {@link ClientHttpRequestFactory}. + * @param bufferRequestBody value of the bufferRequestBody parameter + * @return a new builder instance. + * @since 2.1.0 + */ + public RestTemplateBuilder setBufferRequestBody(boolean bufferRequestBody) { + return new RestTemplateBuilder(this.detectRequestFactory, this.rootUri, + this.messageConverters, this.requestFactorySupplier, + this.uriTemplateHandler, this.errorHandler, this.basicAuthentication, + this.restTemplateCustomizers, + this.requestFactoryCustomizer.bufferRequestBody(bufferRequestBody), + this.interceptors); + } + /** * Build a new {@link RestTemplate} instance and configure it using this builder. * @return a configured {@link RestTemplate} instance. @@ -617,21 +634,35 @@ public class RestTemplateBuilder { private final Duration readTimeout; + private final boolean bufferRequestBody; + + private final boolean bufferRequestBodyFlag; + RequestFactoryCustomizer() { - this(null, null); + this(null, null, true, false); } - private RequestFactoryCustomizer(Duration connectTimeout, Duration readTimeout) { + private RequestFactoryCustomizer(Duration connectTimeout, Duration readTimeout, + boolean bufferRequestBody, boolean bufferRequestBodyFlag) { this.connectTimeout = connectTimeout; this.readTimeout = readTimeout; + this.bufferRequestBody = bufferRequestBody; + this.bufferRequestBodyFlag = bufferRequestBodyFlag; } public RequestFactoryCustomizer connectTimeout(Duration connectTimeout) { - return new RequestFactoryCustomizer(connectTimeout, this.readTimeout); + return new RequestFactoryCustomizer(connectTimeout, this.readTimeout, + this.bufferRequestBody, this.bufferRequestBodyFlag); } public RequestFactoryCustomizer readTimeout(Duration readTimeout) { - return new RequestFactoryCustomizer(this.connectTimeout, readTimeout); + return new RequestFactoryCustomizer(this.connectTimeout, readTimeout, + this.bufferRequestBody, this.bufferRequestBodyFlag); + } + + public RequestFactoryCustomizer bufferRequestBody(boolean bufferRequestBody) { + return new RequestFactoryCustomizer(this.connectTimeout, this.readTimeout, + bufferRequestBody, true); } @Override @@ -646,6 +677,10 @@ public class RestTemplateBuilder { new TimeoutRequestFactoryCustomizer(this.readTimeout, "setReadTimeout") .customize(unwrappedRequestFactory); } + if (this.bufferRequestBodyFlag) { + new BufferRequestBodyFactoryCustomizer(this.bufferRequestBody, + "setBufferRequestBody").customize(unwrappedRequestFactory); + } } private ClientHttpRequestFactory unwrapRequestFactoryIfNecessary( @@ -695,6 +730,39 @@ public class RestTemplateBuilder { } + /** + * {@link ClientHttpRequestFactory} customizer to call a "set buffer request body" + * method. + */ + private static final class BufferRequestBodyFactoryCustomizer { + + private final boolean bufferRequestBody; + + private final String methodName; + + BufferRequestBodyFactoryCustomizer(boolean bufferRequestBody, + String methodName) { + this.bufferRequestBody = bufferRequestBody; + this.methodName = methodName; + } + + void customize(ClientHttpRequestFactory factory) { + ReflectionUtils.invokeMethod(findMethod(factory), factory, + this.bufferRequestBody); + } + + private Method findMethod(ClientHttpRequestFactory factory) { + Method method = ReflectionUtils.findMethod(factory.getClass(), + this.methodName, boolean.class); + if (method != null) { + return method; + } + throw new IllegalStateException("Request factory " + factory.getClass() + + " does not have a " + this.methodName + "(boolean) method"); + } + + } + } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java index 22b54cd147..d94cb43560 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/RestTemplateBuilderTests.java @@ -47,6 +47,7 @@ import org.springframework.web.util.UriTemplateHandler; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.assertj.core.api.Assertions.assertThatIllegalStateException; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.inOrder; import static org.mockito.Mockito.mock; @@ -63,6 +64,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreat * @author Phillip Webb * @author Andy Wilkinson * @author Dmytro Nosan + * @author Kevin Strijbos */ public class RestTemplateBuilderTests { @@ -480,6 +482,23 @@ public class RestTemplateBuilderTests { "requestConfig")).getSocketTimeout()).isEqualTo(1234); } + @Test + public void bufferRequestBodyCanBeConfiguredOnHttpComponentsRequestFactory() { + ClientHttpRequestFactory requestFactory = this.builder + .requestFactory(HttpComponentsClientHttpRequestFactory.class) + .setBufferRequestBody(false).build().getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", + false); + requestFactory = this.builder + .requestFactory(HttpComponentsClientHttpRequestFactory.class) + .setBufferRequestBody(true).build().getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + requestFactory = this.builder + .requestFactory(HttpComponentsClientHttpRequestFactory.class).build() + .getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + } + @Test public void connectTimeoutCanBeConfiguredOnSimpleRequestFactory() { ClientHttpRequestFactory requestFactory = this.builder @@ -496,6 +515,21 @@ public class RestTemplateBuilderTests { assertThat(requestFactory).hasFieldOrPropertyWithValue("readTimeout", 1234); } + @Test + public void bufferRequestBodyCanBeConfiguredOnSimpleRequestFactory() { + ClientHttpRequestFactory requestFactory = this.builder + .requestFactory(SimpleClientHttpRequestFactory.class) + .setBufferRequestBody(false).build().getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", + false); + requestFactory = this.builder.requestFactory(SimpleClientHttpRequestFactory.class) + .setBufferRequestBody(true).build().getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + requestFactory = this.builder.requestFactory(SimpleClientHttpRequestFactory.class) + .build().getRequestFactory(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + } + @Test public void connectTimeoutCanBeConfiguredOnOkHttp3RequestFactory() { ClientHttpRequestFactory requestFactory = this.builder @@ -516,6 +550,15 @@ public class RestTemplateBuilderTests { .isEqualTo(1234); } + @Test + public void bufferRequestBodyCanNotBeConfiguredOnOkHttp3RequestFactory() { + assertThatIllegalStateException() + .isThrownBy(() -> this.builder + .requestFactory(OkHttp3ClientHttpRequestFactory.class) + .setBufferRequestBody(false).build().getRequestFactory()) + .withMessageContaining(OkHttp3ClientHttpRequestFactory.class.getName()); + } + @Test public void connectTimeoutCanBeConfiguredOnAWrappedRequestFactory() { SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); @@ -536,6 +579,27 @@ public class RestTemplateBuilderTests { assertThat(requestFactory).hasFieldOrPropertyWithValue("readTimeout", 1234); } + @Test + public void bufferRequestBodyCanBeConfiguredOnAWrappedRequestFactory() { + SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); + this.builder + .requestFactory( + () -> new BufferingClientHttpRequestFactory(requestFactory)) + .setBufferRequestBody(false).build(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", + false); + this.builder + .requestFactory( + () -> new BufferingClientHttpRequestFactory(requestFactory)) + .setBufferRequestBody(true).build(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + this.builder + .requestFactory( + () -> new BufferingClientHttpRequestFactory(requestFactory)) + .build(); + assertThat(requestFactory).hasFieldOrPropertyWithValue("bufferRequestBody", true); + } + @Test public void unwrappingDoesNotAffectRequestFactoryThatIsSetOnTheBuiltTemplate() { SimpleClientHttpRequestFactory requestFactory = new SimpleClientHttpRequestFactory(); From af1a6d86499e596b57b47018684d89c5150835f0 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Mon, 3 Jun 2019 13:41:12 -0700 Subject: [PATCH 2/2] Polish "Make it easier to set bufferRequestBody" See gh-16972 --- .../boot/web/client/RestTemplateBuilder.java | 118 +++++++----------- 1 file changed, 43 insertions(+), 75 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java index 27eae51380..a387722833 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/RestTemplateBuilder.java @@ -33,8 +33,11 @@ import java.util.function.Supplier; import org.springframework.beans.BeanUtils; import org.springframework.http.client.AbstractClientHttpRequestFactoryWrapper; +import org.springframework.http.client.ClientHttpRequest; import org.springframework.http.client.ClientHttpRequestFactory; import org.springframework.http.client.ClientHttpRequestInterceptor; +import org.springframework.http.client.HttpComponentsClientHttpRequestFactory; +import org.springframework.http.client.SimpleClientHttpRequestFactory; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; @@ -508,11 +511,13 @@ public class RestTemplateBuilder { } /** - * Sets the bufferrequestbody value on the underlying - * {@link ClientHttpRequestFactory}. + * Sets if the underling {@link ClientHttpRequestFactory} should buffer the + * {@linkplain ClientHttpRequest#getBody() request body} internally. * @param bufferRequestBody value of the bufferRequestBody parameter * @return a new builder instance. - * @since 2.1.0 + * @since 2.2.0 + * @see SimpleClientHttpRequestFactory#setBufferRequestBody(boolean) + * @see HttpComponentsClientHttpRequestFactory#setBufferRequestBody(boolean) */ public RestTemplateBuilder setBufferRequestBody(boolean bufferRequestBody) { return new RestTemplateBuilder(this.detectRequestFactory, this.rootUri, @@ -634,35 +639,32 @@ public class RestTemplateBuilder { private final Duration readTimeout; - private final boolean bufferRequestBody; - - private final boolean bufferRequestBodyFlag; + private final Boolean bufferRequestBody; RequestFactoryCustomizer() { - this(null, null, true, false); + this(null, null, null); } private RequestFactoryCustomizer(Duration connectTimeout, Duration readTimeout, - boolean bufferRequestBody, boolean bufferRequestBodyFlag) { + Boolean bufferRequestBody) { this.connectTimeout = connectTimeout; this.readTimeout = readTimeout; this.bufferRequestBody = bufferRequestBody; - this.bufferRequestBodyFlag = bufferRequestBodyFlag; } public RequestFactoryCustomizer connectTimeout(Duration connectTimeout) { return new RequestFactoryCustomizer(connectTimeout, this.readTimeout, - this.bufferRequestBody, this.bufferRequestBodyFlag); + this.bufferRequestBody); } public RequestFactoryCustomizer readTimeout(Duration readTimeout) { return new RequestFactoryCustomizer(this.connectTimeout, readTimeout, - this.bufferRequestBody, this.bufferRequestBodyFlag); + this.bufferRequestBody); } public RequestFactoryCustomizer bufferRequestBody(boolean bufferRequestBody) { return new RequestFactoryCustomizer(this.connectTimeout, this.readTimeout, - bufferRequestBody, true); + bufferRequestBody); } @Override @@ -670,16 +672,13 @@ public class RestTemplateBuilder { ClientHttpRequestFactory unwrappedRequestFactory = unwrapRequestFactoryIfNecessary( requestFactory); if (this.connectTimeout != null) { - new TimeoutRequestFactoryCustomizer(this.connectTimeout, - "setConnectTimeout").customize(unwrappedRequestFactory); + setConnectTimeout(unwrappedRequestFactory); } if (this.readTimeout != null) { - new TimeoutRequestFactoryCustomizer(this.readTimeout, "setReadTimeout") - .customize(unwrappedRequestFactory); + setReadTimeout(unwrappedRequestFactory); } - if (this.bufferRequestBodyFlag) { - new BufferRequestBodyFactoryCustomizer(this.bufferRequestBody, - "setBufferRequestBody").customize(unwrappedRequestFactory); + if (this.bufferRequestBody != null) { + setBufferRequestBody(unwrappedRequestFactory); } } @@ -699,68 +698,37 @@ public class RestTemplateBuilder { return unwrappedRequestFactory; } - /** - * {@link ClientHttpRequestFactory} customizer to call a "set timeout" method. - */ - private static final class TimeoutRequestFactoryCustomizer { - - private final Duration timeout; - - private final String methodName; - - TimeoutRequestFactoryCustomizer(Duration timeout, String methodName) { - this.timeout = timeout; - this.methodName = methodName; - } - - void customize(ClientHttpRequestFactory factory) { - ReflectionUtils.invokeMethod(findMethod(factory), factory, - Math.toIntExact(this.timeout.toMillis())); - } - - private Method findMethod(ClientHttpRequestFactory factory) { - Method method = ReflectionUtils.findMethod(factory.getClass(), - this.methodName, int.class); - if (method != null) { - return method; - } - throw new IllegalStateException("Request factory " + factory.getClass() - + " does not have a " + this.methodName + "(int) method"); - } - + private void setConnectTimeout(ClientHttpRequestFactory factory) { + Method method = findMethod(factory, "setConnectTimeout", int.class); + int timeout = Math.toIntExact(this.connectTimeout.toMillis()); + invoke(factory, method, timeout); } - /** - * {@link ClientHttpRequestFactory} customizer to call a "set buffer request body" - * method. - */ - private static final class BufferRequestBodyFactoryCustomizer { + private void setReadTimeout(ClientHttpRequestFactory factory) { + Method method = findMethod(factory, "setReadTimeout", int.class); + int timeout = Math.toIntExact(this.readTimeout.toMillis()); + invoke(factory, method, timeout); + } - private final boolean bufferRequestBody; + private void setBufferRequestBody(ClientHttpRequestFactory factory) { + Method method = findMethod(factory, "setBufferRequestBody", boolean.class); + invoke(factory, method, this.bufferRequestBody); + } - private final String methodName; - - BufferRequestBodyFactoryCustomizer(boolean bufferRequestBody, - String methodName) { - this.bufferRequestBody = bufferRequestBody; - this.methodName = methodName; - } - - void customize(ClientHttpRequestFactory factory) { - ReflectionUtils.invokeMethod(findMethod(factory), factory, - this.bufferRequestBody); - } - - private Method findMethod(ClientHttpRequestFactory factory) { - Method method = ReflectionUtils.findMethod(factory.getClass(), - this.methodName, boolean.class); - if (method != null) { - return method; - } - throw new IllegalStateException("Request factory " + factory.getClass() - + " does not have a " + this.methodName + "(boolean) method"); + private Method findMethod(ClientHttpRequestFactory requestFactory, + String methodName, Class... parameters) { + Method method = ReflectionUtils.findMethod(requestFactory.getClass(), + methodName, parameters); + if (method != null) { + return method; } + throw new IllegalStateException("Request factory " + requestFactory.getClass() + + " does not have a suitable " + methodName + " method"); + } + private void invoke(ClientHttpRequestFactory requestFactory, Method method, + Object... parameters) { + ReflectionUtils.invokeMethod(method, requestFactory, parameters); } }