Merge pull request #165 from jjNford/master

Always return response object on success callbacks.

retrofit/src/main/java/retrofit/http/Callback.java

 // Copyright 2012 Square, Inc.
 package retrofit.http;
 
+import retrofit.http.client.Response;
+
 /**
  * Communicates responses to server or offline requests. Contains a callback method for each
  * possible outcome. One and only one method will be invoked in response to a given request.
@@ -11,7 +13,7 @@ package retrofit.http;
 public interface Callback<T> {
 
   /** Successful HTTP response. */
-  void success(T t);
+  void success(T t, Response response);
 
   /**
    * Unsuccessful HTTP response due to network failure, non-2XX status code, or unexpected

retrofit/src/main/java/retrofit/http/CallbackRunnable.java

@@ -21,10 +21,10 @@ abstract class CallbackRunnable<T> implements Runnable {
   @SuppressWarnings("unchecked")
   @Override public final void run() {
     try {
-      final Object response = obtainResponse();
+      final ResponseWrapper wrapper = obtainResponse();
       callbackExecutor.execute(new Runnable() {
         @Override public void run() {
-          callback.success((T) response);
+          callback.success((T) wrapper.responseBody, wrapper.response);
         }
       });
     } catch (final RetrofitError e) {
@@ -36,5 +36,5 @@ abstract class CallbackRunnable<T> implements Runnable {
     }
   }
 
-  public abstract Object obtainResponse();
+  public abstract ResponseWrapper obtainResponse();
 }

retrofit/src/main/java/retrofit/http/ResponseWrapper.java

+package retrofit.http;
+
+import retrofit.http.client.Response;
+
+/**
+ * A wrapper that holds the {@link Response} and {@link Converter} response to be used by the
+ * {@link CallbackRunnable} for success method calls on {@link Callback}.
+ *
+ * @author JJ Ford (jj.n.ford@gmail.com)
+ */
+final class ResponseWrapper {
+
+  final Response response;
+  final Object responseBody;
+
+  public ResponseWrapper(Response response, Object responseBody) {
+    this.response = response;
+    this.responseBody = responseBody;
+  }
+}

retrofit/src/main/java/retrofit/http/RestAdapter.java

@@ -125,8 +125,8 @@ public class RestAdapter {
       }
       Callback<?> callback = (Callback<?>) args[args.length - 1];
       httpExecutor.execute(new CallbackRunnable(callback, callbackExecutor) {
-        @Override public Object obtainResponse() {
-          return invokeRequest(methodDetails, args);
+        @Override public ResponseWrapper obtainResponse() {
+          return (ResponseWrapper) invokeRequest(methodDetails, args);
         }
       });
       return null; // Asynchronous methods should have return type of void.
@@ -184,13 +184,20 @@ public class RestAdapter {
         Type type = methodDetails.type;
         if (statusCode >= 200 && statusCode < 300) { // 2XX == successful request
           if (type.equals(Response.class)) {
-            return response;
+            if (methodDetails.isSynchronous) {
+              return response;
+            }
+            return new ResponseWrapper(response, response);
           }
           if (body == null) {
             return null;
           }
           try {
-            return converter.fromBody(body, type);
+            Object convert = converter.fromBody(body, type);
+            if (methodDetails.isSynchronous) {
+              return convert;
+            }
+            return new ResponseWrapper(response, convert);
           } catch (ConversionException e) {
             throw RetrofitError.conversionError(url, response, converter, type, e);
           }

retrofit/src/test/java/retrofit/http/CallbackRunnableTest.java

@@ -21,20 +21,20 @@ public class CallbackRunnableTest {
   @Before public void setUp() {
     callback = mock(Callback.class);
     callbackRunnable = spy(new CallbackRunnable<Object>(callback, executor) {
-      @Override public Object obtainResponse() {
+      @Override public ResponseWrapper obtainResponse() {
         return null; // Must be mocked.
       }
     });
   }
 
   @Test public void responsePassedToSuccess() {
-    Object response = new Object();
-    when(callbackRunnable.obtainResponse()).thenReturn(response);
+    ResponseWrapper wrapper = new ResponseWrapper(null, new Object());
+    when(callbackRunnable.obtainResponse()).thenReturn(wrapper);
 
     callbackRunnable.run();
 
     verify(executor).execute(any(Runnable.class));
-    verify(callback).success(same(response));
+    verify(callback).success(same(wrapper.responseBody), same(wrapper.response));
   }
 
   @Test public void errorPassedToFailure() {

retrofit/src/test/java/retrofit/http/RestAdapterTest.java

@@ -16,6 +16,7 @@ import static org.fest.assertions.api.Assertions.assertThat;
 import static org.fest.assertions.api.Assertions.fail;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyInt;
+import static org.mockito.Matchers.anyString;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.same;
 import static org.mockito.Mockito.mock;
@@ -88,15 +89,16 @@ public class RestAdapterTest {
   }
 
   @Test public void asynchronousUsesExecutors() throws Exception {
+    Response response = new Response(200, "OK", NO_HEADERS, new TypedString("{}"));
     when(mockClient.execute(any(Request.class))) //
-        .thenReturn(new Response(200, "OK", NO_HEADERS, null));
+        .thenReturn(response);
     Callback<Object> callback = mock(Callback.class);
 
     example.something(callback);
 
     verify(mockRequestExecutor).execute(any(CallbackRunnable.class));
     verify(mockCallbackExecutor).execute(any(Runnable.class));
-    verify(callback).success(eq(null));
+    verify(callback).success(anyString(), same(response));
   }
 
   @Test public void malformedResponseThrowsConversionException() throws Exception {
@@ -166,6 +168,6 @@ public class RestAdapterTest {
 
     verify(mockRequestExecutor).execute(any(CallbackRunnable.class));
     verify(mockCallbackExecutor).execute(any(Runnable.class));
-    verify(callback).success(eq(response));
+    verify(callback).success(eq(response), same(response));
   }
 }