From 2cd1664907cc3a78ba59f4548caef2a002de2db5 Mon Sep 17 00:00:00 2001 From: Dongie Agnir Date: Mon, 30 Dec 2019 16:07:09 -0800 Subject: [PATCH] Add RequestBody.fromRemainingByteBuffer() Fixes #1534 --- .../feature-AWSSDKforJavav2-e8d7ec5.json | 5 +++ .../amazon/awssdk/core/sync/RequestBody.java | 15 +++++++ .../awssdk/core/sync/RequestBodyTest.java | 20 ++++++++- .../amazon/awssdk/utils/BinaryUtils.java | 24 +++++++++++ .../amazon/awssdk/utils/BinaryUtilsTest.java | 42 +++++++++++++++++++ 5 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 .changes/next-release/feature-AWSSDKforJavav2-e8d7ec5.json diff --git a/.changes/next-release/feature-AWSSDKforJavav2-e8d7ec5.json b/.changes/next-release/feature-AWSSDKforJavav2-e8d7ec5.json new file mode 100644 index 000000000000..b59223eb7ffe --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-e8d7ec5.json @@ -0,0 +1,5 @@ +{ + "type": "feature", + "category": "AWS SDK for Java v2", + "description": "Add `RequestBody.fromRemainingByteBuffer(ByteBuffer)` that copies only the remaining readable bytes of the buffer. See [#1534](https://github.com/aws/aws-sdk-java-v2/issues/1534)" +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java index 22797469fc24..6559935243b0 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java @@ -161,6 +161,9 @@ public static RequestBody fromBytes(byte[] bytes) { /** * Creates a {@link RequestBody} from a {@link ByteBuffer}. Buffer contents are copied so any modifications * made to the original {@link ByteBuffer} are not reflected in the {@link RequestBody}. + *

