Repository: square/retrofit
Issue: Provide new method to get the original request from Retrofit Call proxies

Hi,

When using Retrofit Call proxies, it's not possible to be aware of the original request through those objects. When implementing a custom response cache (not depending on response cache of okhttp), generating a cache key with the request is necessary. However we cannot access to this original request, headers, params through Call objects (okhttpcall being final as well).

Would be nice to provide in the interface Call the method CreateRawCall already implemented in private.


Issue comments:
Why expose the `Call` instead of the `Request`?

To access to the original request. But dummy from my side, the original request is not public as well.

Hum. which solution would be the best? I can pull request as well okhttp. Thank you a lot for your help.

In general it would be great to have an access from the proxies to the client.

For instance, the cache of okhttp is not available as well through those objects. Is there something planned in this way?

> which solution would be the best?

I would prefer exposing the `Request` given the choice between the two, but I need to think more about the implications of exposing this.

> In general it would be great to have an access from the proxies to the client.

You can access the client by calling `client()` on the `Retrofit` instance. The same client is shared by all created proxies.

Thank you very much for your help and your response!

The largest detractor against something like what this issue proposes is when test code (be it unit, functional, integration, or whatever) creates a fake `Call` instance. This is currently done via the `Calls` class for successful and failing responses.

What `Request` objects would be returned from these method if this proposal was accepted? We can synthesize a good portion of them. In fact, we already do so, depending on what is provided.

Oh, god we really need this. At the moment I am working on a caching library for Retrofit2 and unfortunately this is how I build a new Request: https://github.com/dimitrovskif/Retrofit-SmartCache/blob/master/retrofit-smartcache/src/main/java/dimitrovskif/smartcache/SmartCallFactory.java#L94

It works using ugly reflection, and uses the fact that the `Call<T>` which the `CallAdapter.Factory` gets is always an `OkHttpCall<T>`, so `requestFactory.create(args)` returns a `Request` which can be later used for caching purposes. (reflection takes care of the `private` restriction)

My suggestion would be:
- Provide `Request buildRequest()` in `Call<T>` which will return a valid call with all query params, proper method and endpoint, etc.
- Provide `Type responseType()` and `Annotation[] annotations()` in `Call<T>` to take care of the nasty type erasure problems (I **really** need this, especially for further serialization/deserialization in calls)
- Provide `Retrofit retrofit()` in `Call<T>`

Still not fixed? `OkHttpCall` has private request factory and the `Call` interface itself supports nothing.


Pull request review comments:
fail()

fail()

fail()

fail()

fail()

throw new AssertionError ?

fail()

The latch will just timeout if the wrong thing is called. If you throw here you'll get double stack traces so I left it out.


Patch that resolved the issue:
diff --git a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
index 2d8dd98f91..ac10878fa9 100644
--- a/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
+++ b/retrofit-mock/src/main/java/retrofit2/mock/BehaviorCall.java
@@ -20,6 +20,7 @@
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Future;
 import java.util.concurrent.atomic.AtomicReference;
+import okhttp3.Request;
 import retrofit2.Call;
 import retrofit2.Callback;
 import retrofit2.Response;
@@ -46,6 +47,10 @@ final class BehaviorCall<T> implements Call<T> {
     return new BehaviorCall<>(behavior, backgroundExecutor, delegate.clone());
   }
 
+  @Override public Request request() {
+    return delegate.request();
+  }
+
   @Override public void enqueue(final Callback<T> callback) {
     synchronized (this) {
       if (executed) throw new IllegalStateException("Already executed");
diff --git a/retrofit-mock/src/main/java/retrofit2/mock/Calls.java b/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
index 2bfb73eb97..49b490aee6 100644
--- a/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
+++ b/retrofit-mock/src/main/java/retrofit2/mock/Calls.java
@@ -16,6 +16,7 @@
 package retrofit2.mock;
 
 import java.io.IOException;
+import okhttp3.Request;
 import retrofit2.Call;
 import retrofit2.Callback;
 import retrofit2.Response;
@@ -51,6 +52,10 @@ public static <T> Call<T> response(final Response<T> response) {
       @Override public Call<T> clone() {
         return this;
       }
+
+      @Override public Request request() {
+        return response.raw().request();
+      }
     };
   }
 
@@ -79,6 +84,10 @@ public static <T> Call<T> failure(final IOException failure) {
       @Override public Call<T> clone() {
         return this;
       }
+
+      @Override public Request request() {
+        return new Request.Builder().url("http://localhost").build();
+      }
     };
   }
 
