Merge pull request #1268 from stormpath/Issue-1265

Issue 1265

Author: Jose Luis Barrueta

extensions/httpclient/src/main/java/com/stormpath/sdk/impl/http/httpclient/HttpClientRequestExecutor.java

@@ -16,8 +16,8 @@
 package com.stormpath.sdk.impl.http.httpclient;
 
 import com.stormpath.sdk.client.AuthenticationScheme;
-import com.stormpath.sdk.impl.authc.credentials.ClientCredentials;
 import com.stormpath.sdk.client.Proxy;
+import com.stormpath.sdk.impl.authc.credentials.ClientCredentials;
 import com.stormpath.sdk.impl.http.HttpHeaders;
 import com.stormpath.sdk.impl.http.MediaType;
 import com.stormpath.sdk.impl.http.QueryString;
@@ -32,25 +32,28 @@
 import com.stormpath.sdk.impl.http.support.DefaultRequest;
 import com.stormpath.sdk.impl.http.support.DefaultResponse;
 import com.stormpath.sdk.lang.Assert;
+import org.apache.http.Consts;
 import org.apache.http.Header;
 import org.apache.http.HeaderElement;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpEntityEnclosingRequest;
 import org.apache.http.HttpHost;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
-import org.apache.http.HttpVersion;
 import org.apache.http.NoHttpResponseException;
 import org.apache.http.auth.AuthScope;
+import org.apache.http.auth.Credentials;
 import org.apache.http.auth.UsernamePasswordCredentials;
+import org.apache.http.client.CredentialsProvider;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.entity.GzipDecompressingEntity;
 import org.apache.http.client.methods.HttpRequestBase;
-import org.apache.http.client.params.AllClientPNames;
-import org.apache.http.client.params.ClientPNames;
+import org.apache.http.config.ConnectionConfig;
 import org.apache.http.conn.ConnectTimeoutException;
