Polishing contribution

Fixes gh-33556
spring-projects · Sep 18, 2024 · c832d5f · c832d5f
1 parent 453e796
commit c832d5f
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 13 deletions.
diff --git a/...src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java b/...src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java
@@ -73,7 +73,6 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 
 	private long connectionRequestTimeout = -1;
 
-
 	private long readTimeout = -1;
 
 	/**
@@ -138,7 +137,7 @@ public void setConnectTimeout(int connectTimeout) {
 	 * handshakes or CONNECT requests; for that, it is required to
 	 * use the {@link SocketConfig} on the
 	 * {@link HttpClient} itself.
-	 * @param connectTimeout the timeout as {@code Duration}.
+	 * @param connectTimeout the timeout as a {@code Duration}.
 	 * @since 6.1
 	 * @see RequestConfig#getConnectTimeout()
 	 * @see SocketConfig#getSoTimeout
@@ -155,7 +154,8 @@ public void setConnectTimeout(Duration connectTimeout) {
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * <p>Additional properties can be configured by specifying a
 	 * {@link RequestConfig} instance on a custom {@link HttpClient}.
-	 * @param connectionRequestTimeout the timeout value to request a connection in milliseconds
+	 * @param connectionRequestTimeout the timeout value to request a connection
+	 * in milliseconds
 	 * @see RequestConfig#getConnectionRequestTimeout()
 	 */
 	public void setConnectionRequestTimeout(int connectionRequestTimeout) {
@@ -169,7 +169,8 @@ public void setConnectionRequestTimeout(int connectionRequestTimeout) {
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * <p>Additional properties can be configured by specifying a
 	 * {@link RequestConfig} instance on a custom {@link HttpClient}.
-	 * @param connectionRequestTimeout the timeout value to request a connection as {@code Duration}.
+	 * @param connectionRequestTimeout the timeout value to request a connection
+	 * as a {@code Duration}.
 	 * @since 6.1
 	 * @see RequestConfig#getConnectionRequestTimeout()
 	 */
@@ -184,10 +185,6 @@ public void setConnectionRequestTimeout(Duration connectionRequestTimeout) {
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * <p>Additional properties can be configured by specifying a
 	 * {@link RequestConfig} instance on a custom {@link HttpClient}.
-	 * <p>This options does not affect connection timeouts for SSL
-	 * handshakes or CONNECT requests; for that, it is required to
-	 * use the {@link SocketConfig} on the
-	 * {@link HttpClient} itself.
 	 * @param readTimeout the timeout value in milliseconds
 	 * @since 6.2
 	 * @see RequestConfig#getResponseTimeout()
@@ -202,11 +199,7 @@ public void setReadTimeout(int readTimeout) {
 	 * A timeout value of 0 specifies an infinite timeout.
 	 * <p>Additional properties can be configured by specifying a
 	 * {@link RequestConfig} instance on a custom {@link HttpClient}.
-	 * <p>This options does not affect connection timeouts for SSL
-	 * handshakes or CONNECT requests; for that, it is required to
-	 * use the {@link SocketConfig} on the
-	 * {@link HttpClient} itself.
-	 * @param readTimeout the timeout as {@code Duration}.
+	 * @param readTimeout the timeout as a {@code Duration}.
 	 * @since 6.2
 	 * @see RequestConfig#getResponseTimeout()
 	 */

diff --git a/...est/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java b/...est/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java
@@ -18,6 +18,7 @@
 
 import java.io.InputStreamReader;
 import java.net.URI;
+import java.time.Duration;
 import java.util.stream.Stream;
 
 import org.apache.hc.client5.http.classic.HttpClient;
@@ -66,6 +67,7 @@ void assertCustomConfig() throws Exception {
 		HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient);
 		hrf.setConnectTimeout(1234);
 		hrf.setConnectionRequestTimeout(4321);
+		hrf.setReadTimeout(5678);
 
 		URI uri = URI.create(baseUrl + "/status/ok");
 		HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest) hrf.createRequest(uri, HttpMethod.GET);
@@ -76,6 +78,7 @@ void assertCustomConfig() throws Exception {
 		RequestConfig requestConfig = (RequestConfig) config;
 		assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(Timeout.of(1234, MILLISECONDS));
 		assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(Timeout.of(4321, MILLISECONDS));
+		assertThat(requestConfig.getResponseTimeout()).as("Wrong custom response timeout").isEqualTo(Timeout.of(5678, MILLISECONDS));
 	}
 
 	@Test
@@ -105,6 +108,7 @@ void localSettingsOverrideClientDefaultSettings() throws Exception {
 		RequestConfig defaultConfig = RequestConfig.custom()
 				.setConnectTimeout(1234, MILLISECONDS)
 				.setConnectionRequestTimeout(6789, MILLISECONDS)
+				.setResponseTimeout(4321, MILLISECONDS)
 				.build();
 		CloseableHttpClient client = mock(CloseableHttpClient.class,
 				withSettings().extraInterfaces(Configurable.class));
@@ -113,10 +117,12 @@ void localSettingsOverrideClientDefaultSettings() throws Exception {
 
 		HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(client);
 		hrf.setConnectTimeout(5000);
+		hrf.setReadTimeout(Duration.ofMillis(4000));
 
 		RequestConfig requestConfig = retrieveRequestConfig(hrf);
 		assertThat(requestConfig.getConnectTimeout()).isEqualTo(Timeout.of(5000, MILLISECONDS));
 		assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.of(6789, MILLISECONDS));
+		assertThat(requestConfig.getResponseTimeout()).isEqualTo(Timeout.of(4000, MILLISECONDS));
 	}
 
 	@Test
@@ -141,15 +147,18 @@ public HttpClient getHttpClient() {
 		RequestConfig requestConfig = retrieveRequestConfig(hrf);
 		assertThat(requestConfig.getConnectionRequestTimeout()).isEqualTo(Timeout.of(5000, MILLISECONDS));
 		assertThat(requestConfig.getConnectTimeout()).isEqualTo(RequestConfig.DEFAULT.getConnectTimeout());
+		assertThat(requestConfig.getResponseTimeout()).isEqualTo(RequestConfig.DEFAULT.getResponseTimeout());
 
 		// Update the Http client so that it returns an updated config
 		RequestConfig updatedDefaultConfig = RequestConfig.custom()
 				.setConnectTimeout(1234, MILLISECONDS).build();
 		given(configurable.getConfig()).willReturn(updatedDefaultConfig);
 		hrf.setConnectionRequestTimeout(7000);
+		hrf.setReadTimeout(4000);
 		RequestConfig requestConfig2 = retrieveRequestConfig(hrf);
 		assertThat(requestConfig2.getConnectTimeout()).isEqualTo(Timeout.of(1234, MILLISECONDS));
 		assertThat(requestConfig2.getConnectionRequestTimeout()).isEqualTo(Timeout.of(7000, MILLISECONDS));
+		assertThat(requestConfig2.getResponseTimeout()).isEqualTo(Timeout.of(4000, MILLISECONDS));
 	}
 
 	@ParameterizedTest