From 05f1e84d0bbde80b14a638a0b587351987ef5053 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 12 Jun 2024 12:46:02 -0500 Subject: [PATCH 01/13] feat(storage): add object existence validation option to get presigned url --- .../storage/s3/AWSS3StoragePlugin.java | 7 +- ...WSS3StoragePathGetPresignedUrlOperation.kt | 18 +++ .../AWSS3StorageGetPresignedUrlOptions.java | 23 ++++ .../AWSS3StoragePathGetPresignedUrlRequest.kt | 3 +- .../storage/s3/service/AWSS3StorageService.kt | 29 ++++- .../storage/s3/service/StorageService.java | 9 ++ .../AWSS3StoragePathGetUrlOperationTest.kt | 104 +++++++++++++++--- 7 files changed, 174 insertions(+), 19 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 4af9a374e9..f792e608ae 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 @@ -355,10 +355,15 @@ public StorageGetUrlOperation getUrl( ) { boolean useAccelerateEndpoint = options instanceof AWSS3StorageGetPresignedUrlOptions && ((AWSS3StorageGetPresignedUrlOptions) options).useAccelerateEndpoint(); + + boolean validateObjectExistence = options instanceof AWSS3StorageGetPresignedUrlOptions && + ((AWSS3StorageGetPresignedUrlOptions) options).validateObjectExistence(); + AWSS3StoragePathGetPresignedUrlRequest request = new AWSS3StoragePathGetPresignedUrlRequest( path, options.getExpires() != 0 ? options.getExpires() : defaultUrlExpiration, - useAccelerateEndpoint + useAccelerateEndpoint, + validateObjectExistence ); AWSS3StoragePathGetPresignedUrlOperation operation = diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt index 3faacef715..eb79293587 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt @@ -48,6 +48,24 @@ internal class AWSS3StoragePathGetPresignedUrlOperation( return@submit } + if (request.validateObjectExistence) { + try { + storageService.validateObjectExists(serviceKey) + } catch (se: StorageException) { + onError.accept(se) + return@submit + } catch (exception: Exception) { + onError.accept( + StorageException( + "Encountered an issue while generating pre-signed URL", + exception, + "See included exception for more details and suggestions to fix." + ) + ) + return@submit + } + } + try { val url = storageService.getPresignedUrl( serviceKey, 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 36b7ae7fc8..925cba6f63 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 @@ -26,10 +26,12 @@ */ public final class AWSS3StorageGetPresignedUrlOptions extends StorageGetUrlOptions { private final boolean useAccelerationMode; + private final boolean validateObjectExistence; private AWSS3StorageGetPresignedUrlOptions(final Builder builder) { super(builder); this.useAccelerationMode = builder.useAccelerateEndpoint; + this.validateObjectExistence = builder.validateObjectExistence; } /** @@ -80,6 +82,16 @@ public boolean useAccelerateEndpoint() { return useAccelerationMode; } + /** + * Gets the flag to determine whether to validate whether an S3 object exists. + * Note: Setting this to `true` will result in a latency cost since confirming the existence + * of the underlying S3 object will likely require a round-trip network call. + * @return boolean flag + */ + public boolean validateObjectExistence() { + return validateObjectExistence; + } + @Override @SuppressWarnings("deprecation") public boolean equals(Object obj) { @@ -123,6 +135,7 @@ public String toString() { */ public static final class Builder extends StorageGetUrlOptions.Builder { private boolean useAccelerateEndpoint; + private boolean validateObjectExistence; /** * Configure to use acceleration mode on new StorageGetPresignedUrlOptions instances. @@ -134,6 +147,16 @@ public Builder setUseAccelerateEndpoint(boolean useAccelerateEndpoint) { return this; } + /** + * Configure to validate object existence flag on new StorageGetPresignedUrlOptions instances. + * @param validateObjectExistence flag to represent flag to validate object existence for new GetPresignedUrlOptions instance + * @return Current Builder instance for fluent chaining + */ + public Builder setValidateObjectExistence(boolean validateObjectExistence) { + this.validateObjectExistence = validateObjectExistence; + return this; + } + @Override @NonNull public AWSS3StorageGetPresignedUrlOptions build() { diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt index 47de0dec6f..f3b01bc3ae 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt @@ -22,5 +22,6 @@ import com.amplifyframework.storage.StoragePath internal data class AWSS3StoragePathGetPresignedUrlRequest( val path: StoragePath, val expires: Int, - val useAccelerateEndpoint: Boolean + val useAccelerateEndpoint: Boolean, + val validateObjectExistence: Boolean ) 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 ec7d087e39..3fc2a83bf2 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 @@ -20,11 +20,14 @@ import aws.sdk.kotlin.services.s3.S3Client import aws.sdk.kotlin.services.s3.deleteObject import aws.sdk.kotlin.services.s3.listObjectsV2 import aws.sdk.kotlin.services.s3.model.GetObjectRequest +import aws.sdk.kotlin.services.s3.model.HeadObjectRequest +import aws.sdk.kotlin.services.s3.model.NotFound import aws.sdk.kotlin.services.s3.paginators.listObjectsV2Paginated import aws.sdk.kotlin.services.s3.presigners.presignGetObject import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.storage.ObjectMetadata +import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.StorageItem import com.amplifyframework.storage.result.StorageListResult import com.amplifyframework.storage.s3.transfer.TransferManager @@ -32,6 +35,7 @@ import com.amplifyframework.storage.s3.transfer.TransferObserver import com.amplifyframework.storage.s3.transfer.TransferRecord import com.amplifyframework.storage.s3.transfer.UploadOptions import com.amplifyframework.storage.s3.utils.S3Keys +import kotlinx.coroutines.runBlocking import java.io.File import java.io.IOException import java.io.InputStream @@ -40,7 +44,6 @@ import java.time.Instant 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. @@ -85,6 +88,30 @@ internal class AWSS3StorageService( return URL(presignUrlRequest.url.toString()) } + /** + * Validate if S3 object exists for the given key. + * Throws StorageException if NoSuchKey S3 client exception is caught. + * @param serviceKey S3 service key + */ + override fun validateObjectExists(serviceKey: String) { + try { + runBlocking { + s3Client.headObject( + HeadObjectRequest { + bucket = s3BucketName + key = serviceKey + } + ) + } + } catch (ex: NotFound) { + throw StorageException( + "Unable to generate URL for non-existent path: $serviceKey", + ex, + "Please ensure the path is valid or the object has been uploaded" + ) + } + } + /** * Begin downloading a file. * @param serviceKey S3 service key diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java index cc0eaf762d..916a619069 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java @@ -20,6 +20,7 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.ObjectMetadata; +import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StorageItem; import com.amplifyframework.storage.result.StorageListResult; import com.amplifyframework.storage.s3.transfer.TransferObserver; @@ -36,6 +37,14 @@ */ public interface StorageService { + /** + * Validate if Storage object exists for the given key. + * Throws StorageException if object is not does not exist. + * + * @param serviceKey key to uniquely specify item to generate URL for + */ + void validateObjectExists(@NonNull String serviceKey); + /** * Generate pre-signed download URL for an object. * diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt index 0c43c7566a..40cabddf26 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt @@ -33,7 +33,7 @@ import org.junit.Test class AWSS3StoragePathGetUrlOperationTest { - private lateinit var awsS3StorageDownloadFileOperation: AWSS3StoragePathGetPresignedUrlOperation + private lateinit var awsS3StorageGetPresignedUrlOperation: AWSS3StoragePathGetPresignedUrlOperation private lateinit var storageService: StorageService private lateinit var authCredentialsProvider: AuthCredentialsProvider @@ -53,10 +53,11 @@ class AWSS3StoragePathGetUrlOperationTest { val request = AWSS3StoragePathGetPresignedUrlRequest( path, expectedExpires, - false + false, + validateObjectExistence = false ) val onError = mockk>(relaxed = true) - awsS3StorageDownloadFileOperation = AWSS3StoragePathGetPresignedUrlOperation( + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( request = request, storageService = storageService, executorService = MoreExecutors.newDirectExecutorService(), @@ -66,7 +67,7 @@ class AWSS3StoragePathGetUrlOperationTest { ) // WHEN - awsS3StorageDownloadFileOperation.start() + awsS3StorageGetPresignedUrlOperation.start() // THEN verify(exactly = 0) { onError.accept(any()) } @@ -88,10 +89,11 @@ class AWSS3StoragePathGetUrlOperationTest { val request = AWSS3StoragePathGetPresignedUrlRequest( path, expectedExpires, - false + false, + validateObjectExistence = false ) val onError = mockk>(relaxed = true) - awsS3StorageDownloadFileOperation = AWSS3StoragePathGetPresignedUrlOperation( + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( request = request, storageService = storageService, executorService = MoreExecutors.newDirectExecutorService(), @@ -101,7 +103,7 @@ class AWSS3StoragePathGetUrlOperationTest { ) // WHEN - awsS3StorageDownloadFileOperation.start() + awsS3StorageGetPresignedUrlOperation.start() // THEN verify(exactly = 0) { onError.accept(any()) } @@ -122,10 +124,11 @@ class AWSS3StoragePathGetUrlOperationTest { val request = AWSS3StoragePathGetPresignedUrlRequest( path, expectedExpires, - false + false, + validateObjectExistence = false ) val onError = mockk>(relaxed = true) - awsS3StorageDownloadFileOperation = AWSS3StoragePathGetPresignedUrlOperation( + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( request = request, storageService = storageService, executorService = MoreExecutors.newDirectExecutorService(), @@ -135,7 +138,7 @@ class AWSS3StoragePathGetUrlOperationTest { ) // WHEN - awsS3StorageDownloadFileOperation.start() + awsS3StorageGetPresignedUrlOperation.start() // THEN verify { onError.accept(StoragePathValidationException.invalidStoragePathException()) } @@ -153,10 +156,11 @@ class AWSS3StoragePathGetUrlOperationTest { val request = AWSS3StoragePathGetPresignedUrlRequest( path, expectedExpires, - false + false, + validateObjectExistence = false ) val onError = mockk>(relaxed = true) - awsS3StorageDownloadFileOperation = AWSS3StoragePathGetPresignedUrlOperation( + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( request = request, storageService = storageService, executorService = MoreExecutors.newDirectExecutorService(), @@ -166,7 +170,7 @@ class AWSS3StoragePathGetUrlOperationTest { ) // WHEN - awsS3StorageDownloadFileOperation.start() + awsS3StorageGetPresignedUrlOperation.start() // THEN verify { @@ -190,10 +194,11 @@ class AWSS3StoragePathGetUrlOperationTest { val request = AWSS3StoragePathGetPresignedUrlRequest( path, expectedExpires, - false + false, + validateObjectExistence = false ) val onError = mockk>(relaxed = true) - awsS3StorageDownloadFileOperation = AWSS3StoragePathGetPresignedUrlOperation( + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( request = request, storageService = storageService, executorService = MoreExecutors.newDirectExecutorService(), @@ -203,7 +208,7 @@ class AWSS3StoragePathGetUrlOperationTest { ) // WHEN - awsS3StorageDownloadFileOperation.start() + awsS3StorageGetPresignedUrlOperation.start() // THEN verify { onError.accept(StoragePathValidationException.unsupportedStoragePathException()) } @@ -212,5 +217,72 @@ class AWSS3StoragePathGetUrlOperationTest { } } + @Test + fun `getPresignedUrl fails with non existent S3 path when validateObjectExistence is enabled`() { + // GIVEN + val path = StoragePath.fromString("public/123") + val expectedException = StorageException("Test", "Test") + coEvery { storageService.validateObjectExists(any()) } throws expectedException + val request = AWSS3StoragePathGetPresignedUrlRequest( + path, + expectedExpires, + false, + validateObjectExistence = true + ) + val onError = mockk>(relaxed = true) + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( + request = request, + storageService = storageService, + executorService = MoreExecutors.newDirectExecutorService(), + authCredentialsProvider = authCredentialsProvider, + onSuccess = {}, + onError = onError + ) + + // WHEN + awsS3StorageGetPresignedUrlOperation.start() + + // THEN + verify(exactly = 1) { onError.accept(expectedException) } + verify(exactly = 0) { + storageService.getPresignedUrl(any(), any(), any()) + } + } + + @Test + fun `getPresignedUrl succeeds when validateObjectExistence is enabled`() { + // GIVEN + val path = StoragePath.fromString("public/123") + val expectedServiceKey = "public/123" + val request = AWSS3StoragePathGetPresignedUrlRequest( + path, + expectedExpires, + false, + validateObjectExistence = true + ) + val onError = mockk>(relaxed = true) + awsS3StorageGetPresignedUrlOperation = AWSS3StoragePathGetPresignedUrlOperation( + request = request, + storageService = storageService, + executorService = MoreExecutors.newDirectExecutorService(), + authCredentialsProvider = authCredentialsProvider, + onSuccess = {}, + onError = onError + ) + + // WHEN + awsS3StorageGetPresignedUrlOperation.start() + + // THEN + verify(exactly = 0) { onError.accept(any()) } + verify { + storageService.getPresignedUrl( + expectedServiceKey, + expectedExpires, + false + ) + } + } + class UnsupportedStoragePath : StoragePath() } From 6c637dca055c0511b58a355372f38b89f8d40ce2 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 12 Jun 2024 14:31:16 -0500 Subject: [PATCH 02/13] resolve ktlint errors --- .../storage/s3/options/AWSS3StorageGetPresignedUrlOptions.java | 2 +- .../s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt | 2 +- .../com/amplifyframework/storage/s3/service/StorageService.java | 1 - 3 files changed, 2 insertions(+), 3 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 925cba6f63..419208b602 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 @@ -149,7 +149,7 @@ public Builder setUseAccelerateEndpoint(boolean useAccelerateEndpoint) { /** * Configure to validate object existence flag on new StorageGetPresignedUrlOptions instances. - * @param validateObjectExistence flag to represent flag to validate object existence for new GetPresignedUrlOptions instance + * @param validateObjectExistence boolean flag to represent flag to validate object existence. * @return Current Builder instance for fluent chaining */ public Builder setValidateObjectExistence(boolean validateObjectExistence) { diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt index f3b01bc3ae..cf914e35b1 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StoragePathGetPresignedUrlRequest.kt @@ -23,5 +23,5 @@ internal data class AWSS3StoragePathGetPresignedUrlRequest( val path: StoragePath, val expires: Int, val useAccelerateEndpoint: Boolean, - val validateObjectExistence: Boolean + val validateObjectExistence: Boolean, ) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java index 916a619069..de3209a2b5 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java @@ -20,7 +20,6 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.ObjectMetadata; -import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StorageItem; import com.amplifyframework.storage.result.StorageListResult; import com.amplifyframework.storage.s3.transfer.TransferObserver; From 656543acfe3eaa8bb347304c1a8ebb627e5bbba1 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 12 Jun 2024 14:34:36 -0500 Subject: [PATCH 03/13] fix imports order --- .../amplifyframework/storage/s3/service/AWSS3StorageService.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3fc2a83bf2..b4d733a730 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 @@ -35,7 +35,6 @@ import com.amplifyframework.storage.s3.transfer.TransferObserver import com.amplifyframework.storage.s3.transfer.TransferRecord import com.amplifyframework.storage.s3.transfer.UploadOptions import com.amplifyframework.storage.s3.utils.S3Keys -import kotlinx.coroutines.runBlocking import java.io.File import java.io.IOException import java.io.InputStream @@ -44,6 +43,7 @@ import java.time.Instant 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. From 22977b4f3aab26f162c0f71317fcd91537782c4f Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Thu, 13 Jun 2024 10:15:59 -0500 Subject: [PATCH 04/13] update gen1 getURL to support object existence validation --- aws-storage-s3/api/aws-storage-s3.api | 5 ++ .../storage/s3/AWSS3StoragePathGetUrlTest.kt | 24 ++++++ .../storage/s3/AWSS3StoragePlugin.java | 7 +- .../AWSS3StorageGetPresignedUrlOperation.java | 16 +++- .../AWSS3StorageGetPresignedUrlOptions.java | 11 ++- .../AWSS3StorageGetPresignedUrlRequest.java | 41 ++++++++++ .../storage/s3/service/StorageService.java | 4 +- ...WSS3StorageGetPresignedUrlOperationTest.kt | 78 +++++++++++++++++++ 8 files changed, 177 insertions(+), 9 deletions(-) diff --git a/aws-storage-s3/api/aws-storage-s3.api b/aws-storage-s3/api/aws-storage-s3.api index d2766e227b..96aa6229b1 100644 --- a/aws-storage-s3/api/aws-storage-s3.api +++ b/aws-storage-s3/api/aws-storage-s3.api @@ -176,6 +176,7 @@ public final class com/amplifyframework/storage/s3/options/AWSS3StorageGetPresig public static fun defaultInstance ()Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions; public fun equals (Ljava/lang/Object;)Z public static fun from (Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions;)Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions$Builder; + public fun getValidateObjectExistence ()Z public fun hashCode ()I public fun toString ()Ljava/lang/String; public fun useAccelerateEndpoint ()Z @@ -187,6 +188,7 @@ public final class com/amplifyframework/storage/s3/options/AWSS3StorageGetPresig public synthetic fun build ()Lcom/amplifyframework/storage/options/StorageOptions; public fun build ()Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions; public fun setUseAccelerateEndpoint (Z)Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions$Builder; + public fun setValidateObjectExistence (Z)Lcom/amplifyframework/storage/s3/options/AWSS3StorageGetPresignedUrlOptions$Builder; } public final class com/amplifyframework/storage/s3/options/AWSS3StorageListOptions : com/amplifyframework/storage/options/StorageListOptions { @@ -283,11 +285,13 @@ public final class com/amplifyframework/storage/s3/request/AWSS3StorageDownloadF public final class com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest { public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZ)V + public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZZ)V public fun getAccessLevel ()Lcom/amplifyframework/storage/StorageAccessLevel; public fun getExpires ()I public fun getKey ()Ljava/lang/String; public fun getTargetIdentityId ()Ljava/lang/String; public fun useAccelerateEndpoint ()Z + public fun validateObjectExistence ()Z } public final class com/amplifyframework/storage/s3/request/AWSS3StorageListRequest { @@ -331,6 +335,7 @@ public abstract interface class com/amplifyframework/storage/s3/service/StorageS public abstract fun resumeTransfer (Lcom/amplifyframework/storage/s3/transfer/TransferObserver;)V public abstract fun uploadFile (Ljava/lang/String;Ljava/lang/String;Ljava/io/File;Lcom/amplifyframework/storage/ObjectMetadata;Z)Lcom/amplifyframework/storage/s3/transfer/TransferObserver; public abstract fun uploadInputStream (Ljava/lang/String;Ljava/lang/String;Ljava/io/InputStream;Lcom/amplifyframework/storage/ObjectMetadata;Z)Lcom/amplifyframework/storage/s3/transfer/TransferObserver; + public abstract fun validateObjectExists (Ljava/lang/String;)V } public abstract interface class com/amplifyframework/storage/s3/service/StorageService$Factory { diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index 4fc454a1e4..2d0d778da2 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -18,9 +18,11 @@ import android.content.Context import androidx.test.core.app.ApplicationProvider import com.amplifyframework.auth.cognito.AWSCognitoAuthPlugin import com.amplifyframework.storage.StorageCategory +import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.StoragePath import com.amplifyframework.storage.options.StorageGetUrlOptions import com.amplifyframework.storage.options.StorageUploadFileOptions +import com.amplifyframework.storage.s3.options.AWSS3StorageGetPresignedUrlOptions import com.amplifyframework.storage.s3.test.R import com.amplifyframework.storage.s3.util.WorkmanagerTestUtils.initializeWorkmanagerTestUtil import com.amplifyframework.testutils.random.RandomTempFile @@ -28,6 +30,7 @@ import com.amplifyframework.testutils.sync.SynchronousAuth import com.amplifyframework.testutils.sync.SynchronousStorage import java.io.File import org.junit.Assert.assertEquals +import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.BeforeClass import org.junit.Test @@ -79,4 +82,25 @@ class AWSS3StoragePathGetUrlTest { assertEquals("/public/$SMALL_FILE_NAME", result.url.path) assertTrue(result.url.query.contains("X-Amz-Expires=30")) } + + @Test + fun testGetUrlWithObjectExistenceValidationEnabled() { + val result = synchronousStorage.getUrl( + SMALL_FILE_PATH, + AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(true).expires(30).build() + ) + + assertEquals("/public/$SMALL_FILE_NAME", result.url.path) + assertTrue(result.url.query.contains("X-Amz-Expires=30")) + } + + @Test + fun testGetUrlWithStorageExceptionObjectNotFoundThrown() { + assertThrows(StorageException::class.java) { + synchronousStorage.getUrl( + StoragePath.fromString("SOME_UNKNOWN_FILE"), + AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(true).expires(30).build() + ) + } + } } 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 f792e608ae..e12f047d16 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 @@ -319,6 +319,8 @@ public StorageGetUrlOperation getUrl( @NonNull Consumer onError) { boolean useAccelerateEndpoint = options instanceof AWSS3StorageGetPresignedUrlOptions && ((AWSS3StorageGetPresignedUrlOptions) options).useAccelerateEndpoint(); + boolean validateObjectExistence = options instanceof AWSS3StorageGetPresignedUrlOptions && + ((AWSS3StorageGetPresignedUrlOptions) options).getValidateObjectExistence(); AWSS3StorageGetPresignedUrlRequest request = new AWSS3StorageGetPresignedUrlRequest( key, options.getAccessLevel() != null @@ -328,7 +330,8 @@ public StorageGetUrlOperation getUrl( options.getExpires() != 0 ? options.getExpires() : defaultUrlExpiration, - useAccelerateEndpoint + useAccelerateEndpoint, + validateObjectExistence ); AWSS3StorageGetPresignedUrlOperation operation = @@ -357,7 +360,7 @@ public StorageGetUrlOperation getUrl( ((AWSS3StorageGetPresignedUrlOptions) options).useAccelerateEndpoint(); boolean validateObjectExistence = options instanceof AWSS3StorageGetPresignedUrlOptions && - ((AWSS3StorageGetPresignedUrlOptions) options).validateObjectExistence(); + ((AWSS3StorageGetPresignedUrlOptions) options).getValidateObjectExistence(); AWSS3StoragePathGetPresignedUrlRequest request = new AWSS3StoragePathGetPresignedUrlRequest( path, diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java index 1d3f4eab4b..839eef996a 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java @@ -85,10 +85,20 @@ public void start() { prefix -> { try { String serviceKey = prefix.concat(getRequest().getKey()); + + if (getRequest().validateObjectExistence()) { + try { + storageService.validateObjectExists(serviceKey); + } catch (StorageException exception) { + onError.accept(exception); + return; + } + } + URL url = storageService.getPresignedUrl( - serviceKey, - getRequest().getExpires(), - getRequest().useAccelerateEndpoint()); + serviceKey, + getRequest().getExpires(), + getRequest().useAccelerateEndpoint()); onSuccess.accept(StorageGetUrlResult.fromUrl(url)); } catch (Exception exception) { onError.accept(new StorageException( 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 419208b602..5c204235fd 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 @@ -61,6 +61,8 @@ public static Builder from(@NonNull AWSS3StorageGetPresignedUrlOptions options) return builder() .accessLevel(options.getAccessLevel()) .targetIdentityId(options.getTargetIdentityId()) + .expires(options.getExpires()) + .setValidateObjectExistence(options.getValidateObjectExistence()) .expires(options.getExpires()); } @@ -88,7 +90,7 @@ public boolean useAccelerateEndpoint() { * of the underlying S3 object will likely require a round-trip network call. * @return boolean flag */ - public boolean validateObjectExistence() { + public boolean getValidateObjectExistence() { return validateObjectExistence; } @@ -103,7 +105,8 @@ public boolean equals(Object obj) { AWSS3StorageGetPresignedUrlOptions that = (AWSS3StorageGetPresignedUrlOptions) obj; return ObjectsCompat.equals(getAccessLevel(), that.getAccessLevel()) && ObjectsCompat.equals(getTargetIdentityId(), that.getTargetIdentityId()) && - ObjectsCompat.equals(getExpires(), that.getExpires()); + ObjectsCompat.equals(getExpires(), that.getExpires()) && + ObjectsCompat.equals(getValidateObjectExistence(), that.getValidateObjectExistence()); } } @@ -113,7 +116,8 @@ public int hashCode() { return ObjectsCompat.hash( getAccessLevel(), getTargetIdentityId(), - getExpires() + getExpires(), + getValidateObjectExistence() ); } @@ -125,6 +129,7 @@ public String toString() { "accessLevel=" + getAccessLevel() + ", targetIdentityId=" + getTargetIdentityId() + ", expires=" + getExpires() + + ", validateObjectExistence=" + getValidateObjectExistence() + '}'; } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java index 087f076c10..a193eb5544 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java @@ -33,6 +33,7 @@ public final class AWSS3StorageGetPresignedUrlRequest { private final String targetIdentityId; private final int expires; private final boolean useAccelerateEndpoint; + private final boolean validateObjectExistence; /** * Constructs a new AWSS3StorageGetUrlRequest. @@ -58,6 +59,37 @@ public AWSS3StorageGetPresignedUrlRequest( this.targetIdentityId = targetIdentityId; this.expires = expires; this.useAccelerateEndpoint = useAccelerateEndpoint; + this.validateObjectExistence = false; + } + + /** + * Constructs a new AWSS3StorageGetUrlRequest. + * Although this has public access, it is intended for internal use and should not be used directly by host + * applications. The behavior of this may change without warning. + * + * @param key key for item to obtain URL for + * @param accessLevel Storage access level + * @param targetIdentityId If set, this should override the current user's identity ID. + * If null, the operation will fetch the current identity ID. + * @param expires The number of seconds before the URL expires + * @param useAccelerateEndpoint Flag to enable acceleration mode + * @param validateObjectExistence Flag to validate if object exists in storage + */ + @SuppressWarnings("deprecation") + public AWSS3StorageGetPresignedUrlRequest( + @NonNull String key, + @NonNull StorageAccessLevel accessLevel, + @Nullable String targetIdentityId, + int expires, + boolean useAccelerateEndpoint, + boolean validateObjectExistence + ) { + this.key = key; + this.accessLevel = accessLevel; + this.targetIdentityId = targetIdentityId; + this.expires = expires; + this.useAccelerateEndpoint = useAccelerateEndpoint; + this.validateObjectExistence = validateObjectExistence; } /** @@ -104,5 +136,14 @@ public int getExpires() { public boolean useAccelerateEndpoint() { return useAccelerateEndpoint; } + + /** + * Gets the flag to determine whether to validate for object existence. + * + * @return boolean flag + */ + public boolean validateObjectExistence() { + return validateObjectExistence; + } } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java index de3209a2b5..00fd39ede0 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java @@ -20,6 +20,7 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.ObjectMetadata; +import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StorageItem; import com.amplifyframework.storage.result.StorageListResult; import com.amplifyframework.storage.s3.transfer.TransferObserver; @@ -41,8 +42,9 @@ public interface StorageService { * Throws StorageException if object is not does not exist. * * @param serviceKey key to uniquely specify item to generate URL for + * @throws StorageException If object does not exist in storage */ - void validateObjectExists(@NonNull String serviceKey); + void validateObjectExists(@NonNull String serviceKey) throws StorageException; /** * Generate pre-signed download URL for an object. diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt index 01777f6836..5460a0161c 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt @@ -26,6 +26,7 @@ import com.amplifyframework.storage.s3.service.StorageService import com.google.common.util.concurrent.MoreExecutors import io.mockk.coEvery import io.mockk.mockk +import io.mockk.verify import org.junit.Before import org.junit.Test import org.mockito.Mockito @@ -138,4 +139,81 @@ public class AWSS3StorageGetPresignedUrlOperationTest { awsS3StorageGetPresignedUrlOperation.start() Mockito.verify(storageService).getPresignedUrl(expectedKey, 1, false) } + + @Test + fun `getPresignedUrl fails with non existent S3 path when validateObjectExistence is enabled`() { + val key = "123" + val expectedKey = "public/123" + val request = AWSS3StorageGetPresignedUrlRequest( + key, + StorageAccessLevel.PUBLIC, + "", + 1, + false, + true + ) + val expectedException = StorageException("Test", "Test") + storageService = mockk(relaxed = true) + coEvery { storageService.validateObjectExists(any()) } throws expectedException + coEvery { authCredentialsProvider.getIdentityId() } returns "abc" + val onError = mockk>(relaxed = true) + awsS3StorageGetPresignedUrlOperation = AWSS3StorageGetPresignedUrlOperation( + storageService, + MoreExecutors.newDirectExecutorService(), + authCredentialsProvider, + request, + AWSS3StoragePluginConfiguration {}, + {}, + onError + ) + + // WHEN + awsS3StorageGetPresignedUrlOperation.start() + + // THEN + verify(exactly = 1) { onError.accept(expectedException) } + verify(exactly = 0) { + storageService.getPresignedUrl(any(), any(), any()) + } + } + + @Test + fun `getPresignedUrl succeeds when validateObjectExistence is enabled`() { + // GIVEN + val key = "123" + val expectedKey = "public/123" + val request = AWSS3StorageGetPresignedUrlRequest( + key, + StorageAccessLevel.PUBLIC, + "", + 1, + false, + true + ) + storageService = mockk(relaxed = true) + coEvery { authCredentialsProvider.getIdentityId() } returns "abc" + val onError = mockk>(relaxed = true) + awsS3StorageGetPresignedUrlOperation = AWSS3StorageGetPresignedUrlOperation( + storageService, + MoreExecutors.newDirectExecutorService(), + authCredentialsProvider, + request, + AWSS3StoragePluginConfiguration {}, + {}, + onError + ) + + // WHEN + awsS3StorageGetPresignedUrlOperation.start() + + // THEN + verify(exactly = 0) { onError.accept(any()) } + verify { + storageService.getPresignedUrl( + expectedKey, + 1, + false + ) + } + } } From bb7bcf7e97fefb9f3700a56304d6c4021ca49402 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Thu, 13 Jun 2024 12:03:44 -0500 Subject: [PATCH 05/13] update integ test --- .../amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index 2d0d778da2..95e7512216 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -98,7 +98,7 @@ class AWSS3StoragePathGetUrlTest { fun testGetUrlWithStorageExceptionObjectNotFoundThrown() { assertThrows(StorageException::class.java) { synchronousStorage.getUrl( - StoragePath.fromString("SOME_UNKNOWN_FILE"), + StoragePath.fromString("/public/SOME_UNKNOWN_FILE"), AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(true).expires(30).build() ) } From 4aece779c95bfefff4a71dac4b1c73d9fc86de86 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Thu, 13 Jun 2024 12:28:27 -0500 Subject: [PATCH 06/13] fix storage file path in integ test --- .../amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index 95e7512216..db114fb927 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -98,7 +98,7 @@ class AWSS3StoragePathGetUrlTest { fun testGetUrlWithStorageExceptionObjectNotFoundThrown() { assertThrows(StorageException::class.java) { synchronousStorage.getUrl( - StoragePath.fromString("/public/SOME_UNKNOWN_FILE"), + StoragePath.fromString("public/SOME_UNKNOWN_FILE"), AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(true).expires(30).build() ) } From 550cc8ced002ff0680594aac6412b8a5400a5713 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Mon, 17 Jun 2024 12:23:40 -0500 Subject: [PATCH 07/13] mark AWSS3StorageGetPresignedUrlRequest as internal api --- aws-storage-s3/api/aws-storage-s3.api | 11 ----------- .../request/AWSS3StorageGetPresignedUrlRequest.java | 2 ++ 2 files changed, 2 insertions(+), 11 deletions(-) diff --git a/aws-storage-s3/api/aws-storage-s3.api b/aws-storage-s3/api/aws-storage-s3.api index 96aa6229b1..ac324065f5 100644 --- a/aws-storage-s3/api/aws-storage-s3.api +++ b/aws-storage-s3/api/aws-storage-s3.api @@ -283,17 +283,6 @@ public final class com/amplifyframework/storage/s3/request/AWSS3StorageDownloadF public fun useAccelerateEndpoint ()Z } -public final class com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest { - public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZ)V - public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZZ)V - public fun getAccessLevel ()Lcom/amplifyframework/storage/StorageAccessLevel; - public fun getExpires ()I - public fun getKey ()Ljava/lang/String; - public fun getTargetIdentityId ()Ljava/lang/String; - public fun useAccelerateEndpoint ()Z - public fun validateObjectExistence ()Z -} - public final class com/amplifyframework/storage/s3/request/AWSS3StorageListRequest { public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;)V public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;ILjava/lang/String;)V diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java index a193eb5544..1ac068d04b 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java @@ -18,6 +18,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import com.amplifyframework.annotations.InternalAmplifyApi; import com.amplifyframework.storage.StorageAccessLevel; /** @@ -26,6 +27,7 @@ * @deprecated this class is only constructed internally through deprecated transfer methods. */ @Deprecated +@InternalAmplifyApi public final class AWSS3StorageGetPresignedUrlRequest { private final String key; @SuppressWarnings("deprecation") From a9a3d8548dd97470586ff6c258e593fb5f34e965 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 18 Jun 2024 13:06:12 -0500 Subject: [PATCH 08/13] fix lint error --- .../com/amplifyframework/storage/s3/AWSS3StoragePlugin.java | 1 + .../s3/operation/AWSS3StorageGetPresignedUrlOperation.java | 3 +++ 2 files changed, 4 insertions(+) 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 e12f047d16..71ce928fd3 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 @@ -309,6 +309,7 @@ public StorageGetUrlOperation getUrl( return getUrl(path, StorageGetUrlOptions.defaultInstance(), onSuccess, onError); } + @OptIn(markerClass = InternalAmplifyApi.class) @NonNull @Override @SuppressWarnings("deprecation") diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java index 839eef996a..1213f1c845 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java @@ -17,7 +17,9 @@ import android.annotation.SuppressLint; import androidx.annotation.NonNull; +import androidx.annotation.OptIn; +import com.amplifyframework.annotations.InternalAmplifyApi; import com.amplifyframework.auth.AuthCredentialsProvider; import com.amplifyframework.core.Consumer; import com.amplifyframework.storage.StorageException; @@ -76,6 +78,7 @@ public AWSS3StorageGetPresignedUrlOperation( } @SuppressLint("SyntheticAccessor") + @OptIn(markerClass = InternalAmplifyApi.class) @Override public void start() { executorService.submit(() -> { From 975772e2705bc4e5b8b31dbcb989bbfae15a3afc Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Fri, 28 Jun 2024 12:19:08 -0500 Subject: [PATCH 09/13] resolve review comments --- aws-storage-s3/api/aws-storage-s3.api | 11 ++++ .../storage/s3/AWSS3StoragePlugin.java | 1 - .../AWSS3StorageGetPresignedUrlOperation.java | 54 +++++++++++-------- ...WSS3StoragePathGetPresignedUrlOperation.kt | 13 +++-- .../AWSS3StorageGetPresignedUrlRequest.java | 2 - .../storage/s3/service/AWSS3StorageService.kt | 22 +++----- .../storage/s3/service/StorageService.java | 4 +- ...WSS3StorageGetPresignedUrlOperationTest.kt | 5 +- .../AWSS3StoragePathGetUrlOperationTest.kt | 5 +- 9 files changed, 66 insertions(+), 51 deletions(-) diff --git a/aws-storage-s3/api/aws-storage-s3.api b/aws-storage-s3/api/aws-storage-s3.api index ac324065f5..96aa6229b1 100644 --- a/aws-storage-s3/api/aws-storage-s3.api +++ b/aws-storage-s3/api/aws-storage-s3.api @@ -283,6 +283,17 @@ public final class com/amplifyframework/storage/s3/request/AWSS3StorageDownloadF public fun useAccelerateEndpoint ()Z } +public final class com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest { + public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZ)V + public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;IZZ)V + public fun getAccessLevel ()Lcom/amplifyframework/storage/StorageAccessLevel; + public fun getExpires ()I + public fun getKey ()Ljava/lang/String; + public fun getTargetIdentityId ()Ljava/lang/String; + public fun useAccelerateEndpoint ()Z + public fun validateObjectExistence ()Z +} + public final class com/amplifyframework/storage/s3/request/AWSS3StorageListRequest { public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;)V public fun (Ljava/lang/String;Lcom/amplifyframework/storage/StorageAccessLevel;Ljava/lang/String;ILjava/lang/String;)V 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 71ce928fd3..e12f047d16 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 @@ -309,7 +309,6 @@ public StorageGetUrlOperation getUrl( return getUrl(path, StorageGetUrlOptions.defaultInstance(), onSuccess, onError); } - @OptIn(markerClass = InternalAmplifyApi.class) @NonNull @Override @SuppressWarnings("deprecation") diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java index 1213f1c845..204ba82181 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java @@ -17,9 +17,7 @@ import android.annotation.SuppressLint; import androidx.annotation.NonNull; -import androidx.annotation.OptIn; -import com.amplifyframework.annotations.InternalAmplifyApi; import com.amplifyframework.auth.AuthCredentialsProvider; import com.amplifyframework.core.Consumer; import com.amplifyframework.storage.StorageException; @@ -32,14 +30,17 @@ import java.net.URL; import java.util.concurrent.ExecutorService; +import aws.sdk.kotlin.services.s3.model.NotFound; + /** - * An operation to retrieve pre-signed object URL from AWS S3. - * @deprecated Class should not be public and explicitly cast to. Cast to StorageGetUrlOperation. - * Internal usages are moving to AWSS3StoragePathGetPresignedUrlOperation + * An operation to retrieve pre-signed object URL from AWS S3. + * + * @deprecated Class should not be public and explicitly cast to. Cast to StorageGetUrlOperation. + * Internal usages are moving to AWSS3StoragePathGetPresignedUrlOperation */ @Deprecated public final class AWSS3StorageGetPresignedUrlOperation - extends StorageGetUrlOperation { + extends StorageGetUrlOperation { private final StorageService storageService; private final ExecutorService executorService; private final AuthCredentialsProvider authCredentialsProvider; @@ -60,13 +61,13 @@ public final class AWSS3StorageGetPresignedUrlOperation * @param onError Notified upon URL generation error */ public AWSS3StorageGetPresignedUrlOperation( - @NonNull StorageService storageService, - @NonNull ExecutorService executorService, - @NonNull AuthCredentialsProvider authCredentialsProvider, - @NonNull AWSS3StorageGetPresignedUrlRequest request, - @NonNull AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration, - @NonNull Consumer onSuccess, - @NonNull Consumer onError + @NonNull StorageService storageService, + @NonNull ExecutorService executorService, + @NonNull AuthCredentialsProvider authCredentialsProvider, + @NonNull AWSS3StorageGetPresignedUrlRequest request, + @NonNull AWSS3StoragePluginConfiguration awss3StoragePluginConfiguration, + @NonNull Consumer onSuccess, + @NonNull Consumer onError ) { super(request); this.storageService = storageService; @@ -78,11 +79,10 @@ public AWSS3StorageGetPresignedUrlOperation( } @SuppressLint("SyntheticAccessor") - @OptIn(markerClass = InternalAmplifyApi.class) @Override public void start() { executorService.submit(() -> { - awsS3StoragePluginConfiguration.getAWSS3PluginPrefixResolver(authCredentialsProvider). + awsS3StoragePluginConfiguration.getAWSS3PluginPrefixResolver(authCredentialsProvider). resolvePrefix(getRequest().getAccessLevel(), getRequest().getTargetIdentityId(), prefix -> { @@ -92,8 +92,19 @@ public void start() { if (getRequest().validateObjectExistence()) { try { storageService.validateObjectExists(serviceKey); - } catch (StorageException exception) { - onError.accept(exception); + } catch (NotFound nfe) { + onError.accept(new StorageException( + "Unable to generate URL for non-existent path: $serviceKey", + nfe, + "Please ensure the path is valid or the object has been uploaded." + )); + return; + } catch (Exception exception) { + onError.accept(new StorageException( + "Encountered an issue while validating the existence of object", + exception, + "See included exception for more details and suggestions to fix." + )); return; } } @@ -105,14 +116,13 @@ public void start() { onSuccess.accept(StorageGetUrlResult.fromUrl(url)); } catch (Exception exception) { onError.accept(new StorageException( - "Encountered an issue while generating pre-signed URL", - exception, - "See included exception for more details and suggestions to fix." + "Encountered an issue while generating pre-signed URL", + exception, + "See included exception for more details and suggestions to fix." )); } - }, - onError); + }, onError); } ); } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt index eb79293587..9d7e302c8c 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt @@ -14,6 +14,7 @@ */ package com.amplifyframework.storage.s3.operation +import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageException @@ -51,13 +52,19 @@ internal class AWSS3StoragePathGetPresignedUrlOperation( if (request.validateObjectExistence) { try { storageService.validateObjectExists(serviceKey) - } catch (se: StorageException) { - onError.accept(se) + } catch (nfe: NotFound) { + onError.accept( + StorageException( + "Unable to generate URL for non-existent path: $serviceKey", + nfe, + "Please ensure the path is valid or the object has been uploaded." + ) + ) return@submit } catch (exception: Exception) { onError.accept( StorageException( - "Encountered an issue while generating pre-signed URL", + "Encountered an issue while validating the existence of object", exception, "See included exception for more details and suggestions to fix." ) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java index 1ac068d04b..a193eb5544 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/request/AWSS3StorageGetPresignedUrlRequest.java @@ -18,7 +18,6 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.amplifyframework.annotations.InternalAmplifyApi; import com.amplifyframework.storage.StorageAccessLevel; /** @@ -27,7 +26,6 @@ * @deprecated this class is only constructed internally through deprecated transfer methods. */ @Deprecated -@InternalAmplifyApi public final class AWSS3StorageGetPresignedUrlRequest { private final String key; @SuppressWarnings("deprecation") 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 b4d733a730..822069c491 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 @@ -21,13 +21,11 @@ import aws.sdk.kotlin.services.s3.deleteObject import aws.sdk.kotlin.services.s3.listObjectsV2 import aws.sdk.kotlin.services.s3.model.GetObjectRequest import aws.sdk.kotlin.services.s3.model.HeadObjectRequest -import aws.sdk.kotlin.services.s3.model.NotFound import aws.sdk.kotlin.services.s3.paginators.listObjectsV2Paginated import aws.sdk.kotlin.services.s3.presigners.presignGetObject import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.storage.ObjectMetadata -import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.StorageItem import com.amplifyframework.storage.result.StorageListResult import com.amplifyframework.storage.s3.transfer.TransferManager @@ -94,20 +92,12 @@ internal class AWSS3StorageService( * @param serviceKey S3 service key */ override fun validateObjectExists(serviceKey: String) { - try { - runBlocking { - s3Client.headObject( - HeadObjectRequest { - bucket = s3BucketName - key = serviceKey - } - ) - } - } catch (ex: NotFound) { - throw StorageException( - "Unable to generate URL for non-existent path: $serviceKey", - ex, - "Please ensure the path is valid or the object has been uploaded" + runBlocking { + s3Client.headObject( + HeadObjectRequest { + bucket = s3BucketName + key = serviceKey + } ) } } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java index 00fd39ede0..de3209a2b5 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java @@ -20,7 +20,6 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.ObjectMetadata; -import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StorageItem; import com.amplifyframework.storage.result.StorageListResult; import com.amplifyframework.storage.s3.transfer.TransferObserver; @@ -42,9 +41,8 @@ public interface StorageService { * Throws StorageException if object is not does not exist. * * @param serviceKey key to uniquely specify item to generate URL for - * @throws StorageException If object does not exist in storage */ - void validateObjectExists(@NonNull String serviceKey) throws StorageException; + void validateObjectExists(@NonNull String serviceKey); /** * Generate pre-signed download URL for an object. diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt index 5460a0161c..bd7b6b6ce5 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt @@ -15,6 +15,7 @@ package com.amplifyframework.storage.s3.operation import android.util.Log +import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageAccessLevel @@ -152,7 +153,7 @@ public class AWSS3StorageGetPresignedUrlOperationTest { false, true ) - val expectedException = StorageException("Test", "Test") + val expectedException = NotFound({}) storageService = mockk(relaxed = true) coEvery { storageService.validateObjectExists(any()) } throws expectedException coEvery { authCredentialsProvider.getIdentityId() } returns "abc" @@ -171,7 +172,7 @@ public class AWSS3StorageGetPresignedUrlOperationTest { awsS3StorageGetPresignedUrlOperation.start() // THEN - verify(exactly = 1) { onError.accept(expectedException) } + verify(exactly = 1) { onError.accept(any()) } verify(exactly = 0) { storageService.getPresignedUrl(any(), any(), any()) } diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt index 40cabddf26..490b1af990 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt @@ -15,6 +15,7 @@ package com.amplifyframework.storage.s3.operation +import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageException @@ -221,7 +222,7 @@ class AWSS3StoragePathGetUrlOperationTest { fun `getPresignedUrl fails with non existent S3 path when validateObjectExistence is enabled`() { // GIVEN val path = StoragePath.fromString("public/123") - val expectedException = StorageException("Test", "Test") + val expectedException = NotFound({}) coEvery { storageService.validateObjectExists(any()) } throws expectedException val request = AWSS3StoragePathGetPresignedUrlRequest( path, @@ -243,7 +244,7 @@ class AWSS3StoragePathGetUrlOperationTest { awsS3StorageGetPresignedUrlOperation.start() // THEN - verify(exactly = 1) { onError.accept(expectedException) } + verify(exactly = 1) { onError.accept(any()) } verify(exactly = 0) { storageService.getPresignedUrl(any(), any(), any()) } From b18c4e439fef31f5f4499e648ee493e7e6297db2 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Mon, 1 Jul 2024 10:27:08 -0500 Subject: [PATCH 10/13] refactor exception handling isolate NotFound exception to the Storage service --- .../AWSS3StorageGetPresignedUrlOperation.java | 26 +++++-------------- ...WSS3StoragePathGetPresignedUrlOperation.kt | 11 ++------ .../storage/s3/service/AWSS3StorageService.kt | 22 +++++++++++----- .../storage/s3/service/StorageService.java | 4 ++- ...WSS3StorageGetPresignedUrlOperationTest.kt | 5 ++-- .../AWSS3StoragePathGetUrlOperationTest.kt | 5 ++-- 6 files changed, 32 insertions(+), 41 deletions(-) diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java index 204ba82181..295413985a 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperation.java @@ -30,8 +30,6 @@ import java.net.URL; import java.util.concurrent.ExecutorService; -import aws.sdk.kotlin.services.s3.model.NotFound; - /** * An operation to retrieve pre-signed object URL from AWS S3. * @@ -92,19 +90,8 @@ public void start() { if (getRequest().validateObjectExistence()) { try { storageService.validateObjectExists(serviceKey); - } catch (NotFound nfe) { - onError.accept(new StorageException( - "Unable to generate URL for non-existent path: $serviceKey", - nfe, - "Please ensure the path is valid or the object has been uploaded." - )); - return; - } catch (Exception exception) { - onError.accept(new StorageException( - "Encountered an issue while validating the existence of object", - exception, - "See included exception for more details and suggestions to fix." - )); + } catch (StorageException exception) { + onError.accept(exception); return; } } @@ -116,13 +103,14 @@ public void start() { onSuccess.accept(StorageGetUrlResult.fromUrl(url)); } catch (Exception exception) { onError.accept(new StorageException( - "Encountered an issue while generating pre-signed URL", - exception, - "See included exception for more details and suggestions to fix." + "Encountered an issue while generating pre-signed URL", + exception, + "See included exception for more details and suggestions to fix." )); } - }, onError); + }, + onError); } ); } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt index 9d7e302c8c..63862ca3f6 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetPresignedUrlOperation.kt @@ -14,7 +14,6 @@ */ package com.amplifyframework.storage.s3.operation -import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageException @@ -52,14 +51,8 @@ internal class AWSS3StoragePathGetPresignedUrlOperation( if (request.validateObjectExistence) { try { storageService.validateObjectExists(serviceKey) - } catch (nfe: NotFound) { - onError.accept( - StorageException( - "Unable to generate URL for non-existent path: $serviceKey", - nfe, - "Please ensure the path is valid or the object has been uploaded." - ) - ) + } catch (se: StorageException) { + onError.accept(se) return@submit } catch (exception: Exception) { onError.accept( 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 822069c491..b4d733a730 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 @@ -21,11 +21,13 @@ import aws.sdk.kotlin.services.s3.deleteObject import aws.sdk.kotlin.services.s3.listObjectsV2 import aws.sdk.kotlin.services.s3.model.GetObjectRequest import aws.sdk.kotlin.services.s3.model.HeadObjectRequest +import aws.sdk.kotlin.services.s3.model.NotFound import aws.sdk.kotlin.services.s3.paginators.listObjectsV2Paginated import aws.sdk.kotlin.services.s3.presigners.presignGetObject import aws.sdk.kotlin.services.s3.withConfig import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.storage.ObjectMetadata +import com.amplifyframework.storage.StorageException import com.amplifyframework.storage.StorageItem import com.amplifyframework.storage.result.StorageListResult import com.amplifyframework.storage.s3.transfer.TransferManager @@ -92,12 +94,20 @@ internal class AWSS3StorageService( * @param serviceKey S3 service key */ override fun validateObjectExists(serviceKey: String) { - runBlocking { - s3Client.headObject( - HeadObjectRequest { - bucket = s3BucketName - key = serviceKey - } + try { + runBlocking { + s3Client.headObject( + HeadObjectRequest { + bucket = s3BucketName + key = serviceKey + } + ) + } + } catch (ex: NotFound) { + throw StorageException( + "Unable to generate URL for non-existent path: $serviceKey", + ex, + "Please ensure the path is valid or the object has been uploaded" ) } } diff --git a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java index de3209a2b5..00fd39ede0 100644 --- a/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java +++ b/aws-storage-s3/src/main/java/com/amplifyframework/storage/s3/service/StorageService.java @@ -20,6 +20,7 @@ import androidx.annotation.Nullable; import com.amplifyframework.storage.ObjectMetadata; +import com.amplifyframework.storage.StorageException; import com.amplifyframework.storage.StorageItem; import com.amplifyframework.storage.result.StorageListResult; import com.amplifyframework.storage.s3.transfer.TransferObserver; @@ -41,8 +42,9 @@ public interface StorageService { * Throws StorageException if object is not does not exist. * * @param serviceKey key to uniquely specify item to generate URL for + * @throws StorageException If object does not exist in storage */ - void validateObjectExists(@NonNull String serviceKey); + void validateObjectExists(@NonNull String serviceKey) throws StorageException; /** * Generate pre-signed download URL for an object. diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt index bd7b6b6ce5..5460a0161c 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StorageGetPresignedUrlOperationTest.kt @@ -15,7 +15,6 @@ package com.amplifyframework.storage.s3.operation import android.util.Log -import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageAccessLevel @@ -153,7 +152,7 @@ public class AWSS3StorageGetPresignedUrlOperationTest { false, true ) - val expectedException = NotFound({}) + val expectedException = StorageException("Test", "Test") storageService = mockk(relaxed = true) coEvery { storageService.validateObjectExists(any()) } throws expectedException coEvery { authCredentialsProvider.getIdentityId() } returns "abc" @@ -172,7 +171,7 @@ public class AWSS3StorageGetPresignedUrlOperationTest { awsS3StorageGetPresignedUrlOperation.start() // THEN - verify(exactly = 1) { onError.accept(any()) } + verify(exactly = 1) { onError.accept(expectedException) } verify(exactly = 0) { storageService.getPresignedUrl(any(), any(), any()) } diff --git a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt index 490b1af990..40cabddf26 100644 --- a/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt +++ b/aws-storage-s3/src/test/java/com/amplifyframework/storage/s3/operation/AWSS3StoragePathGetUrlOperationTest.kt @@ -15,7 +15,6 @@ package com.amplifyframework.storage.s3.operation -import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.AuthCredentialsProvider import com.amplifyframework.core.Consumer import com.amplifyframework.storage.StorageException @@ -222,7 +221,7 @@ class AWSS3StoragePathGetUrlOperationTest { fun `getPresignedUrl fails with non existent S3 path when validateObjectExistence is enabled`() { // GIVEN val path = StoragePath.fromString("public/123") - val expectedException = NotFound({}) + val expectedException = StorageException("Test", "Test") coEvery { storageService.validateObjectExists(any()) } throws expectedException val request = AWSS3StoragePathGetPresignedUrlRequest( path, @@ -244,7 +243,7 @@ class AWSS3StoragePathGetUrlOperationTest { awsS3StorageGetPresignedUrlOperation.start() // THEN - verify(exactly = 1) { onError.accept(any()) } + verify(exactly = 1) { onError.accept(expectedException) } verify(exactly = 0) { storageService.getPresignedUrl(any(), any(), any()) } From 52606df1aed18d58478d04279cac6d0f7d57719d Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 2 Jul 2024 12:09:27 -0500 Subject: [PATCH 11/13] update s3 getURL integration test --- .../storage/s3/AWSS3StoragePathGetUrlTest.kt | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index db114fb927..ae8f23868a 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -16,6 +16,7 @@ package com.amplifyframework.storage.s3 import android.content.Context import androidx.test.core.app.ApplicationProvider +import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.cognito.AWSCognitoAuthPlugin import com.amplifyframework.storage.StorageCategory import com.amplifyframework.storage.StorageException @@ -96,11 +97,13 @@ class AWSS3StoragePathGetUrlTest { @Test fun testGetUrlWithStorageExceptionObjectNotFoundThrown() { - assertThrows(StorageException::class.java) { + val exception = assertThrows(StorageException::class.java) { synchronousStorage.getUrl( StoragePath.fromString("public/SOME_UNKNOWN_FILE"), AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(true).expires(30).build() ) } + + assertTrue(exception.cause is NotFound) } } From 077381670a9efe6d18dcf859a2aa83c365582075 Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Tue, 2 Jul 2024 16:06:33 -0500 Subject: [PATCH 12/13] add additional test case for S3 getURL --- .../storage/s3/AWSS3StoragePathGetUrlTest.kt | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index ae8f23868a..92e1b1aca1 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -15,6 +15,7 @@ package com.amplifyframework.storage.s3 import android.content.Context +import android.util.Log import androidx.test.core.app.ApplicationProvider import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.cognito.AWSCognitoAuthPlugin @@ -35,6 +36,7 @@ import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.BeforeClass import org.junit.Test +import java.net.URL /** * Instrumentation test for operational work on download. @@ -106,4 +108,19 @@ class AWSS3StoragePathGetUrlTest { assertTrue(exception.cause is NotFound) } + + @Test + fun testGetUrlWithObjectExistenceValidationDisabledForNonExistentObject() { + val result = synchronousStorage.getUrl( + StoragePath.fromString("public/SOME_UNKNOWN_FILE"), + AWSS3StorageGetPresignedUrlOptions.builder().setValidateObjectExistence(false).expires(30).build() + ) + + assertEquals("/public/SOME_UNKNOWN_FILE", result.url.path) + assertTrue(result.url.query.contains("X-Amz-Expires=30")) + + assertThrows(java.io.FileNotFoundException::class.java) { + result.url.readBytes() + } + } } From 47d43d6087ae3813ecbbe4742485314a5afe597f Mon Sep 17 00:00:00 2001 From: Tuan Pham Date: Wed, 3 Jul 2024 10:09:45 -0500 Subject: [PATCH 13/13] fix lint errors --- .../amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt | 2 -- 1 file changed, 2 deletions(-) diff --git a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt index 92e1b1aca1..ab250b466c 100644 --- a/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt +++ b/aws-storage-s3/src/androidTest/java/com/amplifyframework/storage/s3/AWSS3StoragePathGetUrlTest.kt @@ -15,7 +15,6 @@ package com.amplifyframework.storage.s3 import android.content.Context -import android.util.Log import androidx.test.core.app.ApplicationProvider import aws.sdk.kotlin.services.s3.model.NotFound import com.amplifyframework.auth.cognito.AWSCognitoAuthPlugin @@ -36,7 +35,6 @@ import org.junit.Assert.assertThrows import org.junit.Assert.assertTrue import org.junit.BeforeClass import org.junit.Test -import java.net.URL /** * Instrumentation test for operational work on download.