Add support for OkHttp's MultipartBody.Part.

To do this we need to make `@Part`'s value (i.e., name) optional, since `MultipartBody.Part` brings its name. `MultipartBody.Part` is a replacement for Retrofit 1's `TypedOutput`/`TypedFile` for doing uploads of files with names.
llp · Mar 10, 2016 · 28d3d65 · 28d3d65
1 parent 1373b58
commit 28d3d65
Show file tree

Hide file tree

Showing 5 changed files with 205 additions and 27 deletions.
diff --git a/retrofit/src/main/java/retrofit2/ParameterHandler.java b/retrofit/src/main/java/retrofit2/ParameterHandler.java
@@ -21,6 +21,7 @@
 import java.net.URI;
 import java.util.Map;
 import okhttp3.Headers;
+import okhttp3.MultipartBody;
 import okhttp3.RequestBody;
 
 import static retrofit2.Utils.checkNotNull;
@@ -220,6 +221,19 @@ static final class Part<T> extends ParameterHandler<T> {
     }
   }
 
+  static final class RawPart extends ParameterHandler<MultipartBody.Part> {
+    static final RawPart INSTANCE = new RawPart();
+
+    private RawPart() {
+    }
+
+    @Override void apply(RequestBuilder builder, MultipartBody.Part value) throws IOException {
+      if (value != null) { // Skip null values.
+        builder.addPart(value);
+      }
+    }
+  }
+
   static final class PartMap<T> extends ParameterHandler<Map<String, T>> {
     private final Converter<T, RequestBody> valueConverter;
     private final String transferEncoding;

diff --git a/retrofit/src/main/java/retrofit2/RequestBuilder.java b/retrofit/src/main/java/retrofit2/RequestBuilder.java
@@ -167,6 +167,10 @@ void addPart(Headers headers, RequestBody body) {
     multipartBuilder.addPart(headers, body);
   }
 
+  void addPart(MultipartBody.Part part) {
+    multipartBuilder.addPart(part);
+  }
+
   void setBody(RequestBody body) {
     this.body = body;
   }

diff --git a/retrofit/src/main/java/retrofit2/ServiceMethod.java b/retrofit/src/main/java/retrofit2/ServiceMethod.java
@@ -29,6 +29,7 @@
 import okhttp3.Headers;
 import okhttp3.HttpUrl;
 import okhttp3.MediaType;
+import okhttp3.MultipartBody;
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.ResponseBody;
@@ -535,33 +536,47 @@ private ParameterHandler<?> parseParameterAnnotation(
           throw parameterError(p, "@Part parameters can only be used with multipart encoding.");
         }
         Part part = (Part) annotation;
-        Headers headers = Headers.of(
-            "Content-Disposition", "form-data; name=\"" + part.value() + "\"",
-            "Content-Transfer-Encoding", part.encoding());
+        gotPart = true;
 
+        String partName = part.value();
         Class<?> rawParameterType = Utils.getRawType(type);
-        gotPart = true;
-        if (Iterable.class.isAssignableFrom(rawParameterType)) {
-          if (!(type instanceof ParameterizedType)) {
-            throw parameterError(p, rawParameterType.getSimpleName()
-                + " must include generic type (e.g., "
-                + rawParameterType.getSimpleName()
-                + "<String>)");
+        if (partName.isEmpty()) {
+          if (!MultipartBody.Part.class.isAssignableFrom(rawParameterType)) {
+            throw parameterError(p,
+                "@Part annotation must supply a name or use MultipartBody.Part parameter type.");
           }
-          ParameterizedType parameterizedType = (ParameterizedType) type;
-          Type iterableType = Utils.getParameterUpperBound(0, parameterizedType);
-          Converter<?, RequestBody> converter = retrofit.requestBodyConverter(
-              iterableType, annotations, methodAnnotations);
-          return new ParameterHandler.Part<>(headers, converter).iterable();
-        } else if (rawParameterType.isArray()) {
-          Class<?> arrayComponentType = boxIfPrimitive(rawParameterType.getComponentType());
-          Converter<?, RequestBody> converter =
-              retrofit.requestBodyConverter(arrayComponentType, annotations, methodAnnotations);
-          return new ParameterHandler.Part<>(headers, converter).array();
+
+          return ParameterHandler.RawPart.INSTANCE;
         } else {
-          Converter<?, RequestBody> converter =
-              retrofit.requestBodyConverter(type, annotations, methodAnnotations);
-          return new ParameterHandler.Part<>(headers, converter);
+          Headers headers =
+              Headers.of("Content-Disposition", "form-data; name=\"" + partName + "\"",
+                  "Content-Transfer-Encoding", part.encoding());
+
+          if (Iterable.class.isAssignableFrom(rawParameterType)) {
+            if (!(type instanceof ParameterizedType)) {
+              throw parameterError(p, rawParameterType.getSimpleName()
+                  + " must include generic type (e.g., "
+                  + rawParameterType.getSimpleName()
+                  + "<String>)");
+            }
+            ParameterizedType parameterizedType = (ParameterizedType) type;
+            Type iterableType = Utils.getParameterUpperBound(0, parameterizedType);
+            Converter<?, RequestBody> converter =
+                retrofit.requestBodyConverter(iterableType, annotations, methodAnnotations);
+            return new ParameterHandler.Part<>(headers, converter).iterable();
+          } else if (rawParameterType.isArray()) {
+            Class<?> arrayComponentType = boxIfPrimitive(rawParameterType.getComponentType());
+            Converter<?, RequestBody> converter =
+                retrofit.requestBodyConverter(arrayComponentType, annotations, methodAnnotations);
+            return new ParameterHandler.Part<>(headers, converter).array();
+          } else if (MultipartBody.Part.class.isAssignableFrom(rawParameterType)) {
+            throw parameterError(p, "@Part parameters using the MultipartBody.Part must not "
+                + "include a part name in the annotation.");
+          } else {
+            Converter<?, RequestBody> converter =
+                retrofit.requestBodyConverter(type, annotations, methodAnnotations);
+            return new ParameterHandler.Part<>(headers, converter);
+          }
         }
 
       } else if (annotation instanceof PartMap) {
@@ -578,11 +593,18 @@ private ParameterHandler<?> parseParameterAnnotation(
           throw parameterError(p, "Map must include generic types (e.g., Map<String, String>)");
         }
         ParameterizedType parameterizedType = (ParameterizedType) mapType;
+
         Type keyType = Utils.getParameterUpperBound(0, parameterizedType);
         if (String.class != keyType) {
           throw parameterError(p, "@PartMap keys must be of type String: " + keyType);
         }
+
         Type valueType = Utils.getParameterUpperBound(1, parameterizedType);
+        if (MultipartBody.Part.class.isAssignableFrom(Utils.getRawType(valueType))) {
+          throw parameterError(p, "@PartMap values cannot be MultipartBody.Part. "
+              + "Use @Part List<Part> or a different value type instead.");
+        }
+
         Converter<?, RequestBody> valueConverter =
             retrofit.requestBodyConverter(valueType, annotations, methodAnnotations);
 

diff --git a/retrofit/src/main/java/retrofit2/http/Part.java b/retrofit/src/main/java/retrofit2/http/Part.java
@@ -26,12 +26,16 @@
 /**
  * Denotes a single part of a multi-part request.
  * <p>
- * The parameter type on which this annotation exists will be processed in one of two ways:
+ * The parameter type on which this annotation exists will be processed in one of three ways:
  * <ul>
+ * <li>If the type is {@link okhttp3.MultipartBody.Part} the contents will be used directly. Omit
+ * the name from the annotation (i.e., {@code @Part MultipartBody.Part part}).</li>
  * <li>If the type is {@link okhttp3.RequestBody RequestBody} the value will be used
- * directly with its content type.</li>
+ * directly with its content type. Supply the part name in the annotation (e.g.,
+ * {@code @Part("foo") RequestBody foo}).</li>
  * <li>Other object types will be converted to an appropriate representation by using
- * {@linkplain Converter a converter}.</li>
+ * {@linkplain Converter a converter}. Supply the part name in the annotation (e.g.,
+ * {@code @Part("foo") RequestBody foo}).</li>
  * </ul>
  * <p>
  * Values may be {@code null} which will omit them from the request body.
@@ -50,7 +54,11 @@
 @Target(PARAMETER)
 @Retention(RUNTIME)
 public @interface Part {
-  String value();
+  /**
+   * The name of the part. Required for all parameter types except
+   * {@link okhttp3.MultipartBody.Part}.
+   */
+  String value() default "";
   /** The {@code Content-Transfer-Encoding} of this part. */
   String encoding() default "binary";
 }
diff --git a/retrofit/src/test/java/retrofit2/RequestBuilderTest.java b/retrofit/src/test/java/retrofit2/RequestBuilderTest.java
@@ -27,6 +27,7 @@
 import java.util.Map;
 import java.util.concurrent.atomic.AtomicReference;
 import okhttp3.MediaType;
+import okhttp3.MultipartBody;
 import okhttp3.Request;
 import okhttp3.RequestBody;
 import okhttp3.ResponseBody;
@@ -1393,6 +1394,97 @@ Call<ResponseBody> method(@Part("ping") String[] ping) {
         .contains("\r\npong2\r\n--");
   }
 
+  @Test public void multipartRequiresName() {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@Part RequestBody part) {
+        return null;
+      }
+    }
+
+    try {
+      buildRequest(Example.class, new Object[] { null });
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "@Part annotation must supply a name or use MultipartBody.Part parameter type. (parameter #1)\n"
+              + "    for method Example.method");
+    }
+  }
+
+  @Test public void multipartOkHttpPartForbidsName() {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@Part("name") MultipartBody.Part part) {
+        return null;
+      }
+    }
+
+    try {
+      buildRequest(Example.class, new Object[] { null });
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "@Part parameters using the MultipartBody.Part must not include a part name in the annotation. (parameter #1)\n"
+              + "    for method Example.method");
+    }
+  }
+
+  @Test public void multipartOkHttpPart() throws IOException {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@Part MultipartBody.Part part) {
+        return null;
+      }
+    }
+
+    MultipartBody.Part part = MultipartBody.Part.createFormData("kit", "kat");
+    Request request = buildRequest(Example.class, part);
+    assertThat(request.method()).isEqualTo("POST");
+    assertThat(request.headers().size()).isZero();
+    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/");
+
+    RequestBody body = request.body();
+    Buffer buffer = new Buffer();
+    body.writeTo(buffer);
+    String bodyString = buffer.readUtf8();
+
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"kit\"\r\n")
+        .contains("\r\nkat\r\n--");
+  }
+
+  @Test public void multipartOkHttpPartWithFilename() throws IOException {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@Part MultipartBody.Part part) {
+        return null;
+      }
+    }
+
+    MultipartBody.Part part =
+        MultipartBody.Part.createFormData("kit", "kit.txt", RequestBody.create(null, "kat"));
+    Request request = buildRequest(Example.class, part);
+    assertThat(request.method()).isEqualTo("POST");
+    assertThat(request.headers().size()).isZero();
+    assertThat(request.url().toString()).isEqualTo("http://example.com/foo/bar/");
+
+    RequestBody body = request.body();
+    Buffer buffer = new Buffer();
+    body.writeTo(buffer);
+    String bodyString = buffer.readUtf8();
+
+    assertThat(bodyString)
+        .contains("Content-Disposition: form-data;")
+        .contains("name=\"kit\"; filename=\"kit.txt\"\r\n")
+        .contains("\r\nkat\r\n--");
+  }
+
   @Test public void multipartIterable() throws IOException {
     class Example {
       @Multipart //
@@ -1527,6 +1619,44 @@ Call<ResponseBody> method(@PartMap(encoding = "8-bit") Map<String, RequestBody>
         .contains("\r\nkat\r\n--");
   }
 
+  @Test public void multipartPartMapRejectsNonStringKeys() {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@PartMap Map<Object, RequestBody> parts) {
+        return null;
+      }
+    }
+
+    try {
+      buildRequest(Example.class, new Object[] { null });
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "@PartMap keys must be of type String: class java.lang.Object (parameter #1)\n"
+              + "    for method Example.method");
+    }
+  }
+
+  @Test public void multipartPartMapRejectsOkHttpPartValues() {
+    class Example {
+      @Multipart //
+      @POST("/foo/bar/") //
+      Call<ResponseBody> method(@PartMap Map<String, MultipartBody.Part> parts) {
+        return null;
+      }
+    }
+
+    try {
+      buildRequest(Example.class, new Object[] { null });
+      fail();
+    } catch (IllegalArgumentException e) {
+      assertThat(e).hasMessage(
+          "@PartMap values cannot be MultipartBody.Part. Use @Part List<Part> or a different value type instead. (parameter #1)\n"
+              + "    for method Example.method");
+    }
+  }
+
   @Test public void multipartPartMapRejectsNull() {
     class Example {
       @Multipart //