diff --git a/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift b/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift index 7c0e2230eee..fbaa15a5fda 100644 --- a/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift +++ b/FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift @@ -37,7 +37,6 @@ class StorageTokenAuthorizer: NSObject, GTMSessionFetcherAuthorizer { request?.setValue(googleAppID, forHTTPHeaderField: "x-firebase-gmpid") var tokenError: NSError? - let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main let fetchTokenGroup = DispatchGroup() if let auth { fetchTokenGroup.enter() @@ -101,7 +100,7 @@ class StorageTokenAuthorizer: NSObject, GTMSessionFetcherAuthorizer { var userEmail: String? - let fetcherService: GTMSessionFetcherService + let callbackQueue: DispatchQueue private let googleAppID: String private let auth: AuthInterop? private let appCheck: AppCheckInterop? @@ -109,11 +108,11 @@ class StorageTokenAuthorizer: NSObject, GTMSessionFetcherAuthorizer { private let serialAuthArgsQueue = DispatchQueue(label: "com.google.firebasestorage.authorizer") init(googleAppID: String, - fetcherService: GTMSessionFetcherService, + callbackQueue: DispatchQueue = DispatchQueue.main, authProvider: AuthInterop?, appCheck: AppCheckInterop?) { self.googleAppID = googleAppID - self.fetcherService = fetcherService + self.callbackQueue = callbackQueue auth = authProvider self.appCheck = appCheck } diff --git a/FirebaseStorage/Sources/Storage.swift b/FirebaseStorage/Sources/Storage.swift index 08fc92ac198..3ac124343ed 100644 --- a/FirebaseStorage/Sources/Storage.swift +++ b/FirebaseStorage/Sources/Storage.swift @@ -123,19 +123,7 @@ import FirebaseCore @objc public var uploadChunkSizeBytes: Int64 = .max /// A `DispatchQueue` that all developer callbacks are fired on. Defaults to the main queue. - @objc public var callbackQueue: DispatchQueue { - get { - ensureConfigured() - guard let queue = fetcherService?.callbackQueue else { - fatalError("Internal error: Failed to initialize fetcherService callbackQueue") - } - return queue - } - set(newValue) { - ensureConfigured() - fetcherService?.callbackQueue = newValue - } - } + @objc public var callbackQueue: DispatchQueue = .main /// Creates a `StorageReference` initialized at the root Firebase Storage location. /// - Returns: An instance of `StorageReference` referencing the root of the storage bucket. @@ -317,7 +305,8 @@ import FirebaseCore private static func initFetcherServiceForApp(_ app: FirebaseApp, _ bucket: String, _ auth: AuthInterop?, - _ appCheck: AppCheckInterop?) + _ appCheck: AppCheckInterop?, + _ callbackQueue: DispatchQueue) -> GTMSessionFetcherService { objc_sync_enter(fetcherServiceLock) defer { objc_sync_exit(fetcherServiceLock) } @@ -334,7 +323,7 @@ import FirebaseCore fetcherService?.allowLocalhostRequest = true let authorizer = StorageTokenAuthorizer( googleAppID: app.options.googleAppID, - fetcherService: fetcherService!, + callbackQueue: callbackQueue, authProvider: auth, appCheck: appCheck ) @@ -390,7 +379,8 @@ import FirebaseCore guard fetcherService == nil else { return } - fetcherService = Storage.initFetcherServiceForApp(app, storageBucket, auth, appCheck) + fetcherService = Storage.initFetcherServiceForApp(app, storageBucket, auth, appCheck, + callbackQueue) if usesEmulator { fetcherService?.allowLocalhostRequest = true fetcherService?.allowedInsecureSchemes = ["http"] diff --git a/FirebaseStorage/Sources/StorageDownloadTask.swift b/FirebaseStorage/Sources/StorageDownloadTask.swift index 504056464d3..5bfe3ee4b50 100644 --- a/FirebaseStorage/Sources/StorageDownloadTask.swift +++ b/FirebaseStorage/Sources/StorageDownloadTask.swift @@ -85,7 +85,6 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { } private var fetcher: GTMSessionFetcher? - private var fetcherCompletion: ((Data?, NSError?) -> Void)? var downloadData: Data? // Hold completion in object to force it to be retained until completion block is called. var completionData: ((Data?, Error?) -> Void)? @@ -104,7 +103,7 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { self.fetcher?.stopFetching() } - func enqueueImplementation(resumeWith resumeData: Data? = nil) { + private func enqueueImplementation(resumeWith resumeData: Data? = nil) { dispatchQueue.async { [weak self] in guard let self = self else { return } self.state = .queueing @@ -153,32 +152,29 @@ open class StorageDownloadTask: StorageObservableTask, StorageTaskManagement { } } self.fetcher = fetcher - - // Capture self here to retain until completion. - self.fetcherCompletion = { [self] (data: Data?, error: NSError?) in - defer { - self.removeAllObservers() - self.fetcherCompletion = nil - } - self.fire(for: .progress, snapshot: self.snapshot) - - // Handle potential issues with download - if let error { + self.state = .running + Task { + do { + let data = try await self.fetcher?.beginFetch() + // Fire last progress updates + self.fire(for: .progress, snapshot: self.snapshot) + + // Download completed successfully, fire completion callbacks + self.state = .success + if let data { + self.downloadData = data + } + self.fire(for: .success, snapshot: self.snapshot) + } catch { + self.fire(for: .progress, snapshot: self.snapshot) self.state = .failed - self.error = StorageErrorCode.error(withServerError: error, ref: self.reference) + self.error = StorageErrorCode.error( + withServerError: error as NSError, + ref: self.reference + ) self.fire(for: .failure, snapshot: self.snapshot) - return } - // Download completed successfully, fire completion callbacks - self.state = .success - if let data { - self.downloadData = data - } - self.fire(for: .success, snapshot: self.snapshot) - } - self.state = .running - self.fetcher?.beginFetch { [self] data, error in - self.fetcherCompletion?(data, error as? NSError) + self.removeAllObservers() } } } diff --git a/FirebaseStorage/Sources/StorageObservableTask.swift b/FirebaseStorage/Sources/StorageObservableTask.swift index 82be7989994..6567482b2e9 100644 --- a/FirebaseStorage/Sources/StorageObservableTask.swift +++ b/FirebaseStorage/Sources/StorageObservableTask.swift @@ -168,12 +168,11 @@ import Foundation func fire(handlers: [String: (StorageTaskSnapshot) -> Void], snapshot: StorageTaskSnapshot) { - let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main objc_sync_enter(StorageObservableTask.self) let enumeration = handlers.enumerated() objc_sync_exit(StorageObservableTask.self) for (_, handler) in enumeration { - callbackQueue.async { + reference.storage.callbackQueue.async { handler.value(snapshot) } } diff --git a/FirebaseStorage/Sources/StorageReference.swift b/FirebaseStorage/Sources/StorageReference.swift index 0ab5d286ae4..d341bbdf14d 100644 --- a/FirebaseStorage/Sources/StorageReference.swift +++ b/FirebaseStorage/Sources/StorageReference.swift @@ -206,7 +206,7 @@ import Foundation file: nil) task.completionData = completion - let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main + let callbackQueue = storage.callbackQueue task.observe(.success) { snapshot in let error = self.checkSizeOverflow(task: snapshot.task, maxSize: maxSize) @@ -288,7 +288,7 @@ import Foundation if let completion { task.completionURL = completion - let callbackQueue = fetcherService.callbackQueue ?? DispatchQueue.main + let callbackQueue = storage.callbackQueue task.observe(.success) { snapshot in callbackQueue.async { diff --git a/FirebaseStorage/Sources/StorageUploadTask.swift b/FirebaseStorage/Sources/StorageUploadTask.swift index 48564b1d9b4..a0a4bea7eac 100644 --- a/FirebaseStorage/Sources/StorageUploadTask.swift +++ b/FirebaseStorage/Sources/StorageUploadTask.swift @@ -115,38 +115,36 @@ import Foundation // Process fetches self.state = .running + Task { + do { + let data = try await self.uploadFetcher?.beginFetch() + // Fire last progress updates + self.fire(for: .progress, snapshot: self.snapshot) + + // Upload completed successfully, fire completion callbacks + self.state = .success - self.fetcherCompletion = { [self] (data: Data?, error: NSError?) in - // Fire last progress updates - self.fire(for: .progress, snapshot: self.snapshot) + guard let data = data else { + fatalError("Internal Error: uploadFetcher returned with nil data and no error") + } - // Handle potential issues with upload - if let error { + if let responseDictionary = try? JSONSerialization + .jsonObject(with: data) as? [String: AnyHashable] { + let metadata = StorageMetadata(dictionary: responseDictionary) + metadata.fileType = .file + self.metadata = metadata + } else { + self.error = StorageErrorCode.error(withInvalidRequest: data) + } + self.finishTaskWithStatus(status: .success, snapshot: self.snapshot) + } catch { + self.fire(for: .progress, snapshot: self.snapshot) self.state = .failed - self.error = StorageErrorCode.error(withServerError: error, ref: self.reference) + self.error = StorageErrorCode.error(withServerError: error as NSError, + ref: self.reference) self.metadata = self.uploadMetadata self.finishTaskWithStatus(status: .failure, snapshot: self.snapshot) - return - } - // Upload completed successfully, fire completion callbacks - self.state = .success - - guard let data = data else { - fatalError("Internal Error: fetcherCompletion returned with nil data and nil error") - } - - if let responseDictionary = try? JSONSerialization - .jsonObject(with: data) as? [String: AnyHashable] { - let metadata = StorageMetadata(dictionary: responseDictionary) - metadata.fileType = .file - self.metadata = metadata - } else { - self.error = StorageErrorCode.error(withInvalidRequest: data) } - self.finishTaskWithStatus(status: .success, snapshot: self.snapshot) - } - self.uploadFetcher?.beginFetch { [weak self] (data: Data?, error: Error?) in - self?.fetcherCompletion?(data, error as NSError?) } } } @@ -202,7 +200,6 @@ import Foundation } private var uploadFetcher: GTMSessionUploadFetcher? - private var fetcherCompletion: ((Data?, NSError?) -> Void)? private var uploadMetadata: StorageMetadata private var uploadData: Data? // Hold completion in object to force it to be retained until completion block is called. @@ -247,7 +244,6 @@ import Foundation func finishTaskWithStatus(status: StorageTaskStatus, snapshot: StorageTaskSnapshot) { fire(for: status, snapshot: snapshot) removeAllObservers() - fetcherCompletion = nil } private func GCSEscapedString(_ input: String?) -> String? { diff --git a/FirebaseStorage/Tests/Unit/StorageAuthorizerTests.swift b/FirebaseStorage/Tests/Unit/StorageAuthorizerTests.swift index 42712b7a69f..3b340179d96 100644 --- a/FirebaseStorage/Tests/Unit/StorageAuthorizerTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageAuthorizerTests.swift @@ -43,11 +43,10 @@ class StorageAuthorizerTests: StorageTestHelpers { let fetchRequest = URLRequest(url: StorageTestHelpers().objectURL()) fetcher = GTMSessionFetcher(request: fetchRequest) - fetcherService = GTMSessionFetcherService() auth = FIRAuthInteropFake(token: StorageTestAuthToken, userID: nil, error: nil) appCheck = FIRAppCheckFake() fetcher?.authorizer = StorageTokenAuthorizer(googleAppID: "dummyAppID", - fetcherService: fetcherService!, + callbackQueue: DispatchQueue.main, authProvider: auth, appCheck: appCheck) } @@ -60,53 +59,42 @@ class StorageAuthorizerTests: StorageTestHelpers { super.tearDown() } - func testSuccessfulAuth() { - let expectation = self.expectation(description: #function) + func testSuccessfulAuth() async throws { setFetcherTestBlock(with: 200) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: true) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertEqual(headers!["Authorization"], "Firebase \(self.StorageTestAuthToken)") - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertEqual(headers!["Authorization"], "Firebase \(StorageTestAuthToken)") } - func testUnsuccessfulAuth() { - let expectation = self.expectation(description: #function) + func testUnsuccessfulAuth() async { let authError = NSError(domain: "FIRStorageErrorDomain", code: StorageErrorCode.unauthenticated.rawValue, userInfo: nil) let failedAuth = FIRAuthInteropFake(token: nil, userID: nil, error: authError) fetcher?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: failedAuth, appCheck: nil ) setFetcherTestBlock(with: 401) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: false) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertNil(headers) - let nsError = error as? NSError - XCTAssertEqual(nsError?.domain, "FIRStorageErrorDomain") - XCTAssertEqual(nsError?.code, StorageErrorCode.unauthenticated.rawValue) - XCTAssertEqual(nsError?.localizedDescription, "User is not authenticated, please " + + do { + let _ = try await fetcher?.beginFetch() + } catch { + let nsError = error as NSError + XCTAssertEqual(nsError.domain, "FIRStorageErrorDomain") + XCTAssertEqual(nsError.code, StorageErrorCode.unauthenticated.rawValue) + XCTAssertEqual(nsError.localizedDescription, "User is not authenticated, please " + "authenticate using Firebase Authentication and try again.") - expectation.fulfill() } - waitForExpectation(test: self) } - func testSuccessfulUnauthenticatedAuth() { - let expectation = self.expectation(description: #function) - + func testSuccessfulUnauthenticatedAuth() async throws { // Simulate Auth not being included at all fetcher?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: nil, appCheck: nil ) @@ -114,23 +102,17 @@ class StorageAuthorizerTests: StorageTestHelpers { setFetcherTestBlock(with: 200) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: false) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertNil(headers!["Authorization"]) - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertNil(headers!["Authorization"]) } - func testSuccessfulAppCheckNoAuth() { - let expectation = self.expectation(description: #function) + func testSuccessfulAppCheckNoAuth() async throws { appCheck?.tokenResult = appCheckTokenSuccess! // Simulate Auth not being included at all fetcher?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: nil, appCheck: appCheck ) @@ -138,52 +120,36 @@ class StorageAuthorizerTests: StorageTestHelpers { setFetcherTestBlock(with: 200) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: false) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertEqual(headers!["X-Firebase-AppCheck"], self.appCheckTokenSuccess?.token) - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertEqual(headers!["X-Firebase-AppCheck"], appCheckTokenSuccess?.token) } - func testSuccessfulAppCheckAndAuth() { - let expectation = self.expectation(description: #function) + func testSuccessfulAppCheckAndAuth() async throws { appCheck?.tokenResult = appCheckTokenSuccess! setFetcherTestBlock(with: 200) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: true) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertEqual(headers!["Authorization"], "Firebase \(self.StorageTestAuthToken)") - XCTAssertEqual(headers!["X-Firebase-AppCheck"], self.appCheckTokenSuccess?.token) - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertEqual(headers!["Authorization"], "Firebase \(StorageTestAuthToken)") + XCTAssertEqual(headers!["X-Firebase-AppCheck"], appCheckTokenSuccess?.token) } - func testAppCheckError() { - let expectation = self.expectation(description: #function) + func testAppCheckError() async throws { appCheck?.tokenResult = appCheckTokenError! setFetcherTestBlock(with: 200) { fetcher in self.checkAuthorizer(fetcher: fetcher, trueFalse: true) } - fetcher?.beginFetch { data, error in - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertEqual(headers!["Authorization"], "Firebase \(self.StorageTestAuthToken)") - XCTAssertEqual(headers!["X-Firebase-AppCheck"], self.appCheckTokenError?.token) - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertEqual(headers!["Authorization"], "Firebase \(StorageTestAuthToken)") + XCTAssertEqual(headers!["X-Firebase-AppCheck"], appCheckTokenError?.token) } - func testIsAuthorizing() { - let expectation = self.expectation(description: #function) - + func testIsAuthorizing() async throws { setFetcherTestBlock(with: 200) { fetcher in do { let authorizer = try XCTUnwrap(fetcher.authorizer) @@ -192,16 +158,10 @@ class StorageAuthorizerTests: StorageTestHelpers { XCTFail("Failed to get authorizer: \(error)") } } - fetcher?.beginFetch { data, error in - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() } - func testStopAuthorizingNoop() { - let expectation = self.expectation(description: #function) - + func testStopAuthorizingNoop() async throws { setFetcherTestBlock(with: 200) { fetcher in do { let authorizer = try XCTUnwrap(fetcher.authorizer) @@ -214,18 +174,12 @@ class StorageAuthorizerTests: StorageTestHelpers { XCTFail("Failed to get authorizer: \(error)") } } - fetcher?.beginFetch { data, error in - XCTAssertNil(error) - let headers = self.fetcher!.request?.allHTTPHeaderFields - XCTAssertEqual(headers!["Authorization"], "Firebase \(self.StorageTestAuthToken)") - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() + let headers = fetcher!.request?.allHTTPHeaderFields + XCTAssertEqual(headers!["Authorization"], "Firebase \(StorageTestAuthToken)") } - func testEmail() { - let expectation = self.expectation(description: #function) - + func testEmail() async throws { setFetcherTestBlock(with: 200) { fetcher in do { let authorizer = try XCTUnwrap(fetcher.authorizer) @@ -234,11 +188,7 @@ class StorageAuthorizerTests: StorageTestHelpers { XCTFail("Failed to get authorizer: \(error)") } } - fetcher?.beginFetch { data, error in - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectation(test: self) + let _ = try await fetcher?.beginFetch() } // MARK: Helpers diff --git a/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift b/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift index de20735f7e5..52484c17fca 100644 --- a/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageDeleteTests.swift @@ -27,7 +27,6 @@ class StorageDeleteTests: StorageTestHelpers { fetcherService = GTMSessionFetcherService() fetcherService?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: nil, appCheck: nil ) diff --git a/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift b/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift index d3415ac9fbe..af45994e939 100644 --- a/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift @@ -27,7 +27,6 @@ class StorageGetMetadataTests: StorageTestHelpers { fetcherService = GTMSessionFetcherService() fetcherService?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: nil, appCheck: nil ) diff --git a/FirebaseStorage/Tests/Unit/StorageListTests.swift b/FirebaseStorage/Tests/Unit/StorageListTests.swift index 9d29cf553dc..121a7954292 100644 --- a/FirebaseStorage/Tests/Unit/StorageListTests.swift +++ b/FirebaseStorage/Tests/Unit/StorageListTests.swift @@ -27,7 +27,6 @@ class StorageListTests: StorageTestHelpers { fetcherService = GTMSessionFetcherService() fetcherService?.authorizer = StorageTokenAuthorizer( googleAppID: "dummyAppID", - fetcherService: fetcherService!, authProvider: nil, appCheck: nil )