Skip to content

Commit 910b30f

Browse files
authored
Iterate SdkFields to convert requests (#4177)
* Iterate SdkFields to convert requests * Fix flaky test * Rename convertion utils class
1 parent bc80de0 commit 910b30f

File tree

7 files changed

+309
-290
lines changed

7 files changed

+309
-290
lines changed

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/CopyObjectHelper.java

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import software.amazon.awssdk.annotations.SdkInternalApi;
2525
import software.amazon.awssdk.services.s3.S3AsyncClient;
2626
import software.amazon.awssdk.services.s3.internal.multipart.GenericMultipartHelper;
27+
import software.amazon.awssdk.services.s3.internal.multipart.SdkPojoConversionUtils;
2728
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
2829
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
2930
import software.amazon.awssdk.services.s3.model.CompletedMultipartUpload;
@@ -54,8 +55,8 @@ public CopyObjectHelper(S3AsyncClient s3AsyncClient, long partSizeInBytes) {
5455
this.s3AsyncClient = s3AsyncClient;
5556
this.partSizeInBytes = partSizeInBytes;
5657
this.genericMultipartHelper = new GenericMultipartHelper<>(s3AsyncClient,
57-
RequestConversionUtils::toAbortMultipartUploadRequest,
58-
RequestConversionUtils::toCopyObjectResponse);
58+
SdkPojoConversionUtils::toAbortMultipartUploadRequest,
59+
SdkPojoConversionUtils::toCopyObjectResponse);
5960
}
6061

