Repository: square/retrofit
Issue: Lazily Cache Endpoint Metadata

We calculate a lot of metadata on endpoint definitions with every invocation. The heavy hitters should be cached in the anonymous proxy class generated by `RestAdapter` to avoid doing reflection on every API call.

Might be useful to abstract everything into a POJO and use a `Map<Method, MethodMetadata>` or something.


Issue comments:
Fixed by #129.


Pull request review comments:
not doing this per-request is very good

HashSet --> LinkedHashSet

HashMap --> LinkedHashMap

Would it be better to create all the methods eagerly? I could go either way myself.

Not that I offer any alternatives, but Method.equals() is a painfully slow method. It uses toString(). Sigh.

sucks that these aren't all final. It would probably make it worse, but you could change the code to have a static factory method that does all the parsing and then calls the constructor with all of the ready-to-go values.

nice defensive coding

Does this trigger if I call equals() on a service interface?

Did you rename Types to Utils?

you're calling method.getGenericParameterTypes() here and method.getParameterTypes() below. If you wanna make a small optimization, call only one of these and store the result?

nit: calling `toString()` is a waste of code

Seems like this method duplicates the work in parseResponseType.

LinkedHashSet

make this a static constant? I recall Patterns being expensive to compile

In the other pull request I did.

Yeah they're non-final so that we can load them in the background thread if you're executing things asynchronously. We need to know whether or not the method is a synchronous one or not regardless. Perhaps we just check the synchronous boolean like before and then pass it down so all these values can be computed in the constructor. Will think on it a bit.

`SparseArray` + `hashCode()`?

Ah, nevermind. That's Android-specific. Could pull its sources in. Or just allow boxing and use `Integer` in the map.

What you have here is good for now. If you want, we can profile this later and see if that is indeed a hot spot.

Yep. Filed @ #130

Rather than `Returns whether` say `Returns true if the method is synchronous`. Way less ambiguous.

Perfect

... though the cast could throw a ClassCastException if for whatever reason your last parameter isn't a Class<?>. For example, it'll probably crash if the last parameter is a GenericArrayType like `List<String>[]`. Defend against that by doing

```
   hasCallback = typeToCheck instanceof Class && Callback.class.isAssignableFrom(...);
```

you're duplicating this work. . . lines 330--339 are very similar to 312-317. Extract a method that returns the callbackType or null?

should this do something cleverer with exceptions? Usually when I call Executor.execute() I don't expect a RuntimeException to bubble up

mmmmm I love mexican food-themed test cases

nice

The only usage of this class is when you want asynchronous HTTP requests with callbacks happening on the background thread rather than some main thread. I'm not even sure if there's a use-case for it, really. Perhaps we should just simplify and remove it.

Ah, if you're on the JVM and don't specify your own executors this is the default asynchronous behavior as well.


Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/http/HttpRequestBuilder.java b/retrofit/src/main/java/retrofit/http/HttpRequestBuilder.java
index 37a283e851..4c919f896e 100644
--- a/retrofit/src/main/java/retrofit/http/HttpRequestBuilder.java
+++ b/retrofit/src/main/java/retrofit/http/HttpRequestBuilder.java
@@ -1,22 +1,21 @@
 package retrofit.http;
 
-import org.apache.http.NameValuePair;
-import org.apache.http.client.methods.HttpUriRequest;
-import org.apache.http.message.BasicNameValuePair;
-import retrofit.io.TypedBytes;
-
-import javax.inject.Named;
 import java.io.UnsupportedEncodingException;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
 import java.net.URISyntaxException;
 import java.net.URLEncoder;
 import java.util.ArrayList;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Set;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
