Add Retrofit.Builder.converterFactories(). (#2476)

* Add Retrofit.Builder.converterFactories().

* Don't shift N converter factories.

* Assert clone build exposes no interceptors.

* Remove implementation detail test for when converters are added.

* Ensure void response body handled by built-in converters.

* Remove builtInConvertersRemainFirstInClone.

* Test built in converters remain first in clone.

retrofit/src/main/java/retrofit2/Retrofit.java

@@ -242,7 +242,7 @@ public final class Retrofit {
   }
 
   /**
-   * Returns a list of the factories tried when creating a
+   * Returns an unmodifiable list of the factories tried when creating a
    * {@linkplain #requestBodyConverter(Type, Annotation[], Annotation[]) request body converter}, a
    * {@linkplain #responseBodyConverter(Type, Annotation[]) response body converter}, or a
    * {@linkplain #stringConverter(Type, Annotation[]) string converter}.
@@ -402,9 +402,6 @@ public final class Retrofit {
 
     Builder(Platform platform) {
       this.platform = platform;
-      // Add the built-in converter factory first. This prevents overriding its behavior but also
-      // ensures correct behavior when using converters that consume all types.
-      converterFactories.add(new BuiltInConverters());
     }
 
     public Builder() {
@@ -416,6 +413,8 @@ public final class Retrofit {
       callFactory = retrofit.callFactory;
       baseUrl = retrofit.baseUrl;
       converterFactories.addAll(retrofit.converterFactories);
+      // BuiltInConverters instance added by build().
+      converterFactories.remove(0);
       adapterFactories.addAll(retrofit.adapterFactories);
       // Remove the default, platform-aware call adapter added by build().
       adapterFactories.remove(adapterFactories.size() - 1);
@@ -543,6 +542,11 @@ public final class Retrofit {
       return this;
     }
 
+    /** Returns a modifiable list of converter factories. */
+    public List<Converter.Factory> converterFactories() {
+      return this.converterFactories;
+    }
+
     /**
      * When calling {@link #create} on the resulting {@link Retrofit} instance, eagerly validate
      * the configuration of all methods in the supplied interface.
@@ -578,7 +582,12 @@ public final class Retrofit {
       adapterFactories.add(platform.defaultCallAdapterFactory(callbackExecutor));
 
       // Make a defensive copy of the converters.
-      List<Converter.Factory> converterFactories = new ArrayList<>(this.converterFactories);
+      List<Converter.Factory> converterFactories = new ArrayList<>();
+
+      // Add the built-in converter factory first. This prevents overriding its behavior but also
+      // ensures correct behavior when using converters that consume all types.
+      converterFactories.add(new BuiltInConverters());
+      converterFactories.addAll(this.converterFactories);
 
       return new Retrofit(callFactory, baseUrl, converterFactories, adapterFactories,
           callbackExecutor, validateEagerly);

retrofit/src/test/java/retrofit2/RetrofitTest.java

@@ -63,6 +63,7 @@ import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.verifyZeroInteractions;
 
 public final class RetrofitTest {
   @Rule public final MockWebServer server = new MockWebServer();
@@ -171,6 +172,14 @@ public final class RetrofitTest {
     assertSame(callFactory, two.callFactory());
   }
 
+  @Test public void builtInConvertersAbsentInCloneBuilder() {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .build();
+
+    assertEquals(0, retrofit.newBuilder().converterFactories().size());
+  }
+
   @Test public void responseTypeCannotBeRetrofitResponse() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))
@@ -839,6 +848,23 @@ public final class RetrofitTest {
     assertThat(converterFactories.get(0)).isInstanceOf(BuiltInConverters.class);
   }
 
+  @Test public void builtInConvertersFirstInClone() {
+    Converter<ResponseBody, Void> converter = mock(Converter.class);
+    Converter.Factory factory = mock(Converter.Factory.class);
+    Annotation[] annotations = new Annotation[0];
+
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .addConverterFactory(factory)
+        .build();
+
+    doReturn(converter).when(factory).responseBodyConverter(Void.class, annotations, retrofit);
+
+    retrofit.newBuilder().build().responseBodyConverter(Void.class, annotations);
+
+    verifyZeroInteractions(factory);
+  }
+
   @Test public void requestConverterFactoryQueried() {
     Type type = String.class;
     Annotation[] parameterAnnotations = new Annotation[0];