From 3ac034e18a81667fd5dc9114ddea53ba2d798306 Mon Sep 17 00:00:00 2001
From: Brian Clozel <bclozel@vmware.com>
Date: Wed, 21 Sep 2022 16:10:56 +0200
Subject: [PATCH] Fail when setReadTimeout on httpclient5 request factory

Prior to this commit, the `RestTemplateBuilder` would offer a generic
`setReadTimeout` method to configure the read timeout on the underlying
`ClientHttpRequestFactory`. This would be done in a reflective fashion,
considering that all implementations align with this behavior.

This option cannot be provided for HttpClient5 at the
`ClientHttpRequestFactory` level anymore, so this has been deprecated
in Spring Framework 6.0 and will log a warning. In order to align with
our existing behavior (throwing exceptions if the option cannot be set),
this commit ensures that exceptions are also thrown if the method is
marked as deprecated.

See gh-32461
---
 .../boot/web/client/RestTemplateBuilder.java     | 12 ++++++++----
 .../web/client/RestTemplateBuilderTests.java     | 16 +++++++---------
 2 files changed, 15 insertions(+), 13 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 f5263583c37..7f8d0883123 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
@@ -783,11 +783,15 @@ public class RestTemplateBuilder {
 
 		private Method findMethod(ClientHttpRequestFactory requestFactory, String methodName, Class<?>... parameters) {
 			Method method = ReflectionUtils.findMethod(requestFactory.getClass(), methodName, parameters);
-			if (method != null) {
-				return method;
+			if (method == null) {
+				throw new IllegalStateException("Request factory " + requestFactory.getClass()
+						+ " does not have a suitable " + methodName + " method");
 			}
-			throw new IllegalStateException("Request factory " + requestFactory.getClass()
-					+ " does not have a suitable " + methodName + " method");
+			else if (method.isAnnotationPresent(Deprecated.class)) {
+				throw new IllegalStateException("Request factory " + requestFactory.getClass() + " has the "
+						+ methodName + " method marked as deprecated");
+			}
+			return method;
 		}
 
 		private void invoke(ClientHttpRequestFactory requestFactory, Method method, Object... parameters) {
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 d84e85683eb..2f7e12cff5c 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
@@ -25,7 +25,6 @@ import java.util.Set;
 import java.util.function.Supplier;
 
 import okhttp3.OkHttpClient;
-import org.apache.http.client.config.RequestConfig;
 import org.assertj.core.api.InstanceOfAssertFactories;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
@@ -63,6 +62,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.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.assertj.core.api.Assertions.entry;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.then;
@@ -81,6 +81,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreat
  * @author Dmytro Nosan
  * @author Kevin Strijbos
  * @author Ilya Lukyanovich
+ * @author Brian Clozel
  */
 @ExtendWith(MockitoExtension.class)
 class RestTemplateBuilderTests {
@@ -477,17 +478,14 @@ class RestTemplateBuilderTests {
 		ClientHttpRequestFactory requestFactory = this.builder
 				.requestFactory(HttpComponentsClientHttpRequestFactory.class).setConnectTimeout(Duration.ofMillis(1234))
 				.build().getRequestFactory();
-		assertThat(((RequestConfig) ReflectionTestUtils.getField(requestFactory, "requestConfig")).getConnectTimeout())
-				.isEqualTo(1234);
+		assertThat(((int) ReflectionTestUtils.getField(requestFactory, "connectTimeout"))).isEqualTo(1234);
 	}
 
 	@Test
-	void readTimeoutCanBeConfiguredOnHttpComponentsRequestFactory() {
-		ClientHttpRequestFactory requestFactory = this.builder
-				.requestFactory(HttpComponentsClientHttpRequestFactory.class).setReadTimeout(Duration.ofMillis(1234))
-				.build().getRequestFactory();
-		assertThat(((RequestConfig) ReflectionTestUtils.getField(requestFactory, "requestConfig")).getSocketTimeout())
-				.isEqualTo(1234);
+	void readTimeoutConfigurationFailsOnHttpComponentsRequestFactory() {
+		assertThatThrownBy(() -> this.builder.requestFactory(HttpComponentsClientHttpRequestFactory.class)
+				.setReadTimeout(Duration.ofMillis(1234)).build()).isInstanceOf(IllegalStateException.class)
+						.hasMessageContaining("setReadTimeout method marked as deprecated");
 	}
 
 	@Test