pr comments

- Subscriber
- logs
- typos & javadoc

Author: L-Applin

core/sdk-core/src/main/java/software/amazon/awssdk/core/FileTransformerConfiguration.java

@@ -48,7 +48,7 @@ private FileTransformerConfiguration(DefaultBuilder builder) {
         this.fileWriteOption = Validate.paramNotNull(builder.fileWriteOption, "fileWriteOption");
         this.failureBehavior = Validate.paramNotNull(builder.failureBehavior, "failureBehavior");
         this.executorService = builder.executorService;
-        this.position = builder.position;
+        this.position = Validate.isNotNegativeOrNull(builder.position, "position");
         if (fileWriteOption != FileWriteOption.WRITE_TO_POSITION && position != null) {
             throw new IllegalArgumentException(String.format(
                 "'position' can only be used with 'WRITE_TO_POSITION' file write option, but was used with '%s'",

core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/async/SplittingTransformer.java

@@ -270,6 +270,7 @@ public CompletableFuture<ResponseT> prepare() {
                 if (isCancelled.get()) {
                     return individualFuture;
                 }
+                log.trace(() -> "calling prepare on the upstream transformer");
                 CompletableFuture<ResultT> upstreamFuture = upstreamResponseTransformer.prepare();
                 if (!resultFuture.isDone()) {
                     CompletableFutureUtils.forwardResultTo(upstreamFuture, resultFuture);
@@ -320,6 +321,7 @@ public void onStream(SdkPublisher<ByteBuffer> publisher) {
         @Override
         public void exceptionOccurred(Throwable error) {
             publisherToUpstream.error(error);
+            log.trace(() -> "calling exceptionOccurred on the upstream transformer");
             upstreamResponseTransformer.exceptionOccurred(error);
         }
     }

services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java

@@ -79,7 +79,8 @@
  *                              .build();
  * }
  *
- * <b>Create an S3 Transfer Manager with S3 Multipart Async Client</b>
+ * <b>Create an S3 Transfer Manager with S3 Multipart Async Client</b>. The S3 Multipart Async Client is an alternative to the CRT
+ * client that offers the same multipart upload/download feature.
  * {@snippet :
  *      S3AsyncClient s3AsyncClient = s3AsyncClient.builder()
  *                                                 .multipartEnabled(true)

services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/model/ResumableFileDownload.java

@@ -23,6 +23,7 @@
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Instant;
+import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
@@ -72,7 +73,8 @@ private ResumableFileDownload(DefaultBuilder builder) {
         this.s3ObjectLastModified = builder.s3ObjectLastModified;
         this.totalSizeInBytes = Validate.isPositiveOrNull(builder.totalSizeInBytes, "totalSizeInBytes");
         this.fileLastModified = builder.fileLastModified;
-        this.completedParts = Validate.getOrDefault(builder.completedParts, Collections::emptyList);
+        List<Integer> compledPartsList =  Validate.getOrDefault(builder.completedParts, Collections::emptyList);
+        this.completedParts = Collections.unmodifiableList(new ArrayList<>(compledPartsList));
     }
 
     @Override
@@ -163,7 +165,7 @@ public OptionalLong totalSizeInBytes() {
      * @return part numbers of a multipart download that were completed saved to file.
      */
     public List<Integer> completedParts() {
-        return Collections.unmodifiableList(completedParts);
+        return completedParts;
     }
 
     @Override

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/DownloadObjectHelper.java

@@ -41,7 +41,7 @@ public DownloadObjectHelper(S3AsyncClient s3AsyncClient, long bufferSizeInBytes)
     public <T> CompletableFuture<T> downloadObject(
         GetObjectRequest getObjectRequest, AsyncResponseTransformer<GetObjectResponse, T> asyncResponseTransformer) {
         if (getObjectRequest.range() != null || getObjectRequest.partNumber() != null) {
-            logSinglePartWarning(getObjectRequest);
+            logSinglePartMessage(getObjectRequest);
             return s3AsyncClient.getObject(getObjectRequest, asyncResponseTransformer);
         }
         GetObjectRequest requestToPerform = getObjectRequest.toBuilder().checksumMode(ChecksumMode.ENABLED).build();
@@ -62,7 +62,7 @@ private MultipartDownloaderSubscriber subscriber(GetObjectRequest getObjectReque
             .orElseGet(() -> new MultipartDownloaderSubscriber(s3AsyncClient, getObjectRequest));
     }
 
-    private void logSinglePartWarning(GetObjectRequest getObjectRequest) {
+    private void logSinglePartMessage(GetObjectRequest getObjectRequest) {
         String reason = "";
         if (getObjectRequest.range() != null) {
             reason = " because getObjectRequest range is included in the request."

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloadResumeContext.java

@@ -95,9 +95,10 @@ public void response(GetObjectResponse response) {
     }
 
     /**
-     * Return the highest sequentially completed part, 0 means no parts completed
+     * @return the highest sequentially completed part, 0 means no parts completed. Used for non-sequential operation when parts
+     * may have been completed in a non-sequential order. For example, if parts [1, 2, 3, 6, 7, 10] were completed, this
+     * method will return 3.
      *
-     * @return
      */
     public int highestSequentialCompletedPart() {
         if (completedParts.isEmpty() || completedParts.first() != 1) {

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloaderSubscriber.java

@@ -48,7 +48,7 @@ public class MultipartDownloaderSubscriber implements Subscriber<AsyncResponseTr
 
     /**
      * This value indicates the total number of parts of the object to get. If null, it means we don't know the total amount of
-     * parts, wither because we haven't received a response from s3 yet to set it, or the object to get is not multipart.
+     * parts, either because we haven't received a response from s3 yet to set it, or the object to get is not multipart.
      */
     private volatile Integer totalParts;
 
@@ -74,7 +74,7 @@ public class MultipartDownloaderSubscriber implements Subscriber<AsyncResponseTr
     /**
      * The etag of the object being downloaded.
      */
-    private String eTag;
+    private volatile String eTag;
 
     public MultipartDownloaderSubscriber(S3AsyncClient s3, GetObjectRequest getObjectRequest) {
         this(s3, getObjectRequest, 0);
@@ -101,6 +101,7 @@ public void onSubscribe(Subscription s) {
     public void onNext(AsyncResponseTransformer<GetObjectResponse, GetObjectResponse> asyncResponseTransformer) {
         log.trace(() -> String.format("onNext, completed part = %d", completedParts.get()));
         if (asyncResponseTransformer == null) {
+            subscription.cancel();
             throw new NullPointerException("onNext must not be called with null asyncResponseTransformer");
         }
 
@@ -111,6 +112,7 @@ public void onNext(AsyncResponseTransformer<GetObjectResponse, GetObjectResponse
                 logMulitpartComplete(totalParts);
                 subscription.cancel();
             }
+            return;
         }
 
         GetObjectRequest actualRequest = nextRequest(nextPartToGet);