Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for OkHttp's MultipartBody.Part. #1666

Merged
merged 1 commit into from
Mar 10, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions retrofit/src/main/java/retrofit2/ParameterHandler.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
4 changes: 4 additions & 0 deletions retrofit/src/main/java/retrofit2/RequestBuilder.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
68 changes: 45 additions & 23 deletions retrofit/src/main/java/retrofit2/ServiceMethod.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 + "\"",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably want to eventually restrict this to not contain ", etc.
Or escape it somehow

"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) {
Expand All @@ -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);

Expand Down
16 changes: 12 additions & 4 deletions retrofit/src/main/java/retrofit2/http/Part.java
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May not be RequestBody ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. 210be05

* </ul>
* <p>
* Values may be {@code null} which will omit them from the request body.
Expand All @@ -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";
}
130 changes: 130 additions & 0 deletions retrofit/src/test/java/retrofit2/RequestBuilderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍩

} 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();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat

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 //
Expand Down Expand Up @@ -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 //
Expand Down