Skip to content

Commit 5f84704

Browse files
zoewanggzoewangg
committed
fix the bug where the query parameters are incorrectly moved to http body for async path
1 parent 2a422de commit 5f84704

File tree

8 files changed

+330
-106
lines changed

8 files changed

+330
-106
lines changed

‎.changes/next-release/bugfix-AWSSDKforJavav2-bc19a9a.json

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
{
2+
"category": "AWS SDK for Java v2",
3+
"type": "bugfix",
4+
"description": "Fixed the bug where query parameters are incorrectly moved to body in async clients. See [#958](https://github.com/aws/aws-sdk-java-v2/issues/958)"
5+
}

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/client/handler/BaseAsyncClientHandler.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,16 @@ private <InputT extends SdkRequest, OutputT extends SdkResponse, ReturnT> Comple
9898

9999
SdkHttpFullRequest marshalled = (SdkHttpFullRequest) finalizeSdkHttpRequestContext.httpRequest();
100100

101+
// For non-streaming requests, RequestBody can be modified in the interceptors. eg:
102+
// CreateMultipartUploadRequestInterceptor
103+
if (!finalizeSdkHttpRequestContext.asyncRequestBody().isPresent() &&
104+
finalizeSdkHttpRequestContext.requestBody().isPresent()) {
105+
marshalled = marshalled.toBuilder()
106+
.contentStreamProvider(
107+
finalizeSdkHttpRequestContext.requestBody().get().contentStreamProvider())
108+
.build();
109+
}
110+
101111
TransformingAsyncResponseHandler<ReturnT> successResponseHandler = new InterceptorCallingHttpResponseHandler<>(
102112
sdkHttpResponseHandler, executionContext);
103113

‎core/sdk-core/src/main/java/software/amazon/awssdk/core/internal/http/pipeline/stages/AsyncRetryableStage.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ public AsyncRetryableStage(TransformingAsyncResponseHandler<OutputT> responseHan
6969
this.requestPipeline = requestPipeline;
7070
}
7171

