Add configuration setting for TTL of HTTP connections to IRestfulClientFactory (hapifhir#6204)

* Add configuration setting for TTL of HTTP connections to IRestfulClientFactory

* Update changelog and pom.xml

* Modify changelog

---------

Co-authored-by: James Agnew <[email protected]>

Author: acoteathn

Diff for: hapi-fhir-base/src/main/java/ca/uhn/fhir/rest/client/api/IRestfulClientFactory.java

@@ -37,6 +37,12 @@ public interface IRestfulClientFactory {
 	 */
 	public static final int DEFAULT_CONNECTION_REQUEST_TIMEOUT = 10000;
 
+
+	/**
+	 * Default value for {@link #getConnectionTimeToLive()}
+	 */
+	public static final int DEFAULT_CONNECTION_TTL = 5000;
+
 	/**
 	 * Default value for {@link #getServerValidationModeEnum()}
 	 */
@@ -75,6 +81,17 @@ public interface IRestfulClientFactory {
 	 */
 	int getConnectTimeout();
 
+
+	/**
+	 * Gets the connection time to live, in milliseconds. This is the amount of time to keep connections alive for reuse.
+	 * <p>
+	 * The default value for this setting is defined by {@link #DEFAULT_CONNECTION_TTL}
+	 * </p>
+	 */
+	default int getConnectionTimeToLive() {
+		return DEFAULT_CONNECTION_TTL;
+	}
+
 	/**
 	 * Returns the HTTP client instance. This method will not return null.
 	 * @param theUrl
@@ -179,6 +196,14 @@ IHttpClient getHttpClient(
 	 */
 	void setConnectTimeout(int theConnectTimeout);
 
+	/**
+	 * Sets the connection time to live, in milliseconds. This is the amount of time to keep connections alive for reuse.
+	 * <p>
+	 * The default value for this setting is defined by {@link #DEFAULT_CONNECTION_TTL}
+	 * </p>
+	 */
+	default void setConnectionTimeToLive(int theConnectionTimeToLive) {}
+
 	/**
 	 * Sets the Apache HTTP client instance to be used by any new restful clients created by this factory. If set to
 	 * <code>null</code>, a new HTTP client with default settings will be created.

Diff for: hapi-fhir-cli/hapi-fhir-cli-api/src/main/java/ca/uhn/fhir/cli/client/HapiFhirCliRestfulClientFactory.java

@@ -106,9 +106,9 @@ public HttpClient getNativeHttpClient() {
 						.register("https", sslConnectionSocketFactory)
 						.build();
 				connectionManager =
-						new PoolingHttpClientConnectionManager(registry, null, null, null, 5000, TimeUnit.MILLISECONDS);
+						new PoolingHttpClientConnectionManager(registry, null, null, null, getConnectionTimeToLive(), TimeUnit.MILLISECONDS);
 			} else {
-				connectionManager = new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
+				connectionManager = new PoolingHttpClientConnectionManager(getConnectionTimeToLive(), TimeUnit.MILLISECONDS);
 			}
 
 			connectionManager.setMaxTotal(getPoolMaxTotal());

Diff for: hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/ApacheRestfulClientFactoryTest.java

@@ -3,13 +3,15 @@
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
 import ca.uhn.fhir.rest.client.apache.ApacheRestfulClientFactory;
+import ca.uhn.fhir.rest.client.api.IRestfulClientFactory;
 import ca.uhn.fhir.test.BaseFhirVersionParameterizedTest;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
 import org.apache.http.client.methods.HttpGet;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.util.EntityUtils;
 import org.hl7.fhir.instance.model.api.IBaseResource;
+import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
 import org.junit.jupiter.params.provider.MethodSource;
 
@@ -79,4 +81,13 @@ public void testGenericClientHttpsNoCredentials(FhirVersionEnum theFhirVersion)
 			assertEquals(SSLHandshakeException.class, e.getCause().getCause().getClass());
 		}
 	}
+
+	@Test
+	public void testConnectionTimeToLive() {
+		ApacheRestfulClientFactory clientFactory = new ApacheRestfulClientFactory();
+
+		assertEquals(IRestfulClientFactory.DEFAULT_CONNECTION_TTL, clientFactory.getConnectionTimeToLive());
+		clientFactory.setConnectionTimeToLive(25000);
+		assertEquals(25000, clientFactory.getConnectionTimeToLive());
+	}
 }

Diff for: hapi-fhir-cli/hapi-fhir-cli-api/src/test/java/ca/uhn/fhir/cli/client/HapiFhirCliRestfulClientFactoryTest.java

@@ -3,6 +3,7 @@
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
 import ca.uhn.fhir.i18n.Msg;
+import ca.uhn.fhir.rest.client.api.IRestfulClientFactory;
 import ca.uhn.fhir.test.BaseFhirVersionParameterizedTest;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
@@ -159,4 +160,15 @@ public void testSetProxy(FhirVersionEnum theFhirVersion){
 		}
 	}
 
+	@ParameterizedTest
+	@MethodSource("baseParamsProvider")
+	public void testConnectionTimeToLive(FhirVersionEnum theFhirVersion) {
+		FhirVersionParams fhirVersionParams = getFhirVersionParams(theFhirVersion);
+		HapiFhirCliRestfulClientFactory clientFactory = new HapiFhirCliRestfulClientFactory(fhirVersionParams.getFhirContext());
+
+		assertEquals(IRestfulClientFactory.DEFAULT_CONNECTION_TTL, clientFactory.getConnectionTimeToLive());
+		clientFactory.setConnectionTimeToLive(25000);
+		assertEquals(25000, clientFactory.getConnectionTimeToLive());
+	}
+
 }

Diff for: hapi-fhir-client-okhttp/src/main/java/ca/uhn/fhir/okhttp/client/OkHttpRestfulClientFactory.java

@@ -25,6 +25,7 @@
 import ca.uhn.fhir.rest.client.api.IHttpClient;
 import ca.uhn.fhir.rest.client.impl.RestfulClientFactory;
 import okhttp3.Call;
+import okhttp3.ConnectionPool;
 import okhttp3.OkHttpClient;
 
 import java.net.InetSocketAddress;
@@ -65,6 +66,7 @@ public synchronized Call.Factory getNativeClient() {
 			myNativeClient = new OkHttpClient()
 					.newBuilder()
 					.connectTimeout(getConnectTimeout(), TimeUnit.MILLISECONDS)
+					.connectionPool(new ConnectionPool(5, getConnectionTimeToLive(), TimeUnit.MILLISECONDS))
 					.readTimeout(getSocketTimeout(), TimeUnit.MILLISECONDS)
 					.writeTimeout(getSocketTimeout(), TimeUnit.MILLISECONDS)
 					.build();

Diff for: hapi-fhir-client-okhttp/src/test/java/ca/uhn/fhir/okhttp/OkHttpRestfulClientFactoryTest.java

@@ -3,6 +3,7 @@
 import ca.uhn.fhir.context.FhirContext;
 import ca.uhn.fhir.context.FhirVersionEnum;
 import ca.uhn.fhir.okhttp.client.OkHttpRestfulClientFactory;
+import ca.uhn.fhir.rest.client.api.IRestfulClientFactory;
 import ca.uhn.fhir.test.BaseFhirVersionParameterizedTest;
 import okhttp3.Call;
 import okhttp3.OkHttpClient;
@@ -71,6 +72,13 @@ public void testConnectTimeout() {
 		assertEquals(1516, ((OkHttpClient) clientFactory.getNativeClient()).connectTimeoutMillis());
 	}
 
+	@Test
+	public void testConnectionTimeToLive() {
+		assertEquals(IRestfulClientFactory.DEFAULT_CONNECTION_TTL, clientFactory.getConnectionTimeToLive());
+		clientFactory.setConnectionTimeToLive(25000);
+		assertEquals(25000, clientFactory.getConnectionTimeToLive());
+	}
+
 	@ParameterizedTest
 	@MethodSource("baseParamsProvider")
 	public void testNativeClientHttp(FhirVersionEnum theFhirVersion) throws Exception {

Diff for: hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactory.java

@@ -103,7 +103,7 @@ public HttpClient getNativeHttpClient() {
 					.disableCookieManagement();
 
 			PoolingHttpClientConnectionManager connectionManager =
-					new PoolingHttpClientConnectionManager(5000, TimeUnit.MILLISECONDS);
+					new PoolingHttpClientConnectionManager(getConnectionTimeToLive(), TimeUnit.MILLISECONDS);
 			connectionManager.setMaxTotal(getPoolMaxTotal());
 			connectionManager.setDefaultMaxPerRoute(getPoolMaxPerRoute());
 			builder.setConnectionManager(connectionManager);

Diff for: hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/impl/RestfulClientFactory.java

@@ -57,6 +57,7 @@ public abstract class RestfulClientFactory implements IRestfulClientFactory {
 	private final Set<String> myValidatedServerBaseUrls = Collections.synchronizedSet(new HashSet<>());
 	private int myConnectionRequestTimeout = DEFAULT_CONNECTION_REQUEST_TIMEOUT;
 	private int myConnectTimeout = DEFAULT_CONNECT_TIMEOUT;
+	private int myConnectionTimeToLive = DEFAULT_CONNECTION_TTL;
 	private FhirContext myContext;
 	private final Map<Class<? extends IRestfulClient>, ClientInvocationHandlerFactory> myInvocationHandlers =
 			new HashMap<>();
@@ -91,6 +92,11 @@ public synchronized int getConnectTimeout() {
 		return myConnectTimeout;
 	}
 
+	@Override
+	public synchronized int getConnectionTimeToLive() {
+		return myConnectionTimeToLive;
+	}
+
 	/**
 	 * Return the proxy username to authenticate with the HTTP proxy
 	 */
@@ -210,6 +216,12 @@ public synchronized void setConnectTimeout(int theConnectTimeout) {
 		resetHttpClient();
 	}
 
+	@Override
+	public synchronized void setConnectionTimeToLive(int theConnectionTimeToLive) {
+		myConnectionTimeToLive = theConnectionTimeToLive;
+		resetHttpClient();
+	}
+
 	/**
 	 * Sets the context associated with this client factory. Must not be called more than once.
 	 */

Diff for: hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_6_0/6184-add-configuration-setting-for-ttl-of-http-connections.yaml

@@ -0,0 +1,10 @@
+---
+type: add
+issue: 6184
+title: "Added a configuration setting for the TTL of HTTP connections to IRestfulClientFactory.
+        The following implementations have been updated to respect this new setting:
+        1. ApacheRestfulClientFactory
+        2. OkHttpRestfulClientFactory
+        3. HapiFhirCliRestfulClientFactory
+        Thanks to Alex Kopp and Alex Cote for the contribution!
+"

Diff for: pom.xml

@@ -919,15 +919,21 @@
 			<id>adriennesox</id>
 			<name>Adrienne Sox</name>
 			<organization>Galileo, Inc.</organization>
-    </developer>
-	 <developer>
-		 <id>melihaydogd</id>
-		 <name>Ahmet Melih Aydoğdu</name>
-    </developer>
-	 <developer>
-		 <id>alexrkopp</id>
-		 <name>Alex Kopp</name>
-    </developer>
+		</developer>
+		<developer>
+			<id>melihaydogd</id>
+			<name>Ahmet Melih Aydoğdu</name>
+		</developer>
+		<developer>
+			<id>alexrkopp</id>
+			<name>Alex Kopp</name>
+			<organization>athenahealth</organization>
+		</developer>
+		<developer>
+			<id>acoteathn</id>
+			<name>Alex Cote</name>
+			<organization>athenahealth</organization>
+		</developer>
 	</developers>
 
 	<licenses>