Repository: square/retrofit
Issue: [Request] Add "newBuilder" to Retrofit class

Can we add `newBuilder` builder similar to Okhttp3 (see [here](https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/OkHttpClient.java#L390)) to `Retrofit.Builder`?

I would like to change the URL of an already created builder.


Issue comments:
Why do you need it?

On Wed, Oct 19, 2016, 8:22 PM Jared Burrows notifications@github.com
wrote:

> Can we add newBuilder builder similar to Okhttp3 (see here
> https://github.com/square/okhttp/blob/master/okhttp/src/main/java/okhttp3/OkHttpClient.java#L390)
> to Retrofit.Builder?
> 
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> https://github.com/square/retrofit/issues/2054, or mute the thread
> https://github.com/notifications/unsubscribe-auth/AAEEEXlDnSQ3DS4w7Zeo3ts1VrbzYEFNks5q1rRSgaJpZM4KbmNw
> .

The same reason it is availble in OkHttp3.

For instance:

```
// Some Client
OkHttpClient client = new OkHttpClient.Builder()
  .addInterceptor(new HttpLoggingInterceptor()
    .setLevel(HttpLoggingInterceptor.Level.BODY))
  .cache(cache)
  .build();

// Some client 2
OkHttpClient client2 = client.newBuilder()
  .connectTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
  .writeTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
  .readTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
  .build();
```

I can reuse the existing builder to create another client and change some the existing parameters. I would like to be able to do the same with Retrofit 2.

I know what it's function is, but what specifically would you use it for?
Unlike OkHttpClient the Retrofit instance is much more dumb and
necessitates something like this much less. So actual motivations are much
more compelling than theoretical ones (which is why it's currently absent).

On Wed, Oct 19, 2016 at 8:47 PM Jared Burrows notifications@github.com
wrote:

> The same reason it is availble in OkHttp3.
> 
> For instance:
> 
> // Some Client
>     OkHttpClient client = new OkHttpClient.Builder()
>       .addInterceptor(new HttpLoggingInterceptor()
>         .setLevel(HttpLoggingInterceptor.Level.BODY))
>       .cache(cache)
>       .build();
> 
> // Some client 2
>     OkHttpClient client2 = client.newBuilder().connectTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
>       .writeTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
>       .readTimeout(CLIENT_TIME_OUT, TimeUnit.SECONDS)
>       .build();
> 
> I can reuse the existing builder to create another client and change some
> the existing parameters. I would like to be able to do the same with
> Retrofit 2.
> 
> —
> You are receiving this because you commented.
> 
> Reply to this email directly, view it on GitHub
> https://github.com/square/retrofit/issues/2054#issuecomment-254980927,
> or mute the thread
> https://github.com/notifications/unsubscribe-auth/AAEEER5uY1QdYgzRJg0ABd44T6NKFRVmks5q1ro7gaJpZM4KbmNw
> .

It would be nice to easily call `newBuilder` and pass in the url from the Okhttp mockserver for testing. But in general, it would be nice to change some the existing parameters.

U can hold one Retrofit.Builder instance, then you can create different Retrofit instance with this builder.
like this:
Retrofit.Builder builder = Retrofit.Builder()
                .addConverterFactory(GsonConverterFactory.create())
                .addCallAdapterFactory(RxJavaCallAdapterFactory.create())
                .client(okHttpClient);
Retrofit retrofit1 = builder
                .baseUrl("xxx")
                .build();

Retrofit retrofit2 = builder
                .baseUrl("xxxxx")
                .build();
Do you mean this?

@ly85206559 That is what I am doing now. I do not want to pass `Retrofit.Builder`. It would be nice to pass `Retrofit` as a parameter. With `OkHttp3`, you can call `newBuilder()` on `OkHttpClient`.


Pull request review comments:
All lists should use `addAll` and not retain these unmodifiable references.

What's a "rest adapter"?!? I don't think we want any of this doc. Unlike OkHttp, this isn't something we expect many to do (seeing as we haven't had the ability for Retrofit's 5 years of existence).

Good eye.


Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/Retrofit.java b/retrofit/src/main/java/retrofit2/Retrofit.java
index 9ee1116d50..2cea88098f 100644
--- a/retrofit/src/main/java/retrofit2/Retrofit.java
+++ b/retrofit/src/main/java/retrofit2/Retrofit.java
@@ -59,16 +59,18 @@
 public final class Retrofit {
   private final Map<Method, ServiceMethod<?, ?>> serviceMethodCache = new ConcurrentHashMap<>();
 
-  private final okhttp3.Call.Factory callFactory;
-  private final HttpUrl baseUrl;
-  private final List<Converter.Factory> converterFactories;
-  private final List<CallAdapter.Factory> adapterFactories;
-  private final Executor callbackExecutor;
-  private final boolean validateEagerly;
-
-  Retrofit(okhttp3.Call.Factory callFactory, HttpUrl baseUrl,
+  final Platform platform;
+  final okhttp3.Call.Factory callFactory;
+  final HttpUrl baseUrl;
+  final List<Converter.Factory> converterFactories;
+  final List<CallAdapter.Factory> adapterFactories;
+  final Executor callbackExecutor;
+  final boolean validateEagerly;
+
+  Retrofit(Platform platform, okhttp3.Call.Factory callFactory, HttpUrl baseUrl,
       List<Converter.Factory> converterFactories, List<CallAdapter.Factory> adapterFactories,
       Executor callbackExecutor, boolean validateEagerly) {
+    this.platform = platform;
     this.callFactory = callFactory;
     this.baseUrl = baseUrl;
     this.converterFactories = unmodifiableList(converterFactories); // Defensive copy at call site.
@@ -379,6 +381,10 @@ public Executor callbackExecutor() {
     return callbackExecutor;
   }
 
+  public Builder newBuilder() {
+    return new Builder(this);
+  }
+
   /**
    * Build a new {@link Retrofit}.
    * <p>
@@ -386,13 +392,13 @@ public Executor callbackExecutor() {
    * are optional.
    */
   public static final class Builder {
-    private Platform platform;
-    private okhttp3.Call.Factory callFactory;
-    private HttpUrl baseUrl;
-    private List<Converter.Factory> converterFactories = new ArrayList<>();
-    private List<CallAdapter.Factory> adapterFactories = new ArrayList<>();
-    private Executor callbackExecutor;
-    private boolean validateEagerly;
+    Platform platform;
+    okhttp3.Call.Factory callFactory;
+    HttpUrl baseUrl;
+    List<Converter.Factory> converterFactories = new ArrayList<>();
+    List<CallAdapter.Factory> adapterFactories = new ArrayList<>();
+    Executor callbackExecutor;
+    boolean validateEagerly;
 
     Builder(Platform platform) {
       this.platform = platform;
@@ -405,6 +411,16 @@ public Builder() {
       this(Platform.get());
     }
 
+    public Builder(Retrofit retrofit) {
+      this.platform = retrofit.platform;
+      this.callFactory = retrofit.callFactory;
+      this.baseUrl = retrofit.baseUrl;
+      this.converterFactories.addAll(retrofit.converterFactories);
+      this.adapterFactories.addAll(retrofit.adapterFactories);
+      this.callbackExecutor = retrofit.callbackExecutor;
+      this.validateEagerly = retrofit.validateEagerly;
+    }
+
     /**
      * The HTTP client used for requests.
      * <p>
@@ -562,7 +578,7 @@ public Retrofit build() {
       // Make a defensive copy of the converters.
       List<Converter.Factory> converterFactories = new ArrayList<>(this.converterFactories);
 
-      return new Retrofit(callFactory, baseUrl, converterFactories, adapterFactories,
+      return new Retrofit(platform, callFactory, baseUrl, converterFactories, adapterFactories,
           callbackExecutor, validateEagerly);
     }
   }


Test patch:
diff --git a/retrofit/src/test/java/retrofit2/RetrofitTest.java b/retrofit/src/test/java/retrofit2/RetrofitTest.java
index 885394bfc0..b54cf598fe 100644
--- a/retrofit/src/test/java/retrofit2/RetrofitTest.java
+++ b/retrofit/src/test/java/retrofit2/RetrofitTest.java
@@ -54,6 +54,8 @@
 import static okhttp3.mockwebserver.SocketPolicy.DISCONNECT_AT_START;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.Matchers.any;
@@ -140,6 +142,39 @@ interface MutableParameters {
     }
   }
 
+  @Test public void cloneSharesStatefulInstances() {
+    CallAdapter.Factory callAdapter = mock(CallAdapter.Factory.class);
+    Converter.Factory converter = mock(Converter.Factory.class);
+    Executor executor = mock(Executor.class);
+    okhttp3.Call.Factory call = mock(okhttp3.Call.Factory.class);
+    OkHttpClient client = mock(OkHttpClient.class);
+
+    Retrofit retrofit = new Retrofit.Builder()
+        .addCallAdapterFactory(callAdapter)
+        .addConverterFactory(converter)
+        .baseUrl(server.url("/"))
+        .callbackExecutor(executor)
+        .callFactory(call)
+        .client(client)
+        .build();
+
+    // Values should be non-null.
+    Retrofit a = retrofit.newBuilder().build();
+    assertNotNull(a.callAdapterFactories().get(0));
+    assertNotNull(a.converterFactories().get(0));
+    assertNotNull(a.baseUrl());
+    assertNotNull(a.callbackExecutor());
+    assertNotNull(a.callFactory());
+
+    // Multiple clients share the instances.
+    Retrofit b = retrofit.newBuilder().build();
+    assertSame(a.callAdapterFactories().get(0), b.callAdapterFactories().get(0));
+    assertSame(a.converterFactories().get(0), b.converterFactories().get(0));
+    assertSame(a.baseUrl(), b.baseUrl());
+    assertSame(a.callbackExecutor(), b.callbackExecutor());
+    assertSame(a.callFactory(), b.callFactory());
+  }
+
   @Test public void responseTypeCannotBeRetrofitResponse() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl(server.url("/"))