From e2c454e5daf8623a6f863b6a0777fac5e34208ed Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Tue, 1 Jul 2025 11:09:01 -0400 Subject: [PATCH 1/3] [Functions] Remove completion-handler based internal logic --- FirebaseFunctions/Sources/Functions.swift | 24 --- FirebaseFunctions/Sources/HTTPSCallable.swift | 34 +--- .../Sources/Internal/FunctionsContext.swift | 52 ------ .../Tests/Unit/ContextProviderTests.swift | 161 ++++++------------ .../Tests/Unit/FunctionsAPITests.swift | 1 + .../Tests/Unit/FunctionsTests.swift | 34 ++-- 6 files changed, 75 insertions(+), 231 deletions(-) diff --git a/FirebaseFunctions/Sources/Functions.swift b/FirebaseFunctions/Sources/Functions.swift index 8f04368d8d2..a87c68324a8 100644 --- a/FirebaseFunctions/Sources/Functions.swift +++ b/FirebaseFunctions/Sources/Functions.swift @@ -438,30 +438,6 @@ enum FunctionsConstants { } } - func callFunction(at url: URL, - withObject data: Any?, - options: HTTPSCallableOptions?, - timeout: TimeInterval, - completion: @escaping @MainActor (Result) -> Void) { - // Get context first. - contextProvider.getContext(options: options) { context, error in - // Note: context is always non-nil since some checks could succeed, we're only failing if - // there's an error. - if let error { - DispatchQueue.main.async { - completion(.failure(error)) - } - } else { - self.callFunction(url: url, - withObject: data, - options: options, - timeout: timeout, - context: context, - completion: completion) - } - } - } - private func callFunction(url: URL, withObject data: Any?, options: HTTPSCallableOptions?, diff --git a/FirebaseFunctions/Sources/HTTPSCallable.swift b/FirebaseFunctions/Sources/HTTPSCallable.swift index c7ecdfa0814..b1d00830576 100644 --- a/FirebaseFunctions/Sources/HTTPSCallable.swift +++ b/FirebaseFunctions/Sources/HTTPSCallable.swift @@ -184,34 +184,12 @@ private extension HTTPSCallable { func call(_ data: sending Any? = nil, completion: @escaping @MainActor (HTTPSCallableResult?, Error?) -> Void) { let data = (data as? SendableWrapper)?.value ?? data - if #available(iOS 13, macCatalyst 13, macOS 10.15, tvOS 13, watchOS 7, *) { - Task { - do { - let result = try await call(data) - await completion(result, nil) - } catch { - await completion(nil, error) - } - } - } else { - // This isn’t expected to ever be called because Functions - // doesn’t officially support the older platforms. - functions.callFunction( - at: url, - withObject: data, - options: options, - timeout: timeoutInterval - ) { result in - switch result { - case let .success(callableResult): - DispatchQueue.main.async { - completion(callableResult, nil) - } - case let .failure(error): - DispatchQueue.main.async { - completion(nil, error) - } - } + Task { + do { + let result = try await call(data) + await completion(result, nil) + } catch { + await completion(nil, error) } } } diff --git a/FirebaseFunctions/Sources/Internal/FunctionsContext.swift b/FirebaseFunctions/Sources/Internal/FunctionsContext.swift index dea21a9eb5c..bacf989ed37 100644 --- a/FirebaseFunctions/Sources/Internal/FunctionsContext.swift +++ b/FirebaseFunctions/Sources/Internal/FunctionsContext.swift @@ -82,56 +82,4 @@ struct FunctionsContextProvider: Sendable { } } } - - func getContext(options: HTTPSCallableOptions? = nil, - _ completion: @escaping ((FunctionsContext, Error?) -> Void)) { - let dispatchGroup = DispatchGroup() - - var authToken: String? - var appCheckToken: String? - var error: Error? - var limitedUseAppCheckToken: String? - - if let auth { - dispatchGroup.enter() - - auth.getToken(forcingRefresh: false) { token, authError in - authToken = token - error = authError - dispatchGroup.leave() - } - } - - if let appCheck { - dispatchGroup.enter() - - if options?.requireLimitedUseAppCheckTokens == true { - // `getLimitedUseToken(completion:)` is an optional protocol method. - // If it’s not implemented, we still need to leave the dispatch group. - if let limitedUseTokenClosure = appCheck.getLimitedUseToken { - limitedUseTokenClosure { tokenResult in - // In the case of an error, the token will be the placeholder token. - limitedUseAppCheckToken = tokenResult.token - dispatchGroup.leave() - } - } else { - dispatchGroup.leave() - } - } else { - appCheck.getToken(forcingRefresh: false) { tokenResult in - // In the case of an error, the token will be the placeholder token. - appCheckToken = tokenResult.token - dispatchGroup.leave() - } - } - } - - dispatchGroup.notify(queue: .main) { - let context = FunctionsContext(authToken: authToken, - fcmToken: self.messaging?.fcmToken, - appCheckToken: appCheckToken, - limitedUseAppCheckToken: limitedUseAppCheckToken) - completion(context, error) - } - } } diff --git a/FirebaseFunctions/Tests/Unit/ContextProviderTests.swift b/FirebaseFunctions/Tests/Unit/ContextProviderTests.swift index b528f45ce90..c15ab097617 100644 --- a/FirebaseFunctions/Tests/Unit/ContextProviderTests.swift +++ b/FirebaseFunctions/Tests/Unit/ContextProviderTests.swift @@ -52,18 +52,12 @@ class ContextProviderTests: XCTestCase { XCTAssertEqual(context.fcmToken, messagingFake.fcmToken) } - func testContextWithAuth() { + func testContextWithAuth() async throws { let auth = FIRAuthInteropFake(token: "token", userID: "userID", error: nil) let provider = FunctionsContextProvider(auth: auth, messaging: messagingFake, appCheck: nil) - let expectation = expectation(description: "Context should have auth keys.") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertEqual(context.authToken, "token") - XCTAssertEqual(context.fcmToken, self.messagingFake.fcmToken) - XCTAssertNil(error) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: nil) + XCTAssertEqual(context.authToken, "token") + XCTAssertEqual(context.fcmToken, messagingFake.fcmToken) } func testAsyncContextWithAuthError() async { @@ -79,18 +73,16 @@ class ContextProviderTests: XCTestCase { } } - func testContextWithAuthError() { + func testContextWithAuthError() async throws { let authError = NSError(domain: "com.functions.tests", code: 4, userInfo: nil) let auth = FIRAuthInteropFake(token: nil, userID: "userID", error: authError) let provider = FunctionsContextProvider(auth: auth, messaging: messagingFake, appCheck: nil) - let expectation = expectation(description: "Completion handler should fail with Auth error.") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertNil(context.authToken) + do { + _ = try await provider.context(options: nil) + XCTFail("Expected an error") + } catch { XCTAssertEqual(error as NSError?, authError) - expectation.fulfill() } - waitForExpectations(timeout: 0.1) } func testAsyncContextWithoutAuth() async throws { @@ -102,17 +94,11 @@ class ContextProviderTests: XCTestCase { XCTAssertNil(context.fcmToken) } - func testContextWithoutAuth() { + func testContextWithoutAuth() async throws { let provider = FunctionsContextProvider(auth: nil, messaging: nil, appCheck: nil) - let expectation = expectation(description: "Completion handler should succeed without Auth.") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertNil(context.authToken) - XCTAssertNil(context.fcmToken) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: nil) + XCTAssertNil(context.authToken) + XCTAssertNil(context.fcmToken) } func testAsyncContextWithAppCheckOnlySuccess() async throws { @@ -126,19 +112,13 @@ class ContextProviderTests: XCTestCase { XCTAssertEqual(context.appCheckToken, appCheckTokenSuccess.token) } - func testContextWithAppCheckOnlySuccess() { + func testContextWithAppCheckOnlySuccess() async throws { appCheckFake.tokenResult = appCheckTokenSuccess let provider = FunctionsContextProvider(auth: nil, messaging: nil, appCheck: appCheckFake) - let expectation = expectation(description: "Verify app check.") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertNil(context.authToken) - XCTAssertNil(context.fcmToken) - XCTAssertEqual(context.appCheckToken, self.appCheckTokenSuccess.token) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: nil) + XCTAssertNil(context.authToken) + XCTAssertNil(context.fcmToken) + XCTAssertEqual(context.appCheckToken, appCheckTokenSuccess.token) } func testAsyncContextWithAppCheckOnlyError() async throws { @@ -165,36 +145,25 @@ class ContextProviderTests: XCTestCase { XCTAssertEqual(context.limitedUseAppCheckToken, appCheckFake.limitedUseTokenResult.token) } - func testContextWithAppCheckOnlyError() { + func testContextWithAppCheckOnlyError() async throws { appCheckFake.tokenResult = appCheckTokenError let provider = FunctionsContextProvider(auth: nil, messaging: nil, appCheck: appCheckFake) - let expectation = expectation(description: "Verify bad app check token") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertNil(context.authToken) - XCTAssertNil(context.fcmToken) - // Expect placeholder token in the case of App Check error. - XCTAssertEqual(context.appCheckToken, self.appCheckFake.tokenResult.token) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: nil) + XCTAssertNil(context.authToken) + XCTAssertNil(context.fcmToken) + // Expect placeholder token in the case of App Check error. + XCTAssertEqual(context.appCheckToken, appCheckFake.tokenResult.token) } - func testContextWithAppCheckOnlyError_LimitedUseToken() { + func testContextWithAppCheckOnlyError_LimitedUseToken() async throws { appCheckFake.limitedUseTokenResult = appCheckLimitedUseTokenError let provider = FunctionsContextProvider(auth: nil, messaging: nil, appCheck: appCheckFake) - let expectation = expectation(description: "Verify bad app check token") - provider.getContext(options: .init(requireLimitedUseAppCheckTokens: true)) { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertNil(context.authToken) - XCTAssertNil(context.fcmToken) - // Expect placeholder token in the case of App Check error. - XCTAssertEqual(context.limitedUseAppCheckToken, self.appCheckFake.limitedUseTokenResult.token) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider + .context(options: HTTPSCallableOptions(requireLimitedUseAppCheckTokens: true)) + XCTAssertNil(context.authToken) + XCTAssertNil(context.fcmToken) + // Expect placeholder token in the case of App Check error. + XCTAssertEqual(context.limitedUseAppCheckToken, appCheckFake.limitedUseTokenResult.token) } func testAsyncContextWithAppCheckWithoutOptionalMethods() async throws { @@ -210,23 +179,15 @@ class ContextProviderTests: XCTestCase { XCTAssertNil(context.limitedUseAppCheckToken) } - func testContextWithAppCheckWithoutOptionalMethods() { + func testContextWithAppCheckWithoutOptionalMethods() async throws { let appCheck = AppCheckFakeWithoutOptionalMethods(tokenResult: appCheckTokenSuccess) let provider = FunctionsContextProvider(auth: nil, messaging: nil, appCheck: appCheck) - let expectation = - expectation(description: "Verify non-implemented method for limited-use tokens") - provider.getContext(options: .init(requireLimitedUseAppCheckTokens: true)) { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertNil(context.authToken) - XCTAssertNil(context.fcmToken) - XCTAssertNil(context.appCheckToken) - // If the method for limited-use tokens is not implemented, the value should be `nil`: - XCTAssertNil(context.limitedUseAppCheckToken) - expectation.fulfill() - } - // Importantly, `getContext(options:_:)` must still finish in a timely manner: - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: .init(requireLimitedUseAppCheckTokens: true)) + XCTAssertNil(context.authToken) + XCTAssertNil(context.fcmToken) + XCTAssertNil(context.appCheckToken) + // If the method for limited-use tokens is not implemented, the value should be `nil`: + XCTAssertNil(context.limitedUseAppCheckToken) } func testAsyncAllContextsAvailableSuccess() async throws { @@ -245,7 +206,7 @@ class ContextProviderTests: XCTestCase { XCTAssertEqual(context.appCheckToken, appCheckTokenSuccess.token) } - func testAllContextsAvailableSuccess() { + func testAllContextsAvailableSuccess() async throws { appCheckFake.tokenResult = appCheckTokenSuccess let auth = FIRAuthInteropFake(token: "token", userID: "userID", error: nil) let provider = FunctionsContextProvider( @@ -253,16 +214,10 @@ class ContextProviderTests: XCTestCase { messaging: messagingFake, appCheck: appCheckFake ) - let expectation = expectation(description: "All contexts available") - provider.getContext { context, error in - XCTAssertNotNil(context) - XCTAssertNil(error) - XCTAssertEqual(context.authToken, "token") - XCTAssertEqual(context.fcmToken, self.messagingFake.fcmToken) - XCTAssertEqual(context.appCheckToken, self.appCheckTokenSuccess.token) - expectation.fulfill() - } - waitForExpectations(timeout: 0.1) + let context = try await provider.context(options: nil) + XCTAssertEqual(context.authToken, "token") + XCTAssertEqual(context.fcmToken, messagingFake.fcmToken) + XCTAssertEqual(context.appCheckToken, appCheckTokenSuccess.token) } func testAsyncAllContextsAuthAndAppCheckError() async { @@ -283,7 +238,7 @@ class ContextProviderTests: XCTestCase { } } - func testAllContextsAuthAndAppCheckError() { + func testAllContextsAuthAndAppCheckError() async throws { appCheckFake.tokenResult = appCheckTokenError let authError = NSError(domain: "com.functions.tests", code: 4, userInfo: nil) let auth = FIRAuthInteropFake(token: nil, userID: "userID", error: authError) @@ -292,20 +247,15 @@ class ContextProviderTests: XCTestCase { messaging: messagingFake, appCheck: appCheckFake ) - let expectation = expectation(description: "All contexts with errors") - provider.getContext { context, error in - XCTAssertNotNil(context) + do { + _ = try await provider.context(options: nil) + XCTFail("Expected an error") + } catch { XCTAssertEqual(error as NSError?, authError) - XCTAssertNil(context.authToken) - XCTAssertEqual(context.fcmToken, self.messagingFake.fcmToken) - // Expect placeholder token in the case of App Check error. - XCTAssertEqual(context.appCheckToken, self.appCheckFake.tokenResult.token) - expectation.fulfill() } - waitForExpectations(timeout: 0.1) } - func testAllContextsAuthAndAppCheckError_LimitedUseToken() { + func testAllContextsAuthAndAppCheckError_LimitedUseToken() async throws { appCheckFake.limitedUseTokenResult = appCheckLimitedUseTokenError let authError = NSError(domain: "com.functions.tests", code: 4, userInfo: nil) let auth = FIRAuthInteropFake(token: nil, userID: "userID", error: authError) @@ -314,17 +264,12 @@ class ContextProviderTests: XCTestCase { messaging: messagingFake, appCheck: appCheckFake ) - let expectation = expectation(description: "All contexts with errors") - provider.getContext(options: .init(requireLimitedUseAppCheckTokens: true)) { context, error in - XCTAssertNotNil(context) + do { + _ = try await provider.context(options: .init(requireLimitedUseAppCheckTokens: true)) + XCTFail("Expected an error") + } catch { XCTAssertEqual(error as NSError?, authError) - XCTAssertNil(context.authToken) - XCTAssertEqual(context.fcmToken, self.messagingFake.fcmToken) - // Expect placeholder token in the case of App Check error. - XCTAssertEqual(context.limitedUseAppCheckToken, self.appCheckFake.limitedUseTokenResult.token) - expectation.fulfill() } - waitForExpectations(timeout: 0.1) } } diff --git a/FirebaseFunctions/Tests/Unit/FunctionsAPITests.swift b/FirebaseFunctions/Tests/Unit/FunctionsAPITests.swift index cde13386136..6ab0fb11a81 100644 --- a/FirebaseFunctions/Tests/Unit/FunctionsAPITests.swift +++ b/FirebaseFunctions/Tests/Unit/FunctionsAPITests.swift @@ -89,6 +89,7 @@ final class FunctionsAPITests: XCTestCase { // async/await is a Swift Concurrency feature available on iOS 13+ and macOS 10.15+ Task { do { + let data: Any? = nil let result = try await callableRef.call(data) _ = result.data } catch { diff --git a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift index 255b2785451..5f71d8855b0 100644 --- a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift +++ b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift @@ -134,7 +134,7 @@ class FunctionsTests: XCTestCase { // MARK: - App Check Integration - func testCallFunctionWhenUsingLimitedUseAppCheckTokenThenTokenSuccess() { + @MainActor func testCallFunctionWhenUsingLimitedUseAppCheckTokenThenTokenSuccess() { // Given // Stub returns of two different kinds of App Check tokens. Only the // limited use token should be present in Functions's request header. @@ -174,7 +174,7 @@ class FunctionsTests: XCTestCase { waitForExpectations(timeout: 1.5) } - func testCallFunctionWhenLimitedUseAppCheckTokenDisabledThenCallWithoutToken() { + @MainActor func testCallFunctionWhenLimitedUseAppCheckTokenDisabledThenCallWithoutToken() { // Given let limitedUseDummyToken = "limited use dummy token" appCheckFake.limitedUseTokenResult = FIRAppCheckTokenResultFake( @@ -215,7 +215,7 @@ class FunctionsTests: XCTestCase { waitForExpectations(timeout: 1.5) } - func testCallFunctionWhenLimitedUseAppCheckTokenCannotBeGeneratedThenCallWithPlaceholderToken() { + @MainActor func testCallFunctionWhenLimitedUseAppCheckTokenCannotBeGeneratedThenCallWithPlaceholderToken() { // Given appCheckFake.limitedUseTokenResult = FIRAppCheckTokenResultFake( token: "limited use token", @@ -257,7 +257,7 @@ class FunctionsTests: XCTestCase { waitForExpectations(timeout: 1.5) } - func testCallFunctionWhenAppCheckIsInstalledAndFACTokenSuccess() { + @MainActor func testCallFunctionWhenAppCheckIsInstalledAndFACTokenSuccess() { // Stub returns of two different kinds of App Check tokens. Only the // shared use token should be present in Functions's request header. appCheckFake.tokenResult = FIRAppCheckTokenResultFake(token: "shared_valid_token", error: nil) @@ -329,7 +329,7 @@ class FunctionsTests: XCTestCase { await fulfillment(of: [httpRequestExpectation], timeout: 1.5) } - func testCallFunctionWhenAppCheckIsNotInstalled() { + func testCallFunctionWhenAppCheckIsNotInstalled() async { let networkError = NSError( domain: "testCallFunctionWhenAppCheckIsInstalled", code: -1, @@ -346,20 +346,16 @@ class FunctionsTests: XCTestCase { } let completionExpectation = expectation(description: "completionExpectation") - functionsCustomDomain?.callFunction( - at: URL(string: "https://example.com/fake_func")!, - withObject: nil, - options: nil, - timeout: 10 - ) { result in - switch result { - case .success: - XCTFail("Unexpected success from functions?.callFunction") - case let .failure(error as NSError): - XCTAssertEqual(error, networkError) - } - completionExpectation.fulfill() + do { + _ = try await functionsCustomDomain?.callFunction( + at: URL(string: "https://example.com/fake_func")!, + withObject: nil, + options: nil, + timeout: 10 + ) + XCTFail("Unexpected success from functions?.callFunction") + } catch { + XCTAssertEqual(error as NSError, networkError) } - waitForExpectations(timeout: 1.5) } } From 83e71d92a64a6f8eac1f4e318a9aa1d0166f064e Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Tue, 1 Jul 2025 11:28:05 -0400 Subject: [PATCH 2/3] CI fixes --- .../CombineUnit/HTTPSCallableTests.swift | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift b/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift index 5db8b80fd77..c01521fb2e5 100644 --- a/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift +++ b/FirebaseFunctions/Tests/CombineUnit/HTTPSCallableTests.swift @@ -40,25 +40,6 @@ class MockFunctions: Functions, @unchecked Sendable { return try mockCallFunction() } - override func callFunction(at url: URL, - withObject data: Any?, - options: HTTPSCallableOptions?, - timeout: TimeInterval, - completion: @escaping @MainActor - (Result) -> Void) { - do { - try verifyParameters?(url, data, timeout) - let result = try mockCallFunction() - DispatchQueue.main.async { - completion(.success(result)) - } - } catch { - DispatchQueue.main.async { - completion(.failure(error)) - } - } - } - init(mockCallFunction: @escaping () throws -> HTTPSCallableResult) { self.mockCallFunction = mockCallFunction super.init( From c5a1dedf618d94994e7cad82667d05d3ac5773a6 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Tue, 1 Jul 2025 11:33:51 -0400 Subject: [PATCH 3/3] Fix CI (2) --- FirebaseFunctions/Tests/Unit/FunctionsTests.swift | 3 --- 1 file changed, 3 deletions(-) diff --git a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift index 5f71d8855b0..e44eb1414b6 100644 --- a/FirebaseFunctions/Tests/Unit/FunctionsTests.swift +++ b/FirebaseFunctions/Tests/Unit/FunctionsTests.swift @@ -336,16 +336,13 @@ class FunctionsTests: XCTestCase { userInfo: nil ) - let httpRequestExpectation = expectation(description: "HTTPRequestExpectation") fetcherService.testBlock = { fetcherToTest, testResponse in let appCheckTokenHeader = fetcherToTest.request? .value(forHTTPHeaderField: "X-Firebase-AppCheck") XCTAssertNil(appCheckTokenHeader) testResponse(nil, nil, networkError) - httpRequestExpectation.fulfill() } - let completionExpectation = expectation(description: "completionExpectation") do { _ = try await functionsCustomDomain?.callFunction( at: URL(string: "https://example.com/fake_func")!,