From 84572a5ed57eb1d2d4bb953493622f10a9dc4d91 Mon Sep 17 00:00:00 2001 From: Olivier Lepage-Applin Date: Thu, 6 Jun 2024 10:48:46 -0400 Subject: [PATCH] correctly set total parts in MultipartDownloadResumeContext. Add LogCaptor assertion to integ test. --- ...artDownloadPauseResumeIntegrationTest.java | 22 ++++++++++++++----- .../s3/internal/GenericS3TransferManager.java | 6 +++-- .../MultipartDownloadResumeContext.java | 10 ++++++++- .../MultipartDownloaderSubscriber.java | 2 ++ 4 files changed, 32 insertions(+), 8 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/it/java/software/amazon/awssdk/transfer/s3/S3TransferManagerMultipartDownloadPauseResumeIntegrationTest.java b/services-custom/s3-transfer-manager/src/it/java/software/amazon/awssdk/transfer/s3/S3TransferManagerMultipartDownloadPauseResumeIntegrationTest.java index f8719983934f..dcbace143b5f 100644 --- a/services-custom/s3-transfer-manager/src/it/java/software/amazon/awssdk/transfer/s3/S3TransferManagerMultipartDownloadPauseResumeIntegrationTest.java +++ b/services-custom/s3-transfer-manager/src/it/java/software/amazon/awssdk/transfer/s3/S3TransferManagerMultipartDownloadPauseResumeIntegrationTest.java @@ -22,6 +22,9 @@ import java.io.File; import java.nio.file.Path; import java.time.Duration; +import java.util.List; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.core.LogEvent; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; import org.junit.jupiter.api.Test; @@ -29,6 +32,7 @@ import software.amazon.awssdk.core.waiters.Waiter; import software.amazon.awssdk.core.waiters.WaiterAcceptor; import software.amazon.awssdk.services.s3.model.PutObjectRequest; +import software.amazon.awssdk.testutils.LogCaptor; import software.amazon.awssdk.testutils.RandomTempFile; import software.amazon.awssdk.transfer.s3.model.DownloadFileRequest; import software.amazon.awssdk.transfer.s3.model.FileDownload; @@ -44,12 +48,10 @@ public class S3TransferManagerMultipartDownloadPauseResumeIntegrationTest extend @BeforeAll public static void setup() throws Exception { - System.out.println("CREATING BUCKET"); createBucket(BUCKET); sourceFile = new RandomTempFile(OBJ_SIZE); // use async client for multipart upload (with default part size) - System.out.println("UPLOADING TEST OBJECT"); s3Async.putObject(PutObjectRequest.builder() .bucket(BUCKET) .key(KEY) @@ -90,9 +92,19 @@ void pauseAndResume_whenAlreadyComplete_shouldHandleGracefully() { FileDownload download = tmJava.downloadFile(request); download.completionFuture().join(); ResumableFileDownload resume = download.pause(); - FileDownload resumedDownload = tmJava.resumeDownloadFile(resume); - assertThat(resumedDownload.completionFuture()).isCompleted(); - assertThat(path.toFile()).hasSameBinaryContentAs(sourceFile); + try (LogCaptor logCaptor = LogCaptor.create(Level.DEBUG)) { + FileDownload resumedDownload = tmJava.resumeDownloadFile(resume); + assertThat(resumedDownload.completionFuture()).isCompleted(); + assertThat(path.toFile()).hasSameBinaryContentAs(sourceFile); + + List logEvents = logCaptor.loggedEvents(); + assertThat(logEvents).noneMatch( + event -> event.getMessage().getFormattedMessage().contains("Sending downloadFileRequest")); + LogEvent firstLog = logEvents.get(0); + assertThat(firstLog.getMessage().getFormattedMessage()) + .contains("The multipart download associated to the provided ResumableFileDownload is already completed, " + + "nothing to resume"); + } } private void waitUntilAmountTransferred(FileDownload download, long amountTransferred) { diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java index 94b0f13476ff..0e6e47ce8d90 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/GenericS3TransferManager.java @@ -305,7 +305,7 @@ private CopyObjectRequest attachSdkAttribute(CopyObjectRequest copyObjectRequest .build(); } - private GetObjectRequest attachSdkAttributes(GetObjectRequest request, + private GetObjectRequest attachSdkAttribute(GetObjectRequest request, Consumer builderMutation) { AwsRequestOverrideConfiguration modifiedRequestOverrideConfig = request.overrideConfiguration() @@ -373,7 +373,7 @@ public final Download download(DownloadRequest downl public final FileDownload downloadFile(DownloadFileRequest downloadRequest) { Validate.paramNotNull(downloadRequest, "downloadFileRequest"); - GetObjectRequest getObjectRequestWithAttributes = attachSdkAttributes( + GetObjectRequest getObjectRequestWithAttributes = attachSdkAttribute( downloadRequest.getObjectRequest(), b -> b.putExecutionAttribute(MULTIPART_DOWNLOAD_RESUME_CONTEXT, new MultipartDownloadResumeContext())); DownloadFileRequest downloadFileRequestWithAttributes = @@ -429,6 +429,8 @@ public final FileDownload resumeDownloadFile(ResumableFileDownload resumableFile Optional optCtx = multipartDownloadResumeContext(resumableFileDownload.downloadFileRequest().getObjectRequest()); if (optCtx.map(MultipartDownloadResumeContext::isComplete).orElse(false)) { + log.debug(() -> "The multipart download associated to the provided ResumableFileDownload is already completed, " + + "nothing to resume"); return completedDownload(resumableFileDownload, optCtx.get()); } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloadResumeContext.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloadResumeContext.java index f2337421ae0f..7619074592e5 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloadResumeContext.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloadResumeContext.java @@ -42,7 +42,7 @@ public class MultipartDownloadResumeContext { private Long bytesToLastCompletedParts; /** - * The total amount of part of the multipart download; + * The total number of parts of the multipart download. */ private Integer totalParts; @@ -78,6 +78,14 @@ public void addToBytesToLastCompletedParts(long bytes) { bytesToLastCompletedParts += bytes; } + public void totalParts(int totalParts) { + this.totalParts = totalParts; + } + + public Integer totalParts() { + return totalParts; + } + public GetObjectResponse response() { return this.response; } diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloaderSubscriber.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloaderSubscriber.java index 508ffbdb87a1..782a59d9bbb5 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloaderSubscriber.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartDownloaderSubscriber.java @@ -142,6 +142,8 @@ private void requestMoreIfNeeded(GetObjectResponse response) { Integer partCount = response.partsCount(); if (partCount != null && totalParts == null) { log.trace(() -> String.format("total parts: %d", partCount)); + MultipartDownloadUtils.multipartDownloadResumeContext(getObjectRequest) + .ifPresent(ctx -> ctx.totalParts(partCount)); totalParts = partCount; } synchronized (lock) {