PR 5164 remaining comments (#5403)

* PR Comments
- Improved documentation
- Added warning for null future
- Remove superfluous isCancel() call
- lazy logging

Author: L-Applin

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

@@ -142,6 +142,15 @@ default SplitResult<ResponseT, ResultT> split(SplittingTransformerConfiguration
                                             .build();
     }
 
+    /**
+     * Creates an {@link SplitResult} which contains an {@link SplittingTransformer} that splits the
+     * {@link AsyncResponseTransformer} into multiple ones, publishing them as a {@link SdkPublisher}.
+     *
+     * @param splitConfig configuration for the split transformer
+     * @return SplitAsyncResponseTransformer instance.
+     * @see SplittingTransformer
+     * @see SplitResult
+     */
     default SplitResult<ResponseT, ResultT> split(Consumer<SplittingTransformerConfiguration.Builder> splitConfig) {
         SplittingTransformerConfiguration conf = SplittingTransformerConfiguration.builder()
                                                                                   .applyMutation(splitConfig)

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

@@ -163,6 +163,9 @@ public void exceptionOccurred(Throwable throwable) {
         }
         if (cf != null) {
             cf.completeExceptionally(throwable);
+        } else {
+            log.warn(() -> "An exception occurred before the call to prepare() was able to instantiate the CompletableFuture."
+                           + "The future cannot be completed exceptionally because it is null");
         }
     }
 

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

@@ -155,9 +155,6 @@ public void request(long n) {
                 downstreamSubscriber.onError(new IllegalArgumentException("Amount requested must be positive"));
                 return;
             }
-            if (isCancelled.get()) {
-                return;
-            }
             long newDemand = outstandingDemand.updateAndGet(current -> {
                 if (Long.MAX_VALUE - current < n) {
                     return Long.MAX_VALUE;

core/sdk-core/src/test/java/software/amazon/awssdk/core/internal/async/FileAsyncResponseTransformerTest.java

@@ -52,7 +52,6 @@
 import software.amazon.awssdk.core.FileTransformerConfiguration.FileWriteOption;
 import software.amazon.awssdk.core.FileTransformerConfiguration.FailureBehavior;
 import software.amazon.awssdk.core.async.SdkPublisher;
-import software.amazon.awssdk.core.util.FileUtils;
 
 /**
  * Tests for {@link FileAsyncResponseTransformer}.
@@ -335,9 +334,9 @@ private static void stubException(String newContent, FileAsyncResponseTransforme
         transformer.onStream(SdkPublisher.adapt(Flowable.just(content, content)));
         transformer.exceptionOccurred(runtimeException);
 
-        assertThatThrownBy(() -> future.get(10, TimeUnit.SECONDS))
-            .hasRootCause(runtimeException);
-        assertThat(future.isCompletedExceptionally()).isTrue();
+        assertThat(future).failsWithin(1, TimeUnit.SECONDS)
+            .withThrowableOfType(ExecutionException.class)
+            .withCause(runtimeException);
     }
 
     private static SdkPublisher<ByteBuffer> testPublisher(String content) {

services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/utils/ResumableRequestConverter.java

@@ -92,7 +92,7 @@ private ResumableRequestConverter() {
             return Pair.of(originalDownloadRequest, responseTransformer);
         }
 
-        // ranged GET for the remaining bytes (as a single request, no multipart)
+        // ranged GET for the remaining bytes.
         newDownloadFileRequest = resumedDownloadFileRequest(resumableFileDownload,
                                                             originalDownloadRequest,
                                                             getObjectRequest,

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

@@ -63,15 +63,16 @@ private MultipartDownloaderSubscriber subscriber(GetObjectRequest getObjectReque
     }
 
     private void logSinglePartMessage(GetObjectRequest getObjectRequest) {
-        String reason = "";
-        if (getObjectRequest.range() != null) {
-            reason = " because getObjectRequest range is included in the request."
-                     + " range = " + getObjectRequest.range();
-        } else if (getObjectRequest.partNumber() != null) {
-            reason = " because getObjectRequest part number is included in the request."
-                     + " part number = " + getObjectRequest.partNumber();
-        }
-        String finalReason = reason;
-        log.debug(() -> "Using single part download" + finalReason);
+        log.debug(() -> {
+            String reason = "";
+            if (getObjectRequest.range() != null) {
+                reason = " because getObjectRequest range is included in the request."
+                         + " range = " + getObjectRequest.range();
+            } else if (getObjectRequest.partNumber() != null) {
+                reason = " because getObjectRequest part number is included in the request."
+                         + " part number = " + getObjectRequest.partNumber();
+            }
+            return "Using single part download" + reason;
+        });
     }
 }

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

@@ -108,6 +108,8 @@ public int highestSequentialCompletedPart() {
             return 1;
         }
 
+        // for sequential operation, make sure we don't skip any non-completed part by returning the
+        // highest sequentially completed part
         int previous = completedParts.first();
         for (Integer i : completedParts) {
             if (i - previous > 1) {