Repository: square/retrofit
Issue: Retrofit2 support for alternative HTTP clients?

I see that v2 couples very tightly to `OkHttp`. Specifically, `Retrofit.Builder.client(OkHttpClient)` only takes an implementation of `OkHttpClient` instead of an abstract interface.

I was looking to integrate v2 support in to Ratpack and I can make it work with the `CallAdapter` except that the implementation continues to use OkHttpClient behind the scenes. While functional, I was wondering if there is a way that I could replace the underlying HTTP client with Ratpack's Netty based Http client.

Is there a way to do this (even if hacky)? Is this a feature that you would be interested in supporting?


Issue comments:
The hacky way to do this is with an OkHttp interceptor that calls into your HttpClient instead of calling chain.proceed().

hmmm, clever :)

The only downside here is you still can't use this to adapt to an async HttpClient because the `Interceptor` interface requires a synchronous return value: `Response intercept(Chain)`

I think the place that it needs to be done is in `MethodHandler.invoke`. It currently explicitly creates a `OkHttpCall`, but if that was sometype of provider interface that I could provide my own implementation, then I can plugin in my client...

```
interface CallProvider {
  <T> Call<T> provide(Retrofit, RequestFactory, Converter<ResponseBody, T>, Object...)
}
```

Then `MethodHandler.invoke()` would be:

```
Object invoke(Object... args) {
  return callAdapter.adapt(callProvider.provide(retrofit, requestFactory, responseConverter, args));
}
```

And we'd just need a way to pass the `CallProvider` that far down.

That or allow a `CallAdapter` to get a handle on the original Request, so it could adapt the call by doing it's own actions.

Hmm, I probably wouldn't make this hypothetical type aware of the mechanism for creating a `Request`. Something like:

```
interface Call.Factory {
  <T> Call<T> create(Request request, Converter<ResponseBody, T> converter);
}
```

@JakeWharton are you thinking this interface would go into the `withClient()` method in `Retrofit`?
I'd be happy to code up a PR with some direction.

Give me a day or two to think about it? We explicitly chose not to support multiple HTTP clients, but re-using OkHttp's value types as a model for abstracting different HTTP clients is an interesting one. I need to re-frame some things in my head.

Sure thing. I'll wait to hear back from you on here. Like I said, I don't mind doing the leg work and if you want to provide an initial path than I can code it up and we can re-work that as much as you want.

Ok I'm thinking:

``` java
abstract class Client {
  abstract <T> Call<T> newCall(Request request, Converter<ResponseBody, T> converter);
}
```

along with special treatment in the public API for taking in an `OkHttpClient` and wrapping it internally.

The abstract class buys us the ability to add a method in the future for creating a hypothetical `WebSocketCall<T>` (#924) which can default to `throw new UnsupportedOperationException();`.

@JakeWharton in case you're curious, I finally got around to implementing this in Ratpack - https://ratpack.io/manual/1.4.0/retrofit.html#retrofit_type_safe_clients


Pull request review comments:
I originally used `Callable<Request>` for this, but the fact that it throws a full-on `Exception` was ugly and the naming clashes severely with our `Call` and `Callback` and friends. Really low-value class here. Java 8's `Supplier` doesn't fit either because of the need to throw `IOException` (not that we can use Java 8 types anyway).

copyright

to prevent this if desired?


Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit2/Call.java b/retrofit/src/main/java/retrofit2/Call.java
index 873bb0c6fe..ca57759fd7 100644
--- a/retrofit/src/main/java/retrofit2/Call.java
+++ b/retrofit/src/main/java/retrofit2/Call.java
@@ -15,6 +15,7 @@
  */
 package retrofit2;
 
+import com.squareup.okhttp.ResponseBody;
 import java.io.IOException;
 
 /**
@@ -66,4 +67,13 @@ public interface Call<T> extends Cloneable {
    * has already been.
    */
   Call<T> clone();
+
+  /** Creates {@link Call} instances. */
+  interface Factory {
+    /**
+     * Returns a {@link Call} which will send {@code request} when executed or enqueue and use
+     * {@code converter} to parse the response. May not return null.
+     */
+    <T> Call<T> create(DeferredRequest request, Converter<ResponseBody, T> converter);
+  }
 }