72+
@Override
7273
public CompletableFuture<Response<OutputT>> execute(SdkHttpFullRequest request, RequestExecutionContext context) throws
7374
Exception {
7475
return new RetryExecutor(request, context).execute();

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/AsyncUploadMultiplePartIntegrationTest.java

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/*
2+
* Copyright 2010-2019 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;
17+
18+
import java.util.concurrent.Callable;
19+
import software.amazon.awssdk.core.async.AsyncRequestBody;
20+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadRequest;
21+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadResponse;
22+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
23+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
24+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
25+
import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
26+
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
27+
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
28+
29+
public class AsyncUploadMultiplePartIntegrationTest extends UploadMultiplePartTestBase {
30+
31+
@Override
32+
public Callable<CreateMultipartUploadResponse> createMultipartUpload(String bucket, String key) {
33+
return () -> s3Async.createMultipartUpload(b -> b.bucket(bucket).key(key)).join();
34+
}
35+
36+
@Override
37+
public Callable<UploadPartResponse> uploadPart(UploadPartRequest request, String requestBody) {
38+
return () -> s3Async.uploadPart(request, AsyncRequestBody.fromString(requestBody)).join();
39+
}
40+
41+
@Override
42+
public Callable<ListMultipartUploadsResponse> listMultipartUploads(String bucket) {
43+
return () -> s3Async.listMultipartUploads(b -> b.bucket(bucket)).join();
44+
}
45+
46+
@Override
47+
public Callable<CompleteMultipartUploadResponse> completeMultipartUpload(CompleteMultipartUploadRequest request) {
48+
return () -> s3Async.completeMultipartUpload(request).join();
49+
}
50+
51+
@Override
52+
public Callable<AbortMultipartUploadResponse> abortMultipartUploadResponseCallable(AbortMultipartUploadRequest request) {
53+
return () -> s3Async.abortMultipartUpload(request).join();
54+
}
55+
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/UploadMultiplePartIntegrationTest.java

Lines changed: 20 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -15,125 +15,41 @@
1515

1616
package software.amazon.awssdk.services.s3;
1717

18-
import static org.assertj.core.api.Assertions.assertThat;
19-
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
20-
21-
import java.util.ArrayList;
22-
import java.util.List;
23-
import org.apache.commons.lang3.RandomStringUtils;
24-
import org.junit.AfterClass;
25-
import org.junit.BeforeClass;
26-
import org.junit.Test;
27-
import software.amazon.awssdk.core.ResponseBytes;
18+
import java.util.concurrent.Callable;
2819
import software.amazon.awssdk.core.sync.RequestBody;
29-
import software.amazon.awssdk.core.sync.ResponseTransformer;
20+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadRequest;
3021
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadResponse;
22+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
3123
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
32-
import software.amazon.awssdk.services.s3.model.CompletedMultipartUpload;
33-
import software.amazon.awssdk.services.s3.model.CompletedPart;
34-
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
24+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
3525
import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
36-
import software.amazon.awssdk.services.s3.model.MultipartUpload;
3726
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
3827
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
3928

40-
public class UploadMultiplePartIntegrationTest extends S3IntegrationTestBase {
41-
42-
private static final String BUCKET = temporaryBucketName(UploadMultiplePartIntegrationTest.class);
43-
private static final String CONTENT = RandomStringUtils.randomAscii(1000);
29+
public class UploadMultiplePartIntegrationTest extends UploadMultiplePartTestBase {
4430

45-
@BeforeClass
46-
public static void setupFixture() {
47-
createBucket(BUCKET);
31+
@Override
32+
public Callable<CreateMultipartUploadResponse> createMultipartUpload(String bucket, String key) {
33+
return () -> s3.createMultipartUpload(b -> b.bucket(bucket).key(key));
4834
}
4935

50-
@AfterClass
51-
public static void tearDownFixture() {
52-
deleteBucketAndAllContents(BUCKET);
36+
@Override
37+
public Callable<UploadPartResponse> uploadPart(UploadPartRequest request, String requestBody) {
38+
return () -> s3.uploadPart(request, RequestBody.fromString(requestBody));
5339
}
5440

55-
@Test
56-
public void uploadMultiplePart_complete() throws Exception {
57-
String key = "uploadMultiplePart_complete";
58-
59-
// 1. Initiate multipartUpload request
60-
String uploadId = initiateMultipartUpload(key);
61-
62-
int partCount = 1;
63-
List<String> contentsToUpload = new ArrayList<>();
64-
65-
// 2. Upload each part
66-
List<UploadPartResponse> uploadPartResponses = uploadParts(key, uploadId, partCount, contentsToUpload);
67-
68-
List<CompletedPart> completedParts = new ArrayList<>();
69-
70-
for (int i = 0; i < uploadPartResponses.size(); i++) {
71-
int partNumber = i + 1;
72-
UploadPartResponse response = uploadPartResponses.get(i);
73-
completedParts.add(CompletedPart.builder().eTag(response.eTag()).partNumber(partNumber).build());
74-
}
75-
76-
// 3. Complete multipart upload
77-
CompleteMultipartUploadResponse completeMultipartUploadResponse =
78-
s3.completeMultipartUpload(b -> b.bucket(BUCKET)
79-
.key(key)
80-
.uploadId(uploadId)
81-
.multipartUpload(CompletedMultipartUpload.builder()
82-
.parts(completedParts)
83-
.build()).build());
84-
85-
assertThat(completeMultipartUploadResponse).isNotNull();
86-
verifyMultipartUploadResult(key, contentsToUpload);
41+
@Override
42+
public Callable<ListMultipartUploadsResponse> listMultipartUploads(String bucket) {
43+
return () -> s3.listMultipartUploads(b -> b.bucket(bucket));
8744
}
8845

89-
@Test
90-
public void uploadMultiplePart_abort() {
91-
String key = "uploadMultiplePart_abort";
92-
93-
// 1. Initiate multipartUpload request
94-
String uploadId = initiateMultipartUpload(key);
95-
int partCount = 3;
96-
97-
// 2. Upload each part
98-
List<String> contentsToUpload = new ArrayList<>();
99-
List<UploadPartResponse> uploadPartResponses = uploadParts(key, uploadId, partCount, contentsToUpload);
100-
101-
// 3. abort the multipart upload
102-
AbortMultipartUploadResponse abortMultipartUploadResponse =
103-
s3.abortMultipartUpload(b -> b.bucket(BUCKET).key(key).uploadId(uploadId));
104-
105-
// Verify no in-progress multipart uploads
106-
ListMultipartUploadsResponse listMultipartUploadsResponse = s3.listMultipartUploads(b -> b.bucket(BUCKET));
107-
108-
List<MultipartUpload> uploads = listMultipartUploadsResponse.uploads();
109-
110-
assertThat(uploads).isEmpty();
111-
}
112-
113-
private void verifyMultipartUploadResult(String key, List<String> contentsToUpload) throws Exception {
114-
ResponseBytes<GetObjectResponse> objectAsBytes = s3.getObject(b -> b.bucket(BUCKET).key(key),
115-
ResponseTransformer.toBytes());
116-
String appendedString = String.join("", contentsToUpload);
117-
assertThat(objectAsBytes.asUtf8String()).isEqualTo(appendedString);
118-
}
119-
120-
private List<UploadPartResponse> uploadParts(String key, String uploadId, int partCount, List<String> contentsToUpload) {
121-
List<UploadPartResponse> uploadPartResponses = new ArrayList<>();
122-
123-
for (int i = 0; i < partCount; i++) {
124-
int partNumber = i + 1;
125-
contentsToUpload.add(CONTENT);
126-
UploadPartRequest uploadPartRequest = UploadPartRequest.builder().bucket(BUCKET).key(key)
127-
.uploadId(uploadId)
128-
.partNumber(partNumber)
129-
.build();
130-
131-
uploadPartResponses.add(s3.uploadPart(uploadPartRequest, RequestBody.fromString(CONTENT)));
132-
}
133-
return uploadPartResponses;
46+
@Override
47+
public Callable<CompleteMultipartUploadResponse> completeMultipartUpload(CompleteMultipartUploadRequest request) {
48+
return () -> s3.completeMultipartUpload(request);
13449
}
13550

136-
private String initiateMultipartUpload(String key) {
137-
return s3.createMultipartUpload(b -> b.bucket(BUCKET).key(key)).uploadId();
51+
@Override
52+
public Callable<AbortMultipartUploadResponse> abortMultipartUploadResponseCallable(AbortMultipartUploadRequest request) {
53+
return () -> s3.abortMultipartUpload(request);
13854
}
13955
}

‎services/s3/src/it/java/software/amazon/awssdk/services/s3/UploadMultiplePartTestBase.java

Lines changed: 156 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,156 @@
1+
/*
2+
* Copyright 2010-2019 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;
17+
18+
import static org.assertj.core.api.Assertions.assertThat;
19+
import static software.amazon.awssdk.testutils.service.S3BucketUtils.temporaryBucketName;
20+
21+
import java.util.ArrayList;
22+
import java.util.List;
23+
import java.util.concurrent.Callable;
24+
import org.apache.commons.lang3.RandomStringUtils;
25+
import org.junit.AfterClass;
26+
import org.junit.BeforeClass;
27+
import org.junit.Test;
28+
import software.amazon.awssdk.core.ResponseBytes;
29+
import software.amazon.awssdk.core.sync.ResponseTransformer;
30+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadRequest;
31+
import software.amazon.awssdk.services.s3.model.AbortMultipartUploadResponse;
32+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadRequest;
33+
import software.amazon.awssdk.services.s3.model.CompleteMultipartUploadResponse;
34+
import software.amazon.awssdk.services.s3.model.CompletedMultipartUpload;
35+
import software.amazon.awssdk.services.s3.model.CompletedPart;
36+
import software.amazon.awssdk.services.s3.model.CreateMultipartUploadResponse;
37+
import software.amazon.awssdk.services.s3.model.GetObjectResponse;
38+
import software.amazon.awssdk.services.s3.model.ListMultipartUploadsResponse;
39+
import software.amazon.awssdk.services.s3.model.MultipartUpload;
40+
import software.amazon.awssdk.services.s3.model.UploadPartRequest;
41+
import software.amazon.awssdk.services.s3.model.UploadPartResponse;
42+
43+
/**
44+
* Base classes to test S3Client and S3AsyncClient upload multiple part functions.
45+
*/
46+
public abstract class UploadMultiplePartTestBase extends S3IntegrationTestBase {
47+
48+
private static final String BUCKET = temporaryBucketName(AsyncUploadMultiplePartIntegrationTest.class);
49+
private static final String CONTENT = RandomStringUtils.randomAscii(1000);
50+
51+
@BeforeClass
52+
public static void setupFixture() {
53+
createBucket(BUCKET);
54+
}
55+
56+
@AfterClass
57+
public static void tearDownFixture() {
58+
deleteBucketAndAllContents(BUCKET);
59+
}
60+
61+
@Test
62+
public void uploadMultiplePart_complete() throws Exception {
63+
String key = "uploadMultiplePart_complete";
64+
65+
// 1. Initiate multipartUpload request
66+
String uploadId = initiateMultipartUpload(key);
67+
68+
int partCount = 1;
69+
List<String> contentsToUpload = new ArrayList<>();
70+
71+
// 2. Upload each part
72+
List<UploadPartResponse> uploadPartResponses = uploadParts(key, uploadId, partCount, contentsToUpload);
73+
74+
List<CompletedPart> completedParts = new ArrayList<>();
75+
76+
for (int i = 0; i < uploadPartResponses.size(); i++) {
77+
int partNumber = i + 1;
78+
UploadPartResponse response = uploadPartResponses.get(i);
79+
completedParts.add(CompletedPart.builder().eTag(response.eTag()).partNumber(partNumber).build());
80+
}
81+
82+
// 3. Complete multipart upload
83+
CompleteMultipartUploadResponse completeMultipartUploadResponse =
84+
completeMultipartUpload(CompleteMultipartUploadRequest.builder().bucket(BUCKET)
85+
.key(key)
86+
.uploadId(uploadId)
87+
.multipartUpload(CompletedMultipartUpload.builder()
88+
.parts(completedParts)
89+
.build()).build()).call();
90+
91+
assertThat(completeMultipartUploadResponse).isNotNull();
92+
verifyMultipartUploadResult(key, contentsToUpload);
93+
}
94+
95+
@Test
96+
public void uploadMultiplePart_abort() throws Exception {
97+
String key = "uploadMultiplePart_abort";
98+
99+
// 1. Initiate multipartUpload request
100+
String uploadId = initiateMultipartUpload(key);
101+
int partCount = 3;
102+
103+
// 2. Upload each part
104+
List<String> contentsToUpload = new ArrayList<>();
105+
List<UploadPartResponse> uploadPartResponses = uploadParts(key, uploadId, partCount, contentsToUpload);
106+
107+
// 3. abort the multipart upload
108+
AbortMultipartUploadResponse abortMultipartUploadResponse =
109+
abortMultipartUploadResponseCallable(AbortMultipartUploadRequest.builder().bucket(BUCKET).key(key).uploadId(uploadId).build()).call();
110+
111+
// Verify no in-progress multipart uploads
112+
ListMultipartUploadsResponse listMultipartUploadsResponse = listMultipartUploads(BUCKET).call();
113+
114+
List<MultipartUpload> uploads = listMultipartUploadsResponse.uploads();
115+
116+
assertThat(uploads).isEmpty();
117+
}
118+
119+
private void verifyMultipartUploadResult(String key, List<String> contentsToUpload) throws Exception {
120+
ResponseBytes<GetObjectResponse> objectAsBytes = s3.getObject(b -> b.bucket(BUCKET).key(key),
121+
ResponseTransformer.toBytes());
122+
String appendedString = String.join("", contentsToUpload);
123+
assertThat(objectAsBytes.asUtf8String()).isEqualTo(appendedString);
124+
}
125+
126+
private List<UploadPartResponse> uploadParts(String key, String uploadId, int partCount, List<String> contentsToUpload) throws Exception {
127+
List<UploadPartResponse> uploadPartResponses = new ArrayList<>();
128+
129+
for (int i = 0; i < partCount; i++) {
130+
int partNumber = i + 1;
131+
contentsToUpload.add(CONTENT);
132+
UploadPartRequest uploadPartRequest = UploadPartRequest.builder().bucket(BUCKET).key(key)
133+
.uploadId(uploadId)
134+
.partNumber(partNumber)
135+
.build();
136+
137+
uploadPartResponses.add(uploadPart(uploadPartRequest, CONTENT).call());
138+
}
139+
return uploadPartResponses;
140+
}
141+
142+
private String initiateMultipartUpload(String key) throws Exception {
143+
return createMultipartUpload(BUCKET, key).call().uploadId();
144+
}
145+
146+
147+
public abstract Callable<CreateMultipartUploadResponse> createMultipartUpload(String bucket, String key);
148+
149+
public abstract Callable<UploadPartResponse> uploadPart(UploadPartRequest request, String requestBody);
150+
151+
public abstract Callable<ListMultipartUploadsResponse> listMultipartUploads(String bucket);
152+
153+
public abstract Callable<CompleteMultipartUploadResponse> completeMultipartUpload(CompleteMultipartUploadRequest request);
154+
155+
public abstract Callable<AbortMultipartUploadResponse> abortMultipartUploadResponseCallable(AbortMultipartUploadRequest request);
156+
}

0 commit comments

Comments
 (0)