From 0d13d9f3408f3e779899f7a0c3a15ffe6b57baff Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Mon, 11 Mar 2024 14:33:40 -0500 Subject: [PATCH 01/12] feat(storage): add new storage gen2 APIs (#3559) --- .../Request/StorageDownloadDataRequest.swift | 2 + .../Request/StorageDownloadFileRequest.swift | 2 + .../Request/StorageGetURLRequest.swift | 2 + .../Request/StorageListRequest.swift | 3 + .../Request/StorageRemoveRequest.swift | 1 + .../Request/StorageUploadDataRequest.swift | 2 + .../Request/StorageUploadFileRequest.swift | 2 + .../Storage/Result/StorageListResult.swift | 35 ++++- .../Storage/StorageAccessLevel.swift | 1 + .../StorageCategory+ClientBehavior.swift | 59 +++++++ .../Storage/StorageCategoryBehavior.swift | 147 ++++++++++++++++-- Amplify/Categories/Storage/StoragePath.swift | 44 ++++++ ...SS3StoragePlugin+AsyncClientBehavior.swift | 139 +++++++++++++++++ .../AWSS3StoragePlugin.swift | 1 + .../AWSS3PluginPrefixResolver.swift | 3 + .../AWSS3StoragePluginConfiguration.swift | 2 + .../Mocks/MockStorageCategoryPlugin.swift | 62 ++++++++ .../Hub/AmplifyOperationHubTests.swift | 87 +++++++++++ 18 files changed, 575 insertions(+), 19 deletions(-) create mode 100644 Amplify/Categories/Storage/StoragePath.swift diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift index f4f7ee0b83..2c550b6b6d 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift @@ -40,11 +40,13 @@ public extension StorageDownloadDataRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageDownloadDataRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// /// - Tag: StorageDownloadDataRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Extra plugin specific options, only used in special circumstances when the existing options do not provide diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift index 78d7be6ffd..4a665444dc 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift @@ -46,11 +46,13 @@ public extension StorageDownloadFileRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageDownloadFileRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// /// - Tag: StorageDownloadFileRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Extra plugin specific options, only used in special circumstances when the existing options do not provide diff --git a/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift index 25c4563098..453bbf847a 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift @@ -43,11 +43,13 @@ public extension StorageGetURLRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageListRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// /// - Tag: StorageListRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Number of seconds before the URL expires. Defaults to diff --git a/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift index d82d4f1718..5689e6b47c 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift @@ -32,16 +32,19 @@ public extension StorageListRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageListRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on /// /// - Tag: StorageListRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Path to the keys /// /// - Tag: StorageListRequestOptions.path + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let path: String? /// Number between 1 and 1,000 that indicates the limit of how many entries to fetch when diff --git a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift index 91492a3c70..ace12f1218 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift @@ -38,6 +38,7 @@ public extension StorageRemoveRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageRemoveRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Extra plugin specific options, only used in special circumstances when the existing options do not provide diff --git a/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift index 2f892b936d..522f34192b 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageUploadDataRequest.swift @@ -46,11 +46,13 @@ public extension StorageUploadDataRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageUploadDataRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// /// - Tag: StorageUploadDataRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Metadata for the object to store diff --git a/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift index 9382776c5b..43b786cf2c 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageUploadFileRequest.swift @@ -43,11 +43,13 @@ public extension StorageUploadFileRequest { /// Access level of the storage system. Defaults to `public` /// /// - Tag: StorageUploadFileRequestOptions.accessLevel + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// /// - Tag: StorageUploadFileRequestOptions.targetIdentityId + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Metadata for the object to store diff --git a/Amplify/Categories/Storage/Result/StorageListResult.swift b/Amplify/Categories/Storage/Result/StorageListResult.swift index f01a517c44..45777ae086 100644 --- a/Amplify/Categories/Storage/Result/StorageListResult.swift +++ b/Amplify/Categories/Storage/Result/StorageListResult.swift @@ -42,6 +42,11 @@ extension StorageListResult { /// - Tag: StorageListResultItem public struct Item { + /// The path of the object in storage. + /// + /// - Tag: StorageListResultItem.path + public let path: StoragePath + /// The unique identifier of the object in storage. /// /// - Tag: StorageListResultItem.key @@ -72,11 +77,31 @@ extension StorageListResult { /// [StorageCategoryBehavior.list](x-source-tag://StorageCategoryBehavior.list). /// /// - Tag: StorageListResultItem.init - public init(key: String, - size: Int? = nil, - eTag: String? = nil, - lastModified: Date? = nil, - pluginResults: Any? = nil) { + @available(*, deprecated, message: "Use init(path:key:size:lastModifiedDate:eTag:pluginResults)") + public init( + key: String, + size: Int? = nil, + eTag: String? = nil, + lastModified: Date? = nil, + pluginResults: Any? = nil + ) { + self.key = key + self.size = size + self.eTag = eTag + self.lastModified = lastModified + self.pluginResults = pluginResults + self.path = StringStoragePath(pathResolver: { _ in return "" }) + } + + public init( + path: StoragePath, + key: String, + size: Int? = nil, + eTag: String? = nil, + lastModified: Date? = nil, + pluginResults: Any? = nil + ) { + self.path = path self.key = key self.size = size self.eTag = eTag diff --git a/Amplify/Categories/Storage/StorageAccessLevel.swift b/Amplify/Categories/Storage/StorageAccessLevel.swift index 8319818bc8..726effc9be 100644 --- a/Amplify/Categories/Storage/StorageAccessLevel.swift +++ b/Amplify/Categories/Storage/StorageAccessLevel.swift @@ -11,6 +11,7 @@ import Foundation /// See https://aws-amplify.github.io/docs/ios/storage#storage-access /// /// - Tag: StorageAccessLevel +@available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public enum StorageAccessLevel: String { /// Objects can be read or written by any user without authentication diff --git a/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift b/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift index f36e1f8954..22a76ea0ca 100644 --- a/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift +++ b/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift @@ -17,6 +17,14 @@ extension StorageCategory: StorageCategoryBehavior { try await plugin.getURL(key: key, options: options) } + @discardableResult + public func getURL( + path: StoragePath, + options: StorageGetURLOperation.Request.Options? = nil + ) async throws -> URL { + try await plugin.getURL(path: path, options: options) + } + @discardableResult public func downloadData( key: String, @@ -25,6 +33,14 @@ extension StorageCategory: StorageCategoryBehavior { plugin.downloadData(key: key, options: options) } + @discardableResult + public func downloadData( + path: StoragePath, + options: StorageDownloadDataOperation.Request.Options? = nil + ) -> StorageDownloadDataTask { + plugin.downloadData(path: path, options: options) + } + @discardableResult public func downloadFile( key: String, @@ -34,6 +50,15 @@ extension StorageCategory: StorageCategoryBehavior { plugin.downloadFile(key: key, local: local, options: options) } + @discardableResult + public func downloadFile( + path: StoragePath, + local: URL, + options: StorageDownloadFileOperation.Request.Options? = nil + ) -> StorageDownloadFileTask { + plugin.downloadFile(path: path, local: local, options: options) + } + @discardableResult public func uploadData( key: String, @@ -43,6 +68,15 @@ extension StorageCategory: StorageCategoryBehavior { plugin.uploadData(key: key, data: data, options: options) } + @discardableResult + public func uploadData( + path: StoragePath, + data: Data, + options: StorageUploadDataOperation.Request.Options? = nil + ) -> StorageUploadDataTask { + plugin.uploadData(path: path, data: data, options: options) + } + @discardableResult public func uploadFile( key: String, @@ -52,6 +86,15 @@ extension StorageCategory: StorageCategoryBehavior { plugin.uploadFile(key: key, local: local, options: options) } + @discardableResult + public func uploadFile( + path: StoragePath, + local: URL, + options: StorageUploadFileOperation.Request.Options? = nil + ) -> StorageUploadFileTask { + plugin.uploadFile(path: path, local: local, options: options) + } + @discardableResult public func remove( key: String, @@ -60,6 +103,14 @@ extension StorageCategory: StorageCategoryBehavior { try await plugin.remove(key: key, options: options) } + @discardableResult + public func remove( + path: StoragePath, + options: StorageRemoveRequest.Options? = nil + ) async throws -> String { + try await plugin.remove(path: path, options: options) + } + @discardableResult public func list( options: StorageListOperation.Request.Options? = nil @@ -67,6 +118,14 @@ extension StorageCategory: StorageCategoryBehavior { try await plugin.list(options: options) } + @discardableResult + public func list( + path: StoragePath, + options: StorageListOperation.Request.Options? = nil + ) async throws -> StorageListResult { + try await plugin.list(path: path, options: options) + } + public func handleBackgroundEvents(identifier: String) async -> Bool { await plugin.handleBackgroundEvents(identifier: identifier) } diff --git a/Amplify/Categories/Storage/StorageCategoryBehavior.swift b/Amplify/Categories/Storage/StorageCategoryBehavior.swift index b09b450113..1d519cd898 100644 --- a/Amplify/Categories/Storage/StorageCategoryBehavior.swift +++ b/Amplify/Categories/Storage/StorageCategoryBehavior.swift @@ -22,9 +22,26 @@ public protocol StorageCategoryBehavior { /// - Returns: requested Get URL /// /// - Tag: StorageCategoryBehavior.getURL + @available(*, deprecated, message: "Use getURL(path:options:)") @discardableResult - func getURL(key: String, - options: StorageGetURLOperation.Request.Options?) async throws -> URL + func getURL( + key: String, + options: StorageGetURLOperation.Request.Options? + ) async throws -> URL + + /// Retrieve the remote URL for the object from storage. + /// + /// - Parameters: + /// - path: the path to the object in storage. + /// - options: Parameters to specific plugin behavior + /// - Returns: requested Get URL + /// + /// - Tag: StorageCategoryBehavior.getURL + @discardableResult + func getURL( + path: StoragePath, + options: StorageGetURLOperation.Request.Options? + ) async throws -> URL /// Retrieve the object from storage into memory. /// @@ -34,10 +51,24 @@ public protocol StorageCategoryBehavior { /// - Returns: A task that provides progress updates and the key which was used to download /// /// - Tag: StorageCategoryBehavior.downloadData + @available(*, deprecated, message: "Use downloadData(path:options:)") @discardableResult func downloadData(key: String, options: StorageDownloadDataOperation.Request.Options?) -> StorageDownloadDataTask + /// Retrieve the object from storage into memory. + /// + /// - Parameters: + /// - path: The path for the object in storage + /// - options: Options to adjust the behavior of this request, including plugin-options + /// - Returns: A task that provides progress updates and the key which was used to download + /// + /// - Tag: StorageCategoryBehavior.downloadData + func downloadData( + path: StoragePath, + options: StorageDownloadDataOperation.Request.Options? + ) -> StorageDownloadDataTask + /// Download to file the object from storage. /// /// - Parameters: @@ -47,10 +78,29 @@ public protocol StorageCategoryBehavior { /// - Returns: A task that provides progress updates and the key which was used to download /// /// - Tag: StorageCategoryBehavior.downloadFile + @available(*, deprecated, message: "Use downloadFile(path:options:)") @discardableResult - func downloadFile(key: String, - local: URL, - options: StorageDownloadFileOperation.Request.Options?) -> StorageDownloadFileTask + func downloadFile( + key: String, + local: URL, + options: StorageDownloadFileOperation.Request.Options? + ) -> StorageDownloadFileTask + + /// Download to file the object from storage. + /// + /// - Parameters: + /// - path: The path for the object in storage. + /// - local: The local file to download destination + /// - options: Parameters to specific plugin behavior + /// - Returns: A task that provides progress updates and the key which was used to download + /// + /// - Tag: StorageCategoryBehavior.downloadFile + @discardableResult + func downloadFile( + path: StoragePath, + local: URL, + options: StorageDownloadFileOperation.Request.Options? + ) -> StorageDownloadFileTask /// Upload data to storage /// @@ -61,10 +111,29 @@ public protocol StorageCategoryBehavior { /// - Returns: A task that provides progress updates and the key which was used to upload /// /// - Tag: StorageCategoryBehavior.uploadData + @available(*, deprecated, message: "Use uploadData(path:options:)") @discardableResult - func uploadData(key: String, - data: Data, - options: StorageUploadDataOperation.Request.Options?) -> StorageUploadDataTask + func uploadData( + key: String, + data: Data, + options: StorageUploadDataOperation.Request.Options? + ) -> StorageUploadDataTask + + /// Upload data to storage + /// + /// - Parameters: + /// - path: The path of the object in storage. + /// - data: The data in memory to be uploaded + /// - options: Parameters to specific plugin behavior + /// - Returns: A task that provides progress updates and the key which was used to upload + /// + /// - Tag: StorageCategoryBehavior.uploadData + @discardableResult + func uploadData( + path: StoragePath, + data: Data, + options: StorageUploadDataOperation.Request.Options? + ) -> StorageUploadDataTask /// Upload local file to storage /// @@ -75,10 +144,29 @@ public protocol StorageCategoryBehavior { /// - Returns: A task that provides progress updates and the key which was used to upload /// /// - Tag: StorageCategoryBehavior.uploadFile + @available(*, deprecated, message: "Use uploadFile(path:options:)") + @discardableResult + func uploadFile( + key: String, + local: URL, + options: StorageUploadFileOperation.Request.Options? + ) -> StorageUploadFileTask + + /// Upload local file to storage + /// + /// - Parameters: + /// - path: The path of the object in storage. + /// - local: The path to a local file. + /// - options: Parameters to specific plugin behavior + /// - Returns: A task that provides progress updates and the key which was used to upload + /// + /// - Tag: StorageCategoryBehavior.uploadFile @discardableResult - func uploadFile(key: String, - local: URL, - options: StorageUploadFileOperation.Request.Options?) -> StorageUploadFileTask + func uploadFile( + path: StoragePath, + local: URL, + options: StorageUploadFileOperation.Request.Options? + ) -> StorageUploadFileTask /// Delete object from storage /// @@ -88,21 +176,52 @@ public protocol StorageCategoryBehavior { /// - Returns: An operation object that provides notifications and actions related to the execution of the work /// /// - Tag: StorageCategoryBehavior.remove + @available(*, deprecated, message: "Use remove(path:options:)") + @discardableResult + func remove( + key: String, + options: StorageRemoveOperation.Request.Options? + ) async throws -> String + + /// Delete object from storage + /// + /// - Parameters: + /// - path: The path of the object in storage. + /// - options: Parameters to specific plugin behavior + /// - Returns: An operation object that provides notifications and actions related to the execution of the work + /// + /// - Tag: StorageCategoryBehavior.remove @discardableResult - func remove(key: String, - options: StorageRemoveOperation.Request.Options?) async throws -> String + func remove( + path: StoragePath, + options: StorageRemoveOperation.Request.Options? + ) async throws -> String /// List the object identifiers under the hierarchy specified by the path, relative to access level, from storage /// /// - Parameters: /// - options: Parameters to specific plugin behavior - /// - resultListener: Triggered when the list is complete /// - Returns: An operation object that provides notifications and actions related to the execution of the work /// /// - Tag: StorageCategoryBehavior.list + @available(*, deprecated, message: "Use list(path:options:)") @discardableResult func list(options: StorageListOperation.Request.Options?) async throws -> StorageListResult + /// List the object identifiers under the hierarchy specified by the path, relative to access level, from storage + /// + /// - Parameters: + /// - path: The path of the object in storage. + /// - options: Parameters to specific plugin behavior + /// - Returns: An operation object that provides notifications and actions related to the execution of the work + /// + /// - Tag: StorageCategoryBehavior.list + @discardableResult + func list( + path: StoragePath, + options: StorageListOperation.Request.Options? + ) async throws -> StorageListResult + /// Handles background events which are related to URLSession /// - Parameter identifier: identifier /// - Returns: returns true if the identifier is handled by Amplify diff --git a/Amplify/Categories/Storage/StoragePath.swift b/Amplify/Categories/Storage/StoragePath.swift new file mode 100644 index 0000000000..d011e3e110 --- /dev/null +++ b/Amplify/Categories/Storage/StoragePath.swift @@ -0,0 +1,44 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation + +public typealias StoragePathResolver = (String) -> String + +/// Protocol that provides a closure to resolve the storage path. +/// +/// - Tag: StoragePath +public protocol StoragePath { + var pathResolver: StoragePathResolver { get } +} + +public extension StoragePath where Self == StringStoragePath { + static func fromString(_ path: String) -> Self { + return StringStoragePath(pathResolver: { _ in return path }) + } +} + +public extension StoragePath where Self == IdentityIdStoragePath { + static func fromIdentityId(_ identityIdPathResolver: @escaping StoragePathResolver) -> Self { + return IdentityIdStoragePath(pathResolver: identityIdPathResolver) + } +} + +/// Conforms to StoragePath protocol. Provides a storage path based on a string storage path. +/// +/// - Tag: StringStoragePath +public struct StringStoragePath: StoragePath { + public let pathResolver: StoragePathResolver +} + +/// Conforms to StoragePath protocol. +/// Provides a storage path constructed from an unique identity identifer. +/// +/// - Tag: IdentityStoragePath +public struct IdentityIdStoragePath: StoragePath { + public let pathResolver: StoragePathResolver +} diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index 80167b691d..fde6e7ccf4 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -45,6 +45,55 @@ extension AWSS3StoragePlugin { return result } + public func getURL( + path: StoragePath, + options: StorageGetURLOperation.Request.Options? = nil + ) async throws -> URL { + let options = options ?? StorageGetURLRequest.Options() + let path = "" //TODO: resolve path + let request = StorageGetURLRequest(key: path, options: options) + if let error = request.validate() { + throw error + } + let prefixResolver = storageConfiguration.prefixResolver ?? StorageAccessLevelAwarePrefixResolver(authService: authService) + let prefix = try await prefixResolver.resolvePrefix(for: options.accessLevel, + targetIdentityId: options.targetIdentityId) + let serviceKey = prefix + request.key + if let pluginOptions = options.pluginOptions as? AWSStorageGetURLOptions, pluginOptions.validateObjectExistence { + try await storageService.validateObjectExistence(serviceKey: serviceKey) + } + let accelerate = try AWSS3PluginOptions.accelerateValue( + pluginOptions: options.pluginOptions) + let result = try await storageService.getPreSignedURL( + serviceKey: serviceKey, + signingOperation: .getObject, + metadata: nil, + accelerate: accelerate, + expires: options.expires) + + let channel = HubChannel(from: categoryType) + let payload = HubPayload(eventName: HubPayload.EventName.Storage.getURL, context: options, data: result) + Amplify.Hub.dispatch(to: channel, payload: payload) + return result + } + + public func downloadData( + path: StoragePath, + options: StorageDownloadDataOperation.Request.Options? = nil + ) -> StorageDownloadDataTask { + let options = options ?? StorageDownloadDataRequest.Options() + let path = "" //TODO: resolve path + let request = StorageDownloadDataRequest(key: path, options: options) + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: storageConfiguration, + storageService: storageService, + authService: authService) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + queue.addOperation(operation) + + return taskAdapter + } + @discardableResult public func downloadData( key: String, @@ -80,6 +129,25 @@ extension AWSS3StoragePlugin { return taskAdapter } + @discardableResult + public func downloadFile( + path: StoragePath, + local: URL, + options: StorageDownloadFileOperation.Request.Options? = nil + ) -> StorageDownloadFileTask { + let options = options ?? StorageDownloadFileRequest.Options() + let path = "" //TODO: resolve path + let request = StorageDownloadFileRequest(key: path, local: local, options: options) + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: storageConfiguration, + storageService: storageService, + authService: authService) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + queue.addOperation(operation) + + return taskAdapter + } + @discardableResult public func uploadData( key: String, @@ -98,6 +166,25 @@ extension AWSS3StoragePlugin { return taskAdapter } + @discardableResult + public func uploadData( + path: StoragePath, + data: Data, + options: StorageUploadDataOperation.Request.Options? = nil + ) -> StorageUploadDataTask { + let options = options ?? StorageUploadDataRequest.Options() + let path = "" //TODO: resolve path + let request = StorageUploadDataRequest(key: path, data: data, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: storageConfiguration, + storageService: storageService, + authService: authService) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + queue.addOperation(operation) + + return taskAdapter + } + @discardableResult public func uploadFile( key: String, @@ -116,6 +203,25 @@ extension AWSS3StoragePlugin { return taskAdapter } + @discardableResult + public func uploadFile( + path: StoragePath, + local: URL, + options: StorageUploadFileOperation.Request.Options? = nil + ) -> StorageUploadFileTask { + let options = options ?? StorageUploadFileRequest.Options() + let path = "" //TODO: resolve path + let request = StorageUploadFileRequest(key: path, local: local, options: options) + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: storageConfiguration, + storageService: storageService, + authService: authService) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + queue.addOperation(operation) + + return taskAdapter + } + @discardableResult public func remove( key: String, @@ -133,6 +239,24 @@ extension AWSS3StoragePlugin { return try await taskAdapter.value } + @discardableResult + public func remove( + path: StoragePath, + options: StorageRemoveOperation.Request.Options? = nil + ) async throws -> String { + let options = options ?? StorageRemoveRequest.Options() + let path = "" //TODO: resolve path + let request = StorageRemoveRequest(key: path, options: options) + let operation = AWSS3StorageRemoveOperation(request, + storageConfiguration: storageConfiguration, + storageService: storageService, + authService: authService) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + queue.addOperation(operation) + + return try await taskAdapter.value + } + public func list( options: StorageListRequest.Options? = nil ) async throws -> StorageListResult { @@ -148,6 +272,21 @@ extension AWSS3StoragePlugin { return result } + public func list( + path: StoragePath, + options: StorageListRequest.Options? = nil + ) async throws -> StorageListResult { + let options = options ?? StorageListRequest.Options() + let prefix = "" //TODO: resolve path + let result = try await storageService.list(prefix: prefix, options: options) + + let channel = HubChannel(from: categoryType) + let payload = HubPayload(eventName: HubPayload.EventName.Storage.list, context: options, data: result) + Amplify.Hub.dispatch(to: channel, payload: payload) + + return result + } + public func handleBackgroundEvents(identifier: String) async -> Bool { await withCheckedContinuation { (continuation: CheckedContinuation) in StorageBackgroundEventsRegistry.handleBackgroundEvents(identifier: identifier, continuation: continuation) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin.swift index 66fb690978..989d6528a3 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin.swift @@ -25,6 +25,7 @@ final public class AWSS3StoragePlugin: StorageCategoryPlugin { var queue: OperationQueue! /// The default access level used for API calls. + @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") var defaultAccessLevel: StorageAccessLevel! /// The unique key of the plugin within the storage category. diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift index e26f2aee94..a9ad565410 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift @@ -12,6 +12,7 @@ import AWSPluginsCore /// Resolves the final prefix prepended to the S3 key for a given request. /// /// - Tag: AWSS3PluginPrefixResolver +@available(*, deprecated, message: "Use `StoragePath` instead") public protocol AWSS3PluginPrefixResolver { /// - Tag: AWSS3PluginPrefixResolver.resolvePrefix func resolvePrefix(for accessLevel: StorageAccessLevel, @@ -21,6 +22,7 @@ public protocol AWSS3PluginPrefixResolver { /// Convenience resolver. Resolves the provided key as-is, with no manipulation /// /// - Tag: PassThroughPrefixResolver +@available(*, deprecated, message: "Use `StoragePath` instead") public struct PassThroughPrefixResolver: AWSS3PluginPrefixResolver { public func resolvePrefix(for accessLevel: StorageAccessLevel, targetIdentityId: String?) async throws -> String { @@ -31,6 +33,7 @@ public struct PassThroughPrefixResolver: AWSS3PluginPrefixResolver { /// AWSS3StoragePlugin default logic /// /// - Tag: StorageAccessLevelAwarePrefixResolver +@available(*, deprecated, message: "Use `StoragePath` instead") struct StorageAccessLevelAwarePrefixResolver { let authService: AWSAuthServiceBehavior diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3StoragePluginConfiguration.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3StoragePluginConfiguration.swift index 0cc1ee3996..63ec95ba5e 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3StoragePluginConfiguration.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3StoragePluginConfiguration.swift @@ -13,6 +13,7 @@ import Foundation public struct AWSS3StoragePluginConfiguration { /// - Tag: AWSS3StoragePluginConfiguration.prefixResolver + @available(*, deprecated) public let prefixResolver: AWSS3PluginPrefixResolver? /// - Tag: AWSS3StoragePluginConfiguration.init @@ -21,6 +22,7 @@ public struct AWSS3StoragePluginConfiguration { } /// - Tag: AWSS3StoragePluginConfiguration.prefixResolverFunc + @available(*, deprecated, message: "Use `StoragePath` instead") public static func prefixResolver( _ prefixResolver: AWSS3PluginPrefixResolver) -> AWSS3StoragePluginConfiguration { .init(prefixResolver: prefixResolver) diff --git a/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift b/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift index b8e9acf55e..cccfabe016 100644 --- a/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift +++ b/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift @@ -180,6 +180,68 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { false } + func getURL(path: StoragePath, options: StorageGetURLRequest.Options?) async throws -> URL { + notify("getURL") + let options = options ?? StorageGetURLRequest.Options() + let request = StorageGetURLRequest(key: key, options: options) + let operation = MockStorageGetURLOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } + + func downloadData(path: StoragePath, options: StorageDownloadDataRequest.Options?) -> StorageDownloadDataTask { + notify("downloadData") + let options = options ?? StorageDownloadDataRequest.Options() + let request = StorageDownloadDataRequest(key: key, options: options) + let operation = MockStorageDownloadDataOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + func downloadFile(path: StoragePath, local: URL, options: StorageDownloadFileRequest.Options?) -> StorageDownloadFileTask { + notify("downloadFile") + let options = options ?? StorageDownloadFileRequest.Options() + let request = StorageDownloadFileRequest(key: key, local: local, options: options) + let operation = MockStorageDownloadFileOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + func uploadData(path: StoragePath, data: Data, options: StorageUploadDataRequest.Options?) -> StorageUploadDataTask { + notify("uploadData") + let options = options ?? StorageUploadDataRequest.Options() + let request = StorageUploadDataRequest(key: key, data: data, options: options) + let operation = MockStorageUploadDataOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + func uploadFile(path: StoragePath, local: URL, options: StorageUploadFileRequest.Options?) -> StorageUploadFileTask { + notify("uploadFile") + let options = options ?? StorageUploadFileRequest.Options() + let request = StorageUploadFileRequest(key: key, local: local, options: options) + let operation = MockStorageUploadFileOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + func remove(path: StoragePath, options: StorageRemoveRequest.Options?) async throws -> String { + notify("remove") + let options = options ?? StorageRemoveRequest.Options() + let request = StorageRemoveRequest(key: key, options: options) + let operation = MockStorageRemoveOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } + + func list(path: StoragePath, options: StorageListRequest.Options?) async throws -> StorageListResult { + notify("list") + let options = options ?? StorageListRequest.Options() + let request = StorageListRequest(options: options) + let operation = MockStorageListOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } } class MockSecondStorageCategoryPlugin: MockStorageCategoryPlugin { diff --git a/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift b/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift index baa6629239..6a81984fcb 100644 --- a/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift +++ b/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift @@ -299,6 +299,93 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { return try await taskAdapter.value } + @discardableResult + func getURL( + path: StoragePath, + options: StorageGetURLOperation.Request.Options? + ) async throws -> URL { + let options = options ?? StorageGetURLRequest.Options() + let request = StorageGetURLRequest(key: key, options: options) + let operation = MockStorageGetURLOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } + + @discardableResult + public func downloadData( + path: StoragePath, + options: StorageDownloadDataOperation.Request.Options? = nil + ) -> StorageDownloadDataTask { + let options = options ?? StorageDownloadDataRequest.Options() + let request = StorageDownloadDataRequest(key: key, options: options) + let operation = MockDispatchingStorageDownloadDataOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + @discardableResult + public func downloadFile( + path: StoragePath, + local: URL, + options: StorageDownloadFileOperation.Request.Options? + ) -> StorageDownloadFileTask { + let options = options ?? StorageDownloadFileRequest.Options() + let request = StorageDownloadFileRequest(key: key, local: local, options: options) + let operation = MockDispatchingStorageDownloadFileOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + @discardableResult + public func uploadData( + path: StoragePath, + data: Data, + options: StorageUploadDataOperation.Request.Options? + ) -> StorageUploadDataTask { + let options = options ?? StorageUploadDataRequest.Options() + let request = StorageUploadDataRequest(key: key, data: data, options: options) + let operation = MockDispatchingStorageUploadDataOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + @discardableResult + public func uploadFile( + path: StoragePath, + local: URL, + options: StorageUploadFileOperation.Request.Options? + ) -> StorageUploadFileTask { + let options = options ?? StorageUploadFileRequest.Options() + let request = StorageUploadFileRequest(key: key, local: local, options: options) + let operation = MockDispatchingStorageUploadFileOperation(request: request) + let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) + return taskAdapter + } + + @discardableResult + public func remove( + path: StoragePath, + options: StorageRemoveRequest.Options? = nil + ) async throws -> String { + let options = options ?? StorageRemoveRequest.Options() + let request = StorageRemoveRequest(key: key, options: options) + let operation = MockDispatchingStorageRemoveOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } + + @discardableResult + func list( + path: StoragePath, + options: StorageListOperation.Request.Options? + ) async throws -> StorageListResult { + let options = options ?? StorageListRequest.Options() + let request = StorageListRequest(options: options) + let operation = MockDispatchingStorageListOperation(request: request) + let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) + return try await taskAdapter.value + } + } // swiftlint:disable:next type_name From 3a0c0cbcf04214567dbec8199c6fb7d4f1f86084 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Mon, 18 Mar 2024 10:44:22 -0400 Subject: [PATCH 02/12] feat(storage): refactor storage remove api by including path (#3571) * feat(storage): refactor storage remove api by including path * updated tests --- .../Request/StorageRemoveRequest.swift | 15 ++++ .../Core/Support/AmplifyTaskExecution.swift | 71 +++++++++++++++++++ ...SS3StoragePlugin+AsyncClientBehavior.swift | 16 ++--- .../Error/AWSS3+StorageErrorConvertible.swift | 2 +- .../Storage/AWSS3StorageServiceBehavior.swift | 7 ++ .../Internal/StoragePath+Extensions.swift | 34 +++++++++ .../Tasks/AWSS3StorageRemoveTask.swift | 59 +++++++++++++++ .../Mocks/MockAWSS3StorageService.swift | 4 ++ .../Mocks/MockS3Client.swift | 7 +- .../AWSS3StorageRemoveRequestTests.swift | 1 + .../AWSS3StorageServiceConfigureTests.swift | 14 ---- ...SS3StorageServiceDeleteBehaviorTests.swift | 14 ---- ...orageServiceEscapeHatchBehaviorTests.swift | 15 ---- ...AWSS3StorageServiceListBehaviorTests.swift | 14 ---- ...eServiceMultiPartUploadBehaviorTests.swift | 15 ---- .../Storage/AWSS3StorageServiceTestBase.swift | 41 ----------- ...SS3StorageServiceUploadBehaviorTests.swift | 14 ---- .../Tasks/AWSS3StorageRemoveTaskTests.swift | 70 ++++++++++++++++++ 18 files changed, 274 insertions(+), 139 deletions(-) create mode 100644 Amplify/Core/Support/AmplifyTaskExecution.swift create mode 100644 AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift create mode 100644 AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageRemoveTask.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceConfigureTests.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceDeleteBehaviorTests.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceEscapeHatchBehaviorTests.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceListBehaviorTests.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceMultiPartUploadBehaviorTests.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceTestBase.swift delete mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceUploadBehaviorTests.swift create mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift diff --git a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift index ace12f1218..f0e0310265 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift @@ -14,17 +14,32 @@ public struct StorageRemoveRequest: AmplifyOperationRequest { /// The unique identifier for the object in storage /// /// - Tag: StorageRemoveRequest.key + @available(*, deprecated, message: "Use `path` in Storage API instead of `key`") public let key: String + /// The unique path for the object in storage + /// + /// - Tag: StorageRemoveRequest.path + public let path: StoragePath? + /// Options to adjust the behavior of this request, including plugin-options /// /// - Tag: StorageRemoveRequest.options public let options: Options /// - Tag: StorageRemoveRequest.init + @available(*, deprecated, message: "Use init(path:options)") public init(key: String, options: Options) { self.key = key self.options = options + self.path = nil + } + + /// - Tag: StorageRemoveRequest.init + public init(path: StoragePath, options: Options) { + self.key = "" + self.options = options + self.path = path } } diff --git a/Amplify/Core/Support/AmplifyTaskExecution.swift b/Amplify/Core/Support/AmplifyTaskExecution.swift new file mode 100644 index 0000000000..ff73c60f26 --- /dev/null +++ b/Amplify/Core/Support/AmplifyTaskExecution.swift @@ -0,0 +1,71 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// +import Foundation + +/// Task that supports hub with execution of a single unit of work. . +/// +/// See Also: [AmplifyTask](x-source-tag://AmplifyTask) +/// +/// - Tag: AmplifyTaskExecution +public protocol AmplifyTaskExecution { + + associatedtype Success + associatedtype Request + associatedtype Failure: AmplifyError + + typealias AmplifyTaskExecutionResult = Result + + /// Blocks until the receiver has successfully collected a result or throws if an error was encountered. + /// + /// - Tag: AmplifyTaskExecution.value + var value: Success { get async throws } + + /// Hub event name for the task + /// + /// - Tag: AmplifyTaskExecution.eventName + var eventName: HubPayloadEventName { get } + + /// Category for which the Hub event would be dispatched for. + /// + /// - Tag: AmplifyTaskExecution.eventNameCategoryType + var eventNameCategoryType: CategoryType { get } + + /// Executes work represented by the receiver. + /// + /// - Tag: AmplifyTaskExecution.execute + func execute() async throws -> Success + + /// Dispatches a hub event. + /// + /// - Tag: AmplifyTaskExecution.dispatch + func dispatch(result: AmplifyTaskExecutionResult) + +} + +public extension AmplifyTaskExecution where Self: DefaultLogger { + var value: Success { + get async throws { + do { + log.info("Starting execution for \(eventName)") + let valueReturned = try await execute() + log.info("Successfully completed execution for \(eventName) with result:\n\(valueReturned)") + dispatch(result: .success(valueReturned)) + return valueReturned + } catch let error as Failure { + log.error("Failed execution for \(eventName) with error:\n\(error)") + dispatch(result: .failure(error)) + throw error + } + } + } + + func dispatch(result: AmplifyTaskExecutionResult) { + let channel = HubChannel(from: eventNameCategoryType) + let payload = HubPayload(eventName: eventName, context: nil, data: result) + Amplify.Hub.dispatch(to: channel, payload: payload) + } +} diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index fde6e7ccf4..fede94948d 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -245,16 +245,12 @@ extension AWSS3StoragePlugin { options: StorageRemoveOperation.Request.Options? = nil ) async throws -> String { let options = options ?? StorageRemoveRequest.Options() - let path = "" //TODO: resolve path - let request = StorageRemoveRequest(key: path, options: options) - let operation = AWSS3StorageRemoveOperation(request, - storageConfiguration: storageConfiguration, - storageService: storageService, - authService: authService) - let taskAdapter = AmplifyOperationTaskAdapter(operation: operation) - queue.addOperation(operation) - - return try await taskAdapter.value + let request = StorageRemoveRequest(path: path, options: options) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: storageConfiguration, + storageBehaviour: storageService) + return try await task.value } public func list( diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Error/AWSS3+StorageErrorConvertible.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Error/AWSS3+StorageErrorConvertible.swift index 7b4a08ab76..8b1e7e316e 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Error/AWSS3+StorageErrorConvertible.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Error/AWSS3+StorageErrorConvertible.swift @@ -14,7 +14,7 @@ extension AWSS3.NoSuchBucket: StorageErrorConvertible { var storageError: StorageError { .service( "The specific bucket does not exist", - "", + "Make sure the bucket exists", self ) } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift index 21ae5df171..b1e274a609 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift @@ -33,6 +33,12 @@ protocol AWSS3StorageServiceBehavior { typealias StorageServiceMultiPartUploadEvent = StorageEvent + + /// - Tag: AWSS3StorageService.client + var client: S3ClientProtocol { get } + + var bucket: String! { get } + func reset() func getEscapeHatch() -> S3Client @@ -67,6 +73,7 @@ protocol AWSS3StorageServiceBehavior { func list(prefix: String, options: StorageListRequest.Options) async throws -> StorageListResult + @available(*, deprecated, message: "Use `AWSS3StorageRemoveTask` instead") func delete(serviceKey: String, onEvent: @escaping StorageServiceDeleteEventHandler) } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift new file mode 100644 index 0000000000..94bd595354 --- /dev/null +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -0,0 +1,34 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Foundation +import Amplify +import AWSPluginsCore + +extension StoragePath { + func resolvePath() async throws -> String { + if self is IdentityIdStoragePath { + let authService = AWSAuthService() + let identityId = try await authService.getIdentityID() + let path = pathResolver(identityId) + try validate(path) + return path + } else { + let path = pathResolver("") + try validate(path) + return path + } + } + + func validate(_ path: String) throws { + if !path.hasPrefix("/") { + let errorDescription = "Invalid StoragePath specified." + let recoverySuggestion = "Please specify a valid StoragePath that contains the prefix /." + throw StorageError.validation(path, errorDescription, recoverySuggestion, nil) + } + } +} diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageRemoveTask.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageRemoveTask.swift new file mode 100644 index 0000000000..33b4319db3 --- /dev/null +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageRemoveTask.swift @@ -0,0 +1,59 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Amplify +import Foundation +import AWSS3 +import AWSPluginsCore + +protocol StorageRemoveTask: AmplifyTaskExecution where Request == AWSS3DeleteObjectRequest, Success == String, Failure == StorageError {} + +class AWSS3StorageRemoveTask: StorageRemoveTask, DefaultLogger { + + let request: StorageRemoveRequest + let storageConfiguration: AWSS3StoragePluginConfiguration + let storageBehaviour: AWSS3StorageServiceBehavior + + init(_ request: StorageRemoveRequest, + storageConfiguration: AWSS3StoragePluginConfiguration, + storageBehaviour: AWSS3StorageServiceBehavior) { + self.request = request + self.storageConfiguration = storageConfiguration + self.storageBehaviour = storageBehaviour + } + + var eventName: HubPayloadEventName { + HubPayload.EventName.Storage.remove + } + + var eventNameCategoryType: CategoryType { + .storage + } + + func execute() async throws -> String { + guard let serviceKey = try await request.path?.resolvePath() else { + throw StorageError.validation( + "path", + "`path` is required for removing an object", + "Make sure that a valid `path` is passed for removing an object") + } + let input = DeleteObjectInput( + bucket: storageBehaviour.bucket, + key: serviceKey) + do { + _ = try await storageBehaviour.client.deleteObject(input: input) + } catch let error as StorageErrorConvertible { + throw error.storageError + } catch let error { + throw StorageError.service( + "Service error occurred.", + "Please inspect the underlying error for more details.", + error) + } + return serviceKey + } +} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockAWSS3StorageService.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockAWSS3StorageService.swift index 2b70e3bc90..9249596f04 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockAWSS3StorageService.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockAWSS3StorageService.swift @@ -55,6 +55,10 @@ public class MockAWSS3StorageService: AWSS3StorageServiceBehavior { } */ + public var client: any S3ClientProtocol = MockS3Client() + + public var bucket: String! = "bucket" + public func reset() { interactions.append(#function) } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockS3Client.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockS3Client.swift index a8c8f9f4b1..5fb4f71451 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockS3Client.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Mocks/MockS3Client.swift @@ -28,6 +28,8 @@ final class MockS3Client { var listObjectsV2Handler: (ListObjectsV2Input) async throws -> ListObjectsV2Output = { _ in throw ClientError.missingResult } var headObjectHandler: (HeadObjectInput) async throws -> HeadObjectOutput = { _ in return HeadObjectOutput() } + + var deleteObjectHandler: ((DeleteObjectInput) async throws -> DeleteObjectOutput)? = nil } extension MockS3Client: S3ClientProtocol { @@ -111,7 +113,10 @@ extension MockS3Client: S3ClientProtocol { } func deleteObject(input: AWSS3.DeleteObjectInput) async throws -> AWSS3.DeleteObjectOutput { - throw ClientError.missingImplementation + guard let deleteObjectHandler = deleteObjectHandler else { + throw ClientError.missingImplementation + } + return try await deleteObjectHandler(input) } func deleteObjects(input: AWSS3.DeleteObjectsInput) async throws -> AWSS3.DeleteObjectsOutput { diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift index 76d7cfbff9..f5a741846c 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift @@ -9,6 +9,7 @@ import XCTest import Amplify @testable import AWSS3StoragePlugin +// TODO: [HS] Add path validation test cases once storage path extension is merged. class AWSS3StorageRemoveRequestTests: XCTestCase { let testTargetIdentityId = "TestTargetIdentityId" diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceConfigureTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceConfigureTests.swift deleted file mode 100644 index 8cea4937f5..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceConfigureTests.swift +++ /dev/null @@ -1,14 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -class AWSS3StorageServiceConfigureTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceDeleteBehaviorTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceDeleteBehaviorTests.swift deleted file mode 100644 index 06c8a65ac3..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceDeleteBehaviorTests.swift +++ /dev/null @@ -1,14 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -class AWSS3StorageServiceDeleteBehaviorTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceEscapeHatchBehaviorTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceEscapeHatchBehaviorTests.swift deleted file mode 100644 index d47a94cf89..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceEscapeHatchBehaviorTests.swift +++ /dev/null @@ -1,15 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -// swiftlint:disable:next type_name -class AWSS3StorageServiceEscapeHatchBehaviorTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceListBehaviorTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceListBehaviorTests.swift deleted file mode 100644 index 8cfa49ca95..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceListBehaviorTests.swift +++ /dev/null @@ -1,14 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -class AWSS3StorageServiceListBehaviorTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceMultiPartUploadBehaviorTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceMultiPartUploadBehaviorTests.swift deleted file mode 100644 index 4bc442d657..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceMultiPartUploadBehaviorTests.swift +++ /dev/null @@ -1,15 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -// swiftlint:disable:next type_name -class AWSS3StorageServiceMultiPartUploadBehaviorTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceTestBase.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceTestBase.swift deleted file mode 100644 index d1085d922c..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceTestBase.swift +++ /dev/null @@ -1,41 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest -@testable import AWSS3StoragePlugin -@testable import AmplifyTestCommon - -class AWSS3StorageServiceTestBase: XCTestCase { - /* - var mockTransferUtility: MockAWSS3TransferUtility! - var mockPreSignedURLBuilder: MockAWSS3PreSignedURLBuilder! - var mockS3: MockS3! - - var storageService: AWSS3StorageService! - - var bucket = "bucket" - var identifier = "identifier" - - override func setUp() { - mockTransferUtility = MockAWSS3TransferUtility() - mockPreSignedURLBuilder = MockAWSS3PreSignedURLBuilder() - mockS3 = MockS3() - storageService = AWSS3StorageService(transferUtility: mockTransferUtility, - preSignedURLBuilder: mockPreSignedURLBuilder, - awsS3: mockS3, - bucket: bucket, - identifier: identifier) - } - - func testConfigure() { - - } - - func testReset() async { - } - */ -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceUploadBehaviorTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceUploadBehaviorTests.swift deleted file mode 100644 index 323ae435e5..0000000000 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Service/Storage/AWSS3StorageServiceUploadBehaviorTests.swift +++ /dev/null @@ -1,14 +0,0 @@ -// -// Copyright Amazon.com Inc. or its affiliates. -// All Rights Reserved. -// -// SPDX-License-Identifier: Apache-2.0 -// - -import XCTest - -class AWSS3StorageServiceUploadBehaviorTests: AWSS3StorageServiceTestBase { - func testClassMustNotBeEmpty() { - // Swift format crashes if a test class is empty - } -} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift new file mode 100644 index 0000000000..22a96d6de9 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift @@ -0,0 +1,70 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import XCTest +@testable import Amplify +@testable import AmplifyTestCommon +@testable import AWSPluginsCore +@testable import AWSS3StoragePlugin +@testable import AWSPluginsTestCommon +import AWSS3 + +class AWSS3StorageRemoveTaskTests: XCTestCase { + + + /// - Given: A configured Storage Remove Task with mocked service + /// - When: AWSS3StorageRemoveTask value is invoked + /// - Then: A key should be returned, that has been removed without any errors. + func testRemoveTaskSuccess() async throws { + let serviceMock = MockAWSS3StorageService() + let client = serviceMock.client as! MockS3Client + client.deleteObjectHandler = { input in + return .init() + } + + let request = StorageRemoveRequest( + path: StringStoragePath.fromString("/path"), options: .init()) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + let value = try await task.value + XCTAssertEqual(value, "/path") + } + + /// - Given: A configured Storage Remove Task with mocked service, throwing `NoSuchKey` exception + /// - When: AWSS3StorageRemoveTask value is invoked + /// - Then: A storage service error should be returned, with an underlying service error + func testRemoveTaskNoBucket() async throws { + let serviceMock = MockAWSS3StorageService() + let client = serviceMock.client as! MockS3Client + client.deleteObjectHandler = { input in + throw AWSS3.NoSuchKey() + } + + let request = StorageRemoveRequest( + path: StringStoragePath.fromString("/path"), options: .init()) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .service(_, _, let underlyingError) = storageError else { + XCTFail("Should throw a Storage service error, instead threw \(error)") + return + } + XCTAssertTrue(underlyingError is AWSS3.NoSuchKey, + "Underlying error should be NoSuchKey, instead got \(String(describing: underlyingError))") + } + } + +} From 326ba60447b010bf1bf533e8a32062cfdf487357 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Mon, 18 Mar 2024 16:15:06 -0500 Subject: [PATCH 03/12] feat(storage): update storage download api (#3561) --- .../Request/StorageDownloadDataRequest.swift | 14 ++ .../Request/StorageDownloadFileRequest.swift | 15 ++ .../Request/StorageRemoveRequest.swift | 4 +- .../Storage/Result/StorageListResult.swift | 6 +- .../StorageCategory+ClientBehavior.swift | 14 +- .../Storage/StorageCategoryBehavior.swift | 14 +- Amplify/Categories/Storage/StoragePath.swift | 19 +- ...SS3StoragePlugin+AsyncClientBehavior.swift | 20 +- .../AWSS3PluginPrefixResolver.swift | 6 +- .../AWSS3StorageDownloadDataOperation.swift | 15 +- .../AWSS3StorageDownloadFileOperation.swift | 24 ++- .../StorageDownloadDataRequest+Validate.swift | 5 + .../StorageDownloadFileRequest+Validate.swift | 5 + .../Internal/StoragePath+Extensions.swift | 34 +++- ...SS3StorageDownloadFileOperationTests.swift | 174 ++++++++++++++++++ .../AWSS3StorageGetDataOperationTests.swift | 165 +++++++++++++++++ ...AWSS3StorageDownloadFileRequestTests.swift | 18 +- .../AWSS3StorageGetDataRequestTests.swift | 18 +- .../Mocks/MockStorageCategoryPlugin.swift | 18 +- .../Hub/AmplifyOperationHubTests.swift | 14 +- .../Storage/StoragePathTests.swift | 35 ++++ 21 files changed, 555 insertions(+), 82 deletions(-) create mode 100644 AmplifyTests/CategoryTests/Storage/StoragePathTests.swift diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift index 2c550b6b6d..91a85e20b0 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift @@ -13,9 +13,15 @@ import Foundation /// - Tag: StorageDownloadDataRequest public struct StorageDownloadDataRequest: 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: StorageDownloadDataRequest.key + @available(*, deprecated, message: "Use `StoragePath` instead") public let key: String /// Options to adjust the behavior of this request, including plugin-options @@ -24,9 +30,17 @@ public struct StorageDownloadDataRequest: AmplifyOperationRequest { public let options: Options /// - Tag: StorageDownloadDataRequest.key + @available(*, deprecated, message: "Use init(path:local:options)") public init(key: String, options: Options) { self.key = key self.options = options + self.path = nil + } + + public init(path: any StoragePath, options: Options) { + self.key = "" + self.options = options + self.path = path } } diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift index 4a665444dc..1738ea0212 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageDownloadFileRequest.swift @@ -13,9 +13,15 @@ import Foundation /// - Tag: StorageDownloadFileRequest public struct StorageDownloadFileRequest: 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: StorageDownloadFileRequest.key + @available(*, deprecated, message: "Use `StoragePath` instead") public let key: String /// The local file to download the object to @@ -29,10 +35,19 @@ public struct StorageDownloadFileRequest: AmplifyOperationRequest { public let options: Options /// - Tag: StorageDownloadFileRequest.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 } } diff --git a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift index f0e0310265..31ae1de4f3 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageRemoveRequest.swift @@ -20,7 +20,7 @@ public struct StorageRemoveRequest: AmplifyOperationRequest { /// The unique path for the object in storage /// /// - Tag: StorageRemoveRequest.path - public let path: StoragePath? + public let path: (any StoragePath)? /// Options to adjust the behavior of this request, including plugin-options /// @@ -36,7 +36,7 @@ public struct StorageRemoveRequest: AmplifyOperationRequest { } /// - Tag: StorageRemoveRequest.init - public init(path: StoragePath, options: Options) { + public init(path: any StoragePath, options: Options) { self.key = "" self.options = options self.path = path diff --git a/Amplify/Categories/Storage/Result/StorageListResult.swift b/Amplify/Categories/Storage/Result/StorageListResult.swift index 45777ae086..7294945b9f 100644 --- a/Amplify/Categories/Storage/Result/StorageListResult.swift +++ b/Amplify/Categories/Storage/Result/StorageListResult.swift @@ -45,7 +45,7 @@ extension StorageListResult { /// The path of the object in storage. /// /// - Tag: StorageListResultItem.path - public let path: StoragePath + public let path: String /// The unique identifier of the object in storage. /// @@ -90,11 +90,11 @@ extension StorageListResult { self.eTag = eTag self.lastModified = lastModified self.pluginResults = pluginResults - self.path = StringStoragePath(pathResolver: { _ in return "" }) + self.path = "" } public init( - path: StoragePath, + path: String, key: String, size: Int? = nil, eTag: String? = nil, diff --git a/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift b/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift index 22a76ea0ca..55b69bbe43 100644 --- a/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift +++ b/Amplify/Categories/Storage/StorageCategory+ClientBehavior.swift @@ -19,7 +19,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func getURL( - path: StoragePath, + path: any StoragePath, options: StorageGetURLOperation.Request.Options? = nil ) async throws -> URL { try await plugin.getURL(path: path, options: options) @@ -35,7 +35,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func downloadData( - path: StoragePath, + path: any StoragePath, options: StorageDownloadDataOperation.Request.Options? = nil ) -> StorageDownloadDataTask { plugin.downloadData(path: path, options: options) @@ -52,7 +52,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func downloadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageDownloadFileOperation.Request.Options? = nil ) -> StorageDownloadFileTask { @@ -70,7 +70,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func uploadData( - path: StoragePath, + path: any StoragePath, data: Data, options: StorageUploadDataOperation.Request.Options? = nil ) -> StorageUploadDataTask { @@ -88,7 +88,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func uploadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageUploadFileOperation.Request.Options? = nil ) -> StorageUploadFileTask { @@ -105,7 +105,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func remove( - path: StoragePath, + path: any StoragePath, options: StorageRemoveRequest.Options? = nil ) async throws -> String { try await plugin.remove(path: path, options: options) @@ -120,7 +120,7 @@ extension StorageCategory: StorageCategoryBehavior { @discardableResult public func list( - path: StoragePath, + path: any StoragePath, options: StorageListOperation.Request.Options? = nil ) async throws -> StorageListResult { try await plugin.list(path: path, options: options) diff --git a/Amplify/Categories/Storage/StorageCategoryBehavior.swift b/Amplify/Categories/Storage/StorageCategoryBehavior.swift index 1d519cd898..0b933d4dfc 100644 --- a/Amplify/Categories/Storage/StorageCategoryBehavior.swift +++ b/Amplify/Categories/Storage/StorageCategoryBehavior.swift @@ -39,7 +39,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.getURL @discardableResult func getURL( - path: StoragePath, + path: any StoragePath, options: StorageGetURLOperation.Request.Options? ) async throws -> URL @@ -65,7 +65,7 @@ public protocol StorageCategoryBehavior { /// /// - Tag: StorageCategoryBehavior.downloadData func downloadData( - path: StoragePath, + path: any StoragePath, options: StorageDownloadDataOperation.Request.Options? ) -> StorageDownloadDataTask @@ -97,7 +97,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.downloadFile @discardableResult func downloadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageDownloadFileOperation.Request.Options? ) -> StorageDownloadFileTask @@ -130,7 +130,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.uploadData @discardableResult func uploadData( - path: StoragePath, + path: any StoragePath, data: Data, options: StorageUploadDataOperation.Request.Options? ) -> StorageUploadDataTask @@ -163,7 +163,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.uploadFile @discardableResult func uploadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageUploadFileOperation.Request.Options? ) -> StorageUploadFileTask @@ -193,7 +193,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.remove @discardableResult func remove( - path: StoragePath, + path: any StoragePath, options: StorageRemoveOperation.Request.Options? ) async throws -> String @@ -218,7 +218,7 @@ public protocol StorageCategoryBehavior { /// - Tag: StorageCategoryBehavior.list @discardableResult func list( - path: StoragePath, + path: any StoragePath, options: StorageListOperation.Request.Options? ) async throws -> StorageListResult diff --git a/Amplify/Categories/Storage/StoragePath.swift b/Amplify/Categories/Storage/StoragePath.swift index d011e3e110..b3cf55867a 100644 --- a/Amplify/Categories/Storage/StoragePath.swift +++ b/Amplify/Categories/Storage/StoragePath.swift @@ -7,24 +7,25 @@ import Foundation -public typealias StoragePathResolver = (String) -> String +public typealias IdentityIDPathResolver = (String) -> String /// Protocol that provides a closure to resolve the storage path. /// /// - Tag: StoragePath public protocol StoragePath { - var pathResolver: StoragePathResolver { get } + associatedtype Input + var resolve: (Input) -> String { get } } public extension StoragePath where Self == StringStoragePath { static func fromString(_ path: String) -> Self { - return StringStoragePath(pathResolver: { _ in return path }) + return StringStoragePath(resolve: { _ in return path }) } } -public extension StoragePath where Self == IdentityIdStoragePath { - static func fromIdentityId(_ identityIdPathResolver: @escaping StoragePathResolver) -> Self { - return IdentityIdStoragePath(pathResolver: identityIdPathResolver) +public extension StoragePath where Self == IdentityIDStoragePath { + static func fromIdentityID(_ identityIdPathResolver: @escaping IdentityIDPathResolver) -> Self { + return IdentityIDStoragePath(resolve: identityIdPathResolver) } } @@ -32,13 +33,13 @@ public extension StoragePath where Self == IdentityIdStoragePath { /// /// - Tag: StringStoragePath public struct StringStoragePath: StoragePath { - public let pathResolver: StoragePathResolver + public let resolve: (String) -> String } /// Conforms to StoragePath protocol. /// Provides a storage path constructed from an unique identity identifer. /// /// - Tag: IdentityStoragePath -public struct IdentityIdStoragePath: StoragePath { - public let pathResolver: StoragePathResolver +public struct IdentityIDStoragePath: StoragePath { + public let resolve: IdentityIDPathResolver } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index fede94948d..8dd2bcb82e 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -46,7 +46,7 @@ extension AWSS3StoragePlugin { } public func getURL( - path: StoragePath, + path: any StoragePath, options: StorageGetURLOperation.Request.Options? = nil ) async throws -> URL { let options = options ?? StorageGetURLRequest.Options() @@ -78,12 +78,11 @@ extension AWSS3StoragePlugin { } public func downloadData( - path: StoragePath, + path: any StoragePath, options: StorageDownloadDataOperation.Request.Options? = nil ) -> StorageDownloadDataTask { let options = options ?? StorageDownloadDataRequest.Options() - let path = "" //TODO: resolve path - let request = StorageDownloadDataRequest(key: path, options: options) + let request = StorageDownloadDataRequest(path: path, options: options) let operation = AWSS3StorageDownloadDataOperation(request, storageConfiguration: storageConfiguration, storageService: storageService, @@ -131,13 +130,12 @@ extension AWSS3StoragePlugin { @discardableResult public func downloadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageDownloadFileOperation.Request.Options? = nil ) -> StorageDownloadFileTask { let options = options ?? StorageDownloadFileRequest.Options() - let path = "" //TODO: resolve path - let request = StorageDownloadFileRequest(key: path, local: local, options: options) + let request = StorageDownloadFileRequest(path: path, local: local, options: options) let operation = AWSS3StorageDownloadFileOperation(request, storageConfiguration: storageConfiguration, storageService: storageService, @@ -168,7 +166,7 @@ extension AWSS3StoragePlugin { @discardableResult public func uploadData( - path: StoragePath, + path: any StoragePath, data: Data, options: StorageUploadDataOperation.Request.Options? = nil ) -> StorageUploadDataTask { @@ -205,7 +203,7 @@ extension AWSS3StoragePlugin { @discardableResult public func uploadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageUploadFileOperation.Request.Options? = nil ) -> StorageUploadFileTask { @@ -241,7 +239,7 @@ extension AWSS3StoragePlugin { @discardableResult public func remove( - path: StoragePath, + path: any StoragePath, options: StorageRemoveOperation.Request.Options? = nil ) async throws -> String { let options = options ?? StorageRemoveRequest.Options() @@ -269,7 +267,7 @@ extension AWSS3StoragePlugin { } public func list( - path: StoragePath, + path: any StoragePath, options: StorageListRequest.Options? = nil ) async throws -> StorageListResult { let options = options ?? StorageListRequest.Options() diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift index a9ad565410..d7c970f15c 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Configuration/AWSS3PluginPrefixResolver.swift @@ -12,7 +12,7 @@ import AWSPluginsCore /// Resolves the final prefix prepended to the S3 key for a given request. /// /// - Tag: AWSS3PluginPrefixResolver -@available(*, deprecated, message: "Use `StoragePath` instead") +@available(*, deprecated) public protocol AWSS3PluginPrefixResolver { /// - Tag: AWSS3PluginPrefixResolver.resolvePrefix func resolvePrefix(for accessLevel: StorageAccessLevel, @@ -22,7 +22,7 @@ public protocol AWSS3PluginPrefixResolver { /// Convenience resolver. Resolves the provided key as-is, with no manipulation /// /// - Tag: PassThroughPrefixResolver -@available(*, deprecated, message: "Use `StoragePath` instead") +@available(*, deprecated) public struct PassThroughPrefixResolver: AWSS3PluginPrefixResolver { public func resolvePrefix(for accessLevel: StorageAccessLevel, targetIdentityId: String?) async throws -> String { @@ -33,7 +33,7 @@ public struct PassThroughPrefixResolver: AWSS3PluginPrefixResolver { /// AWSS3StoragePlugin default logic /// /// - Tag: StorageAccessLevelAwarePrefixResolver -@available(*, deprecated, message: "Use `StoragePath` instead") +@available(*, deprecated) struct StorageAccessLevelAwarePrefixResolver { let authService: AWSAuthServiceBehavior diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadDataOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadDataOperation.swift index 4df8672f9d..2e56183048 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadDataOperation.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadDataOperation.swift @@ -84,13 +84,18 @@ class AWSS3StorageDownloadDataOperation: 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) storageService.download(serviceKey: serviceKey, fileURL: nil, accelerate: accelerate) { [weak self] event in self?.onServiceEvent(event: event) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadFileOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadFileOperation.swift index 88616953c1..86d75956b6 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadFileOperation.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageDownloadFileOperation.swift @@ -27,7 +27,6 @@ class AWSS3StorageDownloadFileOperation: AmplifyInProcessReportingOperation< let storageConfiguration: AWSS3StoragePluginConfiguration let storageService: AWSS3StorageServiceBehavior let authService: AWSAuthServiceBehavior - var storageTaskReference: StorageTaskReference? // Serial queue for synchronizing access to `storageTaskReference`. @@ -38,7 +37,8 @@ class AWSS3StorageDownloadFileOperation: AmplifyInProcessReportingOperation< storageService: AWSS3StorageServiceBehavior, authService: AWSAuthServiceBehavior, progressListener: InProcessListener? = nil, - resultListener: ResultListener? = nil) { + resultListener: ResultListener? = nil + ) { self.storageConfiguration = storageConfiguration self.storageService = storageService @@ -87,15 +87,23 @@ class AWSS3StorageDownloadFileOperation: 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: 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) - storageService.download(serviceKey: serviceKey, fileURL: self.request.local, accelerate: accelerate) { [weak self] event in + storageService.download( + serviceKey: serviceKey, + fileURL: self.request.local, + accelerate: accelerate + ) { [weak self] event in self?.onServiceEvent(event: event) } } catch { diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadDataRequest+Validate.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadDataRequest+Validate.swift index ef1ac970dc..61a8861712 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadDataRequest+Validate.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadDataRequest+Validate.swift @@ -11,6 +11,11 @@ import Amplify extension StorageDownloadDataRequest { /// 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.validateTargetIdentityId(options.targetIdentityId, accessLevel: options.accessLevel) { return error diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadFileRequest+Validate.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadFileRequest+Validate.swift index 92706e9c79..a6174b8521 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadFileRequest+Validate.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageDownloadFileRequest+Validate.swift @@ -11,6 +11,11 @@ import Amplify extension StorageDownloadFileRequest { /// 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.validateTargetIdentityId(options.targetIdentityId, accessLevel: options.accessLevel) { return error diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift index 94bd595354..e8fe27bd60 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -10,25 +10,41 @@ import Amplify import AWSPluginsCore extension StoragePath { - func resolvePath() async throws -> String { - if self is IdentityIdStoragePath { - let authService = AWSAuthService() - let identityId = try await authService.getIdentityID() - let path = pathResolver(identityId) + func resolvePath(authService: AWSAuthServiceBehavior? = nil) async throws -> String { + if self is IdentityIDStoragePath { + let authService = authService ?? AWSAuthService() + guard let identityId = try await authService.getIdentityID() as? Input else { + throw StorageError.configuration( + "Unable to resolve identity id as a storage path input type", + "Please verify that storage is configured correctly", + nil + ) + } + let path = resolve(identityId) try validate(path) return path - } else { - let path = pathResolver("") + } else if self is StringStoragePath { + guard let input = "" as? Input else { + throw StorageError.unknown( + "Unable to resolve StringStoragePath resolver input", + nil + ) + } + let path = resolve(input) try validate(path) return path + } else { + let errorDescription = "The StoragePath specified is not supported" + let recoverySuggestion = "Please specify a StoragePath from string or from identityID." + throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) } } func validate(_ path: String) throws { if !path.hasPrefix("/") { let errorDescription = "Invalid StoragePath specified." - let recoverySuggestion = "Please specify a valid StoragePath that contains the prefix /." - throw StorageError.validation(path, errorDescription, recoverySuggestion, nil) + let recoverySuggestion = "Please specify a valid StoragePath that contains the prefix / " + throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) } } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift index b87d1cd939..d514ef2f69 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift @@ -182,5 +182,179 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { mockStorageService.verifyDownload(serviceKey: expectedServiceKey, fileURL: url) } + /// 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() { + let path = StringStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// 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() { + let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// 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() { + let path = InvalidCustomStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Download File Operation + /// When: The operation is executed with a request that has an valid StringStoragePath + /// Then: The operation will succeed + func testDownloadFileOperationWithStringStoragePathSucceeds() async throws { + let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceDownloadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completed(nil)] + let url = URL(fileURLWithPath: "path") + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + case .failure(let error): + XCTFail("Unexpected error on operation: \(error)") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: url) + } + + /// 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)" }) + let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceDownloadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completed(nil)] + let url = URL(fileURLWithPath: "path") + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + case .failure(let error): + XCTFail("Unexpected error on operation: \(error)") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: url) + } + // TODO: missing unit tests for pause resume and cancel. do we create a mock of the StorageTaskReference? } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift index 5acea18c20..d3ec457abc 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift @@ -174,5 +174,170 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { mockStorageService.verifyDownload(serviceKey: expectedServiceKey, fileURL: nil) } + /// Given: Storage Download Data 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() { + let path = StringStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Download Data 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() { + let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Download Data 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() { + let path = InvalidCustomStoragePath(resolve: { _ in return "my/path" }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Download Data Operation + /// When: The operation is executed with a request that has an valid StringStoragePath + /// Then: The operation will succeed + func testDownloadDataOperationWithStringStoragePathSucceeds() async throws { + let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceDownloadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completed(Data())] + let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + case .failure(let error): + XCTFail("Unexpected event invoked on operation: \(error)") + } + }) + + operation.start() + + await fulfillment(of: [inProcessInvoked, completeInvoked], timeout: 1) + XCTAssertTrue(operation.isFinished) + mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: nil) + } + + /// Given: Storage Download Data Operation + /// When: The operation is executed with a request that has an valid IdentityIDStoragePath + /// Then: The operation will succeed + func testDownloadDataOperationWithIdentityIDStoragePathSucceeds() async throws { + let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceDownloadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completed(Data())] + let path = IdentityIDStoragePath(resolve: { id in return "/public/\(self.testKey)" }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + case .failure(let error): + XCTFail("Unexpected event invoked on operation: \(error)") + } + }) + + operation.start() + + await fulfillment(of: [inProcessInvoked, completeInvoked], timeout: 1) + XCTAssertTrue(operation.isFinished) + mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: nil) + } + // TODO: missing unit tets for pause resume and cancel. do we create a mock of the StorageTaskReference? } + +struct InvalidCustomStoragePath: StoragePath { + var resolve: (String) -> String +} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageDownloadFileRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageDownloadFileRequestTests.swift index d04e4d466d..f66b40c2f9 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageDownloadFileRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageDownloadFileRequestTests.swift @@ -6,7 +6,7 @@ // import XCTest -import Amplify +@testable import Amplify @testable import AWSS3StoragePlugin class StorageDownloadFileRequestTests: XCTestCase { @@ -95,4 +95,20 @@ class StorageDownloadFileRequestTests: XCTestCase { XCTAssertEqual(description, StorageErrorConstants.keyIsEmpty.errorDescription) XCTAssertEqual(recovery, StorageErrorConstants.keyIsEmpty.recoverySuggestion) } + + /// Given: StorageDownloadFileRequest with an invalid StringStoragePath + /// When: Request validation is executed + /// Then: There is no error returned even though the storage path is invalid + /// There is no error because the path validation is done at operation execution time and not part of the request + func testValidateWithStoragePath() { + let options = StorageDownloadFileRequest.Options(accessLevel: .private, + targetIdentityId: "", + pluginOptions: testPluginOptions) + let path = StringStoragePath(resolve: {_ in "my/path"}) + let request = StorageDownloadFileRequest(path: path, local: testURL, options: options) + + let storageErrorOptional = request.validate() + + XCTAssertNil(storageErrorOptional) + } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageGetDataRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageGetDataRequestTests.swift index 1aee240bb7..b09a29cea4 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageGetDataRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageGetDataRequestTests.swift @@ -6,7 +6,7 @@ // import XCTest -import Amplify +@testable import Amplify @testable import AWSS3StoragePlugin class StorageDownloadDataRequestTests: XCTestCase { @@ -94,4 +94,20 @@ class StorageDownloadDataRequestTests: XCTestCase { XCTAssertEqual(description, StorageErrorConstants.keyIsEmpty.errorDescription) XCTAssertEqual(recovery, StorageErrorConstants.keyIsEmpty.recoverySuggestion) } + + /// Given: StorageDownloadDataRequest with an invalid StringStoragePath + /// When: Request validation is executed + /// Then: There is no error returned even though the storage path is invalid + /// There is no error because the path validation is done at operation execution time and not part of the request + func testValidateWithStoragePath() { + let options = StorageDownloadDataRequest.Options(accessLevel: .private, + targetIdentityId: "", + pluginOptions: testPluginOptions) + let path = StringStoragePath(resolve: { input in return "my/path/"}) + let request = StorageDownloadDataRequest(path: path, options: options) + + let storageErrorOptional = request.validate() + + XCTAssertNil(storageErrorOptional) + } } diff --git a/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift b/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift index cccfabe016..bc6255567f 100644 --- a/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift +++ b/AmplifyTestCommon/Mocks/MockStorageCategoryPlugin.swift @@ -180,7 +180,7 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { false } - func getURL(path: StoragePath, options: StorageGetURLRequest.Options?) async throws -> URL { + func getURL(path: any StoragePath, options: StorageGetURLRequest.Options?) async throws -> URL { notify("getURL") let options = options ?? StorageGetURLRequest.Options() let request = StorageGetURLRequest(key: key, options: options) @@ -189,25 +189,25 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { return try await taskAdapter.value } - func downloadData(path: StoragePath, options: StorageDownloadDataRequest.Options?) -> StorageDownloadDataTask { + func downloadData(path: any StoragePath, options: StorageDownloadDataRequest.Options?) -> StorageDownloadDataTask { notify("downloadData") let options = options ?? StorageDownloadDataRequest.Options() - let request = StorageDownloadDataRequest(key: key, options: options) + let request = StorageDownloadDataRequest(path: path, options: options) let operation = MockStorageDownloadDataOperation(request: request) let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) return taskAdapter } - func downloadFile(path: StoragePath, local: URL, options: StorageDownloadFileRequest.Options?) -> StorageDownloadFileTask { + func downloadFile(path: any StoragePath, local: URL, options: StorageDownloadFileRequest.Options?) -> StorageDownloadFileTask { notify("downloadFile") let options = options ?? StorageDownloadFileRequest.Options() - let request = StorageDownloadFileRequest(key: key, local: local, options: options) + let request = StorageDownloadFileRequest(path: path, local: local, options: options) let operation = MockStorageDownloadFileOperation(request: request) let taskAdapter = AmplifyInProcessReportingOperationTaskAdapter(operation: operation) return taskAdapter } - func uploadData(path: StoragePath, data: Data, options: StorageUploadDataRequest.Options?) -> StorageUploadDataTask { + func uploadData(path: any StoragePath, data: Data, options: StorageUploadDataRequest.Options?) -> StorageUploadDataTask { notify("uploadData") let options = options ?? StorageUploadDataRequest.Options() let request = StorageUploadDataRequest(key: key, data: data, options: options) @@ -216,7 +216,7 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { return taskAdapter } - func uploadFile(path: StoragePath, local: URL, options: StorageUploadFileRequest.Options?) -> StorageUploadFileTask { + func uploadFile(path: any StoragePath, local: URL, options: StorageUploadFileRequest.Options?) -> StorageUploadFileTask { notify("uploadFile") let options = options ?? StorageUploadFileRequest.Options() let request = StorageUploadFileRequest(key: key, local: local, options: options) @@ -225,7 +225,7 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { return taskAdapter } - func remove(path: StoragePath, options: StorageRemoveRequest.Options?) async throws -> String { + func remove(path: any StoragePath, options: StorageRemoveRequest.Options?) async throws -> String { notify("remove") let options = options ?? StorageRemoveRequest.Options() let request = StorageRemoveRequest(key: key, options: options) @@ -234,7 +234,7 @@ class MockStorageCategoryPlugin: MessageReporter, StorageCategoryPlugin { return try await taskAdapter.value } - func list(path: StoragePath, options: StorageListRequest.Options?) async throws -> StorageListResult { + func list(path: any StoragePath, options: StorageListRequest.Options?) async throws -> StorageListResult { notify("list") let options = options ?? StorageListRequest.Options() let request = StorageListRequest(options: options) diff --git a/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift b/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift index 6a81984fcb..0a5b3a01d1 100644 --- a/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift +++ b/AmplifyTests/CategoryTests/Hub/AmplifyOperationHubTests.swift @@ -301,7 +301,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult func getURL( - path: StoragePath, + path: any StoragePath, options: StorageGetURLOperation.Request.Options? ) async throws -> URL { let options = options ?? StorageGetURLRequest.Options() @@ -313,7 +313,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult public func downloadData( - path: StoragePath, + path: any StoragePath, options: StorageDownloadDataOperation.Request.Options? = nil ) -> StorageDownloadDataTask { let options = options ?? StorageDownloadDataRequest.Options() @@ -325,7 +325,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult public func downloadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageDownloadFileOperation.Request.Options? ) -> StorageDownloadFileTask { @@ -338,7 +338,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult public func uploadData( - path: StoragePath, + path: any StoragePath, data: Data, options: StorageUploadDataOperation.Request.Options? ) -> StorageUploadDataTask { @@ -351,7 +351,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult public func uploadFile( - path: StoragePath, + path: any StoragePath, local: URL, options: StorageUploadFileOperation.Request.Options? ) -> StorageUploadFileTask { @@ -364,7 +364,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult public func remove( - path: StoragePath, + path: any StoragePath, options: StorageRemoveRequest.Options? = nil ) async throws -> String { let options = options ?? StorageRemoveRequest.Options() @@ -376,7 +376,7 @@ class MockDispatchingStoragePlugin: StorageCategoryPlugin { @discardableResult func list( - path: StoragePath, + path: any StoragePath, options: StorageListOperation.Request.Options? ) async throws -> StorageListResult { let options = options ?? StorageListRequest.Options() diff --git a/AmplifyTests/CategoryTests/Storage/StoragePathTests.swift b/AmplifyTests/CategoryTests/Storage/StoragePathTests.swift new file mode 100644 index 0000000000..6dc34ed703 --- /dev/null +++ b/AmplifyTests/CategoryTests/Storage/StoragePathTests.swift @@ -0,0 +1,35 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import XCTest + +@testable import Amplify +@testable import AmplifyTestCommon + +class StoragePathTests: XCTestCase { + + /// Given: StringStoragePath object + /// When: resolve is called + /// Then: a string storage path is returned + func testResolveStringStoragePath() { + let expectedResult = "/my/path" + let path = StringStoragePath(resolve: { input in return expectedResult}) + let result = path.resolve("input") + XCTAssertEqual(result, expectedResult) + } + + /// Given: IdentityIDStoragePath object + /// When: resolve is called + /// Then: a string storage path is returned with the identity id included in the path + func testResolveIdentityIDStoragePath() { + let identityID = "123" + let expectedResult = "/my/\(identityID)/path" + let path = IdentityIDStoragePath(resolve: { id in return "/my/\(id)/path"}) + let result = path.resolve(identityID) + XCTAssertEqual(result, expectedResult) + } +} From c13dfbe4db827cc20d23949db605b2d95862f1d5 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Tue, 19 Mar 2024 13:26:40 -0400 Subject: [PATCH 04/12] feat(Storage): Refactor GetURL API to include `path` (#3573) --- .../Request/StorageGetURLRequest.swift | 47 ++++++-- ...SS3StoragePlugin+AsyncClientBehavior.swift | 30 +---- ...orageService+GetPreSignedURLBehavior.swift | 2 +- .../Tasks/AWSS3torageGetURLTask.swift | 68 +++++++++++ .../AWSS3StorageRemoveRequestTests.swift | 1 - .../Tasks/AWSS3StorageGetURLTaskTests.swift | 106 ++++++++++++++++++ .../Tasks/AWSS3StorageRemoveTaskTests.swift | 27 +++++ 7 files changed, 243 insertions(+), 38 deletions(-) create mode 100644 AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3torageGetURLTask.swift create mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift diff --git a/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift index 453bbf847a..e8ffd22c00 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageGetURLRequest.swift @@ -14,18 +14,33 @@ public struct StorageGetURLRequest: AmplifyOperationRequest { /// The unique identifier for the object in storage /// - /// - Tag: StorageListRequest.key + /// - Tag: StorageGetURLRequest.key + @available(*, deprecated, message: "Use `path` in Storage API instead of `key`") public let key: String - /// Options to adjust the behavior of this request, including plugin-options + /// The unique path for the object in storage + /// + /// - Tag: StorageGetURLRequest.path + public let path: (any StoragePath)? + + /// Options to adjust the behaviour of this request, including plugin-options /// - /// - Tag: StorageListRequest.options + /// - Tag: StorageGetURLRequest.options public let options: Options - /// - Tag: StorageListRequest.init + /// - Tag: StorageGetURLRequest.init + @available(*, deprecated, message: "Use init(path:options)") public init(key: String, options: Options) { self.key = key self.options = options + self.path = nil + } + + /// - Tag: StorageGetURLRequest.init + public init(path: any StoragePath, options: Options) { + self.key = "" + self.options = options + self.path = path } } @@ -33,29 +48,29 @@ public extension StorageGetURLRequest { /// Options to adjust the behavior of this request, including plugin-options /// - /// - Tag: StorageListRequestOptions + /// - Tag: StorageGetURLRequest.Options struct Options { /// The default amount of time before the URL expires is 18000 seconds, or 5 hours. /// - /// - Tag: StorageListRequestOptions.defaultExpireInSeconds + /// - Tag: StorageGetURLRequest.Options.defaultExpireInSeconds public static let defaultExpireInSeconds = 18_000 /// Access level of the storage system. Defaults to `public` /// - /// - Tag: StorageListRequestOptions.accessLevel + /// - Tag: StorageGetURLRequest.Options.accessLevel @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let accessLevel: StorageAccessLevel /// Target user to apply the action on. /// - /// - Tag: StorageListRequestOptions.targetIdentityId + /// - Tag: StorageGetURLRequest.Options.targetIdentityId @available(*, deprecated, message: "Use `path` in Storage API instead of `Options`") public let targetIdentityId: String? /// Number of seconds before the URL expires. Defaults to /// [defaultExpireInSeconds](x-source-tag://StorageListRequestOptions.defaultExpireInSeconds) /// - /// - Tag: StorageListRequestOptions.expires + /// - Tag: StorageGetURLRequest.Options.expires public let expires: Int /// Extra plugin specific options, only used in special circumstances when the existing options do @@ -64,10 +79,11 @@ public extension StorageGetURLRequest { /// [AWSStorageGetURLOptions](x-source-tag://AWSStorageGetURLOptions) for /// expected key/values. /// - /// - Tag: StorageListRequestOptions.pluginOptions + /// - Tag: StorageGetURLRequest.Options.pluginOptions public let pluginOptions: Any? - /// - Tag: StorageListRequestOptions.init + /// - Tag: StorageGetURLRequest.Options.init + @available(*, deprecated, message: "Use init(expires:pluginOptions)") public init(accessLevel: StorageAccessLevel = .guest, targetIdentityId: String? = nil, expires: Int = Options.defaultExpireInSeconds, @@ -77,5 +93,14 @@ public extension StorageGetURLRequest { self.expires = expires self.pluginOptions = pluginOptions } + + /// - Tag: StorageGetURLRequest.Options.init + public init(expires: Int = Options.defaultExpireInSeconds, + pluginOptions: Any? = nil) { + self.expires = expires + self.pluginOptions = pluginOptions + self.accessLevel = .guest + self.targetIdentityId = nil + } } } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index 8dd2bcb82e..8513bf98d6 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -50,31 +50,11 @@ extension AWSS3StoragePlugin { options: StorageGetURLOperation.Request.Options? = nil ) async throws -> URL { let options = options ?? StorageGetURLRequest.Options() - let path = "" //TODO: resolve path - let request = StorageGetURLRequest(key: path, options: options) - if let error = request.validate() { - throw error - } - let prefixResolver = storageConfiguration.prefixResolver ?? StorageAccessLevelAwarePrefixResolver(authService: authService) - let prefix = try await prefixResolver.resolvePrefix(for: options.accessLevel, - targetIdentityId: options.targetIdentityId) - let serviceKey = prefix + request.key - if let pluginOptions = options.pluginOptions as? AWSStorageGetURLOptions, pluginOptions.validateObjectExistence { - try await storageService.validateObjectExistence(serviceKey: serviceKey) - } - let accelerate = try AWSS3PluginOptions.accelerateValue( - pluginOptions: options.pluginOptions) - let result = try await storageService.getPreSignedURL( - serviceKey: serviceKey, - signingOperation: .getObject, - metadata: nil, - accelerate: accelerate, - expires: options.expires) - - let channel = HubChannel(from: categoryType) - let payload = HubPayload(eventName: HubPayload.EventName.Storage.getURL, context: options, data: result) - Amplify.Hub.dispatch(to: channel, payload: payload) - return result + let request = StorageGetURLRequest(path: path, options: options) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: storageService) + return try await task.value } public func downloadData( diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+GetPreSignedURLBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+GetPreSignedURLBehavior.swift index fc3eb40699..755b2416cf 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+GetPreSignedURLBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageService+GetPreSignedURLBehavior.swift @@ -21,7 +21,7 @@ extension AWSS3StorageService { key: serviceKey, signingOperation: signingOperation, metadata: metadata, - accelerate: nil, + accelerate: accelerate, expires: Int64(expires) ) } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3torageGetURLTask.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3torageGetURLTask.swift new file mode 100644 index 0000000000..df803a3b71 --- /dev/null +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3torageGetURLTask.swift @@ -0,0 +1,68 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Amplify +import Foundation +import AWSS3 +import AWSPluginsCore + +protocol StorageGetURLTask: AmplifyTaskExecution where Request == StorageGetURLRequest, Success == URL, Failure == StorageError {} + +class AWSS3StorageGetURLTask: StorageGetURLTask, DefaultLogger { + + let request: StorageGetURLRequest + let storageBehaviour: AWSS3StorageServiceBehavior + + init(_ request: StorageGetURLRequest, + storageBehaviour: AWSS3StorageServiceBehavior) { + self.request = request + self.storageBehaviour = storageBehaviour + } + + var eventName: HubPayloadEventName { + HubPayload.EventName.Storage.getURL + } + + var eventNameCategoryType: CategoryType { + .storage + } + + func execute() async throws -> URL { + guard let serviceKey = try await request.path?.resolvePath() else { + throw StorageError.validation( + "path", + "`path` is required field", + "Make sure that a valid `path` is passed for removing an object") + } + + // Validate object if needed + if let pluginOptions = request.options.pluginOptions as? AWSStorageGetURLOptions, pluginOptions.validateObjectExistence { + try await storageBehaviour.validateObjectExistence(serviceKey: serviceKey) + } + + let accelerate = try AWSS3PluginOptions.accelerateValue( + pluginOptions: request.options.pluginOptions) + + do { + return try await storageBehaviour.getPreSignedURL( + serviceKey: serviceKey, + signingOperation: .getObject, + metadata: nil, + accelerate: accelerate, + expires: request.options.expires + ) + } catch let error as StorageErrorConvertible { + throw error.storageError + } catch let error { + throw StorageError.service( + "Service error occurred.", + "Please inspect the underlying error for more details.", + error) + } + + } +} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift index f5a741846c..76d7cfbff9 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageRemoveRequestTests.swift @@ -9,7 +9,6 @@ import XCTest import Amplify @testable import AWSS3StoragePlugin -// TODO: [HS] Add path validation test cases once storage path extension is merged. class AWSS3StorageRemoveRequestTests: XCTestCase { let testTargetIdentityId = "TestTargetIdentityId" diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift new file mode 100644 index 0000000000..575b2eaf49 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift @@ -0,0 +1,106 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import XCTest +@testable import Amplify +@testable import AmplifyTestCommon +@testable import AWSPluginsCore +@testable import AWSS3StoragePlugin +@testable import AWSPluginsTestCommon +import AWSS3 + +class AWSS3StorageGetURLTaskTests: XCTestCase { + + + /// - Given: A configured Storage GetURL Task with mocked service + /// - When: AWSS3StorageGetURLTask value is invoked + /// - Then: A URL should be returned. + func testRemoveTaskSuccess() async throws { + + let somePath = "/path" + let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) + + let serviceMock = MockAWSS3StorageService() + serviceMock.getPreSignedURLHandler = { path, _, _ in + XCTAssertEqual(somePath, path) + return tempURL + } + + let request = StorageGetURLRequest( + path: StringStoragePath.fromString(somePath), options: .init()) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: serviceMock) + let value = try await task.value + XCTAssertEqual(value, tempURL) + } + + /// - Given: A configured Storage GetURL Task with mocked service, throwing `NotFound` exception + /// - When: AWSS3StorageGetURLTask value is invoked + /// - Then: A storage service error should be returned, with an underlying service error + func testRemoveTaskNoBucket() async throws { + let somePath = "/path" + + let serviceMock = MockAWSS3StorageService() + serviceMock.getPreSignedURLHandler = { _, _, _ in + throw AWSS3.NotFound() + } + + let request = StorageGetURLRequest( + path: StringStoragePath.fromString(somePath), options: .init()) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .service(_, _, let underlyingError) = storageError else { + XCTFail("Should throw a Storage service error, instead threw \(error)") + return + } + XCTAssertTrue(underlyingError is AWSS3.NotFound, + "Underlying error should be NoSuchKey, instead got \(String(describing: underlyingError))") + } + } + + /// - Given: A configured Storage GetURL Task with invalid path + /// - When: AWSS3StorageGetURLTask value is invoked + /// - Then: A storage validation error should be returned + func testGetURLTaskWithInvalidPath() async throws { + let somePath = "path" + let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) + + let serviceMock = MockAWSS3StorageService() + serviceMock.getPreSignedURLHandler = { path, _, _ in + XCTAssertEqual(somePath, path) + return tempURL + } + + let request = StorageGetURLRequest( + path: StringStoragePath.fromString(somePath), options: .init()) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } + +} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift index 22a96d6de9..1ac8651b43 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift @@ -67,4 +67,31 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { } } + /// - Given: A configured Storage Remove Task with invalid path + /// - When: AWSS3StorageRemoveTask value is invoked + /// - Then: A storage validation error should be returned + func testRemoveTaskWithInvalidPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageRemoveRequest( + path: StringStoragePath.fromString("path"), options: .init()) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } + } From 597473a1978feb7938e265d5f6ef353899bfe6e0 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Tue, 19 Mar 2024 12:58:35 -0500 Subject: [PATCH 05/12] feat(storage): update storage upload APIs to use storage path (#3574) --- .../Request/StorageDownloadDataRequest.swift | 14 +- .../Request/StorageDownloadFileRequest.swift | 12 +- .../Request/StorageUploadDataRequest.swift | 31 ++- .../Request/StorageUploadFileRequest.swift | 31 ++- .../AWSS3StorageUploadDataOperation.swift | 16 +- .../AWSS3StorageUploadFileOperation.swift | 16 +- .../StorageUploadDataRequest+Validate.swift | 6 + .../StorageUploadFileRequest+Validate.swift | 6 + ...SS3StorageDownloadFileOperationTests.swift | 13 +- .../AWSS3StorageGetDataOperationTests.swift | 28 +- .../AWSS3StoragePutDataOperationTests.swift | 191 ++++++++++++++ ...AWSS3StorageUploadFileOperationTests.swift | 243 ++++++++++++++++++ .../AWSS3StoragePutDataRequestTests.swift | 19 +- .../AWSS3StorageUploadFileRequestTests.swift | 22 +- 14 files changed, 613 insertions(+), 35 deletions(-) diff --git a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift index 91a85e20b0..1a8ed260b9 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageDownloadDataRequest.swift @@ -21,7 +21,7 @@ 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 @@ -29,7 +29,7 @@ public struct StorageDownloadDataRequest: AmplifyOperationRequest { /// - 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 @@ -37,6 +37,7 @@ public struct StorageDownloadDataRequest: AmplifyOperationRequest { self.path = nil } + /// - Tag: StorageDownloadDataRequest.init public init(path: any StoragePath, options: Options) { self.key = "" self.options = options @@ -89,6 +90,7 @@ public extension StorageDownloadDataRequest { /// /// - Tag: StorageDownloadDataRequestOptions.init + @available(*, deprecated, message: "Use init(pluginOptions)") public init(accessLevel: StorageAccessLevel = .guest, targetIdentityId: String? = nil, pluginOptions: Any? = nil) { @@ -96,5 +98,13 @@ public extension StorageDownloadDataRequest { 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 index 1738ea0212..7ec34c222f 100644 --- 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 @@ -78,6 +79,7 @@ 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) { @@ -85,5 +87,13 @@ public extension StorageDownloadFileRequest { 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 index 522f34192b..572b160bf8 100644 --- 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 } } @@ -73,16 +88,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 index 43b786cf2c..23c8d159f6 100644 --- 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 } } @@ -70,16 +85,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/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift index cbfd7ee52e..6e4288445a 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift +++ b/AmplifyPlugins/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/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift index 617602388a..92e63b5b4d 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift +++ b/AmplifyPlugins/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/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadDataRequest+Validate.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadDataRequest+Validate.swift index 4412186824..28eb48093f 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadDataRequest+Validate.swift +++ b/AmplifyPlugins/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/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadFileRequest+Validate.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadFileRequest+Validate.swift index abb99f1120..04185a8472 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Request/StorageUploadFileRequest+Validate.swift +++ b/AmplifyPlugins/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/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift index d514ef2f69..4a54f12812 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/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? diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift index d3ec457abc..8f2fbb440b 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift @@ -185,17 +185,18 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { storageConfiguration: testStorageConfiguration, storageService: mockStorageService, authService: mockAuthService, - progressListener: nil) { event in - switch event { - case .failure(let error): - guard case .validation = error else { - XCTFail("Should have failed with validation error") - return - } - failedInvoked.fulfill() - default: - XCTFail("Should have received failed event") - } + progressListener: nil + ) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } } operation.start() @@ -302,12 +303,13 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an valid IdentityIDStoragePath /// Then: The operation will succeed func testDownloadDataOperationWithIdentityIDStoragePathSucceeds() async throws { + mockAuthService.identityId = testIdentityId let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceDownloadEvents = [ StorageEvent.initiated(StorageTaskReference(task)), StorageEvent.inProcess(Progress()), StorageEvent.completed(Data())] - let path = IdentityIDStoragePath(resolve: { id in return "/public/\(self.testKey)" }) + let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) let inProcessInvoked = expectation(description: "inProgress was invoked on operation") @@ -332,7 +334,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { await fulfillment(of: [inProcessInvoked, completeInvoked], timeout: 1) XCTAssertTrue(operation.isFinished) - mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: nil) + mockStorageService.verifyDownload(serviceKey: "/public/\(testIdentityId)/\(self.testKey)", fileURL: nil) } // TODO: missing unit tets for pause resume and cancel. do we create a mock of the StorageTaskReference? diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift index c93b6975f3..d500a4450e 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift @@ -219,5 +219,196 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { metadata: metadata) } + /// Given: Storage Upload Data Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testUploadDataOperationStringStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return "my/path" }) + let failedInvoked = expectation(description: "failed was invoked on operation") + let options = StorageUploadDataRequest.Options(accessLevel: .protected) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Upload Data Operation + /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath + /// Then: The operation will fail with a validation error + func testUploadDataOperationIdentityIDStoragePathValidationError() { + let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let failedInvoked = expectation(description: "failed was invoked on operation") + let options = StorageUploadDataRequest.Options(accessLevel: .protected) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Upload Data 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 testUploadDataOperationCustomStoragePathValidationError() { + let path = InvalidCustomStoragePath(resolve: { _ in return "my/path" }) + let failedInvoked = expectation(description: "failed was invoked on operation") + let options = StorageUploadDataRequest.Options(accessLevel: .protected) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Upload Data Operation + /// When: The operation is executed with a request that has an valid StringStoragePath + /// Then: The operation will succeed + func testUploadDataOperationWithStringStoragePathSucceeds() async throws { + let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let expectedUploadSource = UploadSource.data(testData) + let metadata = ["mykey": "Value"] + + let options = StorageUploadDataRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageUploadDataOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + default: + XCTFail("Should have received completed event") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + XCTAssertEqual(mockStorageService.uploadCalled, 1) + mockStorageService.verifyUpload(serviceKey: "/public/\(self.testKey)", + key: testKey, + uploadSource: expectedUploadSource, + contentType: testContentType, + metadata: metadata) + } + + /// Given: Storage UploadData Operation + /// When: The operation is executed with a request that has an valid IdentityIDStoragePath + /// Then: The operation will succeed + func testUploadDataOperationWithIdentityIDStoragePathSucceeds() async throws { + mockAuthService.identityId = testIdentityId + let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let expectedUploadSource = UploadSource.data(testData) + let metadata = ["mykey": "Value"] + + let options = StorageUploadDataRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageUploadDataOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + default: + XCTFail("Should have received completed event") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + XCTAssertEqual(mockStorageService.uploadCalled, 1) + mockStorageService.verifyUpload(serviceKey: "/public/\(testIdentityId)/\(testKey)", + key: testKey, + uploadSource: expectedUploadSource, + contentType: testContentType, + metadata: metadata) + } + // TODO: test pause, resume, canel, etc. } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift index 52800e958e..0105e8e721 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift @@ -255,5 +255,248 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { metadata: metadata) } + /// Given: Storage Upload 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 testUploadFileOperationStringStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return "my/path" }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Upload 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 testUploadFileOperationIdentityIDStoragePathValidationError() { + let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// 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 testUploadFileOperationCustomStoragePathValidationError() { + let path = InvalidCustomStoragePath(resolve: { _ in return "my/path" }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + + /// Given: Storage Download File Operation + /// When: The operation is executed with a request that has an valid StringStoragePath + /// Then: The operation will succeed + func testUploadFileOperationWithStringStoragePathSucceeds() async throws { + let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageUploadFileOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + default: + XCTFail("Should have received completed event") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + XCTAssertEqual(mockStorageService.uploadCalled, 1) + mockStorageService.verifyUpload(serviceKey: "/public/\(testKey)", + key: testKey, + uploadSource: expectedUploadSource, + contentType: testContentType, + metadata: metadata) + } + + /// Given: Storage Upload File Operation + /// When: The operation is executed with a request that has an valid IdentityIDStoragePath + /// Then: The operation will succeed + func testUploadFileOperationWithIdentityIDStoragePathSucceeds() async throws { + let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + let inProcessInvoked = expectation(description: "inProgress was invoked on operation") + let completeInvoked = expectation(description: "complete was invoked on operation") + let operation = AWSS3StorageUploadFileOperation( + request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: { _ in + inProcessInvoked.fulfill() + }, resultListener: { result in + switch result { + case .success: + completeInvoked.fulfill() + default: + XCTFail("Should have received completed event") + } + }) + + operation.start() + + await waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + XCTAssertEqual(mockStorageService.uploadCalled, 1) + mockStorageService.verifyUpload(serviceKey: "/public/\(testIdentityId)/\(testKey)", + key: testKey, + uploadSource: expectedUploadSource, + contentType: testContentType, + metadata: metadata) + } + // TODO: test pause, resume, canel, etc. } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StoragePutDataRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StoragePutDataRequestTests.swift index 5fadb24165..377ca07deb 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StoragePutDataRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StoragePutDataRequestTests.swift @@ -6,7 +6,7 @@ // import XCTest -import Amplify +@testable import Amplify @testable import AWSS3StoragePlugin class AWSS3StorageUploadDataRequestTests: XCTestCase { @@ -103,6 +103,23 @@ class AWSS3StorageUploadDataRequestTests: XCTestCase { XCTAssertEqual(recovery, StorageErrorConstants.metadataKeysInvalid.recoverySuggestion) } + /// Given: StorageUploadDataRequest with an invalid StringStoragePath + /// When: Request validation is executed + /// Then: There is no error returned even though the storage path is invalid + /// There is no error because the path validation is done at operation execution time and not part of the request + func testValidateWithStoragePath() { + let path = StringStoragePath(resolve: {_ in "my/path"}) + let options = StorageUploadDataRequest.Options(accessLevel: .protected, + metadata: testMetadata, + contentType: testContentType, + pluginOptions: testPluginOptions) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + + let storageErrorOptional = request.validate() + + XCTAssertNil(storageErrorOptional) + } + // TODO: testValidateMetadataValuesTooLarge // func testValidateMetadataValuesTooLarge() { // diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageUploadFileRequestTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageUploadFileRequestTests.swift index 16ee59e0af..57bcd34447 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageUploadFileRequestTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Request/AWSS3StorageUploadFileRequestTests.swift @@ -6,7 +6,7 @@ // import XCTest -import Amplify +@testable import Amplify @testable import AWSS3StoragePlugin class AWSS3StorageUploadFileRequestTests: XCTestCase { @@ -115,6 +115,26 @@ class AWSS3StorageUploadFileRequestTests: XCTestCase { XCTAssertEqual(recovery, StorageErrorConstants.metadataKeysInvalid.recoverySuggestion) } + /// Given: StorageUploadFileRequest with an invalid StringStoragePath + /// When: Request validation is executed + /// Then: There is no error returned even though the storage path is invalid + /// There is no error because the path validation is done at operation execution time and not part of the request + func testValidateWithStoragePath() { + let path = StringStoragePath(resolve: {_ in "my/path"}) + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: testMetadata, + contentType: testContentType, + pluginOptions: testPluginOptions) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let storageErrorOptional = request.validate() + + XCTAssertNil(storageErrorOptional) + } + // TODO: testValidateMetadataValuesTooLarge // func testValidateMetadataValuesTooLarge() { // From 836441298d6629ea614726adfb263038e16faac2 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Thu, 21 Mar 2024 12:09:25 -0400 Subject: [PATCH 06/12] feat(Storage): Refactor list objects API to include `path` (#3580) * feat(Storage): Refactor list objects API to include `path` * working on review comments --- .../Request/StorageListRequest.swift | 13 +++ ...SS3StoragePlugin+AsyncClientBehavior.swift | 14 +-- .../Storage/AWSS3StorageServiceBehavior.swift | 1 + .../Tasks/AWSS3StorageListObjectsTask.swift | 73 ++++++++++++ .../Tasks/AWSS3StorageGetURLTaskTests.swift | 4 +- .../AWSS3StorageListObjectsTaskTests.swift | 106 ++++++++++++++++++ 6 files changed, 201 insertions(+), 10 deletions(-) create mode 100644 AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift create mode 100644 AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift diff --git a/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift b/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift index 5689e6b47c..6cc7dbb496 100644 --- a/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift +++ b/Amplify/Categories/Storage/Operation/Request/StorageListRequest.swift @@ -15,9 +15,22 @@ public struct StorageListRequest: AmplifyOperationRequest { /// - Tag: StorageListRequest public let options: Options + /// The unique path for the object in storage + /// + /// - Tag: StorageListRequest.path + public let path: (any StoragePath)? + /// - Tag: StorageListRequest.init + @available(*, deprecated, message: "Use init(path:options)") public init(options: Options) { self.options = options + self.path = nil + } + + /// - Tag: StorageListRequest.init + public init(path: any StoragePath, options: Options) { + self.options = options + self.path = path } } diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index 8513bf98d6..06f2db02a6 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -251,14 +251,12 @@ extension AWSS3StoragePlugin { options: StorageListRequest.Options? = nil ) async throws -> StorageListResult { let options = options ?? StorageListRequest.Options() - let prefix = "" //TODO: resolve path - let result = try await storageService.list(prefix: prefix, options: options) - - let channel = HubChannel(from: categoryType) - let payload = HubPayload(eventName: HubPayload.EventName.Storage.list, context: options, data: result) - Amplify.Hub.dispatch(to: channel, payload: payload) - - return result + let request = StorageListRequest(path: path, options: options) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: storageConfiguration, + storageBehaviour: storageService) + return try await task.value } public func handleBackgroundEvents(identifier: String) async -> Bool { diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift index b1e274a609..6491546d8d 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Service/Storage/AWSS3StorageServiceBehavior.swift @@ -70,6 +70,7 @@ protocol AWSS3StorageServiceBehavior { accelerate: Bool?, onEvent: @escaping StorageServiceMultiPartUploadEventHandler) + @available(*, deprecated, message: "Use `AWSS3StorageListObjectsTask` instead") func list(prefix: String, options: StorageListRequest.Options) async throws -> StorageListResult diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift new file mode 100644 index 0000000000..6da4ce23d0 --- /dev/null +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift @@ -0,0 +1,73 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import Amplify +import Foundation +import AWSS3 +import AWSPluginsCore + +protocol StorageListObjectsTask: AmplifyTaskExecution where Request == StorageListRequest, Success == StorageListResult, Failure == StorageError {} + +class AWSS3StorageListObjectsTask: StorageListObjectsTask, DefaultLogger { + + let request: StorageListRequest + let storageConfiguration: AWSS3StoragePluginConfiguration + let storageBehaviour: AWSS3StorageServiceBehavior + + init(_ request: StorageListRequest, + storageConfiguration: AWSS3StoragePluginConfiguration, + storageBehaviour: AWSS3StorageServiceBehavior) { + self.request = request + self.storageConfiguration = storageConfiguration + self.storageBehaviour = storageBehaviour + } + + var eventName: HubPayloadEventName { + HubPayload.EventName.Storage.list + } + + var eventNameCategoryType: CategoryType { + .storage + } + + func execute() async throws -> StorageListResult { + guard let path = try await request.path?.resolvePath() else { + throw StorageError.validation( + "path", + "`path` is required for removing an object", + "Make sure that a valid `path` is passed for removing an object") + } + let input = ListObjectsV2Input(bucket: storageBehaviour.bucket, + continuationToken: request.options.nextToken, + delimiter: nil, + maxKeys: Int(request.options.pageSize), + prefix: path, + startAfter: nil) + do { + let response = try await storageBehaviour.client.listObjectsV2(input: input) + let contents: S3BucketContents = response.contents ?? [] + let items = try contents.map { s3Object in + guard let key = s3Object.key else { + throw StorageError.unknown("Missing key in response") + } + return StorageListResult.Item( + path: path, + key: key, + eTag: s3Object.eTag, + lastModified: s3Object.lastModified) + } + return StorageListResult(items: items, nextToken: response.nextContinuationToken) + } catch let error as StorageErrorConvertible { + throw error.storageError + } catch { + throw StorageError.service( + "Service error occurred.", + "Please inspect the underlying error for more details.", + error) + } + } +} diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift index 575b2eaf49..4b74e24f01 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift @@ -19,7 +19,7 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { /// - Given: A configured Storage GetURL Task with mocked service /// - When: AWSS3StorageGetURLTask value is invoked /// - Then: A URL should be returned. - func testRemoveTaskSuccess() async throws { + func testGetURLTaskSuccess() async throws { let somePath = "/path" let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) @@ -42,7 +42,7 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { /// - Given: A configured Storage GetURL Task with mocked service, throwing `NotFound` exception /// - When: AWSS3StorageGetURLTask value is invoked /// - Then: A storage service error should be returned, with an underlying service error - func testRemoveTaskNoBucket() async throws { + func testGetURLTaskNoBucket() async throws { let somePath = "/path" let serviceMock = MockAWSS3StorageService() diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift new file mode 100644 index 0000000000..f58fe03e7a --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift @@ -0,0 +1,106 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +import XCTest +@testable import Amplify +@testable import AmplifyTestCommon +@testable import AWSPluginsCore +@testable import AWSS3StoragePlugin +@testable import AWSPluginsTestCommon +import AWSS3 + +class AWSS3StorageListObjectsTaskTests: XCTestCase { + + /// - Given: A configured Storage List Objects Task with mocked service + /// - When: AWSS3StorageListObjectsTask value is invoked + /// - Then: A list of keys should be returned. + func testListObjectsTaskSuccess() async throws { + let serviceMock = MockAWSS3StorageService() + let client = serviceMock.client as! MockS3Client + client.listObjectsV2Handler = { input in + return .init( + contents: [ + .init(eTag: "tag", key: "key", lastModified: Date()), + .init(eTag: "tag", key: "key", lastModified: Date())], + nextContinuationToken: "continuationToken" + ) + } + + let request = StorageListRequest( + path: StringStoragePath.fromString("/path"), options: .init()) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + let value = try await task.value + XCTAssertEqual(value.items.count, 2) + XCTAssertEqual(value.nextToken, "continuationToken") + XCTAssertEqual(value.items[0].eTag, "tag") + XCTAssertEqual(value.items[0].key, "key") + XCTAssertNotNil(value.items[0].lastModified) + + } + + /// - Given: A configured ListObjects Remove Task with mocked service, throwing `NoSuchKey` exception + /// - When: AWSS3StorageListObjectsTask value is invoked + /// - Then: A storage service error should be returned, with an underlying service error + func testListObjectsTaskNoBucket() async throws { + let serviceMock = MockAWSS3StorageService() + let client = serviceMock.client as! MockS3Client + client.listObjectsV2Handler = { input in + throw AWSS3.NoSuchKey() + } + + let request = StorageListRequest( + path: StringStoragePath.fromString("/path"), options: .init()) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .service(_, _, let underlyingError) = storageError else { + XCTFail("Should throw a Storage service error, instead threw \(error)") + return + } + XCTAssertTrue(underlyingError is AWSS3.NoSuchKey, + "Underlying error should be NoSuchKey, instead got \(String(describing: underlyingError))") + } + } + + /// - Given: A configured Storage ListObjects Task with invalid path + /// - When: AWSS3StorageListObjectsTask value is invoked + /// - Then: A storage validation error should be returned + func testListObjectsTaskWithInvalidPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageListRequest( + path: StringStoragePath.fromString("path"), options: .init()) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } + +} From b8ec8d86ee4c0b0dd308213cc178c36c7b085ab9 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Thu, 21 Mar 2024 13:21:38 -0500 Subject: [PATCH 07/12] chore(storage): update storage path validation rule (#3579) --- .../Support/Internal/StoragePath+Extensions.swift | 4 ++-- .../AWSS3StorageDownloadFileOperationTests.swift | 12 ++++++------ .../AWSS3StorageGetDataOperationTests.swift | 12 ++++++------ .../AWSS3StoragePutDataOperationTests.swift | 12 ++++++------ .../AWSS3StorageUploadFileOperationTests.swift | 12 ++++++------ .../Tasks/AWSS3StorageGetURLTaskTests.swift | 6 +++--- .../Tasks/AWSS3StorageListObjectsTaskTests.swift | 6 +++--- .../Tasks/AWSS3StorageRemoveTaskTests.swift | 8 ++++---- 8 files changed, 36 insertions(+), 36 deletions(-) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift index e8fe27bd60..bdc2ebece4 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -41,9 +41,9 @@ extension StoragePath { } func validate(_ path: String) throws { - if !path.hasPrefix("/") { + if path.hasPrefix("/") { let errorDescription = "Invalid StoragePath specified." - let recoverySuggestion = "Please specify a valid StoragePath that contains the prefix / " + let recoverySuggestion = "Please specify a valid StoragePath that does not contain the prefix / " throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift index 4a54f12812..98b15f5954 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift @@ -186,7 +186,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid StringStoragePath /// Then: The operation will fail with a validation error func testDownloadFileOperationStringStoragePathValidationError() { - let path = StringStoragePath(resolve: { _ in return "my/path" }) + let path = StringStoragePath(resolve: { _ in return "/my/path" }) let request = StorageDownloadFileRequest(path: path, local: testURL, options: StorageDownloadFileRequest.Options()) @@ -218,7 +218,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error func testDownloadFileOperationIdentityIDStoragePathValidationError() { - let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let path = IdentityIDStoragePath(resolve: { _ in return "/my/path" }) let request = StorageDownloadFileRequest(path: path, local: testURL, options: StorageDownloadFileRequest.Options()) @@ -282,7 +282,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an valid StringStoragePath /// Then: The operation will succeed func testDownloadFileOperationWithStringStoragePathSucceeds() async throws { - let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let path = StringStoragePath(resolve: { _ in return "public/\(self.testKey)" }) let task = StorageTransferTask(transferType: .download(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceDownloadEvents = [ StorageEvent.initiated(StorageTaskReference(task)), @@ -314,7 +314,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) - mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: url) + mockStorageService.verifyDownload(serviceKey: "public/\(self.testKey)", fileURL: url) } /// Given: Storage Download File Operation @@ -322,7 +322,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { /// Then: The operation will succeed func testDownloadFileOperationWithIdentityIDStoragePathSucceeds() async throws { mockAuthService.identityId = testIdentityId - let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + 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)), @@ -354,7 +354,7 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) - mockStorageService.verifyDownload(serviceKey: "/public/\(testIdentityId)/\(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? diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift index 8f2fbb440b..23a28ee935 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift @@ -178,7 +178,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid StringStoragePath /// Then: The operation will fail with a validation error func testDownloadDataOperationStringStoragePathValidationError() { - let path = StringStoragePath(resolve: { _ in return "my/path" }) + let path = StringStoragePath(resolve: { _ in return "/my/path" }) let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) let failedInvoked = expectation(description: "failed was invoked on operation") let operation = AWSS3StorageDownloadDataOperation(request, @@ -208,7 +208,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error func testDownloadDataOperationIdentityIdStoragePathValidationError() { - let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let path = IdentityIDStoragePath(resolve: { _ in return "/my/path" }) let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) let failedInvoked = expectation(description: "failed was invoked on operation") let operation = AWSS3StorageDownloadDataOperation(request, @@ -271,7 +271,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { StorageEvent.initiated(StorageTaskReference(task)), StorageEvent.inProcess(Progress()), StorageEvent.completed(Data())] - let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let path = StringStoragePath(resolve: { _ in return "public/\(self.testKey)" }) let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) let inProcessInvoked = expectation(description: "inProgress was invoked on operation") @@ -296,7 +296,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { await fulfillment(of: [inProcessInvoked, completeInvoked], timeout: 1) XCTAssertTrue(operation.isFinished) - mockStorageService.verifyDownload(serviceKey: "/public/\(self.testKey)", fileURL: nil) + mockStorageService.verifyDownload(serviceKey: "public/\(self.testKey)", fileURL: nil) } /// Given: Storage Download Data Operation @@ -309,7 +309,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { StorageEvent.initiated(StorageTaskReference(task)), StorageEvent.inProcess(Progress()), StorageEvent.completed(Data())] - let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + let path = IdentityIDStoragePath(resolve: { id in return "public/\(id)/\(self.testKey)" }) let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) let inProcessInvoked = expectation(description: "inProgress was invoked on operation") @@ -334,7 +334,7 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { await fulfillment(of: [inProcessInvoked, completeInvoked], timeout: 1) XCTAssertTrue(operation.isFinished) - mockStorageService.verifyDownload(serviceKey: "/public/\(testIdentityId)/\(self.testKey)", fileURL: nil) + mockStorageService.verifyDownload(serviceKey: "public/\(testIdentityId)/\(self.testKey)", fileURL: nil) } // TODO: missing unit tets for pause resume and cancel. do we create a mock of the StorageTaskReference? diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift index d500a4450e..faeb9fc862 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift @@ -223,7 +223,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid StringStoragePath /// Then: The operation will fail with a validation error func testUploadDataOperationStringStoragePathValidationError() { - let path = StringStoragePath(resolve: { _ in return "my/path" }) + let path = StringStoragePath(resolve: { _ in return "/my/path" }) let failedInvoked = expectation(description: "failed was invoked on operation") let options = StorageUploadDataRequest.Options(accessLevel: .protected) let request = StorageUploadDataRequest(path: path, data: testData, options: options) @@ -254,7 +254,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error func testUploadDataOperationIdentityIDStoragePathValidationError() { - let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let path = IdentityIDStoragePath(resolve: { _ in return "/my/path" }) let failedInvoked = expectation(description: "failed was invoked on operation") let options = StorageUploadDataRequest.Options(accessLevel: .protected) let request = StorageUploadDataRequest(path: path, data: testData, options: options) @@ -316,7 +316,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an valid StringStoragePath /// Then: The operation will succeed func testUploadDataOperationWithStringStoragePathSucceeds() async throws { - let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let path = StringStoragePath(resolve: { _ in return "public/\(self.testKey)" }) let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ StorageEvent.initiated(StorageTaskReference(task)), @@ -354,7 +354,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) XCTAssertEqual(mockStorageService.uploadCalled, 1) - mockStorageService.verifyUpload(serviceKey: "/public/\(self.testKey)", + mockStorageService.verifyUpload(serviceKey: "public/\(self.testKey)", key: testKey, uploadSource: expectedUploadSource, contentType: testContentType, @@ -366,7 +366,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { /// Then: The operation will succeed func testUploadDataOperationWithIdentityIDStoragePathSucceeds() async throws { mockAuthService.identityId = testIdentityId - let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + let path = IdentityIDStoragePath(resolve: { id in return "public/\(id)/\(self.testKey)" }) let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ StorageEvent.initiated(StorageTaskReference(task)), @@ -403,7 +403,7 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) XCTAssertEqual(mockStorageService.uploadCalled, 1) - mockStorageService.verifyUpload(serviceKey: "/public/\(testIdentityId)/\(testKey)", + mockStorageService.verifyUpload(serviceKey: "public/\(testIdentityId)/\(testKey)", key: testKey, uploadSource: expectedUploadSource, contentType: testContentType, diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift index 0105e8e721..87183415e6 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift @@ -259,7 +259,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid StringStoragePath /// Then: The operation will fail with a validation error func testUploadFileOperationStringStoragePathValidationError() { - let path = StringStoragePath(resolve: { _ in return "my/path" }) + let path = StringStoragePath(resolve: { _ in return "/my/path" }) mockAuthService.identityId = testIdentityId let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ @@ -305,7 +305,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error func testUploadFileOperationIdentityIDStoragePathValidationError() { - let path = IdentityIDStoragePath(resolve: { _ in return "my/path" }) + let path = IdentityIDStoragePath(resolve: { _ in return "/my/path" }) mockAuthService.identityId = testIdentityId let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ @@ -397,7 +397,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an valid StringStoragePath /// Then: The operation will succeed func testUploadFileOperationWithStringStoragePathSucceeds() async throws { - let path = StringStoragePath(resolve: { _ in return "/public/\(self.testKey)" }) + let path = StringStoragePath(resolve: { _ in return "public/\(self.testKey)" }) mockAuthService.identityId = testIdentityId let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ @@ -439,7 +439,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) XCTAssertEqual(mockStorageService.uploadCalled, 1) - mockStorageService.verifyUpload(serviceKey: "/public/\(testKey)", + mockStorageService.verifyUpload(serviceKey: "public/\(testKey)", key: testKey, uploadSource: expectedUploadSource, contentType: testContentType, @@ -450,7 +450,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { /// When: The operation is executed with a request that has an valid IdentityIDStoragePath /// Then: The operation will succeed func testUploadFileOperationWithIdentityIDStoragePathSucceeds() async throws { - let path = IdentityIDStoragePath(resolve: { id in return "/public/\(id)/\(self.testKey)" }) + let path = IdentityIDStoragePath(resolve: { id in return "public/\(id)/\(self.testKey)" }) mockAuthService.identityId = testIdentityId let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") mockStorageService.storageServiceUploadEvents = [ @@ -491,7 +491,7 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { await waitForExpectations(timeout: 1) XCTAssertTrue(operation.isFinished) XCTAssertEqual(mockStorageService.uploadCalled, 1) - mockStorageService.verifyUpload(serviceKey: "/public/\(testIdentityId)/\(testKey)", + mockStorageService.verifyUpload(serviceKey: "public/\(testIdentityId)/\(testKey)", key: testKey, uploadSource: expectedUploadSource, contentType: testContentType, diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift index 4b74e24f01..ccd5212bc8 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift @@ -21,7 +21,7 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { /// - Then: A URL should be returned. func testGetURLTaskSuccess() async throws { - let somePath = "/path" + let somePath = "path" let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) let serviceMock = MockAWSS3StorageService() @@ -43,7 +43,7 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { /// - When: AWSS3StorageGetURLTask value is invoked /// - Then: A storage service error should be returned, with an underlying service error func testGetURLTaskNoBucket() async throws { - let somePath = "/path" + let somePath = "path" let serviceMock = MockAWSS3StorageService() serviceMock.getPreSignedURLHandler = { _, _, _ in @@ -74,7 +74,7 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { /// - When: AWSS3StorageGetURLTask value is invoked /// - Then: A storage validation error should be returned func testGetURLTaskWithInvalidPath() async throws { - let somePath = "path" + let somePath = "/path" let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) let serviceMock = MockAWSS3StorageService() diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift index f58fe03e7a..4ba7cff47c 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift @@ -31,7 +31,7 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { } let request = StorageListRequest( - path: StringStoragePath.fromString("/path"), options: .init()) + path: StringStoragePath.fromString("path"), options: .init()) let task = AWSS3StorageListObjectsTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), @@ -56,7 +56,7 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { } let request = StorageListRequest( - path: StringStoragePath.fromString("/path"), options: .init()) + path: StringStoragePath.fromString("path"), options: .init()) let task = AWSS3StorageListObjectsTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), @@ -83,7 +83,7 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { let serviceMock = MockAWSS3StorageService() let request = StorageListRequest( - path: StringStoragePath.fromString("path"), options: .init()) + path: StringStoragePath.fromString("/path"), options: .init()) let task = AWSS3StorageListObjectsTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift index 1ac8651b43..047f130036 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift @@ -27,13 +27,13 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { } let request = StorageRemoveRequest( - path: StringStoragePath.fromString("/path"), options: .init()) + path: StringStoragePath.fromString("path"), options: .init()) let task = AWSS3StorageRemoveTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), storageBehaviour: serviceMock) let value = try await task.value - XCTAssertEqual(value, "/path") + XCTAssertEqual(value, "path") } /// - Given: A configured Storage Remove Task with mocked service, throwing `NoSuchKey` exception @@ -47,7 +47,7 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { } let request = StorageRemoveRequest( - path: StringStoragePath.fromString("/path"), options: .init()) + path: StringStoragePath.fromString("path"), options: .init()) let task = AWSS3StorageRemoveTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), @@ -74,7 +74,7 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { let serviceMock = MockAWSS3StorageService() let request = StorageRemoveRequest( - path: StringStoragePath.fromString("path"), options: .init()) + path: StringStoragePath.fromString("/path"), options: .init()) let task = AWSS3StorageRemoveTask( request, storageConfiguration: AWSS3StoragePluginConfiguration(), From 9e2bc8baef3deba67f2472294dfaa404a9da18b9 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Fri, 22 Mar 2024 13:21:52 -0500 Subject: [PATCH 08/12] chore(storage): add new upload and download integration tests (#3581) --- ...SS3StoragePlugin+AsyncClientBehavior.swift | 6 +- .../AWSS3StorageUploadDataOperation.swift | 9 +- .../AWSS3StorageUploadFileOperation.swift | 9 +- ...toragePluginDownloadIntegrationTests.swift | 61 ++++++ ...3StoragePluginUploadIntegrationTests.swift | 201 ++++++++++++++++++ .../StorageHostApp.xcodeproj/project.pbxproj | 8 + 6 files changed, 286 insertions(+), 8 deletions(-) create mode 100644 AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginDownloadIntegrationTests.swift create mode 100644 AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginUploadIntegrationTests.swift diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift index 06f2db02a6..a960ec1e07 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/AWSS3StoragePlugin+AsyncClientBehavior.swift @@ -151,8 +151,7 @@ extension AWSS3StoragePlugin { options: StorageUploadDataOperation.Request.Options? = nil ) -> StorageUploadDataTask { let options = options ?? StorageUploadDataRequest.Options() - let path = "" //TODO: resolve path - let request = StorageUploadDataRequest(key: path, data: data, options: options) + let request = StorageUploadDataRequest(path: path, data: data, options: options) let operation = AWSS3StorageUploadDataOperation(request, storageConfiguration: storageConfiguration, storageService: storageService, @@ -188,8 +187,7 @@ extension AWSS3StoragePlugin { options: StorageUploadFileOperation.Request.Options? = nil ) -> StorageUploadFileTask { let options = options ?? StorageUploadFileRequest.Options() - let path = "" //TODO: resolve path - let request = StorageUploadFileRequest(key: path, local: local, options: options) + let request = StorageUploadFileRequest(path: path, local: local, options: options) let operation = AWSS3StorageUploadFileOperation(request, storageConfiguration: storageConfiguration, storageService: storageService, diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift index 6e4288445a..7dd70d4f28 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadDataOperation.swift @@ -26,7 +26,7 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation< let authService: AWSAuthServiceBehavior var storageTaskReference: StorageTaskReference? - + private var resolvedPath: String? /// Serial queue for synchronizing access to `storageTaskReference`. private let storageTaskActionQueue = DispatchQueue(label: "com.amazonaws.amplify.StorageTaskActionQueue") @@ -92,6 +92,7 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation< let serviceKey: String if let path = request.path { serviceKey = try await path.resolvePath(authService: self.authService) + resolvedPath = serviceKey } else { let prefixResolver = storageConfiguration.prefixResolver ?? StorageAccessLevelAwarePrefixResolver(authService: authService) @@ -142,7 +143,11 @@ class AWSS3StorageUploadDataOperation: AmplifyInProcessReportingOperation< case .inProcess(let progress): dispatch(progress) case .completed: - dispatch(request.key) + if let path = resolvedPath { + dispatch(path) + } else { + dispatch(request.key) + } finish() case .failed(let error): dispatch(error) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift index 92e63b5b4d..db8ced505c 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Operation/AWSS3StorageUploadFileOperation.swift @@ -26,7 +26,7 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation< let authService: AWSAuthServiceBehavior var storageTaskReference: StorageTaskReference? - + private var resolvedPath: String? /// Serial queue for synchronizing access to `storageTaskReference`. private let storageTaskActionQueue = DispatchQueue(label: "com.amazonaws.amplify.StorageTaskActionQueue") @@ -114,6 +114,7 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation< let serviceKey: String if let path = request.path { serviceKey = try await path.resolvePath(authService: self.authService) + resolvedPath = serviceKey } else { let prefixResolver = storageConfiguration.prefixResolver ?? StorageAccessLevelAwarePrefixResolver(authService: authService) @@ -165,7 +166,11 @@ class AWSS3StorageUploadFileOperation: AmplifyInProcessReportingOperation< case .inProcess(let progress): dispatch(progress) case .completed: - dispatch(request.key) + if let path = resolvedPath { + dispatch(path) + } else { + dispatch(request.key) + } finish() case .failed(let error): dispatch(error) diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginDownloadIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginDownloadIntegrationTests.swift new file mode 100644 index 0000000000..d58a879123 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginDownloadIntegrationTests.swift @@ -0,0 +1,61 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +@testable import Amplify + +import AWSS3StoragePlugin +import ClientRuntime +import CryptoKit +import XCTest + +class AWSS3StoragePluginDownloadIntegrationTests: AWSS3StoragePluginTestBase { + /// Given: An object in storage + /// When: Call the downloadData API + /// Then: The operation completes successfully with the data retrieved + func testDownloadDataToMemory() async throws { + let key = UUID().uuidString + try await uploadData(key: key, data: Data(key.utf8)) + _ = try await Amplify.Storage.downloadData(path: .fromString("public/\(key)"), options: .init()).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + } + /// Given: An object in storage + /// When: Call the downloadFile API + /// Then: The operation completes successfully the local file containing the data from the object + func testDownloadFile() async throws { + let key = UUID().uuidString + let timestamp = String(Date().timeIntervalSince1970) + let timestampData = Data(timestamp.utf8) + try await uploadData(key: key, data: timestampData) + let filePath = NSTemporaryDirectory() + key + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + removeIfExists(fileURL) + + _ = try await Amplify.Storage.downloadFile(path: .fromString("public/\(key)"), local: fileURL, options: .init()).value + + let fileExists = FileManager.default.fileExists(atPath: fileURL.path) + XCTAssertTrue(fileExists) + do { + let result = try String(contentsOf: fileURL, encoding: .utf8) + XCTAssertEqual(result, timestamp) + } catch { + XCTFail("Failed to read file that has been downloaded to") + } + removeIfExists(fileURL) + _ = try await Amplify.Storage.remove(key: key) + } + + func removeIfExists(_ fileURL: URL) { + let fileExists = FileManager.default.fileExists(atPath: fileURL.path) + if fileExists { + do { + try FileManager.default.removeItem(at: fileURL) + } catch { + XCTFail("Failed to delete file at \(fileURL)") + } + } + } +} diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginUploadIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginUploadIntegrationTests.swift new file mode 100644 index 0000000000..565bee8746 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginUploadIntegrationTests.swift @@ -0,0 +1,201 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +@testable import Amplify + +import AWSS3StoragePlugin +import ClientRuntime +import CryptoKit +import XCTest + +class AWSS3StoragePluginUploadIntegrationTests: AWSS3StoragePluginTestBase { + + var uploadedKeys: [String]! + + /// Represents expected pieces of the User-Agent header of an SDK http request. + /// + /// Example SDK User-Agent: + /// ``` + /// User-Agent: aws-sdk-swift/1.0 api/s3/1.0 os/iOS/16.4.0 lang/swift/5.8 + /// ``` + /// - Tag: SdkUserAgentComponent + private enum SdkUserAgentComponent: String, CaseIterable { + case api = "api/s3" + case lang = "lang/swift" + case os = "os/" + case sdk = "aws-sdk-swift/" + } + + /// Represents expected pieces of the User-Agent header of an URLRequest used for uploading or + /// downloading. + /// + /// Example SDK User-Agent: + /// ``` + /// User-Agent: lib/amplify-swift + /// ``` + /// - Tag: SdkUserAgentComponent + private enum URLUserAgentComponent: String, CaseIterable { + case lib = "lib/amplify-swift" + case os = "os/" + } + + override func setUp() async throws { + try await super.setUp() + uploadedKeys = [] + } + + override func tearDown() async throws { + for key in uploadedKeys { + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + } + uploadedKeys = nil + try await super.tearDown() + } + + /// Given: An data object + /// When: Upload the data + /// Then: The operation completes successfully + func testUploadData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + + _ = try await Amplify.Storage.uploadData(path: .fromString("public/\(key)"), data: data, options: nil).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + + // Only the remove operation results in an SDK request + XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method } , [.delete]) + try assertUserAgentComponents(sdkRequests: requestRecorder.sdkRequests) + + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) + try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) + } + + /// Given: A empty data object + /// When: Upload the data + /// Then: The operation completes successfully + func testUploadEmptyData() async throws { + let key = UUID().uuidString + let data = Data("".utf8) + _ = try await Amplify.Storage.uploadData(path: .fromString("public/\(key)"), data: data, options: nil).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) + try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) + } + + /// Given: A file with contents + /// When: Upload the file + /// Then: The operation completes successfully and all URLSession and SDK requests include a user agent + func testUploadFile() async throws { + let key = UUID().uuidString + let filePath = NSTemporaryDirectory() + key + ".tmp" + + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: Data(key.utf8), attributes: nil) + + _ = try await Amplify.Storage.uploadFile(path: .fromString("public/\(key)"), local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + + // Only the remove operation results in an SDK request + XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method} , [.delete]) + try assertUserAgentComponents(sdkRequests: requestRecorder.sdkRequests) + + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) + try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) + } + + /// Given: A file with empty contents + /// When: Upload the file + /// Then: The operation completes successfully + func testUploadFileEmptyData() async throws { + let key = UUID().uuidString + let filePath = NSTemporaryDirectory() + key + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: Data("".utf8), attributes: nil) + + _ = try await Amplify.Storage.uploadFile(path: .fromString("public/\(key)"), local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, ["PUT"]) + try assertUserAgentComponents(urlRequests: requestRecorder.urlRequests) + } + + /// Given: A large data object + /// When: Upload the data + /// Then: The operation completes successfully + func testUploadLargeData() async throws { + let key = "public/" + UUID().uuidString + + let uploadKey = try await Amplify.Storage.uploadData(path: .fromString(key), + data: AWSS3StoragePluginTestBase.largeDataObject, + options: nil).value + XCTAssertEqual(uploadKey, key) + + try await Amplify.Storage.remove(path: .fromString(key)) + + let userAgents = requestRecorder.urlRequests.compactMap { $0.allHTTPHeaderFields?["User-Agent"] } + XCTAssertGreaterThan(userAgents.count, 1) + for userAgent in userAgents { + let expectedComponent = "MultiPart/UploadPart" + XCTAssertTrue(userAgent.contains(expectedComponent), "\(userAgent) does not contain \(expectedComponent)") + } + } + + /// Given: A large file + /// When: Upload the file + /// Then: The operation completes successfully + func testUploadLargeFile() async throws { + let key = UUID().uuidString + let filePath = NSTemporaryDirectory() + key + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + + FileManager.default.createFile(atPath: filePath, + contents: AWSS3StoragePluginTestBase.largeDataObject, + attributes: nil) + + _ = try await Amplify.Storage.uploadFile(path: .fromString("public/\(key)"), local: fileURL, options: nil).value + _ = try await Amplify.Storage.remove(path: .fromString("public/\(key)")) + + let userAgents = requestRecorder.urlRequests.compactMap { $0.allHTTPHeaderFields?["User-Agent"] } + XCTAssertGreaterThan(userAgents.count, 1) + for userAgent in userAgents { + let expectedComponent = "MultiPart/UploadPart" + XCTAssertTrue(userAgent.contains(expectedComponent), "\(userAgent) does not contain \(expectedComponent)") + } + } + + func removeIfExists(_ fileURL: URL) { + let fileExists = FileManager.default.fileExists(atPath: fileURL.path) + if fileExists { + do { + try FileManager.default.removeItem(at: fileURL) + } catch { + XCTFail("Failed to delete file at \(fileURL)") + } + } + } + + private func assertUserAgentComponents(sdkRequests: [SdkHttpRequest], file: StaticString = #filePath, line: UInt = #line) throws { + for request in sdkRequests { + let headers = request.headers.dictionary + let userAgent = try XCTUnwrap(headers["User-Agent"]?.joined(separator:",")) + for component in SdkUserAgentComponent.allCases { + XCTAssertTrue(userAgent.contains(component.rawValue), "\(userAgent.description) does not contain \(component)", file: file, line: line) + } + } + } + + private func assertUserAgentComponents(urlRequests: [URLRequest], file: StaticString = #filePath, line: UInt = #line) throws { + for request in urlRequests { + let headers = try XCTUnwrap(request.allHTTPHeaderFields) + let userAgent = try XCTUnwrap(headers["User-Agent"]) + for component in URLUserAgentComponent.allCases { + XCTAssertTrue(userAgent.contains(component.rawValue), "\(userAgent.description) does not contain \(component)", file: file, line: line) + } + } + } +} diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj b/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj index da28c91c22..dbb60060a3 100644 --- a/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj @@ -59,6 +59,8 @@ 68828E4628C2736C006E7C0A /* AWSS3StoragePluginProgressTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 684FB08C28BEAF8E00C8A6EB /* AWSS3StoragePluginProgressTests.swift */; }; 68828E4728C27745006E7C0A /* AWSS3StoragePluginPutDataResumabilityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 684FB08828BEAF8E00C8A6EB /* AWSS3StoragePluginPutDataResumabilityTests.swift */; }; 68828E4828C2AAA6006E7C0A /* AWSS3StoragePluginGetDataResumabilityTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 684FB08B28BEAF8E00C8A6EB /* AWSS3StoragePluginGetDataResumabilityTests.swift */; }; + 734605222BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 734605212BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift */; }; + 734605242BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 734605232BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift */; }; 901AB3E92AE2C2DC000F825B /* AWSS3StoragePluginUploadMetadataTestCase.swift in Sources */ = {isa = PBXBuildFile; fileRef = 901AB3E82AE2C2DC000F825B /* AWSS3StoragePluginUploadMetadataTestCase.swift */; }; 97914BA32955798D002000EA /* AsyncTesting.swift in Sources */ = {isa = PBXBuildFile; fileRef = 681DFEAF28E748270000C36A /* AsyncTesting.swift */; }; 97914BA52955798D002000EA /* AsyncExpectation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 681DFEB028E748270000C36A /* AsyncExpectation.swift */; }; @@ -128,6 +130,8 @@ 684FB0A928BEB07200C8A6EB /* AWSS3StoragePluginIntegrationTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = AWSS3StoragePluginIntegrationTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; 684FB0C228BEB45600C8A6EB /* AuthSignInHelper.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AuthSignInHelper.swift; sourceTree = ""; }; 684FB0C528BEB84800C8A6EB /* StorageHostApp.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = StorageHostApp.entitlements; sourceTree = ""; }; + 734605212BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginUploadIntegrationTests.swift; sourceTree = ""; }; + 734605232BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginDownloadIntegrationTests.swift; sourceTree = ""; }; 901AB3E82AE2C2DC000F825B /* AWSS3StoragePluginUploadMetadataTestCase.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginUploadMetadataTestCase.swift; sourceTree = ""; }; 97914B972955797E002000EA /* StorageStressTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StorageStressTests.swift; sourceTree = ""; }; 97914BB92955798D002000EA /* StorageStressTests.xctest */ = {isa = PBXFileReference; explicitFileType = wrapper.cfbundle; includeInIndex = 0; path = StorageStressTests.xctest; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -269,6 +273,8 @@ 562B9AA32A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift */, 901AB3E82AE2C2DC000F825B /* AWSS3StoragePluginUploadMetadataTestCase.swift */, 684FB08728BEAF8E00C8A6EB /* ResumabilityTests */, + 734605212BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift */, + 734605232BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift */, ); path = AWSS3StoragePluginIntegrationTests; sourceTree = ""; @@ -616,6 +622,7 @@ 68828E4628C2736C006E7C0A /* AWSS3StoragePluginProgressTests.swift in Sources */, 684FB0B528BEB08900C8A6EB /* AWSS3StoragePluginAccessLevelTests.swift in Sources */, 68828E4028C1549E006E7C0A /* AWSS3StoragePluginDownloadFileResumabilityTests.swift in Sources */, + 734605242BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift in Sources */, 68828E4528C26D2D006E7C0A /* AWSS3StoragePluginPrefixKeyResolverTests.swift in Sources */, 684FB0B328BEB08900C8A6EB /* AWSS3StoragePluginTestBase.swift in Sources */, 68828E3F28C1549B006E7C0A /* AWSS3StoragePluginUploadFileResumabilityTests.swift in Sources */, @@ -623,6 +630,7 @@ 68828E3E28C1546F006E7C0A /* AWSS3StoragePluginConfigurationTests.swift in Sources */, 68828E4728C27745006E7C0A /* AWSS3StoragePluginPutDataResumabilityTests.swift in Sources */, 68828E4128C154E5006E7C0A /* AWSS3StoragePluginNegativeTests.swift in Sources */, + 734605222BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift in Sources */, 68828E3D28C136EB006E7C0A /* AWSS3StoragePluginBasicIntegrationTests.swift in Sources */, 681DFEB428E748270000C36A /* XCTestCase+AsyncTesting.swift in Sources */, 68828E4228C15B8B006E7C0A /* AWSS3StoragePluginOptionsUsabilityTests.swift in Sources */, From 540acc2ba64ae18ee9bf067189de337442b19466 Mon Sep 17 00:00:00 2001 From: Harsh <6162866+harsh62@users.noreply.github.com> Date: Mon, 25 Mar 2024 13:58:12 -0400 Subject: [PATCH 09/12] feat(Storage): Adding integration tests for getURL, remove and list (#3584) --- ...3StoragePluginGetURLIntegrationTests.swift | 85 ++++++++ ...agePluginListObjectsIntegrationTests.swift | 186 ++++++++++++++++++ ...3StoragePluginRemoveIntegrationTests.swift | 166 ++++++++++++++++ .../StorageHostApp.xcodeproj/project.pbxproj | 12 ++ 4 files changed, 449 insertions(+) create mode 100644 AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginGetURLIntegrationTests.swift create mode 100644 AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift create mode 100644 AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginRemoveIntegrationTests.swift diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginGetURLIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginGetURLIntegrationTests.swift new file mode 100644 index 0000000000..d8a4496e82 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginGetURLIntegrationTests.swift @@ -0,0 +1,85 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +@testable import Amplify + +import AWSS3StoragePlugin +import ClientRuntime +import AWSClientRuntime +import CryptoKit +import XCTest +import AWSS3 + +class AWSS3StoragePluginGetURLIntegrationTests: AWSS3StoragePluginTestBase { + + /// Given: An object in storage + /// When: Call the getURL API + /// Then: The operation completes successfully with the URL retrieved + func testGetRemoteURL() async throws { + let key = "public/" + UUID().uuidString + try await uploadData(key: key, dataString: key) + _ = try await Amplify.Storage.uploadData( + path: .fromString(key), + data: Data(key.utf8), + options: .init()) + + let remoteURL = try await Amplify.Storage.getURL(path: .fromString(key)) + + // The presigned URL generation does not result in an SDK or HTTP call. + XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method} , []) + + let (data, response) = try await URLSession.shared.data(from: remoteURL) + let httpResponse = try XCTUnwrap(response as? HTTPURLResponse) + XCTAssertEqual(httpResponse.statusCode, 200) + + let dataString = try XCTUnwrap(String(data: data, encoding: .utf8)) + XCTAssertEqual(dataString, key) + + _ = try await Amplify.Storage.remove(path: .fromString(key)) + } + + /// - Given: A key for a non-existent S3 object + /// - When: A pre-signed URL is requested for that key with `validateObjectExistence = true` + /// - Then: A StorageError.keyNotFound error is thrown + func testGetURLForUnknownKeyWithValidation() async throws { + let unknownKey = "public/" + UUID().uuidString + do { + let url = try await Amplify.Storage.getURL( + path: .fromString(unknownKey), + options: .init( + pluginOptions: AWSStorageGetURLOptions(validateObjectExistence: true) + ) + ) + XCTFail("Expecting failure but got url: \(url)") + } catch StorageError.keyNotFound(let key, _, _, _) { + XCTAssertTrue(key.contains(unknownKey)) + } + + // A S3 HeadObject call is expected + XCTAssert(requestRecorder.sdkRequests.map(\.method).allSatisfy { $0 == .head }) + + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, []) + } + + /// - Given: A key for a non-existent S3 object + /// - When: A pre-signed URL is requested for that key with `validateObjectExistence = false` + /// - Then: A pre-signed URL is returned + func testGetURLForUnknownKeyWithoutValidation() async throws { + let unknownKey = UUID().uuidString + let url = try await Amplify.Storage.getURL( + path: .fromString(unknownKey), + options: .init( + pluginOptions: AWSStorageGetURLOptions(validateObjectExistence: false) + ) + ) + XCTAssertNotNil(url) + + // No SDK or URLRequest calls expected + XCTAssertEqual(requestRecorder.sdkRequests.map { $0.method} , []) + XCTAssertEqual(requestRecorder.urlRequests.map { $0.httpMethod }, []) + } +} diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift new file mode 100644 index 0000000000..6b05278a11 --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift @@ -0,0 +1,186 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +@testable import Amplify + +import AWSS3StoragePlugin +import ClientRuntime +import AWSClientRuntime +import CryptoKit +import XCTest +import AWSS3 + +class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase { + + /// Given: Multiple data object which is uploaded to a public path + /// When: `Amplify.Storage.list` is run + /// Then: The API should execute successfully and list objects for path + func testListObjectsUploadedPublicData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + let uniqueStringPath = "public/\(key)" + + _ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/test1"), data: data, options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + + _ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/test2"), data: data, options: nil).value + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + + // Clean up + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/test1")) + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/test2")) + } + + /// Given: Multiple data object which is uploaded to a protected path + /// When: `Amplify.Storage.list` is run + /// Then: The API should execute successfully and list objects for path + func testListObjectsUploadedProtectedData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + var uniqueStringPath = "" + + // Sign in + _ = try await Amplify.Auth.signIn(username: Self.user1, password: Self.password) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "protected/\(identityId)/\(key)" + return uniqueStringPath + "test1" + }), + data: data, + options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "protected/\(identityId)/\(key)" + return uniqueStringPath + "test2" + }), + data: data, + options: nil).value + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + + // clean up + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test1")) + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test2")) + + } + + /// Given: Multiple data object which is uploaded to a private path + /// When: `Amplify.Storage.list` is run + /// Then: The API should execute successfully and list objects for path + func testListObjectsUploadedPrivateData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + var uniqueStringPath = "" + + // Sign in + _ = try await Amplify.Auth.signIn(username: Self.user1, password: Self.password) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "private/\(identityId)/\(key)" + return uniqueStringPath + "test1" + }), + data: data, + options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "private/\(identityId)/\(key)" + return uniqueStringPath + "test2" + }), + data: data, + options: nil).value + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + + // clean up + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test1")) + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test2")) + + } + + /// Given: Give a unique key that does not exist + /// When: `Amplify.Storage.list` is run + /// Then: The API should execute and throw an error + func testRemoveKeyDoesNotExist() async throws { + let key = UUID().uuidString + let uniqueStringPath = "public/\(key)" + + do { + _ = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + } + catch { + guard let storageError = error as? StorageError else { + XCTFail("Error should be of type StorageError but got \(error)") + return + } + guard case .keyNotFound(_, _, _, let underlyingError) = storageError else { + XCTFail("Error should be of type keyNotFound but got \(error)") + return + } + + guard underlyingError is AWSS3.NotFound else { + XCTFail("Underlying error should be of type AWSS3.NotFound but got \(error)") + return + } + } + } + + /// Given: Give a unique key where is user is NOT logged in + /// When: `Amplify.Storage.list` is run + /// Then: The API should execute and throw an error + func testRemoveKeyWhenNotSignedInForPrivateKey() async throws { + let key = UUID().uuidString + let uniqueStringPath = "private/\(key)" + + do { + _ = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + } + catch { + guard let storageError = error as? StorageError else { + XCTFail("Error should be of type StorageError but got \(error)") + return + } + guard case .accessDenied(_, _, let underlyingError) = storageError else { + XCTFail("Error should be of type keyNotFound but got \(error)") + return + } + + guard underlyingError is UnknownAWSHTTPServiceError else { + XCTFail("Underlying error should be of type UnknownAWSHTTPServiceError but got \(error)") + return + } + } + } + +} diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginRemoveIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginRemoveIntegrationTests.swift new file mode 100644 index 0000000000..561c1504ba --- /dev/null +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginRemoveIntegrationTests.swift @@ -0,0 +1,166 @@ +// +// Copyright Amazon.com Inc. or its affiliates. +// All Rights Reserved. +// +// SPDX-License-Identifier: Apache-2.0 +// + +@testable import Amplify + +import AWSS3StoragePlugin +import ClientRuntime +import AWSClientRuntime +import CryptoKit +import XCTest +import AWSS3 + +class AWSS3StoragePluginRemoveIntegrationTests: AWSS3StoragePluginTestBase { + + /// Given: A data object which is uploaded to a public path + /// When: `Amplify.Storage.remove` is run + /// Then: The API should execute successfully and remove the object + func testRemoveUploadedPublicData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + let uniqueStringPath = "public/\(key)" + + _ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath), data: data, options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.key == uniqueStringPath}).count, 1) + + // Validate + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath)) + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.key == uniqueStringPath}).count, 0) + + } + + /// Given: A data object which is uploaded to a protected path + /// When: `Amplify.Storage.remove` is run + /// Then: The API should execute successfully and remove the object + func testRemoveUploadedProtectedData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + var uniqueStringPath = "" + + // Sign in + _ = try await Amplify.Auth.signIn(username: Self.user1, password: Self.password) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "protected/\(identityId)/\(key)" + return uniqueStringPath + }), + data: data, + options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.key == uniqueStringPath}).count, 1) + + // Validate + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath)) + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.key == uniqueStringPath}).count, 0) + + } + + /// Given: A data object which is uploaded to a private path + /// When: `Amplify.Storage.remove` is run + /// Then: The API should execute successfully and remove the object + func testRemoveUploadedPrivateData() async throws { + let key = UUID().uuidString + let data = Data(key.utf8) + var uniqueStringPath = "" + + // Sign in + _ = try await Amplify.Auth.signIn(username: Self.user1, password: Self.password) + + _ = try await Amplify.Storage.uploadData( + path: .fromIdentityID({ identityId in + uniqueStringPath = "private/\(identityId)/\(key)" + return uniqueStringPath + }), + data: data, + options: nil).value + + let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(firstListResult.items.filter({ $0.key == uniqueStringPath}).count, 1) + + // Validate + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath)) + + let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) + + // Validate the item was uploaded. + XCTAssertEqual(secondListResult.items.filter({ $0.key == uniqueStringPath}).count, 0) + + } + + /// Given: Give a unique key that does not exist + /// When: `Amplify.Storage.remove` is run + /// Then: The API should execute and throw an error + func testRemoveKeyDoesNotExist() async throws { + let key = UUID().uuidString + let uniqueStringPath = "public/\(key)" + + do { + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath)) + } + catch { + guard let storageError = error as? StorageError else { + XCTFail("Error should be of type StorageError but got \(error)") + return + } + guard case .keyNotFound(_, _, _, let underlyingError) = storageError else { + XCTFail("Error should be of type keyNotFound but got \(error)") + return + } + + guard underlyingError is AWSS3.NotFound else { + XCTFail("Underlying error should be of type AWSS3.NotFound but got \(error)") + return + } + } + } + + /// Given: Give a unique key where is user is NOT logged in + /// When: `Amplify.Storage.remove` is run + /// Then: The API should execute and throw an error + func testRemoveKeyWhenNotSignedInForPrivateKey() async throws { + let key = UUID().uuidString + let uniqueStringPath = "private/\(key)" + + do { + _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath)) + } + catch { + guard let storageError = error as? StorageError else { + XCTFail("Error should be of type StorageError but got \(error)") + return + } + guard case .accessDenied(_, _, let underlyingError) = storageError else { + XCTFail("Error should be of type keyNotFound but got \(error)") + return + } + + guard underlyingError is UnknownAWSHTTPServiceError else { + XCTFail("Underlying error should be of type UnknownAWSHTTPServiceError but got \(error)") + return + } + } + } + +} diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj b/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj index dbb60060a3..975f3dea20 100644 --- a/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/StorageHostApp.xcodeproj/project.pbxproj @@ -9,6 +9,9 @@ /* Begin PBXBuildFile section */ 0311113528EBED6500D58441 /* Tests.xcconfig in Resources */ = {isa = PBXBuildFile; fileRef = 0311113428EBED6500D58441 /* Tests.xcconfig */; }; 031BC3F328EC9B2C0047B2E8 /* AppIcon.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 031BC3F228EC9B2C0047B2E8 /* AppIcon.xcassets */; }; + 488C2A732BAE04DC009AD2BA /* AWSS3StoragePluginRemoveIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 488C2A722BAE04DC009AD2BA /* AWSS3StoragePluginRemoveIntegrationTests.swift */; }; + 488C2A752BAFCA7C009AD2BA /* AWSS3StoragePluginListObjectsIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 488C2A742BAFCA7C009AD2BA /* AWSS3StoragePluginListObjectsIntegrationTests.swift */; }; + 488C2A772BAFD4B3009AD2BA /* AWSS3StoragePluginGetURLIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 488C2A762BAFD4B3009AD2BA /* AWSS3StoragePluginGetURLIntegrationTests.swift */; }; 56043E9329FC4D33003E3424 /* amplifyconfiguration.json in Resources */ = {isa = PBXBuildFile; fileRef = D5C0382101A0E23943FDF4CB /* amplifyconfiguration.json */; }; 562B9AA42A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 562B9AA32A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift */; }; 562B9AA52A0D734E00A96FC6 /* AWSS3StoragePluginRequestRecorder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 562B9AA32A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift */; }; @@ -103,6 +106,9 @@ 0311113428EBED6500D58441 /* Tests.xcconfig */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.xcconfig; path = Tests.xcconfig; sourceTree = ""; }; 0311113828EBEEA700D58441 /* Base.xcconfig */ = {isa = PBXFileReference; lastKnownFileType = text.xcconfig; path = Base.xcconfig; sourceTree = ""; }; 031BC3F228EC9B2C0047B2E8 /* AppIcon.xcassets */ = {isa = PBXFileReference; lastKnownFileType = folder.assetcatalog; path = AppIcon.xcassets; sourceTree = ""; }; + 488C2A722BAE04DC009AD2BA /* AWSS3StoragePluginRemoveIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginRemoveIntegrationTests.swift; sourceTree = ""; }; + 488C2A742BAFCA7C009AD2BA /* AWSS3StoragePluginListObjectsIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginListObjectsIntegrationTests.swift; sourceTree = ""; }; + 488C2A762BAFD4B3009AD2BA /* AWSS3StoragePluginGetURLIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginGetURLIntegrationTests.swift; sourceTree = ""; }; 562B9AA32A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginRequestRecorder.swift; sourceTree = ""; }; 565DF16F2953BAEA000DCCF7 /* AWSS3StoragePluginAccelerateIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AWSS3StoragePluginAccelerateIntegrationTests.swift; sourceTree = ""; }; 681D7D392A42637700F7C310 /* StorageWatchApp.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = StorageWatchApp.app; sourceTree = BUILT_PRODUCTS_DIR; }; @@ -275,6 +281,9 @@ 684FB08728BEAF8E00C8A6EB /* ResumabilityTests */, 734605212BACB5CC0039F0EB /* AWSS3StoragePluginUploadIntegrationTests.swift */, 734605232BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift */, + 488C2A722BAE04DC009AD2BA /* AWSS3StoragePluginRemoveIntegrationTests.swift */, + 488C2A742BAFCA7C009AD2BA /* AWSS3StoragePluginListObjectsIntegrationTests.swift */, + 488C2A762BAFD4B3009AD2BA /* AWSS3StoragePluginGetURLIntegrationTests.swift */, ); path = AWSS3StoragePluginIntegrationTests; sourceTree = ""; @@ -613,18 +622,21 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 488C2A772BAFD4B3009AD2BA /* AWSS3StoragePluginGetURLIntegrationTests.swift in Sources */, 565DF1702953BAEA000DCCF7 /* AWSS3StoragePluginAccelerateIntegrationTests.swift in Sources */, 684FB0C328BEB45600C8A6EB /* AuthSignInHelper.swift in Sources */, 681DFEB228E748270000C36A /* AsyncTesting.swift in Sources */, 68828E4828C2AAA6006E7C0A /* AWSS3StoragePluginGetDataResumabilityTests.swift in Sources */, 901AB3E92AE2C2DC000F825B /* AWSS3StoragePluginUploadMetadataTestCase.swift in Sources */, 681DFEB328E748270000C36A /* AsyncExpectation.swift in Sources */, + 488C2A732BAE04DC009AD2BA /* AWSS3StoragePluginRemoveIntegrationTests.swift in Sources */, 68828E4628C2736C006E7C0A /* AWSS3StoragePluginProgressTests.swift in Sources */, 684FB0B528BEB08900C8A6EB /* AWSS3StoragePluginAccessLevelTests.swift in Sources */, 68828E4028C1549E006E7C0A /* AWSS3StoragePluginDownloadFileResumabilityTests.swift in Sources */, 734605242BACB60E0039F0EB /* AWSS3StoragePluginDownloadIntegrationTests.swift in Sources */, 68828E4528C26D2D006E7C0A /* AWSS3StoragePluginPrefixKeyResolverTests.swift in Sources */, 684FB0B328BEB08900C8A6EB /* AWSS3StoragePluginTestBase.swift in Sources */, + 488C2A752BAFCA7C009AD2BA /* AWSS3StoragePluginListObjectsIntegrationTests.swift in Sources */, 68828E3F28C1549B006E7C0A /* AWSS3StoragePluginUploadFileResumabilityTests.swift in Sources */, 562B9AA42A0D703700A96FC6 /* AWSS3StoragePluginRequestRecorder.swift in Sources */, 68828E3E28C1546F006E7C0A /* AWSS3StoragePluginConfigurationTests.swift in Sources */, From 8c85096ccea8248edc28a665d218867fdf19926c Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Thu, 28 Mar 2024 14:55:10 -0500 Subject: [PATCH 10/12] chore(storage): update storage path validation to include empty/white spaces (#3587) --- .../Internal/StoragePath+Extensions.swift | 10 +++- ...SS3StorageDownloadFileOperationTests.swift | 32 +++++++++++++ .../AWSS3StorageGetDataOperationTests.swift | 30 ++++++++++++ .../AWSS3StoragePutDataOperationTests.swift | 31 +++++++++++++ ...AWSS3StorageUploadFileOperationTests.swift | 46 +++++++++++++++++++ .../Tasks/AWSS3StorageGetURLTaskTests.swift | 32 +++++++++++++ .../AWSS3StorageListObjectsTaskTests.swift | 26 +++++++++++ .../Tasks/AWSS3StorageRemoveTaskTests.swift | 26 +++++++++++ 8 files changed, 231 insertions(+), 2 deletions(-) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift index bdc2ebece4..482190be32 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -20,7 +20,7 @@ extension StoragePath { nil ) } - let path = resolve(identityId) + let path = resolve(identityId).trimmingCharacters(in: .whitespaces) try validate(path) return path } else if self is StringStoragePath { @@ -30,7 +30,7 @@ extension StoragePath { nil ) } - let path = resolve(input) + let path = resolve(input).trimmingCharacters(in: .whitespaces) try validate(path) return path } else { @@ -41,6 +41,12 @@ extension StoragePath { } func validate(_ path: String) throws { + guard !path.isEmpty else { + let errorDescription = "Invalid StoragePath specified." + let recoverySuggestion = "Please specify a valid StoragePath" + throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) + } + if path.hasPrefix("/") { let errorDescription = "Invalid StoragePath specified." let recoverySuggestion = "Please specify a valid StoragePath that does not contain the prefix / " diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift index 98b15f5954..9b1b6b6d65 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageDownloadFileOperationTests.swift @@ -214,6 +214,38 @@ class AWSS3StorageDownloadFileOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// 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 testDownloadFileOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let request = StorageDownloadFileRequest(path: path, + local: testURL, + options: StorageDownloadFileRequest.Options()) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// 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 diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift index 23a28ee935..6a7c7a5485 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageGetDataOperationTests.swift @@ -204,6 +204,36 @@ class AWSS3StorageDownloadDataOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Download Data Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testDownloadDataOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let request = StorageDownloadDataRequest(path: path, options: StorageDownloadDataRequest.Options()) + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageDownloadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { event in + switch event { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Download Data Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift index faeb9fc862..5934e419d3 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StoragePutDataOperationTests.swift @@ -250,6 +250,37 @@ class AWSS3StorageUploadDataOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Upload Data Operation + /// When: The operation is executed with a request that has an invalid StringStoragePath + /// Then: The operation will fail with a validation error + func testUploadDataOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + let failedInvoked = expectation(description: "failed was invoked on operation") + let options = StorageUploadDataRequest.Options(accessLevel: .protected) + let request = StorageUploadDataRequest(path: path, data: testData, options: options) + let operation = AWSS3StorageUploadDataOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil + ) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Upload Data Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift index 87183415e6..12374173bb 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Operation/AWSS3StorageUploadFileOperationTests.swift @@ -301,6 +301,52 @@ class AWSS3StorageUploadFileOperationTests: AWSS3StorageOperationTestBase { XCTAssertTrue(operation.isFinished) } + /// Given: Storage Upload 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 testUploadFileOperationEmptyStoragePathValidationError() { + let path = StringStoragePath(resolve: { _ in return " " }) + mockAuthService.identityId = testIdentityId + let task = StorageTransferTask(transferType: .upload(onEvent: { _ in }), bucket: "bucket", key: "key") + mockStorageService.storageServiceUploadEvents = [ + StorageEvent.initiated(StorageTaskReference(task)), + StorageEvent.inProcess(Progress()), + StorageEvent.completedVoid] + + let filePath = NSTemporaryDirectory() + UUID().uuidString + ".tmp" + let fileURL = URL(fileURLWithPath: filePath) + FileManager.default.createFile(atPath: filePath, contents: testData, attributes: nil) + let expectedUploadSource = UploadSource.local(fileURL) + let metadata = ["mykey": "Value"] + + let options = StorageUploadFileRequest.Options(accessLevel: .protected, + metadata: metadata, + contentType: testContentType) + let request = StorageUploadFileRequest(path: path, local: fileURL, options: options) + + let failedInvoked = expectation(description: "failed was invoked on operation") + let operation = AWSS3StorageUploadFileOperation(request, + storageConfiguration: testStorageConfiguration, + storageService: mockStorageService, + authService: mockAuthService, + progressListener: nil) { result in + switch result { + case .failure(let error): + guard case .validation = error else { + XCTFail("Should have failed with validation error") + return + } + failedInvoked.fulfill() + default: + XCTFail("Should have received failed event") + } + } + + operation.start() + waitForExpectations(timeout: 1) + XCTAssertTrue(operation.isFinished) + } + /// Given: Storage Upload File Operation /// When: The operation is executed with a request that has an invalid IdentityIDStoragePath /// Then: The operation will fail with a validation error diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift index ccd5212bc8..71f5ea6ea1 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageGetURLTaskTests.swift @@ -103,4 +103,36 @@ class AWSS3StorageGetURLTaskTests: XCTestCase { } } + /// - Given: A configured Storage GetURL Task with invalid path + /// - When: AWSS3StorageGetURLTask value is invoked + /// - Then: A storage validation error should be returned + func testGetURLTaskWithInvalidEmptyPath() async throws { + let emptyPath = " " + let tempURL = URL(fileURLWithPath: NSTemporaryDirectory()) + + let serviceMock = MockAWSS3StorageService() + serviceMock.getPreSignedURLHandler = { path, _, _ in + XCTAssertEqual(emptyPath, path) + return tempURL + } + + let request = StorageGetURLRequest( + path: StringStoragePath.fromString(emptyPath), options: .init()) + let task = AWSS3StorageGetURLTask( + request, + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift index 4ba7cff47c..251111abfa 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift @@ -103,4 +103,30 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { } } + /// - Given: A configured Storage ListObjects Task with invalid path + /// - When: AWSS3StorageListObjectsTask value is invoked + /// - Then: A storage validation error should be returned + func testListObjectsTaskWithInvalidEmptyPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageListRequest( + path: StringStoragePath.fromString(" "), options: .init()) + let task = AWSS3StorageListObjectsTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift index 047f130036..06f4ecf809 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageRemoveTaskTests.swift @@ -94,4 +94,30 @@ class AWSS3StorageRemoveTaskTests: XCTestCase { } } + /// - Given: A configured Storage Remove Task with invalid path + /// - When: AWSS3StorageRemoveTask value is invoked + /// - Then: A storage validation error should be returned + func testRemoveTaskWithInvalidEmptyPath() async throws { + let serviceMock = MockAWSS3StorageService() + + let request = StorageRemoveRequest( + path: StringStoragePath.fromString(" "), options: .init()) + let task = AWSS3StorageRemoveTask( + request, + storageConfiguration: AWSS3StoragePluginConfiguration(), + storageBehaviour: serviceMock) + do { + _ = try await task.value + XCTFail("Task should throw an exception") + } + catch { + guard let storageError = error as? StorageError, + case .validation(let field, _, _, _) = storageError else { + XCTFail("Should throw a storage validation error, instead threw \(error)") + return + } + + XCTAssertEqual(field, "path", "Field in error should be `path`") + } + } } From 3169654f9b073f0934622feed31c44bb3d307ab9 Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Fri, 12 Apr 2024 10:36:24 -0500 Subject: [PATCH 11/12] chore: deprecate key field in StorageListResult.Item (#3610) --- .../Storage/Result/StorageListResult.swift | 6 +++--- .../Tasks/AWSS3StorageListObjectsTask.swift | 5 ++--- .../AWSS3StorageListObjectsTaskTests.swift | 1 + ...ragePluginListObjectsIntegrationTests.swift | 18 ++++++++++++------ 4 files changed, 18 insertions(+), 12 deletions(-) diff --git a/Amplify/Categories/Storage/Result/StorageListResult.swift b/Amplify/Categories/Storage/Result/StorageListResult.swift index 7294945b9f..057b9e177a 100644 --- a/Amplify/Categories/Storage/Result/StorageListResult.swift +++ b/Amplify/Categories/Storage/Result/StorageListResult.swift @@ -50,6 +50,7 @@ extension StorageListResult { /// The unique identifier of the object in storage. /// /// - Tag: StorageListResultItem.key + @available(*, deprecated, message: "Use `path` instead.") public let key: String /// Size in bytes of the object @@ -77,7 +78,7 @@ extension StorageListResult { /// [StorageCategoryBehavior.list](x-source-tag://StorageCategoryBehavior.list). /// /// - Tag: StorageListResultItem.init - @available(*, deprecated, message: "Use init(path:key:size:lastModifiedDate:eTag:pluginResults)") + @available(*, deprecated, message: "Use init(path:size:lastModifiedDate:eTag:pluginResults)") public init( key: String, size: Int? = nil, @@ -95,14 +96,13 @@ extension StorageListResult { public init( path: String, - key: String, size: Int? = nil, eTag: String? = nil, lastModified: Date? = nil, pluginResults: Any? = nil ) { self.path = path - self.key = key + self.key = path self.size = size self.eTag = eTag self.lastModified = lastModified diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift index 6da4ce23d0..2baadcf539 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Tasks/AWSS3StorageListObjectsTask.swift @@ -38,7 +38,7 @@ class AWSS3StorageListObjectsTask: StorageListObjectsTask, DefaultLogger { guard let path = try await request.path?.resolvePath() else { throw StorageError.validation( "path", - "`path` is required for removing an object", + "`path` is required for listing objects", "Make sure that a valid `path` is passed for removing an object") } let input = ListObjectsV2Input(bucket: storageBehaviour.bucket, @@ -51,12 +51,11 @@ class AWSS3StorageListObjectsTask: StorageListObjectsTask, DefaultLogger { let response = try await storageBehaviour.client.listObjectsV2(input: input) let contents: S3BucketContents = response.contents ?? [] let items = try contents.map { s3Object in - guard let key = s3Object.key else { + guard let path = s3Object.key else { throw StorageError.unknown("Missing key in response") } return StorageListResult.Item( path: path, - key: key, eTag: s3Object.eTag, lastModified: s3Object.lastModified) } diff --git a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift index 251111abfa..922f29974c 100644 --- a/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift +++ b/AmplifyPlugins/Storage/Tests/AWSS3StoragePluginTests/Tasks/AWSS3StorageListObjectsTaskTests.swift @@ -41,6 +41,7 @@ class AWSS3StorageListObjectsTaskTests: XCTestCase { XCTAssertEqual(value.nextToken, "continuationToken") XCTAssertEqual(value.items[0].eTag, "tag") XCTAssertEqual(value.items[0].key, "key") + XCTAssertEqual(value.items[0].path, "key") XCTAssertNotNil(value.items[0].lastModified) } diff --git a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift index 6b05278a11..83ad2ce017 100644 --- a/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift +++ b/AmplifyPlugins/Storage/Tests/StorageHostApp/AWSS3StoragePluginIntegrationTests/AWSS3StoragePluginListObjectsIntegrationTests.swift @@ -29,14 +29,16 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + XCTAssertEqual(firstListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 1) _ = try await Amplify.Storage.uploadData(path: .fromString(uniqueStringPath + "/test2"), data: data, options: nil).value let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + XCTAssertEqual(secondListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 2) // Clean up _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "/test1")) @@ -65,7 +67,8 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + XCTAssertEqual(firstListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 1) _ = try await Amplify.Storage.uploadData( path: .fromIdentityID({ identityId in @@ -78,7 +81,8 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + XCTAssertEqual(secondListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 2) // clean up _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test1")) @@ -108,7 +112,8 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase let firstListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(firstListResult.items.filter({ $0.path == uniqueStringPath}).count, 1) + XCTAssertEqual(firstListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 1) _ = try await Amplify.Storage.uploadData( path: .fromIdentityID({ identityId in @@ -121,7 +126,8 @@ class AWSS3StoragePluginListObjectsIntegrationTests: AWSS3StoragePluginTestBase let secondListResult = try await Amplify.Storage.list(path: .fromString(uniqueStringPath)) // Validate the item was uploaded. - XCTAssertEqual(secondListResult.items.filter({ $0.path == uniqueStringPath}).count, 2) + XCTAssertEqual(secondListResult.items.filter({ $0.path.contains(uniqueStringPath) + }).count, 2) // clean up _ = try await Amplify.Storage.remove(path: .fromString(uniqueStringPath + "test1")) From 8e2bfa7d5dbeedad07c4c54387ff589151c0857e Mon Sep 17 00:00:00 2001 From: Tuan Pham <103537251+phantumcode@users.noreply.github.com> Date: Mon, 15 Apr 2024 13:05:00 -0500 Subject: [PATCH 12/12] chore(storage): align storage validation exception message with Amplify Android (#3612) --- .../Support/Internal/StoragePath+Extensions.swift | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift index 482190be32..95c464cff2 100644 --- a/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift +++ b/AmplifyPlugins/Storage/Sources/AWSS3StoragePlugin/Support/Internal/StoragePath+Extensions.swift @@ -41,15 +41,9 @@ extension StoragePath { } func validate(_ path: String) throws { - guard !path.isEmpty else { - let errorDescription = "Invalid StoragePath specified." - let recoverySuggestion = "Please specify a valid StoragePath" - throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) - } - - if path.hasPrefix("/") { - let errorDescription = "Invalid StoragePath specified." - let recoverySuggestion = "Please specify a valid StoragePath that does not contain the prefix / " + if path.isEmpty || path.hasPrefix("/") { + let errorDescription = "Invalid StoragePath provided." + let recoverySuggestion = "StoragePath must not be empty or start with /" throw StorageError.validation("path", errorDescription, recoverySuggestion, nil) } }