feat(storage): update storage upload APIs to use storage path (#3574)

aws-amplify · Apr 16, 2024 · 3101ca1 · 3101ca1
1 parent e185207
commit 3101ca1
Show file tree

Hide file tree

Showing 14 changed files with 613 additions and 35 deletions.
diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift
@@ -21,22 +21,23 @@ public struct StorageDownloadDataRequest: AmplifyOperationRequest {
     /// The unique identifier for the object in storage
     ///
     /// - Tag: StorageDownloadDataRequest.key
-    @available(*, deprecated, message: "Use `StoragePath` instead")
+    @available(*, deprecated, message: "Use `path` instead of `key`")
     public let key: String
 
     /// Options to adjust the behavior of this request, including plugin-options
     ///
     /// - Tag: StorageDownloadDataRequest.options
     public let options: Options
 
-    /// - Tag: StorageDownloadDataRequest.key
+    /// - Tag: StorageDownloadDataRequest.init
     @available(*, deprecated, message: "Use init(path:local:options)")
     public init(key: String, options: Options) {
         self.key = key
         self.options = options
         self.path = nil
     }
 
+    /// - Tag: StorageDownloadDataRequest.init
     public init(path: any StoragePath, options: Options) {
         self.key = ""
         self.options = options
@@ -87,12 +88,21 @@ public extension StorageDownloadDataRequest {
 
         ///
         /// - Tag: StorageDownloadDataRequestOptions.init
+        @available(*, deprecated, message: "Use init(pluginOptions)")
         public init(accessLevel: StorageAccessLevel = .guest,
                     targetIdentityId: String? = nil,
                     pluginOptions: Any? = nil) {
             self.accessLevel = accessLevel
             self.targetIdentityId = targetIdentityId
             self.pluginOptions = pluginOptions
         }
+
+        ///
+        /// - Tag: StorageDownloadDataRequestOptions.init
+        public init(pluginOptions: Any? = nil) {
+            self.accessLevel = .guest
+            self.targetIdentityId = nil
+            self.pluginOptions = pluginOptions
+        }
     }
 }
diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift
@@ -21,7 +21,7 @@ public struct StorageDownloadFileRequest: AmplifyOperationRequest {
     /// The unique identifier for the object in storage
     ///
     /// - Tag: StorageDownloadFileRequest.key
-    @available(*, deprecated, message: "Use `StoragePath` instead")
+    @available(*, deprecated, message: "Use `path` instead of `key`")
     public let key: String
 
     /// The local file to download the object to
@@ -43,6 +43,7 @@ public struct StorageDownloadFileRequest: AmplifyOperationRequest {
         self.path = nil
     }
 
+    /// - Tag: StorageDownloadFileRequest.init
     public init(path: any StoragePath, local: URL, options: Options) {
         self.key = ""
         self.local = local
@@ -76,12 +77,21 @@ public extension StorageDownloadFileRequest {
         public let pluginOptions: Any?
 
         /// - Tag: StorageDownloadFileRequestOptions.init
+        @available(*, deprecated, message: "Use init(pluginOptions)")
         public init(accessLevel: StorageAccessLevel = .guest,
                     targetIdentityId: String? = nil,
                     pluginOptions: Any? = nil) {
             self.accessLevel = accessLevel
             self.targetIdentityId = targetIdentityId
             self.pluginOptions = pluginOptions
         }
+
+        /// - Tag: StorageDownloadFileRequestOptions.init
+        @available(*, deprecated, message: "Use init(pluginOptions)")
+        public init(pluginOptions: Any? = nil) {
+            self.accessLevel = .guest
+            self.targetIdentityId = nil
+            self.pluginOptions = pluginOptions
+        }
     }
 }
diff --git a/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift
@@ -13,9 +13,15 @@ import Foundation
 /// - Tag: StorageUploadDataRequest
 public struct StorageUploadDataRequest: AmplifyOperationRequest {
 
+    /// The path for the object in storage
+    ///
+    /// - Tag: StorageDownloadFileRequest.path
+    public let path: (any StoragePath)?
+
     /// The unique identifier for the object in storage
     ///
     /// - Tag: StorageUploadDataRequest.key
+    @available(*, deprecated, message: "Use `path` instead of `key`")
     public let key: String
 
     /// The data in memory to be uploaded
@@ -29,10 +35,19 @@ public struct StorageUploadDataRequest: AmplifyOperationRequest {
     public let options: Options
 
     /// - Tag: StorageUploadDataRequest.init
+    @available(*, deprecated, message: "Use init(path:data:options)")
     public init(key: String, data: Data, options: Options) {
         self.key = key
         self.data = data
         self.options = options
+        self.path = nil
+    }
+
+    public init(path: any StoragePath, data: Data, options: Options) {
+        self.key = ""
+        self.data = data
+        self.options = options
+        self.path = path
     }
 }
 
@@ -71,16 +86,30 @@ public extension StorageUploadDataRequest {
         public let pluginOptions: Any?
 
         /// - Tag: StorageUploadDataRequestOptions.init
+        @available(*, deprecated, message: "Use init(metadata:contentType:options)")
         public init(accessLevel: StorageAccessLevel = .guest,
                     targetIdentityId: String? = nil,
                     metadata: [String: String]? = nil,
                     contentType: String? = nil,
-                    pluginOptions: Any? = nil) {
+                    pluginOptions: Any? = nil
+        ) {
             self.accessLevel = accessLevel
             self.targetIdentityId = targetIdentityId
             self.metadata = metadata
             self.contentType = contentType
             self.pluginOptions = pluginOptions
         }
+
+        /// - Tag: StorageUploadDataRequestOptions.init
+        public init(metadata: [String: String]? = nil,
+                    contentType: String? = nil,
+                    pluginOptions: Any? = nil
+        ) {
+            self.accessLevel = .guest
+            self.targetIdentityId = nil
+            self.metadata = metadata
+            self.contentType = contentType
+            self.pluginOptions = pluginOptions
+        }
     }
 }
diff --git a/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift
@@ -13,8 +13,14 @@ import Foundation
 /// - Tag: StorageUploadFileRequest
 public struct StorageUploadFileRequest: AmplifyOperationRequest {
 
+    /// The path for the object in storage
+    ///
+    /// - Tag: StorageDownloadFileRequest.path
+    public let path: (any StoragePath)?
+
     /// The unique identifier for the object in storage
     /// - Tag: StorageUploadFileRequest.key
+    @available(*, deprecated, message: "Use `path` instead of `key`")
     public let key: String
 
     /// The file to be uploaded
@@ -26,10 +32,19 @@ public struct StorageUploadFileRequest: AmplifyOperationRequest {
     public let options: Options
 
     /// - Tag: StorageUploadFileRequest.init
+    @available(*, deprecated, message: "Use init(path:local:options)")
     public init(key: String, local: URL, options: Options) {
         self.key = key
         self.local = local
         self.options = options
+        self.path = nil
+    }
+
+    public init(path: any StoragePath, local: URL, options: Options) {
+        self.key = ""
+        self.local = local
+        self.options = options
+        self.path = path
     }
 }
 
@@ -68,16 +83,30 @@ public extension StorageUploadFileRequest {
         public let pluginOptions: Any?
 
         /// - Tag: StorageUploadFileRequestOptions.init
+        @available(*, deprecated, message: "Use init(metadata:contentType:pluginOptions)")
         public init(accessLevel: StorageAccessLevel = .guest,
                     targetIdentityId: String? = nil,
                     metadata: [String: String]? = nil,
                     contentType: String? = nil,
-                    pluginOptions: Any? = nil) {
+                    pluginOptions: Any? = nil
+        ) {
             self.accessLevel = accessLevel
             self.targetIdentityId = targetIdentityId
             self.metadata = metadata
             self.contentType = contentType
             self.pluginOptions = pluginOptions
         }
+
+        /// - Tag: StorageUploadFileRequestOptions.init
+        public init(metadata: [String: String]? = nil,
+                    contentType: String? = nil,
+                    pluginOptions: Any? = nil
+        ) {
+            self.accessLevel = .guest
+            self.targetIdentityId = nil
+            self.metadata = metadata
+            self.contentType = contentType
+            self.pluginOptions = pluginOptions
+        }
     }
 }
diff --git a/...lugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift b/...lugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift
@@ -84,13 +84,21 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation<
             return
         }
 
-        let prefixResolver = storageConfiguration.prefixResolver ??
-        StorageAccessLevelAwarePrefixResolver(authService: authService)
+
 
         Task {
             do {
-                let prefix = try await prefixResolver.resolvePrefix(for: request.options.accessLevel, targetIdentityId: request.options.targetIdentityId)
-                let serviceKey = prefix + request.key
+
+                let serviceKey: String
+                if let path = request.path {
+                    serviceKey = try await path.resolvePath(authService: self.authService)
+                } else {
+                    let prefixResolver = storageConfiguration.prefixResolver ??
+                    StorageAccessLevelAwarePrefixResolver(authService: authService)
+                    let prefix = try await prefixResolver.resolvePrefix(for: request.options.accessLevel, targetIdentityId: request.options.targetIdentityId)
+                    serviceKey = prefix + request.key
+                }
+
                 let accelerate = try AWSS3PluginOptions.accelerateValue(pluginOptions: request.options.pluginOptions)
                 if request.data.count > StorageUploadDataRequest.Options.multiPartUploadSizeThreshold {
                     storageService.multiPartUpload(

diff --git a/...lugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift b/...lugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift
@@ -108,13 +108,19 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation<
             return
         }
 
-        let prefixResolver = storageConfiguration.prefixResolver ??
-        StorageAccessLevelAwarePrefixResolver(authService: authService)
-
         Task {
             do {
-                let prefix = try await prefixResolver.resolvePrefix(for: request.options.accessLevel, targetIdentityId: request.options.targetIdentityId)
-                let serviceKey = prefix + request.key
+
+                let serviceKey: String
+                if let path = request.path {
+                    serviceKey = try await path.resolvePath(authService: self.authService)
+                } else {
+                    let prefixResolver = storageConfiguration.prefixResolver ??
+                    StorageAccessLevelAwarePrefixResolver(authService: authService)
+                    let prefix = try await prefixResolver.resolvePrefix(for: request.options.accessLevel, targetIdentityId: request.options.targetIdentityId)
+                    serviceKey = prefix + request.key
+                }
+
                 let accelerate = try AWSS3PluginOptions.accelerateValue(pluginOptions: request.options.pluginOptions)
                 if uploadSize > StorageUploadFileRequest.Options.multiPartUploadSizeThreshold {
                     storageService.multiPartUpload(

diff --git a/...lugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadDataRequest+Validate.swift b/...lugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadDataRequest+Validate.swift
@@ -11,6 +11,12 @@ import Amplify
 extension StorageUploadDataRequest {
     /// Performs client side validation and returns a `StorageError` for any validation failures.
     func validate() -> StorageError? {
+        guard path == nil else {
+            // return nil here StoragePath are validated
+            // at during execution of request operation where the path is resolved
+            return nil
+        }
+
         if let error = StorageRequestUtils.validateKey(key) {
             return error
         }

diff --git a/...lugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadFileRequest+Validate.swift b/...lugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadFileRequest+Validate.swift
@@ -11,6 +11,12 @@ import Amplify
 extension StorageUploadFileRequest {
     /// Performs client side validation and returns a `StorageError` for any validation failures.
     func validate() -> StorageError? {
+        guard path == nil else {
+            // return nil here StoragePath are validated
+            // at during execution of request operation where the path is resolved
+            return nil
+        }
+
         if let error = StorageRequestUtils.validateKey(key) {
             return error
         }

diff --git a/...rage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/...rage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift
@@ -185,7 +185,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase {
     /// Given: Storage Download File Operation
     /// When: The operation is executed with a request that has an invalid StringStoragePath
     /// Then: The operation will fail with a validation error
-    func testDownloadDataOperationStringStoragePathValidationError() {
+    func testDownloadFileOperationStringStoragePathValidationError() {
         let path = StringStoragePath(resolve: { _ in return "my/path" })
         let request = StorageDownloadFileRequest(path: path,
                                                  local: testURL,
@@ -217,7 +217,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase {
     /// Given: Storage Download File Operation
     /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath
     /// Then: The operation will fail with a validation error
-    func testDownloadDataOperationIdentityIDStoragePathValidationError() {
+    func testDownloadFileOperationIdentityIDStoragePathValidationError() {
         let path = IdentityIDStoragePath(resolve: { _ in return "my/path" })
         let request = StorageDownloadFileRequest(path: path,
                                                  local: testURL,
@@ -249,7 +249,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase {
     /// Given: Storage Download File Operation
     /// When: The operation is executed with a request that has an a custom implementation of StoragePath
     /// Then: The operation will fail with a validation error
-    func testDownloadDataOperationCustomStoragePathValidationError() {
+    func testDownloadFileOperationCustomStoragePathValidationError() {
         let path = InvalidCustomStoragePath(resolve: { _ in return "my/path" })
         let request = StorageDownloadFileRequest(path: path,
                                                  local: testURL,
@@ -320,8 +320,9 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase {
     /// Given: Storage Download File Operation
     /// When: The operation is executed with a request that has an valid IdentityIDStoragePath
     /// Then: The operation will succeed
-    func testDownloadDataOperationWithIdentityIDStoragePathSucceeds() async throws {
-        let path = IdentityIDStoragePath(resolve: { _ in return "/public/\(self.testKey)" })
+    func testDownloadFileOperationWithIdentityIDStoragePathSucceeds() async throws {
+        mockAuthService.identityId = testIdentityId
+        let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" })
         let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key")
         mockStorageService.storageServiceDownloadEvents = [
             StorageEvent.initiated(StorageTaskReference(task)),
@@ -353,7 +354,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase {
 
         await waitForExpectations(timeout: 1)
         XCTAssertTrue(operation.isFinished)
-        mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: url)
+        mockStorageService.verifyDownload(serviceKey: "/public/\(testIdentityId)/\(self.testKey)", fileURL: url)
     }
 
     // TODO: missing unit tests for pause resume and cancel. do we create a mock of the StorageTaskReference?