6162
public CompletableFuture<CopyObjectResponse> copyObject(CopyObjectRequest copyObjectRequest) {
@@ -64,7 +65,7 @@ public CompletableFuture<CopyObjectResponse> copyObject(CopyObjectRequest copyOb
6465

6566
try {
6667
CompletableFuture<HeadObjectResponse> headFuture =
67-
s3AsyncClient.headObject(RequestConversionUtils.toHeadObjectRequest(copyObjectRequest));
68+
s3AsyncClient.headObject(SdkPojoConversionUtils.toHeadObjectRequest(copyObjectRequest));
6869

6970
// Ensure cancellations are forwarded to the head future
7071
CompletableFutureUtils.forwardExceptionTo(returnFuture, headFuture);
@@ -101,7 +102,7 @@ private void copyInParts(CopyObjectRequest copyObjectRequest,
101102
Long contentLength,
102103
CompletableFuture<CopyObjectResponse> returnFuture) {
103104

104-
CreateMultipartUploadRequest request = RequestConversionUtils.toCreateMultipartUploadRequest(copyObjectRequest);
105+
CreateMultipartUploadRequest request = SdkPojoConversionUtils.toCreateMultipartUploadRequest(copyObjectRequest);
105106
CompletableFuture<CreateMultipartUploadResponse> createMultipartUploadFuture =
106107
s3AsyncClient.createMultipartUpload(request);
107108

@@ -212,7 +213,7 @@ private static CompletedPart convertUploadPartCopyResponse(AtomicReferenceArray<
212213
UploadPartCopyResponse uploadPartCopyResponse) {
213214
CopyPartResult copyPartResult = uploadPartCopyResponse.copyPartResult();
214215
CompletedPart completedPart =
215-
RequestConversionUtils.toCompletedPart(copyPartResult,
216+
SdkPojoConversionUtils.toCompletedPart(copyPartResult,
216217
partNumber);
217218

218219
completedParts.set(partNumber - 1, completedPart);

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/RequestConversionUtils.java

Lines changed: 0 additions & 260 deletions
This file was deleted.

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/crt/UploadPartCopyRequestIterable.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import java.util.NoSuchElementException;
2020
import software.amazon.awssdk.annotations.SdkInternalApi;
2121
import software.amazon.awssdk.core.pagination.sync.SdkIterable;
22+
import software.amazon.awssdk.services.s3.internal.multipart.SdkPojoConversionUtils;
2223
import software.amazon.awssdk.services.s3.model.CopyObjectRequest;
2324
import software.amazon.awssdk.services.s3.model.UploadPartCopyRequest;
2425

@@ -65,7 +66,7 @@ public UploadPartCopyRequest next() {
6566
long partSize = Math.min(optimalPartSize, remainingBytes);
6667
String range = range(partSize);
6768
UploadPartCopyRequest uploadPartCopyRequest =
68-
RequestConversionUtils.toUploadPartCopyRequest(copyObjectRequest,
69+
SdkPojoConversionUtils.toUploadPartCopyRequest(copyObjectRequest,
6970
partNumber,
7071
uploadId,
7172
range);

services/s3/src/main/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartUploadHelper.java

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
package software.amazon.awssdk.services.s3.internal.multipart;
1717

1818

19-
import static software.amazon.awssdk.services.s3.internal.crt.RequestConversionUtils.toAbortMultipartUploadRequest;
19+
import static software.amazon.awssdk.services.s3.internal.multipart.SdkPojoConversionUtils.toAbortMultipartUploadRequest;
2020

2121
import java.util.Collection;
2222
import java.util.concurrent.CompletableFuture;
@@ -27,7 +27,6 @@
2727
import software.amazon.awssdk.core.async.AsyncRequestBody;
2828
import software.amazon.awssdk.core.internal.async.SplittingPublisher;
2929
import software.amazon.awssdk.services.s3.S3AsyncClient;
30-
import software.amazon.awssdk.services.s3.internal.crt.RequestConversionUtils;
3130
import software.amazon.awssdk.services.s3.model.CompletedPart;
3231
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
3332
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
@@ -60,8 +59,8 @@ public MultipartUploadHelper(S3AsyncClient s3AsyncClient,
6059
this.s3AsyncClient = s3AsyncClient;
6160
this.partSizeInBytes = partSizeInBytes;
6261
this.genericMultipartHelper = new GenericMultipartHelper<>(s3AsyncClient,
63-
RequestConversionUtils::toAbortMultipartUploadRequest,
64-
RequestConversionUtils::toPutObjectResponse);
62+
SdkPojoConversionUtils::toAbortMultipartUploadRequest,
63+
SdkPojoConversionUtils::toPutObjectResponse);
6564
this.maxMemoryUsageInBytes = maxMemoryUsageInBytes;
6665
this.multipartUploadThresholdInBytes = multipartUploadThresholdInBytes;
6766
}
@@ -96,7 +95,7 @@ public CompletableFuture<PutObjectResponse> uploadObject(PutObjectRequest putObj
9695
private void uploadInParts(PutObjectRequest putObjectRequest, long contentLength, AsyncRequestBody asyncRequestBody,
9796
CompletableFuture<PutObjectResponse> returnFuture) {
9897

99-
CreateMultipartUploadRequest request = RequestConversionUtils.toCreateMultipartUploadRequest(putObjectRequest);
98+
CreateMultipartUploadRequest request = SdkPojoConversionUtils.toCreateMultipartUploadRequest(putObjectRequest);
10099
CompletableFuture<CreateMultipartUploadResponse> createMultipartUploadFuture =
101100
s3AsyncClient.createMultipartUpload(request);
102101

@@ -215,7 +214,7 @@ private void sendIndividualUploadPartRequest(String uploadId,
215214
private static CompletedPart convertUploadPartResponse(AtomicReferenceArray<CompletedPart> completedParts,
216215
Integer partNumber,
217216
UploadPartResponse uploadPartResponse) {
218-
CompletedPart completedPart = RequestConversionUtils.toCompletedPart(uploadPartResponse, partNumber);
217+
CompletedPart completedPart = SdkPojoConversionUtils.toCompletedPart(uploadPartResponse, partNumber);
219218

220219
completedParts.set(partNumber - 1, completedPart);
221220
return completedPart;
@@ -245,7 +244,7 @@ private static final class BodyToRequestConverter implements Function<AsyncReque
245244
public Pair<UploadPartRequest, AsyncRequestBody> apply(AsyncRequestBody asyncRequestBody) {
246245
log.trace(() -> "Generating uploadPartRequest for partNumber " + partNumber);
247246
UploadPartRequest uploadRequest =
248-
RequestConversionUtils.toUploadPartRequest(putObjectRequest,
247+
SdkPojoConversionUtils.toUploadPartRequest(putObjectRequest,
249248
partNumber,
250249
uploadId);
251250
++partNumber;
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,185 @@
1+
/*
2+
* Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License").
5+
* You may not use this file except in compliance with the License.
6+
* A copy of the License is located at
7+
*
8+
* http://aws.amazon.com/apache2.0
9+
*
10+
* or in the "license" file accompanying this file. This file is distributed
11+
* on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
12+
* express or implied. See the License for the specific language governing
13+
* permissions and limitations under the License.
14+
*/
15+
16+
package software.amazon.awssdk.services.s3.internal.multipart;
17+
18+
import java.util.Arrays;
19+
import java.util.HashMap;
20+
import java.util.HashSet;
21+
import java.util.List;
22+
import java.util.Map;
23+
import java.util.Set;
24+
import software.amazon.awssdk.annotations.SdkInternalApi;
25+
import software.amazon.awssdk.core.SdkField;
26+
import software.amazon.awssdk.core.SdkPojo;
27+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadRequest;
28+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
29+
import software.amazon.awssdk.services.s3.model.CompletedPart;
30+
import software.amazon.awssdk.services.s3.model.CopyObjectRequest;
31+
import software.amazon.awssdk.services.s3.model.CopyObjectResponse;
32+
import software.amazon.awssdk.services.s3.model.CopyObjectResult;
33+
import software.amazon.awssdk.services.s3.model.CopyPartResult;
34+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadRequest;
35+
import software.amazon.awssdk.services.s3.model.HeadObjectRequest;
36+
import software.amazon.awssdk.services.s3.model.PutObjectRequest;
37+
import software.amazon.awssdk.services.s3.model.PutObjectResponse;
38+
import software.amazon.awssdk.services.s3.model.UploadPartCopyRequest;
39+
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
40+
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
41+
42+
/**
43+
* Request conversion utility method for POJO classes associated with multipart feature.
44+
*/
45+
@SdkInternalApi
46+
public final class SdkPojoConversionUtils {
47+
48+
private static final HashSet<String> PUT_OBJECT_REQUEST_TO_UPLOAD_PART_FIELDS_TO_IGNORE =
49+
new HashSet<>(Arrays.asList("ChecksumSHA1", "ChecksumSHA256", "ContentMD5", "ChecksumCRC32C", "ChecksumCRC32"));
50+
51+
private SdkPojoConversionUtils() {
52+
}
53+
54+
public static UploadPartRequest toUploadPartRequest(PutObjectRequest putObjectRequest, int partNumber, String uploadId) {
55+
56+
UploadPartRequest.Builder builder = UploadPartRequest.builder();
57+
58+
setSdkFields(builder, putObjectRequest, PUT_OBJECT_REQUEST_TO_UPLOAD_PART_FIELDS_TO_IGNORE);
59+
60+
return builder.uploadId(uploadId).partNumber(partNumber).build();
61+
}
62+
63+
public static CreateMultipartUploadRequest toCreateMultipartUploadRequest(PutObjectRequest putObjectRequest) {
64+
65+
CreateMultipartUploadRequest.Builder builder = CreateMultipartUploadRequest.builder();
66+
setSdkFields(builder, putObjectRequest);
67+
return builder.build();
68+
}
69+
70+
public static HeadObjectRequest toHeadObjectRequest(CopyObjectRequest copyObjectRequest) {
71+
HeadObjectRequest.Builder builder = HeadObjectRequest.builder();
72+
setSdkFields(builder, copyObjectRequest);
73+
return builder.build();
74+
}
75+
76+
public static CompletedPart toCompletedPart(CopyPartResult copyPartResult, int partNumber) {
77+
CompletedPart.Builder builder = CompletedPart.builder();
78+
79+
setSdkFields(builder, copyPartResult);
80+
return builder.partNumber(partNumber).build();
81+
}
82+
83+
public static CompletedPart toCompletedPart(UploadPartResponse partResponse, int partNumber) {
84+
CompletedPart.Builder builder = CompletedPart.builder();
85+
setSdkFields(builder, partResponse);
86+
return builder.partNumber(partNumber).build();
87+
}
88+
89+
private static void setSdkFields(SdkPojo targetBuilder, SdkPojo sourceObject) {
90+
setSdkFields(targetBuilder, sourceObject, new HashSet<>());
91+
}
92+
93+
private static void setSdkFields(SdkPojo targetBuilder, SdkPojo sourceObject, Set<String> fieldsToIgnore) {
94+
Map<String, Object> sourceFields = retrieveSdkFields(sourceObject, sourceObject.sdkFields());
95+
List<SdkField<?>> targetSdkFields = targetBuilder.sdkFields();
96+
97+
for (SdkField<?> field : targetSdkFields) {
98+
if (fieldsToIgnore.contains(field.memberName())) {
99+
continue;
100+
}
101+
field.set(targetBuilder, sourceFields.getOrDefault(field.memberName(), null));
102+
}
103+
}
104+
105+
public static CreateMultipartUploadRequest toCreateMultipartUploadRequest(CopyObjectRequest copyObjectRequest) {
106+
CreateMultipartUploadRequest.Builder builder = CreateMultipartUploadRequest.builder();
107+
108+
setSdkFields(builder, copyObjectRequest);
109+
return builder.build();
110+
}
111+
112+
public static CopyObjectResponse toCopyObjectResponse(CompleteMultipartUploadResponse response) {
113+
CopyObjectResponse.Builder builder = CopyObjectResponse.builder();
114+
115+
setSdkFields(builder, response);
116+
117+
if (response.responseMetadata() != null) {
118+
builder.responseMetadata(response.responseMetadata());
119+
}
120+
121+
if (response.sdkHttpResponse() != null) {
122+
builder.sdkHttpResponse(response.sdkHttpResponse());
123+
}
124+
125+
return builder.copyObjectResult(toCopyObjectResult(response))
126+
.build();
127+
}
128+
129+
private static CopyObjectResult toCopyObjectResult(CompleteMultipartUploadResponse response) {
130+
CopyObjectResult.Builder builder = CopyObjectResult.builder();
131+
132+
setSdkFields(builder, response);
133+
return builder.build();
134+
}
135+
136+
public static AbortMultipartUploadRequest.Builder toAbortMultipartUploadRequest(CopyObjectRequest copyObjectRequest) {
137+
AbortMultipartUploadRequest.Builder builder = AbortMultipartUploadRequest.builder();
138+
setSdkFields(builder, copyObjectRequest);
139+
return builder;
140+
}
141+
142+
public static AbortMultipartUploadRequest.Builder toAbortMultipartUploadRequest(PutObjectRequest putObjectRequest) {
143+
AbortMultipartUploadRequest.Builder builder = AbortMultipartUploadRequest.builder();
144+
setSdkFields(builder, putObjectRequest);
145+
return builder;
146+
}
147+
148+
public static UploadPartCopyRequest toUploadPartCopyRequest(CopyObjectRequest copyObjectRequest,
149+
int partNumber,
150+
String uploadId,
151+
String range) {
152+
UploadPartCopyRequest.Builder builder = UploadPartCopyRequest.builder();
153+
setSdkFields(builder, copyObjectRequest);
154+
return builder.copySourceRange(range)
155+
.partNumber(partNumber)
156+
.uploadId(uploadId)
157+
.build();
158+
}
159+
160+
public static PutObjectResponse toPutObjectResponse(CompleteMultipartUploadResponse response) {
161+
162+
PutObjectResponse.Builder builder = PutObjectResponse.builder();
163+
164+
setSdkFields(builder, response);
165+
166+
// TODO: check why we have to do null check
167+
if (response.responseMetadata() != null) {
168+
builder.responseMetadata(response.responseMetadata());
169+
}
170+
171+
if (response.sdkHttpResponse() != null) {
172+
builder.sdkHttpResponse(response.sdkHttpResponse());
173+
}
174+
175+
return builder.build();
176+
}
177+
178+
private static Map<String, Object> retrieveSdkFields(SdkPojo sourceObject, List<SdkField<?>> sdkFields) {
179+
return sdkFields.stream().collect(
180+
HashMap::new,
181+
(map, field) -> map.put(field.memberName(),
182+
field.getValueOrDefault(sourceObject)),
183+
Map::putAll);
184+
}
185+
}

services/s3/src/test/java/software/amazon/awssdk/services/s3/internal/multipart/MultipartUploadHelperTest.java

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import static org.assertj.core.api.Assertions.assertThat;
1919
import static org.assertj.core.api.Assertions.assertThatThrownBy;
20+
import static org.assertj.core.api.Assertions.fail;
2021
import static org.mockito.ArgumentMatchers.any;
2122
import static org.mockito.Mockito.never;
2223
import static org.mockito.Mockito.times;
@@ -27,6 +28,9 @@
2728
import java.io.IOException;
2829
import java.util.List;
2930
import java.util.concurrent.CompletableFuture;
31+
import java.util.concurrent.ExecutionException;
32+
import java.util.concurrent.TimeUnit;
33+
import java.util.concurrent.TimeoutException;
3034
import org.junit.jupiter.api.AfterAll;
3135
import org.junit.jupiter.api.BeforeAll;
3236
import org.junit.jupiter.api.BeforeEach;
@@ -164,7 +168,12 @@ void mpu_onePartFailed_shouldFailOtherPartsAndAbort() {
164168
AbortMultipartUploadRequest actualRequest = argumentCaptor.getValue();
165169
assertThat(actualRequest.uploadId()).isEqualTo(UPLOAD_ID);
166170

167-
assertThat(ongoingRequest).isCompletedExceptionally();
171+
try {
172+
ongoingRequest.get(1, TimeUnit.MILLISECONDS);
173+
fail("no exception thrown");
174+
} catch (Exception e) {
175+
assertThat(e.getCause()).hasMessageContaining("request failed");
176+
}
168177
}
169178

170179
@Test

0 commit comments

Comments
 (0)