Properly support small files in multipart responses in REST Client Re…

…active fixes quarkusio#23986
michalszynkiewicz · Feb 28, 2022 · e0f3d69 · e0f3d69
1 parent ec4e267
commit e0f3d69
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 26 deletions.
diff --git a/...oyment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java b/...oyment/src/test/java/io/quarkus/rest/client/reactive/multipart/MultipartResponseTest.java
@@ -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");
@@ -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);
@@ -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);
         }
@@ -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")
@@ -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")

diff --git a/...in/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java b/...in/java/org/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartFormUpload.java
@@ -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) {

diff --git a/...rg/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java b/...rg/jboss/resteasy/reactive/client/impl/multipart/QuarkusMultipartResponseDataFactory.java
@@ -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;
@@ -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;
     }