Skip to content
This repository has been archived by the owner on Apr 7, 2022. It is now read-only.

Fix RestartableHttpClient. #119

Merged
merged 4 commits into from
Jun 2, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions hbc-core/src/main/java/com/twitter/hbc/ClientBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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";
Expand Down Expand Up @@ -99,7 +99,7 @@ public ClientBuilder() {
socketTimeoutMillis = 60000;
connectionTimeoutMillis = 4000;

connectionManager = new PoolingClientConnectionManager();
schemeRegistry = SchemeRegistryFactory.createDefault();
}

/**
Expand Down Expand Up @@ -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;
}

Expand All @@ -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);
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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);

Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
}
}