-import org.apache.http.conn.params.ConnRoutePNames;
-import org.apache.http.impl.client.DefaultHttpClient;
-import org.apache.http.impl.conn.PoolingClientConnectionManager;
+import org.apache.http.impl.client.BasicCredentialsProvider;
+import org.apache.http.impl.client.HttpClientBuilder;
+import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
 import org.apache.http.util.EntityUtils;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -93,7 +96,7 @@ public class HttpClientRequestExecutor implements RequestExecutor {
 
     private final RequestAuthenticator requestAuthenticator;
 
-    private DefaultHttpClient httpClient;
+    private HttpClient httpClient;
 
     private BackoffStrategy backoffStrategy;
 
@@ -150,9 +153,8 @@ public HttpClientRequestExecutor(ClientCredentials clientCredentials, Proxy prox
 
         this.requestAuthenticator = factory.create(authenticationScheme, clientCredentials);
 
-        this.httpClientRequestFactory = new HttpClientRequestFactory();
+        PoolingHttpClientConnectionManager connMgr = new PoolingHttpClientConnectionManager();
 
-        PoolingClientConnectionManager connMgr = new PoolingClientConnectionManager();
         if (MAX_CONNECTIONS_TOTAL >= MAX_CONNECTIONS_PER_ROUTE) {
             connMgr.setDefaultMaxPerRoute(MAX_CONNECTIONS_PER_ROUTE);
             connMgr.setMaxTotal(MAX_CONNECTIONS_TOTAL);
@@ -170,28 +172,34 @@ public HttpClientRequestExecutor(ClientCredentials clientCredentials, Proxy prox
         }
 
         // The connectionTimeout value is specified in seconds in Stormpath configuration settings.
-        // Therefore, multiply it by 1000 to be milliseconds since DefaultHttpClient expects milliseconds.
+        // Therefore, multiply it by 1000 to be milliseconds since RequestConfig expects milliseconds.
         int connectionTimeoutAsMilliseconds = connectionTimeout * 1000;
 
-        this.httpClient = new DefaultHttpClient(connMgr);
-        httpClient.getParams().setParameter(AllClientPNames.PROTOCOL_VERSION, HttpVersion.HTTP_1_1);
-        httpClient.getParams().setParameter(AllClientPNames.SO_TIMEOUT, connectionTimeoutAsMilliseconds);
-        httpClient.getParams().setParameter(AllClientPNames.CONNECTION_TIMEOUT, connectionTimeoutAsMilliseconds);
-        httpClient.getParams().setParameter(ClientPNames.HANDLE_REDIRECTS, false);
-        httpClient.getParams().setParameter("http.protocol.content-charset", "UTF-8");
+        RequestConfig requestConfig = RequestConfig.custom().setConnectTimeout(connectionTimeoutAsMilliseconds)
+                .setSocketTimeout(connectionTimeoutAsMilliseconds).setRedirectsEnabled(false).build();
+
+        ConnectionConfig connectionConfig = ConnectionConfig.custom().setCharset(Consts.UTF_8).build();
+
+        HttpClientBuilder httpClientBuilder = HttpClientBuilder.create().setDefaultRequestConfig(requestConfig)
+                .disableCookieManagement().setDefaultConnectionConfig(connectionConfig).setConnectionManager(connMgr);
+
+        this.httpClientRequestFactory = new HttpClientRequestFactory(requestConfig);
 
         if (proxy != null) {
             //We have some proxy setting to use!
             HttpHost httpProxyHost = new HttpHost(proxy.getHost(), proxy.getPort());
-            httpClient.getParams().setParameter(ConnRoutePNames.DEFAULT_PROXY, httpProxyHost);
+            httpClientBuilder.setProxy(httpProxyHost);
 
             if (proxy.isAuthenticationRequired()) {
-                httpClient.getCredentialsProvider().setCredentials(
-                        new AuthScope(proxy.getHost(), proxy.getPort()),
-                        new UsernamePasswordCredentials(proxy.getUsername(), proxy.getPassword()));
+                AuthScope authScope = new AuthScope(proxy.getHost(), proxy.getPort());
+                Credentials credentials = new UsernamePasswordCredentials(proxy.getUsername(), proxy.getPassword());
+                CredentialsProvider credentialsProviderProvider = new BasicCredentialsProvider();
+                credentialsProviderProvider.setCredentials(authScope, credentials);
+                httpClientBuilder.setDefaultCredentialsProvider(credentialsProviderProvider);
             }
-
         }
+
+        this.httpClient = httpClientBuilder.build();
     }
 
     public int getNumRetries() {
@@ -213,7 +221,7 @@ public void setBackoffStrategy(BackoffStrategy backoffStrategy) {
         this.backoffStrategy = backoffStrategy;
     }
 
-    public void setHttpClient(DefaultHttpClient httpClient) {
+    public void setHttpClient(HttpClient httpClient) {
         this.httpClient = httpClient;
     }
 

extensions/httpclient/src/main/java/com/stormpath/sdk/impl/http/httpclient/HttpClientRequestFactory.java

@@ -20,8 +20,11 @@
 import com.stormpath.sdk.impl.http.Request;
 import com.stormpath.sdk.impl.http.RestException;
 import com.stormpath.sdk.impl.util.RequestUtils;
+import com.stormpath.sdk.lang.Assert;
 import com.stormpath.sdk.lang.Strings;
 import org.apache.http.HttpEntity;
+import org.apache.http.HttpVersion;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.HttpDelete;
 import org.apache.http.client.methods.HttpEntityEnclosingRequestBase;
 import org.apache.http.client.methods.HttpGet;
@@ -30,7 +33,6 @@
 import org.apache.http.client.methods.HttpPut;
 import org.apache.http.client.methods.HttpRequestBase;
 import org.apache.http.entity.BufferedHttpEntity;
-import org.apache.http.params.CoreProtocolPNames;
 
 import java.io.IOException;
 import java.io.InputStream;
@@ -45,6 +47,13 @@
  */
 class HttpClientRequestFactory {
 
+    private final RequestConfig defaultRequestConfig;
+
+    HttpClientRequestFactory(RequestConfig defaultRequestConfig) {
+        Assert.notNull(defaultRequestConfig, "defaultRequestConfig");
+        this.defaultRequestConfig = defaultRequestConfig;
+    }
+
     /**
      * Creates an HttpClient method object based on the specified request and
      * populates any parameters, headers, etc. from the original request.
@@ -84,7 +93,7 @@ HttpRequestBase createHttpClientRequest(Request request, HttpEntity previousEnti
                 // large amounts of data and want to find out as early as possible if an operation will fail. We
                 // don't want to do this for all operations since it will cause extra latency in the network
                 // interaction.
-                base.getParams().setParameter(CoreProtocolPNames.USE_EXPECT_CONTINUE, true);
+                base.setConfig(RequestConfig.copy(defaultRequestConfig).setExpectContinueEnabled(true).build());
 
                 if (previousEntity != null) {
                     ((HttpEntityEnclosingRequestBase)base).setEntity(previousEntity);
@@ -100,6 +109,8 @@ HttpRequestBase createHttpClientRequest(Request request, HttpEntity previousEnti
                 throw new IllegalArgumentException("Unrecognized HttpMethod: " + method);
         }
 
+        base.setProtocolVersion(HttpVersion.HTTP_1_1);
+
         applyHeaders(base, request);
 
         return base;

extensions/httpclient/src/test/groovy/com/stormpath/sdk/client/ClientsTest.groovy

@@ -59,8 +59,9 @@ public class ClientsTest {
         //match what is set in stormpath.properties since it can't be created/deleted while testing
         assertEquals client.dataStore.cacheManager.defaultTimeToLive, new Duration(300, TimeUnit.SECONDS)
         assertEquals client.dataStore.cacheManager.defaultTimeToIdle, new Duration(300, TimeUnit.SECONDS)
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.SO_TIMEOUT), 30 * 1000)
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.CONNECTION_TIMEOUT), 30 * 1000)
+
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.socketTimeout, 30 * 1000)
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.connectTimeout, 30 * 1000)
     }
 
     @Test
@@ -195,8 +196,9 @@ public class ClientsTest {
         def builder = Clients.builder().setConnectionTimeout(990)
         assertEquals(builder.clientConfig.connectionTimeout, 990)
         def client = builder.build()
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.SO_TIMEOUT), 990000)
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.CONNECTION_TIMEOUT), 990000)
+
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.socketTimeout, 990000)
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.connectTimeout, 990000)
     }
 
     /* @since 1.0.RC3 */

extensions/httpclient/src/test/groovy/com/stormpath/sdk/impl/client/DefaultClientTest.groovy

@@ -81,8 +81,8 @@ class DefaultClientTest {
         Client client = new DefaultClient(apiKeyCredentials, apiKeyResolver, baseUrlResolver, proxy, cacheManager, authcScheme, null, connectionTimeout)
 
         assertEquals(client.dataStore.requestExecutor.requestAuthenticator.apiKeyCredentials, apiKeyCredentials)
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.SO_TIMEOUT), connectionTimeout * 1000)
-        assertEquals(client.dataStore.requestExecutor.httpClient.getParams().getParameter(AllClientPNames.CONNECTION_TIMEOUT), connectionTimeout * 1000)
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.socketTimeout, connectionTimeout * 1000)
+        assertEquals(client.dataStore.requestExecutor.httpClient.defaultConfig.connectTimeout, connectionTimeout * 1000)
 
         verify(apiKeyCredentials, proxy, cacheManager)
     }