diff --git a/.changes/next-release/bugfix-S3TransferManager-a0f0de6.json b/.changes/next-release/bugfix-S3TransferManager-a0f0de6.json new file mode 100644 index 000000000000..ff9c6183b4f6 --- /dev/null +++ b/.changes/next-release/bugfix-S3TransferManager-a0f0de6.json @@ -0,0 +1,6 @@ +{ + "type": "bugfix", + "category": "S3 Transfer Manager", + "contributor": "", + "description": "Skip downloading S3 folders (0-content-length folders created in the S3 console) in downloadDirectory in the S3 Transfer Manager." +} diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/config/DownloadFilter.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/config/DownloadFilter.java index e9cb4bdd8e1d..1398f7bd6ec4 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/config/DownloadFilter.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/config/DownloadFilter.java @@ -16,7 +16,6 @@ package software.amazon.awssdk.transfer.s3.config; import java.util.function.Predicate; -import software.amazon.awssdk.annotations.SdkPreviewApi; import software.amazon.awssdk.annotations.SdkPublicApi; import software.amazon.awssdk.services.s3.model.S3Object; import software.amazon.awssdk.transfer.s3.model.DownloadDirectoryRequest; @@ -28,7 +27,6 @@ * {@link #or(Predicate)} methods. */ @SdkPublicApi -@SdkPreviewApi public interface DownloadFilter extends Predicate { /** @@ -41,10 +39,18 @@ public interface DownloadFilter extends Predicate { boolean test(S3Object s3Object); /** - * A {@link DownloadFilter} that downloads all objects. This is the default behavior if no filter is provided. + * A {@link DownloadFilter} that downloads all non-folder objects. A folder is a 0-byte object created when a customer + * uses S3 console to create a folder, and it always ends with "/". + * + *

+ * This is the default behavior if no filter is provided. */ - @SdkPreviewApi static DownloadFilter allObjects() { - return ctx -> true; + return s3Object -> { + boolean isFolder = s3Object.key().endsWith("/") && + s3Object.size() != null && + s3Object.size() == 0; + return !isFolder; + }; } } diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/config/DownloadFilterTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/config/DownloadFilterTest.java new file mode 100644 index 000000000000..cc6da432435d --- /dev/null +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/config/DownloadFilterTest.java @@ -0,0 +1,41 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package software.amazon.awssdk.transfer.s3.config; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.stream.Stream; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import software.amazon.awssdk.services.s3.model.S3Object; + +public class DownloadFilterTest { + + public static Stream s3Objects() { + return Stream.of( + Arguments.of(S3Object.builder().key("no-slash-zero-content").size(0L).build(), true), + Arguments.of(S3Object.builder().key("slash-zero-content/").size(0L).build(), false), + Arguments.of(S3Object.builder().key("key").size(10L).build(), true) + ); + } + + @ParameterizedTest + @MethodSource("s3Objects") + void allObjectsFilter_shouldWork(S3Object s3Object, boolean result) { + assertThat(DownloadFilter.allObjects().test(s3Object)).isEqualTo(result); + } +} diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/DownloadDirectoryHelperTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/DownloadDirectoryHelperTest.java index 59cff162b3d4..700f65fb56d8 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/DownloadDirectoryHelperTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/DownloadDirectoryHelperTest.java @@ -54,6 +54,7 @@ import software.amazon.awssdk.services.s3.model.GetObjectRequest; import software.amazon.awssdk.services.s3.model.GetObjectResponse; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; +import software.amazon.awssdk.services.s3.model.S3Object; import software.amazon.awssdk.transfer.s3.internal.model.DefaultFileDownload; import software.amazon.awssdk.transfer.s3.internal.progress.DefaultTransferProgress; import software.amazon.awssdk.transfer.s3.internal.progress.DefaultTransferProgressSnapshot; @@ -124,6 +125,37 @@ void downloadDirectory_allDownloadsSucceed_failedDownloadsShouldBeEmpty() throws "key2")); } + @Test + void downloadDirectory_containsFolderObjects_shouldSkip() throws Exception { + stubSuccessfulListObjects(listObjectsHelper, S3Object.builder().key("key1").size(10L).build(), + S3Object.builder().key("key2").size(0L).build(), + S3Object.builder().key("folder/").size(0L).build()); + + FileDownload fileDownload = newSuccessfulDownload(); + FileDownload fileDownload2 = newSuccessfulDownload(); + + when(singleDownloadFunction.apply(any(DownloadFileRequest.class))).thenReturn(fileDownload, fileDownload2); + + DirectoryDownload downloadDirectory = + downloadDirectoryHelper.downloadDirectory(DownloadDirectoryRequest.builder() + .destination(directory) + .bucket("bucket") + .build()); + + CompletedDirectoryDownload completedDirectoryDownload = downloadDirectory.completionFuture().get(5, TimeUnit.SECONDS); + + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(DownloadFileRequest.class); + verify(singleDownloadFunction, times(2)).apply(argumentCaptor.capture()); + + assertThat(completedDirectoryDownload.failedTransfers()).isEmpty(); + List allValues = argumentCaptor.getAllValues(); + assertThat(allValues).size().isEqualTo(2); + assertThat(allValues).element(0).satisfies(d -> assertThat(d.getObjectRequest().key()).isEqualTo( + "key1")); + assertThat(allValues).element(1).satisfies(d -> assertThat(d.getObjectRequest().key()).isEqualTo( + "key2")); + } + @ParameterizedTest @ValueSource(strings = {"/blah", "../blah/object.dat", diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/util/S3ApiCallMockUtils.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/util/S3ApiCallMockUtils.java index 270ceb52d912..2f32ea081818 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/util/S3ApiCallMockUtils.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/util/S3ApiCallMockUtils.java @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; import java.util.stream.Collectors; +import org.apache.commons.lang3.RandomStringUtils; import software.amazon.awssdk.core.async.SdkPublisher; import software.amazon.awssdk.services.s3.model.ListObjectsV2Request; import software.amazon.awssdk.services.s3.model.S3Object; @@ -33,8 +34,14 @@ private S3ApiCallMockUtils() { } public static void stubSuccessfulListObjects(ListObjectsHelper helper, String... keys) { - List s3Objects = Arrays.stream(keys).map(k -> S3Object.builder().key(k).build()).collect(Collectors.toList()); + List s3Objects = + Arrays.stream(keys).map(k -> S3Object.builder().key(k).size(100L).build()).collect(Collectors.toList()); when(helper.listS3ObjectsRecursively(any(ListObjectsV2Request.class))).thenReturn(SdkPublisher.adapt(Flowable.fromIterable(s3Objects))); } + public static void stubSuccessfulListObjects(ListObjectsHelper helper, S3Object... s3Objects) { + when(helper.listS3ObjectsRecursively(any(ListObjectsV2Request.class))) + .thenReturn(SdkPublisher.adapt(Flowable.fromIterable(Arrays.asList(s3Objects)))); + } + }