diff --git a/retrofit/src/main/java/retrofit2/Call.java b/retrofit/src/main/java/retrofit2/Call.java
index 873bb0c6fe..171d9e0c41 100644
--- a/retrofit/src/main/java/retrofit2/Call.java
+++ b/retrofit/src/main/java/retrofit2/Call.java
@@ -16,6 +16,7 @@
 package retrofit2;
 
 import java.io.IOException;
+import okhttp3.Request;
 
 /**
  * An invocation of a Retrofit method that sends a request to a webserver and returns a response.
@@ -66,4 +67,7 @@ public interface Call<T> extends Cloneable {
    * has already been.
    */
   Call<T> clone();
+
+  /** The original HTTP request. */
+  Request request();
 }
diff --git a/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java b/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
index ce6b5e602a..983b5eb401 100644
--- a/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
+++ b/retrofit/src/main/java/retrofit2/ExecutorCallAdapterFactory.java
@@ -19,6 +19,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.concurrent.Executor;
+import okhttp3.Request;
 
 final class ExecutorCallAdapterFactory implements CallAdapter.Factory {
   final Executor callbackExecutor;
@@ -98,5 +99,9 @@ static final class ExecutorCallbackCall<T> implements Call<T> {
     @Override public Call<T> clone() {
       return new ExecutorCallbackCall<>(callbackExecutor, delegate.clone());
     }
+
+    @Override public Request request() {
+      return delegate.request();
+    }
   }
 }
diff --git a/retrofit/src/main/java/retrofit2/OkHttpCall.java b/retrofit/src/main/java/retrofit2/OkHttpCall.java
index 2b6f74e8a0..a0ffa95c48 100644
--- a/retrofit/src/main/java/retrofit2/OkHttpCall.java
+++ b/retrofit/src/main/java/retrofit2/OkHttpCall.java
@@ -17,6 +17,7 @@
 
 import java.io.IOException;
 import okhttp3.MediaType;
+import okhttp3.Request;
 import okhttp3.ResponseBody;
 import okio.Buffer;
 import okio.BufferedSource;
