From 279ac2a6fc4ffae7fac6d60a0695bba084a500f3 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Thu, 1 Aug 2024 10:04:43 -0700 Subject: [PATCH] [storage] Migrate to actor to fix a potential data race in initialization (#13428) --- FirebaseStorage/CHANGELOG.md | 3 + .../Sources/Internal/StorageDeleteTask.swift | 2 - .../Internal/StorageFetcherService.swift | 87 ++++++++++ .../Internal/StorageGetDownloadURLTask.swift | 8 - .../Internal/StorageGetMetadataTask.swift | 8 - .../Internal/StorageInternalTask.swift | 26 +-- .../Sources/Internal/StorageListTask.swift | 8 - .../Internal/StorageUpdateMetadataTask.swift | 8 - FirebaseStorage/Sources/Storage.swift | 98 ++---------- .../Sources/StorageDownloadTask.swift | 148 +++++++++--------- .../Sources/StorageObservableTask.swift | 9 +- .../Sources/StorageReference.swift | 28 +--- FirebaseStorage/Sources/StorageTask.swift | 5 - .../Sources/StorageUploadTask.swift | 108 ++++++------- .../Tests/Unit/StorageComponentTests.swift | 4 +- .../Tests/Unit/StorageDeleteTests.swift | 66 +++----- .../Tests/Unit/StorageGetMetadataTests.swift | 67 +++----- .../Tests/Unit/StorageListTests.swift | 95 ++++------- .../Tests/Unit/StorageTestHelpers.swift | 13 +- .../Unit/StorageUpdateMetadataTests.swift | 94 ++++------- 20 files changed, 367 insertions(+), 518 deletions(-) create mode 100644 FirebaseStorage/Sources/Internal/StorageFetcherService.swift diff --git a/FirebaseStorage/CHANGELOG.md b/FirebaseStorage/CHANGELOG.md index 7da1289bc6b..2738305acf3 100644 --- a/FirebaseStorage/CHANGELOG.md +++ b/FirebaseStorage/CHANGELOG.md @@ -1,3 +1,6 @@ +# Unreleased +- [fixed] Fix a potential data race in Storage initialization. (#13369) + # 11.0.0 - [fixed] Updated error handling to support both Swift error enum handling and NSError error handling. Some of the Swift enums have additional parameters which may be a **breaking** change. diff --git a/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift b/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift index 37c80170a7a..d2917d18c49 100644 --- a/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageDeleteTask.swift @@ -24,11 +24,9 @@ import Foundation @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) enum StorageDeleteTask { static func deleteTask(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, completion: ((_: Data?, _: Error?) -> Void)?) { StorageInternalTask(reference: reference, - fetcherService: fetcherService, queue: queue, httpMethod: "DELETE", fetcherComment: "DeleteTask", diff --git a/FirebaseStorage/Sources/Internal/StorageFetcherService.swift b/FirebaseStorage/Sources/Internal/StorageFetcherService.swift new file mode 100644 index 00000000000..6d635ff001a --- /dev/null +++ b/FirebaseStorage/Sources/Internal/StorageFetcherService.swift @@ -0,0 +1,87 @@ +// Copyright 2024 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Foundation + +#if COCOAPODS + import GTMSessionFetcher +#else + import GTMSessionFetcherCore +#endif + +/// Manage Storage's fetcherService +@available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) +actor StorageFetcherService { + private var _fetcherService: GTMSessionFetcherService? + + func fetcherService(_ storage: Storage) -> GTMSessionFetcherService { + if let _fetcherService { + return _fetcherService + } + let app = storage.app + if StorageFetcherService.fetcherServiceMap[app.name] == nil { + StorageFetcherService.fetcherServiceMap[app.name] = [:] + } + var fetcherService = StorageFetcherService.fetcherServiceMap[app.name]?[storage.storageBucket] + if fetcherService == nil { + fetcherService = GTMSessionFetcherService() + fetcherService?.isRetryEnabled = true + fetcherService?.retryBlock = retryWhenOffline + fetcherService?.allowLocalhostRequest = true + fetcherService?.maxRetryInterval = storage.maxOperationRetryInterval + fetcherService?.testBlock = testBlock + let authorizer = StorageTokenAuthorizer( + googleAppID: app.options.googleAppID, + callbackQueue: storage.callbackQueue, + authProvider: storage.auth, + appCheck: storage.appCheck + ) + fetcherService?.authorizer = authorizer + StorageFetcherService.fetcherServiceMap[app.name]?[storage.storageBucket] = fetcherService + } + if storage.usesEmulator { + fetcherService?.allowLocalhostRequest = true + fetcherService?.allowedInsecureSchemes = ["http"] + } + _fetcherService = fetcherService + return fetcherService! + } + + /// Update the testBlock for unit testing. Save it as a property since this may be called before + /// fetcherService is initialized. + func updateTestBlock(_ block: @escaping GTMSessionFetcherTestBlock) { + testBlock = block + if let _fetcherService { + _fetcherService.testBlock = testBlock + } + } + + private var testBlock: GTMSessionFetcherTestBlock? + + /// Map of apps to a dictionary of buckets to GTMSessionFetcherService. + private static var fetcherServiceMap: [String: [String: GTMSessionFetcherService]] = [:] + + private var retryWhenOffline: GTMSessionFetcherRetryBlock = { + (suggestedWillRetry: Bool, + error: Error?, + response: @escaping GTMSessionFetcherRetryResponse) in + var shouldRetry = suggestedWillRetry + // GTMSessionFetcher does not consider being offline a retryable error, but we do, so we + // special-case it here. + if !shouldRetry, error != nil { + shouldRetry = (error as? NSError)?.code == URLError.notConnectedToInternet.rawValue + } + response(shouldRetry) + } +} diff --git a/FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift b/FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift index 868fad36f45..be4e99f4066 100644 --- a/FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageGetDownloadURLTask.swift @@ -14,21 +14,13 @@ import Foundation -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif - /// Task which provides the ability to get a download URL for an object in Firebase Storage. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) enum StorageGetDownloadURLTask { static func getDownloadURLTask(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, completion: ((_: URL?, _: Error?) -> Void)?) { StorageInternalTask(reference: reference, - fetcherService: fetcherService, queue: queue, httpMethod: "GET", fetcherComment: "GetDownloadURLTask") { (data: Data?, error: Error?) in diff --git a/FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift b/FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift index 5886f2ae551..dffbe26c46c 100644 --- a/FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageGetMetadataTask.swift @@ -14,21 +14,13 @@ import Foundation -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif - /// Task which provides the ability to delete an object in Firebase Storage. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) enum StorageGetMetadataTask { static func getMetadataTask(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, completion: ((_: StorageMetadata?, _: Error?) -> Void)?) { StorageInternalTask(reference: reference, - fetcherService: fetcherService, queue: queue, httpMethod: "GET", fetcherComment: "GetMetadataTask") { (data: Data?, error: Error?) in diff --git a/FirebaseStorage/Sources/Internal/StorageInternalTask.swift b/FirebaseStorage/Sources/Internal/StorageInternalTask.swift index 3d2233e4b90..82429009e29 100644 --- a/FirebaseStorage/Sources/Internal/StorageInternalTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageInternalTask.swift @@ -27,35 +27,35 @@ class StorageInternalTask: StorageTask { @discardableResult init(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, request: URLRequest? = nil, httpMethod: String, fetcherComment: String, completion: ((_: Data?, _: Error?) -> Void)?) { - super.init(reference: reference, service: fetcherService, queue: queue) + super.init(reference: reference, queue: queue) // Prepare a task and begins execution. dispatchQueue.async { [self] in self.state = .queueing - var request = request ?? self.baseRequest - request.httpMethod = httpMethod - request.timeoutInterval = self.reference.storage.maxOperationRetryTime + Task { + let fetcherService = await reference.storage.fetcherService + .fetcherService(reference.storage) - let fetcher = self.fetcherService.fetcher(with: request) - fetcher.comment = fetcherComment - self.fetcher = fetcher + var request = request ?? self.baseRequest + request.httpMethod = httpMethod + request.timeoutInterval = self.reference.storage.maxOperationRetryTime - Task { - let callbackQueue = reference.storage.fetcherService != nil ? - reference.storage.fetcherServiceForApp.callbackQueue : DispatchQueue.main + let fetcher = fetcherService.fetcher(with: request) + fetcher.comment = fetcherComment + self.fetcher = fetcher + let callbackQueue = reference.storage.callbackQueue do { let data = try await self.fetcher?.beginFetch() - callbackQueue?.async { + callbackQueue.async { completion?(data, nil) } } catch { - callbackQueue?.async { + callbackQueue.async { completion?(nil, StorageErrorCode.error(withServerError: error as NSError, ref: self.reference)) } diff --git a/FirebaseStorage/Sources/Internal/StorageListTask.swift b/FirebaseStorage/Sources/Internal/StorageListTask.swift index c8a69c2ba24..8d9369a6768 100644 --- a/FirebaseStorage/Sources/Internal/StorageListTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageListTask.swift @@ -14,17 +14,10 @@ import Foundation -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif - /// A Task that lists the entries under a StorageReference @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) enum StorageListTask { static func listTask(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, pageSize: Int64?, previousPageToken: String?, @@ -60,7 +53,6 @@ enum StorageListTask { ) StorageInternalTask(reference: reference, - fetcherService: fetcherService, queue: queue, request: request, httpMethod: "GET", diff --git a/FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift b/FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift index 470cf5f7144..12f4924576f 100644 --- a/FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift +++ b/FirebaseStorage/Sources/Internal/StorageUpdateMetadataTask.swift @@ -14,17 +14,10 @@ import Foundation -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif - /// A Task that lists the entries under a StorageReference @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) enum StorageUpdateMetadataTask { static func updateMetadataTask(reference: StorageReference, - fetcherService: GTMSessionFetcherService, queue: DispatchQueue, metadata: StorageMetadata, completion: ((_: StorageMetadata?, _: Error?) -> Void)?) { @@ -37,7 +30,6 @@ enum StorageUpdateMetadataTask { } StorageInternalTask(reference: reference, - fetcherService: fetcherService, queue: queue, request: request, httpMethod: "PATCH", diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index 3ac124343ed..c9a33522b03 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -17,11 +17,6 @@ import Foundation import FirebaseAppCheckInterop import FirebaseAuthInterop import FirebaseCore -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif // Avoids exposing internal FirebaseCore APIs to Swift users. @_implementationOnly import FirebaseCoreExtension @@ -129,7 +124,7 @@ import FirebaseCore /// - Returns: An instance of `StorageReference` referencing the root of the storage bucket. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) @objc open func reference() -> StorageReference { - ensureConfigured() + configured = true let path = StoragePath(with: storageBucket) return StorageReference(storage: self, path: path) } @@ -146,7 +141,7 @@ import FirebaseCore /// initialize this Storage instance. @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) @objc open func reference(forURL url: String) -> StorageReference { - ensureConfigured() + configured = true do { let path = try StoragePath.path(string: url) @@ -178,7 +173,7 @@ import FirebaseCore /// - Throws: Throws an Error if `url` is not associated with the `FirebaseApp` used to initialize /// this Storage instance. open func reference(for url: URL) throws -> StorageReference { - ensureConfigured() + configured = true var path: StoragePath do { path = try StoragePath.path(string: url.absoluteString) @@ -222,7 +217,7 @@ import FirebaseCore guard port >= 0 else { fatalError("Invalid port argument: Port must be greater or equal to zero.") } - guard fetcherService == nil else { + guard configured == false else { fatalError("Cannot connect to emulator after Storage SDK initialization. " + "Call useEmulator(host:port:) before creating a Storage " + "reference or trying to load data.") @@ -254,14 +249,7 @@ import FirebaseCore // MARK: - Internal and Private APIs - var fetcherService: GTMSessionFetcherService? - - var fetcherServiceForApp: GTMSessionFetcherService { - guard let value = fetcherService else { - fatalError("Internal error: fetcherServiceForApp not yet configured.") - } - return value - } + let fetcherService = StorageFetcherService() let dispatchQueue: DispatchQueue @@ -275,7 +263,6 @@ import FirebaseCore host = "firebasestorage.googleapis.com" scheme = "https" port = 443 - fetcherService = nil // Configured in `ensureConfigured()` // Must be a serial queue. dispatchQueue = DispatchQueue(label: "com.google.firebase.storage") maxDownloadRetryTime = 600.0 @@ -286,57 +273,13 @@ import FirebaseCore maxUploadRetryInterval = Storage.computeRetryInterval(fromRetryTime: maxUploadRetryTime) } - /// Map of apps to a dictionary of buckets to GTMSessionFetcherService. - private static let fetcherServiceLock = NSObject() - private static var fetcherServiceMap: [String: [String: GTMSessionFetcherService]] = [:] - private static var retryWhenOffline: GTMSessionFetcherRetryBlock = { - (suggestedWillRetry: Bool, - error: Error?, - response: @escaping GTMSessionFetcherRetryResponse) in - var shouldRetry = suggestedWillRetry - // GTMSessionFetcher does not consider being offline a retryable error, but we do, so we - // special-case it here. - if !shouldRetry, error != nil { - shouldRetry = (error as? NSError)?.code == URLError.notConnectedToInternet.rawValue - } - response(shouldRetry) - } + let auth: AuthInterop? + let appCheck: AppCheckInterop? + let storageBucket: String + var usesEmulator = false - private static func initFetcherServiceForApp(_ app: FirebaseApp, - _ bucket: String, - _ auth: AuthInterop?, - _ appCheck: AppCheckInterop?, - _ callbackQueue: DispatchQueue) - -> GTMSessionFetcherService { - objc_sync_enter(fetcherServiceLock) - defer { objc_sync_exit(fetcherServiceLock) } - var bucketMap = fetcherServiceMap[app.name] - if bucketMap == nil { - bucketMap = [:] - fetcherServiceMap[app.name] = bucketMap - } - var fetcherService = bucketMap?[bucket] - if fetcherService == nil { - fetcherService = GTMSessionFetcherService() - fetcherService?.isRetryEnabled = true - fetcherService?.retryBlock = retryWhenOffline - fetcherService?.allowLocalhostRequest = true - let authorizer = StorageTokenAuthorizer( - googleAppID: app.options.googleAppID, - callbackQueue: callbackQueue, - authProvider: auth, - appCheck: appCheck - ) - fetcherService?.authorizer = authorizer - bucketMap?[bucket] = fetcherService - } - return fetcherService! - } - - private let auth: AuthInterop? - private let appCheck: AppCheckInterop? - private let storageBucket: String - private var usesEmulator: Bool = false + /// Once `configured` is true, the emulator can no longer be enabled. + var configured = false /// A map of active instances, grouped by app. Keys are FirebaseApp names and values are /// instances of Storage associated with the given app. @@ -354,9 +297,9 @@ import FirebaseCore /// Performs a crude translation of the user provided timeouts to the retry intervals that /// GTMSessionFetcher accepts. GTMSessionFetcher times out operations if the time between - /// individual - /// retry attempts exceed a certain threshold, while our API contract looks at the total observed - /// time of the operation (i.e. the sum of all retries). + /// individual retry attempts exceed a certain threshold, while our API contract looks at the + /// total + /// observed time of the operation (i.e. the sum of all retries). /// @param retryTime A timeout that caps the sum of all retry attempts /// @return A timeout that caps the timeout of the last retry attempt static func computeRetryInterval(fromRetryTime retryTime: TimeInterval) -> TimeInterval { @@ -374,19 +317,6 @@ import FirebaseCore return lastInterval } - /// Configures the storage instance. Freezes the host setting. - private func ensureConfigured() { - guard fetcherService == nil else { - return - } - fetcherService = Storage.initFetcherServiceForApp(app, storageBucket, auth, appCheck, - callbackQueue) - if usesEmulator { - fetcherService?.allowLocalhostRequest = true - fetcherService?.allowedInsecureSchemes = ["http"] - } - } - private static func bucket(for app: FirebaseApp) -> String { guard let bucket = app.options.storageBucket else { fatalError("No default Storage bucket found. Did you configure Firebase Storage properly?") diff --git a/FirebaseStorage/Sources/StorageDownloadTask.swift b/FirebaseStorage/Sources/StorageDownloadTask.swift index 5bfe3ee4b50..0f0776c777a 100644 --- a/FirebaseStorage/Sources/StorageDownloadTask.swift +++ b/FirebaseStorage/Sources/StorageDownloadTask.swift @@ -39,7 +39,9 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { * Prepares a task and begins execution. */ @objc open func enqueue() { - enqueueImplementation() + Task { + await enqueueImplementation() + } } /** @@ -80,7 +82,9 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { self.state = .resuming self.fire(for: .resume, snapshot: self.snapshot) self.state = .running - self.enqueueImplementation(resumeWith: self.downloadData) + Task { + await self.enqueueImplementation(resumeWith: self.downloadData) + } } } @@ -93,90 +97,87 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { // MARK: - Internal Implementations override init(reference: StorageReference, - service: GTMSessionFetcherService, queue: DispatchQueue, file: URL?) { - super.init(reference: reference, service: service, queue: queue, file: file) + super.init(reference: reference, queue: queue, file: file) } deinit { self.fetcher?.stopFetching() } - private func enqueueImplementation(resumeWith resumeData: Data? = nil) { - dispatchQueue.async { [weak self] in - guard let self = self else { return } - self.state = .queueing - var request = self.baseRequest - request.httpMethod = "GET" - request.timeoutInterval = self.reference.storage.maxDownloadRetryTime - var components = URLComponents(url: request.url!, resolvingAgainstBaseURL: false) - components?.query = "alt=media" - request.url = components?.url - - var fetcher: GTMSessionFetcher - if let resumeData { - fetcher = GTMSessionFetcher(downloadResumeData: resumeData) - fetcher.comment = "Resuming DownloadTask" - } else { - fetcher = self.fetcherService.fetcher(with: request) - fetcher.comment = "Starting DownloadTask" - } - fetcher.maxRetryInterval = self.reference.storage.maxDownloadRetryInterval - - if let fileURL { - // Handle file downloads - fetcher.destinationFileURL = fileURL - fetcher.downloadProgressBlock = { [weak self] (bytesWritten: Int64, - totalBytesWritten: Int64, - totalBytesExpectedToWrite: Int64) in - guard let self = self else { return } - self.state = .progress - self.progress.completedUnitCount = totalBytesWritten - self.progress.totalUnitCount = totalBytesExpectedToWrite - self.fire(for: .progress, snapshot: self.snapshot) - self.state = .running - } - } else { - // Handle data downloads - fetcher.receivedProgressBlock = { [weak self] (bytesWritten: Int64, - totalBytesWritten: Int64) in - guard let self = self else { return } - self.state = .progress - self.progress.completedUnitCount = totalBytesWritten - if let totalLength = self.fetcher?.response?.expectedContentLength { - self.progress.totalUnitCount = totalLength - } - self.fire(for: .progress, snapshot: self.snapshot) - self.state = .running - } - } - self.fetcher = fetcher - self.state = .running - Task { - do { - let data = try await self.fetcher?.beginFetch() - // Fire last progress updates - self.fire(for: .progress, snapshot: self.snapshot) + private func enqueueImplementation(resumeWith resumeData: Data? = nil) async { + state = .queueing - // Download completed successfully, fire completion callbacks - self.state = .success - if let data { - self.downloadData = data + var request = baseRequest + request.httpMethod = "GET" + request.timeoutInterval = reference.storage.maxDownloadRetryTime + var components = URLComponents(url: request.url!, resolvingAgainstBaseURL: false) + components?.query = "alt=media" + request.url = components?.url + + var fetcher: GTMSessionFetcher + if let resumeData { + fetcher = GTMSessionFetcher(downloadResumeData: resumeData) + fetcher.comment = "Resuming DownloadTask" + } else { + let fetcherService = await reference.storage.fetcherService.fetcherService(reference.storage) + + fetcher = fetcherService.fetcher(with: request) + fetcher.comment = "Starting DownloadTask" + } + fetcher.maxRetryInterval = reference.storage.maxDownloadRetryInterval + + if let fileURL { + // Handle file downloads + fetcher.destinationFileURL = fileURL + fetcher.downloadProgressBlock = { [weak self] (bytesWritten: Int64, + totalBytesWritten: Int64, + totalBytesExpectedToWrite: Int64) in + guard let self = self else { return } + self.state = .progress + self.progress.completedUnitCount = totalBytesWritten + self.progress.totalUnitCount = totalBytesExpectedToWrite + self.fire(for: .progress, snapshot: self.snapshot) + self.state = .running + } + } else { + // Handle data downloads + fetcher.receivedProgressBlock = { [weak self] (bytesWritten: Int64, + totalBytesWritten: Int64) in + guard let self = self else { return } + self.state = .progress + self.progress.completedUnitCount = totalBytesWritten + if let totalLength = self.fetcher?.response?.expectedContentLength { + self.progress.totalUnitCount = totalLength } - self.fire(for: .success, snapshot: self.snapshot) - } catch { self.fire(for: .progress, snapshot: self.snapshot) - self.state = .failed - self.error = StorageErrorCode.error( - withServerError: error as NSError, - ref: self.reference - ) - self.fire(for: .failure, snapshot: self.snapshot) - } - self.removeAllObservers() + self.state = .running + } + } + self.fetcher = fetcher + state = .running + do { + let data = try await self.fetcher?.beginFetch() + // Fire last progress updates + fire(for: .progress, snapshot: snapshot) + + // Download completed successfully, fire completion callbacks + state = .success + if let data { + downloadData = data } + fire(for: .success, snapshot: snapshot) + } catch { + fire(for: .progress, snapshot: snapshot) + state = .failed + self.error = StorageErrorCode.error( + withServerError: error as NSError, + ref: reference + ) + fire(for: .failure, snapshot: snapshot) } + removeAllObservers() } func cancel(withError error: NSError) { @@ -186,6 +187,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { self.fetcher?.stopFetching() self.error = error self.fire(for: .failure, snapshot: self.snapshot) + self.removeAllObservers() } } } diff --git a/FirebaseStorage/Sources/StorageObservableTask.swift b/FirebaseStorage/Sources/StorageObservableTask.swift index 6567482b2e9..4330526496e 100644 --- a/FirebaseStorage/Sources/StorageObservableTask.swift +++ b/FirebaseStorage/Sources/StorageObservableTask.swift @@ -14,12 +14,6 @@ import Foundation -#if COCOAPODS - import GTMSessionFetcher -#else - import GTMSessionFetcherCore -#endif - /** * An extended `StorageTask` providing observable semantics that can be used for responding to changes * in task state. @@ -134,7 +128,6 @@ import Foundation // MARK: - Internal Implementations init(reference: StorageReference, - service: GTMSessionFetcherService, queue: DispatchQueue, file: URL?) { handlerDictionaries = [ @@ -146,7 +139,7 @@ import Foundation ] handleToStatusMap = [:] fileURL = file - super.init(reference: reference, service: service, queue: queue) + super.init(reference: reference, queue: queue) } func updateHandlerDictionary(for status: StorageTaskStatus, diff --git a/FirebaseStorage/Sources/StorageReference.swift b/FirebaseStorage/Sources/StorageReference.swift index d341bbdf14d..26d4b27ee5a 100644 --- a/FirebaseStorage/Sources/StorageReference.swift +++ b/FirebaseStorage/Sources/StorageReference.swift @@ -128,7 +128,6 @@ import Foundation putMetadata.name = (path as NSString).lastPathComponent as String } let task = StorageUploadTask(reference: self, - service: storage.fetcherServiceForApp, queue: storage.dispatchQueue, data: uploadData, metadata: putMetadata) @@ -175,7 +174,6 @@ import Foundation putMetadata.name = (path as NSString).lastPathComponent as String } let task = StorageUploadTask(reference: self, - service: storage.fetcherServiceForApp, queue: storage.dispatchQueue, file: fileURL, metadata: putMetadata) @@ -199,9 +197,7 @@ import Foundation @objc(dataWithMaxSize:completion:) @discardableResult open func getData(maxSize: Int64, completion: @escaping ((_: Data?, _: Error?) -> Void)) -> StorageDownloadTask { - let fetcherService = storage.fetcherServiceForApp let task = StorageDownloadTask(reference: self, - service: fetcherService, queue: storage.dispatchQueue, file: nil) @@ -240,9 +236,7 @@ import Foundation /// or an error on failure. @objc(downloadURLWithCompletion:) open func downloadURL(completion: @escaping ((_: URL?, _: Error?) -> Void)) { - let fetcherService = storage.fetcherServiceForApp StorageGetDownloadURLTask.getDownloadURLTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, completion: completion) } @@ -252,7 +246,6 @@ import Foundation /// in the Firebase Console. /// - Throws: An error if the download URL could not be retrieved. /// - Returns: The URL on success. - @available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *) open func downloadURL() async throws -> URL { return try await withCheckedThrowingContinuation { continuation in self.downloadURL { result in @@ -280,9 +273,7 @@ import Foundation @objc(writeToFile:completion:) @discardableResult open func write(toFile fileURL: URL, completion: ((_: URL?, _: Error?) -> Void)?) -> StorageDownloadTask { - let fetcherService = storage.fetcherServiceForApp let task = StorageDownloadTask(reference: self, - service: fetcherService, queue: storage.dispatchQueue, file: fileURL) @@ -322,7 +313,6 @@ import Foundation /// the current `StorageReference`. @objc(listAllWithCompletion:) open func listAll(completion: @escaping ((_: StorageListResult?, _: Error?) -> Void)) { - let fetcherService = storage.fetcherServiceForApp var prefixes = [StorageReference]() var items = [StorageReference]() @@ -343,7 +333,6 @@ import Foundation if let pageToken = listResult.pageToken { StorageListTask.listTask(reference: strongSelf, - fetcherService: fetcherService, queue: strongSelf.storage.dispatchQueue, pageSize: nil, previousPageToken: pageToken, @@ -358,7 +347,6 @@ import Foundation } StorageListTask.listTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, pageSize: nil, previousPageToken: nil, @@ -372,7 +360,6 @@ import Foundation /// `listAll()` is only available for projects using Firebase Rules Version 2. /// - Throws: An error if the list operation failed. /// - Returns: All items and prefixes under the current `StorageReference`. - @available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *) open func listAll() async throws -> StorageListResult { return try await withCheckedThrowingContinuation { continuation in self.listAll { result in @@ -401,9 +388,7 @@ import Foundation message: "Argument 'maxResults' must be between 1 and 1000 inclusive." )) } else { - let fetcherService = storage.fetcherServiceForApp StorageListTask.listTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, pageSize: maxResults, previousPageToken: nil, @@ -436,9 +421,7 @@ import Foundation message: "Argument 'maxResults' must be between 1 and 1000 inclusive." )) } else { - let fetcherService = storage.fetcherServiceForApp StorageListTask.listTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, pageSize: maxResults, previousPageToken: pageToken, @@ -453,9 +436,7 @@ import Foundation /// or an error on failure. @objc(metadataWithCompletion:) open func getMetadata(completion: @escaping ((_: StorageMetadata?, _: Error?) -> Void)) { - let fetcherService = storage.fetcherServiceForApp StorageGetMetadataTask.getMetadataTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, completion: completion) } @@ -463,7 +444,6 @@ import Foundation /// Retrieves metadata associated with an object at the current path. /// - Throws: An error if the object metadata could not be retrieved. /// - Returns: The object metadata on success. - @available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *) open func getMetadata() async throws -> StorageMetadata { return try await withCheckedThrowingContinuation { continuation in self.getMetadata { result in @@ -480,9 +460,7 @@ import Foundation @objc(updateMetadata:completion:) open func updateMetadata(_ metadata: StorageMetadata, completion: ((_: StorageMetadata?, _: Error?) -> Void)?) { - let fetcherService = storage.fetcherServiceForApp StorageUpdateMetadataTask.updateMetadataTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, metadata: metadata, completion: completion) @@ -492,7 +470,6 @@ import Foundation /// - Parameter metadata: A `StorageMetadata` object with the metadata to update. /// - Throws: An error if the metadata update operation failed. /// - Returns: The object metadata on success. - @available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *) open func updateMetadata(_ metadata: StorageMetadata) async throws -> StorageMetadata { return try await withCheckedThrowingContinuation { continuation in self.updateMetadata(metadata) { result in @@ -507,21 +484,18 @@ import Foundation /// - Parameter completion: A completion block which returns a nonnull error on failure. @objc(deleteWithCompletion:) open func delete(completion: ((_: Error?) -> Void)?) { - let fetcherService = storage.fetcherServiceForApp let completionWrap = { (_: Data?, error: Error?) in if let completion { completion(error) } } StorageDeleteTask.deleteTask(reference: self, - fetcherService: fetcherService, queue: storage.dispatchQueue, completion: completionWrap) } /// Deletes the object at the current path. /// - Throws: An error if the delete operation failed. - @available(iOS 13, tvOS 13, macOS 10.15, watchOS 8, *) open func delete() async throws { return try await withCheckedThrowingContinuation { continuation in self.delete { error in @@ -590,7 +564,7 @@ import Foundation completion: ((_: StorageMetadata?, _: Error?) -> Void)?) { if let completion { task.completionMetadata = completion - let callbackQueue = storage.fetcherServiceForApp.callbackQueue ?? DispatchQueue.main + let callbackQueue = storage.callbackQueue task.observe(.success) { snapshot in callbackQueue.async { diff --git a/FirebaseStorage/Sources/StorageTask.swift b/FirebaseStorage/Sources/StorageTask.swift index 356d131373b..a745b50db32 100644 --- a/FirebaseStorage/Sources/StorageTask.swift +++ b/FirebaseStorage/Sources/StorageTask.swift @@ -81,16 +81,11 @@ import Foundation */ let dispatchQueue: DispatchQueue - let fetcherService: GTMSessionFetcherService - let baseRequest: URLRequest init(reference: StorageReference, - service: GTMSessionFetcherService, queue: DispatchQueue) { self.reference = reference - fetcherService = service - fetcherService.maxRetryInterval = reference.storage.maxOperationRetryInterval dispatchQueue = queue state = .unknown progress = Progress(totalUnitCount: 0) diff --git a/FirebaseStorage/Sources/StorageUploadTask.swift b/FirebaseStorage/Sources/StorageUploadTask.swift index a0a4bea7eac..5870d4e3689 100644 --- a/FirebaseStorage/Sources/StorageUploadTask.swift +++ b/FirebaseStorage/Sources/StorageUploadTask.swift @@ -54,68 +54,71 @@ import Foundation } self.state = .queueing - var request = self.baseRequest - request.httpMethod = "POST" - request.timeoutInterval = self.reference.storage.maxUploadRetryTime let dataRepresentation = self.uploadMetadata.dictionaryRepresentation() let bodyData = try? JSONSerialization.data(withJSONObject: dataRepresentation) - request.httpBody = bodyData - request.setValue("application/json; charset=UTF-8", forHTTPHeaderField: "Content-Type") - if let count = bodyData?.count { - request.setValue("\(count)", forHTTPHeaderField: "Content-Length") - } + Task { + let fetcherService = await reference.storage.fetcherService + .fetcherService(reference.storage) + var request = self.baseRequest + request.httpMethod = "POST" + request.timeoutInterval = self.reference.storage.maxUploadRetryTime + request.httpBody = bodyData + request.setValue("application/json; charset=UTF-8", forHTTPHeaderField: "Content-Type") + if let count = bodyData?.count { + request.setValue("\(count)", forHTTPHeaderField: "Content-Length") + } - var components = URLComponents(url: request.url!, resolvingAgainstBaseURL: false) - if components?.host == "www.googleapis.com", - let path = components?.path { - components?.percentEncodedPath = "/upload\(path)" - } - guard let path = self.GCSEscapedString(self.uploadMetadata.path) else { - fatalError("Internal error enqueueing a Storage task") - } - components?.percentEncodedQuery = "uploadType=resumable&name=\(path)" + var components = URLComponents(url: request.url!, resolvingAgainstBaseURL: false) + if components?.host == "www.googleapis.com", + let path = components?.path { + components?.percentEncodedPath = "/upload\(path)" + } + guard let path = self.GCSEscapedString(self.uploadMetadata.path) else { + fatalError("Internal error enqueueing a Storage task") + } + components?.percentEncodedQuery = "uploadType=resumable&name=\(path)" - request.url = components?.url + request.url = components?.url - guard let contentType = self.uploadMetadata.contentType else { - fatalError("Internal error enqueueing a Storage task") - } - let uploadFetcher = GTMSessionUploadFetcher( - request: request, - uploadMIMEType: contentType, - chunkSize: self.reference.storage.uploadChunkSizeBytes, - fetcherService: self.fetcherService - ) - if let uploadData { - uploadFetcher.uploadData = uploadData - uploadFetcher.comment = "Data UploadTask" - } else if let fileURL { - uploadFetcher.uploadFileURL = fileURL - uploadFetcher.comment = "File UploadTask" + guard let contentType = self.uploadMetadata.contentType else { + fatalError("Internal error enqueueing a Storage task") + } - if GULAppEnvironmentUtil.isAppExtension() { - uploadFetcher.useBackgroundSession = false + let uploadFetcher = GTMSessionUploadFetcher( + request: request, + uploadMIMEType: contentType, + chunkSize: self.reference.storage.uploadChunkSizeBytes, + fetcherService: fetcherService + ) + if let uploadData { + uploadFetcher.uploadData = uploadData + uploadFetcher.comment = "Data UploadTask" + } else if let fileURL { + uploadFetcher.uploadFileURL = fileURL + uploadFetcher.comment = "File UploadTask" + + if GULAppEnvironmentUtil.isAppExtension() { + uploadFetcher.useBackgroundSession = false + } } - } - uploadFetcher.maxRetryInterval = self.reference.storage.maxUploadRetryInterval + uploadFetcher.maxRetryInterval = self.reference.storage.maxUploadRetryInterval - uploadFetcher.sendProgressBlock = { [weak self] (bytesSent: Int64, totalBytesSent: Int64, - totalBytesExpectedToSend: Int64) in - guard let self = self else { return } - self.state = .progress - self.progress.completedUnitCount = totalBytesSent - self.progress.totalUnitCount = totalBytesExpectedToSend - self.metadata = self.uploadMetadata - self.fire(for: .progress, snapshot: self.snapshot) - self.state = .running - } - self.uploadFetcher = uploadFetcher + uploadFetcher.sendProgressBlock = { [weak self] (bytesSent: Int64, totalBytesSent: Int64, + totalBytesExpectedToSend: Int64) in + guard let self = self else { return } + self.state = .progress + self.progress.completedUnitCount = totalBytesSent + self.progress.totalUnitCount = totalBytesExpectedToSend + self.metadata = self.uploadMetadata + self.fire(for: .progress, snapshot: self.snapshot) + self.state = .running + } + self.uploadFetcher = uploadFetcher - // Process fetches - self.state = .running - Task { + // Process fetches + self.state = .running do { let data = try await self.uploadFetcher?.beginFetch() // Fire last progress updates @@ -208,14 +211,13 @@ import Foundation // MARK: - Internal Implementations init(reference: StorageReference, - service: GTMSessionFetcherService, queue: DispatchQueue, file: URL? = nil, data: Data? = nil, metadata: StorageMetadata) { uploadMetadata = metadata uploadData = data - super.init(reference: reference, service: service, queue: queue, file: file) + super.init(reference: reference, queue: queue, file: file) if uploadMetadata.contentType == nil { uploadMetadata.contentType = StorageUtils.MIMETypeForExtension(file?.pathExtension) diff --git a/FirebaseStorage/Tests/Unit/StorageComponentTests.swift b/FirebaseStorage/Tests/Unit/StorageComponentTests.swift index c8c42f949d9..6342a15c590 100644 --- a/FirebaseStorage/Tests/Unit/StorageComponentTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageComponentTests.swift @@ -32,14 +32,14 @@ class StorageComponentTests: StorageTestHelpers { /// Tests that a Storage instance can be created properly. func testStorageInstanceCreation() throws { - let app = try XCTUnwrap(StorageComponentTests.app) + let app = try XCTUnwrap(app) let storage1 = Storage.storage(app: app, url: "gs://foo-bar.appspot.com") XCTAssertNotNil(storage1) } /// Tests that a Storage instances are reused properly. func testMultipleComponentInstancesCreated() throws { - let app = try XCTUnwrap(StorageComponentTests.app) + let app = try XCTUnwrap(app) let storage1 = Storage.storage(app: app, url: "gs://foo-bar.appspot.com") let storage2 = Storage.storage(app: app, url: "gs://foo-bar.appspot.com") diff --git a/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift b/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift index 52484c17fca..1f6a2ed4efd 100644 --- a/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift @@ -56,7 +56,6 @@ class StorageDeleteTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageDeleteTask.deleteTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { _, error in expectation.fulfill() @@ -82,7 +81,6 @@ class StorageDeleteTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageDeleteTask.deleteTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { _, error in expectation.fulfill() @@ -105,7 +103,6 @@ class StorageDeleteTests: StorageTestHelpers { let ref = StorageReference(storage: storage, path: path) StorageDeleteTask.deleteTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { _, error in expectation.fulfill() @@ -113,54 +110,39 @@ class StorageDeleteTests: StorageTestHelpers { waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthenticated() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthenticatedBlock() + func testUnsuccessfulFetchUnauthenticated() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthenticatedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageDeleteTask.deleteTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { _, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthenticated.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + try await ref.delete() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthenticated.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthorized() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthorizedBlock() + func testUnsuccessfulFetchUnauthorized() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthorizedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageDeleteTask.deleteTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { _, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthorized.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + try await ref.delete() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthorized.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchObjectDoesntExist() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = notFoundBlock() + func testUnsuccessfulFetchObjectDoesntExist() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(notFoundBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageDeleteTask.deleteTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { _, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.objectNotFound.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + try await ref.delete() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.objectNotFound.rawValue) } - waitForExpectation(test: self) } } diff --git a/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift b/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift index af45994e939..a2428d792fc 100644 --- a/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift @@ -56,7 +56,6 @@ class StorageGetMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageGetMetadataTask.getMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { metadata, error in expectation.fulfill() @@ -82,7 +81,6 @@ class StorageGetMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageGetMetadataTask.getMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { metadata, error in expectation.fulfill() @@ -105,7 +103,6 @@ class StorageGetMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage, path: path) StorageGetMetadataTask.getMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { metadata, error in expectation.fulfill() @@ -113,55 +110,40 @@ class StorageGetMetadataTests: StorageTestHelpers { waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthenticated() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthenticatedBlock() + func testUnsuccessfulFetchUnauthenticated() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthenticatedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageGetMetadataTask.getMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthenticated.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.getMetadata() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthenticated.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthorized() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthorizedBlock() + func testUnsuccessfulFetchUnauthorized() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthorizedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageGetMetadataTask.getMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthorized.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.getMetadata() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthorized.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchObjectDoesntExist() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = notFoundBlock() + func testUnsuccessfulFetchObjectDoesntExist() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(notFoundBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageGetMetadataTask.getMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.objectNotFound.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.getMetadata() + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.objectNotFound.rawValue) } - waitForExpectation(test: self) } func testUnsuccessfulFetchBadJSON() { @@ -172,7 +154,6 @@ class StorageGetMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageGetMetadataTask.getMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self ) { metadata, error in XCTAssertNil(metadata) diff --git a/FirebaseStorage/Tests/Unit/StorageListTests.swift b/FirebaseStorage/Tests/Unit/StorageListTests.swift index 121a7954292..1b03ee62a18 100644 --- a/FirebaseStorage/Tests/Unit/StorageListTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageListTests.swift @@ -108,7 +108,6 @@ class StorageListTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageListTask.listTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, pageSize: nil, previousPageToken: nil @@ -118,25 +117,24 @@ class StorageListTests: StorageTestHelpers { waitForExpectation(test: self) } - func testDefaultListWithEmulator() { - let expectation = self.expectation(description: #function) + func testDefaultListWithEmulator() async throws { let storage = self.storage() storage.useEmulator(withHost: "localhost", port: 8080) - fetcherService?.allowLocalhostRequest = true - fetcherService?.testBlock = { (fetcher: GTMSessionFetcher, - response: GTMSessionFetcherTestResponse) in + let testBlock = { (fetcher: GTMSessionFetcher, + response: GTMSessionFetcherTestResponse) in let url = fetcher.request!.url! XCTAssertEqual(url.scheme, "http") XCTAssertEqual(url.host, "localhost") XCTAssertEqual(url.port, 8080) XCTAssertEqual(url.path, "/v0/b/bucket/o") let queryItems = URLComponents(url: url, resolvingAgainstBaseURL: false)!.queryItems! - XCTAssertEqual(queryItems.count, 2) + XCTAssertEqual(queryItems.count, 3) for item in queryItems { switch item.name { case "prefix": XCTAssertEqual(item.value, "object/") case "delimiter": XCTAssertEqual(item.value, "/") + case "maxResults": XCTAssertEqual(item.value, "123") default: XCTFail("Unexpected URLComponent Query Item") } } @@ -147,20 +145,9 @@ class StorageListTests: StorageTestHelpers { headerFields: nil) response(httpResponse, "{}".data(using: .utf8), nil) } - - let path = objectPath() - let ref = StorageReference(storage: storage, path: path) - StorageListTask.listTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - pageSize: nil, - previousPageToken: nil - ) { result, error in - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + await storage.fetcherService.updateTestBlock(testBlock) + let ref = storage.reference(withPath: "object") + let _ = try await ref.list(maxResults: 123) } func testListWithPageSizeAndPageToken() { @@ -196,7 +183,6 @@ class StorageListTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageListTask.listTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, pageSize: 42, previousPageToken: "foo" @@ -237,7 +223,6 @@ class StorageListTests: StorageTestHelpers { let ref = storage.reference(withPath: "+foo") StorageListTask.listTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, pageSize: nil, previousPageToken: nil @@ -247,9 +232,7 @@ class StorageListTests: StorageTestHelpers { waitForExpectation(test: self) } - func testListWithResponse() throws { - let expectation = self.expectation(description: #function) - + func testListWithResponse() async throws { let jsonString = "{\n" + " \"prefixes\": [\n" + " \"object/prefixWithoutSlash\",\n" + @@ -267,11 +250,10 @@ class StorageListTests: StorageTestHelpers { " ],\n" + " \"nextPageToken\": \"foo\"" + "}" - let responseData = try XCTUnwrap(jsonString.data(using: .utf8)) - fetcherService?.testBlock = { (fetcher: GTMSessionFetcher, - response: GTMSessionFetcherTestResponse) in + let testBlock = { (fetcher: GTMSessionFetcher, + response: GTMSessionFetcherTestResponse) in let httpResponse = HTTPURLResponse(url: (fetcher.request?.url)!, statusCode: 200, httpVersion: "HTTP/1.1", @@ -280,36 +262,22 @@ class StorageListTests: StorageTestHelpers { } let storage = storage() + await storage.fetcherService.updateTestBlock(testBlock) let ref = storage.reference(withPath: "object") - StorageListTask.listTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - pageSize: nil, - previousPageToken: nil - ) { result, error in - XCTAssertNotNil(result) - XCTAssertNil(error) - - XCTAssertEqual(result?.items, [ref.child("data1.dat"), ref.child("data2.dat")]) - XCTAssertEqual( - result?.prefixes, - [ref.child("prefixWithoutSlash"), ref.child("prefixWithSlash")] - ) - XCTAssertEqual(result?.pageToken, "foo") - - expectation.fulfill() - } - waitForExpectation(test: self) + let result = try await ref.list(maxResults: 1000) + XCTAssertEqual(result.items, [ref.child("data1.dat"), ref.child("data2.dat")]) + XCTAssertEqual( + result.prefixes, + [ref.child("prefixWithoutSlash"), ref.child("prefixWithSlash")] + ) + XCTAssertEqual(result.pageToken, "foo") } - func testListWithErrorResponse() throws { - let expectation = self.expectation(description: #function) - + func testListWithErrorResponse() async { let error = NSError(domain: "com.google.firebase.storage", code: 404) - fetcherService?.testBlock = { (fetcher: GTMSessionFetcher, - response: GTMSessionFetcherTestResponse) in + let testBlock = { (fetcher: GTMSessionFetcher, + response: GTMSessionFetcherTestResponse) in let httpResponse = HTTPURLResponse(url: (fetcher.request?.url)!, statusCode: 403, httpVersion: "HTTP/1.1", @@ -318,20 +286,13 @@ class StorageListTests: StorageTestHelpers { } let storage = storage() + await storage.fetcherService.updateTestBlock(testBlock) let ref = storage.reference(withPath: "object") - StorageListTask.listTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - pageSize: nil, - previousPageToken: nil - ) { result, error in - XCTAssertNotNil(error) - XCTAssertNil(result) - XCTAssertEqual((error as? NSError)!.domain, "FIRStorageErrorDomain") - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.objectNotFound.rawValue) - expectation.fulfill() + do { + let _ = try await ref.list(maxResults: 1000) + } catch { + XCTAssertEqual((error as NSError).domain, "FIRStorageErrorDomain") + XCTAssertEqual((error as NSError).code, StorageErrorCode.objectNotFound.rawValue) } - waitForExpectation(test: self) } } diff --git a/FirebaseStorage/Tests/Unit/StorageTestHelpers.swift b/FirebaseStorage/Tests/Unit/StorageTestHelpers.swift index 97f2838bfe0..9e1cfeb2951 100644 --- a/FirebaseStorage/Tests/Unit/StorageTestHelpers.swift +++ b/FirebaseStorage/Tests/Unit/StorageTestHelpers.swift @@ -22,20 +22,23 @@ import XCTest @available(iOS 13, tvOS 13, macOS 10.15, macCatalyst 13, watchOS 7, *) class StorageTestHelpers: XCTestCase { - static var app: FirebaseApp! + var app: FirebaseApp! + static var uniqueApp = 0 func storage() -> Storage { - return Storage(app: FirebaseApp.app()!, bucket: "bucket") + return Storage(app: app, bucket: "bucket") } - override class func setUp() { + override func setUp() { super.setUp() if app == nil { let options = FirebaseOptions(googleAppID: "0:0000000000000:ios:0000000000000000", gcmSenderID: "00000000000000000-00000000000-000000000") options.projectID = "myProjectID" - FirebaseApp.configure(options: options) - app = FirebaseApp(instanceWithName: "test", options: options) + StorageTestHelpers.uniqueApp += 1 + let appName = "test\(StorageTestHelpers.uniqueApp)" + FirebaseApp.configure(name: appName, options: options) + app = FirebaseApp.app(name: appName) } } diff --git a/FirebaseStorage/Tests/Unit/StorageUpdateMetadataTests.swift b/FirebaseStorage/Tests/Unit/StorageUpdateMetadataTests.swift index eb08980d84d..7650032d179 100644 --- a/FirebaseStorage/Tests/Unit/StorageUpdateMetadataTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageUpdateMetadataTests.swift @@ -21,7 +21,7 @@ import XCTest class StorageUpdateMetadataTests: StorageTestHelpers { var fetcherService: GTMSessionFetcherService? var dispatchQueue: DispatchQueue? - var metadata: StorageMetadata? + var metadata: StorageMetadata! override func setUp() { super.setUp() @@ -53,7 +53,6 @@ class StorageUpdateMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageUpdateMetadataTask.updateMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, metadata: metadata! ) { metadata, error in @@ -62,23 +61,14 @@ class StorageUpdateMetadataTests: StorageTestHelpers { waitForExpectation(test: self) } - func testSuccessfulFetch() { - let expectation = self.expectation(description: #function) - fetcherService!.testBlock = successBlock(withMetadata: metadata) + func testSuccessfulFetch() async throws { + let storage = storage() + await storage.fetcherService.updateTestBlock(successBlock(withMetadata: metadata)) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageUpdateMetadataTask.updateMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - metadata: metadata! - ) { metadata, error in - XCTAssertNil(error) - XCTAssertEqual(self.metadata?.bucket, metadata?.bucket) - XCTAssertEqual(self.metadata?.name, metadata?.name) - expectation.fulfill() - } - waitForExpectation(test: self) + let ref = StorageReference(storage: storage, path: path) + let metadata = try await ref.updateMetadata(metadata) + XCTAssertEqual(self.metadata?.bucket, metadata.bucket) + XCTAssertEqual(self.metadata?.name, metadata.name) } func testSuccessfulFetchWithEmulator() { @@ -96,7 +86,6 @@ class StorageUpdateMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage, path: path) StorageUpdateMetadataTask.updateMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, metadata: metadata! ) { metadata, error in @@ -105,58 +94,40 @@ class StorageUpdateMetadataTests: StorageTestHelpers { waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthenticated() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthenticatedBlock() + func testUnsuccessfulFetchUnauthenticated() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthenticatedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageUpdateMetadataTask.updateMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - metadata: metadata! - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthenticated.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.updateMetadata(metadata) + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthenticated.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchUnauthorized() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = unauthorizedBlock() + func testUnsuccessfulFetchUnauthorized() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(unauthorizedBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageUpdateMetadataTask.updateMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - metadata: metadata! - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.unauthorized.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.updateMetadata(metadata) + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.unauthorized.rawValue) } - waitForExpectation(test: self) } - func testUnsuccessfulFetchObjectDoesntExist() { - let expectation = self.expectation(description: #function) - - fetcherService!.testBlock = notFoundBlock() + func testUnsuccessfulFetchObjectDoesntExist() async { + let storage = storage() + await storage.fetcherService.updateTestBlock(notFoundBlock()) let path = objectPath() - let ref = StorageReference(storage: storage(), path: path) - StorageUpdateMetadataTask.updateMetadataTask( - reference: ref, - fetcherService: fetcherService!.self, - queue: dispatchQueue!.self, - metadata: metadata! - ) { metadata, error in - XCTAssertEqual((error as? NSError)!.code, StorageErrorCode.objectNotFound.rawValue) - expectation.fulfill() + let ref = StorageReference(storage: storage, path: path) + do { + let _ = try await ref.updateMetadata(metadata) + } catch { + XCTAssertEqual((error as NSError).code, StorageErrorCode.objectNotFound.rawValue) } - waitForExpectation(test: self) } func testUnsuccessfulFetchBadJSON() { @@ -167,7 +138,6 @@ class StorageUpdateMetadataTests: StorageTestHelpers { let ref = StorageReference(storage: storage(), path: path) StorageUpdateMetadataTask.updateMetadataTask( reference: ref, - fetcherService: fetcherService!.self, queue: dispatchQueue!.self, metadata: metadata! ) { metadata, error in