Merge pull request #119 from paradigmsort/fix_restartable

Fix RestartableHttpClient.

hbc-core/src/main/java/com/twitter/hbc/ClientBuilder.java

@@ -24,8 +24,8 @@
 import com.twitter.hbc.httpclient.BasicClient;
 import com.twitter.hbc.httpclient.auth.Authentication;
 import org.apache.http.HttpVersion;
-import org.apache.http.conn.ClientConnectionManager;
-import org.apache.http.impl.conn.PoolingClientConnectionManager;
+import org.apache.http.conn.scheme.SchemeRegistry;
+import org.apache.http.impl.conn.SchemeRegistryFactory;
 import org.apache.http.params.BasicHttpParams;
 import org.apache.http.params.HttpConnectionParams;
 import org.apache.http.params.HttpParams;
@@ -55,7 +55,7 @@ public class ClientBuilder {
   protected ReconnectionManager reconnectionManager;
   protected int socketTimeoutMillis;
   protected int connectionTimeoutMillis;
-  protected ClientConnectionManager connectionManager;
+  protected SchemeRegistry schemeRegistry;
 
   private static String loadVersion() {
     String userAgent = "Hosebird-Client";
@@ -99,7 +99,7 @@ public ClientBuilder() {
     socketTimeoutMillis = 60000;
     connectionTimeoutMillis = 4000;
 
-    connectionManager = new PoolingClientConnectionManager();
+    schemeRegistry = SchemeRegistryFactory.createDefault();
   }
 
   /**
@@ -188,8 +188,8 @@ public ClientBuilder rateTracker(RateTracker rateTracker) {
       return this;
   }
 
-  public ClientBuilder connectionManager(ClientConnectionManager connectionManager) {
-      this.connectionManager = Preconditions.checkNotNull(connectionManager);
+  public ClientBuilder schemeRegistry(SchemeRegistry schemeRegistry) {
+      this.schemeRegistry = Preconditions.checkNotNull(schemeRegistry);
       return this;
   }
 
@@ -200,7 +200,7 @@ public BasicClient build() {
     HttpConnectionParams.setSoTimeout(params, socketTimeoutMillis);
     HttpConnectionParams.setConnectionTimeout(params, connectionTimeoutMillis);
     return new BasicClient(name, hosts, endpoint, auth, enableGZip, processor, reconnectionManager,
-            rateTracker, executorService, eventQueue, params, connectionManager);
+            rateTracker, executorService, eventQueue, params, schemeRegistry);
   }
 }
 

hbc-core/src/main/java/com/twitter/hbc/httpclient/BasicClient.java

@@ -25,8 +25,9 @@
 import com.twitter.hbc.core.processor.HosebirdMessageProcessor;
 import com.twitter.hbc.httpclient.auth.Authentication;
 import org.apache.http.client.HttpClient;
-import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.scheme.SchemeRegistry;
 import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.conn.PoolingClientConnectionManager;
 import org.apache.http.params.HttpParams;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -53,13 +54,13 @@ public class BasicClient implements Client {
 
   public BasicClient(String name, Hosts hosts, StreamingEndpoint endpoint, Authentication auth, boolean enableGZip, HosebirdMessageProcessor processor,
                      ReconnectionManager reconnectionManager, RateTracker rateTracker, ExecutorService executorService,
-                     @Nullable BlockingQueue<Event> eventsQueue, HttpParams params, ClientConnectionManager connectionManager) {
+                     @Nullable BlockingQueue<Event> eventsQueue, HttpParams params, SchemeRegistry schemeRegistry) {
     Preconditions.checkNotNull(auth);
     HttpClient client;
     if (enableGZip) {
-      client = new RestartableHttpClient(auth, enableGZip, params, connectionManager);
+      client = new RestartableHttpClient(auth, enableGZip, params, schemeRegistry);
     } else {
-      DefaultHttpClient defaultClient = new DefaultHttpClient(connectionManager, params);
+      DefaultHttpClient defaultClient = new DefaultHttpClient(new PoolingClientConnectionManager(schemeRegistry), params);
 
       /** Set auth **/
       auth.setupConnection(defaultClient);

hbc-core/src/main/java/com/twitter/hbc/httpclient/RestartableHttpClient.java

@@ -21,8 +21,10 @@
 import org.apache.http.client.ResponseHandler;
 import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.conn.ClientConnectionManager;
+import org.apache.http.conn.scheme.SchemeRegistry;
 import org.apache.http.impl.client.DecompressingHttpClient;
 import org.apache.http.impl.client.DefaultHttpClient;
+import org.apache.http.impl.conn.PoolingClientConnectionManager;
 import org.apache.http.params.HttpParams;
 import org.apache.http.protocol.HttpContext;
 
@@ -39,19 +41,19 @@ public class RestartableHttpClient implements HttpClient {
   private final Authentication auth;
   private final HttpParams params;
   private final boolean enableGZip;
-  private final ClientConnectionManager connectionManager;
+  private final SchemeRegistry schemeRegistry;
 
-  public RestartableHttpClient(Authentication auth, boolean enableGZip, HttpParams params, ClientConnectionManager connectionManager) {
+  public RestartableHttpClient(Authentication auth, boolean enableGZip, HttpParams params, SchemeRegistry schemeRegistry) {
     this.auth = Preconditions.checkNotNull(auth);
     this.enableGZip = enableGZip;
     this.params = Preconditions.checkNotNull(params);
-    this.connectionManager = Preconditions.checkNotNull(connectionManager);
+    this.schemeRegistry = Preconditions.checkNotNull(schemeRegistry);
 
     this.underlying = new AtomicReference<HttpClient>();
   }
 
   public void setup() {
-    DefaultHttpClient defaultClient = new DefaultHttpClient(connectionManager, params);
+    DefaultHttpClient defaultClient = new DefaultHttpClient(new PoolingClientConnectionManager(schemeRegistry), params);
 
     auth.setupConnection(defaultClient);
 

hbc-core/src/test/java/com/twitter/hbc/httpclient/RestartableHttpClientTest.java

@@ -0,0 +1,41 @@
+package com.twitter.hbc.httpclient;
+
+import com.twitter.hbc.httpclient.auth.Authentication;
+import java.net.UnknownHostException;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.conn.scheme.SchemeRegistry;
+import org.apache.http.impl.conn.SchemeRegistryFactory;
+import org.apache.http.params.HttpParams;
+import org.junit.Before;
+import org.junit.Test;
+import static org.junit.Assert.fail;
+import static org.mockito.Mockito.mock;
+
+public class RestartableHttpClientTest {
+  private Authentication mockAuth;
+  private SchemeRegistry defaultSchemeRegistry;
+  private HttpParams mockParams;
+  private HttpUriRequest request;
+
+  @Before
+  public void setup() throws Exception {
+    mockAuth = mock(Authentication.class);
+    mockParams = mock(HttpParams.class);
+    defaultSchemeRegistry = SchemeRegistryFactory.createDefault();
+    request = new HttpGet("http://hi");
+  }
+
+  @Test
+  public void testRestart() throws Exception {
+    RestartableHttpClient client = new RestartableHttpClient(mockAuth, true, mockParams, defaultSchemeRegistry);
+    client.setup();
+    client.restart();
+    try {
+      client.execute(request); // used to crash, https://github.com/twitter/hbc/issues/113
+      fail("should not reach here");
+    } catch (UnknownHostException e) {
+      // expected
+    }
+  }
+}