From a5ad71bb3fa660a1402a1783828312b73bb12807 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Thu, 8 Feb 2024 12:52:04 -0800 Subject: [PATCH 1/8] Enable setting multipart to true/false by defining it as a custom client context parameter --- .../poet/builder/AsyncClientBuilderClass.java | 13 ---- .../s3/internal/TransferManagerFactory.java | 10 ++- .../internal/TransferManagerLoggingTest.java | 64 ++++++++++++++----- .../client/S3AsyncClientDecorator.java | 6 +- .../codegen-resources/customization.config | 7 +- 5 files changed, 58 insertions(+), 42 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java index 0cc74f08bdb9..e0b4c0502970 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java @@ -81,7 +81,6 @@ public TypeSpec poetSpec() { MultipartCustomization multipartCustomization = model.getCustomizationConfig().getMultipartCustomization(); if (multipartCustomization != null) { - builder.addMethod(multipartEnabledMethod(multipartCustomization)); builder.addMethod(multipartConfigMethods(multipartCustomization)); } @@ -159,18 +158,6 @@ private MethodSpec tokenProviderMethod() { .build(); } - private MethodSpec multipartEnabledMethod(MultipartCustomization multipartCustomization) { - return MethodSpec.methodBuilder("multipartEnabled") - .addAnnotation(Override.class) - .addModifiers(Modifier.PUBLIC) - .returns(builderInterfaceName) - .addParameter(Boolean.class, "enabled") - .addStatement("clientContextParams.put($N, enabled)", - multipartCustomization.getContextParamEnabledKey()) - .addStatement("return this") - .build(); - } - private MethodSpec multipartConfigMethods(MultipartCustomization multipartCustomization) { ClassName mulitpartConfigClassName = PoetUtils.classNameFromFqcn(multipartCustomization.getMultipartConfigurationClass()); diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index a87d8d6fcd60..4cdc1d6b4b8c 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -52,11 +52,7 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder) return new CrtS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); } - if (s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.DefaultS3AsyncClient")) { - log.warn(() -> "The provided DefaultS3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" - + " upload/download feature is not enabled and resumable file upload is not supported. To benefit " - + "from maximum throughput, consider using S3AsyncClient.crtBuilder().build() instead."); - } else { + if (!Boolean.TRUE.equals(s3AsyncClient.serviceClientConfiguration().multipartEnabled())) { log.debug(() -> "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + " upload/download feature may not be enabled and resumable file upload may not be supported."); } @@ -68,7 +64,9 @@ private static Supplier defaultS3AsyncClient() { if (crtInClasspath()) { return S3AsyncClient::crtCreate; } - return S3AsyncClient::create; + return () -> S3AsyncClient.builder() + .multipartEnabled(true) + .build(); } private static boolean crtInClasspath() { diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index 4f2be4d00063..bbb6b8c1d2ca 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -17,12 +17,9 @@ import static org.assertj.core.api.Assertions.assertThat; -import java.util.HashSet; import java.util.List; -import java.util.Set; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.LogEvent; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.regions.Region; @@ -35,33 +32,66 @@ class TransferManagerLoggingTest { @Test void transferManager_withCrtClient_shouldNotLogWarnMessages() { - try (S3AsyncClient s3Crt = S3AsyncClient.crtBuilder() - .region(Region.US_WEST_2) - .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) - .build(); - LogCaptor logCaptor = LogCaptor.create(Level.WARN); - S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { + try (S3AsyncClient s3AsyncClient = S3AsyncClient.crtBuilder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .build(); + LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); + S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { List events = logCaptor.loggedEvents(); assertThat(events).isEmpty(); } } @Test - void transferManager_withJavaClient_shouldLogWarnMessage() { + void transferManager_withCustomJavaClientWhenMultiPartEnabledIsNull_shouldLogDebugMessage() { + try (S3AsyncClient s3AsyncClient = S3AsyncClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .build(); + LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); + S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { + List events = logCaptor.loggedEvents(); + assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + + " upload/download feature may not be enabled and resumable file upload may not be supported."); + } + } + @Test + void transferManager_withCustomJavaClientWhenMultiPartEnabledIsFalse_shouldLogDebugMessage() { + try (S3AsyncClient s3AsyncClient = S3AsyncClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .multipartEnabled(false) + .build(); + LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); + S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { + List events = logCaptor.loggedEvents(); + assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + + " upload/download feature may not be enabled and resumable file upload may not be supported."); + } + } + @Test + void transferManager_withDefaultJavaClient_shouldNotLogDebugMessage() { + try (LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); + S3TransferManager tm = S3TransferManager.builder().build()) { + List events = logCaptor.loggedEvents(); + assertThat(events).isEmpty(); + } + } + + @Test + void transferManager_withCustomJavaClientMultiPartEnabled_shouldNotLogDebugMessage() { try (S3AsyncClient s3Crt = S3AsyncClient.builder() .region(Region.US_WEST_2) .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .multipartEnabled(true) .build(); - LogCaptor logCaptor = LogCaptor.create(Level.WARN); + LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); - assertLogged(events, Level.WARN, "The provided DefaultS3AsyncClient is not an instance of S3CrtAsyncClient, and " - + "thus multipart upload/download feature is not enabled and resumable file upload" - + " is " - + "not supported. To benefit from maximum throughput, consider using " - + "S3AsyncClient.crtBuilder().build() instead."); + assertThat(events).isEmpty(); } } @@ -72,4 +102,4 @@ private static void assertLogged(List events, org.apache.logging.log4j assertThat(msg).isEqualTo(message); assertThat(event.getLevel()).isEqualTo(level); } -} +} \ No newline at end of file diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java index fe2997a7d73c..c9819f6cad92 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java @@ -33,8 +33,6 @@ public class S3AsyncClientDecorator { public static final AttributeMap.Key MULTIPART_CONFIGURATION_KEY = new AttributeMap.Key(MultipartConfiguration.class){}; - public static final AttributeMap.Key MULTIPART_ENABLED_KEY = - new AttributeMap.Key(Boolean.class){}; public S3AsyncClientDecorator() { } @@ -62,7 +60,7 @@ private Predicate isCrossRegionEnabledAsync(AttributeMap clientCo } private Predicate isMultipartEnable(AttributeMap clientContextParams) { - Boolean multipartEnabled = clientContextParams.get(MULTIPART_ENABLED_KEY); + Boolean multipartEnabled = clientContextParams.get(S3ClientContextParams.MULTIPART_ENABLED); return client -> multipartEnabled != null && multipartEnabled.booleanValue(); } -} +} \ No newline at end of file diff --git a/services/s3/src/main/resources/codegen-resources/customization.config b/services/s3/src/main/resources/codegen-resources/customization.config index 7bd3369c2b88..b12ffbbd3e38 100644 --- a/services/s3/src/main/resources/codegen-resources/customization.config +++ b/services/s3/src/main/resources/codegen-resources/customization.config @@ -241,7 +241,6 @@ "multipartConfigurationClass": "software.amazon.awssdk.services.s3.multipart.MultipartConfiguration", "multipartConfigMethodDoc": "Configuration for multipart operation of this client.", "multipartEnableMethodDoc": "Enables automatic conversion of put and copy method to their equivalent multipart operation.", - "contextParamEnabledKey": "S3AsyncClientDecorator.MULTIPART_ENABLED_KEY", "contextParamConfigKey": "S3AsyncClientDecorator.MULTIPART_CONFIGURATION_KEY" }, "interceptors": [ @@ -271,6 +270,10 @@ "CrossRegionAccessEnabled":{ "documentation":"Enables cross-region bucket access for this client", "type":"boolean" + }, + "multipartEnabled":{ + "documentation":"Enables automatic conversion of put and copy method to their equivalent multipart operation.", + "type":"boolean" } }, "endpointAuthSchemeConfig": { @@ -285,4 +288,4 @@ }, "useSraAuth": true, "enableGenerateCompiledEndpointRules": true -} +} \ No newline at end of file From 01c5cb95f9135c3dc115f200fb8e6851a6821727 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Thu, 8 Feb 2024 13:53:07 -0800 Subject: [PATCH 2/8] Refactor indentation --- .../awssdk/transfer/s3/internal/TransferManagerFactory.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index 4cdc1d6b4b8c..688be96a4c95 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -64,9 +64,7 @@ private static Supplier defaultS3AsyncClient() { if (crtInClasspath()) { return S3AsyncClient::crtCreate; } - return () -> S3AsyncClient.builder() - .multipartEnabled(true) - .build(); + return () -> S3AsyncClient.builder().multipartEnabled(true).build(); } private static boolean crtInClasspath() { From b3bed43d3855e96dd7e2df2deb8a8abdf7dd7c29 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Sat, 10 Feb 2024 20:55:35 -0800 Subject: [PATCH 3/8] Checking if multi-part is enabled before logging a debug message --- .../poet/builder/AsyncClientBuilderClass.java | 13 +++++ .../s3/internal/TransferManagerFactory.java | 2 +- .../internal/TransferManagerLoggingTest.java | 52 +++++++++++-------- .../client/S3AsyncClientDecorator.java | 4 +- .../codegen-resources/customization.config | 5 +- 5 files changed, 49 insertions(+), 27 deletions(-) diff --git a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java index e0b4c0502970..0cc74f08bdb9 100644 --- a/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java +++ b/codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java @@ -81,6 +81,7 @@ public TypeSpec poetSpec() { MultipartCustomization multipartCustomization = model.getCustomizationConfig().getMultipartCustomization(); if (multipartCustomization != null) { + builder.addMethod(multipartEnabledMethod(multipartCustomization)); builder.addMethod(multipartConfigMethods(multipartCustomization)); } @@ -158,6 +159,18 @@ private MethodSpec tokenProviderMethod() { .build(); } + private MethodSpec multipartEnabledMethod(MultipartCustomization multipartCustomization) { + return MethodSpec.methodBuilder("multipartEnabled") + .addAnnotation(Override.class) + .addModifiers(Modifier.PUBLIC) + .returns(builderInterfaceName) + .addParameter(Boolean.class, "enabled") + .addStatement("clientContextParams.put($N, enabled)", + multipartCustomization.getContextParamEnabledKey()) + .addStatement("return this") + .build(); + } + private MethodSpec multipartConfigMethods(MultipartCustomization multipartCustomization) { ClassName mulitpartConfigClassName = PoetUtils.classNameFromFqcn(multipartCustomization.getMultipartConfigurationClass()); diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index 688be96a4c95..b2c9305ead32 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -52,7 +52,7 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder) return new CrtS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); } - if (!Boolean.TRUE.equals(s3AsyncClient.serviceClientConfiguration().multipartEnabled())) { + if (!s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient")) { log.debug(() -> "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + " upload/download feature may not be enabled and resumable file upload may not be supported."); } diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index bbb6b8c1d2ca..42b752694438 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -17,9 +17,12 @@ import static org.assertj.core.api.Assertions.assertThat; +import java.util.HashSet; import java.util.List; +import java.util.Set; import org.apache.logging.log4j.Level; import org.apache.logging.log4j.core.LogEvent; +import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import software.amazon.awssdk.auth.credentials.AwsBasicCredentials; import software.amazon.awssdk.regions.Region; @@ -32,25 +35,27 @@ class TransferManagerLoggingTest { @Test void transferManager_withCrtClient_shouldNotLogWarnMessages() { - try (S3AsyncClient s3AsyncClient = S3AsyncClient.crtBuilder() - .region(Region.US_WEST_2) - .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) - .build(); - LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); - S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { + try (S3AsyncClient s3Crt = S3AsyncClient.crtBuilder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .build(); + LogCaptor logCaptor = LogCaptor.create(Level.WARN); + S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); assertThat(events).isEmpty(); } } @Test - void transferManager_withCustomJavaClientWhenMultiPartEnabledIsNull_shouldLogDebugMessage() { - try (S3AsyncClient s3AsyncClient = S3AsyncClient.builder() - .region(Region.US_WEST_2) - .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) - .build(); + void transferManager_withJavaClientMultiPartNotSet_shouldLogDebugMessage() { + + + try (S3AsyncClient s3Crt = S3AsyncClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .build(); LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); - S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { + S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + " upload/download feature may not be enabled and resumable file upload may not be supported."); @@ -58,14 +63,16 @@ void transferManager_withCustomJavaClientWhenMultiPartEnabledIsNull_shouldLogDeb } @Test - void transferManager_withCustomJavaClientWhenMultiPartEnabledIsFalse_shouldLogDebugMessage() { - try (S3AsyncClient s3AsyncClient = S3AsyncClient.builder() - .region(Region.US_WEST_2) - .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) - .multipartEnabled(false) - .build(); + void transferManager_withJavaClientMultiPartEqualsFalse_shouldLogDebugMessage() { + + + try (S3AsyncClient s3Crt = S3AsyncClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .multipartEnabled(false) + .build(); LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); - S3TransferManager tm = S3TransferManager.builder().s3Client(s3AsyncClient).build()) { + S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + " upload/download feature may not be enabled and resumable file upload may not be supported."); @@ -73,7 +80,9 @@ void transferManager_withCustomJavaClientWhenMultiPartEnabledIsFalse_shouldLogDe } @Test - void transferManager_withDefaultJavaClient_shouldNotLogDebugMessage() { + void transferManager_withDefaultClient_shouldNotLogDebugMessage() { + + try (LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); S3TransferManager tm = S3TransferManager.builder().build()) { List events = logCaptor.loggedEvents(); @@ -82,7 +91,8 @@ void transferManager_withDefaultJavaClient_shouldNotLogDebugMessage() { } @Test - void transferManager_withCustomJavaClientMultiPartEnabled_shouldNotLogDebugMessage() { + void transferManager_withMultiPartEnabledJavaClient_shouldNotLogDebugMessage() { + try (S3AsyncClient s3Crt = S3AsyncClient.builder() .region(Region.US_WEST_2) .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java index c9819f6cad92..9e042965f760 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java @@ -33,6 +33,8 @@ public class S3AsyncClientDecorator { public static final AttributeMap.Key MULTIPART_CONFIGURATION_KEY = new AttributeMap.Key(MultipartConfiguration.class){}; + public static final AttributeMap.Key MULTIPART_ENABLED_KEY = + new AttributeMap.Key(Boolean.class){}; public S3AsyncClientDecorator() { } @@ -60,7 +62,7 @@ private Predicate isCrossRegionEnabledAsync(AttributeMap clientCo } private Predicate isMultipartEnable(AttributeMap clientContextParams) { - Boolean multipartEnabled = clientContextParams.get(S3ClientContextParams.MULTIPART_ENABLED); + Boolean multipartEnabled = clientContextParams.get(MULTIPART_ENABLED_KEY); return client -> multipartEnabled != null && multipartEnabled.booleanValue(); } } \ No newline at end of file diff --git a/services/s3/src/main/resources/codegen-resources/customization.config b/services/s3/src/main/resources/codegen-resources/customization.config index b12ffbbd3e38..4d788b49376b 100644 --- a/services/s3/src/main/resources/codegen-resources/customization.config +++ b/services/s3/src/main/resources/codegen-resources/customization.config @@ -241,6 +241,7 @@ "multipartConfigurationClass": "software.amazon.awssdk.services.s3.multipart.MultipartConfiguration", "multipartConfigMethodDoc": "Configuration for multipart operation of this client.", "multipartEnableMethodDoc": "Enables automatic conversion of put and copy method to their equivalent multipart operation.", + "contextParamEnabledKey": "S3AsyncClientDecorator.MULTIPART_ENABLED_KEY", "contextParamConfigKey": "S3AsyncClientDecorator.MULTIPART_CONFIGURATION_KEY" }, "interceptors": [ @@ -270,10 +271,6 @@ "CrossRegionAccessEnabled":{ "documentation":"Enables cross-region bucket access for this client", "type":"boolean" - }, - "multipartEnabled":{ - "documentation":"Enables automatic conversion of put and copy method to their equivalent multipart operation.", - "type":"boolean" } }, "endpointAuthSchemeConfig": { From 28c5d375f07eb2f8ee6932680f5a73f8124d9c3e Mon Sep 17 00:00:00 2001 From: Krishnan Date: Sat, 10 Feb 2024 21:03:22 -0800 Subject: [PATCH 4/8] Updated Javadocs --- .../software/amazon/awssdk/transfer/s3/S3TransferManager.java | 3 +++ .../transfer/s3/internal/TransferManagerLoggingTest.java | 2 +- .../services/s3/internal/client/S3AsyncClientDecorator.java | 2 +- .../src/main/resources/codegen-resources/customization.config | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java index 362bf1419e2d..48c682709620 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/S3TransferManager.java @@ -157,6 +157,9 @@ * // Wait for the transfer to complete * CompletedCopy completedCopy = copy.completionFuture().join(); * } + * Warns the user if a multi-part operation disabled client is provided for TransferManager to use + * If no client is provided, a multi-part enabled async client is instantiated and used. + * In the event of a multi-part disabled defaultS3AsyncClient being used, a warning is logged */ @SdkPublicApi @ThreadSafe diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index 42b752694438..824d77110dc4 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -112,4 +112,4 @@ private static void assertLogged(List events, org.apache.logging.log4j assertThat(msg).isEqualTo(message); assertThat(event.getLevel()).isEqualTo(level); } -} \ No newline at end of file +} diff --git a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java index 9e042965f760..fe2997a7d73c 100644 --- a/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java +++ b/services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/client/S3AsyncClientDecorator.java @@ -65,4 +65,4 @@ private Predicate isMultipartEnable(AttributeMap clientContextPar Boolean multipartEnabled = clientContextParams.get(MULTIPART_ENABLED_KEY); return client -> multipartEnabled != null && multipartEnabled.booleanValue(); } -} \ No newline at end of file +} diff --git a/services/s3/src/main/resources/codegen-resources/customization.config b/services/s3/src/main/resources/codegen-resources/customization.config index 4d788b49376b..7bd3369c2b88 100644 --- a/services/s3/src/main/resources/codegen-resources/customization.config +++ b/services/s3/src/main/resources/codegen-resources/customization.config @@ -285,4 +285,4 @@ }, "useSraAuth": true, "enableGenerateCompiledEndpointRules": true -} \ No newline at end of file +} From f843a5da5e9c99b8525714da6c02e54b6c460692 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Sat, 10 Feb 2024 23:21:59 -0800 Subject: [PATCH 5/8] fix checkstyle --- .../awssdk/transfer/s3/internal/TransferManagerFactory.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index b2c9305ead32..0d7f36434897 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -52,7 +52,8 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder) return new CrtS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); } - if (!s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient")) { + if (!s3AsyncClient.getClass().getName() + .equals("software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient")) { log.debug(() -> "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" + " upload/download feature may not be enabled and resumable file upload may not be supported."); } From ca256751eb2cbfdf3915f2715d148b2c67b0e1c8 Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 12 Feb 2024 10:10:32 -0800 Subject: [PATCH 6/8] Modified logging message and added test case to enable cross region along with multi-part --- .../s3/internal/TransferManagerFactory.java | 10 ++++--- .../internal/TransferManagerLoggingTest.java | 30 ++++++++++++++++--- 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index 0d7f36434897..94474169fffb 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -52,10 +52,12 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder) return new CrtS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); } - if (!s3AsyncClient.getClass().getName() - .equals("software.amazon.awssdk.services.s3.internal.multipart.MultipartS3AsyncClient")) { - log.debug(() -> "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" - + " upload/download feature may not be enabled and resumable file upload may not be supported."); + if (!s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.internal.multipart" + + ".MultipartS3AsyncClient")) { + log.debug(() -> "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, " + + "and thus multipart upload/download feature may not be enabled and resumable file upload may not " + + "be supported.\n" + "To benefit from maximum throughput, consider using S3AsyncClient.crtBuilder().build() " + + "or S3AsyncClient.builder().multipartEnabled(true).build() instead"); } return new GenericS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index 824d77110dc4..62ff5efde120 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -32,6 +32,14 @@ class TransferManagerLoggingTest { + private static final String EXPECTED_DEBUG_MESSAGE = "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient" + + " or MultipartS3AsyncClient, " + + "and thus multipart upload/download feature may not be enabled and " + + "resumable file upload may not be supported.\n" + + "To benefit from maximum throughput, consider using S3AsyncClient" + + ".crtBuilder().build() " + + "or S3AsyncClient.builder().multipartEnabled(true).build() instead"; + @Test void transferManager_withCrtClient_shouldNotLogWarnMessages() { @@ -57,8 +65,7 @@ void transferManager_withJavaClientMultiPartNotSet_shouldLogDebugMessage() { LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); - assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" - + " upload/download feature may not be enabled and resumable file upload may not be supported."); + assertLogged(events, Level.DEBUG, EXPECTED_DEBUG_MESSAGE); } } @@ -74,8 +81,7 @@ void transferManager_withJavaClientMultiPartEqualsFalse_shouldLogDebugMessage() LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { List events = logCaptor.loggedEvents(); - assertLogged(events, Level.DEBUG, "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart" - + " upload/download feature may not be enabled and resumable file upload may not be supported."); + assertLogged(events, Level.DEBUG, EXPECTED_DEBUG_MESSAGE); } } @@ -105,6 +111,22 @@ void transferManager_withMultiPartEnabledJavaClient_shouldNotLogDebugMessage() { } } + @Test + void transferManager_withMultiPartEnabledAndCrossRegionEnabledJavaClient_shouldNotLogDebugMessage() { + + try (S3AsyncClient s3Crt = S3AsyncClient.builder() + .region(Region.US_WEST_2) + .credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar")) + .multipartEnabled(true) + .crossRegionAccessEnabled(true) + .build(); + LogCaptor logCaptor = LogCaptor.create(Level.DEBUG); + S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) { + List events = logCaptor.loggedEvents(); + assertThat(events).isEmpty(); + } + } + private static void assertLogged(List events, org.apache.logging.log4j.Level level, String message) { assertThat(events).withFailMessage("Expecting events to not be empty").isNotEmpty(); LogEvent event = events.remove(0); From 85e3a24b7bc5aa1985c92dc5936d5c8fa94cda4a Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 12 Feb 2024 13:42:49 -0800 Subject: [PATCH 7/8] Fixed checkstyle --- .../s3/internal/TransferManagerFactory.java | 9 +++++---- .../s3/internal/TransferManagerLoggingTest.java | 16 +++++++--------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java index 94474169fffb..f247778bd267 100644 --- a/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java +++ b/services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java @@ -54,10 +54,11 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder) if (!s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.internal.multipart" + ".MultipartS3AsyncClient")) { - log.debug(() -> "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, " + - "and thus multipart upload/download feature may not be enabled and resumable file upload may not " + - "be supported.\n" + "To benefit from maximum throughput, consider using S3AsyncClient.crtBuilder().build() " + - "or S3AsyncClient.builder().multipartEnabled(true).build() instead"); + log.debug(() -> "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, " + + "and thus multipart upload/download feature may not be enabled and resumable file upload may not " + + "be supported. To benefit from maximum throughput, consider using " + + "S3AsyncClient.crtBuilder().build() or " + + "S3AsyncClient.builder().multipartEnabled(true).build() instead"); } return new GenericS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient); diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index 62ff5efde120..29925cb0a869 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -32,13 +32,11 @@ class TransferManagerLoggingTest { - private static final String EXPECTED_DEBUG_MESSAGE = "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient" - + " or MultipartS3AsyncClient, " - + "and thus multipart upload/download feature may not be enabled and " - + "resumable file upload may not be supported.\n" - + "To benefit from maximum throughput, consider using S3AsyncClient" - + ".crtBuilder().build() " - + "or S3AsyncClient.builder().multipartEnabled(true).build() instead"; + private static final String EXPECTED_DEBUG_MESSAGE = "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, " + + "and thus multipart upload/download feature may not be enabled and resumable file upload may not " + + "be supported. To benefit from maximum throughput, consider using " + + "S3AsyncClient.crtBuilder().build() or " + + "S3AsyncClient.builder().multipartEnabled(true).build() instead"; @Test void transferManager_withCrtClient_shouldNotLogWarnMessages() { @@ -85,7 +83,7 @@ void transferManager_withJavaClientMultiPartEqualsFalse_shouldLogDebugMessage() } } - @Test + /*@Test void transferManager_withDefaultClient_shouldNotLogDebugMessage() { @@ -94,7 +92,7 @@ void transferManager_withDefaultClient_shouldNotLogDebugMessage() { List events = logCaptor.loggedEvents(); assertThat(events).isEmpty(); } - } + }*/ @Test void transferManager_withMultiPartEnabledJavaClient_shouldNotLogDebugMessage() { From 9960c411620e2b4eac0b4f858d623a00a004537f Mon Sep 17 00:00:00 2001 From: Krishnan Date: Mon, 12 Feb 2024 13:43:33 -0800 Subject: [PATCH 8/8] Fixed checkstyle issues --- .../transfer/s3/internal/TransferManagerLoggingTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java index 29925cb0a869..42de93bd9e43 100644 --- a/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java +++ b/services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java @@ -83,7 +83,7 @@ void transferManager_withJavaClientMultiPartEqualsFalse_shouldLogDebugMessage() } } - /*@Test + @Test void transferManager_withDefaultClient_shouldNotLogDebugMessage() { @@ -92,7 +92,7 @@ void transferManager_withDefaultClient_shouldNotLogDebugMessage() { List events = logCaptor.loggedEvents(); assertThat(events).isEmpty(); } - }*/ + } @Test void transferManager_withMultiPartEnabledJavaClient_shouldNotLogDebugMessage() {