Skip to content

Commit c163d2a

Browse files
zoewanggzoewangg
committed
Remove ListPartRequest when cleaning up resources for multipart upload
1 parent b2b8a01 commit c163d2a

File tree

3 files changed

+10
-41
lines changed

3 files changed

+10
-41
lines changed

‎services-custom/s3-transfermanager/src/main/java/software/amazon/awssdk/custom/s3/transfer/internal/MultiPartUploadManager.java

Lines changed: 1 addition & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
import static software.amazon.awssdk.custom.s3.transfer.internal.RequestConversionUtils.toAbortPartRequest;
1919
import static software.amazon.awssdk.custom.s3.transfer.internal.RequestConversionUtils.toCompleteMultipartUploadRequest;
2020
import static software.amazon.awssdk.custom.s3.transfer.internal.RequestConversionUtils.toCreateMultipartUploadRequest;
21-
import static software.amazon.awssdk.custom.s3.transfer.internal.RequestConversionUtils.toListPartsRequest;
2221
import static software.amazon.awssdk.custom.s3.transfer.internal.RequestConversionUtils.toUploadPartRequest;
2322
import static software.amazon.awssdk.utils.CompletableFutureUtils.allOfCancelForwarded;
2423

@@ -38,10 +37,8 @@
3837
import software.amazon.awssdk.services.s3.model.CompletedPart;
3938
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
4039
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
41-
import software.amazon.awssdk.services.s3.model.ListPartsRequest;
4240
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
4341
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
44-
import software.amazon.awssdk.utils.CollectionUtils;
4542
import software.amazon.awssdk.utils.Logger;
4643

