HTTP Get with proxy

Question

The code is implemented to perform HTTP get with HTTP proxy.

Future object is adopted to avoid blocking in current thread.

I'm not quite sure about the usage of HTTP connection pooling manager and the way using Future obj to avoid blocking looks ugly.

Can somebody review this?

import java.io.BufferedReader;
import java.io.InputStreamReader;
import java.util.concurrent.Callable;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;
import java.util.concurrent.Future;
import java.util.concurrent.TimeUnit;
import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.client.HttpClient;
import org.apache.http.client.methods.HttpGet;
import org.apache.http.conn.ClientConnectionManager;
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.conn.SchemeRegistryFactory;
import org.apache.http.params.CoreConnectionPNames;
import org.apache.http.params.HttpParams;
import org.apache.log4j.Logger;
@SuppressWarnings("deprecation")
public class ProxiedClientUtil {
 /* data member */
 private static final Integer HTTPCLIENT_TIMEOUT = 1000 * 30;
 private static final Integer HTTPGET_TIMEOUT = 1000 * 60; // 1min
 private static final Integer BUFFER_SIZE = 1024 * 1024;
 private static final Integer MAX_TOTAL = 200;
 private static final Integer MAX_PER_ROUTE = 20;
 private static Logger log = Logger.getLogger(ProxiedClientUtil.class);
 private static PoolingClientConnectionManager connManager = null;
 private static class Proxy {
 private String host;
 private Integer port;
 private Proxy(String host, Integer port) {
 this.host = host;
 this.port = port;
 }
 }
 /* private methods */
 private static ClientConnectionManager getSingletonConnectionManager() {
 if (connManager == null) {
 connManager = new PoolingClientConnectionManager(SchemeRegistryFactory.createDefault());
 connManager.setMaxTotal(MAX_TOTAL);
 connManager.setDefaultMaxPerRoute(MAX_PER_ROUTE);
 }
 return connManager;
 }
 private static HttpClient getPooledHttpClient() {
 HttpClient httpClient = new DefaultHttpClient(getSingletonConnectionManager());
 httpClient.getParams().setParameter(CoreConnectionPNames.SO_TIMEOUT, HTTPCLIENT_TIMEOUT);
 httpClient.getParams().setParameter(CoreConnectionPNames.CONNECTION_TIMEOUT, HTTPCLIENT_TIMEOUT);
 httpClient.getParams().setParameter(CoreConnectionPNames.TCP_NODELAY, false);
 httpClient.getParams().setParameter(CoreConnectionPNames.SOCKET_BUFFER_SIZE, BUFFER_SIZE);
 return httpClient;
 }
 private static HttpClient getProxiedHttpClient(String host, Integer port)
 throws Exception {
 HttpClient httpClient = getPooledHttpClient();
 HttpParams params = httpClient.getParams();
 if (host != null && port != null) {
 HttpHost proxy = new HttpHost(host, port);
 params.setParameter(ConnRoutePNames.DEFAULT_PROXY, proxy);
 }
 return httpClient;
 }
 private static HttpResponse executeHttpGet(final HttpClient httpClient,
 final HttpGet httpGet) throws Exception {
 Callable<HttpResponse> readCallable = new Callable<HttpResponse>() {
 @Override
 public HttpResponse call() throws Exception {
 return httpClient.execute(httpGet);
 }
 };
 HttpResponse httpResponse = null;
 ExecutorService futurExecutor = Executors.newFixedThreadPool(1);
 Future<HttpResponse> future = futurExecutor.submit(readCallable);
 try {
 httpResponse = future.get(HTTPGET_TIMEOUT, TimeUnit.MILLISECONDS);
 futurExecutor.shutdown();
 } catch (Exception e) {
 futurExecutor.shutdown();
 throw e;
 }
 return httpResponse;
 }
 /* public methods */
 public static String fetch(String url) {
 String pageContent = null;
 HttpGet httpGet = null;
 Proxy proxy = getProxy();
 try {
 HttpClient httpClient = getProxiedHttpClient(proxy.host, proxy.port);
 httpGet = new HttpGet(url);
 httpGet.setHeader("accept-language", "zh,en-us;q=0.8,en;q=0.6");
 httpGet.setHeader("content-type", "text/html;charset=UTF-8");
 HttpResponse response = executeHttpGet(httpClient, httpGet);
 if (response.getStatusLine().getStatusCode() == HttpStatus.SC_OK) {
 BufferedReader in = new BufferedReader(new InputStreamReader(response.getEntity().getContent(), "utf-8"));
 StringBuilder sb = new StringBuilder();
 String line = null;
 while ((line = in.readLine()) != null) {
 sb.append(line).append(System.getProperty("line.separator"));
 }
 pageContent = sb.toString();
 } else {
 log.info("Response status code error " + httpClient.getParams().getParameter(ConnRoutePNames.DEFAULT_PROXY));
 }
 } catch (Exception e) {
 log.info(e.getMessage());
 } finally {
 if (httpGet != null){
 httpGet.releaseConnection();
 }
 }
 return pageContent;
 }
 /* fake method for test */
 protected static Proxy getProxy() {
 // return new Proxy(null, null);
 return new Proxy("14.37.69.97", 3128);
 }
 public static void main(String[] args) {
 String url = "https://github.com";
 String pageContent = fetch(url);
 log.info(pageContent);
 }
}

• chillworld: what java version do you use?
• upton: java version "1.6.0_65"

Answer 1

 @SuppressWarnings("deprecation")

This is a big no from me. Deprecation is something that is there for a reason, I rarely advocate for comments but this would be a good opportunity to do so. If there is a good reason to use a deprecated function/class it should be documented since this is a deal breaker for me.

---

/* fake method for test */
protected static Proxy getProxy() {
 // return new Proxy(null, null);
 return new Proxy("14.37.69.97", 3128);
}

I don't like test method mix with "production" code. It easy to forget, people could rely on it and a lot of bad things can happen. What I consider worst is that you actually need it in your code : Proxy proxy = getProxy();. If I remove getProxy() because it's only a test method, your code won't run anymore, it's dependent on it. There is no way for the user to "inject" a Proxy to use.

---

} else {
 log.info("Response status code error " + httpClient.getParams().getParameter(ConnRoutePNames.DEFAULT_PROXY));
}

If you have something different than HttpStatus.SC_OK you still need to consume the entity/connection or you will face problem when you will have a lot of connection that will be left hanging.