Code review feedback

.gitignore

@@ -4,3 +4,4 @@ build
 
 .classpath
 .project
+eclipsebin

modules/http/src-tests/retrofit/http/RestAdapterTest.java

+package retrofit.http;
+
+import com.google.gson.Gson;
+import com.google.inject.AbstractModule;
+import com.google.inject.Guice;
+import com.google.inject.Injector;
+import com.google.inject.name.Named;
+import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.util.concurrent.Executor;
+import junit.framework.TestCase;
+import org.apache.http.HttpMessage;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolVersion;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpDelete;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpPut;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.message.BasicStatusLine;
+import org.easymock.Capture;
+import org.easymock.IAnswer;
+import org.junit.Before;
+import retrofit.core.Callback;
+import retrofit.core.MainThread;
+
+import static org.easymock.EasyMock.capture;
+import static org.easymock.EasyMock.createMock;
+import static org.easymock.EasyMock.expect;
+import static org.easymock.EasyMock.expectLastCall;
+import static org.easymock.EasyMock.isA;
+import static org.easymock.EasyMock.replay;
+import static org.easymock.EasyMock.verify;
+import static retrofit.http.RestAdapter.service;
+
+public class RestAdapterTest extends TestCase {
+  private static final String ID = "123";
+  private static final String ENTITY = "entity";
+  private static final String ENTITY_PATH_PARAM = "entity/{id}";
+  private static final String BASE_URL = "http://host/api/entity";
+  private static final String PATH_URL_PREFIX = BASE_URL + "/";
+  private static final String GET_DELETE_SIMPLE_URL = BASE_URL + "?";
+
+  private Injector injector;
+  private HttpClient mockHttpClient;
+  private Executor mockExecutor;
+  private MainThread mockMainThread;
+  private Headers mockHeaders;
+  @SuppressWarnings("rawtypes") private Callback mockCallback;
+  private HttpResponse mockResponse;
+
+  @Override @Before public void setUp() throws Exception {
+    mockHttpClient = createMock(HttpClient.class);
+    mockExecutor   = createMock(Executor.class);
+    mockMainThread = createMock(MainThread.class);
+    mockHeaders    = createMock(Headers.class);
+    mockCallback   = createMock(Callback.class);
+    mockResponse   = createMock(HttpResponse.class);
+
+    injector = Guice.createInjector(
+        new AbstractModule() {
+          @Override protected void configure() {
+            bind(Server.class).toInstance(new Server("http://host/api/",
+                "http://host/web/", true));
+            bind(HttpClient.class).toInstance(mockHttpClient);
+            bind(Executor.class).toInstance(mockExecutor);
+            bind(MainThread.class).toInstance(mockMainThread);
+            bind(Headers.class).toInstance(mockHeaders);
+            install(service(DeleteService.class));
+            install(service(GetService.class));
+            install(service(PostService.class));
+            install(service(PutService.class));
+          }
+        });
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceDeleteSimple() throws IOException {
+    expectLifecycle(HttpDelete.class, GET_DELETE_SIMPLE_URL);
+    replayAll();
+
+    DeleteService service = injector.getInstance(DeleteService.class);
+    service.delete(mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceDeleteParam() throws IOException {
+    expectLifecycle(HttpDelete.class, GET_DELETE_SIMPLE_URL + "id=" + ID);
+    replayAll();
+
+    DeleteService service = injector.getInstance(DeleteService.class);
+    service.deleteWithParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceDeletePathParam() throws IOException {
+    expectLifecycle(HttpDelete.class, PATH_URL_PREFIX + ID + "?");
+    replayAll();
+
+    DeleteService service = injector.getInstance(DeleteService.class);
+    service.deleteWithPathParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceGetSimple() throws IOException {
+    expectLifecycle(HttpGet.class, GET_DELETE_SIMPLE_URL);
+    replayAll();
+
+    GetService service = injector.getInstance(GetService.class);
+    service.get(mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceGetParam() throws IOException {
+    expectLifecycle(HttpGet.class, GET_DELETE_SIMPLE_URL + "id=" + ID);
+    replayAll();
+
+    GetService service = injector.getInstance(GetService.class);
+    service.getWithParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServiceGetPathParam() throws IOException {
+    expectLifecycle(HttpGet.class, PATH_URL_PREFIX + ID + "?");
+    replayAll();
+
+    GetService service = injector.getInstance(GetService.class);
+    service.getWithPathParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePostSimple() throws IOException {
+    expectLifecycle(HttpPost.class, BASE_URL);
+    replayAll();
+
+    PostService service = injector.getInstance(PostService.class);
+    service.post(mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePostParam() throws IOException {
+    expectLifecycle(HttpPost.class, BASE_URL);
+    replayAll();
+
+    PostService service = injector.getInstance(PostService.class);
+    service.postWithParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePostPathParam() throws IOException {
+    expectLifecycle(HttpPost.class, PATH_URL_PREFIX + ID);
+    replayAll();
+
+    PostService service = injector.getInstance(PostService.class);
+    service.postWithPathParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePutSimple() throws IOException {
+    expectLifecycle(HttpPut.class, BASE_URL);
+    replayAll();
+
+    PutService service = injector.getInstance(PutService.class);
+    service.put(mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePutParam() throws IOException {
+    expectLifecycle(HttpPut.class, BASE_URL);
+    replayAll();
+
+    PutService service = injector.getInstance(PutService.class);
+    service.putWithParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  @SuppressWarnings("unchecked")
+  public void testServicePutPathParam() throws IOException {
+    expectLifecycle(HttpPut.class, PATH_URL_PREFIX + ID);
+    replayAll();
+
+    PutService service = injector.getInstance(PutService.class);
+    service.putWithPathParam(ID, mockCallback);
+    verifyAll();
+  }
+
+  //
+  // Utility Methods:
+  //
+  private void replayAll() {
+    replay(mockExecutor, mockHeaders, mockHttpClient, mockMainThread,
+        mockCallback, mockResponse);
+  }
+
+  private void verifyAll() {
+    verify(mockExecutor, mockHeaders, mockHttpClient, mockMainThread,
+        mockCallback, mockResponse);
+  }
+
+  private <T extends HttpUriRequest> void expectLifecycle(Class<T> requestClass,
+      String requestUrl) throws UnsupportedEncodingException, IOException {
+    expectExecution(mockExecutor);
+    expectExecution(mockMainThread);
+    expectSetOnWithRequest(requestClass, requestUrl);
+    Response response = new Response("some text");
+    expectResponseCalls(new Gson().toJson(response));
+    expectHttpClientExecute();
+    expectCall(response);
+  }
+
+  @SuppressWarnings("unchecked") private void expectCall(Response response) {
+    mockCallback.call(response);
+    expectLastCall().once();
+  }
+
+  private void expectHttpClientExecute() throws IOException {
+    final Capture<GsonResponseHandler<?>> capture
+        = new Capture<GsonResponseHandler<?>>();
+    mockHttpClient.execute(isA(HttpUriRequest.class), capture(capture));
+    expectLastCall().andAnswer(new IAnswer<Object>() {
+      @Override public Object answer() throws Throwable {
+        GsonResponseHandler<?> responseHandler = capture.getValue();
+        responseHandler.handleResponse(mockResponse);
+        return null;
+      }
+    });
+  }
+
+  private void expectResponseCalls(String jsonToReturn)
+      throws UnsupportedEncodingException {
+    expect(mockResponse.getEntity()).andReturn(new StringEntity(jsonToReturn));
+    expect(mockResponse.getStatusLine()).andReturn(new BasicStatusLine(new ProtocolVersion("HTTP", 1, 1), 200, ""));
+  }
+
+  private <T extends HttpUriRequest> void expectSetOnWithRequest(
+      final Class<T> expectedRequestClass, final String expectedUri) {
+    final Capture<HttpMessage> capture = new Capture<HttpMessage>();
+    mockHeaders.setOn(capture(capture));
+    expectLastCall().andAnswer(new IAnswer<Object>() {
+      @Override public Object answer() throws Throwable {
+        T request = expectedRequestClass.cast(capture.getValue());
+        assertEquals("uri should match expectations", expectedUri, request.getURI().toString());
+        return null;
+      }
+    });
+  }
+
+  private void expectExecution(Executor executor) {
+    final Capture<Runnable> capture = new Capture<Runnable>();
+    executor.execute(capture(capture));
+    expectLastCall().andAnswer(new IAnswer<Object>() {
+      @Override public Object answer() throws Throwable {
+        capture.getValue().run();
+        return null;
+      }
+    });
+  }
+
+  private interface DeleteService {
+
+    @DELETE(ENTITY) void delete(Callback<Response> callback);
+
+    @DELETE(ENTITY) void deleteWithParam(@Named("id") String id,
+        Callback<Response> callback);
+
+    @DELETE(ENTITY_PATH_PARAM) void deleteWithPathParam(@Named("id") String id,
+        Callback<Response> callback);
+  }
+
+  private interface GetService {
+    @GET(ENTITY) void get(Callback<Response> callback);
+
+    @GET(ENTITY) void getWithParam(@Named("id") String id,
+        Callback<Response> callback);
+
+    @GET(ENTITY_PATH_PARAM) void getWithPathParam(@Named("id") String id,
+        Callback<Response> callback);
+  }
+
+  private interface PostService {
+    @POST(ENTITY) void post(Callback<Response> callback);
+
+    @POST(ENTITY) void postWithParam(@Named("id") String id,
+        Callback<Response> callback);
+
+    @POST(ENTITY_PATH_PARAM) void postWithPathParam(@Named("id") String id,
+        Callback<Response> callback);
+  }
+
+  private interface PutService {
+    @PUT(ENTITY) void put(Callback<Response> callback);
+
+    @PUT(ENTITY) void putWithParam(@Named("id") String id,
+        Callback<Response> callback);
+
+    @PUT(ENTITY_PATH_PARAM) void putWithPathParam(@Named("id") String id,
+        Callback<Response> callback);
+  }
+
+  private static class Response {
+    final String text;
+    public Response() {
+      this("");
+    }
+    public Response(String text) {
+      this.text = text;
+    }
+    @Override public int hashCode() {
+      return 7;
+    }
+    @Override public boolean equals(Object obj) {
+      return text.equals(((Response)obj).text);
+    }
+
+  }
+}

modules/http/src/retrofit/http/CallbackResponseHandler.java

@@ -2,6 +2,9 @@
 package retrofit.http;
 
 import com.google.gson.Gson;
+import java.io.IOException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
@@ -9,10 +12,6 @@ import org.apache.http.client.ResponseHandler;
 import org.apache.http.entity.BufferedHttpEntity;
 import retrofit.core.Callback;
 
-import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
-
 /**
  * Support for response handlers that invoke {@link Callback}.
  *
@@ -29,9 +28,9 @@ public abstract class CallbackResponseHandler<T>
   private static final int BAD_GATEWAY = 502;
   private static final int GATEWAY_TIMEOUT = 504;
 
-  private final UiCallback<T> callback;
+  private final Callback<T> callback;
 
-  public CallbackResponseHandler(UiCallback<T> callback) {
+  public CallbackResponseHandler(Callback<T> callback) {
     this.callback = callback;
   }
 

modules/http/src/retrofit/http/DELETE.java

@@ -2,6 +2,7 @@ package retrofit.http;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
+import retrofit.http.HttpMethod.Type;
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
@@ -9,10 +10,11 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
 /**
  * Make a DELETE request to a REST path relative to base URL.
  *
- * @author Rob Dickerson
+ * @author Patrick Forhan (patrick@squareup.com)
  */
 @Target({ METHOD })
 @Retention(RUNTIME)
+@HttpMethod(value = Type.DELETE)
 public @interface DELETE {
   String value();
 }

modules/http/src/retrofit/http/GET.java

@@ -2,6 +2,7 @@ package retrofit.http;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
+import retrofit.http.HttpMethod.Type;
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
@@ -13,6 +14,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
  */
 @Target({ METHOD })
 @Retention(RUNTIME)
+@HttpMethod(value = Type.GET)
 public @interface GET {
   String value();
 }

modules/http/src/retrofit/http/GsonResponseHandler.java

+package retrofit.http;
+
+import com.google.gson.Gson;
+import com.google.gson.JsonParseException;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.lang.reflect.Type;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+import org.apache.http.HttpEntity;
+import retrofit.core.Callback;
+
+/**
+ * Converts JSON response to an object using Gson and then passes it to {@link
+ * Callback#call(T)}.
+ */
+public class GsonResponseHandler<T> extends CallbackResponseHandler<T> {
+  private static final Logger logger =
+    Logger.getLogger(GsonResponseHandler.class.getName());
+
+  private final Type type;
+
+  GsonResponseHandler(Type type, Callback<T> callback) {
+    super(callback);
+    this.type = type;
+  }
+
+  static <T> GsonResponseHandler<T> create(Type type,
+      Callback<T> callback) {
+    return new GsonResponseHandler<T>(type, callback);
+  }
+
+  @Override protected T parse(HttpEntity entity) throws IOException,
+      ServerException {
+    try {
+      if (logger.isLoggable(Level.FINE)) {
+        entity = HttpClients.copyAndLog(entity);
+      }
+
+      // TODO: Use specified encoding.
+      InputStreamReader in = new InputStreamReader(entity.getContent(),
+          "UTF-8");
+
+      /*
+       * It technically isn't safe for fromJson() to return T here.
+       * We derived type from Callback<T>, so we know we're safe.
+       */
+      @SuppressWarnings("unchecked")
+      T t = (T) new Gson().fromJson(in, type);
+      return t;
+    } catch (JsonParseException e) {
+      // The server returned us bad JSON!
+      throw new ServerException(e);
+    }
+  }
+}
\ No newline at end of file

modules/http/src/retrofit/http/HttpMethod.java

@@ -3,6 +3,7 @@ package retrofit.http;
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
 
+import static java.lang.annotation.ElementType.ANNOTATION_TYPE;
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
 
@@ -11,9 +12,9 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
  *
  * @author Rob Dickerson (rdickerson@squareup.com)
  */
-@Target({ METHOD })
+@Target({ METHOD, ANNOTATION_TYPE })
 @Retention(RUNTIME)
 public @interface HttpMethod {
-  enum Type {GET, POST}
+  enum Type {DELETE, GET, POST, PUT}
   Type value();
 }

modules/http/src/retrofit/http/POST.java

@@ -2,6 +2,7 @@ package retrofit.http;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
+import retrofit.http.HttpMethod.Type;
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
@@ -13,6 +14,7 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
  */
 @Target({ METHOD })
 @Retention(RUNTIME)
+@HttpMethod(value = Type.POST)
 public @interface POST {
   String value();
 }

modules/http/src/retrofit/http/PUT.java

@@ -2,6 +2,7 @@ package retrofit.http;
 
 import java.lang.annotation.Retention;
 import java.lang.annotation.Target;
+import retrofit.http.HttpMethod.Type;
 
 import static java.lang.annotation.ElementType.METHOD;
 import static java.lang.annotation.RetentionPolicy.RUNTIME;
@@ -9,10 +10,11 @@ import static java.lang.annotation.RetentionPolicy.RUNTIME;
 /**
  * Make a PUT request to a REST path relative to base URL.
  *
- * @author Rob Dickerson
+ * @author Patrick Forhan (patrick@squareup.com)
  */
 @Target({ METHOD })
 @Retention(RUNTIME)
+@HttpMethod(value = Type.PUT)
 public @interface PUT {
   String value();
 }

modules/http/src/retrofit/http/RestAdapter.java

 package retrofit.http;
 
-import com.google.gson.Gson;
-import com.google.gson.JsonParseException;
 import com.google.inject.Binder;
 import com.google.inject.Inject;
 import com.google.inject.Module;
@@ -9,7 +7,6 @@ import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
 import java.io.IOException;
-import java.io.InputStreamReader;
 import java.io.OutputStream;
 import java.io.UnsupportedEncodingException;
 import java.lang.annotation.Annotation;
@@ -22,11 +19,11 @@ import java.lang.reflect.WildcardType;
 import java.net.URI;
 import java.net.URISyntaxException;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.Executor;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.NameValuePair;
 import org.apache.http.client.HttpClient;
@@ -112,10 +109,37 @@ import retrofit.io.TypedBytes;
     private final String relativePath;
     private final HttpMethod httpMethod;
 
-    public RequestLine(String relativePath, HttpMethod method) {
-      this.relativePath = relativePath;
-      this.httpMethod = method;
+    private RequestLine(retrofit.http.HttpMethod.Type methodType,
+        Annotation methodAnnotation) {
+      relativePath = getValue(methodAnnotation);
+      httpMethod = fromType(methodType);
+    }
+
+    private static HttpMethod fromType(
+        retrofit.http.HttpMethod.Type methodType)
+        throws AssertionError {
+      switch (methodType) {
+        case DELETE: return HttpMethod.DELETE;
+        case GET:    return HttpMethod.GET;
+        case POST:   return HttpMethod.POST;
+        case PUT:    return HttpMethod.PUT;
+        default:
+          throw new AssertionError("This line should be unreachable. Has a" +
+          		" new kind of http method annotation been added?");
+      }
+    }
+
+    private static String getValue(Annotation methodAnnotation) {
+      try {
+        final Method valueMethod = methodAnnotation.annotationType()
+            .getMethod("value");
+        return (String) valueMethod.invoke(methodAnnotation);
+
+      } catch (Exception ex) {
+        throw new IllegalStateException("Failed to extract method path", ex);
+      }
     }
+
     public String getRelativePath() {
       return relativePath;
     }
@@ -124,38 +148,36 @@ import retrofit.io.TypedBytes;
     }
   }
 
+  /**
+   * Looks for exactly one annotation of type {@link DELETE}, {@link GET},
+   * {@link POST}, or {@link PUT} and extracts its path data.  Throws an
+   * {@link IllegalStateException} if none or multiple are found.
+   */
   private static RequestLine readHttpMethodAnnotation(Method method) {
-    Annotation httpMethod = findAnnotation(method, GET.class, null);
-    httpMethod = findAnnotation(method, POST.class, httpMethod);
-    httpMethod = findAnnotation(method, PUT.class, httpMethod);
-    httpMethod = findAnnotation(method, DELETE.class, httpMethod);
-
-    if (httpMethod instanceof GET) {
-      return new RequestLine(((GET) httpMethod).value(), HttpMethod.GET);
-    } else if (httpMethod instanceof POST) {
-      return new RequestLine(((POST) httpMethod).value(), HttpMethod.POST);
-    } else if (httpMethod instanceof PUT) {
-      return new RequestLine(((PUT) httpMethod).value(), HttpMethod.PUT);
-    } else if (httpMethod instanceof DELETE) {
-      return new RequestLine(((DELETE) httpMethod).value(), HttpMethod.DELETE);
-    } else {
-      throw new IllegalArgumentException(
-          "Method not annotated with GET, POST, PUT, or DELETE: "
-            + method.getName());
+    Annotation[] annotations = method.getAnnotations();
+    RequestLine found = null;
+    for (Annotation annotation : annotations) {
+      // look for an HttpMethod annotation describing the type:
+      final retrofit.http.HttpMethod typeAnnotation = annotation.annotationType()
+          .getAnnotation(retrofit.http.HttpMethod.class);
+      if (typeAnnotation != null) {
+        if (found != null) {
+          throw new IllegalStateException(
+              "Method annotated with multiple HTTP method annotations: "
+                + method.toString());
+        }
+        found = new RequestLine(typeAnnotation.value(), annotation);
+      }
     }
-  }
 
-  private static <T extends Annotation> T findAnnotation(Method method,
-      Class<T> annotationClass, Annotation previousFind) {
-    T annotation = method.getAnnotation(annotationClass);
-    if (annotation != null && previousFind != null) {
-      throw new IllegalArgumentException(
-          "Method annotated with multiple HTTP method annotations: "
-            + annotationClass.getSimpleName() + " and " + previousFind.getClass().getName());
+    if (found == null) {
+      throw new IllegalStateException(
+          "Method not annotated with GET, POST, PUT, or DELETE: "
+            + method.toString());
     }
-
-    return annotation;
+    return found;
   }
+
   /** Gets the parameter name from the @Named annotation. */
   private static String getName(Annotation[] annotations, Method method,
       int parameterIndex) {
@@ -185,7 +207,7 @@ import retrofit.io.TypedBytes;
     GET {
       @Override HttpUriRequest createFrom(HttpRequestBuilder builder)
           throws URISyntaxException {
-        List<NameValuePair> queryParams = builder.createParamList();
+        List<NameValuePair> queryParams = builder.getParamList(false);
         String queryString = URLEncodedUtils.format(queryParams, "UTF-8");
         URI uri = URIUtils.createURI(builder.getScheme(), builder.getHost(), -1,
             builder.getRelativePath(), queryString, null);
@@ -222,7 +244,7 @@ import retrofit.io.TypedBytes;
     DELETE {
       @Override HttpUriRequest createFrom(HttpRequestBuilder builder)
           throws URISyntaxException {
-        List<NameValuePair> queryParams = builder.createParamList();
+        List<NameValuePair> queryParams = builder.getParamList(false);
         String queryString = URLEncodedUtils.format(queryParams, "UTF-8");
         URI uri = URIUtils.createURI(builder.getScheme(), builder.getHost(), -1,
             builder.getRelativePath(), queryString, null);
@@ -276,7 +298,7 @@ import retrofit.io.TypedBytes;
         request.setEntity(form);
       } else {
         try {
-          List<NameValuePair> paramList = builder.createParamList();
+          List<NameValuePair> paramList = builder.getParamList(true);
           request.setEntity(new UrlEncodedFormEntity(paramList));
         } catch (UnsupportedEncodingException e) {
           throw new AssertionError(e);
@@ -303,13 +325,15 @@ import retrofit.io.TypedBytes;
     private Object[] args;
     private HttpMethod httpMethod;
     private String apiUrl;
-    private String relativePath;
+    private String replacedRelativePath;
     private Headers headers;
+    private String originalRelativePath;
+    private List<NameValuePair> nonPathParams;
 
     public HttpRequestBuilder setMethod(Method method) {
       this.javaMethod = method;
       RequestLine requestLine = readHttpMethodAnnotation(method);
-      this.relativePath = requestLine.getRelativePath();
+      this.originalRelativePath = requestLine.getRelativePath();
       this.httpMethod = requestLine.getHttpMethod();
       return this;
     }
@@ -319,21 +343,12 @@ import retrofit.io.TypedBytes;
     }
 
     public String getRelativePath() {
-      if (hasPathParameters()) {
-        String replacedPath = relativePath;
-        List<NameValuePair> paramList = createParamList();
-        for (NameValuePair pair : paramList) {
-          replacedPath = replacedPath.replaceAll(
-              "\\{" + pair.getName() + "\\}", replacedPath);
-        }
-
-        return replacedPath;
-      }
-      return relativePath;
+      return replacedRelativePath != null ? replacedRelativePath
+          : originalRelativePath;
     }
 
     private boolean hasPathParameters() {
-      return relativePath.contains("{");
+      return originalRelativePath.contains("{");
     }
 
     public HttpRequestBuilder setApiUrl(String apiUrl) {
@@ -371,11 +386,21 @@ import retrofit.io.TypedBytes;
       return host;
     }
 
+    /**
+     * Converts all but the last method argument to a list of HTTP request
+     * parameters.  If includePathParams is true, path parameters (like id in
+     * "/entity/{id}"will be included in this list.
+     */
+    public List<NameValuePair> getParamList(boolean includePathParams) {
+      if (includePathParams || nonPathParams == null) return createParamListN();
+      return nonPathParams;
+    }
+
     /**
      * Converts all but the last method argument to a list of HTTP request
      * parameters.
      */
-    public List<NameValuePair> createParamList() {
+    private List<NameValuePair> createParamListN() {
       Annotation[][] parameterAnnotations =
           javaMethod.getParameterAnnotations();
       int count = parameterAnnotations.length - 1;
@@ -392,6 +417,26 @@ import retrofit.io.TypedBytes;
     }
 
     public HttpUriRequest build() throws URISyntaxException {
+      // special handling if there are path parameters:
+      if (hasPathParameters()) {
+        List<NameValuePair> paramList = createParamListN();
+
+        String replacedPath = originalRelativePath;
+        Iterator<NameValuePair> itor = paramList.iterator();
+        while (itor.hasNext()) {
+          NameValuePair pair = itor.next();
+          String paramName = pair.getName();
+          if (replacedPath.contains("{" + paramName + "}")) {
+            replacedPath = replacedPath.replaceAll(
+                "\\{" + paramName + "\\}", pair.getValue());
+            itor.remove();
+          }
+        }
+
+        replacedRelativePath = replacedPath;
+        nonPathParams = paramList;
+      }
+
       return httpMethod.createFrom(this);
     }
   }
@@ -446,6 +491,7 @@ import retrofit.io.TypedBytes;
         logger.log(Level.WARNING, e.getMessage(), e);
         callback.networkError();
       } catch (Throwable t) {
+        t.printStackTrace();
         callback.unexpectedError(t);
       }
     }
@@ -554,49 +600,6 @@ import retrofit.io.TypedBytes;
     }
   }
 
-  /**
-   * Converts JSON response to an object using Gson and then passes it to {@link
-   * Callback#call(T)}.
-   */
-  static class GsonResponseHandler<T> extends CallbackResponseHandler<T> {
-
-    private final Type type;
-
-    GsonResponseHandler(Type type, UiCallback<T> callback) {
-      super(callback);
-      this.type = type;
-    }
-
-    static <T> GsonResponseHandler<T> create(Type type,
-        UiCallback<T> callback) {
-      return new GsonResponseHandler<T>(type, callback);
-    }
-
-    @Override protected T parse(HttpEntity entity) throws IOException,
-        ServerException {
-      try {
-        if (logger.isLoggable(Level.FINE)) {
-          entity = HttpClients.copyAndLog(entity);
-        }
-
-        // TODO: Use specified encoding.
-        InputStreamReader in = new InputStreamReader(entity.getContent(),
-            "UTF-8");
-
-        /*
-         * It technically isn't safe for fromJson() to return T here.
-         * We derived type from Callback<T>, so we know we're safe.
-         */
-        @SuppressWarnings("unchecked")
-        T t = (T) new Gson().fromJson(in, type);
-        return t;
-      } catch (JsonParseException e) {
-        // The server returned us bad JSON!
-        throw new ServerException(e);
-      }
-    }
-  }
-
   /**
    * Sends server call times and response status codes to {@link HttpProfiler}.
    */