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
This commit is contained in:
		
							parent
							
								
									58f3054624
								
							
						
					
					
						commit
						3ac034e18a
					
				|  | @ -783,12 +783,16 @@ public class RestTemplateBuilder { | ||||||
| 
 | 
 | ||||||
| 		private Method findMethod(ClientHttpRequestFactory requestFactory, String methodName, Class<?>... parameters) { | 		private Method findMethod(ClientHttpRequestFactory requestFactory, String methodName, Class<?>... parameters) { | ||||||
| 			Method method = ReflectionUtils.findMethod(requestFactory.getClass(), methodName, parameters); | 			Method method = ReflectionUtils.findMethod(requestFactory.getClass(), methodName, parameters); | ||||||
| 			if (method != null) { | 			if (method == null) { | ||||||
| 				return method; |  | ||||||
| 			} |  | ||||||
| 				throw new IllegalStateException("Request factory " + requestFactory.getClass() | 				throw new IllegalStateException("Request factory " + requestFactory.getClass() | ||||||
| 						+ " does not have a suitable " + methodName + " method"); | 						+ " 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) { | 		private void invoke(ClientHttpRequestFactory requestFactory, Method method, Object... parameters) { | ||||||
| 			ReflectionUtils.invokeMethod(method, requestFactory, parameters); | 			ReflectionUtils.invokeMethod(method, requestFactory, parameters); | ||||||
|  |  | ||||||
|  | @ -25,7 +25,6 @@ import java.util.Set; | ||||||
| import java.util.function.Supplier; | import java.util.function.Supplier; | ||||||
| 
 | 
 | ||||||
| import okhttp3.OkHttpClient; | import okhttp3.OkHttpClient; | ||||||
| import org.apache.http.client.config.RequestConfig; |  | ||||||
| import org.assertj.core.api.InstanceOfAssertFactories; | import org.assertj.core.api.InstanceOfAssertFactories; | ||||||
| import org.junit.jupiter.api.Test; | import org.junit.jupiter.api.Test; | ||||||
| import org.junit.jupiter.api.extension.ExtendWith; | 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.assertThat; | ||||||
| import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; | import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; | ||||||
| import static org.assertj.core.api.Assertions.assertThatIllegalStateException; | 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.assertj.core.api.Assertions.entry; | ||||||
| import static org.mockito.ArgumentMatchers.any; | import static org.mockito.ArgumentMatchers.any; | ||||||
| import static org.mockito.BDDMockito.then; | import static org.mockito.BDDMockito.then; | ||||||
|  | @ -81,6 +81,7 @@ import static org.springframework.test.web.client.response.MockRestResponseCreat | ||||||
|  * @author Dmytro Nosan |  * @author Dmytro Nosan | ||||||
|  * @author Kevin Strijbos |  * @author Kevin Strijbos | ||||||
|  * @author Ilya Lukyanovich |  * @author Ilya Lukyanovich | ||||||
|  |  * @author Brian Clozel | ||||||
|  */ |  */ | ||||||
| @ExtendWith(MockitoExtension.class) | @ExtendWith(MockitoExtension.class) | ||||||
| class RestTemplateBuilderTests { | class RestTemplateBuilderTests { | ||||||
|  | @ -477,17 +478,14 @@ class RestTemplateBuilderTests { | ||||||
| 		ClientHttpRequestFactory requestFactory = this.builder | 		ClientHttpRequestFactory requestFactory = this.builder | ||||||
| 				.requestFactory(HttpComponentsClientHttpRequestFactory.class).setConnectTimeout(Duration.ofMillis(1234)) | 				.requestFactory(HttpComponentsClientHttpRequestFactory.class).setConnectTimeout(Duration.ofMillis(1234)) | ||||||
| 				.build().getRequestFactory(); | 				.build().getRequestFactory(); | ||||||
| 		assertThat(((RequestConfig) ReflectionTestUtils.getField(requestFactory, "requestConfig")).getConnectTimeout()) | 		assertThat(((int) ReflectionTestUtils.getField(requestFactory, "connectTimeout"))).isEqualTo(1234); | ||||||
| 				.isEqualTo(1234); |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Test | 	@Test | ||||||
| 	void readTimeoutCanBeConfiguredOnHttpComponentsRequestFactory() { | 	void readTimeoutConfigurationFailsOnHttpComponentsRequestFactory() { | ||||||
| 		ClientHttpRequestFactory requestFactory = this.builder | 		assertThatThrownBy(() -> this.builder.requestFactory(HttpComponentsClientHttpRequestFactory.class) | ||||||
| 				.requestFactory(HttpComponentsClientHttpRequestFactory.class).setReadTimeout(Duration.ofMillis(1234)) | 				.setReadTimeout(Duration.ofMillis(1234)).build()).isInstanceOf(IllegalStateException.class) | ||||||
| 				.build().getRequestFactory(); | 						.hasMessageContaining("setReadTimeout method marked as deprecated"); | ||||||
| 		assertThat(((RequestConfig) ReflectionTestUtils.getField(requestFactory, "requestConfig")).getSocketTimeout()) |  | ||||||
| 				.isEqualTo(1234); |  | ||||||
| 	} | 	} | ||||||
| 
 | 
 | ||||||
| 	@Test | 	@Test | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue