Permalink
Browse files

Added safeExecute(HttpHost, HttpUriRequest)

  • Loading branch information...
1 parent eb5eed1 commit f60700552b63e6690fafd4424ea2bc15e4332440 @jberkel jberkel committed Mar 19, 2012
@@ -3,7 +3,6 @@
import org.apache.http.ConnectionReuseStrategy;
import org.apache.http.Header;
import org.apache.http.HttpHost;
-import org.apache.http.HttpRequest;
import org.apache.http.HttpResponse;
import org.apache.http.HttpStatus;
import org.apache.http.auth.AUTH;
@@ -532,28 +531,36 @@ public synchronized void setTokenListener(TokenListener listener) {
/**
* Execute an API request, adds the necessary headers.
- * @param req the HTTP request
+ * @param request the HTTP request
* @return the HTTP response
* @throws java.io.IOException network error etc.
*/
- public HttpResponse execute(HttpUriRequest req) throws IOException {
+ public HttpResponse execute(HttpUriRequest request) throws IOException {
+ return safeExecute(env.sslResourceHost, addHeaders(request));
+ }
+
+ public HttpResponse safeExecute(HttpHost target, HttpUriRequest request) throws IOException {
+ if (target == null) {
+ target = determineTarget(request);
+ }
+
try {
- return getHttpClient().execute(env.sslResourceHost, addHeaders(req));
+ return getHttpClient().execute(target, request);
} catch (NullPointerException e) {
// this is a workaround for a broken httpclient version,
// cf. http://code.google.com/p/android/issues/detail?id=5255
// NPE in DefaultRequestDirector.java:456
- if (!req.isAborted() && req.getParams().isParameterFalse("npe-retried")) {
- req.getParams().setBooleanParameter("npe-retried", true);
- return execute(req);
+ if (!request.isAborted() && request.getParams().isParameterFalse("npe-retried")) {
+ request.getParams().setBooleanParameter("npe-retried", true);
+ return safeExecute(target, request);
} else {
- req.abort();
+ request.abort();
throw new BrokenHttpClientException(e);
}
} catch (IllegalArgumentException e) {
// more brokenness
// cf. http://code.google.com/p/android/issues/detail?id=2690
- req.abort();
+ request.abort();
throw new BrokenHttpClientException(e);
}
}
@@ -563,6 +570,21 @@ protected HttpResponse execute(Request req, Class<? extends HttpRequestBase> req
return execute(req.buildRequest(reqType));
}
+
+ protected HttpHost determineTarget(HttpUriRequest request) {
+ // A null target may be acceptable if there is a default target.
+ // Otherwise, the null target is detected in the director.
+ URI requestURI = request.getURI();
+ if (requestURI.isAbsolute()) {
+ return new HttpHost(
+ requestURI.getHost(),
+ requestURI.getPort(),
+ requestURI.getScheme());
+ } else {
+ return null;
+ }
+ }
+
/**
* serialize the wrapper to a File
* @param f target
@@ -618,23 +640,23 @@ public static Header createOAuthHeader(Token token) {
}
/** Adds an OAuth2 header to a given request */
- protected HttpRequest addAuthHeader(HttpRequest request) {
+ protected HttpUriRequest addAuthHeader(HttpUriRequest request) {
if (!request.containsHeader(AUTH.WWW_AUTH_RESP)) {
request.addHeader(createOAuthHeader(getToken()));
}
return request;
}
/** Forces JSON */
- protected HttpRequest addAcceptHeader(HttpRequest request) {
+ protected HttpUriRequest addAcceptHeader(HttpUriRequest request) {
if (!request.containsHeader("Accept")) {
request.addHeader("Accept", getDefaultContentType());
}
return request;
}
/** Adds all required headers to the request */
- protected HttpRequest addHeaders(HttpRequest req) {
+ protected HttpUriRequest addHeaders(HttpUriRequest req) {
return addAcceptHeader(
addAuthHeader(req));
}
@@ -659,11 +681,4 @@ protected RequestDirector getRequestDirector(HttpRequestExecutor requestExec,
stateHandler, params);
}
- public static class BrokenHttpClientException extends IOException {
- private static final long serialVersionUID = -4764332412926419313L;
-
- BrokenHttpClientException(Throwable throwable) {
- super(throwable);
- }
- }
}
@@ -1,7 +1,9 @@
package com.soundcloud.api;
+import org.apache.http.HttpHost;
import org.apache.http.HttpResponse;
import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpUriRequest;
import java.io.IOException;
import java.net.URI;
@@ -157,6 +159,17 @@
HttpClient getHttpClient();
/**
+ * Generic execute method, with added workarounds for various HTTPClient bugs.
+ *
+ * @param target the target host (can be null)
+ * @param request the request
+ * @return the HTTP response
+ * @throws IOException network errors
+ * @throws BrokenHttpClientException in case of HTTPClient framework bugs
+ */
+ HttpResponse safeExecute(HttpHost target, HttpUriRequest request) throws IOException;
+
+ /**
* Resolve the given SoundCloud URI
*
* @param uri SoundCloud model URI, e.g. http://soundcloud.com/bob
@@ -271,4 +284,13 @@ public String getMessage() {
return super.getMessage()+" "+(response != null ? response.getStatusLine() : "");
}
}
+
+
+ class BrokenHttpClientException extends IOException {
+ private static final long serialVersionUID = -4764332412926419313L;
+
+ BrokenHttpClientException(Throwable throwable) {
+ super(throwable);
+ }
+ }
}
@@ -485,4 +485,34 @@ public HttpClient getHttpClient() {
verify(client, times(1)).execute(any(HttpHost.class), any(HttpUriRequest.class));
}
}
+
+ @SuppressWarnings("serial")
+ @Test
+ public void shouldSafeExecute() throws Exception {
+
+ final HttpClient client = mock(HttpClient.class);
+ ApiWrapper broken = new ApiWrapper("invalid", "invalid", URI.create("redirect://me"), null, Env.SANDBOX) {
+ @Override
+ public HttpClient getHttpClient() {
+ return client;
+ }
+ };
+ when(client.execute(any(HttpHost.class), any(HttpUriRequest.class))).thenThrow(new IllegalArgumentException());
+ try {
+ broken.safeExecute(null, new HttpGet("/foo"));
+ fail("expected BrokenHttpClientException");
+ } catch (ApiWrapper.BrokenHttpClientException expected) {
+ verify(client, times(1)).execute(any(HttpHost.class), any(HttpUriRequest.class));
+ }
+
+ reset(client);
+ when(client.execute(any(HttpHost.class), any(HttpUriRequest.class))).thenThrow(new NullPointerException());
+ try {
+ broken.execute(new HttpGet("/foo"));
+ fail("expected BrokenHttpClientException");
+ } catch (ApiWrapper.BrokenHttpClientException expected) {
+ // make sure client retried request
+ verify(client, times(2)).execute(any(HttpHost.class), any(HttpUriRequest.class));
+ }
+ }
}

0 comments on commit f607005

Please sign in to comment.