From c832d5f496737617850ea8f9bb0b6025736e238c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Wed, 18 Sep 2024 11:39:36 +0200 Subject: [PATCH] Polishing contribution Fixes gh-33556 --- ...ttpComponentsClientHttpRequestFactory.java | 19 ++++++------------- ...mponentsClientHttpRequestFactoryTests.java | 9 +++++++++ 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java b/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java index eea1cb720af7..2cc0463a8696 100644 --- a/spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java +++ b/spring-web/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. *

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. *

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. *

Additional properties can be configured by specifying a * {@link RequestConfig} instance on a custom {@link HttpClient}. - *

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. *

Additional properties can be configured by specifying a * {@link RequestConfig} instance on a custom {@link HttpClient}. - *

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/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java b/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java index 8ad64e76b514..5ef09b4bd5bc 100644 --- a/spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java +++ b/spring-web/src/test/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