Merge pull request square#1009 from square/jw/multiple-converters-and…

…-call-adapters

Allow multiple factories for converters and call adapters.

retrofit-adapters/rxjava/src/test/java/retrofit/ObservableCallAdapterFactoryTest.java

@@ -49,8 +49,8 @@ interface Service {
   @Before public void setUp() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(new StringConverterFactory())
-        .callAdapterFactory(ObservableCallAdapterFactory.create())
+        .addConverterFactory(new StringConverterFactory())
+        .addCallAdapterFactory(ObservableCallAdapterFactory.create())
         .build();
     service = retrofit.create(Service.class);
   }

retrofit-converters/gson/src/main/java/retrofit/GsonConverterFactory.java

@@ -45,6 +45,7 @@ private GsonConverterFactory(Gson gson) {
     this.gson = gson;
   }
 
+  /** Create a converter for {@code type}. */
   @Override public Converter<?> get(Type type) {
     TypeAdapter<?> adapter = gson.getAdapter(TypeToken.get(type));
     return new GsonConverter<>(adapter);

retrofit-converters/gson/src/test/java/retrofit/GsonConverterFactoryTest.java

@@ -89,7 +89,7 @@ interface Service {
         .create();
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(GsonConverterFactory.create(gson))
+        .addConverterFactory(GsonConverterFactory.create(gson))
         .build();
     service = retrofit.create(Service.class);
   }

retrofit-converters/jackson/src/main/java/retrofit/JacksonConverterFactory.java

@@ -40,6 +40,7 @@ private JacksonConverterFactory(ObjectMapper mapper) {
     this.mapper = mapper;
   }
 
+  /** Create a converter for {@code type}. */
   @Override public Converter<?> get(Type type) {
     JavaType javaType = mapper.getTypeFactory().constructType(type);
     ObjectWriter writer = mapper.writerWithType(javaType);

retrofit-converters/jackson/src/test/java/retrofit/JacksonConverterTest.java → retrofit-converters/jackson/src/test/java/retrofit/JacksonConverterFactoryTest.java

@@ -38,7 +38,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public class JacksonConverterTest {
+public class JacksonConverterFactoryTest {
   interface AnInterface {
     String getName();
   }
@@ -121,7 +121,7 @@ interface Service {
 
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(JacksonConverterFactory.create(mapper))
+        .addConverterFactory(JacksonConverterFactory.create(mapper))
         .build();
     service = retrofit.create(Service.class);
   }

retrofit-converters/moshi/src/main/java/retrofit/MoshiConverterFactory.java

@@ -38,6 +38,7 @@ private MoshiConverterFactory(Moshi moshi) {
     this.moshi = moshi;
   }
 
+  /** Create a converter for {@code type}. */
   @Override public Converter<?> get(Type type) {
     JsonAdapter<Object> adapter = moshi.adapter(type);
     return new MoshiConverter<>(adapter);

retrofit-converters/moshi/src/test/java/retrofit/MoshiConverterTest.java → retrofit-converters/moshi/src/test/java/retrofit/MoshiConverterFactoryTest.java

@@ -32,7 +32,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
-public final class MoshiConverterTest {
+public final class MoshiConverterFactoryTest {
   interface AnInterface {
     String getName();
   }
@@ -88,7 +88,7 @@ interface Service {
         .build();
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(MoshiConverterFactory.create(moshi))
+        .addConverterFactory(MoshiConverterFactory.create(moshi))
         .build();
     service = retrofit.create(Service.class);
   }

retrofit-converters/protobuf/src/main/java/retrofit/ProtoConverterFactory.java

@@ -26,13 +26,18 @@ public static ProtoConverterFactory create() {
     return new ProtoConverterFactory();
   }
 
+
+  /**
+   * Create a converter for {@code type} provided it is a {@link MessageLite} type. Returns null
+   * otherwise.
+   */
   @Override public Converter<?> get(Type type) {
     if (!(type instanceof Class<?>)) {
-      throw new IllegalArgumentException("Expected a raw Class<?> but was " + type);
+      return null;
     }
     Class<?> c = (Class<?>) type;
     if (!MessageLite.class.isAssignableFrom(c)) {
-      throw new IllegalArgumentException("Expected a protobuf message but was " + c.getName());
+      return null;
     }
 
     Parser<MessageLite> parser;
@@ -42,7 +47,7 @@ public static ProtoConverterFactory create() {
       parser = (Parser<MessageLite>) field.get(null);
     } catch (NoSuchFieldException | IllegalAccessException e) {
       throw new IllegalArgumentException(
-          "Expected a protobuf message but " + c.getName() + " had no PARSER field.");
+          "Found a protobuf message but " + c.getName() + " had no PARSER field.");
     }
 
     return new ProtoConverter<>(parser);

retrofit-converters/protobuf/src/test/java/retrofit/ProtoConverterTest.java → retrofit-converters/protobuf/src/test/java/retrofit/ProtoConverterFactoryTest.java

@@ -34,7 +34,7 @@
 import static org.junit.Assert.fail;
 import static retrofit.PhoneProtos.Phone;
 
-public final class ProtoConverterTest {
+public final class ProtoConverterFactoryTest {
   interface Service {
     @GET("/") Call<Phone> get();
     @POST("/") Call<Phone> post(@Body Phone impl);
@@ -49,7 +49,7 @@ interface Service {
   @Before public void setUp() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(ProtoConverterFactory.create())
+        .addConverterFactory(ProtoConverterFactory.create())
         .build();
     service = retrofit.create(Service.class);
   }
@@ -85,7 +85,12 @@ interface Service {
       service.wrongClass();
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Expected a protobuf message but was java.lang.String");
+      assertThat(e).hasMessage("Unable to create converter for class java.lang.String\n"
+          + "    for method Service.wrongClass");
+      assertThat(e.getCause()).hasMessage(
+          "Could not locate converter for class java.lang.String. Tried:\n"
+              + " * retrofit.ProtoConverterFactory\n"
+              + " * retrofit.OkHttpBodyConverterFactory");
     }
   }
 
@@ -97,7 +102,12 @@ interface Service {
       service.wrongType();
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Expected a raw Class<?> but was java.util.List<java.lang.String>");
+      assertThat(e).hasMessage("Unable to create converter for java.util.List<java.lang.String>\n"
+          + "    for method Service.wrongType");
+      assertThat(e.getCause()).hasMessage(
+          "Could not locate converter for java.util.List<java.lang.String>. Tried:\n"
+              + " * retrofit.ProtoConverterFactory\n"
+              + " * retrofit.OkHttpBodyConverterFactory");
     }
   }
 

retrofit-converters/simplexml/src/main/java/retrofit/SimpleXmlConverterFactory.java

@@ -54,9 +54,10 @@ public boolean isStrict() {
     return strict;
   }
 
+  /** Create a converter for {@code type} provided it is a class. Returns null otherwise. */
   @Override public Converter<?> get(Type type) {
     if (!(type instanceof Class)) {
-      throw new IllegalArgumentException("Expected a raw class but was " + type);
+      return null;
     }
     Class<?> cls = (Class<?>) type;
     return new SimpleXmlConverter<>(cls, serializer, strict);

retrofit-converters/simplexml/src/test/java/retrofit/SimpleXmlConverterTest.java → retrofit-converters/simplexml/src/test/java/retrofit/SimpleXmlConverterFactoryTest.java

@@ -34,7 +34,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 
-public class SimpleXmlConverterTest {
+public class SimpleXmlConverterFactoryTest {
   interface Service {
     @GET("/") Call<MyObject> get();
     @POST("/") Call<MyObject> post(@Body MyObject impl);
@@ -50,7 +50,7 @@ interface Service {
     Persister persister = new Persister(format);
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(SimpleXmlConverterFactory.create(persister))
+        .addConverterFactory(SimpleXmlConverterFactory.create(persister))
         .build();
     service = retrofit.create(Service.class);
   }

retrofit-converters/wire/src/main/java/retrofit/WireConverterFactory.java

@@ -39,13 +39,17 @@ private WireConverterFactory(Wire wire) {
     this.wire = wire;
   }
 
+  /**
+   * Create a converter for {@code type} provided it is a {@link Message} type. Returns null
+   * otherwise.
+   */
   @Override public Converter<?> get(Type type) {
     if (!(type instanceof Class<?>)) {
-      throw new IllegalArgumentException("Expected a raw Class<?> but was " + type);
+      return null;
     }
     Class<?> c = (Class<?>) type;
     if (!Message.class.isAssignableFrom(c)) {
-      throw new IllegalArgumentException("Expected a proto message but was " + c.getName());
+      return null;
     }
     //noinspection unchecked
     return new WireConverter<>(wire, (Class<Message>) c);

retrofit-converters/wire/src/test/java/retrofit/WireConverterTest.java → retrofit-converters/wire/src/test/java/retrofit/WireConverterFactoryTest.java

@@ -33,7 +33,7 @@
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.fail;
 
-public final class WireConverterTest {
+public final class WireConverterFactoryTest {
   interface Service {
     @GET("/") Call<Phone> get();
     @POST("/") Call<Phone> post(@Body Phone impl);
@@ -48,7 +48,7 @@ interface Service {
   @Before public void setUp() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
-        .converterFactory(WireConverterFactory.create())
+        .addConverterFactory(WireConverterFactory.create())
         .build();
     service = retrofit.create(Service.class);
   }
@@ -84,7 +84,12 @@ interface Service {
       service.wrongClass();
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Expected a proto message but was java.lang.String");
+      assertThat(e).hasMessage("Unable to create converter for class java.lang.String\n"
+          + "    for method Service.wrongClass");
+      assertThat(e.getCause()).hasMessage(
+          "Could not locate converter for class java.lang.String. Tried:\n"
+              + " * retrofit.WireConverterFactory\n"
+              + " * retrofit.OkHttpBodyConverterFactory");
     }
   }
 
@@ -96,7 +101,12 @@ interface Service {
       service.wrongType();
       fail();
     } catch (IllegalArgumentException e) {
-      assertThat(e).hasMessage("Expected a raw Class<?> but was java.util.List<java.lang.String>");
+      assertThat(e).hasMessage("Unable to create converter for java.util.List<java.lang.String>\n"
+          + "    for method Service.wrongType");
+      assertThat(e.getCause()).hasMessage(
+          "Could not locate converter for java.util.List<java.lang.String>. Tried:\n"
+              + " * retrofit.WireConverterFactory\n"
+              + " * retrofit.OkHttpBodyConverterFactory");
     }
   }
 

retrofit/src/main/java/retrofit/Converter.java

@@ -29,6 +29,7 @@ public interface Converter<T> {
   RequestBody toBody(T value);
 
   interface Factory {
+    /** Create a converter for {@code type}. Returns null if the type cannot be handled. */
     Converter<?> get(Type type);
   }
 }

retrofit/src/main/java/retrofit/DefaultCallAdapter.java

@@ -31,10 +31,6 @@ final class DefaultCallAdapter<T> implements CallAdapter<T> {
       Type responseType = Utils.getCallResponseType(returnType);
       return new DefaultCallAdapter<>(responseType);
     }
-
-    @Override public String toString() {
-      return "Default";
-    }
   };
 
   private final Type responseType;

retrofit/src/main/java/retrofit/MethodHandler.java

@@ -19,63 +19,53 @@
 import com.squareup.okhttp.ResponseBody;
 import java.lang.reflect.Method;
 import java.lang.reflect.Type;
+import java.util.List;
 import retrofit.http.Streaming;
 
-import static retrofit.Utils.methodError;
-
 final class MethodHandler<T> {
   @SuppressWarnings("unchecked")
   static MethodHandler<?> create(Method method, OkHttpClient client, BaseUrl baseUrl,
-      CallAdapter.Factory callAdapterFactory, Converter.Factory converterFactory) {
+      List<CallAdapter.Factory> callAdapterFactories, List<Converter.Factory> converterFactories) {
     CallAdapter<Object> callAdapter =
-        (CallAdapter<Object>) createCallAdapter(method, callAdapterFactory);
+        (CallAdapter<Object>) createCallAdapter(method, callAdapterFactories);
     Converter<Object> responseConverter =
         (Converter<Object>) createResponseConverter(method, callAdapter.responseType(),
-            converterFactory);
-    RequestFactory requestFactory = RequestFactoryParser.parse(method, baseUrl, converterFactory);
+            converterFactories);
+    RequestFactory requestFactory = RequestFactoryParser.parse(method, baseUrl, converterFactories);
     return new MethodHandler<>(client, requestFactory, callAdapter, responseConverter);
   }
 
   private static CallAdapter<?> createCallAdapter(Method method,
-      CallAdapter.Factory adapterFactory) {
+      List<CallAdapter.Factory> adapterFactories) {
     Type returnType = method.getGenericReturnType();
     if (Utils.hasUnresolvableType(returnType)) {
-      throw methodError(method,
+      throw Utils.methodError(method,
           "Method return type must not include a type variable or wildcard: %s", returnType);
     }
-
     if (returnType == void.class) {
-      throw methodError(method, "Service methods cannot return void.");
+      throw Utils.methodError(method, "Service methods cannot return void.");
     }
-
-    CallAdapter<?> adapter = adapterFactory.get(returnType);
-    if (adapter == null) {
-      throw methodError(method, "Call adapter factory '%s' was unable to handle return type %s",
-          adapterFactory, returnType);
+    try {
+      return Utils.resolveCallAdapter(adapterFactories, returnType);
+    } catch (RuntimeException e) { // Wide exception range because factories are user code.
+      throw Utils.methodError(e, method, "Unable to create call adapter for %s", returnType);
     }
-    return adapter;
+
   }
 
   private static Converter<?> createResponseConverter(Method method, Type responseType,
-      Converter.Factory converterFactory) {
+      List<Converter.Factory> converterFactories) {
+    // TODO how can we not special case this? See TODO below, maybe...
     if (responseType == ResponseBody.class) {
       boolean isStreaming = method.isAnnotationPresent(Streaming.class);
       return new OkHttpResponseBodyConverter(isStreaming);
     }
 
-    if (converterFactory == null) {
-      throw methodError(method, "Method response type is "
-          + responseType
-          + " but no converter factory registered. "
-          + "Either add a converter factory to the Retrofit instance or use ResponseBody.");
-    }
-
-    Converter<?> converter = converterFactory.get(responseType);
-    if (converter == null) {
-      throw methodError(method, "Converter factory '%s' was unable to handle response type %s",
-          converterFactory, responseType);
+    try {
+      return Utils.resolveConverter(converterFactories, responseType);
+    } catch (RuntimeException e) { // Wide exception range because factories are user code.
+      throw Utils.methodError(e, method, "Unable to create converter for %s", responseType);
     }
-    return converter;
   }
 
   private final OkHttpClient client;

retrofit/src/main/java/retrofit/OkHttpBodyConverterFactory.java

@@ -0,0 +1,36 @@
+/*
+ * Copyright (C) 2015 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package retrofit;
+
+import com.squareup.okhttp.RequestBody;
+import com.squareup.okhttp.ResponseBody;
+import java.lang.reflect.Type;
+
+final class OkHttpBodyConverterFactory implements Converter.Factory {
+  @Override public Converter<?> get(Type type) {
+    if (!(type instanceof Class)) {
+      return null;
+    }
+    Class<?> cls = (Class<?>) type;
+    if (RequestBody.class.isAssignableFrom(cls)) {
+      return new OkHttpRequestBodyConverter();
+    }
+    if (ResponseBody.class.isAssignableFrom(cls)) {
+      return new OkHttpResponseBodyConverter(false);
+    }
+    return null;
+  }
+}