Add an Optional getter for the contentLength of RequestBody, also add a new fromContentProvider method allowing contentLength to be null.

Author: Quanzzzz

.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."
+}

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();
     }

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<Long> 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.
      * <p>
      * 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).
      */

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);

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<Long>` 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<Long>` 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

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).
      *