Repository: square/retrofit
Issue: Add @Raw or similiar to avoid convertig body stream to byte[] in Response object

Hey there,

first of all thanks for providing Retrofit! It's an amazing framework that impressively simplifies calling REST-APIs!

We use it in one of our server application to communicate with another remote server to download binary data (i.e. files) from there using Response as return type for the interface method. Our server operates as a proxy and copies the response.getBody().in() to the the user requesting resources. This approach works as expected, but has a performance flaw.

At the moment Retrofit converts every Response body stream to a byte array (see RestAdapter.java):

``` java
if (type.equals(Response.class)) {
    // Read the entire stream and replace with one backed by a byte[]
    response = Utils.readBodyToBytesIfNecessary(response);

    if (methodInfo.isSynchronous) {
        return response;
    }
    return new ResponseWrapper(response, response);
}
```

If you just want to proxy the stream, converting to a byte[] first can result in a real performance hit. The larger the binary data, the worse it will be. We would like to see _response = Utils.readBodyToBytesIfNecessary(response);_ optional, e.g. by annotating the interface method with Response return type with an additional _@Raw_.

What do you think of this approach? Maybe something like this is already planned?
If not and if you consider this a valid approach, we can also prepare a pull request to implement this behaviour. 


Issue comments:

Pull request review comments:

Patch that resolved the issue:
diff --git a/retrofit/src/main/java/retrofit/RestAdapter.java b/retrofit/src/main/java/retrofit/RestAdapter.java
index bdd1412f2c..d0ae8b8b0a 100644
--- a/retrofit/src/main/java/retrofit/RestAdapter.java
+++ b/retrofit/src/main/java/retrofit/RestAdapter.java
@@ -338,8 +338,10 @@ private Object invokeRequest(RequestInterceptor requestInterceptor, RestMethodIn
         if (statusCode >= 200 && statusCode < 300) { // 2XX == successful request
           // Caller requested the raw Response object directly.
           if (type.equals(Response.class)) {
-            // Read the entire stream and replace with one backed by a byte[]
-            response = Utils.readBodyToBytesIfNecessary(response);
+            if (!methodInfo.isRaw) {
+              // Read the entire stream and replace with one backed by a byte[]
+              response = Utils.readBodyToBytesIfNecessary(response);
+            }
 
             if (methodInfo.isSynchronous) {
               return response;
diff --git a/retrofit/src/main/java/retrofit/RestMethodInfo.java b/retrofit/src/main/java/retrofit/RestMethodInfo.java
index 3371bef75f..340bb58901 100644
--- a/retrofit/src/main/java/retrofit/RestMethodInfo.java
+++ b/retrofit/src/main/java/retrofit/RestMethodInfo.java
@@ -27,6 +27,7 @@
 import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
+import retrofit.client.Response;
 import retrofit.http.Body;
 import retrofit.http.EncodedPath;
 import retrofit.http.EncodedQuery;
@@ -42,6 +43,7 @@
 import retrofit.http.Path;
 import retrofit.http.Query;
 import retrofit.http.QueryMap;
+import retrofit.http.Raw;
 import retrofit.http.RestMethod;
 import rx.Observable;
 
@@ -100,6 +102,7 @@ enum RequestType {
   String requestQuery;
   List<retrofit.client.Header> headers;
   String contentTypeHeader;
+  boolean isRaw;
 
   // Parameter-level details
   String[] requestParamNames;
@@ -178,6 +181,13 @@ private void parseMethodAnnotations() {
           throw methodError("Only one encoding annotation is allowed.");
         }
         requestType = RequestType.FORM_URL_ENCODED;
+      } else if (annotationType == Raw.class) {
+        if (responseObjectType != Response.class) {
+          throw methodError(
+              "Only methods having %s as response data type are allowed to have @%s annotation.",
+              Response.class.getSimpleName(), Raw.class.getSimpleName());
+        }
+        isRaw = true;
       }
     }
 
diff --git a/retrofit/src/main/java/retrofit/http/Raw.java b/retrofit/src/main/java/retrofit/http/Raw.java
new file mode 100644
index 0000000000..3aba33dd1d
--- /dev/null
+++ b/retrofit/src/main/java/retrofit/http/Raw.java
@@ -0,0 +1,34 @@
+/*
+ * Copyright (C) 2014 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 retrofit.http;
+
+import java.lang.annotation.Documented;
+import java.lang.annotation.Retention;
+import java.lang.annotation.Target;
+
+import static java.lang.annotation.ElementType.METHOD;
+import static java.lang.annotation.RetentionPolicy.RUNTIME;
+
+/**
+ * Treat the response body on methods returning {@link retrofit.client.Response} as is, i.e.
+ * without converting {@link retrofit.client.Response#getBody()} to byte[].
+ */
+@Documented
+@Target(METHOD)
+@Retention(RUNTIME)
+public @interface Raw {
+
+}


Test patch:
diff --git a/retrofit/src/test/java/retrofit/RestMethodInfoTest.java b/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
index ece88dd5ce..1d4ee6a5ab 100644
--- a/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
+++ b/retrofit/src/test/java/retrofit/RestMethodInfoTest.java
@@ -32,6 +32,7 @@
 import retrofit.http.Path;
 import retrofit.http.Query;
 import retrofit.http.QueryMap;
+import retrofit.http.Raw;
 import retrofit.http.RestMethod;
 import retrofit.mime.TypedOutput;
 import rx.Observable;
@@ -219,6 +220,70 @@ class Example {
     assertThat(methodInfo.responseObjectType).isEqualTo(expected);
   }
 
+  @Test public void rawResponse() {
+    class Example {
+      @GET("/foo") @Raw retrofit.client.Response a() {
+        return null;
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    methodInfo.init();
+
+    assertThat(methodInfo.isRaw).isTrue();
+    assertThat(methodInfo.responseObjectType).isEqualTo(retrofit.client.Response.class);
+  }
+
+  @Test public void rawResponseWithCallback() {
+    class Example {
+      @GET("/foo") @Raw void a(Callback<retrofit.client.Response> callback) {
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    methodInfo.init();
+
+    assertThat(methodInfo.isRaw).isTrue();
+    assertThat(methodInfo.responseObjectType).isEqualTo(retrofit.client.Response.class);
+  }
+
+  @Test public void rawResponseNotAllowed() {
+    class Example {
+      @GET("/foo") @Raw String a() {
+        return null;
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    try {
+      methodInfo.init();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+              "Example.a: Only methods having Response as response data type are allowed to have @Raw annotation.");
+    }
+  }
+
+  @Test public void rawResponseWithCallbackNotAllowed() {
+    class Example {
+      @GET("/foo") @Raw void a(Callback<String> callback) {
+      }
+    }
+
+    Method method = TestingUtils.getMethod(Example.class, "a");
+    RestMethodInfo methodInfo = new RestMethodInfo(method);
+    try {
+      methodInfo.init();
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+              "Example.a: Only methods having Response as response data type are allowed to have @Raw annotation.");
+    }
+  }
+
   @Test public void observableResponse() {
     class Example {
       @GET("/foo") Observable<Response> a() {