Skip to content

Commit 70e591e

Browse files
y-chandray-chandra
authored
Merge pull request #245 from th3jamin/feature/add-retry-configuration
Pass RetryConfiguration to UploadManager
2 parents 1b497c0 + 3eff3d3 commit 70e591e

File tree

4 files changed

+204
-6
lines changed

4 files changed

+204
-6
lines changed

‎bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssembler.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import com.oracle.bmc.objectstorage.responses.ListMultipartUploadsResponse;
3131
import com.oracle.bmc.objectstorage.transfer.internal.MultipartManifestImpl;
3232
import com.oracle.bmc.objectstorage.transfer.internal.MultipartTransferManager;
33+
import com.oracle.bmc.retrier.RetryConfiguration;
3334
import com.oracle.bmc.util.StreamUtils;
3435
import com.oracle.bmc.util.internal.Consumer;
3536
import lombok.Builder;
@@ -64,6 +65,8 @@ public class MultipartObjectAssembler {
6465
private MultipartManifestImpl manifest;
6566
private boolean initialized = false;
6667

68+
private RetryConfiguration retryConfiguration;
69+
6770
/**
6871
* The opcClientRequestId to send for all requests related to this multi-part upload.
6972
*/
@@ -96,7 +99,8 @@ public MultipartObjectAssembler(
9699
allowOverwrite,
97100
executorService,
98101
null /* opcClientRequestId */,
99-
null /* invocationCallback */);
102+
null /* invocationCallback */,
103+
UploadManager.RETRY_CONFIGURATION); /* backwards compatibility */
100104
}
101105

