From 5d43f4c35ed22352870edc87dc2163ec04c96d09 Mon Sep 17 00:00:00 2001 From: Vladimir Nemergut Date: Mon, 14 Dec 2020 12:01:17 +0100 Subject: [PATCH] Use system properties in ApacheRestfulClientFactory (#2241) * Use system properties in ApacheRestfulClientFactory #2192 * Add changelog entry for #2192 --- .../rest/client/apache/ApacheHttpClient.java | 23 ++--- .../apache/ApacheRestfulClientFactory.java | 11 ++- .../5_3_0/2192-apache-client-proxy.yaml | 4 + hapi-fhir-structures-dstu2/pom.xml | 6 ++ .../ApacheRestfulClientFactoryTest.java | 98 ++++++++++++++++++- 5 files changed, 122 insertions(+), 20 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2192-apache-client-proxy.yaml diff --git a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheHttpClient.java b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheHttpClient.java index c44788918be..8fc3782ab0e 100644 --- a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheHttpClient.java +++ b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheHttpClient.java @@ -45,7 +45,7 @@ import ca.uhn.fhir.rest.server.exceptions.InternalErrorException; */ public class ApacheHttpClient extends BaseHttpClient implements IHttpClient { - private HttpClient myClient; + private final HttpClient myClient; public ApacheHttpClient(HttpClient theClient, StringBuilder theUrl, Map> theIfNoneExistParams, String theIfNoneExistString, RequestTypeEnum theRequestType, List
theHeaders) { super(theUrl, theIfNoneExistParams, theIfNoneExistString, theRequestType, theHeaders); @@ -89,8 +89,7 @@ public class ApacheHttpClient extends BaseHttpClient implements IHttpClient { @Override protected IHttpRequest createHttpRequest() { - IHttpRequest retVal = createHttpRequest((HttpEntity)null); - return retVal; + return createHttpRequest((HttpEntity)null); } @Override @@ -101,19 +100,17 @@ public class ApacheHttpClient extends BaseHttpClient implements IHttpClient { * the newer ones for whatever reason. */ ByteArrayEntity entity = new ByteArrayEntity(content); - IHttpRequest retVal = createHttpRequest(entity); - return retVal; + return createHttpRequest(entity); } private ApacheHttpRequest createHttpRequest(HttpEntity theEntity) { HttpRequestBase request = constructRequestBase(theEntity); - ApacheHttpRequest result = new ApacheHttpRequest(myClient, request); - return result; + return new ApacheHttpRequest(myClient, request); } @Override protected IHttpRequest createHttpRequest(Map> theParams) { - List parameters = new ArrayList(); + List parameters = new ArrayList<>(); for (Entry> nextParam : theParams.entrySet()) { List value = nextParam.getValue(); for (String s : value) { @@ -122,8 +119,7 @@ public class ApacheHttpClient extends BaseHttpClient implements IHttpClient { } UrlEncodedFormEntity entity = createFormEntity(parameters); - IHttpRequest retVal = createHttpRequest(entity); - return retVal; + return createHttpRequest(entity); } @@ -136,11 +132,6 @@ public class ApacheHttpClient extends BaseHttpClient implements IHttpClient { * which one we use anyhow. */ ByteArrayEntity entity = new ByteArrayEntity(theContents.getBytes(Constants.CHARSET_UTF8)); - IHttpRequest retVal = createHttpRequest(entity); - return retVal; + return createHttpRequest(entity); } - - - - } diff --git a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactory.java b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactory.java index 5e5030c8a71..dadd5500a44 100644 --- a/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactory.java +++ b/hapi-fhir-client/src/main/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactory.java @@ -100,8 +100,11 @@ public class ApacheRestfulClientFactory extends RestfulClientFactory { .setProxy(myProxy) .build(); - HttpClientBuilder builder = HttpClients.custom().setConnectionManager(connectionManager) - .setDefaultRequestConfig(defaultRequestConfig).disableCookieManagement(); + HttpClientBuilder builder = getHttpClientBuilder() + .useSystemProperties() + .setConnectionManager(connectionManager) + .setDefaultRequestConfig(defaultRequestConfig) + .disableCookieManagement(); if (myProxy != null && StringUtils.isNotBlank(getProxyUsername()) && StringUtils.isNotBlank(getProxyPassword())) { CredentialsProvider credsProvider = new BasicCredentialsProvider(); @@ -118,6 +121,10 @@ public class ApacheRestfulClientFactory extends RestfulClientFactory { return myHttpClient; } + protected HttpClientBuilder getHttpClientBuilder() { + return HttpClients.custom(); + } + @Override protected void resetHttpClient() { this.myHttpClient = null; diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2192-apache-client-proxy.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2192-apache-client-proxy.yaml new file mode 100644 index 00000000000..d591b23d6cc --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/5_3_0/2192-apache-client-proxy.yaml @@ -0,0 +1,4 @@ +--- +type: fix +issue: 2192 +title: "ApacheRestfulClientFactory now uses system properties for proxy configuration" diff --git a/hapi-fhir-structures-dstu2/pom.xml b/hapi-fhir-structures-dstu2/pom.xml index 4cae999a780..6cf912aca5b 100644 --- a/hapi-fhir-structures-dstu2/pom.xml +++ b/hapi-fhir-structures-dstu2/pom.xml @@ -143,6 +143,12 @@ + + org.junit-pioneer + junit-pioneer + 1.1.0 + test + net.sf.json-lib json-lib diff --git a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactoryTest.java b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactoryTest.java index 8502e7bc563..c1f021be86f 100644 --- a/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactoryTest.java +++ b/hapi-fhir-structures-dstu2/src/test/java/ca/uhn/fhir/rest/client/apache/ApacheRestfulClientFactoryTest.java @@ -1,9 +1,29 @@ package ca.uhn.fhir.rest.client.apache; -import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.fail; +import java.io.IOException; +import java.util.function.Consumer; +import static org.junit.jupiter.api.Assertions.*; +import static org.mockito.Mockito.*; + +import org.apache.http.ConnectionReuseStrategy; +import org.apache.http.HttpException; +import org.apache.http.HttpHost; +import org.apache.http.client.AuthenticationStrategy; +import org.apache.http.client.UserTokenHandler; +import org.apache.http.client.methods.*; +import org.apache.http.client.protocol.HttpClientContext; +import org.apache.http.conn.ConnectionKeepAliveStrategy; +import org.apache.http.conn.HttpClientConnectionManager; +import org.apache.http.conn.routing.HttpRoute; +import org.apache.http.impl.client.HttpClientBuilder; +import org.apache.http.impl.execchain.ClientExecChain; +import org.apache.http.protocol.HttpProcessor; +import org.apache.http.protocol.HttpRequestExecutor; import org.junit.jupiter.api.Test; +import org.junitpioneer.jupiter.SetSystemProperty; +import org.mockito.ArgumentCaptor; +import org.mockito.internal.stubbing.defaultanswers.ReturnsDeepStubs; import ca.uhn.fhir.context.FhirContext; import ca.uhn.fhir.rest.client.exceptions.FhirClientConnectionException; @@ -37,4 +57,78 @@ public class ApacheRestfulClientFactoryTest { assertEquals("Failed to retrieve the server metadata statement during client initialization. URL used was http://127.0.0.1:22225metadata", e.getMessage()); } } + + @Test + public void testGetNativeHttpClientWithProvidedProxy() throws IOException, HttpException { + HttpRoute httpRoute = executeRequestAndCaptureHttpRoute( + f -> f.setProxy("theHost", 0)); + + HttpHost proxyHost = httpRoute.getProxyHost(); + assertNotNull(proxyHost.getHostName()); + assertEquals("theHost", proxyHost.getHostName()); + assertEquals(0, proxyHost.getPort()); + } + + @Test + @SetSystemProperty(key = "http.proxyHost", value = "hostFromSystemProperty") + @SetSystemProperty(key = "http.proxyPort", value = "1234") + public void testGetNativeHttpClientWithSystemProxy() throws IOException, HttpException { + HttpRoute httpRoute = executeRequestAndCaptureHttpRoute(f -> { + }); + + HttpHost proxyHost = httpRoute.getProxyHost(); + assertNotNull(proxyHost.getHostName()); + assertEquals("hostFromSystemProperty", proxyHost.getHostName()); + assertEquals(1234, proxyHost.getPort()); + } + + @Test + @SetSystemProperty(key = "http.proxyHost", value = "hostFromSystemProperty") + @SetSystemProperty(key = "http.proxyPort", value = "1234") + public void testGetNativeHttpClientWithSystemAndProvidedProxy() throws IOException, HttpException { + HttpRoute httpRoute = executeRequestAndCaptureHttpRoute( + f -> f.setProxy("providedProxy", 0)); + + HttpHost proxyHost = httpRoute.getProxyHost(); + assertNotNull(proxyHost.getHostName()); + assertEquals("providedProxy", proxyHost.getHostName()); + assertEquals(0, proxyHost.getPort()); + } + + private HttpRoute executeRequestAndCaptureHttpRoute(Consumer factoryConsumer) + throws IOException, HttpException { + ClientExecChain mainExec = mock(ClientExecChain.class); + ArgumentCaptor httpRouteCaptor = ArgumentCaptor.forClass(HttpRoute.class); + when(mainExec.execute( + any(HttpRoute.class), + any(HttpRequestWrapper.class), + any(HttpClientContext.class), + any(HttpExecutionAware.class))) + .thenReturn(mock(CloseableHttpResponse.class, new ReturnsDeepStubs())); + + ApacheRestfulClientFactory testableSut = createTestableSut(mainExec); + factoryConsumer.accept(testableSut); + testableSut.getNativeHttpClient().execute(new HttpGet("http://somewhere.net")); + + verify(mainExec).execute( + httpRouteCaptor.capture(), + any(HttpRequestWrapper.class), + any(HttpClientContext.class), + any(HttpExecutionAware.class)); + return httpRouteCaptor.getValue(); + } + + private ApacheRestfulClientFactory createTestableSut(ClientExecChain mainExec) { + return new ApacheRestfulClientFactory() { + @Override + protected HttpClientBuilder getHttpClientBuilder() { + return new HttpClientBuilder() { + @Override + protected ClientExecChain createMainExec(HttpRequestExecutor requestExec, HttpClientConnectionManager connManager, ConnectionReuseStrategy reuseStrategy, ConnectionKeepAliveStrategy keepAliveStrategy, HttpProcessor proxyHttpProcessor, AuthenticationStrategy targetAuthStrategy, AuthenticationStrategy proxyAuthStrategy, UserTokenHandler userTokenHandler) { + return mainExec; + } + }; + } + }; + } }