From 2101e9149e1db1d10b0797ceb6a573cc8f098ac3 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 10:09:21 -0500 Subject: [PATCH 01/14] feat(storage): update AWSS3StoragePlugin to support multiple buckets --- .../storage/s3/AWSS3StoragePlugin.java | 248 ++++++++++++------ .../storage/s3/AWSS3StoragePluginTest.kt | 85 ++++++ core/api/core.api | 48 ++++ .../core/configuration/AmplifyOutputsData.kt | 18 +- .../amplifyframework/storage/BucketInfo.kt | 17 ++ .../storage/InvalidStorageBucketException.kt | 23 ++ .../amplifyframework/storage/StorageBucket.kt | 32 +++ .../storage/options/StorageGetUrlOptions.java | 12 +- .../storage/options/StorageOptions.java | 29 ++ .../configuration/AmplifyOutputsDataTest.kt | 40 +++ .../AmplifyOutputsDataBuilder.kt | 10 + 11 files changed, 482 insertions(+), 80 deletions(-) create mode 100644 core/src/main/java/com/amplifyframework/storage/BucketInfo.kt create mode 100644 core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt create mode 100644 core/src/main/java/com/amplifyframework/storage/StorageBucket.kt diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 9375a14d9..d7b7398ec 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -18,6 +18,7 @@ import android.annotation.SuppressLint; import android.content.Context; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import androidx.annotation.OptIn; import androidx.annotation.VisibleForTesting; @@ -28,7 +29,11 @@ import com.amplifyframework.core.Consumer; import com.amplifyframework.core.NoOpConsumer; import com.amplifyframework.core.configuration.AmplifyOutputsData; +import com.amplifyframework.storage.InvalidStorageBucketException; +import com.amplifyframework.storage.OutputsStorageBucket; +import com.amplifyframework.storage.ResolvedStorageBucket; import com.amplifyframework.storage.StorageAccessLevel; +import com.amplifyframework.storage.StorageBucket; import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StoragePath; import com.amplifyframework.storage.StoragePlugin; @@ -95,6 +100,9 @@ import java.io.File; import java.io.InputStream; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -121,11 +129,16 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private final ExecutorService executorService; private final AuthCredentialsProvider authCredentialsProvider; private final AWSS3StoragePluginConfiguration awsS3StoragePluginConfiguration; - private AWSS3StorageService storageService; + private AWSS3StorageService defaultStorageService; @SuppressWarnings("deprecation") private StorageAccessLevel defaultAccessLevel; private int defaultUrlExpiration; + private Map awsS3StorageServicesByBucketName; + private Context context; + @SuppressLint("UnsafeOptInUsageError") + private List configuredBuckets; + /** * Constructs the AWS S3 Storage Plugin initializing the executor service. */ @@ -252,6 +265,7 @@ public void configure(@NonNull AmplifyOutputsData configuration, @NonNull Contex ); } + this.configuredBuckets = storage.getBuckets(); configure(context, storage.getAwsRegion(), storage.getBucketName()); } @@ -262,7 +276,10 @@ private void configure( @NonNull String bucket ) throws StorageException { try { - this.storageService = (AWSS3StorageService) storageServiceFactory.create(context, region, bucket); + this.context = context; + this.defaultStorageService = (AWSS3StorageService) storageServiceFactory.create(context, region, bucket); + this.awsS3StorageServicesByBucketName = new HashMap(); + this.awsS3StorageServicesByBucketName.put(bucket, this.defaultStorageService); } catch (RuntimeException exception) { throw new StorageException( "Failed to create storage service.", @@ -280,7 +297,7 @@ private void configure( @NonNull @Override public S3Client getEscapeHatch() { - return storageService.getClient(); + return defaultStorageService.getClient(); } @NonNull @@ -334,6 +351,18 @@ public StorageGetUrlOperation getUrl( validateObjectExistence ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (InvalidStorageBucketException exception) { + onError.accept( + new StorageException( + "Unable to find bucket from name in Amplify Outputs.", + exception, + "Ensure the bucket name used is available in Amplify Outputs.") + ); + } + AWSS3StorageGetPresignedUrlOperation operation = new AWSS3StorageGetPresignedUrlOperation( storageService, @@ -369,6 +398,18 @@ public StorageGetUrlOperation getUrl( validateObjectExistence ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (InvalidStorageBucketException exception) { + onError.accept( + new StorageException( + "Unable to find bucket from name in Amplify Outputs.", + exception, + "Ensure the bucket name used is available in Amplify Outputs.") + ); + } + AWSS3StoragePathGetPresignedUrlOperation operation = new AWSS3StoragePathGetPresignedUrlOperation( storageService, @@ -457,14 +498,14 @@ public StorageDownloadFileOperation downloadFile( ); AWSS3StorageDownloadFileOperation operation = new AWSS3StorageDownloadFileOperation( - storageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onProgress, + onSuccess, + onError ); operation.start(); @@ -493,7 +534,7 @@ public StorageDownloadFileOperation downloadFile( AWSS3StoragePathDownloadFileOperation operation = new AWSS3StoragePathDownloadFileOperation( request, - storageService, + defaultStorageService, executorService, authCredentialsProvider, onProgress, @@ -584,14 +625,14 @@ public StorageUploadFileOperation uploadFile( ); AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( - storageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onProgress, + onSuccess, + onError ); operation.start(); @@ -623,7 +664,7 @@ public StorageUploadFileOperation uploadFile( AWSS3StoragePathUploadFileOperation operation = new AWSS3StoragePathUploadFileOperation( request, - storageService, + defaultStorageService, executorService, authCredentialsProvider, onProgress, @@ -712,14 +753,14 @@ public StorageUploadInputStreamOperation uploadInputStream( ); AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( - storageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - request, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + request, + onProgress, + onSuccess, + onError ); operation.start(); @@ -752,7 +793,7 @@ public StorageUploadInputStreamOperation uploadInputStream( AWSS3StoragePathUploadInputStreamOperation operation = new AWSS3StoragePathUploadInputStreamOperation( request, - storageService, + defaultStorageService, executorService, authCredentialsProvider, onProgress, @@ -804,13 +845,13 @@ public StorageRemoveOperation remove( AWSS3StorageRemoveOperation operation = new AWSS3StorageRemoveOperation( - storageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onSuccess, + onError); operation.start(); @@ -829,7 +870,7 @@ public StorageRemoveOperation remove( AWSS3StoragePathRemoveOperation operation = new AWSS3StoragePathRemoveOperation( - storageService, + defaultStorageService, executorService, authCredentialsProvider, request, @@ -849,55 +890,55 @@ public void getTransfer( @NonNull Consumer onError) { executorService.submit(() -> { try { - TransferRecord transferRecord = storageService.getTransfer(transferId); + TransferRecord transferRecord = defaultStorageService.getTransfer(transferId); if (transferRecord != null) { TransferObserver transferObserver = new TransferObserver( transferRecord.getId(), - storageService.getTransferManager().getTransferStatusUpdater(), - transferRecord.getBucketName(), - transferRecord.getKey(), - transferRecord.getFile(), - null, - transferRecord.getState() != null ? transferRecord.getState() : TransferState.UNKNOWN); + defaultStorageService.getTransferManager().getTransferStatusUpdater(), + transferRecord.getBucketName(), + transferRecord.getKey(), + transferRecord.getFile(), + null, + transferRecord.getState() != null ? transferRecord.getState() : TransferState.UNKNOWN); TransferType transferType = transferRecord.getType(); switch (Objects.requireNonNull(transferType)) { case UPLOAD: if (transferRecord.getFile().startsWith(TransferStatusUpdater.TEMP_FILE_PREFIX)) { AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( - transferId, - storageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(operation); } else { AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( - transferId, - storageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(operation); } break; case DOWNLOAD: AWSS3StorageDownloadFileOperation downloadFileOperation = new AWSS3StorageDownloadFileOperation( - transferId, - new File(transferRecord.getFile()), - storageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + new File(transferRecord.getFile()), + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(downloadFileOperation); break; default: @@ -960,13 +1001,13 @@ public StorageListOperation list(@NonNull String path, AWSS3StorageListOperation operation = new AWSS3StorageListOperation( - storageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onSuccess, + onError); operation.start(); @@ -989,7 +1030,7 @@ public StorageListOperation list( AWSS3StoragePathListOperation operation = new AWSS3StoragePathListOperation( - storageService, + defaultStorageService, executorService, authCredentialsProvider, request, @@ -1001,6 +1042,63 @@ public StorageListOperation list( return operation; } + @SuppressLint("UnsafeOptInUsageError") + @VisibleForTesting + @NonNull + AWSS3StorageService getStorageService(@Nullable StorageBucket bucket) throws InvalidStorageBucketException { + if (bucket == null) { + return defaultStorageService; + } + + if (bucket instanceof OutputsStorageBucket) { + AWSS3StorageService service = getAWSS3StorageService((OutputsStorageBucket) bucket); + if (service == null) { + throw new InvalidStorageBucketException(); + } else { + return service; + } + } + + if (bucket instanceof ResolvedStorageBucket) { + return getAWSS3StorageService((ResolvedStorageBucket) bucket); + } + + return defaultStorageService; + } + + @SuppressLint("UnsafeOptInUsageError") + private AWSS3StorageService getAWSS3StorageService(OutputsStorageBucket outputsStorageBucket) { + if (configuredBuckets != null && !configuredBuckets.isEmpty()) { + String name = outputsStorageBucket.getName(); + for (AmplifyOutputsData.StorageBucket configuredBucket : configuredBuckets) { + if (configuredBucket.getName().equals(name)) { + String bucketName = configuredBucket.getBucketName(); + AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); + if (service == null) { + String region = configuredBucket.getAwsRegion(); + service = (AWSS3StorageService) storageServiceFactory.create(context, region, bucketName); + awsS3StorageServicesByBucketName.put(bucketName, service); + } + + return service; + } + } + } + return null; + } + + @SuppressLint("UnsafeOptInUsageError") + private AWSS3StorageService getAWSS3StorageService(ResolvedStorageBucket resolvedStorageBucket) { + String bucketName = resolvedStorageBucket.getBucketInfo().getName(); + AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); + if (service == null) { + String region = resolvedStorageBucket.getBucketInfo().getRegion(); + service = (AWSS3StorageService) storageServiceFactory.create(context, region, bucketName); + awsS3StorageServicesByBucketName.put(bucketName, service); + } + return service; + } + /** * Holds the keys for the various configuration properties for this plugin. */ diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt index ebcbdd9fd..e5e13cfe4 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt @@ -15,11 +15,15 @@ package com.amplifyframework.storage.s3 +import com.amplifyframework.storage.BucketInfo +import com.amplifyframework.storage.InvalidStorageBucketException +import com.amplifyframework.storage.StorageBucket import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.s3.service.AWSS3StorageService import com.amplifyframework.storage.s3.service.StorageService import com.amplifyframework.testutils.configuration.amplifyOutputsData import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.shouldNotBe import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -63,4 +67,85 @@ class AWSS3StoragePluginTest { plugin.configure(data, mockk()) } } + + @Test + fun `getStorageService returns default storage service if bucket is null`() { + val data = amplifyOutputsData { + storage { + awsRegion = "test-region" + bucketName = "test-bucket" + buckets { + awsRegion = "test-region" + bucketName = "test-bucket" + name = "test=name" + } + } + } + + plugin.configure(data, mockk()) + val service = plugin.getStorageService(null) + service shouldNotBe null + } + + @Test + fun `get AWSS3StorageService from BucketInfo`() { + val data = amplifyOutputsData { + storage { + awsRegion = "test-region" + bucketName = "test-bucket" + buckets { + awsRegion = "test-region" + bucketName = "test-bucket" + name = "test=name" + } + } + } + + plugin.configure(data, mockk()) + val bucketInfo = BucketInfo("test-bucket", "test-region") + val bucket = StorageBucket.fromBucketInfo(bucketInfo) + val service = plugin.getStorageService(bucket) + service shouldNotBe null + } + + @Test + fun `get AWSS3StorageService from AmplifyOutputs`() { + val data = amplifyOutputsData { + storage { + awsRegion = "test-region" + bucketName = "test-bucket" + buckets { + awsRegion = "test-region" + bucketName = "test-bucket" + name = "test=name" + } + } + } + + plugin.configure(data, mockk()) + val bucket = StorageBucket.fromOutputs("test=name") + val service = plugin.getStorageService(bucket) + service shouldNotBe null + } + + @Test + fun `getStorageService throws InvalidStorageBucketException`() { + val data = amplifyOutputsData { + storage { + awsRegion = "test-region" + bucketName = "test-bucket" + buckets { + awsRegion = "test-region" + bucketName = "test-bucket" + name = "test=name" + } + } + } + + plugin.configure(data, mockk()) + val bucket = StorageBucket.fromOutputs("myBucket") + shouldThrow { + plugin.getStorageService(bucket) + } + } } diff --git a/core/api/core.api b/core/api/core.api index 4beafbf12..6df942a7d 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -3930,6 +3930,19 @@ public final class com/amplifyframework/predictions/result/TranslateTextResult$B public fun translatedText (Ljava/lang/String;)Lcom/amplifyframework/predictions/result/TranslateTextResult$Builder; } +public final class com/amplifyframework/storage/BucketInfo { + public fun (Ljava/lang/String;Ljava/lang/String;)V + public final fun component1 ()Ljava/lang/String; + public final fun component2 ()Ljava/lang/String; + public final fun copy (Ljava/lang/String;Ljava/lang/String;)Lcom/amplifyframework/storage/BucketInfo; + public static synthetic fun copy$default (Lcom/amplifyframework/storage/BucketInfo;Ljava/lang/String;Ljava/lang/String;ILjava/lang/Object;)Lcom/amplifyframework/storage/BucketInfo; + public fun equals (Ljava/lang/Object;)Z + public final fun getName ()Ljava/lang/String; + public final fun getRegion ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + public final class com/amplifyframework/storage/IdentityIdProvidedStoragePath : com/amplifyframework/storage/StoragePath { public final fun copy (Lkotlin/jvm/functions/Function1;)Lcom/amplifyframework/storage/IdentityIdProvidedStoragePath; public static synthetic fun copy$default (Lcom/amplifyframework/storage/IdentityIdProvidedStoragePath;Lkotlin/jvm/functions/Function1;ILjava/lang/Object;)Lcom/amplifyframework/storage/IdentityIdProvidedStoragePath; @@ -3938,6 +3951,9 @@ public final class com/amplifyframework/storage/IdentityIdProvidedStoragePath : public fun toString ()Ljava/lang/String; } +public final class com/amplifyframework/storage/InvalidStorageBucketException : com/amplifyframework/AmplifyException { +} + public final class com/amplifyframework/storage/ObjectMetadata { public static final field CACHE_CONTROL Ljava/lang/String; public static final field CONTENT_DISPOSITION Ljava/lang/String; @@ -3991,6 +4007,26 @@ public final class com/amplifyframework/storage/ObjectMetadata { public final class com/amplifyframework/storage/ObjectMetadata$Companion { } +public final class com/amplifyframework/storage/OutputsStorageBucket : com/amplifyframework/storage/StorageBucket { + public final fun component1 ()Ljava/lang/String; + public final fun copy (Ljava/lang/String;)Lcom/amplifyframework/storage/OutputsStorageBucket; + public static synthetic fun copy$default (Lcom/amplifyframework/storage/OutputsStorageBucket;Ljava/lang/String;ILjava/lang/Object;)Lcom/amplifyframework/storage/OutputsStorageBucket; + public fun equals (Ljava/lang/Object;)Z + public final fun getName ()Ljava/lang/String; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + +public final class com/amplifyframework/storage/ResolvedStorageBucket : com/amplifyframework/storage/StorageBucket { + public final fun component1 ()Lcom/amplifyframework/storage/BucketInfo; + public final fun copy (Lcom/amplifyframework/storage/BucketInfo;)Lcom/amplifyframework/storage/ResolvedStorageBucket; + public static synthetic fun copy$default (Lcom/amplifyframework/storage/ResolvedStorageBucket;Lcom/amplifyframework/storage/BucketInfo;ILjava/lang/Object;)Lcom/amplifyframework/storage/ResolvedStorageBucket; + public fun equals (Ljava/lang/Object;)Z + public final fun getBucketInfo ()Lcom/amplifyframework/storage/BucketInfo; + public fun hashCode ()I + public fun toString ()Ljava/lang/String; +} + public final class com/amplifyframework/storage/StorageAccessLevel : java/lang/Enum { public static final field PRIVATE Lcom/amplifyframework/storage/StorageAccessLevel; public static final field PROTECTED Lcom/amplifyframework/storage/StorageAccessLevel; @@ -3999,6 +4035,18 @@ public final class com/amplifyframework/storage/StorageAccessLevel : java/lang/E public static fun values ()[Lcom/amplifyframework/storage/StorageAccessLevel; } +public abstract class com/amplifyframework/storage/StorageBucket { + public static final field Companion Lcom/amplifyframework/storage/StorageBucket$Companion; + public fun ()V + public static final fun fromBucketInfo (Lcom/amplifyframework/storage/BucketInfo;)Lcom/amplifyframework/storage/StorageBucket; + public static final fun fromOutputs (Ljava/lang/String;)Lcom/amplifyframework/storage/StorageBucket; +} + +public final class com/amplifyframework/storage/StorageBucket$Companion { + public final fun fromBucketInfo (Lcom/amplifyframework/storage/BucketInfo;)Lcom/amplifyframework/storage/StorageBucket; + public final fun fromOutputs (Ljava/lang/String;)Lcom/amplifyframework/storage/StorageBucket; +} + public final class com/amplifyframework/storage/StorageCategory : com/amplifyframework/core/category/Category, com/amplifyframework/storage/StorageCategoryBehavior { public fun ()V public fun downloadFile (Lcom/amplifyframework/storage/StoragePath;Ljava/io/File;Lcom/amplifyframework/core/Consumer;Lcom/amplifyframework/core/Consumer;)Lcom/amplifyframework/storage/operation/StorageDownloadFileOperation; diff --git a/core/src/main/java/com/amplifyframework/core/configuration/AmplifyOutputsData.kt b/core/src/main/java/com/amplifyframework/core/configuration/AmplifyOutputsData.kt index 0dae51b79..ac76d9634 100644 --- a/core/src/main/java/com/amplifyframework/core/configuration/AmplifyOutputsData.kt +++ b/core/src/main/java/com/amplifyframework/core/configuration/AmplifyOutputsData.kt @@ -187,6 +187,14 @@ interface AmplifyOutputsData { interface Storage { val awsRegion: String val bucketName: String + val buckets: List + } + + @InternalAmplifyApi + interface StorageBucket { + val name: String + val awsRegion: String + val bucketName: String } @InternalAmplifyApi @@ -353,9 +361,17 @@ internal data class AmplifyOutputsDataImpl( @Serializable data class Storage( override val awsRegion: String, - override val bucketName: String + override val bucketName: String, + override val buckets: List = emptyList() ) : AmplifyOutputsData.Storage + @Serializable + data class StorageBucket( + override val name: String, + override val awsRegion: String, + override val bucketName: String + ) : AmplifyOutputsData.StorageBucket + @Serializable data class AmazonLocationServiceConfig( override val style: String diff --git a/core/src/main/java/com/amplifyframework/storage/BucketInfo.kt b/core/src/main/java/com/amplifyframework/storage/BucketInfo.kt new file mode 100644 index 000000000..ca3da3606 --- /dev/null +++ b/core/src/main/java/com/amplifyframework/storage/BucketInfo.kt @@ -0,0 +1,17 @@ +/* + * Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage + +data class BucketInfo(val name: String, val region: String) diff --git a/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt new file mode 100644 index 000000000..ab8d2d9cb --- /dev/null +++ b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt @@ -0,0 +1,23 @@ +/* + * Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage + +import com.amplifyframework.AmplifyException +import com.amplifyframework.annotations.InternalAmplifyApi + +class InvalidStorageBucketException @InternalAmplifyApi constructor( + message: String = "Unable to find bucket from name in Amplify Outputs.", + recoverySuggestion: String = "Ensure the bucket name used is available in Amplify Outputs." +) : AmplifyException(message, recoverySuggestion) diff --git a/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt b/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt new file mode 100644 index 000000000..e56946252 --- /dev/null +++ b/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt @@ -0,0 +1,32 @@ +/* + * Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage + +abstract class StorageBucket { + companion object { + @JvmStatic + fun fromOutputs(name: String): StorageBucket = OutputsStorageBucket(name) + @JvmStatic + fun fromBucketInfo(bucketInfo: BucketInfo): StorageBucket = ResolvedStorageBucket(bucketInfo) + } +} + +// While class may be technically public, the constructor is internal, +// Customer is never expected to see or attempt to use this extended class +data class OutputsStorageBucket internal constructor(val name: String) : StorageBucket() + +// While class may be technically public, the constructor is internal, +// Customer is never expected to see or attempt to use this extended class +data class ResolvedStorageBucket internal constructor(val bucketInfo: BucketInfo) : StorageBucket() diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageGetUrlOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageGetUrlOptions.java index ddb3be33d..27ef4f424 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageGetUrlOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageGetUrlOptions.java @@ -32,7 +32,7 @@ public class StorageGetUrlOptions extends StorageOptions { */ @SuppressWarnings("deprecation") protected StorageGetUrlOptions(final Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); this.expires = builder.getExpires(); } @@ -69,9 +69,10 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull StorageGetUrlOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .expires(options.getExpires()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()) + .expires(options.getExpires()); } /** @@ -97,6 +98,7 @@ public boolean equals(Object obj) { StorageGetUrlOptions that = (StorageGetUrlOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()) && ObjectsCompat.equals(getExpires(), that.getExpires()); } } @@ -110,6 +112,7 @@ public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), getTargetIdentityId(), + getBucket(), getExpires() ); } @@ -124,6 +127,7 @@ public String toString() { return "StorageGetUrlOptions {" + "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + ", expires=" + getExpires() + '}'; } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java index 1be0c4431..fd0c6b6c8 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java @@ -19,6 +19,7 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.StorageAccessLevel; +import com.amplifyframework.storage.StorageBucket; import com.amplifyframework.storage.StoragePath; /** @@ -31,11 +32,23 @@ abstract class StorageOptions { private final StorageAccessLevel accessLevel; private final String targetIdentityId; + private final StorageBucket bucket; + @SuppressWarnings("deprecation") StorageOptions(StorageAccessLevel accessLevel, String targetIdentityId) { this.accessLevel = accessLevel; this.targetIdentityId = targetIdentityId; + this.bucket = null; + } + + @SuppressWarnings("deprecation") + StorageOptions(StorageAccessLevel accessLevel, + String targetIdentityId, + StorageBucket bucket) { + this.accessLevel = accessLevel; + this.targetIdentityId = targetIdentityId; + this.bucket = bucket; } /** @@ -61,6 +74,11 @@ public final String getTargetIdentityId() { return targetIdentityId; } + @Nullable + public final StorageBucket getBucket() { + return bucket; + } + /** * Builds storage options. */ @@ -69,6 +87,7 @@ abstract static class Builder { @SuppressWarnings("deprecation") private StorageAccessLevel accessLevel; private String targetIdentityId; + private StorageBucket bucket; /** * Configures the storage access level to set on new @@ -99,6 +118,11 @@ public final B targetIdentityId(@Nullable String targetIdentityId) { return (B) this; } + public final B bucket(StorageBucket bucket) { + this.bucket = bucket; + return (B) this; + } + @SuppressWarnings("deprecation") @Deprecated @Nullable @@ -112,6 +136,11 @@ public final String getTargetIdentityId() { return targetIdentityId; } + @Nullable + public final StorageBucket getBucket() { + return bucket; + } + /** * Constructs and returns a new immutable instance of the * StorageOptions, using the configurations that diff --git a/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt b/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt index a570ed861..1242533f6 100644 --- a/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt +++ b/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt @@ -253,6 +253,44 @@ class AmplifyOutputsDataTest { outputs.storage?.run { awsRegion shouldBe "us-east-1" bucketName shouldBe "myBucket" + buckets.size shouldBe 0 + } + } + + @Test + fun `parses multi-bucket storage configuration`() { + val json = createJson( + Keys.storage to mapOf( + Keys.region to "us-east-1", + Keys.bucket to "myBucket", + Keys.buckets to listOf( + mapOf( + Keys.region to "us-east-1", + Keys.bucket to "myBucket", + Keys.name to "name1" + ), + mapOf( + Keys.region to "us-east-2", + Keys.bucket to "myBucket2", + Keys.name to "name2" + ) + ) + ) + ) + + val outputs = AmplifyOutputsData.deserialize(json) + + outputs.storage.shouldNotBeNull() + outputs.storage?.run { + awsRegion shouldBe "us-east-1" + bucketName shouldBe "myBucket" + buckets.size shouldBe 2 + buckets[0].name shouldBe "name1" + buckets[0].awsRegion shouldBe "us-east-1" + buckets[0].bucketName shouldBe "myBucket" + buckets[1].name shouldBe "name2" + buckets[1].awsRegion shouldBe "us-east-2" + buckets[1].bucketName shouldBe "myBucket2" } } @@ -361,6 +399,8 @@ class AmplifyOutputsDataTest { // Storage const val storage = "storage" const val bucket = "bucket_name" + const val buckets = "buckets" + const val name = "name" // Custom const val custom = "custom" diff --git a/testutils/src/main/java/com/amplifyframework/testutils/configuration/AmplifyOutputsDataBuilder.kt b/testutils/src/main/java/com/amplifyframework/testutils/configuration/AmplifyOutputsDataBuilder.kt index eada856d3..f799ccf69 100644 --- a/testutils/src/main/java/com/amplifyframework/testutils/configuration/AmplifyOutputsDataBuilder.kt +++ b/testutils/src/main/java/com/amplifyframework/testutils/configuration/AmplifyOutputsDataBuilder.kt @@ -163,4 +163,14 @@ class NotificationsBuilder : AmplifyOutputsData.Notifications { class StorageBuilder : AmplifyOutputsData.Storage { override var awsRegion: String = "us-east-1" override var bucketName: String = "bucket-name" + override var buckets: MutableList = mutableListOf() + fun buckets(func: StorageBucketBuilder.() -> Unit) { + buckets += StorageBucketBuilder().apply(func) + } +} + +class StorageBucketBuilder : AmplifyOutputsData.StorageBucket { + override var awsRegion: String = "us-east-1" + override var bucketName: String = "bucket-name" + override var name: String = "test-name" } From 1ca99f0cb9b501d7bb218923d8796a33a8528d16 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 11:52:04 -0500 Subject: [PATCH 02/14] add code comments --- .../storage/InvalidStorageBucketException.kt | 3 +++ .../storage/options/StorageOptions.java | 13 +++++++++++++ 2 files changed, 16 insertions(+) diff --git a/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt index ab8d2d9cb..dee0d8307 100644 --- a/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt +++ b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt @@ -17,6 +17,9 @@ package com.amplifyframework.storage import com.amplifyframework.AmplifyException import com.amplifyframework.annotations.InternalAmplifyApi +/** + * Exception thrown when an invalid StorageBucket is specified. + */ class InvalidStorageBucketException @InternalAmplifyApi constructor( message: String = "Unable to find bucket from name in Amplify Outputs.", recoverySuggestion: String = "Ensure the bucket name used is available in Amplify Outputs." diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java index fd0c6b6c8..95f111b5c 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageOptions.java @@ -74,6 +74,10 @@ public final String getTargetIdentityId() { return targetIdentityId; } + /** + * Gets the storage bucket. + * @return storage bucket + */ @Nullable public final StorageBucket getBucket() { return bucket; @@ -118,6 +122,11 @@ public final B targetIdentityId(@Nullable String targetIdentityId) { return (B) this; } + /** + * Configure the storage bucket that will be used on newly built StorageOptions. + * @param bucket Storage bucket for new StorageOptions instances + * @return Current Builder instance, for fluent method chaining + */ public final B bucket(StorageBucket bucket) { this.bucket = bucket; return (B) this; @@ -136,6 +145,10 @@ public final String getTargetIdentityId() { return targetIdentityId; } + /** + * Gets the storage bucket. + * @return storage bucket + */ @Nullable public final StorageBucket getBucket() { return bucket; From 5fb67241311475892d224a2c9ff5187d7f89bc11 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 13:13:28 -0500 Subject: [PATCH 03/14] update storage options --- .../AWSS3StorageGetPresignedUrlOptions.java | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java index 5c204235f..0fa721da6 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java @@ -59,11 +59,12 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull AWSS3StorageGetPresignedUrlOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .expires(options.getExpires()) - .setValidateObjectExistence(options.getValidateObjectExistence()) - .expires(options.getExpires()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .expires(options.getExpires()) + .setValidateObjectExistence(options.getValidateObjectExistence()) + .expires(options.getExpires()) + .bucket(options.getBucket()); } /** @@ -106,6 +107,7 @@ public boolean equals(Object obj) { return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getExpires(), that.getExpires()) && + ObjectsCompat.equals(getBucket(), that.getBucket()) && ObjectsCompat.equals(getValidateObjectExistence(), that.getValidateObjectExistence()); } } @@ -117,7 +119,8 @@ public int hashCode() { getAccessLevel(), getTargetIdentityId(), getExpires(), - getValidateObjectExistence() + getValidateObjectExistence(), + getBucket() ); } @@ -130,6 +133,7 @@ public String toString() { ", targetIdentityId=" + getTargetIdentityId() + ", expires=" + getExpires() + ", validateObjectExistence=" + getValidateObjectExistence() + + ", bucket=" + getBucket() + '}'; } From 2f914fa2777ffcc45e3f84ae59ebb7b3762a2538 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 14:16:59 -0500 Subject: [PATCH 04/14] resolve review comments --- .../storage/s3/AWSS3StoragePlugin.java | 168 +++++++++--------- core/api/core.api | 21 +-- .../storage/InvalidStorageBucketException.kt | 3 +- .../amplifyframework/storage/StorageBucket.kt | 8 +- .../configuration/AmplifyOutputsDataTest.kt | 17 +- 5 files changed, 101 insertions(+), 116 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index d7b7398ec..85daefdd4 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -498,14 +498,14 @@ public StorageDownloadFileOperation downloadFile( ); AWSS3StorageDownloadFileOperation operation = new AWSS3StorageDownloadFileOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onProgress, + onSuccess, + onError ); operation.start(); @@ -533,13 +533,13 @@ public StorageDownloadFileOperation downloadFile( ); AWSS3StoragePathDownloadFileOperation operation = new AWSS3StoragePathDownloadFileOperation( - request, - defaultStorageService, - executorService, - authCredentialsProvider, - onProgress, - onSuccess, - onError + request, + defaultStorageService, + executorService, + authCredentialsProvider, + onProgress, + onSuccess, + onError ); operation.start(); @@ -625,14 +625,14 @@ public StorageUploadFileOperation uploadFile( ); AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onProgress, + onSuccess, + onError ); operation.start(); @@ -663,13 +663,13 @@ public StorageUploadFileOperation uploadFile( ); AWSS3StoragePathUploadFileOperation operation = new AWSS3StoragePathUploadFileOperation( - request, - defaultStorageService, - executorService, - authCredentialsProvider, - onProgress, - onSuccess, - onError + request, + defaultStorageService, + executorService, + authCredentialsProvider, + onProgress, + onSuccess, + onError ); operation.start(); @@ -753,14 +753,14 @@ public StorageUploadInputStreamOperation uploadInputStream( ); AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - request, - onProgress, - onSuccess, - onError + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + request, + onProgress, + onSuccess, + onError ); operation.start(); @@ -845,13 +845,13 @@ public StorageRemoveOperation remove( AWSS3StorageRemoveOperation operation = new AWSS3StorageRemoveOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onSuccess, + onError); operation.start(); @@ -870,12 +870,12 @@ public StorageRemoveOperation remove( AWSS3StoragePathRemoveOperation operation = new AWSS3StoragePathRemoveOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + onSuccess, + onError); operation.start(); @@ -895,36 +895,36 @@ public void getTransfer( TransferObserver transferObserver = new TransferObserver( transferRecord.getId(), - defaultStorageService.getTransferManager().getTransferStatusUpdater(), - transferRecord.getBucketName(), - transferRecord.getKey(), - transferRecord.getFile(), - null, - transferRecord.getState() != null ? transferRecord.getState() : TransferState.UNKNOWN); + defaultStorageService.getTransferManager().getTransferStatusUpdater(), + transferRecord.getBucketName(), + transferRecord.getKey(), + transferRecord.getFile(), + null, + transferRecord.getState() != null ? transferRecord.getState() : TransferState.UNKNOWN); TransferType transferType = transferRecord.getType(); switch (Objects.requireNonNull(transferType)) { case UPLOAD: if (transferRecord.getFile().startsWith(TransferStatusUpdater.TEMP_FILE_PREFIX)) { AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( - transferId, - defaultStorageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(operation); } else { AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( - transferId, - defaultStorageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(operation); } break; @@ -1001,13 +1001,13 @@ public StorageListOperation list(@NonNull String path, AWSS3StorageListOperation operation = new AWSS3StorageListOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - awsS3StoragePluginConfiguration, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + awsS3StoragePluginConfiguration, + onSuccess, + onError); operation.start(); @@ -1030,12 +1030,12 @@ public StorageListOperation list( AWSS3StoragePathListOperation operation = new AWSS3StoragePathListOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + onSuccess, + onError); operation.start(); diff --git a/core/api/core.api b/core/api/core.api index 6df942a7d..d1b72287f 100644 --- a/core/api/core.api +++ b/core/api/core.api @@ -3952,6 +3952,7 @@ public final class com/amplifyframework/storage/IdentityIdProvidedStoragePath : } public final class com/amplifyframework/storage/InvalidStorageBucketException : com/amplifyframework/AmplifyException { + public fun ()V } public final class com/amplifyframework/storage/ObjectMetadata { @@ -4007,26 +4008,6 @@ public final class com/amplifyframework/storage/ObjectMetadata { public final class com/amplifyframework/storage/ObjectMetadata$Companion { } -public final class com/amplifyframework/storage/OutputsStorageBucket : com/amplifyframework/storage/StorageBucket { - public final fun component1 ()Ljava/lang/String; - public final fun copy (Ljava/lang/String;)Lcom/amplifyframework/storage/OutputsStorageBucket; - public static synthetic fun copy$default (Lcom/amplifyframework/storage/OutputsStorageBucket;Ljava/lang/String;ILjava/lang/Object;)Lcom/amplifyframework/storage/OutputsStorageBucket; - public fun equals (Ljava/lang/Object;)Z - public final fun getName ()Ljava/lang/String; - public fun hashCode ()I - public fun toString ()Ljava/lang/String; -} - -public final class com/amplifyframework/storage/ResolvedStorageBucket : com/amplifyframework/storage/StorageBucket { - public final fun component1 ()Lcom/amplifyframework/storage/BucketInfo; - public final fun copy (Lcom/amplifyframework/storage/BucketInfo;)Lcom/amplifyframework/storage/ResolvedStorageBucket; - public static synthetic fun copy$default (Lcom/amplifyframework/storage/ResolvedStorageBucket;Lcom/amplifyframework/storage/BucketInfo;ILjava/lang/Object;)Lcom/amplifyframework/storage/ResolvedStorageBucket; - public fun equals (Ljava/lang/Object;)Z - public final fun getBucketInfo ()Lcom/amplifyframework/storage/BucketInfo; - public fun hashCode ()I - public fun toString ()Ljava/lang/String; -} - public final class com/amplifyframework/storage/StorageAccessLevel : java/lang/Enum { public static final field PRIVATE Lcom/amplifyframework/storage/StorageAccessLevel; public static final field PROTECTED Lcom/amplifyframework/storage/StorageAccessLevel; diff --git a/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt index dee0d8307..4117be8b9 100644 --- a/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt +++ b/core/src/main/java/com/amplifyframework/storage/InvalidStorageBucketException.kt @@ -15,12 +15,11 @@ package com.amplifyframework.storage import com.amplifyframework.AmplifyException -import com.amplifyframework.annotations.InternalAmplifyApi /** * Exception thrown when an invalid StorageBucket is specified. */ -class InvalidStorageBucketException @InternalAmplifyApi constructor( +class InvalidStorageBucketException internal constructor( message: String = "Unable to find bucket from name in Amplify Outputs.", recoverySuggestion: String = "Ensure the bucket name used is available in Amplify Outputs." ) : AmplifyException(message, recoverySuggestion) diff --git a/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt b/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt index e56946252..60f6e5c30 100644 --- a/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt +++ b/core/src/main/java/com/amplifyframework/storage/StorageBucket.kt @@ -14,6 +14,8 @@ */ package com.amplifyframework.storage +import com.amplifyframework.annotations.InternalAmplifyApi + abstract class StorageBucket { companion object { @JvmStatic @@ -23,10 +25,8 @@ abstract class StorageBucket { } } -// While class may be technically public, the constructor is internal, -// Customer is never expected to see or attempt to use this extended class +@InternalAmplifyApi data class OutputsStorageBucket internal constructor(val name: String) : StorageBucket() -// While class may be technically public, the constructor is internal, -// Customer is never expected to see or attempt to use this extended class +@InternalAmplifyApi data class ResolvedStorageBucket internal constructor(val bucketInfo: BucketInfo) : StorageBucket() diff --git a/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt b/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt index 1242533f6..8bbfc8228 100644 --- a/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt +++ b/core/src/test/java/com/amplifyframework/core/configuration/AmplifyOutputsDataTest.kt @@ -285,12 +285,17 @@ class AmplifyOutputsDataTest { awsRegion shouldBe "us-east-1" bucketName shouldBe "myBucket" buckets.size shouldBe 2 - buckets[0].name shouldBe "name1" - buckets[0].awsRegion shouldBe "us-east-1" - buckets[0].bucketName shouldBe "myBucket" - buckets[1].name shouldBe "name2" - buckets[1].awsRegion shouldBe "us-east-2" - buckets[1].bucketName shouldBe "myBucket2" + buckets[0].apply { + name shouldBe "name1" + awsRegion shouldBe "us-east-1" + bucketName shouldBe "myBucket" + } + + buckets[1].apply { + name shouldBe "name2" + awsRegion shouldBe "us-east-2" + bucketName shouldBe "myBucket2" + } } } From 97e8d514f66ba6471a09b5239f891faa0af27723 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 14:21:54 -0500 Subject: [PATCH 05/14] undo formatting --- .../storage/s3/AWSS3StoragePlugin.java | 68 +++++++++---------- .../AWSS3StorageGetPresignedUrlOptions.java | 12 ++-- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 85daefdd4..2a1d8bccc 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -533,13 +533,13 @@ public StorageDownloadFileOperation downloadFile( ); AWSS3StoragePathDownloadFileOperation operation = new AWSS3StoragePathDownloadFileOperation( - request, - defaultStorageService, - executorService, - authCredentialsProvider, - onProgress, - onSuccess, - onError + request, + defaultStorageService, + executorService, + authCredentialsProvider, + onProgress, + onSuccess, + onError ); operation.start(); @@ -663,13 +663,13 @@ public StorageUploadFileOperation uploadFile( ); AWSS3StoragePathUploadFileOperation operation = new AWSS3StoragePathUploadFileOperation( - request, - defaultStorageService, - executorService, - authCredentialsProvider, - onProgress, - onSuccess, - onError + request, + defaultStorageService, + executorService, + authCredentialsProvider, + onProgress, + onSuccess, + onError ); operation.start(); @@ -870,12 +870,12 @@ public StorageRemoveOperation remove( AWSS3StoragePathRemoveOperation operation = new AWSS3StoragePathRemoveOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + onSuccess, + onError); operation.start(); @@ -931,14 +931,14 @@ public void getTransfer( case DOWNLOAD: AWSS3StorageDownloadFileOperation downloadFileOperation = new AWSS3StorageDownloadFileOperation( - transferId, - new File(transferRecord.getFile()), - defaultStorageService, - executorService, - authCredentialsProvider, - awsS3StoragePluginConfiguration, - null, - transferObserver); + transferId, + new File(transferRecord.getFile()), + defaultStorageService, + executorService, + authCredentialsProvider, + awsS3StoragePluginConfiguration, + null, + transferObserver); onReceived.accept(downloadFileOperation); break; default: @@ -1030,12 +1030,12 @@ public StorageListOperation list( AWSS3StoragePathListOperation operation = new AWSS3StoragePathListOperation( - defaultStorageService, - executorService, - authCredentialsProvider, - request, - onSuccess, - onError); + defaultStorageService, + executorService, + authCredentialsProvider, + request, + onSuccess, + onError); operation.start(); diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java index 0fa721da6..754c52874 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java @@ -59,12 +59,12 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull AWSS3StorageGetPresignedUrlOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .expires(options.getExpires()) - .setValidateObjectExistence(options.getValidateObjectExistence()) - .expires(options.getExpires()) - .bucket(options.getBucket()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .expires(options.getExpires()) + .setValidateObjectExistence(options.getValidateObjectExistence()) + .expires(options.getExpires()) + .bucket(options.getBucket()); } /** From c38919b3ee2c9cf214ac603b44a292dce1487492 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 14:35:14 -0500 Subject: [PATCH 06/14] refactor configure method in AWSS3StoragePlugin --- .../storage/s3/AWSS3StoragePlugin.java | 26 ++++++++++++------- 1 file changed, 17 insertions(+), 9 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 2a1d8bccc..26403ccb7 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -29,6 +29,7 @@ import com.amplifyframework.core.Consumer; import com.amplifyframework.core.NoOpConsumer; import com.amplifyframework.core.configuration.AmplifyOutputsData; +import com.amplifyframework.storage.BucketInfo; import com.amplifyframework.storage.InvalidStorageBucketException; import com.amplifyframework.storage.OutputsStorageBucket; import com.amplifyframework.storage.ResolvedStorageBucket; @@ -134,7 +135,7 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private StorageAccessLevel defaultAccessLevel; private int defaultUrlExpiration; - private Map awsS3StorageServicesByBucketName; + private Map awsS3StorageServicesByBucketName = new HashMap<>(); private Context context; @SuppressLint("UnsafeOptInUsageError") private List configuredBuckets; @@ -207,6 +208,7 @@ public String getPluginKey() { return AWS_S3_STORAGE_PLUGIN_KEY; } + @SuppressLint("UnsafeOptInUsageError") @Override @SuppressWarnings("deprecation") public void configure( @@ -250,7 +252,8 @@ public void configure( ); } - configure(context, region, bucket); + BucketInfo bucketInfo = new BucketInfo(bucket, region); + configure(context, region, (ResolvedStorageBucket) StorageBucket.fromBucketInfo(bucketInfo)); } @Override @@ -266,20 +269,25 @@ public void configure(@NonNull AmplifyOutputsData configuration, @NonNull Contex } this.configuredBuckets = storage.getBuckets(); - configure(context, storage.getAwsRegion(), storage.getBucketName()); + BucketInfo bucketInfo = new BucketInfo(storage.getBucketName(), storage.getAwsRegion()); + configure(context, storage.getAwsRegion(), (ResolvedStorageBucket) StorageBucket.fromBucketInfo(bucketInfo)); } @SuppressWarnings("deprecation") + @SuppressLint("UnsafeOptInUsageError") private void configure( - @NonNull Context context, - @NonNull String region, - @NonNull String bucket + @NonNull Context context, + @NonNull String region, + @NonNull ResolvedStorageBucket bucket ) throws StorageException { try { this.context = context; - this.defaultStorageService = (AWSS3StorageService) storageServiceFactory.create(context, region, bucket); - this.awsS3StorageServicesByBucketName = new HashMap(); - this.awsS3StorageServicesByBucketName.put(bucket, this.defaultStorageService); + this.defaultStorageService = (AWSS3StorageService) storageServiceFactory.create( + context, + region, + bucket.getBucketInfo().getName()); + this.awsS3StorageServicesByBucketName.clear(); + this.awsS3StorageServicesByBucketName.put(bucket.getBucketInfo().getName(), this.defaultStorageService); } catch (RuntimeException exception) { throw new StorageException( "Failed to create storage service.", From 93ab0fd92f0f23c9e5aed168fc8d9003a7cabf44 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 14 Aug 2024 16:43:54 -0500 Subject: [PATCH 07/14] feat(storage): update storage API options to include bucket --- .../storage/s3/AWSS3StoragePlugin.java | 97 ++++++++++++++----- .../AWSS3StorageDownloadFileOptions.java | 14 ++- .../s3/options/AWSS3StorageListOptions.java | 12 ++- .../s3/options/AWSS3StorageRemoveOptions.java | 12 ++- .../AWSS3StorageUploadFileOptions.java | 18 ++-- .../AWSS3StorageUploadInputStreamOptions.java | 10 +- .../storage/s3/AWSS3StoragePluginTest.kt | 5 +- .../options/StorageDownloadFileOptions.java | 20 ++-- .../storage/options/StorageListOptions.java | 14 ++- .../options/StoragePagedListOptions.java | 2 +- .../storage/options/StorageRemoveOptions.java | 14 ++- .../options/StorageUploadFileOptions.java | 16 +-- .../StorageUploadInputStreamOptions.java | 16 +-- .../storage/options/StorageUploadOptions.java | 9 +- 14 files changed, 175 insertions(+), 84 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 26403ccb7..01fde9ba4 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -362,13 +362,8 @@ public StorageGetUrlOperation getUrl( AWSS3StorageService storageService = defaultStorageService; try { storageService = getStorageService(options.getBucket()); - } catch (InvalidStorageBucketException exception) { - onError.accept( - new StorageException( - "Unable to find bucket from name in Amplify Outputs.", - exception, - "Ensure the bucket name used is available in Amplify Outputs.") - ); + } catch (StorageException exception) { + onError.accept(exception); } AWSS3StorageGetPresignedUrlOperation operation = @@ -409,13 +404,8 @@ public StorageGetUrlOperation getUrl( AWSS3StorageService storageService = defaultStorageService; try { storageService = getStorageService(options.getBucket()); - } catch (InvalidStorageBucketException exception) { - onError.accept( - new StorageException( - "Unable to find bucket from name in Amplify Outputs.", - exception, - "Ensure the bucket name used is available in Amplify Outputs.") - ); + } catch (StorageException exception) { + onError.accept(exception); } AWSS3StoragePathGetPresignedUrlOperation operation = @@ -505,8 +495,15 @@ public StorageDownloadFileOperation downloadFile( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StorageDownloadFileOperation operation = new AWSS3StorageDownloadFileOperation( - defaultStorageService, + storageService, executorService, authCredentialsProvider, request, @@ -540,9 +537,16 @@ public StorageDownloadFileOperation downloadFile( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StoragePathDownloadFileOperation operation = new AWSS3StoragePathDownloadFileOperation( request, - defaultStorageService, + storageService, executorService, authCredentialsProvider, onProgress, @@ -632,8 +636,15 @@ public StorageUploadFileOperation uploadFile( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( - defaultStorageService, + storageService, executorService, authCredentialsProvider, request, @@ -670,9 +681,16 @@ public StorageUploadFileOperation uploadFile( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StoragePathUploadFileOperation operation = new AWSS3StoragePathUploadFileOperation( request, - defaultStorageService, + storageService, executorService, authCredentialsProvider, onProgress, @@ -760,8 +778,15 @@ public StorageUploadInputStreamOperation uploadInputStream( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( - defaultStorageService, + storageService, executorService, authCredentialsProvider, awsS3StoragePluginConfiguration, @@ -798,10 +823,17 @@ public StorageUploadInputStreamOperation uploadInputStream( useAccelerateEndpoint ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StoragePathUploadInputStreamOperation operation = new AWSS3StoragePathUploadInputStreamOperation( request, - defaultStorageService, + storageService, executorService, authCredentialsProvider, onProgress, @@ -851,9 +883,16 @@ public StorageRemoveOperation remove( options.getTargetIdentityId() ); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StorageRemoveOperation operation = new AWSS3StorageRemoveOperation( - defaultStorageService, + storageService, executorService, authCredentialsProvider, request, @@ -876,9 +915,16 @@ public StorageRemoveOperation remove( ) { AWSS3StoragePathRemoveRequest request = new AWSS3StoragePathRemoveRequest(path); + AWSS3StorageService storageService = defaultStorageService; + try { + storageService = getStorageService(options.getBucket()); + } catch (StorageException exception) { + onError.accept(exception); + } + AWSS3StoragePathRemoveOperation operation = new AWSS3StoragePathRemoveOperation( - defaultStorageService, + storageService, executorService, authCredentialsProvider, request, @@ -1053,7 +1099,7 @@ public StorageListOperation list( @SuppressLint("UnsafeOptInUsageError") @VisibleForTesting @NonNull - AWSS3StorageService getStorageService(@Nullable StorageBucket bucket) throws InvalidStorageBucketException { + AWSS3StorageService getStorageService(@Nullable StorageBucket bucket) throws StorageException { if (bucket == null) { return defaultStorageService; } @@ -1061,7 +1107,10 @@ AWSS3StorageService getStorageService(@Nullable StorageBucket bucket) throws Inv if (bucket instanceof OutputsStorageBucket) { AWSS3StorageService service = getAWSS3StorageService((OutputsStorageBucket) bucket); if (service == null) { - throw new InvalidStorageBucketException(); + throw new StorageException( + "Unable to find bucket from name in Amplify Outputs.", + new InvalidStorageBucketException(), + "Ensure the bucket name used is available in Amplify Outputs."); } else { return service; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageDownloadFileOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageDownloadFileOptions.java index e9d7f024e..416df3001 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageDownloadFileOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageDownloadFileOptions.java @@ -57,9 +57,10 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final AWSS3StorageDownloadFileOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .setUseAccelerateEndpoint(options.useAccelerateEndpoint()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .setUseAccelerateEndpoint(options.useAccelerateEndpoint()) + .bucket(options.getBucket()); } /** @@ -90,7 +91,8 @@ public boolean equals(Object obj) { } else { AWSS3StorageDownloadFileOptions that = (AWSS3StorageDownloadFileOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -99,7 +101,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -111,6 +114,7 @@ public String toString() { "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + ", useAccelerationMode=" + useAccelerateEndpoint() + + ", bucket=" + getBucket() + '}'; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageListOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageListOptions.java index d6335b85d..b314ba292 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageListOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageListOptions.java @@ -55,8 +55,9 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final AWSS3StorageListOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()); } /** @@ -78,7 +79,8 @@ public boolean equals(Object obj) { } else { AWSS3StorageListOptions that = (AWSS3StorageListOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -87,7 +89,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -98,6 +101,7 @@ public String toString() { return "AWSS3StorageListOptions {" + "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + '}'; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageRemoveOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageRemoveOptions.java index 244abe1f8..b74f08b75 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageRemoveOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageRemoveOptions.java @@ -56,8 +56,9 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final AWSS3StorageRemoveOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()); } /** @@ -79,7 +80,8 @@ public boolean equals(Object obj) { } else { AWSS3StorageRemoveOptions that = (AWSS3StorageRemoveOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -88,7 +90,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -99,6 +102,7 @@ public String toString() { return "AWSS3StorageRemoveOptions {" + "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + '}'; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadFileOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadFileOptions.java index 39dfdcaf4..00ce8ff61 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadFileOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadFileOptions.java @@ -71,11 +71,12 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final AWSS3StorageUploadFileOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .contentType(options.getContentType()) - .serverSideEncryption(options.getServerSideEncryption()) - .metadata(options.getMetadata()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .contentType(options.getContentType()) + .serverSideEncryption(options.getServerSideEncryption()) + .metadata(options.getMetadata()) + .bucket(options.getBucket()); } /** @@ -109,7 +110,8 @@ public boolean equals(Object obj) { ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getContentType(), that.getContentType()) && ObjectsCompat.equals(getServerSideEncryption(), that.getServerSideEncryption()) && - ObjectsCompat.equals(getMetadata(), that.getMetadata()); + ObjectsCompat.equals(getMetadata(), that.getMetadata()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -121,7 +123,8 @@ public int hashCode() { getTargetIdentityId(), getContentType(), getServerSideEncryption(), - getMetadata() + getMetadata(), + getBucket() ); } @@ -135,6 +138,7 @@ public String toString() { ", contentType=" + getContentType() + ", serverSideEncryption=" + getServerSideEncryption().getName() + ", metadata=" + getMetadata() + + ", bucket=" + getBucket() + '}'; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadInputStreamOptions.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadInputStreamOptions.java index e14296a2c..3bf4f6a36 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadInputStreamOptions.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/options/AWSS3StorageUploadInputStreamOptions.java @@ -75,7 +75,8 @@ public static Builder from(@NonNull final AWSS3StorageUploadInputStreamOptions o .targetIdentityId(options.getTargetIdentityId()) .contentType(options.getContentType()) .serverSideEncryption(options.getServerSideEncryption()) - .metadata(options.getMetadata()); + .metadata(options.getMetadata()) + .bucket(options.getBucket()); } /** @@ -109,7 +110,8 @@ public boolean equals(Object obj) { ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getContentType(), that.getContentType()) && ObjectsCompat.equals(getServerSideEncryption(), that.getServerSideEncryption()) && - ObjectsCompat.equals(getMetadata(), that.getMetadata()); + ObjectsCompat.equals(getMetadata(), that.getMetadata()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -121,7 +123,8 @@ public int hashCode() { getTargetIdentityId(), getContentType(), getServerSideEncryption(), - getMetadata() + getMetadata(), + getBucket() ); } @@ -135,6 +138,7 @@ public String toString() { ", contentType=" + getContentType() + ", serverSideEncryption=" + getServerSideEncryption().getName() + ", metadata=" + getMetadata() + + ", bucket=" + getBucket() + '}'; } diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt index e5e13cfe4..5d15f2d3a 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt @@ -16,7 +16,6 @@ package com.amplifyframework.storage.s3 import com.amplifyframework.storage.BucketInfo -import com.amplifyframework.storage.InvalidStorageBucketException import com.amplifyframework.storage.StorageBucket import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.s3.service.AWSS3StorageService @@ -129,7 +128,7 @@ class AWSS3StoragePluginTest { } @Test - fun `getStorageService throws InvalidStorageBucketException`() { + fun `getStorageService throws StorageException`() { val data = amplifyOutputsData { storage { awsRegion = "test-region" @@ -144,7 +143,7 @@ class AWSS3StoragePluginTest { plugin.configure(data, mockk()) val bucket = StorageBucket.fromOutputs("myBucket") - shouldThrow { + shouldThrow { plugin.getStorageService(bucket) } } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageDownloadFileOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageDownloadFileOptions.java index 846991f96..18e8b22a4 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageDownloadFileOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageDownloadFileOptions.java @@ -30,7 +30,7 @@ public class StorageDownloadFileOptions extends StorageOptions { */ @SuppressWarnings("deprecation") protected StorageDownloadFileOptions(final Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); } /** @@ -59,8 +59,9 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final StorageDownloadFileOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()); } /** @@ -85,7 +86,8 @@ public boolean equals(Object obj) { } else { StorageDownloadFileOptions that = (StorageDownloadFileOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -97,7 +99,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -109,9 +112,10 @@ public int hashCode() { @SuppressWarnings("deprecation") public String toString() { return "StorageDownloadFileOptions {" + - "accessLevel=" + getAccessLevel() + - ", targetIdentityId=" + getTargetIdentityId() + - '}'; + "accessLevel=" + getAccessLevel() + + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + + '}'; } /** diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageListOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageListOptions.java index 966704e5c..ebd9ae733 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageListOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageListOptions.java @@ -31,7 +31,7 @@ public class StorageListOptions extends StorageOptions { */ @SuppressWarnings("deprecation") protected StorageListOptions(final Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); } /** @@ -58,8 +58,9 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final StorageListOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()); } /** @@ -85,7 +86,8 @@ public boolean equals(Object obj) { } else { StorageListOptions that = (StorageListOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -97,7 +99,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -111,6 +114,7 @@ public String toString() { return "StorageListOptions {" + "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + '}'; } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StoragePagedListOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StoragePagedListOptions.java index 515f92f25..560997816 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StoragePagedListOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StoragePagedListOptions.java @@ -33,7 +33,7 @@ public class StoragePagedListOptions extends StorageOptions { */ @SuppressWarnings("deprecation") protected StoragePagedListOptions(Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); pageSize = builder.pageSize; nextToken = builder.nextToken; subpathStrategy = builder.subpathStrategy; diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageRemoveOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageRemoveOptions.java index 24c3adc7a..f2f9ef171 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageRemoveOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageRemoveOptions.java @@ -31,7 +31,7 @@ public class StorageRemoveOptions extends StorageOptions { */ @SuppressWarnings("deprecation") protected StorageRemoveOptions(final Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); } /** @@ -60,8 +60,9 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final StorageRemoveOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .bucket(options.getBucket()); } /** @@ -86,7 +87,8 @@ public boolean equals(Object obj) { } else { StorageRemoveOptions that = (StorageRemoveOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && - ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()); + ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -98,7 +100,8 @@ public boolean equals(Object obj) { public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), - getTargetIdentityId() + getTargetIdentityId(), + getBucket() ); } @@ -112,6 +115,7 @@ public String toString() { return "StorageRemoveOptions {" + "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + + ", bucket=" + getBucket() + '}'; } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadFileOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadFileOptions.java index 18dbcca58..1a17ab3cc 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadFileOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadFileOptions.java @@ -58,10 +58,11 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final StorageUploadFileOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .contentType(options.getContentType()) - .metadata(options.getMetadata()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .contentType(options.getContentType()) + .metadata(options.getMetadata()) + .bucket(options.getBucket()); } /** @@ -88,7 +89,8 @@ public boolean equals(Object obj) { return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getContentType(), that.getContentType()) && - ObjectsCompat.equals(getMetadata(), that.getMetadata()); + ObjectsCompat.equals(getMetadata(), that.getMetadata()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -102,7 +104,8 @@ public int hashCode() { getAccessLevel(), getTargetIdentityId(), getContentType(), - getMetadata() + getMetadata(), + getBucket() ); } @@ -118,6 +121,7 @@ public String toString() { ", targetIdentityId=" + getTargetIdentityId() + ", contentType=" + getContentType() + ", metadata=" + getMetadata() + + ", bucket=" + getBucket() + '}'; } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadInputStreamOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadInputStreamOptions.java index ba4623497..9e5c2dac5 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadInputStreamOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadInputStreamOptions.java @@ -58,10 +58,11 @@ public static Builder builder() { @SuppressWarnings("deprecation") public static Builder from(@NonNull final StorageUploadInputStreamOptions options) { return builder() - .accessLevel(options.getAccessLevel()) - .targetIdentityId(options.getTargetIdentityId()) - .contentType(options.getContentType()) - .metadata(options.getMetadata()); + .accessLevel(options.getAccessLevel()) + .targetIdentityId(options.getTargetIdentityId()) + .contentType(options.getContentType()) + .metadata(options.getMetadata()) + .bucket(options.getBucket()); } /** @@ -88,7 +89,8 @@ public boolean equals(Object obj) { return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getContentType(), that.getContentType()) && - ObjectsCompat.equals(getMetadata(), that.getMetadata()); + ObjectsCompat.equals(getMetadata(), that.getMetadata()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -102,7 +104,8 @@ public int hashCode() { getAccessLevel(), getTargetIdentityId(), getContentType(), - getMetadata() + getMetadata(), + getBucket() ); } @@ -118,6 +121,7 @@ public String toString() { ", targetIdentityId=" + getTargetIdentityId() + ", contentType=" + getContentType() + ", metadata=" + getMetadata() + + ", bucket=" + getBucket() + '}'; } diff --git a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadOptions.java b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadOptions.java index 36dcf2a22..9a4144ac9 100644 --- a/core/src/main/java/com/amplifyframework/storage/options/StorageUploadOptions.java +++ b/core/src/main/java/com/amplifyframework/storage/options/StorageUploadOptions.java @@ -41,7 +41,7 @@ public abstract class StorageUploadOptions extends StorageOptions { @SuppressWarnings("deprecation") protected , O extends StorageUploadOptions> StorageUploadOptions(final Builder builder) { - super(builder.getAccessLevel(), builder.getTargetIdentityId()); + super(builder.getAccessLevel(), builder.getTargetIdentityId(), builder.getBucket()); this.contentType = builder.getContentType(); this.metadata = builder.getMetadata(); } @@ -79,7 +79,8 @@ public boolean equals(Object obj) { return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && ObjectsCompat.equals(getContentType(), that.getContentType()) && - ObjectsCompat.equals(getMetadata(), that.getMetadata()); + ObjectsCompat.equals(getMetadata(), that.getMetadata()) && + ObjectsCompat.equals(getBucket(), that.getBucket()); } } @@ -93,7 +94,8 @@ public int hashCode() { getAccessLevel(), getTargetIdentityId(), getContentType(), - getMetadata() + getMetadata(), + getBucket() ); } @@ -109,6 +111,7 @@ public String toString() { ", targetIdentityId=" + getTargetIdentityId() + ", contentType=" + getContentType() + ", metadata=" + getMetadata() + + ", bucket=" + getBucket() + '}'; } From 764b9a9619033e345e05b1b401c6be74cc103e52 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 20 Aug 2024 09:46:31 -0500 Subject: [PATCH 08/14] feat(storage): update multipart operations to support multiple buckets and regions --- .../storage/s3/AWSS3StoragePlugin.java | 76 ++++++++++++++----- .../storage/s3/TransferOperations.kt | 1 + .../storage/s3/service/AWSS3StorageService.kt | 38 +++++++++- .../S3StorageTransferClientProvider.kt | 27 +++++++ .../transfer/StorageTransferClientProvider.kt | 23 ++++++ .../storage/s3/transfer/TransferDB.kt | 17 ++++- .../storage/s3/transfer/TransferDBHelper.kt | 2 +- .../storage/s3/transfer/TransferManager.kt | 14 +++- .../storage/s3/transfer/TransferObserver.kt | 1 + .../storage/s3/transfer/TransferRecord.kt | 3 + .../storage/s3/transfer/TransferTable.kt | 13 ++++ .../storage/s3/transfer/UploadOptions.kt | 1 + .../worker/AbortMultiPartUploadWorker.kt | 4 +- .../worker/CompleteMultiPartUploadWorker.kt | 4 +- .../s3/transfer/worker/DownloadWorker.kt | 4 +- .../InitiateMultiPartUploadTransferWorker.kt | 4 +- .../worker/PartUploadTransferWorker.kt | 4 +- .../transfer/worker/SinglePartUploadWorker.kt | 4 +- .../transfer/worker/TransferWorkerFactory.kt | 15 ++-- .../storage/s3/AWSS3StoragePluginTest.kt | 7 +- .../storage/s3/StorageComponentTest.java | 5 +- .../worker/AbortMultiPartUploadWorkerTest.kt | 30 +++++++- .../s3/transfer/worker/DownloadWorkerTest.kt | 10 ++- ...itiateMultiPartUploadTransferWorkerTest.kt | 10 ++- 24 files changed, 263 insertions(+), 54 deletions(-) create mode 100644 aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt create mode 100644 aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 01fde9ba4..090e8f7d8 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -90,7 +90,8 @@ import com.amplifyframework.storage.s3.request.AWSS3StorageRemoveRequest; import com.amplifyframework.storage.s3.request.AWSS3StorageUploadRequest; import com.amplifyframework.storage.s3.service.AWSS3StorageService; -import com.amplifyframework.storage.s3.service.StorageService; +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider; +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider; import com.amplifyframework.storage.s3.transfer.TransferObserver; import com.amplifyframework.storage.s3.transfer.TransferRecord; import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater; @@ -126,20 +127,33 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private static final int DEFAULT_URL_EXPIRATION_DAYS = 7; - private final StorageService.Factory storageServiceFactory; + private final AWSS3StorageService.Factory storageServiceFactory; private final ExecutorService executorService; - private final AuthCredentialsProvider authCredentialsProvider; + private AuthCredentialsProvider authCredentialsProvider; private final AWSS3StoragePluginConfiguration awsS3StoragePluginConfiguration; private AWSS3StorageService defaultStorageService; @SuppressWarnings("deprecation") private StorageAccessLevel defaultAccessLevel; private int defaultUrlExpiration; - private Map awsS3StorageServicesByBucketName = new HashMap<>(); + private final Map awsS3StorageServicesByBucketName = new HashMap<>(); private Context context; @SuppressLint("UnsafeOptInUsageError") private List configuredBuckets; + @SuppressLint("UnsafeOptInUsageError") + private StorageTransferClientProvider clientProvider = new S3StorageTransferClientProvider((region, bucketName) -> { + if (region != null && bucketName != null) { + StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(region, bucketName)); + return getAWSS3StorageService((ResolvedStorageBucket) bucket).getClient(); + } + + if (region != null) { + return AWSS3StorageService.getS3Client(region, authCredentialsProvider); + } + return defaultStorageService.getClient(); + }); + /** * Constructs the AWS S3 Storage Plugin initializing the executor service. */ @@ -162,13 +176,14 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf @VisibleForTesting AWSS3StoragePlugin(AuthCredentialsProvider authCredentialsProvider) { - this((context, region, bucket) -> + this((context, region, bucket, clientProvider) -> new AWSS3StorageService( context, region, bucket, authCredentialsProvider, - AWS_S3_STORAGE_PLUGIN_KEY + AWS_S3_STORAGE_PLUGIN_KEY, + clientProvider ), authCredentialsProvider, new AWSS3StoragePluginConfiguration.Builder().build()); @@ -177,13 +192,14 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf @VisibleForTesting AWSS3StoragePlugin(AuthCredentialsProvider authCredentialsProvider, AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration) { - this((context, region, bucket) -> + this((context, region, bucket, clientProvider) -> new AWSS3StorageService( context, region, bucket, authCredentialsProvider, - AWS_S3_STORAGE_PLUGIN_KEY + AWS_S3_STORAGE_PLUGIN_KEY, + clientProvider ), authCredentialsProvider, awss3StoragePluginConfiguration); @@ -191,7 +207,7 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf @VisibleForTesting AWSS3StoragePlugin( - StorageService.Factory storageServiceFactory, + AWSS3StorageService.Factory storageServiceFactory, AuthCredentialsProvider authCredentialsProvider, AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration ) { @@ -282,10 +298,11 @@ private void configure( ) throws StorageException { try { this.context = context; - this.defaultStorageService = (AWSS3StorageService) storageServiceFactory.create( + this.defaultStorageService = storageServiceFactory.create( context, region, - bucket.getBucketInfo().getName()); + bucket.getBucketInfo().getName(), + clientProvider); this.awsS3StorageServicesByBucketName.clear(); this.awsS3StorageServicesByBucketName.put(bucket.getBucketInfo().getName(), this.defaultStorageService); } catch (RuntimeException exception) { @@ -935,7 +952,8 @@ public StorageRemoveOperation remove( return operation; } - + + @SuppressLint("UnsafeOptInUsageError") @Override @SuppressWarnings("deprecation") public void getTransfer( @@ -951,18 +969,23 @@ public void getTransfer( transferRecord.getId(), defaultStorageService.getTransferManager().getTransferStatusUpdater(), transferRecord.getBucketName(), + transferRecord.getRegion(), transferRecord.getKey(), transferRecord.getFile(), null, transferRecord.getState() != null ? transferRecord.getState() : TransferState.UNKNOWN); TransferType transferType = transferRecord.getType(); + + AWSS3StorageService storageService + = getAwss3StorageServiceFromTransferRecord(onError, transferRecord); + switch (Objects.requireNonNull(transferType)) { case UPLOAD: if (transferRecord.getFile().startsWith(TransferStatusUpdater.TEMP_FILE_PREFIX)) { AWSS3StorageUploadInputStreamOperation operation = new AWSS3StorageUploadInputStreamOperation( transferId, - defaultStorageService, + storageService, executorService, authCredentialsProvider, awsS3StoragePluginConfiguration, @@ -973,7 +996,7 @@ public void getTransfer( AWSS3StorageUploadFileOperation operation = new AWSS3StorageUploadFileOperation( transferId, - defaultStorageService, + storageService, executorService, authCredentialsProvider, awsS3StoragePluginConfiguration, @@ -987,7 +1010,7 @@ public void getTransfer( downloadFileOperation = new AWSS3StorageDownloadFileOperation( transferId, new File(transferRecord.getFile()), - defaultStorageService, + storageService, executorService, authCredentialsProvider, awsS3StoragePluginConfiguration, @@ -1009,6 +1032,25 @@ public void getTransfer( }); } + private AWSS3StorageService getAwss3StorageServiceFromTransferRecord( + @NonNull Consumer onError, + TransferRecord transferRecord + ) { + AWSS3StorageService storageService = defaultStorageService; + if (transferRecord.getRegion() != null && transferRecord.getBucketName() != null) { + try { + BucketInfo bucketInfo = new BucketInfo( + transferRecord.getBucketName(), + transferRecord.getRegion()); + StorageBucket bucket = StorageBucket.fromBucketInfo(bucketInfo); + storageService = getStorageService(bucket); + } catch (StorageException exception) { + onError.accept(exception); + } + } + return storageService; + } + @NonNull @SuppressWarnings("deprecation") @Override @@ -1133,7 +1175,7 @@ private AWSS3StorageService getAWSS3StorageService(OutputsStorageBucket outputsS AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); if (service == null) { String region = configuredBucket.getAwsRegion(); - service = (AWSS3StorageService) storageServiceFactory.create(context, region, bucketName); + service = storageServiceFactory.create(context, region, bucketName, clientProvider); awsS3StorageServicesByBucketName.put(bucketName, service); } @@ -1150,7 +1192,7 @@ private AWSS3StorageService getAWSS3StorageService(ResolvedStorageBucket resolve AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); if (service == null) { String region = resolvedStorageBucket.getBucketInfo().getRegion(); - service = (AWSS3StorageService) storageServiceFactory.create(context, region, bucketName); + service = storageServiceFactory.create(context, region, bucketName, clientProvider); awsS3StorageServicesByBucketName.put(bucketName, service); } return service; diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/TransferOperations.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/TransferOperations.kt index 47cd602c4..99d15ea9d 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/TransferOperations.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/TransferOperations.kt @@ -70,6 +70,7 @@ internal object TransferOperations { transferRecord.id, transferStatusUpdater, transferRecord.bucketName, + transferRecord.region, transferRecord.key, transferRecord.file, listener diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt index 7d9eef745..8d35bba5e 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt @@ -32,6 +32,7 @@ import com.amplifyframework.storage.StorageItem import com.amplifyframework.storage.options.SubpathStrategy import com.amplifyframework.storage.options.SubpathStrategy.Exclude import com.amplifyframework.storage.result.StorageListResult +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferManager import com.amplifyframework.storage.s3.transfer.TransferObserver import com.amplifyframework.storage.s3.transfer.TransferRecord @@ -46,7 +47,6 @@ import java.util.Date import kotlin.time.Duration.Companion.seconds import kotlin.time.ExperimentalTime import kotlinx.coroutines.runBlocking - /** * A representation of an S3 backend service endpoint. */ @@ -55,16 +55,27 @@ internal class AWSS3StorageService( private val awsRegion: String, private val s3BucketName: String, private val authCredentialsProvider: AuthCredentialsProvider, - private val awsS3StoragePluginKey: String + private val awsS3StoragePluginKey: String, + private val clientProvider: StorageTransferClientProvider ) : StorageService { + companion object { + @JvmStatic + fun getS3Client(region: String, authCredentialsProvider: AuthCredentialsProvider): S3Client { + return S3Client { + this.region = region + this.credentialsProvider = authCredentialsProvider + } + } + } + private var s3Client: S3Client = S3Client { region = awsRegion credentialsProvider = authCredentialsProvider } val transferManager: TransferManager = - TransferManager(context, s3Client, awsS3StoragePluginKey) + TransferManager(context, clientProvider, awsS3StoragePluginKey) /** * Generate pre-signed URL for an object. @@ -130,6 +141,7 @@ internal class AWSS3StorageService( return transferManager.download( transferId, s3BucketName, + awsRegion, serviceKey, file, useAccelerateEndpoint = useAccelerateEndpoint @@ -153,6 +165,7 @@ internal class AWSS3StorageService( return transferManager.upload( transferId, s3BucketName, + awsRegion, serviceKey, file, metadata, @@ -175,7 +188,7 @@ internal class AWSS3StorageService( metadata: ObjectMetadata, useAccelerateEndpoint: Boolean ): TransferObserver { - val uploadOptions = UploadOptions(s3BucketName, metadata) + val uploadOptions = UploadOptions(s3BucketName, awsRegion, metadata) return transferManager.upload(transferId, serviceKey, inputStream, uploadOptions, useAccelerateEndpoint) } @@ -420,4 +433,21 @@ internal class AWSS3StorageService( fun getClient(): S3Client { return s3Client } + + interface Factory { + /** + * Factory interface to instantiate [StorageService] object. + * + * @param context Android context + * @param region S3 bucket region + * @param bucketName Name of the bucket where the items are stored + * @return An instantiated storage service instance + */ + fun create( + context: Context, + region: String, + bucketName: String, + clientProvider: StorageTransferClientProvider + ): AWSS3StorageService + } } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt new file mode 100644 index 000000000..7f0e1ebe5 --- /dev/null +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt @@ -0,0 +1,27 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage.s3.transfer + +import aws.sdk.kotlin.services.s3.S3Client +import com.amplifyframework.auth.AuthCredentialsProvider +import com.amplifyframework.storage.StorageException + +internal class S3StorageTransferClientProvider( + private val createS3Client: (region: String?, bucketName: String?) -> S3Client +) : StorageTransferClientProvider { + override fun getStorageTransferClient(region: String?, bucketName: String?): S3Client { + return createS3Client(region, bucketName) + } +} diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt new file mode 100644 index 000000000..5b430e3fc --- /dev/null +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt @@ -0,0 +1,23 @@ +/* + * Copyright 2021 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage.s3.transfer + +import aws.sdk.kotlin.services.s3.S3Client +import com.amplifyframework.annotations.InternalApiWarning + +@InternalApiWarning +internal interface StorageTransferClientProvider { + fun getStorageTransferClient(region: String?, bucketName: String?): S3Client +} diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt index 6413cf7c4..91195026a 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt @@ -79,6 +79,7 @@ internal class TransferDB private constructor(context: Context) { fun insertMultipartUploadRecord( transferId: String, bucket: String, + region: String, key: String, file: File, fileOffset: Long, @@ -90,7 +91,7 @@ internal class TransferDB private constructor(context: Context) { ): Uri { val values: ContentValues = generateContentValuesForMultiPartUpload( transferId, - bucket, key, file, + bucket, region, key, file, fileOffset, partNumber, uploadId, bytesTotal, isLastPart, ObjectMetadata(), null, useAccelerateEndpoint @@ -114,6 +115,7 @@ internal class TransferDB private constructor(context: Context) { transferId: String, type: TransferType, bucket: String, + region: String, key: String, file: File?, cannedAcl: ObjectCannedAcl? = null, @@ -124,6 +126,7 @@ internal class TransferDB private constructor(context: Context) { transferId, type, bucket, + region, key, file, metadata, @@ -398,7 +401,7 @@ internal class TransferDB private constructor(context: Context) { */ fun getTransferRecordById(id: Int): TransferRecord? { var transferRecord: TransferRecord? = null - var c: Cursor? = null + var c: Cursor? try { c = queryTransferById(id) c?.use { @@ -415,7 +418,7 @@ internal class TransferDB private constructor(context: Context) { fun getTransferByTransferId(transferId: String): TransferRecord? { var transferRecord: TransferRecord? = null - var c: Cursor? = null + var c: Cursor? try { c = transferDBHelper.query(getTransferRecordIdUri(transferId)) c.use { @@ -560,6 +563,7 @@ internal class TransferDB private constructor(context: Context) { transferId: String, type: TransferType, bucket: String, + region: String, key: String, file: File, metadata: ObjectMetadata?, @@ -570,6 +574,7 @@ internal class TransferDB private constructor(context: Context) { transferId, type, bucket, + region, key, file, metadata, @@ -602,6 +607,7 @@ internal class TransferDB private constructor(context: Context) { fun generateContentValuesForMultiPartUpload( transferId: String, bucket: String?, + region: String?, key: String?, file: File, fileOffset: Long, @@ -611,13 +617,14 @@ internal class TransferDB private constructor(context: Context) { isLastPart: Int, metadata: ObjectMetadata?, cannedAcl: ObjectCannedAcl?, - useAccelerateEndpoint: Boolean + useAccelerateEndpoint: Boolean, ): ContentValues { val values = ContentValues() values.put(TransferTable.COLUMN_TRANSFER_ID, transferId) values.put(TransferTable.COLUMN_TYPE, TransferType.UPLOAD.toString()) values.put(TransferTable.COLUMN_STATE, TransferState.WAITING.toString()) values.put(TransferTable.COLUMN_BUCKET_NAME, bucket) + values.put(TransferTable.COLUMN_REGION, region) values.put(TransferTable.COLUMN_KEY, key) values.put(TransferTable.COLUMN_FILE, file.absolutePath) values.put(TransferTable.COLUMN_BYTES_CURRENT, 0L) @@ -723,6 +730,7 @@ internal class TransferDB private constructor(context: Context) { transferId: String, type: TransferType, bucket: String, + region: String, key: String, file: File?, metadata: ObjectMetadata?, @@ -734,6 +742,7 @@ internal class TransferDB private constructor(context: Context) { values.put(TransferTable.COLUMN_TYPE, type.toString()) values.put(TransferTable.COLUMN_STATE, TransferState.WAITING.toString()) values.put(TransferTable.COLUMN_BUCKET_NAME, bucket) + values.put(TransferTable.COLUMN_REGION, region) values.put(TransferTable.COLUMN_KEY, key) values.put(TransferTable.COLUMN_FILE, file?.absolutePath) values.put(TransferTable.COLUMN_BYTES_CURRENT, 0L) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt index 211ea745e..4403d51df 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDBHelper.kt @@ -48,7 +48,7 @@ internal class TransferDBHelper(private val context: Context) : SQLiteOpenHelper // This represents the latest database version. // Update this when the database is being upgraded. - private const val DATABASE_VERSION = 9 + private const val DATABASE_VERSION = 10 private const val BASE_PATH = "transfers" private const val TRANSFERS = 10 private const val TRANSFER_ID = 20 diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt index 8a4bd9cb3..119aeeafb 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferManager.kt @@ -45,7 +45,7 @@ import kotlin.math.min */ internal class TransferManager( context: Context, - s3: S3Client, + clientProvider: StorageTransferClientProvider, private val pluginKey: String, private val workManager: WorkManager = WorkManager.getInstance(context) ) { @@ -71,7 +71,7 @@ internal class TransferManager( init { RouterWorker.workerFactories[pluginKey] = TransferWorkerFactory( transferDB, - s3, + clientProvider, transferStatusUpdater ) } @@ -93,6 +93,7 @@ internal class TransferManager( fun upload( transferId: String, bucket: String, + region: String, key: String, file: File, metadata: ObjectMetadata, @@ -101,12 +102,13 @@ internal class TransferManager( useAccelerateEndpoint: Boolean = false ): TransferObserver { val transferRecordId = if (shouldUploadInMultipart(file)) { - createMultipartUploadRecords(transferId, bucket, key, file, metadata, cannedAcl, useAccelerateEndpoint) + createMultipartUploadRecords(transferId, bucket, region, key, file, metadata, cannedAcl, useAccelerateEndpoint) } else { val uri = transferDB.insertSingleTransferRecord( transferId, TransferType.UPLOAD, bucket, + region, key, file, cannedAcl, @@ -147,6 +149,7 @@ internal class TransferManager( return upload( transferId, options.bucket, + options.region, key, file, options.objectMetadata, @@ -160,6 +163,7 @@ internal class TransferManager( fun download( transferId: String, bucket: String, + region: String, key: String, file: File, listener: TransferListener? = null, @@ -172,6 +176,7 @@ internal class TransferManager( transferId, TransferType.DOWNLOAD, bucket, + region, key, file, useAccelerateEndpoint = useAccelerateEndpoint @@ -246,6 +251,7 @@ internal class TransferManager( private fun createMultipartUploadRecords( transferId: String, bucket: String, + region: String, key: String, file: File, metadata: ObjectMetadata, @@ -263,6 +269,7 @@ internal class TransferManager( contentValues[0] = transferDB.generateContentValuesForMultiPartUpload( transferId, bucket, + region, key, file, fileOffset, @@ -279,6 +286,7 @@ internal class TransferManager( contentValues[partNum] = transferDB.generateContentValuesForMultiPartUpload( UUID.randomUUID().toString(), bucket, + region, key, file, fileOffset, diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferObserver.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferObserver.kt index 021e06a5b..b6147f5cf 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferObserver.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferObserver.kt @@ -24,6 +24,7 @@ internal data class TransferObserver @JvmOverloads constructor( val id: Int, private val transferStatusUpdater: TransferStatusUpdater, val bucket: String? = null, + val region: String? = null, val key: String? = null, val filePath: String? = null, private val listener: TransferListener? = null, diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferRecord.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferRecord.kt index c32a85c7c..73d219b97 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferRecord.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferRecord.kt @@ -36,6 +36,7 @@ internal data class TransferRecord( var type: TransferType? = null, var state: TransferState? = null, var bucketName: String? = null, + var region: String? = null, var key: String? = null, var versionId: String? = null, var file: String = "", @@ -80,6 +81,8 @@ internal data class TransferRecord( ) this.bucketName = c.getString(c.getColumnIndexOrThrow(TransferTable.COLUMN_BUCKET_NAME)) + this.region = + c.getString(c.getColumnIndexOrThrow(TransferTable.COLUMN_REGION)) this.key = c.getString(c.getColumnIndexOrThrow(TransferTable.COLUMN_KEY)) this.versionId = c.getString(c.getColumnIndexOrThrow(TransferTable.COLUMN_VERSION_ID)) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt index 85b515062..c80a95320 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt @@ -134,6 +134,8 @@ internal class TransferTable { const val COLUMN_USE_ACCELERATE_ENDPOINT = "useAccelerateEndpoint" + const val COLUMN_REGION = "region" + private const val TABLE_VERSION_2 = 2 private const val TABLE_VERSION_3 = 3 private const val TABLE_VERSION_4 = 4 @@ -142,6 +144,7 @@ internal class TransferTable { private const val TABLE_VERSION_7 = 7 private const val TABLE_VERSION_8 = 8 private const val TABLE_VERSION_9 = 9 + private const val TABLE_VERSION_10 = 10 // Database creation SQL statement const val DATABASE_CREATE = "create table $TABLE_TRANSFER (" + @@ -150,6 +153,7 @@ internal class TransferTable { "$COLUMN_TYPE text not null, " + "$COLUMN_STATE text not null, " + "$COLUMN_BUCKET_NAME text not null, " + + "$COLUMN_REGION text, " + "$COLUMN_KEY text not null," + "$COLUMN_VERSION_ID text, " + "$COLUMN_BYTES_TOTAL bigint, " + @@ -219,6 +223,9 @@ internal class TransferTable { if (TABLE_VERSION_9 in (oldVersion + 1)..newVersion) { addVersion9Columns(database) } + if (TABLE_VERSION_10 in (oldVersion + 1)..newVersion) { + addVersion10Columns(database) + } database.setTransactionSuccessful() database.endTransaction() } @@ -296,5 +303,11 @@ internal class TransferTable { "DEFAULT 0;" database.execSQL(addConnectionType) } + + private fun addVersion10Columns(database: SQLiteDatabase) { + val addRegion = "ALTER TABLE $TABLE_TRANSFER ADD COLUMN $COLUMN_REGION text " + + "DEFAULT null;" + database.execSQL(addRegion) + } } } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/UploadOptions.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/UploadOptions.kt index f68453c75..635bc7eb7 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/UploadOptions.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/UploadOptions.kt @@ -24,6 +24,7 @@ import com.amplifyframework.storage.ObjectMetadata internal data class UploadOptions @JvmOverloads constructor( val bucket: String, + val region: String, val objectMetadata: ObjectMetadata = ObjectMetadata(), val cannedAcl: ObjectCannedAcl? = null, val transferListener: TransferListener? = null diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorker.kt index 30aa2d20b..3f9cb5efb 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorker.kt @@ -20,6 +20,7 @@ import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.abortMultipartUpload import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.storage.TransferState +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -27,7 +28,7 @@ import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater * Worker to abort pending multipart upload **/ internal class AbortMultiPartUploadWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -35,6 +36,7 @@ internal class AbortMultiPartUploadWorker( ) : BaseTransferWorker(transferStatusUpdater, transferDB, context, workerParameters) { override suspend fun performWork(): Result { + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) return s3.withConfig { enableAccelerate = transferRecord.useAccelerateEndpoint == 1 }.abortMultipartUpload { diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/CompleteMultiPartUploadWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/CompleteMultiPartUploadWorker.kt index 8b48014a8..d7cadd5dd 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/CompleteMultiPartUploadWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/CompleteMultiPartUploadWorker.kt @@ -20,6 +20,7 @@ import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.completeMultipartUpload import aws.sdk.kotlin.services.s3.model.CompletedMultipartUpload import aws.sdk.kotlin.services.s3.withConfig +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -27,7 +28,7 @@ import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater * Worker to complete multipart upload **/ internal class CompleteMultiPartUploadWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -36,6 +37,7 @@ internal class CompleteMultiPartUploadWorker( override suspend fun performWork(): Result { val completedParts = transferDB.queryPartETagsOfUpload(transferRecord.id) + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) return s3.withConfig { enableAccelerate = transferRecord.useAccelerateEndpoint == 1 }.completeMultipartUpload { diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorker.kt index 87a9db561..9af6f8d70 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorker.kt @@ -26,6 +26,7 @@ import aws.smithy.kotlin.runtime.io.SdkSource import aws.smithy.kotlin.runtime.io.buffer import com.amplifyframework.storage.s3.transfer.DownloadProgressListener import com.amplifyframework.storage.s3.transfer.DownloadProgressListenerInterceptor +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater import java.io.BufferedOutputStream @@ -40,7 +41,7 @@ import kotlinx.coroutines.withContext * Worker to perform download file task. */ internal class DownloadWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -50,6 +51,7 @@ internal class DownloadWorker( private lateinit var downloadProgressListener: DownloadProgressListener private val defaultBufferSize = 8192L override suspend fun performWork(): Result { + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) s3.withConfig { enableAccelerate = transferRecord.useAccelerateEndpoint == 1 } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorker.kt index ec7be7cb3..b3c013b4b 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorker.kt @@ -21,6 +21,7 @@ import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.createMultipartUpload import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.storage.TransferState +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -28,7 +29,7 @@ import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater * Worker to initiate multipart upload **/ internal class InitiateMultiPartUploadTransferWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -36,6 +37,7 @@ internal class InitiateMultiPartUploadTransferWorker( ) : BaseTransferWorker(transferStatusUpdater, transferDB, context, workerParameters) { override suspend fun performWork(): Result { + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) transferStatusUpdater.updateTransferState(transferRecord.id, TransferState.IN_PROGRESS) val putObjectRequest = createPutObjectRequest(transferRecord, null) return s3.withConfig { diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/PartUploadTransferWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/PartUploadTransferWorker.kt index d145786b6..b7a0f6760 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/PartUploadTransferWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/PartUploadTransferWorker.kt @@ -22,6 +22,7 @@ import aws.sdk.kotlin.services.s3.withConfig import aws.smithy.kotlin.runtime.content.asByteStream import com.amplifyframework.storage.TransferState import com.amplifyframework.storage.s3.transfer.PartUploadProgressListener +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater import com.amplifyframework.storage.s3.transfer.UploadProgressListenerInterceptor @@ -33,7 +34,7 @@ import kotlinx.coroutines.isActive * Worker to upload a part for multipart upload **/ internal class PartUploadTransferWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -51,6 +52,7 @@ internal class PartUploadTransferWorker( transferStatusUpdater.updateTransferState(transferRecord.mainUploadId, TransferState.IN_PROGRESS) multiPartUploadId = inputData.keyValueMap[MULTI_PART_UPLOAD_ID] as String partUploadProgressListener = PartUploadProgressListener(transferRecord, transferStatusUpdater) + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) return s3.withConfig { interceptors += UploadProgressListenerInterceptor(partUploadProgressListener) enableAccelerate = transferRecord.useAccelerateEndpoint == 1 diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/SinglePartUploadWorker.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/SinglePartUploadWorker.kt index 21de0db80..515c36bef 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/SinglePartUploadWorker.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/SinglePartUploadWorker.kt @@ -22,13 +22,14 @@ import android.content.Context import androidx.work.WorkerParameters import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.withConfig +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater import com.amplifyframework.storage.s3.transfer.UploadProgressListener import com.amplifyframework.storage.s3.transfer.UploadProgressListenerInterceptor internal class SinglePartUploadWorker( - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferDB: TransferDB, private val transferStatusUpdater: TransferStatusUpdater, context: Context, @@ -40,6 +41,7 @@ internal class SinglePartUploadWorker( override suspend fun performWork(): Result { uploadProgressListener = UploadProgressListener(transferRecord, transferStatusUpdater) val putObjectRequest = createPutObjectRequest(transferRecord, uploadProgressListener) + val s3: S3Client = clientProvider.getStorageTransferClient(transferRecord.region, transferRecord.bucketName) return s3.withConfig { interceptors += UploadProgressListenerInterceptor(uploadProgressListener) enableAccelerate = transferRecord.useAccelerateEndpoint == 1 diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/TransferWorkerFactory.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/TransferWorkerFactory.kt index 88c0dc19f..f491d1ee9 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/TransferWorkerFactory.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/worker/TransferWorkerFactory.kt @@ -18,6 +18,7 @@ import android.content.Context import androidx.work.WorkerFactory import androidx.work.WorkerParameters import aws.sdk.kotlin.services.s3.S3Client +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -26,7 +27,7 @@ import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater **/ internal class TransferWorkerFactory( private val transferDB: TransferDB, - private val s3: S3Client, + private val clientProvider: StorageTransferClientProvider, private val transferStatusUpdater: TransferStatusUpdater ) : WorkerFactory() { override fun createWorker( @@ -37,7 +38,7 @@ internal class TransferWorkerFactory( when (workerClassName) { DownloadWorker::class.java.name -> return DownloadWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, @@ -45,7 +46,7 @@ internal class TransferWorkerFactory( ) SinglePartUploadWorker::class.java.name -> return SinglePartUploadWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, @@ -53,7 +54,7 @@ internal class TransferWorkerFactory( ) InitiateMultiPartUploadTransferWorker::class.java.name -> return InitiateMultiPartUploadTransferWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, @@ -61,7 +62,7 @@ internal class TransferWorkerFactory( ) PartUploadTransferWorker::class.java.name -> return PartUploadTransferWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, @@ -69,7 +70,7 @@ internal class TransferWorkerFactory( ) CompleteMultiPartUploadWorker::class.java.name -> return CompleteMultiPartUploadWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, @@ -77,7 +78,7 @@ internal class TransferWorkerFactory( ) AbortMultiPartUploadWorker::class.java.name -> return AbortMultiPartUploadWorker( - s3, + clientProvider, transferDB, transferStatusUpdater, appContext, diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt index 5d15f2d3a..da4d54b36 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt @@ -19,7 +19,6 @@ import com.amplifyframework.storage.BucketInfo import com.amplifyframework.storage.StorageBucket import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.s3.service.AWSS3StorageService -import com.amplifyframework.storage.s3.service.StorageService import com.amplifyframework.testutils.configuration.amplifyOutputsData import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldNotBe @@ -30,8 +29,8 @@ import org.junit.Test class AWSS3StoragePluginTest { - private val storageServiceFactory = mockk { - every { create(any(), any(), any()) } returns mockk() + private val storageServiceFactory = mockk { + every { create(any(), any(), any(), any()) } returns mockk() } private val plugin = AWSS3StoragePlugin( @@ -52,7 +51,7 @@ class AWSS3StoragePluginTest { plugin.configure(data, mockk()) verify { - storageServiceFactory.create(any(), "test-region", "test-bucket") + storageServiceFactory.create(any(), "test-region", "test-bucket", any()) } } diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/StorageComponentTest.java b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/StorageComponentTest.java index c1ac9ca54..3e238acce 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/StorageComponentTest.java +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/StorageComponentTest.java @@ -32,6 +32,7 @@ import com.amplifyframework.storage.s3.configuration.AWSS3StoragePluginConfiguration; import com.amplifyframework.storage.s3.service.AWSS3StorageService; import com.amplifyframework.storage.s3.service.StorageService; +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider; import com.amplifyframework.storage.s3.transfer.TransferListener; import com.amplifyframework.storage.s3.transfer.TransferObserver; import com.amplifyframework.testutils.Await; @@ -76,6 +77,7 @@ public final class StorageComponentTest { private StorageCategory storage; private StorageService storageService; + private StorageTransferClientProvider clientProvider; /** * Sets up Storage category by registering a mock AWSS3StoragePlugin @@ -88,7 +90,8 @@ public final class StorageComponentTest { public void setup() throws AmplifyException { this.storage = new StorageCategory(); this.storageService = mock(AWSS3StorageService.class); - StorageService.Factory storageServiceFactory = (context, region, bucket) -> storageService; + AWSS3StorageService.Factory storageServiceFactory + = (context, region, bucket, clientProvider) -> (AWSS3StorageService) storageService; AuthCredentialsProvider cognitoAuthProvider = mock(AuthCredentialsProvider.class); doReturn(RandomString.string()).when(cognitoAuthProvider).getIdentityId(null); this.storage.addPlugin(new AWSS3StoragePlugin(storageServiceFactory, diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorkerTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorkerTest.kt index c3097d71c..577288131 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorkerTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/AbortMultiPartUploadWorkerTest.kt @@ -24,6 +24,8 @@ import aws.sdk.kotlin.services.s3.model.AbortMultipartUploadRequest import aws.sdk.kotlin.services.s3.model.AbortMultipartUploadResponse import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.storage.TransferState +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferRecord import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -52,6 +54,7 @@ internal class AbortMultiPartUploadWorkerTest { private lateinit var transferDB: TransferDB private lateinit var transferStatusUpdater: TransferStatusUpdater private lateinit var workerParameters: WorkerParameters + private lateinit var clientProvider: StorageTransferClientProvider @Before fun setup() { @@ -59,6 +62,7 @@ internal class AbortMultiPartUploadWorkerTest { context = ApplicationProvider.getApplicationContext() workerParameters = mockk(WorkerParameters::class.java.name) s3Client = spyk(recordPrivateCalls = true) + clientProvider = mockk(S3StorageTransferClientProvider::class.java.name) mockkStatic(S3Client::withConfig) transferDB = mockk(TransferDB::class.java.name) transferStatusUpdater = mockk(TransferStatusUpdater::class.java.name) @@ -66,6 +70,7 @@ internal class AbortMultiPartUploadWorkerTest { every { workerParameters.runAttemptCount }.answers { 1 } every { workerParameters.taskExecutor }.answers { ImmediateTaskExecutor() } every { any().withConfig(any()) }.answers { s3Client } + every { clientProvider.getStorageTransferClient(any(), any()) }.answers { s3Client } } @After @@ -95,13 +100,20 @@ internal class AbortMultiPartUploadWorkerTest { every { transferDB.getTransferRecordById(any()) }.answers { transferRecord } every { transferStatusUpdater.updateTransferState(any(), any()) }.answers { } - val worker = AbortMultiPartUploadWorker(s3Client, transferDB, transferStatusUpdater, context, workerParameters) + val worker = AbortMultiPartUploadWorker( + clientProvider, + transferDB, + transferStatusUpdater, + context, + workerParameters + ) val result = worker.doWork() val expectedResult = ListenableWorker.Result.success(workDataOf(BaseTransferWorker.OUTPUT_TRANSFER_RECORD_ID to 1)) verify(exactly = 1) { transferStatusUpdater.updateTransferState(1, TransferState.FAILED) } verify(exactly = 1) { any().withConfig(any()) } + verify(exactly = 1) { clientProvider.getStorageTransferClient(any(), any()) } assertEquals(expectedResult, result) } @@ -128,7 +140,13 @@ internal class AbortMultiPartUploadWorkerTest { every { transferDB.getTransferRecordById(any()) }.answers { transferRecord } every { transferStatusUpdater.updateTransferState(any(), any()) }.answers { } - val worker = AbortMultiPartUploadWorker(s3Client, transferDB, transferStatusUpdater, context, workerParameters) + val worker = AbortMultiPartUploadWorker( + clientProvider, + transferDB, + transferStatusUpdater, + context, + workerParameters + ) val result = worker.doWork() val expectedResult = @@ -157,7 +175,13 @@ internal class AbortMultiPartUploadWorkerTest { every { transferStatusUpdater.updateTransferState(any(), any()) }.answers { } every { transferStatusUpdater.updateOnError(any(), any()) }.answers { } - val worker = AbortMultiPartUploadWorker(s3Client, transferDB, transferStatusUpdater, context, workerParameters) + val worker = AbortMultiPartUploadWorker( + clientProvider, + transferDB, + transferStatusUpdater, + context, + workerParameters + ) val result = worker.doWork() val expectedResult = diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorkerTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorkerTest.kt index 9d7ad8114..446015b0c 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorkerTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/DownloadWorkerTest.kt @@ -26,6 +26,8 @@ import aws.smithy.kotlin.runtime.content.ByteStream import aws.smithy.kotlin.runtime.content.fromFile import com.amplifyframework.storage.TransferState import com.amplifyframework.storage.s3.transfer.DownloadProgressListenerInterceptor +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferRecord import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -56,12 +58,14 @@ internal class DownloadWorkerTest { private lateinit var transferStatusUpdater: TransferStatusUpdater private lateinit var workerParameters: WorkerParameters private lateinit var downloadInterceptor: DownloadProgressListenerInterceptor + private lateinit var clientProvider: StorageTransferClientProvider @Before fun setup() { context = ApplicationProvider.getApplicationContext() workerParameters = mockk(WorkerParameters::class.java.name) s3Client = mockk(relaxed = true, relaxUnitFun = true) + clientProvider = mockk(S3StorageTransferClientProvider::class.java.name) mockkStatic(S3Client::withConfig) downloadInterceptor = mockk(relaxed = true, relaxUnitFun = true) transferDB = mockk(TransferDB::class.java.name) @@ -70,6 +74,7 @@ internal class DownloadWorkerTest { every { workerParameters.runAttemptCount }.answers { 1 } every { workerParameters.taskExecutor }.answers { ImmediateTaskExecutor() } every { s3Client.withConfig(any()) } returns s3Client + every { clientProvider.getStorageTransferClient(any(), any())}.answers { s3Client } } @After @@ -102,10 +107,11 @@ internal class DownloadWorkerTest { every { transferStatusUpdater.updateProgress(1, any(), any(), true, false) }.answers { } every { transferStatusUpdater.updateProgress(1, any(), any(), true, true) }.answers { } - val worker = DownloadWorker(s3Client, transferDB, transferStatusUpdater, context, workerParameters) + val worker = DownloadWorker(clientProvider, transferDB, transferStatusUpdater, context, workerParameters) val result = worker.doWork() verify(atLeast = 1) { transferStatusUpdater.updateProgress(1, 10 * 1024 * 1024, 10 * 1024 * 1024, true, true) } + verify(exactly = 1) { clientProvider.getStorageTransferClient(any(), any()) } val expectedResult = ListenableWorker.Result.success(workDataOf(BaseTransferWorker.OUTPUT_TRANSFER_RECORD_ID to 1)) assertEquals(expectedResult, result) @@ -131,7 +137,7 @@ internal class DownloadWorkerTest { every { transferStatusUpdater.updateTransferState(1, TransferState.FAILED) }.answers { } every { transferStatusUpdater.updateOnError(1, any()) }.answers { } - val worker = DownloadWorker(s3Client, transferDB, transferStatusUpdater, context, workerParameters) + val worker = DownloadWorker(clientProvider, transferDB, transferStatusUpdater, context, workerParameters) val result = worker.doWork() verify(exactly = 0) { diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorkerTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorkerTest.kt index ea423d392..852f660a9 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorkerTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/transfer/worker/InitiateMultiPartUploadTransferWorkerTest.kt @@ -23,6 +23,8 @@ import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.model.CreateMultipartUploadResponse import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.storage.TransferState +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferDB import com.amplifyframework.storage.s3.transfer.TransferRecord import com.amplifyframework.storage.s3.transfer.TransferStatusUpdater @@ -51,12 +53,14 @@ internal class InitiateMultiPartUploadTransferWorkerTest { private lateinit var transferDB: TransferDB private lateinit var transferStatusUpdater: TransferStatusUpdater private lateinit var workerParameters: WorkerParameters + private lateinit var clientProvider: StorageTransferClientProvider @Before fun setup() { context = ApplicationProvider.getApplicationContext() workerParameters = mockk(WorkerParameters::class.java.name) s3Client = mockk(relaxed = true) + clientProvider = mockk(S3StorageTransferClientProvider::class.java.name) mockkStatic(S3Client::withConfig) transferDB = mockk(TransferDB::class.java.name) transferStatusUpdater = mockk(TransferStatusUpdater::class.java.name) @@ -64,6 +68,7 @@ internal class InitiateMultiPartUploadTransferWorkerTest { every { workerParameters.runAttemptCount }.answers { 1 } every { workerParameters.taskExecutor }.answers { ImmediateTaskExecutor() } every { s3Client.withConfig(any()) } returns s3Client + every { clientProvider.getStorageTransferClient(any(), any())}.answers { s3Client } } @After @@ -89,7 +94,7 @@ internal class InitiateMultiPartUploadTransferWorkerTest { every { transferStatusUpdater.updateMultipartId(1, "upload_id") }.answers { } every { transferStatusUpdater.updateTransferState(any(), TransferState.IN_PROGRESS) }.answers { } val worker = InitiateMultiPartUploadTransferWorker( - s3Client, + clientProvider, transferDB, transferStatusUpdater, context, @@ -97,6 +102,7 @@ internal class InitiateMultiPartUploadTransferWorkerTest { ) val result = worker.doWork() verify(exactly = 1) { transferStatusUpdater.updateMultipartId(1, "upload_id") } + verify(exactly = 1) { clientProvider.getStorageTransferClient(any(), any()) } val output = workDataOf( BaseTransferWorker.MULTI_PART_UPLOAD_ID to "upload_id", BaseTransferWorker.TRANSFER_RECORD_ID to 1 @@ -124,7 +130,7 @@ internal class InitiateMultiPartUploadTransferWorkerTest { every { transferStatusUpdater.updateTransferState(any(), any()) }.answers { } val worker = InitiateMultiPartUploadTransferWorker( - s3Client, + clientProvider, transferDB, transferStatusUpdater, context, From 4aa2389dab0a3d699c7e4cffba3057df32e4f852 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 20 Aug 2024 10:10:42 -0500 Subject: [PATCH 09/14] chore: minor code cleanup --- .../storage/s3/AWSS3StoragePlugin.java | 23 +++++++++++-------- .../storage/s3/transfer/TransferDB.kt | 2 +- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 090e8f7d8..ec90f6a57 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -142,17 +142,20 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private List configuredBuckets; @SuppressLint("UnsafeOptInUsageError") - private StorageTransferClientProvider clientProvider = new S3StorageTransferClientProvider((region, bucketName) -> { - if (region != null && bucketName != null) { - StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(region, bucketName)); - return getAWSS3StorageService((ResolvedStorageBucket) bucket).getClient(); - } + private StorageTransferClientProvider clientProvider + = new S3StorageTransferClientProvider((region, bucketName) -> { + if (region != null && bucketName != null) { + StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(region, bucketName)); + return getAWSS3StorageService((ResolvedStorageBucket) bucket).getClient(); + } - if (region != null) { - return AWSS3StorageService.getS3Client(region, authCredentialsProvider); - } - return defaultStorageService.getClient(); - }); + if (region != null) { + // unable to create a new S3Client from java code, + // redirecting to AWSS3Service to create S3 Client from kotlin + return AWSS3StorageService.getS3Client(region, authCredentialsProvider); + } + return defaultStorageService.getClient(); + }); /** * Constructs the AWS S3 Storage Plugin initializing the executor service. diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt index 91195026a..9c2949654 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferDB.kt @@ -617,7 +617,7 @@ internal class TransferDB private constructor(context: Context) { isLastPart: Int, metadata: ObjectMetadata?, cannedAcl: ObjectCannedAcl?, - useAccelerateEndpoint: Boolean, + useAccelerateEndpoint: Boolean ): ContentValues { val values = ContentValues() values.put(TransferTable.COLUMN_TRANSFER_ID, transferId) From f06bbc04450a012a5897e0630371a56d4865775c Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 20 Aug 2024 10:16:41 -0500 Subject: [PATCH 10/14] fix incorrect bucket info --- .../com/amplifyframework/storage/s3/AWSS3StoragePlugin.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index ec90f6a57..7b9982f9e 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -145,7 +145,7 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private StorageTransferClientProvider clientProvider = new S3StorageTransferClientProvider((region, bucketName) -> { if (region != null && bucketName != null) { - StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(region, bucketName)); + StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(bucketName, region)); return getAWSS3StorageService((ResolvedStorageBucket) bucket).getClient(); } From 5d08c8d6195c3c319589828e0b60c7a93eaf8ad8 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 20 Aug 2024 10:45:07 -0500 Subject: [PATCH 11/14] update storage database integration test --- .../storage/s3/transfer/TransferDBTest.kt | 9 +++++++++ .../storage/s3/transfer/TransferTable.kt | 1 - 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt index 199428831..1922c896a 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/transfer/TransferDBTest.kt @@ -31,6 +31,7 @@ import org.junit.Test open class TransferDBTest { private val bucketName = "bucket_name" + private val region = "us-east-1" private val fileKey = "file_key" private lateinit var transferDB: TransferDB private lateinit var tempFile: File @@ -55,6 +56,7 @@ open class TransferDBTest { transferId, TransferType.UPLOAD, bucketName, + region, fileKey, tempFile, null, @@ -67,6 +69,7 @@ open class TransferDBTest { Assert.assertEquals(tempFile, File(this.file)) Assert.assertEquals(fileKey, this.key) Assert.assertEquals(bucketName, this.bucketName) + Assert.assertEquals(region, this.region) } ?: Assert.fail("InsertedRecord is null") } @@ -76,6 +79,7 @@ open class TransferDBTest { val uri = transferDB.insertMultipartUploadRecord( uploadID, bucketName, + region, fileKey, tempFile, 1L, @@ -91,6 +95,7 @@ open class TransferDBTest { Assert.assertEquals(fileKey, this.key) Assert.assertEquals(bucketName, this.bucketName) Assert.assertEquals(uploadID, this.multipartId) + Assert.assertEquals(region, this.region) } ?: Assert.fail("InsertedRecord is null") } @@ -104,6 +109,7 @@ open class TransferDBTest { contentValues[0] = transferDB.generateContentValuesForMultiPartUpload( key, bucketName, + region, key, tempFile, 0L, @@ -137,6 +143,7 @@ open class TransferDBTest { contentValues[0] = transferDB.generateContentValuesForMultiPartUpload( key, bucketName, + region, key, tempFile, 0L, @@ -151,6 +158,7 @@ open class TransferDBTest { contentValues[1] = transferDB.generateContentValuesForMultiPartUpload( key, bucketName, + region, key, tempFile, 0L, @@ -165,6 +173,7 @@ open class TransferDBTest { contentValues[2] = transferDB.generateContentValuesForMultiPartUpload( key, bucketName, + region, key, tempFile, 0L, diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt index c80a95320..bc4352535 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt @@ -153,7 +153,6 @@ internal class TransferTable { "$COLUMN_TYPE text not null, " + "$COLUMN_STATE text not null, " + "$COLUMN_BUCKET_NAME text not null, " + - "$COLUMN_REGION text, " + "$COLUMN_KEY text not null," + "$COLUMN_VERSION_ID text, " + "$COLUMN_BYTES_TOTAL bigint, " + From 70b357d4ae0b85e72cf159a2a0091d086e447cee Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 20 Aug 2024 10:55:07 -0500 Subject: [PATCH 12/14] update unit test per code review suggestion --- .../amplifyframework/storage/s3/AWSS3StoragePluginTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt index 5d15f2d3a..a13158f1b 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/AWSS3StoragePluginTest.kt @@ -16,6 +16,7 @@ package com.amplifyframework.storage.s3 import com.amplifyframework.storage.BucketInfo +import com.amplifyframework.storage.InvalidStorageBucketException import com.amplifyframework.storage.StorageBucket import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.s3.service.AWSS3StorageService @@ -23,6 +24,7 @@ import com.amplifyframework.storage.s3.service.StorageService import com.amplifyframework.testutils.configuration.amplifyOutputsData import io.kotest.assertions.throwables.shouldThrow import io.kotest.matchers.shouldNotBe +import io.kotest.matchers.throwable.shouldHaveCauseOfType import io.mockk.every import io.mockk.mockk import io.mockk.verify @@ -143,8 +145,9 @@ class AWSS3StoragePluginTest { plugin.configure(data, mockk()) val bucket = StorageBucket.fromOutputs("myBucket") - shouldThrow { + val exception = shouldThrow { plugin.getStorageService(bucket) } + exception.shouldHaveCauseOfType() } } From e2e4307d7a2000946dff29b84e76a37da8a7887f Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Thu, 22 Aug 2024 15:04:27 -0500 Subject: [PATCH 13/14] chore: remove unnecessary annotation --- .../storage/s3/transfer/StorageTransferClientProvider.kt | 1 - 1 file changed, 1 deletion(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt index 5b430e3fc..523c0dcf3 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/StorageTransferClientProvider.kt @@ -17,7 +17,6 @@ package com.amplifyframework.storage.s3.transfer import aws.sdk.kotlin.services.s3.S3Client import com.amplifyframework.annotations.InternalApiWarning -@InternalApiWarning internal interface StorageTransferClientProvider { fun getStorageTransferClient(region: String?, bucketName: String?): S3Client } From cdb3813e878227524fcfbb1345edafe4afc44e8a Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Mon, 26 Aug 2024 09:46:11 -0500 Subject: [PATCH 14/14] refactor and resolve review comments --- .../storage/s3/AWSS3StoragePlugin.java | 76 ++++------- .../storage/s3/service/AWSS3StorageService.kt | 16 +-- .../service/AWSS3StorageServiceContainer.kt | 88 +++++++++++++ .../S3StorageTransferClientProvider.kt | 10 +- .../storage/s3/transfer/TransferTable.kt | 9 +- .../AWSS3StorageServiceContainerTest.kt | 121 ++++++++++++++++++ 6 files changed, 249 insertions(+), 71 deletions(-) create mode 100644 aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageServiceContainer.kt create mode 100644 aws-storage-s3/src/test/java/com/amplifyframework/storage/AWSS3StorageServiceContainerTest.kt diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java index 7b9982f9e..46316bef0 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/AWSS3StoragePlugin.java @@ -90,6 +90,7 @@ import com.amplifyframework.storage.s3.request.AWSS3StorageRemoveRequest; import com.amplifyframework.storage.s3.request.AWSS3StorageUploadRequest; import com.amplifyframework.storage.s3.service.AWSS3StorageService; +import com.amplifyframework.storage.s3.service.AWSS3StorageServiceContainer; import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider; import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider; import com.amplifyframework.storage.s3.transfer.TransferObserver; @@ -102,9 +103,7 @@ import java.io.File; import java.io.InputStream; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.Objects; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; @@ -136,8 +135,7 @@ public final class AWSS3StoragePlugin extends StoragePlugin { private StorageAccessLevel defaultAccessLevel; private int defaultUrlExpiration; - private final Map awsS3StorageServicesByBucketName = new HashMap<>(); - private Context context; + private AWSS3StorageServiceContainer awss3StorageServiceContainer; @SuppressLint("UnsafeOptInUsageError") private List configuredBuckets; @@ -146,13 +144,11 @@ public final class AWSS3StoragePlugin extends StoragePlugin { = new S3StorageTransferClientProvider((region, bucketName) -> { if (region != null && bucketName != null) { StorageBucket bucket = StorageBucket.fromBucketInfo(new BucketInfo(bucketName, region)); - return getAWSS3StorageService((ResolvedStorageBucket) bucket).getClient(); + return awss3StorageServiceContainer.get((ResolvedStorageBucket) bucket).getClient(); } if (region != null) { - // unable to create a new S3Client from java code, - // redirecting to AWSS3Service to create S3 Client from kotlin - return AWSS3StorageService.getS3Client(region, authCredentialsProvider); + return S3StorageTransferClientProvider.getS3Client(region, authCredentialsProvider); } return defaultStorageService.getClient(); }); @@ -195,6 +191,7 @@ public AWSS3StoragePlugin(AWSS3StoragePluginConfiguration awsS3StoragePluginConf @VisibleForTesting AWSS3StoragePlugin(AuthCredentialsProvider authCredentialsProvider, AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration) { + this((context, region, bucket, clientProvider) -> new AWSS3StorageService( context, @@ -300,14 +297,15 @@ private void configure( @NonNull ResolvedStorageBucket bucket ) throws StorageException { try { - this.context = context; this.defaultStorageService = storageServiceFactory.create( context, region, bucket.getBucketInfo().getName(), clientProvider); - this.awsS3StorageServicesByBucketName.clear(); - this.awsS3StorageServicesByBucketName.put(bucket.getBucketInfo().getName(), this.defaultStorageService); + this.awss3StorageServiceContainer = new AWSS3StorageServiceContainer( + context, storageServiceFactory, + (S3StorageTransferClientProvider) clientProvider); + this.awss3StorageServiceContainer.put(bucket.getBucketInfo().getName(), this.defaultStorageService); } catch (RuntimeException exception) { throw new StorageException( "Failed to create storage service.", @@ -1150,55 +1148,27 @@ AWSS3StorageService getStorageService(@Nullable StorageBucket bucket) throws Sto } if (bucket instanceof OutputsStorageBucket) { - AWSS3StorageService service = getAWSS3StorageService((OutputsStorageBucket) bucket); - if (service == null) { - throw new StorageException( - "Unable to find bucket from name in Amplify Outputs.", - new InvalidStorageBucketException(), - "Ensure the bucket name used is available in Amplify Outputs."); - } else { - return service; - } - } - - if (bucket instanceof ResolvedStorageBucket) { - return getAWSS3StorageService((ResolvedStorageBucket) bucket); - } - - return defaultStorageService; - } - - @SuppressLint("UnsafeOptInUsageError") - private AWSS3StorageService getAWSS3StorageService(OutputsStorageBucket outputsStorageBucket) { - if (configuredBuckets != null && !configuredBuckets.isEmpty()) { - String name = outputsStorageBucket.getName(); - for (AmplifyOutputsData.StorageBucket configuredBucket : configuredBuckets) { - if (configuredBucket.getName().equals(name)) { - String bucketName = configuredBucket.getBucketName(); - AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); - if (service == null) { + if (configuredBuckets != null && !configuredBuckets.isEmpty()) { + String name = ((OutputsStorageBucket) bucket).getName(); + for (AmplifyOutputsData.StorageBucket configuredBucket : configuredBuckets) { + if (configuredBucket.getName().equals(name)) { + String bucketName = configuredBucket.getBucketName(); String region = configuredBucket.getAwsRegion(); - service = storageServiceFactory.create(context, region, bucketName, clientProvider); - awsS3StorageServicesByBucketName.put(bucketName, service); + return awss3StorageServiceContainer.get(bucketName, region); } - - return service; } } + throw new StorageException( + "Unable to find bucket from name in Amplify Outputs.", + new InvalidStorageBucketException(), + "Ensure the bucket name used is available in Amplify Outputs."); } - return null; - } - @SuppressLint("UnsafeOptInUsageError") - private AWSS3StorageService getAWSS3StorageService(ResolvedStorageBucket resolvedStorageBucket) { - String bucketName = resolvedStorageBucket.getBucketInfo().getName(); - AWSS3StorageService service = awsS3StorageServicesByBucketName.get(bucketName); - if (service == null) { - String region = resolvedStorageBucket.getBucketInfo().getRegion(); - service = storageServiceFactory.create(context, region, bucketName, clientProvider); - awsS3StorageServicesByBucketName.put(bucketName, service); + if (bucket instanceof ResolvedStorageBucket) { + return awss3StorageServiceContainer.get((ResolvedStorageBucket) bucket); } - return service; + + return defaultStorageService; } /** diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt index 8d35bba5e..eb7e573c2 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageService.kt @@ -32,6 +32,7 @@ import com.amplifyframework.storage.StorageItem import com.amplifyframework.storage.options.SubpathStrategy import com.amplifyframework.storage.options.SubpathStrategy.Exclude import com.amplifyframework.storage.result.StorageListResult +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider import com.amplifyframework.storage.s3.transfer.TransferManager import com.amplifyframework.storage.s3.transfer.TransferObserver @@ -59,20 +60,7 @@ internal class AWSS3StorageService( private val clientProvider: StorageTransferClientProvider ) : StorageService { - companion object { - @JvmStatic - fun getS3Client(region: String, authCredentialsProvider: AuthCredentialsProvider): S3Client { - return S3Client { - this.region = region - this.credentialsProvider = authCredentialsProvider - } - } - } - - private var s3Client: S3Client = S3Client { - region = awsRegion - credentialsProvider = authCredentialsProvider - } + private var s3Client: S3Client = S3StorageTransferClientProvider.getS3Client(awsRegion, authCredentialsProvider) val transferManager: TransferManager = TransferManager(context, clientProvider, awsS3StoragePluginKey) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageServiceContainer.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageServiceContainer.kt new file mode 100644 index 000000000..f22131df2 --- /dev/null +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/AWSS3StorageServiceContainer.kt @@ -0,0 +1,88 @@ +/* + * Copyright 2022 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ +package com.amplifyframework.storage.s3.service + +import android.content.Context +import com.amplifyframework.storage.ResolvedStorageBucket +import com.amplifyframework.storage.s3.transfer.S3StorageTransferClientProvider +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider +import java.util.concurrent.ConcurrentHashMap + +/** + * A container that stores a list of AWSS3StorageService based on the bucket name associated with the service. + * repository. + */ +internal class AWSS3StorageServiceContainer( + private val context: Context, + private val storageServiceFactory: AWSS3StorageService.Factory, + private val clientProvider: StorageTransferClientProvider, + private val awsS3StorageServicesByBucketName: ConcurrentHashMap +) { + constructor( + context: Context, + storageServiceFactory: AWSS3StorageService.Factory, + clientProvider: S3StorageTransferClientProvider + ) : this(context, storageServiceFactory, clientProvider, ConcurrentHashMap()) + + private val lock = Any() + + /** + * Stores a instance of AWSS3StorageService + * + * @param bucketName the bucket name + * @param service the AWSS3StorageService instance + */ + fun put(bucketName: String, service: AWSS3StorageService) { + synchronized(lock) { + awsS3StorageServicesByBucketName.put(bucketName, service) + } + } + + /** + * Get an AWSS3StorageSErvice instance based on a ResolvedStorageBucket + * @param resolvedStorageBucket An instance of ResolvedStorageBucket with bucket info + * @return An AWSS3StorageService instance associated with the ResolvedStorageBucket + */ + fun get(resolvedStorageBucket: ResolvedStorageBucket): AWSS3StorageService { + synchronized(lock) { + val bucketName: String = resolvedStorageBucket.bucketInfo.name + var service = awsS3StorageServicesByBucketName.get(bucketName) + if (service == null) { + val region: String = resolvedStorageBucket.bucketInfo.region + service = storageServiceFactory.create(context, region, bucketName, clientProvider) + awsS3StorageServicesByBucketName[bucketName] = service + } + return service + } + } + + /** + * Get an AWSS3StorageSErvice instance based on a bucket name and region + * @param bucketName the bucket name associated with the AWSS3StorageService + * @param bucketName the region to associate with a new AWSS3StorageService instance if one doesn't exist + * @return An AWSS3StorageService instance associated with the ResolvedStorageBucket + */ + fun get(bucketName: String, region: String): AWSS3StorageService { + synchronized(lock) { + var service = awsS3StorageServicesByBucketName[bucketName] + if (service == null) { + service = storageServiceFactory.create(context, region, bucketName, clientProvider) + awsS3StorageServicesByBucketName[bucketName] = service + } + + return service + } + } +} diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt index 7f0e1ebe5..49d976f7b 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/S3StorageTransferClientProvider.kt @@ -16,11 +16,19 @@ package com.amplifyframework.storage.s3.transfer import aws.sdk.kotlin.services.s3.S3Client import com.amplifyframework.auth.AuthCredentialsProvider -import com.amplifyframework.storage.StorageException internal class S3StorageTransferClientProvider( private val createS3Client: (region: String?, bucketName: String?) -> S3Client ) : StorageTransferClientProvider { + companion object { + @JvmStatic + fun getS3Client(region: String, authCredentialsProvider: AuthCredentialsProvider): S3Client { + return S3Client { + this.region = region + this.credentialsProvider = authCredentialsProvider + } + } + } override fun getStorageTransferClient(region: String?, bucketName: String?): S3Client { return createS3Client(region, bucketName) } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt index bc4352535..d7417c493 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/transfer/TransferTable.kt @@ -146,7 +146,11 @@ internal class TransferTable { private const val TABLE_VERSION_9 = 9 private const val TABLE_VERSION_10 = 10 - // Database creation SQL statement + // **** DO NOT UPDATE *** + // Database creation SQL statement for TABLE_VERSION 1 + // The current database migration implementation assumes that the original version 1 is always created + // and then incrementally upgrades from the original version 1 to latest version. + // instead of of upgrading from the last/previous version to the latest version. const val DATABASE_CREATE = "create table $TABLE_TRANSFER (" + "$COLUMN_ID integer primary key autoincrement, " + "$COLUMN_MAIN_UPLOAD_ID integer, " + @@ -304,8 +308,7 @@ internal class TransferTable { } private fun addVersion10Columns(database: SQLiteDatabase) { - val addRegion = "ALTER TABLE $TABLE_TRANSFER ADD COLUMN $COLUMN_REGION text " + - "DEFAULT null;" + val addRegion = "ALTER TABLE $TABLE_TRANSFER ADD COLUMN $COLUMN_REGION text " + "DEFAULT null;" database.execSQL(addRegion) } } diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/AWSS3StorageServiceContainerTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/AWSS3StorageServiceContainerTest.kt new file mode 100644 index 000000000..3119dab16 --- /dev/null +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/AWSS3StorageServiceContainerTest.kt @@ -0,0 +1,121 @@ +/* + * Copyright 2024 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://aws.amazon.com/apache2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amplifyframework.storage.s3 + +import android.content.Context +import com.amplifyframework.storage.BucketInfo +import com.amplifyframework.storage.ResolvedStorageBucket +import com.amplifyframework.storage.StorageBucket +import com.amplifyframework.storage.s3.service.AWSS3StorageService +import com.amplifyframework.storage.s3.service.AWSS3StorageServiceContainer +import com.amplifyframework.storage.s3.transfer.StorageTransferClientProvider +import io.kotest.matchers.shouldBe +import io.kotest.matchers.shouldNotBe +import io.mockk.every +import io.mockk.mockk +import java.util.concurrent.ConcurrentHashMap +import org.junit.Before +import org.junit.Test + +class AWSS3StorageServiceContainerTest { + + private val storageServiceFactory = mockk { + every { create(any(), any(), any(), any()) } returns mockk() + } + private val context = mockk() + private val clientProvider = mockk() + private val bucketName = "testBucket" + private val region = "us-east-1" + + private lateinit var serviceContainerHashMap: ConcurrentHashMap + private lateinit var serviceContainer: AWSS3StorageServiceContainer + @Before + fun setUp() { + serviceContainerHashMap = ConcurrentHashMap() + serviceContainer = AWSS3StorageServiceContainer( + context, + storageServiceFactory, + clientProvider, + serviceContainerHashMap + ) + } + + @Test + fun `put default AWSS3Service in container`() { + val service = storageServiceFactory.create(context, region, bucketName, clientProvider) + serviceContainer.put(bucketName, service) + + serviceContainerHashMap.size shouldBe 1 + serviceContainerHashMap[bucketName] shouldNotBe null + } + + @Test + fun `get non-existent AWSS3Service in container with ResolvedStorageBucket creates new AWSService`() { + val bucketInfo = BucketInfo(bucketName, region) + val bucket: ResolvedStorageBucket = StorageBucket.fromBucketInfo(bucketInfo) as ResolvedStorageBucket + + val service = serviceContainer.get(bucket) + + service shouldNotBe null + serviceContainerHashMap.size shouldBe 1 + serviceContainerHashMap[bucketName] shouldNotBe null + serviceContainerHashMap[bucketName] shouldBe service + } + + @Test + fun `get WSS3Service in container multiple times with ResolvedStorageBucket creates only one service`() { + val bucketInfo = BucketInfo(bucketName, region) + val bucket: ResolvedStorageBucket = StorageBucket.fromBucketInfo(bucketInfo) as ResolvedStorageBucket + + val service = serviceContainer.get(bucket) + val service2 = serviceContainer.get(bucket) + + service shouldNotBe null + service2 shouldNotBe null + service shouldBe service2 + + serviceContainerHashMap.size shouldBe 1 + serviceContainerHashMap[bucketName] shouldNotBe null + serviceContainerHashMap[bucketName] shouldBe service + serviceContainerHashMap[bucketName] shouldBe service2 + } + + @Test + fun `get non-existent AWSS3Service in container with bucket name and region creates new AWSService`() { + val service = serviceContainer.get(bucketName, region) + + service shouldNotBe null + serviceContainerHashMap.size shouldBe 1 + serviceContainerHashMap[bucketName] shouldNotBe null + serviceContainerHashMap[bucketName] shouldBe service + } + + @Test + fun `get WSS3Service in container multiple times with bucket name and region creates only one service`() { + + val service = serviceContainer.get(bucketName, region) + val service2 = serviceContainer.get(bucketName, region) + + service shouldNotBe null + service2 shouldNotBe null + service shouldBe service2 + + serviceContainerHashMap.size shouldBe 1 + serviceContainerHashMap[bucketName] shouldNotBe null + serviceContainerHashMap[bucketName] shouldBe service + serviceContainerHashMap[bucketName] shouldBe service2 + } +}