@@ -29,10 +30,13 @@ final class OkHttpCall<T> implements Call<T> {
   private final Object[] args;
   private final Converter<ResponseBody, T> responseConverter;
 
-  private volatile okhttp3.Call rawCall;
-  private boolean executed; // Guarded by this.
   private volatile boolean canceled;
 
+  // All guarded by this.
+  private okhttp3.Call rawCall;
+  private Throwable creationFailure; // Either a RuntimeException or IOException.
+  private boolean executed;
+
   OkHttpCall(okhttp3.Call.Factory callFactory, RequestFactory requestFactory, Object[] args,
       Converter<ResponseBody, T> responseConverter) {
     this.callFactory = callFactory;
@@ -46,25 +50,58 @@ final class OkHttpCall<T> implements Call<T> {
     return new OkHttpCall<>(callFactory, requestFactory, args, responseConverter);
   }
 
+  @Override public synchronized Request request() {
+    okhttp3.Call call = rawCall;
+    if (call != null) {
+      return call.request();
+    }
+    if (creationFailure != null) {
+      if (creationFailure instanceof IOException) {
+        throw new RuntimeException("Unable to create request.", creationFailure);
+      } else {
+        throw (RuntimeException) creationFailure;
+      }
+    }
+    try {
+      return (rawCall = createRawCall()).request();
+    } catch (RuntimeException e) {
+      creationFailure = e;
+      throw e;
+    } catch (IOException e) {
+      creationFailure = e;
+      throw new RuntimeException("Unable to create request.", e);
+    }
+  }
+
   @Override public void enqueue(final Callback<T> callback) {
+    okhttp3.Call call;
+    Throwable failure;
+
     synchronized (this) {
       if (executed) throw new IllegalStateException("Already executed.");
       executed = true;
+
+      call = rawCall;
+      failure = creationFailure;
+      if (call == null && failure == null) {
+        try {
+          call = rawCall = createRawCall();
+        } catch (Throwable t) {
+          failure = creationFailure = t;
+        }
+      }
     }
 
-    okhttp3.Call rawCall;
-    try {
-      rawCall = createRawCall();
-    } catch (Throwable t) {
-      callback.onFailure(this, t);
+    if (failure != null) {
+      callback.onFailure(this, failure);
       return;
     }
+
     if (canceled) {
-      rawCall.cancel();
+      call.cancel();
     }
-    this.rawCall = rawCall;
 
-    rawCall.enqueue(new okhttp3.Callback() {
+    call.enqueue(new okhttp3.Callback() {
       @Override public void onResponse(okhttp3.Call call, okhttp3.Response rawResponse)
           throws IOException {
         Response<T> response;
@@ -108,18 +145,36 @@ private void callSuccess(Response<T> response) {
   }
 
   @Override public Response<T> execute() throws IOException {
+    okhttp3.Call call;
+
     synchronized (this) {
       if (executed) throw new IllegalStateException("Already executed.");
       executed = true;
+
+      if (creationFailure != null) {
+        if (creationFailure instanceof IOException) {
+          throw (IOException) creationFailure;
+        } else {
+          throw (RuntimeException) creationFailure;
+        }
+      }
+
+      call = rawCall;
+      if (call == null) {
+        try {
+          call = rawCall = createRawCall();
+        } catch (IOException | RuntimeException e) {
+          creationFailure = e;
+          throw e;
+        }
+      }
     }
 
-    okhttp3.Call rawCall = createRawCall();
     if (canceled) {
-      rawCall.cancel();
+      call.cancel();
     }
-    this.rawCall = rawCall;
 
-    return parseResponse(rawCall.execute());
+    return parseResponse(call.execute());
   }
 
   private okhttp3.Call createRawCall() throws IOException {
@@ -167,9 +222,13 @@ Response<T> parseResponse(okhttp3.Response rawResponse) throws IOException {
 
   public void cancel() {
     canceled = true;
-    okhttp3.Call rawCall = this.rawCall;
-    if (rawCall != null) {
-      rawCall.cancel();
+
+    okhttp3.Call call;
+    synchronized (this) {
+      call = rawCall;
+    }
+    if (call != null) {
+      call.cancel();
     }
   }
 


Test patch:
diff --git a/retrofit/src/test/java/retrofit2/CallTest.java b/retrofit/src/test/java/retrofit2/CallTest.java
index ef1e923e0e..ae08c0be6f 100644
--- a/retrofit/src/test/java/retrofit2/CallTest.java
+++ b/retrofit/src/test/java/retrofit2/CallTest.java
@@ -19,6 +19,7 @@
 import java.lang.annotation.Annotation;
 import java.lang.reflect.Type;
 import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import okhttp3.Interceptor;
 import okhttp3.OkHttpClient;
@@ -36,6 +37,7 @@
 import retrofit2.http.Body;
 import retrofit2.http.GET;
 import retrofit2.http.POST;
+import retrofit2.http.Path;
 import retrofit2.http.Streaming;
 
 import static java.util.concurrent.TimeUnit.SECONDS;
@@ -54,6 +56,7 @@ interface Service {
     @GET("/") Call<ResponseBody> getBody();
     @GET("/") @Streaming Call<ResponseBody> getStreamingBody();
     @POST("/") Call<String> postString(@Body String body);
+    @POST("/{a}") Call<String> postRequestBody(@Path("a") Object a);
   }
 
   @Test public void http200Sync() throws IOException {
@@ -667,4 +670,275 @@ public Converter<?, RequestBody> requestBodyConverter(Type type, Annotation[] an
     assertTrue(latch.await(2, SECONDS));
     assertThat(failureRef.get()).isInstanceOf(IOException.class).hasMessage("Canceled");
   }
+
+  @Test public void requestBeforeExecuteCreates() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        return "Hello";
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    call.request();
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    call.execute();
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
+
+  @Test public void requestThrowingBeforeExecuteFailsExecute() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        throw new RuntimeException("Broken!");
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    try {
+      call.request();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    try {
+      call.execute();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
+
+  @Test public void requestAfterExecuteReturnsCachedValue() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        return "Hello";
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    call.execute();
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    call.request();
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
+
+  @Test public void requestAfterExecuteThrowingAlsoThrows() throws IOException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        throw new RuntimeException("Broken!");
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    try {
+      call.execute();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    try {
+      call.request();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
+
+  @Test public void requestBeforeEnqueueCreates() throws IOException, InterruptedException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        return "Hello";
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    call.request();
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    call.enqueue(new Callback<String>() {
+      @Override public void onResponse(Call<String> call, Response<String> response) {
+        assertThat(writeCount.get()).isEqualTo(1);
+        latch.countDown();
+      }
+
+      @Override public void onFailure(Call<String> call, Throwable t) {
+      }
+    });
+    assertTrue(latch.await(1, SECONDS));
+  }
+
+  @Test public void requestThrowingBeforeEnqueueFailsEnqueue()
+      throws IOException, InterruptedException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        throw new RuntimeException("Broken!");
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    try {
+      call.request();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    call.enqueue(new Callback<String>() {
+      @Override public void onResponse(Call<String> call, Response<String> response) {
+      }
+
+      @Override public void onFailure(Call<String> call, Throwable t) {
+        assertThat(t).isExactlyInstanceOf(RuntimeException.class).hasMessage("Broken!");
+        assertThat(writeCount.get()).isEqualTo(1);
+        latch.countDown();
+      }
+    });
+    assertTrue(latch.await(1, SECONDS));
+  }
+
+  @Test public void requestAfterEnqueueReturnsCachedValue() throws IOException,
+      InterruptedException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        return "Hello";
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    call.enqueue(new Callback<String>() {
+      @Override public void onResponse(Call<String> call, Response<String> response) {
+        assertThat(writeCount.get()).isEqualTo(1);
+        latch.countDown();
+      }
+
+      @Override public void onFailure(Call<String> call, Throwable t) {
+      }
+    });
+    assertTrue(latch.await(1, SECONDS));
+
+    call.request();
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
+
+  @Test public void requestAfterEnqueueFailingThrows() throws IOException,
+      InterruptedException {
+    Retrofit retrofit = new Retrofit.Builder()
+        .baseUrl(server.url("/"))
+        .addConverterFactory(new ToStringConverterFactory())
+        .build();
+    Service service = retrofit.create(Service.class);
+
+    server.enqueue(new MockResponse());
+
+    final AtomicInteger writeCount = new AtomicInteger();
+    Object a = new Object() {
+      @Override public String toString() {
+        writeCount.incrementAndGet();
+        throw new RuntimeException("Broken!");
+      }
+    };
+    Call<String> call = service.postRequestBody(a);
+
+    final CountDownLatch latch = new CountDownLatch(1);
+    call.enqueue(new Callback<String>() {
+      @Override public void onResponse(Call<String> call, Response<String> response) {
+      }
+
+      @Override public void onFailure(Call<String> call, Throwable t) {
+        assertThat(t).isExactlyInstanceOf(RuntimeException.class).hasMessage("Broken!");
+        assertThat(writeCount.get()).isEqualTo(1);
+        latch.countDown();
+      }
+    });
+    assertTrue(latch.await(1, SECONDS));
+
+    try {
+      call.request();
+      fail();
+    } catch (RuntimeException e) {
+      assertThat(e).hasMessage("Broken!");
+    }
+    assertThat(writeCount.get()).isEqualTo(1);
+  }
 }
diff --git a/retrofit/src/test/java/retrofit2/ExecutorCallAdapterFactoryTest.java b/retrofit/src/test/java/retrofit2/ExecutorCallAdapterFactoryTest.java
index e4bcc11f78..ba45b5fe67 100644
--- a/retrofit/src/test/java/retrofit2/ExecutorCallAdapterFactoryTest.java
+++ b/retrofit/src/test/java/retrofit2/ExecutorCallAdapterFactoryTest.java
@@ -21,6 +21,7 @@
 import java.lang.reflect.Type;
 import java.util.List;
 import java.util.concurrent.Executor;
+import okhttp3.Request;
 import org.junit.Test;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -161,5 +162,9 @@ static class EmptyCall implements Call<String> {
     @Override public Call<String> clone() {
       throw new UnsupportedOperationException();
     }
+
+    @Override public Request request() {
+      throw new UnsupportedOperationException();
+    }
   }
 }