-
Notifications
You must be signed in to change notification settings - Fork 7.3k
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May not be There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. | ||
|
@@ -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"; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 // | ||
|
@@ -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 // | ||
|
There was a problem hiding this comment.
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