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.

Sign up for GitHub

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

Properly support small files in multipart responses in REST Client Reactive #24014

Merged
merged 1 commit into from
Mar 1, 2022
Merged

Properly support small files in multipart responses in REST Client Reactive #24014

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 33 additions & 3 deletions ...oyment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void shouldParseMultipartResponse() {
Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class);
MultipartData data = client.getFile();
assertThat(data.file).exists();
verifyWooHooFile(data.file);
verifyWooHooFile(data.file, 10000);
assertThat(data.name).isEqualTo("foo");
assertThat(data.panda.weight).isEqualTo("huge");
assertThat(data.panda.height).isEqualTo("medium");
Expand All @@ -52,6 +52,18 @@ void shouldParseMultipartResponse() {
assertThat(data.numberz).containsSequence(2008, 2011, 2014);
}

@Test
void shouldParseMultipartResponseWithSmallFile() {
Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class);
MultipartData data = client.getSmallFile();
assertThat(data.file).exists();
verifyWooHooFile(data.file, 1);
assertThat(data.name).isEqualTo("foo");
assertThat(data.panda).isNull();
assertThat(data.number).isEqualTo(1984);
assertThat(data.numberz).isNull();
}

@Test
void shouldParseMultipartResponseWithNulls() {
Client client = RestClientBuilder.newBuilder().baseUri(baseUri).build(Client.class);
Expand Down Expand Up @@ -93,15 +105,15 @@ void shouldParseMultipartResponseWithClientBuilderApi() {
assertThat(data.numbers).containsSequence(2008, 2011, 2014);
}

void verifyWooHooFile(File file) {
void verifyWooHooFile(File file, int expectedTimes) {
int position = 0;
try (FileReader reader = new FileReader(file)) {
int read;
while ((read = reader.read()) > 0) {
assertThat((char) read).isEqualTo(WOO_HOO_WOO_HOO_HOO.charAt(position % WOO_HOO_WOO_HOO_HOO.length()));
position++;
}
assertThat(position).isEqualTo(WOO_HOO_WOO_HOO_HOO.length() * 10000);
assertThat(position).isEqualTo(WOO_HOO_WOO_HOO_HOO.length() * expectedTimes);
} catch (IOException e) {
fail("failed to read provided file", e);
}
Expand All @@ -113,6 +125,11 @@ public interface Client {
@Produces(MediaType.MULTIPART_FORM_DATA)
MultipartData getFile();

@GET
@Produces(MediaType.MULTIPART_FORM_DATA)
@Path("/small")
MultipartData getSmallFile();

@GET
@Produces(MediaType.MULTIPART_FORM_DATA)
@Path("/empty")
Expand Down Expand Up @@ -146,6 +163,19 @@ public MultipartData getFile() throws IOException {
1984, new int[] { 2008, 2011, 2014 });
}

@GET
@Produces(MediaType.MULTIPART_FORM_DATA)
@Path("/small")
public MultipartData getSmallFile() throws IOException {
File file = File.createTempFile("toDownload", ".txt");
file.deleteOnExit();
// let's write Woo hoo, woo hoo hoo 1 time
try (FileOutputStream out = new FileOutputStream(file)) {
out.write(WOO_HOO_WOO_HOO_HOO.getBytes(StandardCharsets.UTF_8));
}
return new MultipartData("foo", file, null, 1984, null);
}

@GET
@Produces(MediaType.MULTIPART_FORM_DATA)
@Path("/empty")
Expand Down
2 changes: 1 addition & 1 deletion ...in/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public QuarkusMultipartFormUpload(Context context,
io.netty.handler.codec.http.HttpMethod.POST,
"/");
Charset charset = parts.getCharset() != null ? parts.getCharset() : HttpConstants.DEFAULT_CHARSET;
DefaultHttpDataFactory httpDataFactory = new DefaultHttpDataFactory(DefaultHttpDataFactory.MINSIZE, charset) {
DefaultHttpDataFactory httpDataFactory = new DefaultHttpDataFactory(-1, charset) {
@Override
public FileUpload createFileUpload(HttpRequest request, String name, String filename, String contentType,
String contentTransferEncoding, Charset _charset, long size) {
Expand Down
27 changes: 5 additions & 22 deletions ...rg/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@
import io.netty.handler.codec.http.multipart.HttpData;
import io.netty.handler.codec.http.multipart.InterfaceHttpData;
import io.netty.handler.codec.http.multipart.MemoryAttribute;
import io.netty.handler.codec.http.multipart.MemoryFileUpload;
import io.netty.handler.codec.http.multipart.MixedAttribute;
import io.netty.handler.codec.http.multipart.MixedFileUpload;
import io.vertx.core.http.HttpClientResponse;
import java.io.IOException;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -240,31 +238,16 @@ public Attribute createAttribute(HttpClientResponse response, String name, Strin
}

// to reuse netty stuff as much as possible, we use FileUpload class to represent the downloaded file
// the difference between this and the original is that we always use DiskFileUpload
public FileUpload createFileUpload(HttpClientResponse response, String name, String filename,
String contentType, String contentTransferEncoding, Charset charset,
long size) {
if (useDisk) {
FileUpload fileUpload = new DiskFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(response);
list.add(fileUpload);
return fileUpload;
}
if (checkSize) {
FileUpload fileUpload = new MixedFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size, minSize, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(response);
list.add(fileUpload);
return fileUpload;
}
MemoryFileUpload fileUpload = new MemoryFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size);
FileUpload fileUpload = new DiskFileUpload(name, filename, contentType,
contentTransferEncoding, charset, size, baseDir, deleteOnExit);
fileUpload.setMaxSize(maxSize);
checkHttpDataSize(fileUpload);
List<HttpData> list = getList(response);
list.add(fileUpload);
return fileUpload;
}

Expand Down