4744
/**
@@ -122,7 +119,7 @@ private void cleanUpIfNeeded(PutObjectRequest request, String uploadId) {
122119
log.warn(() -> String.format("Failed to abort multipartUpload. You should invoke abortMultipart to avoid "
123120
+ "being charged for storage of the uploaded parts. (uploadId: %s)", uploadId), t);
124121
} else {
125-
verifyNoPartsPersisted(request, uploadId);
122+
log.debug(() -> "Upload " + uploadId + " has been aborted");
126123
}
127124
});
128125
}
@@ -132,23 +129,6 @@ private CompletableFuture<AbortMultipartUploadResponse> abortMultipart(PutObject
132129
return s3Client.abortMultipartUpload(abortMultipartUploadRequest);
133130
}
134131

135-
private void verifyNoPartsPersisted(PutObjectRequest request,
136-
String uploadId) {
137-
ListPartsRequest listPartsRequest = toListPartsRequest(request, uploadId);
138-
s3Client.listParts(listPartsRequest).whenComplete((listPartsResponse, exception) -> {
139-
if (exception == null) {
140-
if (!CollectionUtils.isNullOrEmpty(listPartsResponse.parts())) {
141-
log.warn(() -> String.format("Not all parts have been deleted. You should invoke abortMultipart to avoid"
142-
+ " being charged for storage of the uploaded parts (uploadId = %s)", uploadId));
143-
} else {
144-
log.debug(() -> "All uploaded parts have been deleted");
145-
}
146-
} else {
147-
log.warn(() -> "Failed to list parts", exception);
148-
}
149-
});
150-
}
151-
152132
private CompletionStage<Void> completeMultipartUpload(CompletionStage<Void> completedMultiPartsFuture,
153133
PutObjectRequest request,
154134
String uploadId,

‎services-custom/s3-transfermanager/src/main/java/software/amazon/awssdk/custom/s3/transfer/internal/RequestConversionUtils.java

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
2424
import software.amazon.awssdk.services.s3.model.GetObjectRequest;
2525
import software.amazon.awssdk.services.s3.model.HeadObjectRequest;
26-
import software.amazon.awssdk.services.s3.model.ListPartsRequest;
2726
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
2827
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
2928

@@ -66,9 +65,8 @@ static CreateMultipartUploadRequest toCreateMultipartUploadRequest(PutObjectRequ
6665
.sseCustomerAlgorithm(putObjectRequest.sseCustomerAlgorithm())
6766
.sseCustomerKey(putObjectRequest.sseCustomerKey())
6867
.sseCustomerKeyMD5(putObjectRequest.sseCustomerKeyMD5())
69-
//TODO: uncomment this once #1352 is merged
70-
//.requestPayer(putObjectRequest.requestPayer())
71-
//.acl(putObjectRequest.acl())
68+
.requestPayer(putObjectRequest.requestPayer())
69+
.acl(putObjectRequest.acl())
7270
.cacheControl(putObjectRequest.cacheControl())
7371
.metadata(putObjectRequest.metadata())
7472
.contentDisposition(putObjectRequest.contentDisposition())
@@ -81,6 +79,7 @@ static CreateMultipartUploadRequest toCreateMultipartUploadRequest(PutObjectRequ
8179
.grantFullControl(putObjectRequest.grantFullControl())
8280
.grantReadACP(putObjectRequest.grantReadACP())
8381
.grantWriteACP(putObjectRequest.grantWriteACP())
82+
//TODO filter out headers
8483
//.overrideConfiguration(putObjectRequest.overrideConfiguration())
8584
.build();
8685
}
@@ -99,7 +98,7 @@ static UploadPartRequest toUploadPartRequest(PutObjectRequest putObjectRequest,
9998
.sseCustomerKeyMD5(putObjectRequest.sseCustomerKeyMD5())
10099
.uploadId(uploadId)
101100
.partNumber(partNumber)
102-
//.requestPayer(putObjectRequest.requestPayer())
101+
.requestPayer(putObjectRequest.requestPayer())
103102
//.overrideConfiguration(putObjectRequest.overrideConfiguration())
104103
.build();
105104
}
@@ -113,32 +112,19 @@ static CompleteMultipartUploadRequest toCompleteMultipartUploadRequest(PutObject
113112
.multipartUpload(CompletedMultipartUpload.builder()
114113
.parts(parts)
115114
.build())
116-
//.requestPayer(putObjectRequest.requestPayer())
115+
.requestPayer(putObjectRequest.requestPayer())
117116
.uploadId(uploadId)
118117
.build();
119118

120119
}
121120

122-
static ListPartsRequest toListPartsRequest(PutObjectRequest putObjectRequest,
123-
String uploadId) {
124-
return ListPartsRequest.builder()
125-
.bucket(putObjectRequest.bucket())
126-
.key(putObjectRequest.key())
127-
.uploadId(uploadId)
128-
//.partNumberMarker()
129-
//.requestPayer(putObjectRequest.requestPayer())
130-
.build();
131-
132-
}
133-
134121
static AbortMultipartUploadRequest toAbortPartRequest(PutObjectRequest putObjectRequest,
135122
String uploadId) {
136123
return AbortMultipartUploadRequest.builder()
137124
.bucket(putObjectRequest.bucket())
138125
.key(putObjectRequest.key())
139126
.uploadId(uploadId)
140-
//.partNumberMarker()
141-
//.requestPayer(putObjectRequest.requestPayer())
127+
.requestPayer(putObjectRequest.requestPayer())
142128
.build();
143129

144130
}

‎services-custom/s3-transfermanager/src/test/java/software/amazon/awssdk/custom/s3/transfer/internal/UploadManagerTest.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import software.amazon.awssdk.custom.s3.transfer.UploadRequest;
3737
import software.amazon.awssdk.custom.s3.transfer.util.SizeConstant;
3838
import software.amazon.awssdk.services.s3.S3AsyncClient;
39+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadRequest;
3940
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
4041
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
4142
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
@@ -166,6 +167,7 @@ public void uploadObjectApiRequest_multiplePart_uploadPartFail_shouldPropagateEx
166167
transferRequestBody);
167168

168169
assertThatThrownBy(() -> upload.completionFuture().join()).hasCause(exception);
170+
verify(s3Client).abortMultipartUpload(any(AbortMultipartUploadRequest.class));
169171
}
170172

171173
@Test
@@ -189,6 +191,7 @@ public void uploadObjectApiRequest_multiplePart_completeMultiPartFail_shouldProp
189191
transferRequestBody);
190192

191193
assertThatThrownBy(() -> upload.completionFuture().join()).hasCause(exception);
194+
verify(s3Client).abortMultipartUpload(any(AbortMultipartUploadRequest.class));
192195
}
193196

194197
private void stubSuccessfulCreateMultipartUpload() {

0 commit comments

Comments
 (0)