+import javax.inject.Named;
+import org.apache.http.NameValuePair;
+import org.apache.http.client.methods.HttpUriRequest;
+import org.apache.http.message.BasicNameValuePair;
+import retrofit.io.TypedBytes;
+
+import static retrofit.http.RestAdapter.MethodDetails;
 
 /**
  * Builds HTTP requests from Java method invocations.  Handles "path parameters" in the
@@ -31,37 +30,33 @@
 final class HttpRequestBuilder {
   private final Converter converter;
 
-  private Method javaMethod;
-  private boolean isSynchronous;
+  private MethodDetails methodDetails;
   private Object[] args;
   private String apiUrl;
   private String replacedRelativePath;
   private Headers headers;
   private List<NameValuePair> nonPathParams;
-  private RequestLine requestLine;
   private TypedBytes singleEntity;
 
   HttpRequestBuilder(Converter converter) {
     this.converter = converter;
   }
 
-  HttpRequestBuilder setMethod(Method method, boolean isSynchronous) {
-    this.javaMethod = method;
-    this.isSynchronous = isSynchronous;
-    requestLine = RequestLine.fromMethod(method);
+  HttpRequestBuilder setMethod(MethodDetails methodDetails) {
+    this.methodDetails = methodDetails;
     return this;
   }
 
   Method getMethod() {
-    return javaMethod;
+    return methodDetails.method;
   }
 
   boolean isSynchronous() {
-    return isSynchronous;
+    return methodDetails.isSynchronous;
   }
 
   String getRelativePath() {
-    return replacedRelativePath != null ? replacedRelativePath : requestLine.getRelativePath();
+    return replacedRelativePath != null ? replacedRelativePath : methodDetails.path;
   }
 
   HttpRequestBuilder setApiUrl(String apiUrl) {
@@ -69,7 +64,6 @@ HttpRequestBuilder setApiUrl(String apiUrl) {
     return this;
   }
 
-  /** The last argument is assumed to be the Callback and is ignored. */
   HttpRequestBuilder setArgs(Object[] args) {
     this.args = args;
     return this;
@@ -110,46 +104,28 @@ List<NameValuePair> getParamList(boolean includePathParams) {
 
   /** Converts all but the last method argument to a list of HTTP request parameters. */
   private List<NameValuePair> createParamList() {
-    Annotation[][] parameterAnnotations = javaMethod.getParameterAnnotations();
-    int count = parameterAnnotations.length;
-    if (!isSynchronous) {
-      count -= 1;
-    }
-
-    List<NameValuePair> params = new ArrayList<NameValuePair>(count);
+    List<NameValuePair> params = new ArrayList<NameValuePair>();
 
     // Add query parameter(s), if specified.
-    QueryParams queryParams = javaMethod.getAnnotation(QueryParams.class);
-    if (queryParams != null) {
-      QueryParam[] annotations = queryParams.value();
-      for (QueryParam annotation : annotations) {
-        params.add(addPair(annotation));
-      }
-    }
-
-    // Also check for a single specified query parameter.
-    QueryParam queryParam = javaMethod.getAnnotation(QueryParam.class);
-    if (queryParam != null) {
-      params.add(addPair(queryParam));
+    for (QueryParam annotation : methodDetails.pathQueryParams) {
+      params.add(new BasicNameValuePair(annotation.name(), annotation.value()));
     }
 
     // Add arguments as parameters.
-    for (int i = 0; i < count; i++) {
+    String[] pathNamedParams = methodDetails.pathNamedParams;
+    int singleEntityArgumentIndex = methodDetails.singleEntityArgumentIndex;
+    for (int i = 0; i < pathNamedParams.length; i++) {
       Object arg = args[i];
       if (arg == null) continue;
-      for (Annotation annotation : parameterAnnotations[i]) {
-        final Class<? extends Annotation> type = annotation.annotationType();
-        if (type == Named.class) {
-          String name = getName(parameterAnnotations[i], javaMethod, i);
-          params.add(new BasicNameValuePair(name, String.valueOf(arg)));
-        } else if (type == SingleEntity.class) {
-          if (arg instanceof TypedBytes) {
-            // Let the object specify its own entity representation.
-            singleEntity = (TypedBytes) arg;
-          } else {
-            // Just an object: serialize it with supplied converter
-            singleEntity = converter.from(arg);
-          }
+      if (i != singleEntityArgumentIndex) {
+        params.add(new BasicNameValuePair(pathNamedParams[i], String.valueOf(arg)));
+      } else {
+        if (arg instanceof TypedBytes) {
+          // Let the object specify its own entity representation.
+          singleEntity = (TypedBytes) arg;
+        } else {
+          // Just an object: serialize it with supplied converter.
+          singleEntity = converter.from(arg);
         }
       }
     }
@@ -170,16 +146,12 @@ public String getMimeType() {
     return singleEntity == null ? null : singleEntity.mimeType().mimeName();
   }
 
-  private BasicNameValuePair addPair(QueryParam queryParam) {
-    return new BasicNameValuePair(queryParam.name(), queryParam.value());
-  }
-
   HttpUriRequest build() throws URISyntaxException {
     // Alter parameter list if path parameters are present.
-    Set<String> pathParams = getPathParameters(requestLine.getRelativePath());
+    Set<String> pathParams = new LinkedHashSet<String>(methodDetails.pathParams);
     List<NameValuePair> paramList = createParamList();
     if (!pathParams.isEmpty()) {
-      String replacedPath = requestLine.getRelativePath();
+      String replacedPath = methodDetails.path;
 
       for (String pathParam : pathParams) {
         NameValuePair found = null;
@@ -216,7 +188,7 @@ HttpUriRequest build() throws URISyntaxException {
       }
     }
 
-    return requestLine.getHttpMethod().createFrom(this);
+    return methodDetails.httpMethod.createFrom(this);
   }
 
   private String doReplace(String replacedPath, String paramName, String newVal) {
@@ -224,23 +196,6 @@ private String doReplace(String replacedPath, String paramName, String newVal) {
     return replacedPath;
   }
 
-  /**
-   * Gets the set of unique path params used in the given uri.  If a param is used twice in the
-   * uri, it will only show up once in the set.
-   *
-   * @param path the path to search through.
-   * @return set of path params.
-   */
-  static Set<String> getPathParameters(String path) {
-    Pattern p = Pattern.compile("\\{([a-z_-]*)\\}");
-    Matcher m = p.matcher(path);
-    Set<String> patterns = new HashSet<String>();
-    while (m.find()) {
-      patterns.add(m.group(1));
-    }
-    return patterns;
-  }
-
   /** Gets the parameter name from the {@link Named} annotation. */
   static String getName(Annotation[] annotations, Method method, int parameterIndex) {
     return findAnnotation(annotations, Named.class, method, parameterIndex).value();
diff --git a/retrofit/src/main/java/retrofit/http/Platform.java b/retrofit/src/main/java/retrofit/http/Platform.java
index f8dfd7b03e..0e4ddda90f 100644
--- a/retrofit/src/main/java/retrofit/http/Platform.java
+++ b/retrofit/src/main/java/retrofit/http/Platform.java
@@ -13,7 +13,7 @@
 import retrofit.android.MainThreadExecutor;
 
 import static android.os.Process.THREAD_PRIORITY_BACKGROUND;
-import static retrofit.http.RestAdapter.SynchronousExecutor;
+import static retrofit.http.Utils.SynchronousExecutor;
 import static retrofit.http.RestAdapter.THREAD_PREFIX;
 
 abstract class Platform {
diff --git a/retrofit/src/main/java/retrofit/http/RequestLine.java b/retrofit/src/main/java/retrofit/http/RequestLine.java
deleted file mode 100644
index 82a9ce2c71..0000000000
--- a/retrofit/src/main/java/retrofit/http/RequestLine.java
+++ /dev/null
@@ -1,66 +0,0 @@
-package retrofit.http;
-
-import java.lang.annotation.Annotation;
-import java.lang.reflect.Method;
-
-/**
- * Contains the desired {@link HttpMethodType} and relative path specified by a service method. See
- * also the factory method {@link #fromMethod(Method)}.
- *
- * @author Patrick Forhan (patrick@squareup.com)
- */
-final class RequestLine {
-  private final String relativePath;
-  private final HttpMethodType httpMethod;
-
-  private RequestLine(HttpMethodType methodType, Annotation methodAnnotation) {
-    relativePath = getValue(methodAnnotation);
-    httpMethod = methodType;
-  }
-
-  String getRelativePath() {
-    return relativePath;
-  }
-
-  HttpMethodType getHttpMethod() {
-    return httpMethod;
-  }
-
-  /** Using reflection, get the value field of the specified annotation. */
-  private static String getValue(Annotation annotation) {
-    try {
-      final Method valueMethod = annotation.annotationType().getMethod("value");
-      return (String) valueMethod.invoke(annotation);
-    } catch (Exception ex) {
-      throw new IllegalStateException("Failed to extract URI path", ex);
-    }
-  }
-
-  /**
-   * 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.
-   */
-  static RequestLine fromMethod(Method method) {
-    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);
-      }
-    }
-
-    if (found == null) {
-      throw new IllegalStateException(
-          "Method not annotated with GET, POST, PUT, or DELETE: " + method.toString());
-    }
-    return found;
-  }
-}
\ No newline at end of file
diff --git a/retrofit/src/main/java/retrofit/http/RestAdapter.java b/retrofit/src/main/java/retrofit/http/RestAdapter.java
index d82e3663c3..0721eff342 100644
--- a/retrofit/src/main/java/retrofit/http/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/http/RestAdapter.java
@@ -3,18 +3,24 @@
 
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
+import java.lang.annotation.Annotation;
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.Method;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Proxy;
 import java.lang.reflect.Type;
 import java.lang.reflect.WildcardType;
-import java.util.HashMap;
+import java.util.LinkedHashMap;
+import java.util.LinkedHashSet;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.Executor;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.inject.Named;
 import javax.inject.Provider;
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
@@ -26,6 +32,8 @@
 import org.apache.http.util.EntityUtils;
 import retrofit.http.HttpProfiler.RequestInformation;
 
+import static retrofit.http.Utils.SynchronousExecutor;
+
 /**
  * Converts Java method calls to Rest calls.
  *
@@ -96,23 +104,36 @@ public <T> T create(Class<T> type) {
   }
 
   private class RestHandler implements InvocationHandler {
-    private final Map<Method, Type> responseTypeCache = new HashMap<Method, Type>();
-
-    @SuppressWarnings("unchecked") @Override
-    public Object invoke(Object proxy, final Method method, final Object[] args) {
-      if (methodWantsSynchronousInvocation(method)) {
-        return invokeRequest(method, args, true);
-      } else {
-        if (httpExecutor == null || callbackExecutor == null) {
-          throw new IllegalStateException("Asynchronous invocation requires calling setExecutors.");
+    final Map<Method, MethodDetails> methodDetailsCache =
+        new LinkedHashMap<Method, MethodDetails>();
+
+    @SuppressWarnings("unchecked")
+    @Override public Object invoke(Object proxy, Method method, final Object[] args) {
+      // Load or create the details cache for the current method.
+      final MethodDetails methodDetails;
+      synchronized (methodDetailsCache) {
+        MethodDetails tempMethodDetails = methodDetailsCache.get(method);
+        if (tempMethodDetails == null) {
+          tempMethodDetails = new MethodDetails(method);
+          methodDetailsCache.put(method, tempMethodDetails);
         }
-        httpExecutor.execute(new CallbackRunnable(obtainCallback(args), callbackExecutor) {
-          @Override public Object obtainResponse() {
-            return invokeRequest(method, args, false);
-          }
-        });
-        return null; // Asynchronous methods should have return type of void.
+        methodDetails = tempMethodDetails;
+      }
+
+      if (methodDetails.isSynchronous) {
+        return invokeRequest(methodDetails, args);
+      }
+
+      if (httpExecutor == null || callbackExecutor == null) {
+        throw new IllegalStateException("Asynchronous invocation requires calling setExecutors.");
       }
+      Callback<?> callback = (Callback<?>) args[args.length - 1];
+      httpExecutor.execute(new CallbackRunnable(callback, callbackExecutor) {
+        @Override public Object obtainResponse() {
+          return invokeRequest(methodDetails, args);
+        }
+      });
+      return null; // Asynchronous methods should have return type of void.
     }
 
     /**
@@ -121,31 +142,27 @@ public Object invoke(Object proxy, final Method method, final Object[] args) {
      * @return HTTP response object of specified {@code type}.
      * @throws RetrofitError Thrown if any error occurs during the HTTP request.
      */
-    private Object invokeRequest(Method method, Object[] args, boolean isSynchronousInvocation) {
+    private Object invokeRequest(MethodDetails methodDetails, Object[] args) {
       long start = System.nanoTime();
+
+      methodDetails.init(); // Ensure all relevant method information has been loaded.
+
       String url = server.apiUrl();
       try {
         // Build the request and headers.
         final HttpUriRequest request = new HttpRequestBuilder(converter) //
-            .setMethod(method, isSynchronousInvocation)
+            .setMethod(methodDetails)
             .setArgs(args)
             .setApiUrl(url)
             .setHeaders(requestHeaders)
             .build();
         url = request.getURI().toString();
 
-        if (!isSynchronousInvocation) {
+        if (!methodDetails.isSynchronous) {
           // If we are executing asynchronously then update the current thread with a useful name.
           Thread.currentThread().setName(THREAD_PREFIX + url);
         }
 
-        // Determine deserialization type by return type or generic parameter to Callback argument.
-        Type type = responseTypeCache.get(method);
-        if (type == null) {
-          type = getResponseObjectType(method, isSynchronousInvocation);
-          responseTypeCache.put(method, type);
-        }
-
         Object profilerObject = null;
         if (profiler != null) {
           profilerObject = profiler.beforeCall();
@@ -158,7 +175,7 @@ private Object invokeRequest(Method method, Object[] args, boolean isSynchronous
 
         long elapsedTime = TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start);
         if (profiler != null) {
-          RequestInformation requestInfo = getRequestInfo(server, method, request);
+          RequestInformation requestInfo = getRequestInfo(server, methodDetails, request);
           profiler.afterCall(requestInfo, elapsedTime, statusCode, profilerObject);
         }
 
@@ -189,6 +206,7 @@ private Object invokeRequest(Method method, Object[] args, boolean isSynchronous
           }
         }
 
+        Type type = methodDetails.type;
         if (statusCode >= 200 && statusCode < 300) { // 2XX == successful request
           try {
             return converter.to(body, type);
@@ -207,6 +225,181 @@ private Object invokeRequest(Method method, Object[] args, boolean isSynchronous
     }
   }
 
+  /** Cached details about an interface method. */
+  static class MethodDetails {
+    private static final Pattern PATH_PARAMETERS = Pattern.compile("\\{([a-z_-]*)\\}");
+
+    final Method method;
+    final boolean isSynchronous;
+
+    private boolean loaded = false;
+
+    Type type;
+    HttpMethodType httpMethod;
+    String path;
+    Set<String> pathParams;
+    QueryParam[] pathQueryParams;
+    String[] pathNamedParams;
+    int singleEntityArgumentIndex = -1;
+
+    MethodDetails(Method method) {
+      this.method = method;
+      isSynchronous = parseResponseType();
+    }
+
+    synchronized void init() {
+      if (loaded) return;
+
+      parseMethodAnnotations();
+      parseParameterAnnotations();
+
+      loaded = true;
+    }
+
+    /** Loads {@link #httpMethod}, {@link #path}, and {@link #pathQueryParams}. */
+    private void parseMethodAnnotations() {
+      for (Annotation annotation : method.getAnnotations()) {
+        Class<? extends Annotation> annotationType = annotation.annotationType();
+
+        // Look for an HttpMethod annotation describing the request type.
+        if (annotationType == GET.class
+            || annotationType == POST.class
+            || annotationType == PUT.class
+            || annotationType == DELETE.class) {
+          if (this.httpMethod != null) {
+            throw new IllegalStateException(
+                "Method annotated with multiple HTTP method annotations: " + method);
+          }
+          this.httpMethod = annotationType.getAnnotation(HttpMethod.class).value();
+          try {
+            path = (String) annotationType.getMethod("value").invoke(annotation);
+          } catch (Exception e) {
+            throw new IllegalStateException("Failed to extract URI path.", e);
+          }
+
+          pathParams = parsePathParameters(path);
+        } else if (annotationType == QueryParams.class) {
+          if (this.pathQueryParams != null) {
+            throw new IllegalStateException(
+                "QueryParam and QueryParams annotations are mutually exclusive.");
+          }
+          this.pathQueryParams = ((QueryParams) annotation).value();
+        } else if (annotationType == QueryParam.class) {
+          if (this.pathQueryParams != null) {
+            throw new IllegalStateException(
+                "QueryParam and QueryParams annotations are mutually exclusive.");
+          }
+          this.pathQueryParams = new QueryParam[] { (QueryParam) annotation };
+        }
+      }
+
+      if (httpMethod == null) {
+        throw new IllegalStateException(
+            "Method not annotated with GET, POST, PUT, or DELETE: " + method);
+      }
+      if (pathQueryParams == null) {
+        pathQueryParams = new QueryParam[0];
+      }
+    }
+
+    /** Loads {@link #type}. Returns true if the method is synchronous. */
+    private boolean parseResponseType() {
+      // Synchronous methods have a non-void return type.
+      Type returnType = method.getGenericReturnType();
+
+      // Asynchronous methods should have a Callback type as the last argument.
+      Type lastArgType = null;
+      Class<?> lastArgClass = null;
+      Type[] parameterTypes = method.getGenericParameterTypes();
+      if (parameterTypes.length > 0) {
+        Type typeToCheck = parameterTypes[parameterTypes.length - 1];
+        lastArgType = typeToCheck;
+        if (typeToCheck instanceof ParameterizedType) {
+          typeToCheck = ((ParameterizedType) typeToCheck).getRawType();
+        }
+        if (typeToCheck instanceof Class) {
+          lastArgClass = (Class<?>) typeToCheck;
+        }
+      }
+
+      boolean hasReturnType = returnType != void.class;
+      boolean hasCallback = lastArgClass != null && Callback.class.isAssignableFrom(lastArgClass);
+
+      // Check for invalid configurations.
+      if (hasReturnType && hasCallback) {
+        throw new IllegalArgumentException(
+            "Method may only have return type or Callback as last argument, not both.");
+      }
+      if (!hasReturnType && !hasCallback) {
+        throw new IllegalArgumentException(
+            "Method must have either a return type or Callback as last argument.");
+      }
+
+      if (hasReturnType) {
+        type = returnType;
+        return true;
+      }
+
+      lastArgType = Utils.getGenericSupertype(lastArgType, lastArgClass, Callback.class);
+      if (lastArgType instanceof ParameterizedType) {
+        Type[] types = ((ParameterizedType) lastArgType).getActualTypeArguments();
+        for (int i = 0; i < types.length; i++) {
+          Type type = types[i];
+          if (type instanceof WildcardType) {
+            types[i] = ((WildcardType) type).getUpperBounds()[0];
+          }
+        }
+        type = types[0];
+        return false;
+      }
+      throw new IllegalArgumentException(
+          String.format("Last parameter of %s must be of type Callback<X> or Callback<? super X>.",
+              method));
+    }
+
+    /** Loads {@link #pathNamedParams} and {@link #singleEntityArgumentIndex}. */
+    private void parseParameterAnnotations() {
+      Annotation[][] parameterAnnotations = method.getParameterAnnotations();
+      int count = parameterAnnotations.length;
+      if (!isSynchronous) {
+        count -= 1; // Callback is last argument when not a synchronous method.
+      }
+
+      String[] namedParams = new String[count];
+      for (int i = 0; i < count; i++) {
+        for (Annotation parameterAnnotation : parameterAnnotations[i]) {
+          Class<? extends Annotation> annotationType = parameterAnnotation.annotationType();
+          if (annotationType == Named.class) {
+            namedParams[i] = ((Named) parameterAnnotation).value();
+          } else if (annotationType == SingleEntity.class) {
+            if (singleEntityArgumentIndex != -1) {
+              throw new IllegalStateException(
+                  "Method annotated with multiple SingleEntity method annotations: " + method);
+            }
+            singleEntityArgumentIndex = i;
+          } else {
+            throw new IllegalArgumentException(
+                "Method argument " + i + " not annotated with Named or SingleEntity: " + method);
+          }
+        }
+      }
+      pathNamedParams = namedParams;
+    }
+
+    /**
+     * Gets the set of unique path parameters used in the given URI. If a parameter is used twice in
+     * the URI, it will only show up once in the set.
+     */
+    static Set<String> parsePathParameters(String path) {
+      Matcher m = PATH_PARAMETERS.matcher(path);
+      Set<String> patterns = new LinkedHashSet<String>();
+      while (m.find()) {
+        patterns.add(m.group(1));
+      }
+      return patterns;
+    }
+  }
+
   private static void logResponseBody(String url, byte[] body, int statusCode, long elapsedTime)
       throws UnsupportedEncodingException {
     LOGGER.fine("---- HTTP " + statusCode + " from " + url + " (" + elapsedTime + "ms)");
@@ -218,14 +411,9 @@ private static void logResponseBody(String url, byte[] body, int statusCode, lon
     LOGGER.fine("---- END HTTP");
   }
 
-  private static Callback<?> obtainCallback(Object[] args) {
-    return (Callback<?>) args[args.length - 1];
-  }
-
-  private static HttpProfiler.RequestInformation getRequestInfo(Server server, Method method,
-      HttpUriRequest request) {
-    RequestLine requestLine = RequestLine.fromMethod(method);
-    HttpMethodType httpMethod = requestLine.getHttpMethod();
+  private static HttpProfiler.RequestInformation getRequestInfo(Server server,
+      MethodDetails methodDetails, HttpUriRequest request) {
+    HttpMethodType httpMethod = methodDetails.httpMethod;
     HttpProfiler.Method profilerMethod = httpMethod.profilerMethod();
 
     long contentLength = 0;
@@ -239,63 +427,8 @@ private static HttpProfiler.RequestInformation getRequestInfo(Server server, Met
       contentType = entityContentType != null ? entityContentType.getValue() : null;
     }
 
-    return new HttpProfiler.RequestInformation(profilerMethod, server.apiUrl(),
-        requestLine.getRelativePath(), contentLength, contentType);
-  }
-
-  /**
-   * Determine whether or not execution for a method should be done synchronously.
-   *
-   * @throws IllegalArgumentException if the supplied {@code method} has both a return type and
-   * {@link Callback} argument or neither of the two.
-   */
-  static boolean methodWantsSynchronousInvocation(Method method) {
-    boolean hasReturnType = method.getReturnType() != void.class;
-
-    Class<?>[] parameterTypes = method.getParameterTypes();
-    boolean hasCallback = parameterTypes.length > 0 && Callback.class.isAssignableFrom(
-        parameterTypes[parameterTypes.length - 1]);
-
-    if ((hasReturnType && hasCallback) || (!hasReturnType && !hasCallback)) {
-      throw new IllegalArgumentException(
-          "Method must have either a return type or Callback as last argument.");
-    }
-    return hasReturnType;
-  }
-
-  /** Get the callback parameter types. */
-  static Type getResponseObjectType(Method method, boolean isSynchronousInvocation) {
-    if (isSynchronousInvocation) {
-      return method.getGenericReturnType();
-    }
-
-    Type[] parameterTypes = method.getGenericParameterTypes();
-    Type callbackType = parameterTypes[parameterTypes.length - 1];
-    Class<?> callbackClass;
-    if (callbackType instanceof Class) {
-      callbackClass = (Class<?>) callbackType;
-    } else if (callbackType instanceof ParameterizedType) {
-      callbackClass = (Class<?>) ((ParameterizedType) callbackType).getRawType();
-    } else {
-      throw new ClassCastException(
-          String.format("Last parameter of %s must be a Class or ParameterizedType", method));
-    }
-    if (Callback.class.isAssignableFrom(callbackClass)) {
-      callbackType = Utils.getGenericSupertype(callbackType, callbackClass, Callback.class);
-      if (callbackType instanceof ParameterizedType) {
-        Type[] types = ((ParameterizedType) callbackType).getActualTypeArguments();
-        for (int i = 0; i < types.length; i++) {
-          Type type = types[i];
-          if (type instanceof WildcardType) {
-            types[i] = ((WildcardType) type).getUpperBounds()[0];
-          }
-        }
-        return types[0];
-      }
-    }
-    throw new IllegalArgumentException(String.format(
-        "Last parameter of %s must be of type Callback<X,Y,Z> or Callback<? super X,..,..>.",
-        method));
+    return new HttpProfiler.RequestInformation(profilerMethod, server.apiUrl(), methodDetails.path,
+        contentLength, contentType);
   }
 
   /**
@@ -406,10 +539,4 @@ private void ensureSaneDefaults() {
       }
     }
   }
-
-  static class SynchronousExecutor implements Executor {
-    @Override public void execute(Runnable runnable) {
-      runnable.run();
-    }
-  }
 }
\ No newline at end of file
diff --git a/retrofit/src/main/java/retrofit/http/Utils.java b/retrofit/src/main/java/retrofit/http/Utils.java
index 60cc067931..2c01ac816f 100644
--- a/retrofit/src/main/java/retrofit/http/Utils.java
+++ b/retrofit/src/main/java/retrofit/http/Utils.java
@@ -2,6 +2,7 @@
 package retrofit.http;
 
 import java.lang.reflect.Type;
+import java.util.concurrent.Executor;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -58,4 +59,10 @@ static String parseCharset(String headerValue) {
     }
     return UTF_8;
   }
+
+  static class SynchronousExecutor implements Executor {
+    @Override public void execute(Runnable runnable) {
+      runnable.run();
+    }
+  }
 }
\ No newline at end of file


Test patch:
diff --git a/retrofit/src/test/java/retrofit/http/HttpRequestBuilderTest.java b/retrofit/src/test/java/retrofit/http/HttpRequestBuilderTest.java
index 59077f472b..b13e752130 100644
--- a/retrofit/src/test/java/retrofit/http/HttpRequestBuilderTest.java
+++ b/retrofit/src/test/java/retrofit/http/HttpRequestBuilderTest.java
@@ -6,7 +6,6 @@
 import java.lang.reflect.Method;
 import java.net.URISyntaxException;
 import java.net.URLEncoder;
-import java.util.Set;
 import java.util.UUID;
 import javax.inject.Named;
 import org.apache.http.client.methods.HttpGet;
@@ -16,32 +15,13 @@
 
 import static org.fest.assertions.api.Assertions.assertThat;
 import static org.fest.assertions.api.Fail.fail;
+import static retrofit.http.RestAdapter.MethodDetails;
 
 /** @author Eric Denman (edenman@squareup.com) */
 public class HttpRequestBuilderTest {
   private static final Gson GSON = new Gson();
   private static final String API_URL = "http://taqueria.com/lengua/taco";
 
-  @Test public void testRegex() throws Exception {
-    expectParams("");
-    expectParams("foo");
-    expectParams("foo/bar");
-    expectParams("foo/bar/{taco}", "taco");
-    expectParams("foo/bar/{t}", "t");
-    expectParams("foo/bar/{taco}/or/{burrito}", "taco", "burrito");
-    expectParams("foo/bar/{taco}/or/{taco}", "taco");
-    expectParams("foo/bar/{taco-shell}", "taco-shell");
-    expectParams("foo/bar/{taco_shell}", "taco_shell");
-  }
-
-  private void expectParams(String path, String... expected) {
-    Set<String> calculated = HttpRequestBuilder.getPathParameters(path);
-    assertThat(calculated.size()).isEqualTo(expected.length);
-    for (String val : expected) {
-      assertThat(calculated).contains(val);
-    }
-  }
-
   @Test public void testNormalGet() throws Exception {
     Method method = getTestMethod("normalGet");
     String expectedId = UUID.randomUUID().toString();
@@ -198,8 +178,10 @@ private static Method getTestMethod(String name) {
   }
 
   private HttpUriRequest build(Method method, Object[] args) throws URISyntaxException {
+    MethodDetails methodDetails = new MethodDetails(method);
+    methodDetails.init();
     return new HttpRequestBuilder(new GsonConverter(GSON)) //
-        .setMethod(method, false) //
+        .setMethod(methodDetails) //
         .setArgs(args) //
         .setApiUrl(API_URL) //
         .build();
diff --git a/retrofit/src/test/java/retrofit/http/RestAdapterTest.java b/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
index 2b7c86a708..068ec3da3b 100644
--- a/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/http/RestAdapterTest.java
@@ -3,6 +3,7 @@
 
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
+import java.util.Set;
 import org.apache.http.HttpMessage;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpStatus;
@@ -38,6 +39,7 @@
 import static org.easymock.EasyMock.verify;
 import static org.fest.assertions.api.Assertions.assertThat;
 import static org.fest.assertions.api.Assertions.fail;
+import static retrofit.http.RestAdapter.MethodDetails;
 
 public class RestAdapterTest {
   private static final String ID = "123";
@@ -75,6 +77,26 @@ public class RestAdapterTest {
         .build();
   }
 
+  @Test public void testRegex() throws Exception {
+    expectParams("");
+    expectParams("foo");
+    expectParams("foo/bar");
+    expectParams("foo/bar/{taco}", "taco");
+    expectParams("foo/bar/{t}", "t");
+    expectParams("foo/bar/{taco}/or/{burrito}", "taco", "burrito");
+    expectParams("foo/bar/{taco}/or/{taco}", "taco");
+    expectParams("foo/bar/{taco-shell}", "taco-shell");
+    expectParams("foo/bar/{taco_shell}", "taco_shell");
+  }
+
+  private void expectParams(String path, String... expected) {
+    Set<String> calculated = MethodDetails.parsePathParameters(path);
+    assertThat(calculated.size()).isEqualTo(expected.length);
+    for (String val : expected) {
+      assertThat(calculated).contains(val);
+    }
+  }
+
   @Test public void testServiceDeleteSimpleAsync() throws IOException {
     expectAsyncLifecycle(HttpDelete.class, GET_DELETE_SIMPLE_URL);
     replayAll();
@@ -433,78 +455,94 @@ public class RestAdapterTest {
 
   @Test public void testConcreteCallbackTypes() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("a");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("a").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("a"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testConcreteCallbackTypesWithParams() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("b");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("b").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("b"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testGenericCallbackTypes() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("c");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("c").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("c"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testGenericCallbackTypesWithParams() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("d");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("d").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("d"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testWildcardGenericCallbackTypes() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("e");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("e").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("e"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testGenericCallbackWithGenericType() {
     Type expected = new TypeToken<List<String>>() {}.getType();
-    Method method = getTypeTestMethod("f");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("f").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("f"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Ignore // TODO support this case!
   @Test public void testExtendingGenericCallback() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("g");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    assertThat(RestAdapter.getResponseObjectType(method, false)).as("g").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("g"));
+    method.init();
+    assertThat(method.isSynchronous).isFalse();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test(expected = IllegalArgumentException.class)
   public void testMissingCallbackTypes() {
-    Method method = getTypeTestMethod("h");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isFalse();
-    RestAdapter.getResponseObjectType(method, false);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("h"));
+    assertThat(method.isSynchronous).isFalse();
+    method.init();
   }
 
   @Test public void testSynchronousResponse() {
     Type expected = Response.class;
-    Method method = getTypeTestMethod("x");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isTrue();
-    assertThat(RestAdapter.getResponseObjectType(method, true)).as("x").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("x"));
+    method.init();
+    assertThat(method.isSynchronous).isTrue();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test public void testSynchronousGenericResponse() {
     Type expected = new TypeToken<List<String>>() {}.getType();
-    Method method = getTypeTestMethod("y");
-    assertThat(RestAdapter.methodWantsSynchronousInvocation(method)).isTrue();
-    assertThat(RestAdapter.getResponseObjectType(method, true)).as("y").isEqualTo(expected);
+    MethodDetails method = new MethodDetails(getTypeTestMethod("y"));
+    method.init();
+    assertThat(method.isSynchronous).isTrue();
+    assertThat(method.type).as("a").isEqualTo(expected);
   }
 
   @Test(expected = IllegalArgumentException.class)
   public void testSynchronousWithAsyncCallback() {
-    RestAdapter.methodWantsSynchronousInvocation(getTypeTestMethod("z"));
+    MethodDetails method = new MethodDetails(getTypeTestMethod("z"));
+    method.init();
+  }
+
+  @Ignore // TODO Issue #130
+  @Test public void testNonEndpointMethodsSucceed() {
+    TypeTestService service = restAdapter.create(TypeTestService.class);
+    assertThat(service.equals(new Object())).isFalse();
   }
 
   private void replayAll() {