Skip to content

Commit b247de9

Browse files
anirudh9391anirudh9391
authored
Enable setting multipart to true/false by defining it as a custom client context parameter (#4903)
* Enable setting multipart to true/false by defining it as a custom client context parameter * Refactor indentation * Checking if multi-part is enabled before logging a debug message * Updated Javadocs * fix checkstyle * Modified logging message and added test case to enable cross region along with multi-part * Fixed checkstyle * Fixed checkstyle issues
1 parent 8c18e39 commit b247de9

File tree

3 files changed

+78
-15
lines changed

3 files changed

+78
-15
lines changed

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,9 @@
157157
* // Wait for the transfer to complete
158158
* CompletedCopy completedCopy = copy.completionFuture().join();
159159
* }
160+
* <b> Warns the user if a multi-part operation disabled client is provided for TransferManager to use
161+
* If no client is provided, a multi-part enabled async client is instantiated and used.
162+
* In the event of a multi-part disabled defaultS3AsyncClient being used, a warning is logged </b>
160163
*/
161164
@SdkPublicApi
162165
@ThreadSafe

‎services-custom/s3-transfer-manager/src/main/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerFactory.java

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -52,13 +52,13 @@ public static S3TransferManager createTransferManager(DefaultBuilder tmBuilder)
5252
return new CrtS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient);
5353
}
5454

55-
if (s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.DefaultS3AsyncClient")) {
56-
log.warn(() -> "The provided DefaultS3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart"
57-
+ " upload/download feature is not enabled and resumable file upload is not supported. To benefit "
58-
+ "from maximum throughput, consider using S3AsyncClient.crtBuilder().build() instead.");
59-
} else {
60-
log.debug(() -> "The provided S3AsyncClient is not an instance of S3CrtAsyncClient, and thus multipart"
61-
+ " upload/download feature may not be enabled and resumable file upload may not be supported.");
55+
if (!s3AsyncClient.getClass().getName().equals("software.amazon.awssdk.services.s3.internal.multipart"
56+
+ ".MultipartS3AsyncClient")) {
57+
log.debug(() -> "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, "
58+
+ "and thus multipart upload/download feature may not be enabled and resumable file upload may not "
59+
+ "be supported. To benefit from maximum throughput, consider using "
60+
+ "S3AsyncClient.crtBuilder().build() or "
61+
+ "S3AsyncClient.builder().multipartEnabled(true).build() instead");
6262
}
6363

6464
return new GenericS3TransferManager(transferConfiguration, s3AsyncClient, isDefaultS3AsyncClient);
@@ -68,7 +68,7 @@ private static Supplier<S3AsyncClient> defaultS3AsyncClient() {
6868
if (crtInClasspath()) {
6969
return S3AsyncClient::crtCreate;
7070
}
71-
return S3AsyncClient::create;
71+
return () -> S3AsyncClient.builder().multipartEnabled(true).build();
7272
}
7373

7474
private static boolean crtInClasspath() {

‎services-custom/s3-transfer-manager/src/test/java/software/amazon/awssdk/transfer/s3/internal/TransferManagerLoggingTest.java

Lines changed: 67 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,12 @@
3232

3333
class TransferManagerLoggingTest {
3434

35+
private static final String EXPECTED_DEBUG_MESSAGE = "The provided S3AsyncClient is neither an instance of S3CrtAsyncClient or MultipartS3AsyncClient, "
36+
+ "and thus multipart upload/download feature may not be enabled and resumable file upload may not "
37+
+ "be supported. To benefit from maximum throughput, consider using "
38+
+ "S3AsyncClient.crtBuilder().build() or "
39+
+ "S3AsyncClient.builder().multipartEnabled(true).build() instead";
40+
3541
@Test
3642
void transferManager_withCrtClient_shouldNotLogWarnMessages() {
3743

@@ -47,21 +53,75 @@ void transferManager_withCrtClient_shouldNotLogWarnMessages() {
4753
}
4854

4955
@Test
50-
void transferManager_withJavaClient_shouldLogWarnMessage() {
56+
void transferManager_withJavaClientMultiPartNotSet_shouldLogDebugMessage() {
5157

5258

5359
try (S3AsyncClient s3Crt = S3AsyncClient.builder()
5460
.region(Region.US_WEST_2)
5561
.credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar"))
5662
.build();
57-
LogCaptor logCaptor = LogCaptor.create(Level.WARN);
63+
LogCaptor logCaptor = LogCaptor.create(Level.DEBUG);
64+
S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) {
65+
List<LogEvent> events = logCaptor.loggedEvents();
66+
assertLogged(events, Level.DEBUG, EXPECTED_DEBUG_MESSAGE);
67+
}
68+
}
69+
70+
@Test
71+
void transferManager_withJavaClientMultiPartEqualsFalse_shouldLogDebugMessage() {
72+
73+
74+
try (S3AsyncClient s3Crt = S3AsyncClient.builder()
75+
.region(Region.US_WEST_2)
76+
.credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar"))
77+
.multipartEnabled(false)
78+
.build();
79+
LogCaptor logCaptor = LogCaptor.create(Level.DEBUG);
80+
S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) {
81+
List<LogEvent> events = logCaptor.loggedEvents();
82+
assertLogged(events, Level.DEBUG, EXPECTED_DEBUG_MESSAGE);
83+
}
84+
}
85+
86+
@Test
87+
void transferManager_withDefaultClient_shouldNotLogDebugMessage() {
88+
89+
90+
try (LogCaptor logCaptor = LogCaptor.create(Level.DEBUG);
91+
S3TransferManager tm = S3TransferManager.builder().build()) {
92+
List<LogEvent> events = logCaptor.loggedEvents();
93+
assertThat(events).isEmpty();
94+
}
95+
}
96+
97+
@Test
98+
void transferManager_withMultiPartEnabledJavaClient_shouldNotLogDebugMessage() {
99+
100+
try (S3AsyncClient s3Crt = S3AsyncClient.builder()
101+
.region(Region.US_WEST_2)
102+
.credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar"))
103+
.multipartEnabled(true)
104+
.build();
105+
LogCaptor logCaptor = LogCaptor.create(Level.DEBUG);
58106
S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) {
59107
List<LogEvent> events = logCaptor.loggedEvents();
60-
assertLogged(events, Level.WARN, "The provided DefaultS3AsyncClient is not an instance of S3CrtAsyncClient, and "
61-
+ "thus multipart upload/download feature is not enabled and resumable file upload"
62-
+ " is "
63-
+ "not supported. To benefit from maximum throughput, consider using "
64-
+ "S3AsyncClient.crtBuilder().build() instead.");
108+
assertThat(events).isEmpty();
109+
}
110+
}
111+
112+
@Test
113+
void transferManager_withMultiPartEnabledAndCrossRegionEnabledJavaClient_shouldNotLogDebugMessage() {
114+
115+
try (S3AsyncClient s3Crt = S3AsyncClient.builder()
116+
.region(Region.US_WEST_2)
117+
.credentialsProvider(() -> AwsBasicCredentials.create("foo", "bar"))
118+
.multipartEnabled(true)
119+
.crossRegionAccessEnabled(true)
120+
.build();
121+
LogCaptor logCaptor = LogCaptor.create(Level.DEBUG);
122+
S3TransferManager tm = S3TransferManager.builder().s3Client(s3Crt).build()) {
123+
List<LogEvent> events = logCaptor.loggedEvents();
124+
assertThat(events).isEmpty();
65125
}
66126
}
67127

0 commit comments

Comments
 (0)