Repository: square/retrofit
Issue: Generic API interfaces

Hello guys,

First of all, thank you for this amazing project. It saves a lot of coding for me!

Now, I came across a small problem. 

My API is simple and virtually all trafficked objects are similar. Therefore all have the same parent class. However, I can not create an API interface based on that parent class, or using generics. For example:

``` java
public class BaseModel {
    // ...
}

public class TodoItem extends BaseModel {
    // ...
}

// ------------

public interface SyncApi {
    @GET("/some/url/with/{id}")
    TodoItem get(@Path("id") long id);  // This works well :)

    @GET("/some/url/with/{id}")
    BaseModel get(@Path("id") long id);  // This fail because GsonConverter does not have an known de/serializer for BaseModel

    @GET("/some/url/with/{id}")
    <T extends BaseModel> T get(@Path("id") long id); // This return a LinkedTreeMap from Gson
}

// ---------

public interface SyncApi<T extends BaseModel> {
    @GET("/some/url/with/{id}")
    T get(@Path("id") long id);
}

public interface TodoItemApi extends SyncApi<TodoItem> {
    // This raises exception from Retrofit validation about heritance
}
```

Have you some suggestion?


Issue comments:
How many subclasses would you have out of curiosity?

Jake, for now, is a proof-of-concept app, with one class. I have a previous dificulty with data synchronization on Android, so, I'm building a kind of "kit" with bootstrap for sync data.

My goal is create a kind of "plug / implements models / call sync / all done" lib.

So, the number of classes can grow.

I containerized retrofit, gson and RabbitMQ on that lib, to avoid installing all on app again.

I think that is possible to implement all I want, if the app provides one "wrapper" to API using retrofit, but I prefer to don't implement these way if possible.

I'm in a similar situation. A SyncAdapter syncing 18 tables, each represented by an entity with a common base class. I thought this might be due to Java's type erasure with generics. Many mostly-duplicate lines of code could be replaced by one API call if we can somehow support generics in Retrofit API interfaces, any new thoughts on this?

EDIT: Digging into the source I see how problematic this would be, and goes against explicit API contracts, so perhaps it's better to just write more code in this case.


Pull request review comments:
Not a type variable


Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/MethodInfo.java b/retrofit/src/main/java/retrofit/MethodInfo.java
index d766030e1a..ff50e6cf42 100644
--- a/retrofit/src/main/java/retrofit/MethodInfo.java
+++ b/retrofit/src/main/java/retrofit/MethodInfo.java
@@ -236,6 +236,9 @@ private void parseResponseType() {
           + factories);
     }
     Type responseType = adapter.responseType();