102106
@Builder
@@ -108,7 +112,8 @@ private MultipartObjectAssembler(
108112
boolean allowOverwrite,
109113
ExecutorService executorService,
110114
String opcClientRequestId,
111-
Consumer<Invocation.Builder> invocationCallback) {
115+
Consumer<Invocation.Builder> invocationCallback,
116+
RetryConfiguration retryConfiguration) {
112117
this.service = service;
113118
this.namespaceName = namespaceName;
114119
this.bucketName = bucketName;
@@ -117,6 +122,7 @@ private MultipartObjectAssembler(
117122
this.executorService = executorService;
118123
this.opcClientRequestId = opcClientRequestId;
119124
this.invocationCallback = invocationCallback;
125+
this.retryConfiguration = retryConfiguration;
120126
}
121127

122128
/**
@@ -299,7 +305,7 @@ private int doUploadPart(InputStream stream, long contentLength, String md5, int
299305
.opcClientRequestId(createClientRequestId("-" + partNumber))
300306
.build();
301307

302-
request.setRetryConfiguration(UploadManager.RETRY_CONFIGURATION);
308+
request.setRetryConfiguration(this.retryConfiguration);
303309

304310
transferManager.startTransfer(request);
305311
return partNumber;

‎bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ private UploadResponse singleUpload(UploadRequest uploadRequest, long contentLen
130130
.build();
131131
}
132132

133-
putObjectRequest.setRetryConfiguration(RETRY_CONFIGURATION);
133+
/* RetryConfiguration used should either be the one set on this UploadRequest or a default */
134+
putObjectRequest.setRetryConfiguration(getRetryToUse(putObjectRequest.getRetryConfiguration()));
134135

135136
PutObjectResponse response = objectStorage.putObject(putObjectRequest);
136137
return new UploadResponse(
@@ -247,11 +248,33 @@ private UploadResponse multipartUpload(UploadRequest uploadRequest) {
247248
}
248249
}
249250

251+
/**
252+
* Determines the first non-null RetryConfiguration
253+
* 1 -> RetryConfiguration set on UploadConfiguration
254+
* 2 -> Default static RetryConfiguration for UploadManager
255+
*
256+
* @return RetryConfiguration first non-null condition or UploadManager default
257+
*/
258+
private static RetryConfiguration getRetryToUse(RetryConfiguration ...configs) {
259+
for (RetryConfiguration cfg : configs) {
260+
if (cfg != null)
261+
return cfg;
262+
}
263+
264+
return UploadManager.RETRY_CONFIGURATION;
265+
}
266+
250267
@VisibleForTesting
251268
protected MultipartObjectAssembler createAssembler(
252269
PutObjectRequest request,
253270
UploadRequest uploadRequest,
254271
ExecutorService executorService) {
272+
273+
// in case request != uploadRequest.putObjectRequest then choose the correct RetryConfiguration
274+
RetryConfiguration retryToUse = getRetryToUse(
275+
uploadRequest.putObjectRequest.getRetryConfiguration(),
276+
request.getRetryConfiguration());
277+
255278
return MultipartObjectAssembler.builder()
256279
.allowOverwrite(uploadRequest.allowOverwrite)
257280
.bucketName(request.getBucketName())
@@ -261,6 +284,7 @@ protected MultipartObjectAssembler createAssembler(
261284
.objectName(request.getObjectName())
262285
.opcClientRequestId(request.getOpcClientRequestId())
263286
.service(objectStorage)
287+
.retryConfiguration(retryToUse)
264288
.build();
265289
}
266290

‎bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424
import java.util.concurrent.ExecutorService;
2525
import java.util.concurrent.Executors;
2626

27+
import com.oracle.bmc.retrier.DefaultRetryCondition;
28+
import com.oracle.bmc.retrier.RetryConfiguration;
2729
import com.oracle.bmc.util.internal.Consumer;
30+
import com.oracle.bmc.waiter.ExponentialBackoffDelayStrategy;
2831
import org.junit.After;
2932
import org.junit.Before;
3033
import org.junit.Rule;
@@ -65,8 +68,14 @@ public class MultipartObjectAssemblerTest {
6568
private static final Map<String, String> OPC_META = new HashMap<>();
6669
private static final boolean ALLOW_OVERWRITE = false;
6770

71+
private static final RetryConfiguration RETRY_CONFIGURATION = RetryConfiguration.builder()
72+
.delayStrategy(new ExponentialBackoffDelayStrategy(120000))
73+
.retryCondition(new DefaultRetryCondition())
74+
.build();
75+
6876
private ExecutorService executorService;
6977
private MultipartObjectAssembler assembler;
78+
private MultipartObjectAssembler assemblerWithRetryConfiguration;
7079

7180
@Mock private Consumer<Invocation.Builder> mockInvocationCallback;
7281

@@ -87,13 +96,52 @@ public void setUp() {
8796
.objectName(OBJECT)
8897
.service(service)
8998
.build();
99+
assemblerWithRetryConfiguration =
100+
MultipartObjectAssembler.builder()
101+
.allowOverwrite(ALLOW_OVERWRITE)
102+
.bucketName(BUCKET)
103+
.executorService(executorService)
104+
.invocationCallback(mockInvocationCallback)
105+
.namespaceName(NAMESPACE)
106+
.objectName(OBJECT)
107+
.service(service)
108+
.retryConfiguration(RETRY_CONFIGURATION)
109+
.build();
90110
}
91111

92112
@After
93113
public void tearDown() {
94114
executorService.shutdownNow();
95115
}
96116

117+
@Test
118+
public void newRequest_andVerifyManifestWithRetryConfiguration() {
119+
String uploadId = "uploadId";
120+
121+
initializeCreateMultipartUpload(uploadId);
122+
123+
MultipartManifest manifest =
124+
assemblerWithRetryConfiguration.newRequest(CONTENT_TYPE, CONTENT_LANGUAGE, CONTENT_ENCODING, OPC_META);
125+
assertNotNull(manifest);
126+
assertEquals(uploadId, manifest.getUploadId());
127+
128+
ArgumentCaptor<CreateMultipartUploadRequest> captor =
129+
ArgumentCaptor.forClass(CreateMultipartUploadRequest.class);
130+
verify(service).createMultipartUpload(captor.capture());
131+
132+
CreateMultipartUploadRequest request = captor.getValue();
133+
assertEquals(NAMESPACE, request.getNamespaceName());
134+
assertEquals(BUCKET, request.getBucketName());
135+
assertEquals(OBJECT, request.getCreateMultipartUploadDetails().getObject());
136+
assertEquals(CONTENT_TYPE, request.getCreateMultipartUploadDetails().getContentType());
137+
assertEquals(
138+
CONTENT_LANGUAGE, request.getCreateMultipartUploadDetails().getContentLanguage());
139+
assertEquals(
140+
CONTENT_ENCODING, request.getCreateMultipartUploadDetails().getContentEncoding());
141+
assertEquals(OPC_META, request.getCreateMultipartUploadDetails().getMetadata());
142+
assertEquals(mockInvocationCallback, request.getInvocationCallback());
143+
}
144+
97145
@Test
98146
public void newRequest_andVerifyManifest() {
99147
String uploadId = "uploadId";
@@ -302,6 +350,72 @@ public void addParts_allSuccessful_commit() throws Exception {
302350
file.delete();
303351
}
304352

353+
@Test
354+
public void addParts_allSuccessful_withRetryConfiguration_commit() throws Exception {
355+
String uploadId = "uploadId";
356+
initializeCreateMultipartUpload(uploadId);
357+
MultipartManifest manifest =
358+
assemblerWithRetryConfiguration.newRequest(CONTENT_TYPE, CONTENT_LANGUAGE, CONTENT_ENCODING, OPC_META);
359+
360+
byte[] bytes = "abcd".getBytes();
361+
362+
File file = File.createTempFile("unitTest", ".txt");
363+
file.deleteOnExit();
364+
try (FileOutputStream fos = new FileOutputStream(file)) {
365+
fos.write(bytes);
366+
}
367+
368+
String etag1 = "etag1";
369+
String etag2 = "etag2";
370+
UploadPartResponse uploadPartResponse1 = UploadPartResponse.builder().eTag(etag1).build();
371+
UploadPartResponse uploadPartResponse2 = UploadPartResponse.builder().eTag(etag2).build();
372+
when(service.uploadPart(any(UploadPartRequest.class)))
373+
.thenReturn(uploadPartResponse1)
374+
.thenReturn(uploadPartResponse2);
375+
376+
CommitMultipartUploadResponse finalCommitResponse =
377+
CommitMultipartUploadResponse.builder().build();
378+
when(service.commitMultipartUpload(any(CommitMultipartUploadRequest.class)))
379+
.thenReturn(finalCommitResponse);
380+
381+
String md5_1 = "md5_1";
382+
String md5_2 = "md5_2";
383+
384+
assemblerWithRetryConfiguration.addPart(file, md5_1);
385+
assemblerWithRetryConfiguration.addPart(StreamUtils.createByteArrayInputStream(bytes), bytes.length, md5_2);
386+
387+
CommitMultipartUploadResponse commitResponse = assemblerWithRetryConfiguration.commit();
388+
assertSame(finalCommitResponse, commitResponse);
389+
390+
ArgumentCaptor<CommitMultipartUploadRequest> commitCaptor =
391+
ArgumentCaptor.forClass(CommitMultipartUploadRequest.class);
392+
verify(service).commitMultipartUpload(commitCaptor.capture());
393+
CommitMultipartUploadRequest actualCommitRequest = commitCaptor.getValue();
394+
assertEquals(NAMESPACE, actualCommitRequest.getNamespaceName());
395+
assertEquals(BUCKET, actualCommitRequest.getBucketName());
396+
assertEquals(OBJECT, actualCommitRequest.getObjectName());
397+
assertEquals(uploadId, actualCommitRequest.getUploadId());
398+
assertEquals(mockInvocationCallback, actualCommitRequest.getInvocationCallback());
399+
400+
assertTrue(manifest.isUploadComplete());
401+
assertTrue(manifest.isUploadSuccessful());
402+
assertEquals(2, manifest.listCompletedParts().size());
403+
assertEquals(1, manifest.listCompletedParts().get(0).getPartNum().intValue());
404+
assertEquals(etag1, manifest.listCompletedParts().get(0).getEtag());
405+
assertEquals(2, manifest.listCompletedParts().get(1).getPartNum().intValue());
406+
assertEquals(etag2, manifest.listCompletedParts().get(1).getEtag());
407+
408+
ArgumentCaptor<UploadPartRequest> uploadCaptor =
409+
ArgumentCaptor.forClass(UploadPartRequest.class);
410+
verify(service, times(2)).uploadPart(uploadCaptor.capture());
411+
verifyUploadPart(uploadCaptor.getAllValues().get(0), uploadId, 1, md5_1);
412+
verifyUploadPart(uploadCaptor.getAllValues().get(1), uploadId, 2, md5_2);
413+
414+
uploadCaptor.getAllValues().forEach(r -> assertSame(RETRY_CONFIGURATION, r.getRetryConfiguration()));
415+
416+
file.delete();
417+
}
418+
305419
@Test
306420
public void addParts_someFailed_commitFailure() throws Exception {
307421
String uploadId = "uploadId";

‎bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java‎

Lines changed: 56 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,10 @@
3838
import com.oracle.bmc.objectstorage.transfer.UploadManager.UploadResponse;
3939
import com.oracle.bmc.objectstorage.transfer.internal.MultipartManifestImpl;
4040
import com.oracle.bmc.objectstorage.transfer.internal.MultipartUtils;
41+
import com.oracle.bmc.retrier.DefaultRetryCondition;
42+
import com.oracle.bmc.retrier.RetryConfiguration;
4143
import com.oracle.bmc.util.StreamUtils;
44+
import com.oracle.bmc.waiter.ExponentialBackoffDelayStrategy;
4245
import org.hamcrest.Matchers;
4346
import org.junit.Before;
4447
import org.junit.Test;
@@ -144,6 +147,7 @@ public void upload_singleUpload() throws IOException {
144147
byte[] buffer = new byte[(int) CONTENT_LENGTH];
145148
putRequestCaptor.getValue().getPutObjectBody().read(buffer);
146149
assertEquals(CONTENT, new String(buffer));
150+
assertSame(UploadManager.RETRY_CONFIGURATION, putRequestCaptor.getValue().getRetryConfiguration());
147151
assertEquals(CONTENT_LENGTH, putRequestCaptor.getValue().getContentLength().longValue());
148152
assertEquals(CLIENT_REQ_ID, putRequestCaptor.getValue().getOpcClientRequestId());
149153
assertSame(METADATA, putRequestCaptor.getValue().getOpcMeta());
@@ -173,6 +177,47 @@ public void upload_singleUpload_enforceMd5() throws Exception {
173177
putRequestCaptor.getValue().getContentMD5()); // 'a' times content-length
174178
}
175179

180+
@Test
181+
public void upload_singleUpload_uploadRequestRetryConfiguration() throws Exception {
182+
RetryConfiguration retryConfiguration = RetryConfiguration.builder()
183+
.delayStrategy(new ExponentialBackoffDelayStrategy(120000))
184+
.retryCondition(new DefaultRetryCondition())
185+
.build();
186+
187+
UploadConfiguration uploadConfiguration =
188+
UploadConfiguration.builder().allowMultipartUploads(false).build();
189+
UploadManager uploadManager = new UploadManager(objectStorage, uploadConfiguration);
190+
191+
UploadRequest request = createUploadRequest(retryConfiguration);
192+
193+
ArgumentCaptor<PutObjectRequest> putRequestCaptor =
194+
ArgumentCaptor.forClass(PutObjectRequest.class);
195+
PutObjectResponse putResponse =
196+
PutObjectResponse.builder()
197+
.eTag("etag")
198+
.opcContentMd5("md5")
199+
.opcRequestId(REQ_ID)
200+
.opcClientRequestId(CLIENT_REQ_ID)
201+
.build();
202+
when(objectStorage.putObject(putRequestCaptor.capture())).thenReturn(putResponse);
203+
204+
UploadResponse uploadResponse = uploadManager.upload(request);
205+
206+
assertNotNull(uploadResponse);
207+
assertEquals("etag", uploadResponse.getETag());
208+
assertEquals("md5", uploadResponse.getContentMd5());
209+
assertNull(uploadResponse.getMultipartMd5());
210+
assertEquals(REQ_ID, uploadResponse.getOpcRequestId());
211+
assertEquals(CLIENT_REQ_ID, uploadResponse.getOpcClientRequestId());
212+
byte[] buffer = new byte[(int) CONTENT_LENGTH];
213+
putRequestCaptor.getValue().getPutObjectBody().read(buffer);
214+
assertSame(retryConfiguration, putRequestCaptor.getValue().getRetryConfiguration());
215+
assertEquals(CONTENT, new String(buffer));
216+
assertEquals(CONTENT_LENGTH, putRequestCaptor.getValue().getContentLength().longValue());
217+
assertEquals(CLIENT_REQ_ID, putRequestCaptor.getValue().getOpcClientRequestId());
218+
assertSame(METADATA, putRequestCaptor.getValue().getOpcMeta());
219+
}
220+
176221
@Test(expected = BmcException.class)
177222
public void upload_singleUpload_enforceMd5_streamTooLargeToBuffer() {
178223
UploadConfiguration uploadConfiguration =
@@ -655,7 +700,7 @@ private static UploadConfiguration getMultipartUploadConfiguration() {
655700
.build();
656701
}
657702

658-
private UploadRequest createUploadRequest(ProgressReporter progressReporter) {
703+
private UploadRequest createUploadRequest(ProgressReporter progressReporter, RetryConfiguration retryConfiguration) {
659704
PutObjectRequest request =
660705
PutObjectRequest.builder()
661706
.opcMeta(METADATA)
@@ -666,14 +711,23 @@ private UploadRequest createUploadRequest(ProgressReporter progressReporter) {
666711
.namespaceName(NAMESPACE_NAME)
667712
.bucketName(BUCKET_NAME)
668713
.objectName(OBJECT_NAME)
714+
.retryConfiguration(retryConfiguration)
669715
.build();
670716
return UploadRequest.builder(body, CONTENT_LENGTH)
671717
.progressReporter(progressReporter)
672718
.build(request);
673719
}
674720

721+
private UploadRequest createUploadRequest(ProgressReporter progressReporter) {
722+
return createUploadRequest(progressReporter, null);
723+
}
724+
675725
private UploadRequest createUploadRequest() {
676-
return createUploadRequest(null);
726+
return createUploadRequest(null, null);
727+
}
728+
729+
private UploadRequest createUploadRequest(RetryConfiguration retryConfiguration) {
730+
return createUploadRequest(null, retryConfiguration);
677731
}
678732

679733
private static void validateUploadResponseForMultipart(final UploadResponse response) {

0 commit comments

Comments
 (0)