Revisit resource cleanup in ClientHttpRequestFactory

Prior to this commit, resource management around
`ClientHttpRequestFactory` and `RestTemplate` was unclear. Some
factories implementation were implementing a `DisposableBean` and other
contracts were not managing request factory resources.

In the meantime, neither `ClientHttpRequestFactory` nor `RestTemplate`
are typically meant to be contributed as beans to the application
context. Most often, they're instantiated within beans and their
lifecycle should be managed by those.

This commit makes all `ClientHttpRequestFactory` `Closeable` and ensures
that all existing implementations have a similar behavior between
`dispose()` and `close()`. Since `RestTemplate` (actually
`HttpAccessor`) can instantiate factories on its own, they also now
extend `Closeable` to properly close those resources, if not externally
managed.

Closes gh-29010

spring-web/src/main/java/org/springframework/http/client/ClientHttpRequestFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package org.springframework.http.client;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.net.URI;
 
@@ -29,7 +30,7 @@ import org.springframework.http.HttpMethod;
  * @since 3.0
  */
 @FunctionalInterface
-public interface ClientHttpRequestFactory {
+public interface ClientHttpRequestFactory extends Closeable {
 
 	/**
 	 * Create a new {@link ClientHttpRequest} for the specified URI and HTTP method.
@@ -42,4 +43,8 @@ public interface ClientHttpRequestFactory {
 	 */
 	ClientHttpRequest createRequest(URI uri, HttpMethod httpMethod) throws IOException;
 
+	@Override
+	default void close() throws IOException {
+
+	}
 }

spring-web/src/main/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -329,13 +329,17 @@ public class HttpComponentsClientHttpRequestFactory implements ClientHttpRequest
 	 */
 	@Override
 	public void destroy() throws Exception {
+		close();
+	}
+
+	@Override
+	public void close() throws IOException {
 		HttpClient httpClient = getHttpClient();
 		if (httpClient instanceof Closeable) {
 			((Closeable) httpClient).close();
 		}
 	}
 
-
 	/**
 	 * An alternative to {@link org.apache.http.client.methods.HttpDelete} that
 	 * extends {@link org.apache.http.client.methods.HttpEntityEnclosingRequestBase}

spring-web/src/main/java/org/springframework/http/client/OkHttp3ClientHttpRequestFactory.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -107,6 +107,11 @@ public class OkHttp3ClientHttpRequestFactory implements ClientHttpRequestFactory
 
 	@Override
 	public void destroy() throws IOException {
+		close();
+	}
+
+	@Override
+	public void close() throws IOException {
 		if (this.defaultClient) {
 			// Clean up the client if we created it in the constructor
 			Cache cache = this.client.cache();
@@ -118,7 +123,6 @@ public class OkHttp3ClientHttpRequestFactory implements ClientHttpRequestFactory
 		}
 	}
 
-
 	static Request buildRequest(HttpHeaders headers, byte[] content, URI uri, HttpMethod method)
 			throws MalformedURLException {
 

spring-web/src/main/java/org/springframework/http/client/support/HttpAccessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,6 +16,7 @@
 
 package org.springframework.http.client.support;
 
+import java.io.Closeable;
 import java.io.IOException;
 import java.net.URI;
 import java.util.ArrayList;
@@ -48,7 +49,7 @@ import org.springframework.util.Assert;
  * @see ClientHttpRequestFactory
  * @see org.springframework.web.client.RestTemplate
  */
-public abstract class HttpAccessor {
+public abstract class HttpAccessor implements Closeable {
 
 	/** Logger available to subclasses. */
 	protected final Log logger = HttpLogging.forLogName(getClass());
@@ -66,7 +67,7 @@ public abstract class HttpAccessor {
 	 * Configure the Apache HttpComponents or OkHttp request factory to enable PATCH.</b>
 	 * @see #createRequest(URI, HttpMethod)
 	 * @see SimpleClientHttpRequestFactory
-	 * @see org.springframework.http.client.HttpComponentsAsyncClientHttpRequestFactory
+	 * @see org.springframework.http.client.HttpComponentsClientHttpRequestFactory
 	 * @see org.springframework.http.client.OkHttp3ClientHttpRequestFactory
 	 */
 	public void setRequestFactory(ClientHttpRequestFactory requestFactory) {
@@ -133,4 +134,15 @@ public abstract class HttpAccessor {
 		this.clientHttpRequestInitializers.forEach(initializer -> initializer.initialize(request));
 	}
 
+	/**
+	 * Close the underlying {@link ClientHttpRequestFactory}.
+	 * <p>This should not be called if the factory has been {@link #setRequestFactory(ClientHttpRequestFactory) set}
+	 * and is externally managed.
+	 * @throws IOException if the request factory cannot be closed properly
+	 */
+	@Override
+	public void close() throws IOException {
+		this.requestFactory.close();
+	}
+
 }

spring-web/src/test/java/org/springframework/http/client/AbstractHttpRequestFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -25,7 +25,6 @@ import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
-import org.springframework.beans.factory.DisposableBean;
 import org.springframework.beans.factory.InitializingBean;
 import org.springframework.http.HttpMethod;
 import org.springframework.http.HttpStatus;
@@ -47,17 +46,15 @@ abstract class AbstractHttpRequestFactoryTests extends AbstractMockWebServerTest
 
 	@BeforeEach
 	final void createFactory() throws Exception {
-		factory = createRequestFactory();
-		if (factory instanceof InitializingBean) {
-			((InitializingBean) factory).afterPropertiesSet();
+		this.factory = createRequestFactory();
+		if (this.factory instanceof InitializingBean) {
+			((InitializingBean) this.factory).afterPropertiesSet();
 		}
 	}
 
 	@AfterEach
 	final void destroyFactory() throws Exception {
-		if (factory instanceof DisposableBean) {
-			((DisposableBean) factory).destroy();
-		}
+		this.factory.close();
 	}
 
 

spring-web/src/test/java/org/springframework/http/client/HttpComponentsClientHttpRequestFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2021 the original author or authors.
+ * Copyright 2002-2022 the original author or authors.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -55,22 +55,24 @@ public class HttpComponentsClientHttpRequestFactoryTests extends AbstractHttpReq
 	@Test
 	public void assertCustomConfig() throws Exception {
 		HttpClient httpClient = HttpClientBuilder.create().build();
-		HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient);
-		hrf.setConnectTimeout(1234);
-		hrf.setConnectionRequestTimeout(4321);
-		hrf.setReadTimeout(4567);
-
-		URI uri = new URI(baseUrl + "/status/ok");
-		HttpComponentsClientHttpRequest request = (HttpComponentsClientHttpRequest)
-				hrf.createRequest(uri, HttpMethod.GET);
-
-		Object config = request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG);
-		assertThat(config).as("Request config should be set").isNotNull();
-		assertThat(config).as("Wrong request config type " + config.getClass().getName()).isInstanceOf(RequestConfig.class);
-		RequestConfig requestConfig = (RequestConfig) config;
-		assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(1234);
-		assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(4321);
-		assertThat(requestConfig.getSocketTimeout()).as("Wrong custom socket timeout").isEqualTo(4567);
+		HttpComponentsClientHttpRequest request;
+		try (HttpComponentsClientHttpRequestFactory hrf = new HttpComponentsClientHttpRequestFactory(httpClient)) {
+			hrf.setConnectTimeout(1234);
+			hrf.setConnectionRequestTimeout(4321);
+			hrf.setReadTimeout(4567);
+
+			URI uri = new URI(baseUrl + "/status/ok");
+			request = (HttpComponentsClientHttpRequest)
+					hrf.createRequest(uri, HttpMethod.GET);
+
+			Object config = request.getHttpContext().getAttribute(HttpClientContext.REQUEST_CONFIG);
+			assertThat(config).as("Request config should be set").isNotNull();
+			assertThat(config).as("Wrong request config type " + config.getClass().getName()).isInstanceOf(RequestConfig.class);
+			RequestConfig requestConfig = (RequestConfig) config;
+			assertThat(requestConfig.getConnectTimeout()).as("Wrong custom connection timeout").isEqualTo(1234);
+			assertThat(requestConfig.getConnectionRequestTimeout()).as("Wrong custom connection request timeout").isEqualTo(4321);
+			assertThat(requestConfig.getSocketTimeout()).as("Wrong custom socket timeout").isEqualTo(4567);
+		}
 	}
 
 	@Test