+    if (Utils.hasTypeVariable(responseType)) {
+      throw methodError("Method response type must not include a type variable.");
+    }
     if (converter == null && responseType != ResponseBody.class) {
       throw methodError("Method response type is "
           + responseType
diff --git a/retrofit/src/main/java/retrofit/Utils.java b/retrofit/src/main/java/retrofit/Utils.java
index 6bec958783..8c8d1c33bc 100644
--- a/retrofit/src/main/java/retrofit/Utils.java
+++ b/retrofit/src/main/java/retrofit/Utils.java
@@ -81,6 +81,33 @@ public static Type getSingleParameterUpperBound(ParameterizedType type) {
     return paramType;
   }
 
+  public static boolean hasTypeVariable(Type type) {
+    if (type instanceof Class<?>) {
+      return false;
+    }
+    if (type instanceof ParameterizedType) {
+      ParameterizedType parameterizedType = (ParameterizedType) type;
+      for (Type typeArgument : parameterizedType.getActualTypeArguments()) {
+        if (hasTypeVariable(typeArgument)) {
+          return true;
+        }
+      }
+      return false;
+    }
+    if (type instanceof GenericArrayType) {
+      return hasTypeVariable(((GenericArrayType) type).getGenericComponentType());
+    }
+    if (type instanceof TypeVariable) {
+      return true;
+    }
+    if (type instanceof WildcardType) {
+      return true;
+    }
+    String className = type == null ? "null" : type.getClass().getName();
+    throw new IllegalArgumentException("Expected a Class, ParameterizedType, or "
+        + "GenericArrayType, but <" + type + "> is of type " + className);
+  }
+
   // This method is copyright 2008 Google Inc. and is taken from Gson under the Apache 2.0 license.
   public static Class<?> getRawType(Type type) {
     if (type instanceof Class<?>) {


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RestAdapterTest.java b/retrofit/src/test/java/retrofit/RestAdapterTest.java
index 07bbca54fd..fa4aedb6a4 100644
--- a/retrofit/src/test/java/retrofit/RestAdapterTest.java
+++ b/retrofit/src/test/java/retrofit/RestAdapterTest.java
@@ -9,6 +9,9 @@
 import java.io.IOException;
 import java.lang.reflect.ParameterizedType;
 import java.lang.reflect.Type;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.junit.Rule;
@@ -34,6 +37,14 @@ interface FutureMethod {
   }
   interface Extending extends CallMethod {
   }
+  interface StringService {
+    @GET("/") String get();
+  }
+  interface BoundsService {
+    @GET("/") <T> Call<T> none();
+    @GET("/") <T extends ResponseBody> Call<T> upper();
+    @GET("/") <T> Call<List<Map<String, Set<T[]>>>> crazy();
+  }
 
   @SuppressWarnings("EqualsBetweenInconvertibleTypes") // We are explicitly testing this behavior.
   @Test public void objectMethodsStillWork() {
@@ -99,10 +110,6 @@ class MyCallAdapterFactory implements CallAdapter.Factory {
     assertThat(adapterCalled.get()).isTrue();
   }
 
-  interface StringService {
-    @GET("/") String get();
-  }
-
   @Test public void customReturnTypeAdapter() {
     class GreetingCallAdapterFactory implements CallAdapter.Factory {
       @Override public CallAdapter<?> get(Type returnType) {
@@ -154,7 +161,9 @@ class GreetingCallAdapterFactory implements CallAdapter.Factory {
       example.disallowed("Hi!");
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("CallMethod.disallowed: @Body parameter is class java.lang.String but no converter registered. Either add a converter to the RestAdapter or use RequestBody. (parameter #1)");
+      assertThat(e).hasMessage(
+          "CallMethod.disallowed: @Body parameter is class java.lang.String but no converter registered. "
+              + "Either add a converter to the RestAdapter or use RequestBody. (parameter #1)");
     }
   }
 
@@ -170,7 +179,9 @@ class GreetingCallAdapterFactory implements CallAdapter.Factory {
       example.disallowed();
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("CallMethod.disallowed: Method response type is class java.lang.String but no converter registered. Either add a converter to the RestAdapter or use ResponseBody.");
+      assertThat(e).hasMessage(
+          "CallMethod.disallowed: Method response type is class java.lang.String but no converter registered. "
+              + "Either add a converter to the RestAdapter or use ResponseBody.");
     }
   }
 
@@ -200,4 +211,52 @@ class GreetingCallAdapterFactory implements CallAdapter.Factory {
 
     assertThat(server.takeRequest().getBody().readUtf8()).isEqualTo("Hey");
   }
+
+  @Test public void typeVariableNoBoundThrows() {
+    RestAdapter ra = new RestAdapter.Builder()
+        .endpoint(server.getUrl("/").toString())
+        .converter(new StringConverter())
+        .build();
+    BoundsService example = ra.create(BoundsService.class);
+
+    try {
+      example.none();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "BoundsService.none: Method response type must not include a type variable.");
+    }
+  }
+
+  @Test public void typeVariableUpperBoundThrows() {
+    RestAdapter ra = new RestAdapter.Builder()
+        .endpoint(server.getUrl("/").toString())
+        .converter(new StringConverter())
+        .build();
+    BoundsService example = ra.create(BoundsService.class);
+
+    try {
+      example.upper();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "BoundsService.upper: Method response type must not include a type variable.");
+    }
+  }
+
+  @Test public void typeVariableNestedThrows() {
+    RestAdapter ra = new RestAdapter.Builder()
+        .endpoint(server.getUrl("/").toString())
+        .converter(new StringConverter())
+        .build();
+    BoundsService example = ra.create(BoundsService.class);
+
+    try {
+      example.crazy();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "BoundsService.crazy: Method response type must not include a type variable.");
+    }
+  }
 }