From bb3cce23560ea3a315d335945a5be0674a97b5a5 Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Thu, 20 Aug 2020 13:10:39 -0600 Subject: [PATCH 1/6] Initial commit of the ability to pass a RetryConfiguration through to UploadManager --- .../transfer/MultipartObjectAssembler.java | 12 +- .../objectstorage/transfer/UploadManager.java | 26 +++- .../MultipartObjectAssemblerTest.java | 114 ++++++++++++++++++ .../transfer/UploadManagerTest.java | 58 ++++++++- 4 files changed, 204 insertions(+), 6 deletions(-) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssembler.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssembler.java index d63d6b2d398..685e887861d 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssembler.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssembler.java @@ -30,6 +30,7 @@ import com.oracle.bmc.objectstorage.responses.ListMultipartUploadsResponse; import com.oracle.bmc.objectstorage.transfer.internal.MultipartManifestImpl; import com.oracle.bmc.objectstorage.transfer.internal.MultipartTransferManager; +import com.oracle.bmc.retrier.RetryConfiguration; import com.oracle.bmc.util.StreamUtils; import com.oracle.bmc.util.internal.Consumer; import lombok.Builder; @@ -64,6 +65,8 @@ public class MultipartObjectAssembler { private MultipartManifestImpl manifest; private boolean initialized = false; + private RetryConfiguration retryConfiguration; + /** * The opcClientRequestId to send for all requests related to this multi-part upload. */ @@ -96,7 +99,8 @@ public MultipartObjectAssembler( allowOverwrite, executorService, null /* opcClientRequestId */, - null /* invocationCallback */); + null /* invocationCallback */, + UploadManager.RETRY_CONFIGURATION); /* backwards compatibility */ } @Builder @@ -108,7 +112,8 @@ private MultipartObjectAssembler( boolean allowOverwrite, ExecutorService executorService, String opcClientRequestId, - Consumer invocationCallback) { + Consumer invocationCallback, + RetryConfiguration retryConfiguration) { this.service = service; this.namespaceName = namespaceName; this.bucketName = bucketName; @@ -117,6 +122,7 @@ private MultipartObjectAssembler( this.executorService = executorService; this.opcClientRequestId = opcClientRequestId; this.invocationCallback = invocationCallback; + this.retryConfiguration = retryConfiguration; } /** @@ -299,7 +305,7 @@ private int doUploadPart(InputStream stream, long contentLength, String md5, int .opcClientRequestId(createClientRequestId("-" + partNumber)) .build(); - request.setRetryConfiguration(UploadManager.RETRY_CONFIGURATION); + request.setRetryConfiguration(this.retryConfiguration); transferManager.startTransfer(request); return partNumber; diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java index efad56a7698..c90be1580d7 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java @@ -130,7 +130,8 @@ private UploadResponse singleUpload(UploadRequest uploadRequest, long contentLen .build(); } - putObjectRequest.setRetryConfiguration(RETRY_CONFIGURATION); + /* RetryConfiguration used should either be the one set on this UploadRequest or a default */ + putObjectRequest.setRetryConfiguration(getRetryToUse(putObjectRequest.getRetryConfiguration())); PutObjectResponse response = objectStorage.putObject(putObjectRequest); return new UploadResponse( @@ -247,11 +248,33 @@ private UploadResponse multipartUpload(UploadRequest uploadRequest) { } } + /** + * Determines the first non-null RetryCondition + * 1 -> RetryConfiguration set on UploadConfiguration + * 2 -> Default static RetryConfiguration for UploadManager + * + * @return RetryConfiguration + */ + private RetryConfiguration getRetryToUse(RetryConfiguration ...configs) { + for (RetryConfiguration cfg : configs) { + if (cfg != null) + return cfg; + } + + return UploadManager.RETRY_CONFIGURATION; + } + @VisibleForTesting protected MultipartObjectAssembler createAssembler( PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { + + // in case request != uploadRequest.putObjectRequest then choose the correct RetryConfiguration + RetryConfiguration retryToUse = getRetryToUse( + uploadRequest.putObjectRequest.getRetryConfiguration(), + request.getRetryConfiguration()); + return MultipartObjectAssembler.builder() .allowOverwrite(uploadRequest.allowOverwrite) .bucketName(request.getBucketName()) @@ -261,6 +284,7 @@ protected MultipartObjectAssembler createAssembler( .objectName(request.getObjectName()) .opcClientRequestId(request.getOpcClientRequestId()) .service(objectStorage) + .retryConfiguration(retryToUse) .build(); } diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java index 0703170b59b..0189589d8a9 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java @@ -24,7 +24,10 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import com.oracle.bmc.retrier.DefaultRetryCondition; +import com.oracle.bmc.retrier.RetryConfiguration; import com.oracle.bmc.util.internal.Consumer; +import com.oracle.bmc.waiter.ExponentialBackoffDelayStrategy; import org.junit.After; import org.junit.Before; import org.junit.Rule; @@ -65,8 +68,14 @@ public class MultipartObjectAssemblerTest { private static final Map OPC_META = new HashMap<>(); private static final boolean ALLOW_OVERWRITE = false; + private static final RetryConfiguration RETRY_CONFIGURATION = RetryConfiguration.builder() + .delayStrategy(new ExponentialBackoffDelayStrategy(120000)) + .retryCondition(new DefaultRetryCondition()) + .build(); + private ExecutorService executorService; private MultipartObjectAssembler assembler; + private MultipartObjectAssembler assemblerWithRetryConfiguration; @Mock private Consumer mockInvocationCallback; @@ -87,6 +96,17 @@ public void setUp() { .objectName(OBJECT) .service(service) .build(); + assemblerWithRetryConfiguration = + MultipartObjectAssembler.builder() + .allowOverwrite(ALLOW_OVERWRITE) + .bucketName(BUCKET) + .executorService(executorService) + .invocationCallback(mockInvocationCallback) + .namespaceName(NAMESPACE) + .objectName(OBJECT) + .service(service) + .retryConfiguration(RETRY_CONFIGURATION) + .build(); } @After @@ -100,6 +120,34 @@ public void newRequest_andVerifyManifest() { initializeCreateMultipartUpload(uploadId); + MultipartManifest manifest = + assemblerWithRetryConfiguration.newRequest(CONTENT_TYPE, CONTENT_LANGUAGE, CONTENT_ENCODING, OPC_META); + assertNotNull(manifest); + assertEquals(uploadId, manifest.getUploadId()); + + ArgumentCaptor captor = + ArgumentCaptor.forClass(CreateMultipartUploadRequest.class); + verify(service).createMultipartUpload(captor.capture()); + + CreateMultipartUploadRequest request = captor.getValue(); + assertEquals(NAMESPACE, request.getNamespaceName()); + assertEquals(BUCKET, request.getBucketName()); + assertEquals(OBJECT, request.getCreateMultipartUploadDetails().getObject()); + assertEquals(CONTENT_TYPE, request.getCreateMultipartUploadDetails().getContentType()); + assertEquals( + CONTENT_LANGUAGE, request.getCreateMultipartUploadDetails().getContentLanguage()); + assertEquals( + CONTENT_ENCODING, request.getCreateMultipartUploadDetails().getContentEncoding()); + assertEquals(OPC_META, request.getCreateMultipartUploadDetails().getMetadata()); + assertEquals(mockInvocationCallback, request.getInvocationCallback()); + } + + @Test + public void newRequest_andVerifyManifestWithRetryConfiguration() { + String uploadId = "uploadId"; + + initializeCreateMultipartUpload(uploadId); + MultipartManifest manifest = assembler.newRequest(CONTENT_TYPE, CONTENT_LANGUAGE, CONTENT_ENCODING, OPC_META); assertNotNull(manifest); @@ -302,6 +350,72 @@ public void addParts_allSuccessful_commit() throws Exception { file.delete(); } + @Test + public void addParts_allSuccessful_withRetryConfiguration_commit() throws Exception { + String uploadId = "uploadId"; + initializeCreateMultipartUpload(uploadId); + MultipartManifest manifest = + assemblerWithRetryConfiguration.newRequest(CONTENT_TYPE, CONTENT_LANGUAGE, CONTENT_ENCODING, OPC_META); + + byte[] bytes = "abcd".getBytes(); + + File file = File.createTempFile("unitTest", ".txt"); + file.deleteOnExit(); + try (FileOutputStream fos = new FileOutputStream(file)) { + fos.write(bytes); + } + + String etag1 = "etag1"; + String etag2 = "etag2"; + UploadPartResponse uploadPartResponse1 = UploadPartResponse.builder().eTag(etag1).build(); + UploadPartResponse uploadPartResponse2 = UploadPartResponse.builder().eTag(etag2).build(); + when(service.uploadPart(any(UploadPartRequest.class))) + .thenReturn(uploadPartResponse1) + .thenReturn(uploadPartResponse2); + + CommitMultipartUploadResponse finalCommitResponse = + CommitMultipartUploadResponse.builder().build(); + when(service.commitMultipartUpload(any(CommitMultipartUploadRequest.class))) + .thenReturn(finalCommitResponse); + + String md5_1 = "md5_1"; + String md5_2 = "md5_2"; + + assemblerWithRetryConfiguration.addPart(file, md5_1); + assemblerWithRetryConfiguration.addPart(StreamUtils.createByteArrayInputStream(bytes), bytes.length, md5_2); + + CommitMultipartUploadResponse commitResponse = assemblerWithRetryConfiguration.commit(); + assertSame(finalCommitResponse, commitResponse); + + ArgumentCaptor commitCaptor = + ArgumentCaptor.forClass(CommitMultipartUploadRequest.class); + verify(service).commitMultipartUpload(commitCaptor.capture()); + CommitMultipartUploadRequest actualCommitRequest = commitCaptor.getValue(); + assertEquals(NAMESPACE, actualCommitRequest.getNamespaceName()); + assertEquals(BUCKET, actualCommitRequest.getBucketName()); + assertEquals(OBJECT, actualCommitRequest.getObjectName()); + assertEquals(uploadId, actualCommitRequest.getUploadId()); + assertEquals(mockInvocationCallback, actualCommitRequest.getInvocationCallback()); + + assertTrue(manifest.isUploadComplete()); + assertTrue(manifest.isUploadSuccessful()); + assertEquals(2, manifest.listCompletedParts().size()); + assertEquals(1, manifest.listCompletedParts().get(0).getPartNum().intValue()); + assertEquals(etag1, manifest.listCompletedParts().get(0).getEtag()); + assertEquals(2, manifest.listCompletedParts().get(1).getPartNum().intValue()); + assertEquals(etag2, manifest.listCompletedParts().get(1).getEtag()); + + ArgumentCaptor uploadCaptor = + ArgumentCaptor.forClass(UploadPartRequest.class); + verify(service, times(2)).uploadPart(uploadCaptor.capture()); + verifyUploadPart(uploadCaptor.getAllValues().get(0), uploadId, 1, md5_1); + verifyUploadPart(uploadCaptor.getAllValues().get(1), uploadId, 2, md5_2); + + uploadCaptor.getAllValues().forEach(r -> assertSame(RETRY_CONFIGURATION, r.getRetryConfiguration())); + + file.delete(); + } + @Test public void addParts_someFailed_commitFailure() throws Exception { String uploadId = "uploadId"; diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java index a4b0f388cda..f2643953670 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java @@ -38,7 +38,10 @@ import com.oracle.bmc.objectstorage.transfer.UploadManager.UploadResponse; import com.oracle.bmc.objectstorage.transfer.internal.MultipartManifestImpl; import com.oracle.bmc.objectstorage.transfer.internal.MultipartUtils; +import com.oracle.bmc.retrier.DefaultRetryCondition; +import com.oracle.bmc.retrier.RetryConfiguration; import com.oracle.bmc.util.StreamUtils; +import com.oracle.bmc.waiter.ExponentialBackoffDelayStrategy; import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; @@ -144,6 +147,7 @@ public void upload_singleUpload() throws IOException { byte[] buffer = new byte[(int) CONTENT_LENGTH]; putRequestCaptor.getValue().getPutObjectBody().read(buffer); assertEquals(CONTENT, new String(buffer)); + assertSame(UploadManager.RETRY_CONFIGURATION, putRequestCaptor.getValue().getRetryConfiguration()); assertEquals(CONTENT_LENGTH, putRequestCaptor.getValue().getContentLength().longValue()); assertEquals(CLIENT_REQ_ID, putRequestCaptor.getValue().getOpcClientRequestId()); assertSame(METADATA, putRequestCaptor.getValue().getOpcMeta()); @@ -173,6 +177,47 @@ public void upload_singleUpload_enforceMd5() throws Exception { putRequestCaptor.getValue().getContentMD5()); // 'a' times content-length } + @Test + public void upload_singleUpload_uploadRequestRetryConfiguration() throws Exception { + RetryConfiguration retryConfiguration = RetryConfiguration.builder() + .delayStrategy(new ExponentialBackoffDelayStrategy(120000)) + .retryCondition(new DefaultRetryCondition()) + .build(); + + UploadConfiguration uploadConfiguration = + UploadConfiguration.builder().allowMultipartUploads(false).build(); + UploadManager uploadManager = new UploadManager(objectStorage, uploadConfiguration); + + UploadRequest request = createUploadRequest(retryConfiguration); + + ArgumentCaptor putRequestCaptor = + ArgumentCaptor.forClass(PutObjectRequest.class); + PutObjectResponse putResponse = + PutObjectResponse.builder() + .eTag("etag") + .opcContentMd5("md5") + .opcRequestId(REQ_ID) + .opcClientRequestId(CLIENT_REQ_ID) + .build(); + when(objectStorage.putObject(putRequestCaptor.capture())).thenReturn(putResponse); + + UploadResponse uploadResponse = uploadManager.upload(request); + + assertNotNull(uploadResponse); + assertEquals("etag", uploadResponse.getETag()); + assertEquals("md5", uploadResponse.getContentMd5()); + assertNull(uploadResponse.getMultipartMd5()); + assertEquals(REQ_ID, uploadResponse.getOpcRequestId()); + assertEquals(CLIENT_REQ_ID, uploadResponse.getOpcClientRequestId()); + byte[] buffer = new byte[(int) CONTENT_LENGTH]; + putRequestCaptor.getValue().getPutObjectBody().read(buffer); + assertSame(retryConfiguration, putRequestCaptor.getValue().getRetryConfiguration()); + assertEquals(CONTENT, new String(buffer)); + assertEquals(CONTENT_LENGTH, putRequestCaptor.getValue().getContentLength().longValue()); + assertEquals(CLIENT_REQ_ID, putRequestCaptor.getValue().getOpcClientRequestId()); + assertSame(METADATA, putRequestCaptor.getValue().getOpcMeta()); + } + @Test(expected = BmcException.class) public void upload_singleUpload_enforceMd5_streamTooLargeToBuffer() { UploadConfiguration uploadConfiguration = @@ -655,7 +700,7 @@ private static UploadConfiguration getMultipartUploadConfiguration() { .build(); } - private UploadRequest createUploadRequest(ProgressReporter progressReporter) { + private UploadRequest createUploadRequest(ProgressReporter progressReporter, RetryConfiguration retryConfiguration) { PutObjectRequest request = PutObjectRequest.builder() .opcMeta(METADATA) @@ -666,14 +711,23 @@ private UploadRequest createUploadRequest(ProgressReporter progressReporter) { .namespaceName(NAMESPACE_NAME) .bucketName(BUCKET_NAME) .objectName(OBJECT_NAME) + .retryConfiguration(retryConfiguration) .build(); return UploadRequest.builder(body, CONTENT_LENGTH) .progressReporter(progressReporter) .build(request); } + private UploadRequest createUploadRequest(ProgressReporter progressReporter) { + return createUploadRequest(progressReporter, null); + } + private UploadRequest createUploadRequest() { - return createUploadRequest(null); + return createUploadRequest(null, null); + } + + private UploadRequest createUploadRequest(RetryConfiguration retryConfiguration) { + return createUploadRequest(null, retryConfiguration); } private static void validateUploadResponseForMultipart(final UploadResponse response) { From 6c7d61584187e19b4f25aeeb90f2afb8a25a0b3a Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Fri, 21 Aug 2020 12:27:23 -0600 Subject: [PATCH 2/6] added static method declaration and updated comments --- .../oracle/bmc/objectstorage/transfer/UploadManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java index c90be1580d7..31c8e356ef0 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java @@ -249,13 +249,13 @@ private UploadResponse multipartUpload(UploadRequest uploadRequest) { } /** - * Determines the first non-null RetryCondition + * Determines the first non-null RetryConfiguration * 1 -> RetryConfiguration set on UploadConfiguration * 2 -> Default static RetryConfiguration for UploadManager * - * @return RetryConfiguration + * @return RetryConfiguration first non-null condition or UploadManager default */ - private RetryConfiguration getRetryToUse(RetryConfiguration ...configs) { + private static RetryConfiguration getRetryToUse(RetryConfiguration ...configs) { for (RetryConfiguration cfg : configs) { if (cfg != null) return cfg; From 639bfa92fa0118b8665c79b87c6ffc551e9824c1 Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Fri, 21 Aug 2020 12:41:44 -0600 Subject: [PATCH 3/6] Refactored createAssembler and unit tests to remove redundant param --- .../objectstorage/transfer/UploadManager.java | 17 ++++++----------- .../transfer/UploadManagerTest.java | 7 ------- 2 files changed, 6 insertions(+), 18 deletions(-) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java index 31c8e356ef0..46dd77f35cd 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java @@ -90,6 +90,7 @@ public boolean shouldBeRetried(@NonNull BmcException e) { * @param uploadDetails The upload request. * @return The response. * @throws BmcException if the upload fails for any reason. + * @throws IllegalArgumentException if UploadRequest */ public UploadResponse upload(UploadRequest uploadDetails) { if (MultipartUtils.shouldUseMultipart( @@ -171,8 +172,7 @@ private UploadResponse multipartUpload(UploadRequest uploadRequest) { shutdownExecutor = true; } - MultipartObjectAssembler assembler = - createAssembler(request, uploadRequest, executorServiceToUse); + MultipartObjectAssembler assembler = createAssembler(uploadRequest, executorServiceToUse); MultipartManifest manifest = null; try { manifest = @@ -265,15 +265,10 @@ private static RetryConfiguration getRetryToUse(RetryConfiguration ...configs) { } @VisibleForTesting - protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, - UploadRequest uploadRequest, - ExecutorService executorService) { - - // in case request != uploadRequest.putObjectRequest then choose the correct RetryConfiguration - RetryConfiguration retryToUse = getRetryToUse( - uploadRequest.putObjectRequest.getRetryConfiguration(), - request.getRetryConfiguration()); + protected MultipartObjectAssembler createAssembler(UploadRequest uploadRequest, ExecutorService executorService) { + + PutObjectRequest request = uploadRequest.putObjectRequest; + RetryConfiguration retryToUse = getRetryToUse(request.getRetryConfiguration()); return MultipartObjectAssembler.builder() .allowOverwrite(uploadRequest.allowOverwrite) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java index f2643953670..e1f2050b70b 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java @@ -242,7 +242,6 @@ public void upload_multipartUpload() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -280,7 +279,6 @@ public void upload_multipartUpload_enforceMd5() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -320,7 +318,6 @@ public void upload_multipartUpload_duplicatableStream_canNotParallelize() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -357,7 +354,6 @@ public void upload_multipartUpload_exception() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -391,7 +387,6 @@ public void upload_multipartUpload_exception_noAbort() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -449,7 +444,6 @@ public void multipartUpload_progressReporter() { new UploadManager(objectStorage, getMultipartUploadConfiguration()) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -646,7 +640,6 @@ public void upload_multipartUpload_with_minConfiguration() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( - PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; From 75853a63e8e90cc4c0e2e76cf9a5c32ce9b0ff9d Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Fri, 21 Aug 2020 12:58:15 -0600 Subject: [PATCH 4/6] rollback of createAssembler changes --- .../objectstorage/transfer/UploadManager.java | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java index 46dd77f35cd..31c8e356ef0 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/main/java/com/oracle/bmc/objectstorage/transfer/UploadManager.java @@ -90,7 +90,6 @@ public boolean shouldBeRetried(@NonNull BmcException e) { * @param uploadDetails The upload request. * @return The response. * @throws BmcException if the upload fails for any reason. - * @throws IllegalArgumentException if UploadRequest */ public UploadResponse upload(UploadRequest uploadDetails) { if (MultipartUtils.shouldUseMultipart( @@ -172,7 +171,8 @@ private UploadResponse multipartUpload(UploadRequest uploadRequest) { shutdownExecutor = true; } - MultipartObjectAssembler assembler = createAssembler(uploadRequest, executorServiceToUse); + MultipartObjectAssembler assembler = + createAssembler(request, uploadRequest, executorServiceToUse); MultipartManifest manifest = null; try { manifest = @@ -265,10 +265,15 @@ private static RetryConfiguration getRetryToUse(RetryConfiguration ...configs) { } @VisibleForTesting - protected MultipartObjectAssembler createAssembler(UploadRequest uploadRequest, ExecutorService executorService) { - - PutObjectRequest request = uploadRequest.putObjectRequest; - RetryConfiguration retryToUse = getRetryToUse(request.getRetryConfiguration()); + protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, + UploadRequest uploadRequest, + ExecutorService executorService) { + + // in case request != uploadRequest.putObjectRequest then choose the correct RetryConfiguration + RetryConfiguration retryToUse = getRetryToUse( + uploadRequest.putObjectRequest.getRetryConfiguration(), + request.getRetryConfiguration()); return MultipartObjectAssembler.builder() .allowOverwrite(uploadRequest.allowOverwrite) From a32b0a7ce6e7ac48657f0d242b364833509a1e1e Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Fri, 21 Aug 2020 15:26:22 -0600 Subject: [PATCH 5/6] rollback of createAssembler changes, missed the unit test --- .../bmc/objectstorage/transfer/UploadManagerTest.java | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java index e1f2050b70b..f2643953670 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/UploadManagerTest.java @@ -242,6 +242,7 @@ public void upload_multipartUpload() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -279,6 +280,7 @@ public void upload_multipartUpload_enforceMd5() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -318,6 +320,7 @@ public void upload_multipartUpload_duplicatableStream_canNotParallelize() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -354,6 +357,7 @@ public void upload_multipartUpload_exception() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -387,6 +391,7 @@ public void upload_multipartUpload_exception_noAbort() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -444,6 +449,7 @@ public void multipartUpload_progressReporter() { new UploadManager(objectStorage, getMultipartUploadConfiguration()) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; @@ -640,6 +646,7 @@ public void upload_multipartUpload_with_minConfiguration() { new UploadManager(objectStorage, uploadConfiguration) { @Override protected MultipartObjectAssembler createAssembler( + PutObjectRequest request, UploadRequest uploadRequest, ExecutorService executorService) { return assembler; From 3eff3d3222d5f573e1177b896f23428e6518f512 Mon Sep 17 00:00:00 2001 From: Benjamin Pucci Date: Mon, 24 Aug 2020 16:00:10 -0600 Subject: [PATCH 6/6] fixed reversed test naming --- .../objectstorage/transfer/MultipartObjectAssemblerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java index 0189589d8a9..0cbcd981892 100644 --- a/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java +++ b/bmc-objectstorage/bmc-objectstorage-extensions/src/test/java/com/oracle/bmc/objectstorage/transfer/MultipartObjectAssemblerTest.java @@ -115,7 +115,7 @@ public void tearDown() { } @Test - public void newRequest_andVerifyManifest() { + public void newRequest_andVerifyManifestWithRetryConfiguration() { String uploadId = "uploadId"; initializeCreateMultipartUpload(uploadId); @@ -143,7 +143,7 @@ public void newRequest_andVerifyManifest() { } @Test - public void newRequest_andVerifyManifestWithRetryConfiguration() { + public void newRequest_andVerifyManifest() { String uploadId = "uploadId"; initializeCreateMultipartUpload(uploadId);