+ * NOTE: This method always copies the entire contents of the buffer, ignoring the current read position. Use + * {@link #fromRemainingByteBuffer(ByteBuffer)} if you need it to copy only the remaining readable bytes. * * @param byteBuffer ByteBuffer to send to the service. * @return RequestBody instance. @@ -169,6 +172,18 @@ public static RequestBody fromByteBuffer(ByteBuffer byteBuffer) { return fromBytesDirect(BinaryUtils.copyAllBytesFrom(byteBuffer)); } + /** + * Creates a {@link RequestBody} from the remaining readable bytes from a {@link ByteBuffer}. Unlike + * {@link #fromByteBuffer(ByteBuffer)}, this method respects the current read position of the buffer and reads only + * the remaining bytes. The buffer is copied before reading so no changes are made to original buffer. + * + * @param byteBuffer ByteBuffer to send to the service. + * @return RequestBody instance. + */ + public static RequestBody fromRemainingByteBuffer(ByteBuffer byteBuffer) { + return fromBytesDirect(BinaryUtils.copyRemainingBytesFrom(byteBuffer)); + } + /** * Creates a {@link RequestBody} with no content. * diff --git a/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/RequestBodyTest.java b/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/RequestBodyTest.java index 6191f8db0173..7150dbe8e447 100644 --- a/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/RequestBodyTest.java +++ b/core/sdk-core/src/test/java/software/amazon/awssdk/core/sync/RequestBodyTest.java @@ -20,7 +20,6 @@ import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; import java.io.File; -import java.io.FileInputStream; import java.io.FileWriter; import java.io.IOException; import java.io.InputStream; @@ -104,4 +103,23 @@ public void emptyBytesConstructorHasCorrectContentType() { RequestBody requestBody = RequestBody.empty(); assertThat(requestBody.contentType()).isEqualTo(Mimetype.MIMETYPE_OCTET_STREAM); } + + @Test + public void remainingByteBufferConstructorOnlyRemainingBytesCopied() throws IOException { + ByteBuffer bb = ByteBuffer.allocate(4); + bb.put(new byte[]{1, 2, 3, 4}); + bb.flip(); + + bb.get(); + bb.get(); + + int originalRemaining = bb.remaining(); + + RequestBody requestBody = RequestBody.fromRemainingByteBuffer(bb); + + assertThat(requestBody.contentLength()).isEqualTo(originalRemaining); + + byte[] requestBodyBytes = IoUtils.toByteArray(requestBody.contentStreamProvider().newStream()); + assertThat(ByteBuffer.wrap(requestBodyBytes)).isEqualTo(bb); + } } diff --git a/utils/src/main/java/software/amazon/awssdk/utils/BinaryUtils.java b/utils/src/main/java/software/amazon/awssdk/utils/BinaryUtils.java index 8737b88d0514..efa6cd5fcb56 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/BinaryUtils.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/BinaryUtils.java @@ -28,6 +28,7 @@ */ @SdkProtectedApi public final class BinaryUtils { + private static final byte[] EMPTY_BYTE_ARRAY = new byte[0]; private BinaryUtils() { } @@ -150,6 +151,29 @@ public static byte[] copyAllBytesFrom(ByteBuffer bb) { return dst; } + public static byte[] copyRemainingBytesFrom(ByteBuffer bb) { + if (bb == null) { + return null; + } + + if (!bb.hasRemaining()) { + return EMPTY_BYTE_ARRAY; + } + + if (bb.hasArray()) { + int endIdx = bb.arrayOffset() + bb.limit(); + int startIdx = endIdx - bb.remaining(); + return Arrays.copyOfRange(bb.array(), startIdx, endIdx); + } + + ByteBuffer copy = bb.asReadOnlyBuffer(); + + byte[] dst = new byte[copy.remaining()]; + copy.get(dst); + + return dst; + } + /** * Returns a copy of the bytes from the given ByteBuffer, * ranging from the the buffer's current position to the buffer's limit; or diff --git a/utils/src/test/java/software/amazon/awssdk/utils/BinaryUtilsTest.java b/utils/src/test/java/software/amazon/awssdk/utils/BinaryUtilsTest.java index ed2987ebc5f1..10f4907d754c 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/BinaryUtilsTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/BinaryUtilsTest.java @@ -15,6 +15,7 @@ package software.amazon.awssdk.utils; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; @@ -159,4 +160,45 @@ public void testCopyBytesFrom_DirectByteBuffer_Idempotent() { assertTrue(partial1.length == 3); assertTrue(Arrays.equals(new byte[] {2, 3, 4}, partial1)); } + + @Test + public void testCopyRemainingBytesFrom_nullBuffer() { + assertThat(BinaryUtils.copyRemainingBytesFrom(null)).isNull(); + } + + @Test + public void testCopyRemainingBytesFrom_noRemainingBytes() { + ByteBuffer bb = ByteBuffer.allocate(1); + bb.put(new byte[]{1}); + bb.flip(); + + bb.get(); + + assertThat(BinaryUtils.copyRemainingBytesFrom(bb)).hasSize(0); + } + + @Test + public void testCopyRemainingBytesFrom_fullBuffer() { + ByteBuffer bb = ByteBuffer.allocate(4); + bb.put(new byte[]{1, 2, 3, 4}); + bb.flip(); + + byte[] copy = BinaryUtils.copyRemainingBytesFrom(bb); + assertThat(bb).isEqualTo(ByteBuffer.wrap(copy)); + assertThat(copy).hasSize(4); + } + + @Test + public void testCopyRemainingBytesFrom_partiallyReadBuffer() { + ByteBuffer bb = ByteBuffer.allocate(4); + bb.put(new byte[]{1, 2, 3, 4}); + bb.flip(); + + bb.get(); + bb.get(); + + byte[] copy = BinaryUtils.copyRemainingBytesFrom(bb); + assertThat(bb).isEqualTo(ByteBuffer.wrap(copy)); + assertThat(copy).hasSize(2); + } }