Skip to content

Commit

Permalink
[storage] Simplify callback implementation (#13413)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulb777 authored Jul 27, 2024
1 parent 4883ced commit af5efdb
Show file tree
Hide file tree
Showing 8 changed files with 51 additions and 116 deletions.
7 changes: 3 additions & 4 deletions FirebaseStorage/Sources/Internal/StorageTokenAuthorizer.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -101,19 +100,19 @@ 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?

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
}
Expand Down
22 changes: 6 additions & 16 deletions FirebaseStorage/Sources/Storage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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) }
Expand All @@ -334,7 +323,7 @@ import FirebaseCore
fetcherService?.allowLocalhostRequest = true
let authorizer = StorageTokenAuthorizer(
googleAppID: app.options.googleAppID,
fetcherService: fetcherService!,
callbackQueue: callbackQueue,
authProvider: auth,
appCheck: appCheck
)
Expand Down Expand Up @@ -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"]
Expand Down
3 changes: 1 addition & 2 deletions FirebaseStorage/Sources/StorageObservableTask.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
4 changes: 2 additions & 2 deletions FirebaseStorage/Sources/StorageReference.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down
128 changes: 39 additions & 89 deletions FirebaseStorage/Tests/Unit/StorageAuthorizerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -60,130 +59,97 @@ 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
)

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
)

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)
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion FirebaseStorage/Tests/Unit/StorageDeleteTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class StorageDeleteTests: StorageTestHelpers {
fetcherService = GTMSessionFetcherService()
fetcherService?.authorizer = StorageTokenAuthorizer(
googleAppID: "dummyAppID",
fetcherService: fetcherService!,
authProvider: nil,
appCheck: nil
)
Expand Down
1 change: 0 additions & 1 deletion FirebaseStorage/Tests/Unit/StorageGetMetadataTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class StorageGetMetadataTests: StorageTestHelpers {
fetcherService = GTMSessionFetcherService()
fetcherService?.authorizer = StorageTokenAuthorizer(
googleAppID: "dummyAppID",
fetcherService: fetcherService!,
authProvider: nil,
appCheck: nil
)
Expand Down
1 change: 0 additions & 1 deletion FirebaseStorage/Tests/Unit/StorageListTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ class StorageListTests: StorageTestHelpers {
fetcherService = GTMSessionFetcherService()
fetcherService?.authorizer = StorageTokenAuthorizer(
googleAppID: "dummyAppID",
fetcherService: fetcherService!,
authProvider: nil,
appCheck: nil
)
Expand Down

0 comments on commit af5efdb

Please sign in to comment.