From 947ded394490fc840b8191bc7ad69ae0ea5f5c7d Mon Sep 17 00:00:00 2001 From: Kamil Berdychowski Date: Fri, 3 Mar 2023 12:47:02 +0100 Subject: [PATCH] fix: Wrong progress reported to `ProgressListener` by `AbstractBoxMultipartRequest` (#1151) Fixes: #1149 --- .../box/sdk/AbstractBoxMultipartRequest.java | 66 +------------- .../com/box/sdk/RequestBodyFromCallback.java | 30 +++++++ .../com/box/sdk/RequestBodyFromStream.java | 53 +++++++++++ .../box/sdk/RequestBodyFromStreamTest.java | 90 +++++++++++++++++++ 4 files changed, 174 insertions(+), 65 deletions(-) create mode 100644 src/main/java/com/box/sdk/RequestBodyFromCallback.java create mode 100644 src/main/java/com/box/sdk/RequestBodyFromStream.java create mode 100644 src/test/java/com/box/sdk/RequestBodyFromStreamTest.java diff --git a/src/main/java/com/box/sdk/AbstractBoxMultipartRequest.java b/src/main/java/com/box/sdk/AbstractBoxMultipartRequest.java index c80c45c5f..d814cee02 100644 --- a/src/main/java/com/box/sdk/AbstractBoxMultipartRequest.java +++ b/src/main/java/com/box/sdk/AbstractBoxMultipartRequest.java @@ -10,7 +10,6 @@ import okhttp3.MultipartBody; import okhttp3.Request; import okhttp3.RequestBody; -import okio.BufferedSink; /** *

Base class for multipart uploads

@@ -20,7 +19,7 @@ */ abstract class AbstractBoxMultipartRequest extends BoxAPIRequest { protected static final String BOUNDARY = "da39a3ee5e6b4b0d3255bfef95601890afd80709"; - private static final int BUFFER_SIZE = 8192; + static final int BUFFER_SIZE = 8192; private final StringBuilder loggedRequest = new StringBuilder(); private final Map fields = new HashMap<>(); private InputStream inputStream; @@ -158,67 +157,4 @@ private RequestBody getBody(ProgressListener progressListener) { } } - private static final class RequestBodyFromStream extends RequestBody { - private final InputStream inputStream; - private final ProgressListener progressListener; - private final MediaType mediaType; - private final int contentLength; - - private RequestBodyFromStream(InputStream inputStream, MediaType mediaType, ProgressListener progressListener) { - this.inputStream = inputStream; - this.progressListener = progressListener; - this.mediaType = mediaType; - try { - this.contentLength = inputStream.available(); - } catch (IOException e) { - throw new RuntimeException("Cannot read input stream for upload", e); - } - } - - @Override - public long contentLength() { - return contentLength; - } - - @Override - public MediaType contentType() { - return mediaType; - } - - @Override - public void writeTo(BufferedSink bufferedSink) throws IOException { - byte[] buffer = new byte[BUFFER_SIZE]; - int n = this.inputStream.read(buffer); - int totalWritten = n; - while (n != -1) { - bufferedSink.write(buffer, 0, n); - if (progressListener != null) { - progressListener.onProgressChanged(totalWritten, this.contentLength()); - } - totalWritten += n; - n = this.inputStream.read(buffer); - } - } - } - - private static final class RequestBodyFromCallback extends RequestBody { - - private final UploadFileCallback callback; - private final MediaType mediaType; - - private RequestBodyFromCallback(UploadFileCallback callback, MediaType mediaType) { - this.callback = callback; - this.mediaType = mediaType; - } - - @Override - public MediaType contentType() { - return mediaType; - } - - @Override - public void writeTo(BufferedSink bufferedSink) throws IOException { - callback.writeToStream(bufferedSink.outputStream()); - } - } } diff --git a/src/main/java/com/box/sdk/RequestBodyFromCallback.java b/src/main/java/com/box/sdk/RequestBodyFromCallback.java new file mode 100644 index 000000000..552010496 --- /dev/null +++ b/src/main/java/com/box/sdk/RequestBodyFromCallback.java @@ -0,0 +1,30 @@ +package com.box.sdk; + +import java.io.IOException; +import okhttp3.MediaType; +import okhttp3.RequestBody; +import okio.BufferedSink; + +/** + * + */ +final class RequestBodyFromCallback extends RequestBody { + + private final UploadFileCallback callback; + private final MediaType mediaType; + + RequestBodyFromCallback(UploadFileCallback callback, MediaType mediaType) { + this.callback = callback; + this.mediaType = mediaType; + } + + @Override + public MediaType contentType() { + return mediaType; + } + + @Override + public void writeTo(BufferedSink bufferedSink) throws IOException { + callback.writeToStream(bufferedSink.outputStream()); + } +} diff --git a/src/main/java/com/box/sdk/RequestBodyFromStream.java b/src/main/java/com/box/sdk/RequestBodyFromStream.java new file mode 100644 index 000000000..a28275d1d --- /dev/null +++ b/src/main/java/com/box/sdk/RequestBodyFromStream.java @@ -0,0 +1,53 @@ +package com.box.sdk; + +import java.io.IOException; +import java.io.InputStream; +import okhttp3.MediaType; +import okhttp3.RequestBody; +import okio.BufferedSink; + +/** + * Utility class to write body from stream to BufferedSink used by OkHttp. + */ +final class RequestBodyFromStream extends RequestBody { + private final InputStream inputStream; + private final ProgressListener progressListener; + private final MediaType mediaType; + private final int contentLength; + + RequestBodyFromStream(InputStream inputStream, MediaType mediaType, ProgressListener progressListener) { + this.inputStream = inputStream; + this.progressListener = progressListener; + this.mediaType = mediaType; + try { + this.contentLength = inputStream.available(); + } catch (IOException e) { + throw new RuntimeException("Cannot read input stream for upload", e); + } + } + + @Override + public long contentLength() { + return contentLength; + } + + @Override + public MediaType contentType() { + return mediaType; + } + + @Override + public void writeTo(BufferedSink bufferedSink) throws IOException { + byte[] buffer = new byte[AbstractBoxMultipartRequest.BUFFER_SIZE]; + int n = this.inputStream.read(buffer); + int totalWritten = 0; + while (n != -1) { + bufferedSink.write(buffer, 0, n); + totalWritten += n; + if (progressListener != null) { + progressListener.onProgressChanged(totalWritten, this.contentLength()); + } + n = this.inputStream.read(buffer); + } + } +} diff --git a/src/test/java/com/box/sdk/RequestBodyFromStreamTest.java b/src/test/java/com/box/sdk/RequestBodyFromStreamTest.java new file mode 100644 index 000000000..7e3fbd10a --- /dev/null +++ b/src/test/java/com/box/sdk/RequestBodyFromStreamTest.java @@ -0,0 +1,90 @@ +package com.box.sdk; + +import static com.box.sdk.AbstractBoxMultipartRequest.BUFFER_SIZE; + +import java.io.ByteArrayInputStream; +import java.io.IOException; +import java.util.Random; +import java.util.concurrent.atomic.AtomicInteger; +import okhttp3.MediaType; +import okio.Buffer; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; +import org.junit.Test; + +public class RequestBodyFromStreamTest { + + @Test + public void reportCorrectProgressWhenFileIsEmpty() throws IOException { + ByteArrayInputStream inputStream = new ByteArrayInputStream(new byte[]{}); + ProgressListener progressListener = (numBytes, totalBytes) -> { + MatcherAssert.assertThat(numBytes, Matchers.is((long) 0)); + MatcherAssert.assertThat(totalBytes, Matchers.is((long) 0)); + }; + + RequestBodyFromStream request = new RequestBodyFromStream( + inputStream, MediaType.parse("application/json"), progressListener + ); + + request.writeTo(new Buffer()); + } + @Test + public void reportCorrectProgressWhenFileSizeIfLessThanBuffer() throws IOException { + int howManyBytes = 1000; + ByteArrayInputStream inputStream = new ByteArrayInputStream(generateBytes(howManyBytes)); + ProgressListener progressListener = (numBytes, totalBytes) -> { + MatcherAssert.assertThat(numBytes, Matchers.is((long) howManyBytes)); + MatcherAssert.assertThat(totalBytes, Matchers.is((long) howManyBytes)); + }; + + RequestBodyFromStream request = new RequestBodyFromStream( + inputStream, MediaType.parse("application/json"), progressListener + ); + + request.writeTo(new Buffer()); + } + + @Test + public void reportCorrectProgressWhenFileSizeIfEqualToBuffer() throws IOException { + int howManyBytes = BUFFER_SIZE; + ByteArrayInputStream inputStream = new ByteArrayInputStream(generateBytes(howManyBytes)); + ProgressListener progressListener = (numBytes, totalBytes) -> { + MatcherAssert.assertThat(numBytes, Matchers.is((long) howManyBytes)); + MatcherAssert.assertThat(totalBytes, Matchers.is((long) howManyBytes)); + }; + + RequestBodyFromStream request = new RequestBodyFromStream( + inputStream, MediaType.parse("application/json"), progressListener + ); + + request.writeTo(new Buffer()); + } + + @Test + public void reportCorrectProgressWhenFileSizeIfGreaterThanBuffer() throws IOException { + int howManyBytes = BUFFER_SIZE + 1000; + ByteArrayInputStream inputStream = new ByteArrayInputStream(generateBytes(howManyBytes)); + AtomicInteger counter = new AtomicInteger(0); + ProgressListener progressListener = (numBytes, totalBytes) -> { + if (counter.getAndIncrement() == 0) { + MatcherAssert.assertThat(numBytes, Matchers.is((long) BUFFER_SIZE)); + MatcherAssert.assertThat(totalBytes, Matchers.is((long) howManyBytes)); + } else { + MatcherAssert.assertThat(numBytes, Matchers.is((long) howManyBytes)); + MatcherAssert.assertThat(totalBytes, Matchers.is((long) howManyBytes)); + } + }; + + RequestBodyFromStream request = new RequestBodyFromStream( + inputStream, MediaType.parse("application/json"), progressListener + ); + + request.writeTo(new Buffer()); + } + + private byte[] generateBytes(int howManyBytes) { + byte[] bytes = new byte[howManyBytes]; + new Random().nextBytes(bytes); + return bytes; + } +}