Skip to content

Commit 92f7414

Browse files
mhalbrittermhalbritter
committed
Favor Duration accepting timeout method over int method
Closes gh-40635
1 parent c47cdda commit 92f7414

File tree

8 files changed

+148
-7
lines changed

8 files changed

+148
-7
lines changed

‎spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/client/ClientHttpRequestFactories.java‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ static SimpleClientHttpRequestFactory get(ClientHttpRequestFactorySettings setti
329329
*/
330330
private static class SimpleClientHttpsRequestFactory extends SimpleClientHttpRequestFactory {
331331

332-
private SslBundle sslBundle;
332+
private final SslBundle sslBundle;
333333

334334
SimpleClientHttpsRequestFactory(SslBundle sslBundle) {
335335
this.sslBundle = sslBundle;
@@ -385,13 +385,23 @@ private static ClientHttpRequestFactory unwrapRequestFactoryIfNecessary(
385385
}
386386

387387
private static void setConnectTimeout(ClientHttpRequestFactory factory, Duration connectTimeout) {
388-
Method method = findMethod(factory, "setConnectTimeout", int.class);
388+
Method method = tryFindMethod(factory, "setConnectTimeout", Duration.class);
389+
if (method != null) {
390+
invoke(factory, method, connectTimeout);
391+
return;
392+
}
393+
method = findMethod(factory, "setConnectTimeout", int.class);
389394
int timeout = Math.toIntExact(connectTimeout.toMillis());
390395
invoke(factory, method, timeout);
391396
}
392397

393398
private static void setReadTimeout(ClientHttpRequestFactory factory, Duration readTimeout) {
394-
Method method = findMethod(factory, "setReadTimeout", int.class);
399+
Method method = tryFindMethod(factory, "setReadTimeout", Duration.class);
400+
if (method != null) {
401+
invoke(factory, method, readTimeout);
402+
return;
403+
}
404+
method = findMethod(factory, "setReadTimeout", int.class);
395405
int timeout = Math.toIntExact(readTimeout.toMillis());
396406
invoke(factory, method, timeout);
397407
}
@@ -407,6 +417,18 @@ private static Method findMethod(ClientHttpRequestFactory requestFactory, String
407417
return method;
408418
}
409419

420+
private static Method tryFindMethod(ClientHttpRequestFactory requestFactory, String methodName,
421+
Class<?>... parameters) {
422+
Method method = ReflectionUtils.findMethod(requestFactory.getClass(), methodName, parameters);
423+
if (method == null) {
424+
return null;
425+
}
426+
if (method.isAnnotationPresent(Deprecated.class)) {
427+
return null;
428+
}
429+
return method;
430+
}
431+
410432
private static void invoke(ClientHttpRequestFactory requestFactory, Method method, Object... parameters) {
411433
ReflectionUtils.invokeMethod(method, requestFactory, parameters);
412434
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/AbstractClientHttpRequestFactoriesTests.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import jakarta.servlet.http.HttpServlet;
2828
import jakarta.servlet.http.HttpServletRequest;
2929
import jakarta.servlet.http.HttpServletResponse;
30+
import org.junit.jupiter.api.Assumptions;
3031
import org.junit.jupiter.api.Test;
3132
import org.junit.jupiter.params.ParameterizedTest;
3233
import org.junit.jupiter.params.provider.ValueSource;
@@ -101,6 +102,26 @@ void getReturnsRequestFactoryWithConfiguredReadTimeout() {
101102
assertThat(readTimeout((T) requestFactory)).isEqualTo(Duration.ofSeconds(120).toMillis());
102103
}
103104

105+
@Test
106+
void shouldSetConnectTimeoutsWhenUsingReflective() {
107+
Assumptions.assumeTrue(supportsSettingConnectTimeout());
108+
ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.DEFAULTS
109+
.withConnectTimeout(Duration.ofSeconds(1));
110+
T requestFactory = ClientHttpRequestFactories
111+
.get(() -> ClientHttpRequestFactories.get(this.requestFactoryType, settings), settings);
112+
assertThat(connectTimeout(requestFactory)).isEqualTo(1000);
113+
}
114+
115+
@Test
116+
void shouldSetReadTimeoutsWhenUsingReflective() {
117+
Assumptions.assumeTrue(supportsSettingReadTimeout());
118+
ClientHttpRequestFactorySettings settings = ClientHttpRequestFactorySettings.DEFAULTS
119+
.withReadTimeout(Duration.ofSeconds(2));
120+
T requestFactory = ClientHttpRequestFactories
121+
.get(() -> ClientHttpRequestFactories.get(this.requestFactoryType, settings), settings);
122+
assertThat(readTimeout(requestFactory)).isEqualTo(2000);
123+
}
124+
104125
@ParameterizedTest
105126
@ValueSource(strings = { "GET", "POST" })
106127
void connectWithSslBundle(String httpMethod) throws Exception {
@@ -136,8 +157,12 @@ void connectWithSslBundle(String httpMethod) throws Exception {
136157
}
137158
}
138159

160+
protected abstract boolean supportsSettingConnectTimeout();
161+
139162
protected abstract long connectTimeout(T requestFactory);
140163

164+
protected abstract boolean supportsSettingReadTimeout();
165+
141166
protected abstract long readTimeout(T requestFactory);
142167

143168
public static class TestServlet extends HttpServlet {

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesHttpComponentsTests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,4 +53,14 @@ protected long readTimeout(HttpComponentsClientHttpRequestFactory requestFactory
5353
return socketConfig.getSoTimeout().toMilliseconds();
5454
}
5555

56+
@Override
57+
protected boolean supportsSettingConnectTimeout() {
58+
return true;
59+
}
60+
61+
@Override
62+
protected boolean supportsSettingReadTimeout() {
63+
return false;
64+
}
65+
5666
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesJettyTests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,14 @@ protected long readTimeout(JettyClientHttpRequestFactory requestFactory) {
4545
return (long) ReflectionTestUtils.getField(requestFactory, "readTimeout");
4646
}
4747

48+
@Override
49+
protected boolean supportsSettingConnectTimeout() {
50+
return true;
51+
}
52+
53+
@Override
54+
protected boolean supportsSettingReadTimeout() {
55+
return true;
56+
}
57+
4858
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesOkHttp3Tests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,4 +61,14 @@ protected long readTimeout(OkHttp3ClientHttpRequestFactory requestFactory) {
6161
return ((OkHttpClient) ReflectionTestUtils.getField(requestFactory, "client")).readTimeoutMillis();
6262
}
6363

64+
@Override
65+
protected boolean supportsSettingConnectTimeout() {
66+
return true;
67+
}
68+
69+
@Override
70+
protected boolean supportsSettingReadTimeout() {
71+
return true;
72+
}
73+
6474
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesOkHttp4Tests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,4 +59,14 @@ protected long readTimeout(OkHttp3ClientHttpRequestFactory requestFactory) {
5959
return ((OkHttpClient) ReflectionTestUtils.getField(requestFactory, "client")).readTimeoutMillis();
6060
}
6161

62+
@Override
63+
protected boolean supportsSettingConnectTimeout() {
64+
return true;
65+
}
66+
67+
@Override
68+
protected boolean supportsSettingReadTimeout() {
69+
return true;
70+
}
71+
6272
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesSimpleTests.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,4 +44,14 @@ protected long readTimeout(SimpleClientHttpRequestFactory requestFactory) {
4444
return (int) ReflectionTestUtils.getField(requestFactory, "readTimeout");
4545
}
4646

47+
@Override
48+
protected boolean supportsSettingConnectTimeout() {
49+
return true;
50+
}
51+
52+
@Override
53+
protected boolean supportsSettingReadTimeout() {
54+
return true;
55+
}
56+
4757
}

‎spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/client/ClientHttpRequestFactoriesTests.java‎

Lines changed: 48 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.boot.web.client;
1818

19-
import java.io.IOException;
2019
import java.net.URI;
2120
import java.time.Duration;
2221

@@ -162,14 +161,26 @@ void readTimeoutCanBeConfiguredOnAWrappedRequestFactory() {
162161
assertThat(requestFactory).hasFieldOrPropertyWithValue("readTimeout", 1234);
163162
}
164163

164+
@Test
165+
void reflectiveShouldFavorDurationTimeoutMethods() {
166+
IntAndDurationTimeoutsClientHttpRequestFactory requestFactory = ClientHttpRequestFactories.get(
167+
IntAndDurationTimeoutsClientHttpRequestFactory.class,
168+
ClientHttpRequestFactorySettings.DEFAULTS.withConnectTimeout(Duration.ofSeconds(1))
169+
.withReadTimeout(Duration.ofSeconds(2)));
170+
assertThat((requestFactory).connectTimeout).isZero();
171+
assertThat((requestFactory).readTimeout).isZero();
172+
assertThat((requestFactory).connectTimeoutDuration).isEqualTo(Duration.ofSeconds(1));
173+
assertThat((requestFactory).readTimeoutDuration).isEqualTo(Duration.ofSeconds(2));
174+
}
175+
165176
public static class TestClientHttpRequestFactory implements ClientHttpRequestFactory {
166177

167178
private int connectTimeout;
168179

169180
private int readTimeout;
170181

171182
@Override
172-
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
183+
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) {
173184
throw new UnsupportedOperationException();
174185
}
175186

@@ -186,7 +197,7 @@ public void setReadTimeout(int timeout) {
186197
public static class UnconfigurableClientHttpRequestFactory implements ClientHttpRequestFactory {
187198

188199
@Override
189-
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
200+
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) {
190201
throw new UnsupportedOperationException();
191202
}
192203

@@ -195,7 +206,7 @@ public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IO
195206
public static class DeprecatedMethodsClientHttpRequestFactory implements ClientHttpRequestFactory {
196207

197208
@Override
198-
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException {
209+
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) {
199210
throw new UnsupportedOperationException();
200211
}
201212

@@ -213,4 +224,37 @@ public void setBufferRequestBody(boolean bufferRequestBody) {
213224

214225
}
215226

227+
public static class IntAndDurationTimeoutsClientHttpRequestFactory implements ClientHttpRequestFactory {
228+
229+
private int readTimeout;
230+
231+
private int connectTimeout;
232+
233+
private Duration readTimeoutDuration;
234+
235+
private Duration connectTimeoutDuration;
236+
237+
@Override
238+
public ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) {
239+
throw new UnsupportedOperationException();
240+
}
241+
242+
public void setConnectTimeout(int timeout) {
243+
this.connectTimeout = timeout;
244+
}
245+
246+
public void setReadTimeout(int timeout) {
247+
this.readTimeout = timeout;
248+
}
249+
250+
public void setConnectTimeout(Duration timeout) {
251+
this.connectTimeoutDuration = timeout;
252+
}
253+
254+
public void setReadTimeout(Duration timeout) {
255+
this.readTimeoutDuration = timeout;
256+
}
257+
258+
}
259+
216260
}

0 commit comments

Comments
 (0)