diff --git a/.changes/next-release/feature-AWSSDKforJavav2-645c1ac.json b/.changes/next-release/feature-AWSSDKforJavav2-645c1ac.json new file mode 100644 index 000000000000..c6ee7e014ee0 --- /dev/null +++ b/.changes/next-release/feature-AWSSDKforJavav2-645c1ac.json @@ -0,0 +1,6 @@ +{ + "category": "AWS SDK for Java v2", + "contributor": "", + "type": "feature", + "description": "Add a new Optional getter for the content length of RequestBody, also add a new fromContentProvider method allowing contentLength to be null." +} diff --git a/core/sdk-core/src/main/java/software/amazon/awssdk/core/runtime/transform/StreamingRequestMarshaller.java b/core/sdk-core/src/main/java/software/amazon/awssdk/core/runtime/transform/StreamingRequestMarshaller.java index d275bab6c7aa..6b6f1ee0c13f 100644 --- a/core/sdk-core/src/main/java/software/amazon/awssdk/core/runtime/transform/StreamingRequestMarshaller.java +++ b/core/sdk-core/src/main/java/software/amazon/awssdk/core/runtime/transform/StreamingRequestMarshaller.java @@ -17,7 +17,6 @@ import static software.amazon.awssdk.http.Header.CONTENT_TYPE; -import java.util.Optional; import software.amazon.awssdk.annotations.SdkProtectedApi; import software.amazon.awssdk.core.internal.transform.AbstractStreamingRequestMarshaller; import software.amazon.awssdk.core.sync.RequestBody; @@ -56,7 +55,7 @@ public SdkHttpFullRequest marshall(T in) { // Currently, SDK always require content length in RequestBody. So we always // send Content-Length header for sync APIs // This change will be useful if SDK relaxes the content-length requirement in RequestBody - addHeaders(marshalled, Optional.of(requestBody.contentLength()), requiresLength, transferEncoding, useHttp2); + addHeaders(marshalled, requestBody.optionalContentLength(), requiresLength, transferEncoding, useHttp2); return marshalled.build(); } 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 c5debbae00e3..876de78dfef2 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 @@ -16,6 +16,7 @@ package software.amazon.awssdk.core.sync; import static software.amazon.awssdk.utils.FunctionalUtils.invokeSafely; +import static software.amazon.awssdk.utils.Validate.isNotNegative; import static software.amazon.awssdk.utils.Validate.paramNotNull; import static software.amazon.awssdk.utils.Validate.validState; @@ -28,6 +29,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.Arrays; +import java.util.Optional; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.core.internal.sync.FileContentStreamProvider; import software.amazon.awssdk.core.internal.util.Mimetype; @@ -46,14 +48,13 @@ public final class RequestBody { // TODO Handle stream management (progress listener, orig input stream tracking, etc private final ContentStreamProvider contentStreamProvider; - private final long contentLength; + private final Long contentLength; private final String contentType; - private RequestBody(ContentStreamProvider contentStreamProvider, long contentLength, String contentType) { + private RequestBody(ContentStreamProvider contentStreamProvider, Long contentLength, String contentType) { this.contentStreamProvider = paramNotNull(contentStreamProvider, "contentStreamProvider"); - this.contentLength = contentLength; + this.contentLength = contentLength != null ? isNotNegative(contentLength, "Content-length") : null; this.contentType = paramNotNull(contentType, "contentType"); - validState(contentLength >= 0, "Content length must be greater than or equal to zero"); } /** @@ -64,12 +65,23 @@ public ContentStreamProvider contentStreamProvider() { } /** + * @deprecated by {@link #optionalContentLength()} * @return Content length of {@link RequestBody}. */ + @Deprecated public long contentLength() { + validState(this.contentLength != null, + "Content length is invalid, please use optionalContentLength() for your case."); return contentLength; } + /** + * @return Optional object of content length of {@link RequestBody}. + */ + public Optional optionalContentLength() { + return Optional.ofNullable(contentLength); + } + /** * @return Content type of {@link RequestBody}. */ @@ -108,7 +120,8 @@ public static RequestBody fromFile(File file) { * could tamper with the sending of the request. *

* To support resetting via {@link ContentStreamProvider}, this uses {@link InputStream#reset()} and uses a read limit of - * 128 KiB. If you need more control, use {@link #fromContentProvider(ContentStreamProvider, long, String)}. + * 128 KiB. If you need more control, use {@link #fromContentProvider(ContentStreamProvider, long, String)} or + * {@link #fromContentProvider(ContentStreamProvider, String)}. * * @param inputStream Input stream to send to the service. The stream will not be closed by the SDK. * @param contentLength Content length of data in input stream. @@ -205,6 +218,18 @@ public static RequestBody fromContentProvider(ContentStreamProvider provider, lo return new RequestBody(provider, contentLength, mimeType); } + /** + * Creates a {@link RequestBody} from the given {@link ContentStreamProvider}. + * + * @param provider The content provider. + * @param mimeType The MIME type of the content. + * + * @return The created {@code RequestBody}. + */ + public static RequestBody fromContentProvider(ContentStreamProvider provider, String mimeType) { + return new RequestBody(provider, null, mimeType); + } + /** * Creates a {@link RequestBody} using the specified bytes (without copying). */ 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 c84003651b95..2959646885eb 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 @@ -16,9 +16,11 @@ package software.amazon.awssdk.core.sync; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import com.google.common.jimfs.Configuration; import com.google.common.jimfs.Jimfs; +import java.io.ByteArrayInputStream; import java.io.File; import java.io.FileWriter; import java.io.IOException; @@ -104,6 +106,15 @@ public void emptyBytesConstructorHasCorrectContentType() { assertThat(requestBody.contentType()).isEqualTo(Mimetype.MIMETYPE_OCTET_STREAM); } + @Test + public void contentProviderConstuctorWithNullContentLength_NoContentLength() { + byte[] bytes = new byte[0]; + RequestBody requestBody = RequestBody.fromContentProvider(() -> new ByteArrayInputStream(bytes), + Mimetype.MIMETYPE_OCTET_STREAM); + assertThat(requestBody.optionalContentLength().isPresent()).isFalse(); + assertThatThrownBy(() -> requestBody.contentLength()).isInstanceOf(IllegalStateException.class); + } + @Test public void remainingByteBufferConstructorOnlyRemainingBytesCopied() throws IOException { ByteBuffer bb = ByteBuffer.allocate(4); diff --git a/docs/design/core/sdk-core/sync/RequestBody/DecisionLog.md b/docs/design/core/sdk-core/sync/RequestBody/DecisionLog.md new file mode 100644 index 000000000000..a8315e63e2f7 --- /dev/null +++ b/docs/design/core/sdk-core/sync/RequestBody/DecisionLog.md @@ -0,0 +1,31 @@ +# Decision Log for SDK V2 RequestBody Content-Length + +## Log Entry Template + +**Source:** (Meeting/aside/pair programming discussion/daily standup) to (discuss/implement) X + +**Attendees:** Anna-Karin, Dongie, Nico, John, Zoe + +**Closed Decisions:** + +1. Question? Decision. Justification. + +**Open Decisions:** + +1. (Old/Reopened/New) Question? + +## 4/8/2021 + +**Source:** Meeting to discuss how to address the deprecated content-length getter after adding a new `Optional` getter in sync RequestBody: https://github.com/aws/aws-sdk-java-v2/blob/master/core/sdk-core/src/main/java/software/amazon/awssdk/core/sync/RequestBody.java#L66-L71 + +**Attendees:** Anna-Karin, Dongie, Nico, John, Zoe + +**Closed Decisions:** + +1. Should we replace the old content-length getter with a new optional getter? No, because that's going to be a breaking change, adding a Deprecated annotation is more acceptable. +2. Should we use a negative value to denote the null content-length? Yes, because a negative value can be distinguished from normal zero content-length. +3. Should we throw an exception in the deprecated getter when the content-length is negative? Yes, because with an exception the customers using this deprecated method would be notified that this method is deprecated and start to use the `Optional` getter. On the other hand, since the content-length was always greater or equal to zero before this change, throwing an exception here wouldn't break any existed customers. + +**Open Decisions:** + +None diff --git a/utils/src/main/java/software/amazon/awssdk/utils/Validate.java b/utils/src/main/java/software/amazon/awssdk/utils/Validate.java index c05564702e7c..7d98cc31246e 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/Validate.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/Validate.java @@ -628,6 +628,15 @@ public static int isNotNegative(int num, String fieldName) { return num; } + public static long isNotNegative(long num, String fieldName) { + + if (num < 0) { + throw new IllegalArgumentException(String.format("%s must not be negative", fieldName)); + } + + return num; + } + /** * Asserts that the given duration is positive (non-negative and non-zero). *