diff --git a/retrofit/src/main/java/retrofit2/DeferredRequest.java b/retrofit/src/main/java/retrofit2/DeferredRequest.java
new file mode 100644
index 0000000000..37d83711db
--- /dev/null
+++ b/retrofit/src/main/java/retrofit2/DeferredRequest.java
@@ -0,0 +1,28 @@
+/*
+ * 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 retrofit2;
+
+import com.squareup.okhttp.Request;
+import java.io.IOException;
+
+/**
+ * An un-built HTTP request. This class defers any work necessary to create an HTTP request until
+ * {@link #get()} is called.
+ */
+public interface DeferredRequest {
+  /** Perform the work necessary to create and then return the {@link Request}. */
+  Request get() throws IOException;
+}
diff --git a/retrofit/src/main/java/retrofit2/MethodHandler.java b/retrofit/src/main/java/retrofit2/MethodHandler.java
index 8e4fe720c0..440e78ef6c 100644
--- a/retrofit/src/main/java/retrofit2/MethodHandler.java
+++ b/retrofit/src/main/java/retrofit2/MethodHandler.java
@@ -15,7 +15,6 @@
  */
 package retrofit2;
 
-import com.squareup.okhttp.OkHttpClient;
 import com.squareup.okhttp.ResponseBody;
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Method;
@@ -29,7 +28,8 @@ static MethodHandler create(Retrofit retrofit, Method method) {
     Converter<ResponseBody, ?> responseConverter =
         createResponseConverter(method, retrofit, responseType);
     RequestFactory requestFactory = RequestFactoryParser.parse(method, responseType, retrofit);
-    return new MethodHandler(retrofit.client(), requestFactory, callAdapter, responseConverter);
+    return new MethodHandler(retrofit.callFactory(), requestFactory, callAdapter,
+        responseConverter);
   }
 
   private static CallAdapter<?> createCallAdapter(Method method, Retrofit retrofit) {
@@ -59,20 +59,25 @@ private static CallAdapter<?> createCallAdapter(Method method, Retrofit retrofit
     }
   }
 
-  private final OkHttpClient client;
+  private final Call.Factory callFactory;
   private final RequestFactory requestFactory;
   private final CallAdapter<?> callAdapter;
   private final Converter<ResponseBody, ?> responseConverter;
 
-  private MethodHandler(OkHttpClient client, RequestFactory requestFactory,
+  private MethodHandler(Call.Factory callFactory, RequestFactory requestFactory,
       CallAdapter<?> callAdapter, Converter<ResponseBody, ?> responseConverter) {
-    this.client = client;
+    this.callFactory = callFactory;
     this.requestFactory = requestFactory;
     this.callAdapter = callAdapter;
     this.responseConverter = responseConverter;
   }
 
   Object invoke(Object... args) {
-    return callAdapter.adapt(new OkHttpCall<>(client, requestFactory, responseConverter, args));
+    DeferredRequest request = requestFactory.defer(args);
+    Call<?> call = callFactory.create(request, responseConverter);
+    if (call == null) {
+      throw new NullPointerException("Call.Factory returned null.");
+    }
+    return callAdapter.adapt(call);
   }
 }
diff --git a/retrofit/src/main/java/retrofit2/OkHttpCall.java b/retrofit/src/main/java/retrofit2/OkHttpCall.java
index fc2d70b6ff..3972ffed54 100644
--- a/retrofit/src/main/java/retrofit2/OkHttpCall.java
+++ b/retrofit/src/main/java/retrofit2/OkHttpCall.java
@@ -29,25 +29,23 @@
 
 final class OkHttpCall<T> implements Call<T> {
   private final OkHttpClient client;
-  private final RequestFactory requestFactory;
+  private final DeferredRequest request;
   private final Converter<ResponseBody, T> responseConverter;
-  private final Object[] args;
 
   private volatile com.squareup.okhttp.Call rawCall;
   private boolean executed; // Guarded by this.
   private volatile boolean canceled;
 
-  OkHttpCall(OkHttpClient client, RequestFactory requestFactory,
-      Converter<ResponseBody, T> responseConverter, Object[] args) {
+  OkHttpCall(OkHttpClient client, DeferredRequest request,
+      Converter<ResponseBody, T> responseConverter) {
     this.client = client;
-    this.requestFactory = requestFactory;
+    this.request = request;
     this.responseConverter = responseConverter;
-    this.args = args;
   }
 
   @SuppressWarnings("CloneDoesntCallSuperClone") // We are a final type & this saves clearing state.
   @Override public OkHttpCall<T> clone() {
-    return new OkHttpCall<>(client, requestFactory, responseConverter, args);
+    return new OkHttpCall<>(client, request, responseConverter);
   }
 
   @Override public void enqueue(final Callback<T> callback) {
@@ -122,7 +120,7 @@ private void callSuccess(Response<T> response) {
   }
 
   private com.squareup.okhttp.Call createRawCall() throws IOException {
-    return client.newCall(requestFactory.create(args));
+    return client.newCall(request.get());
   }
 
   private Response<T> parseResponse(com.squareup.okhttp.Response rawResponse) throws IOException {
diff --git a/retrofit/src/main/java/retrofit2/OkHttpCallFactory.java b/retrofit/src/main/java/retrofit2/OkHttpCallFactory.java
new file mode 100644
index 0000000000..7c9e666bde
--- /dev/null
+++ b/retrofit/src/main/java/retrofit2/OkHttpCallFactory.java
@@ -0,0 +1,32 @@
+/*
+ * 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 retrofit2;
+
+import com.squareup.okhttp.OkHttpClient;
+import com.squareup.okhttp.ResponseBody;
+
+final class OkHttpCallFactory implements Call.Factory {
+  final OkHttpClient client;
+
+  OkHttpCallFactory(OkHttpClient client) {
+    this.client = client;
+  }
+
+  @Override
+  public <T> Call<T> create(DeferredRequest request, Converter<ResponseBody, T> converter) {
+    return new OkHttpCall<>(client, request, converter);
+  }
+}
diff --git a/retrofit/src/main/java/retrofit2/RequestFactory.java b/retrofit/src/main/java/retrofit2/RequestFactory.java
index 7a266483c7..23063def4b 100644
--- a/retrofit/src/main/java/retrofit2/RequestFactory.java
+++ b/retrofit/src/main/java/retrofit2/RequestFactory.java
@@ -45,6 +45,14 @@ final class RequestFactory {
     this.requestActions = requestActions;
   }
 
+  DeferredRequest defer(final Object... args) {
+    return new DeferredRequest() {
+      @Override public Request get() throws IOException {
+        return create(args);
+      }
+    };
+  }
+
   Request create(Object... args) throws IOException {
     RequestBuilder requestBuilder =
         new RequestBuilder(method, baseUrl.url(), relativeUrl, headers, contentType, hasBody,
diff --git a/retrofit/src/main/java/retrofit2/Retrofit.java b/retrofit/src/main/java/retrofit2/Retrofit.java
index 606295a630..5427ad33e7 100644
--- a/retrofit/src/main/java/retrofit2/Retrofit.java
+++ b/retrofit/src/main/java/retrofit2/Retrofit.java
@@ -92,17 +92,17 @@
 public final class Retrofit {
   private final Map<Method, MethodHandler> methodHandlerCache = new LinkedHashMap<>();
 
-  private final OkHttpClient client;
+  private final Call.Factory callFactory;
   private final BaseUrl baseUrl;
   private final List<Converter.Factory> converterFactories;
   private final List<CallAdapter.Factory> adapterFactories;
   private final Executor callbackExecutor;
   private final boolean validateEagerly;
 
-  private Retrofit(OkHttpClient client, BaseUrl baseUrl, List<Converter.Factory> converterFactories,
-      List<CallAdapter.Factory> adapterFactories, Executor callbackExecutor,
-      boolean validateEagerly) {
-    this.client = client;
+  private Retrofit(Call.Factory callFactory, BaseUrl baseUrl,
+      List<Converter.Factory> converterFactories, List<CallAdapter.Factory> adapterFactories,
+      Executor callbackExecutor, boolean validateEagerly) {
+    this.callFactory = callFactory;
     this.baseUrl = baseUrl;
     this.converterFactories = converterFactories;
     this.adapterFactories = adapterFactories;
@@ -156,8 +156,11 @@ MethodHandler loadMethodHandler(Method method) {
     return handler;
   }
 
-  public OkHttpClient client() {
-    return client;
+  /**
+   * TODO
+   */
+  public Call.Factory callFactory() {
+    return callFactory;
   }
 
   public BaseUrl baseUrl() {
@@ -344,7 +347,7 @@ public Executor callbackExecutor() {
    * are optional.
    */
   public static final class Builder {
-    private OkHttpClient client;
+    private Call.Factory callFactory;
     private BaseUrl baseUrl;
     private List<Converter.Factory> converterFactories = new ArrayList<>();
     private List<CallAdapter.Factory> adapterFactories = new ArrayList<>();
@@ -357,9 +360,25 @@ public Builder() {
       converterFactories.add(new BuiltInConverters());
     }
 
-    /** The HTTP client used for requests. */
+    /**
+     * The HTTP client used for requests.
+     * <p>
+     * This is a convenience method for calling {@link #callFactory} with an instance that uses
+     * {@code client} for execution.
+     * <p>
+     * Note: This method <b>does not</b> make a defensive copy of {@code client}. Changes to its
+     * settings will affect subsequent requests. Pass in a {@linkplain OkHttpClient#clone() cloned}
+     * instance to prevent this if desired.
+     */
     public Builder client(OkHttpClient client) {
-      this.client = checkNotNull(client, "client == null");
+      return callFactory(new OkHttpCallFactory(checkNotNull(client, "client == null")));
+    }
+
+    /**
+     * TODO
+     */
+    public Builder callFactory(Call.Factory factory) {
+      this.callFactory = checkNotNull(factory, "factory == null");
       return this;
     }
 
@@ -488,9 +507,9 @@ public Retrofit build() {
         throw new IllegalStateException("Base URL required.");
       }
 
-      OkHttpClient client = this.client;
-      if (client == null) {
-        client = new OkHttpClient();
+      Call.Factory callFactory = this.callFactory;
+      if (callFactory == null) {
+        callFactory = new OkHttpCallFactory(new OkHttpClient());
       }
 
       // Make a defensive copy of the adapters and add the default Call adapter.
@@ -500,8 +519,8 @@ public Retrofit build() {
       // Make a defensive copy of the converters.
       List<Converter.Factory> converterFactories = new ArrayList<>(this.converterFactories);
 
-      return new Retrofit(client, baseUrl, converterFactories, adapterFactories, callbackExecutor,
-          validateEagerly);
+      return new Retrofit(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 24c3ccd362..600771966e 100644
--- a/retrofit/src/test/java/retrofit2/RetrofitTest.java
+++ b/retrofit/src/test/java/retrofit2/RetrofitTest.java
@@ -648,20 +648,92 @@ class MyConverterFactory extends Converter.Factory {
     }
   }
 
-  @Test public void clientDefault() {
+  @Test public void callFactoryDefault() {
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl("http://example.com")
         .build();
-      assertThat(retrofit.client()).isNotNull();
+    assertThat(retrofit.callFactory()).isNotNull();
   }
 
-  @Test public void clientPropagated() {
+  @Test public void callFactoryPropagated() {
+    Call.Factory callFactory = mock(Call.Factory.class);
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .callFactory(callFactory)
+        .build();
+    assertThat(retrofit.callFactory()).isSameAs(callFactory);
+  }
+
+  @Test public void callFactoryClientPropagated() {
     OkHttpClient client = new OkHttpClient();
     Retrofit retrofit = new Retrofit.Builder()
         .baseUrl("http://example.com/")
         .client(client)
         .build();
-    assertThat(retrofit.client()).isSameAs(client);
+    OkHttpCallFactory factory = (OkHttpCallFactory) retrofit.callFactory();
+    assertThat(factory.client).isSameAs(client);
+  }
+
+  @Test public void callFactoryUsed() {
+    final Retrofit blackbox = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .build();
+    Call.Factory callFactory = spy(new Call.Factory() {
+      @Override
+      public <T> Call<T> create(DeferredRequest request, Converter<ResponseBody, T> converter) {
+        // Wrap the default Call.Factory without directly relying on its implementation.
+        return blackbox.callFactory().create(request, converter);
+      }
+    });
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .callFactory(callFactory)
+        .build();
+    CallMethod service = retrofit.create(CallMethod.class);
+    service.getResponseBody();
+    verify(callFactory).create(any(DeferredRequest.class), any(Converter.class));
+    verifyNoMoreInteractions(callFactory);
+  }
+
+  @Test public void callFactoryReturningNullThrows() {
+    Call.Factory callFactory = new Call.Factory() {
+      @Override
+      public <T> Call<T> create(DeferredRequest request, Converter<ResponseBody, T> converter) {
+        return null;
+      }
+    };
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .callFactory(callFactory)
+        .build();
+    CallMethod service = retrofit.create(CallMethod.class);
+    try {
+      service.getResponseBody();
+      fail();
+    } catch (NullPointerException e) {
+      assertThat(e).hasMessage("Call.Factory returned null.");
+    }
+  }
+
+  @Test public void callFactoryThrowingPropagates() {
+    final RuntimeException cause = new RuntimeException("Broken!");
+    Call.Factory callFactory = new Call.Factory() {
+      @Override
+      public <T> Call<T> create(DeferredRequest request, Converter<ResponseBody, T> converter) {
+        throw cause;
+      }
+    };
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl("http://example.com/")
+        .callFactory(callFactory)
+        .build();
+    CallMethod service = retrofit.create(CallMethod.class);
+    try {
+      service.getResponseBody();
+      fail();
+    } catch (Exception e) {
+      assertThat(e).isSameAs(cause);
+    }
   }
 
   @Test public void converterNullThrows() {