From 8829f45c68dbb4e7ec58168e76ea5b6d29f5fe34 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 3 Jan 2024 16:52:17 -0800 Subject: [PATCH 1/3] [auth-swift] Fix heartbeats for SPM --- .../Core/FIRHeartbeatLogger+AppCheck.m | 3 +- .../Sources/Swift/Auth/AuthDataResult.swift | 8 +- .../Sources/Swift/Backend/AuthBackend.swift | 12 +- .../AuthBackendRPCImplentationTests.swift | 164 +++++++++--------- FirebaseCore/Extension/FIRHeartbeatLogger.h | 4 +- FirebaseCore/Sources/FIRHeartbeatLogger.m | 4 + .../FIRInstallationsAPIService.m | 3 +- .../FIRInstallationsIntegrationTests.m | 9 +- 8 files changed, 99 insertions(+), 108 deletions(-) diff --git a/FirebaseAppCheck/Sources/Core/FIRHeartbeatLogger+AppCheck.m b/FirebaseAppCheck/Sources/Core/FIRHeartbeatLogger+AppCheck.m index 78b0628f38a..6db6fd13094 100644 --- a/FirebaseAppCheck/Sources/Core/FIRHeartbeatLogger+AppCheck.m +++ b/FirebaseAppCheck/Sources/Core/FIRHeartbeatLogger+AppCheck.m @@ -23,8 +23,7 @@ @implementation FIRHeartbeatLogger (AppCheck) - (GACAppCheckAPIRequestHook)requestHook { return ^(NSMutableURLRequest *request) { - NSString *heartbeatsValue = - FIRHeaderValueFromHeartbeatsPayload([self flushHeartbeatsIntoPayload]); + NSString *heartbeatsValue = [self headerValue]; if (heartbeatsValue) { [request setValue:heartbeatsValue forHTTPHeaderField:kFIRHeartbeatLoggerPayloadHeaderKey]; } diff --git a/FirebaseAuth/Sources/Swift/Auth/AuthDataResult.swift b/FirebaseAuth/Sources/Swift/Auth/AuthDataResult.swift index 7636cae7aec..10c0946420a 100644 --- a/FirebaseAuth/Sources/Swift/Auth/AuthDataResult.swift +++ b/FirebaseAuth/Sources/Swift/Auth/AuthDataResult.swift @@ -41,17 +41,15 @@ import Foundation private let kUserCodingKey = "user" private let kCredentialCodingKey = "credential" - // TODO: All below here should be internal - /** @fn initWithUser:additionalUserInfo: @brief Designated initializer. @param user The signed in user reference. @param additionalUserInfo The additional user info. @param credential The updated OAuth credential if available. */ - @objc public init(withUser user: User, - additionalUserInfo: AdditionalUserInfo?, - credential: OAuthCredential? = nil) { + init(withUser user: User, + additionalUserInfo: AdditionalUserInfo?, + credential: OAuthCredential? = nil) { self.user = user self.additionalUserInfo = additionalUserInfo self.credential = credential diff --git a/FirebaseAuth/Sources/Swift/Backend/AuthBackend.swift b/FirebaseAuth/Sources/Swift/Backend/AuthBackend.swift index 481d80de518..b163dcb3022 100644 --- a/FirebaseAuth/Sources/Swift/Backend/AuthBackend.swift +++ b/FirebaseAuth/Sources/Swift/Backend/AuthBackend.swift @@ -112,15 +112,9 @@ class AuthBackend: NSObject { request.setValue(clientVersion, forHTTPHeaderField: "X-Client-Version") request.setValue(Bundle.main.bundleIdentifier, forHTTPHeaderField: "X-Ios-Bundle-Identifier") request.setValue(requestConfiguration.appID, forHTTPHeaderField: "X-Firebase-GMPID") - // TODO: Enable for SPM. Can we directly call the Swift library? - #if COCOAPODS - if let heartbeatLogger = requestConfiguration.heartbeatLogger { - request.setValue( - FIRHeaderValueFromHeartbeatsPayload(heartbeatLogger.flushHeartbeatsIntoPayload()), - forHTTPHeaderField: "X-Firebase-Client" - ) - } - #endif + if let heartbeatLogger = requestConfiguration.heartbeatLogger { + request.setValue(heartbeatLogger.headerValue(), forHTTPHeaderField: "X-Firebase-Client") + } request.httpMethod = requestConfiguration.httpMethod let preferredLocalizations = Bundle.main.preferredLocalizations if preferredLocalizations.count > 0 { diff --git a/FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift b/FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift index bbad8d424f1..dc43d57168b 100644 --- a/FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift +++ b/FirebaseAuth/Tests/Unit/AuthBackendRPCImplentationTests.swift @@ -529,66 +529,69 @@ class AuthBackendRPCImplementationTests: RPCBaseTests { XCTAssertEqual(try XCTUnwrap(rpcResponse.receivedDictionary[kTestKey] as? String), kTestValue) } - // TODO: enable heartbeat logger tests for SPM - #if COCOAPODS - private class FakeHeartbeatLogger: NSObject, FIRHeartbeatLoggerProtocol { - var onFlushHeartbeatsIntoPayloadHandler: (() -> _ObjC_HeartbeatsPayload)? - - func log() { - // This API should not be used by the below tests because the Auth - // SDK does not log heartbeats in it's networking context. - fatalError("FakeHeartbeatLogger log should not be used in tests.") + private class FakeHeartbeatLogger: NSObject, FIRHeartbeatLoggerProtocol { + func headerValue() -> String? { + let payload = flushHeartbeatsIntoPayload() + guard !payload.isEmpty else { + return nil } + return payload.headerValue() + } - func flushHeartbeatsIntoPayload() -> FirebaseCoreInternal._ObjC_HeartbeatsPayload { - guard let handler = onFlushHeartbeatsIntoPayloadHandler else { - fatalError("Missing Handler") - } - return handler() - } + var onFlushHeartbeatsIntoPayloadHandler: (() -> _ObjC_HeartbeatsPayload)? - func heartbeatCodeForToday() -> FIRDailyHeartbeatCode { - // This API should not be used by the below tests because the Auth - // SDK uses only the V2 heartbeat API (`flushHeartbeatsIntoPayload`) for - // getting heartbeats. - return FIRDailyHeartbeatCode.none - } + func log() { + // This API should not be used by the below tests because the Auth + // SDK does not log heartbeats in it's networking context. + fatalError("FakeHeartbeatLogger log should not be used in tests.") } - /** @fn testRequest_IncludesHeartbeatPayload_WhenHeartbeatsNeedSending - @brief This test checks the behavior of @c postWithRequest:response:callback: - to verify that a heartbeats payload is attached as a header to an - outgoing request when there are stored heartbeats that need sending. - */ - func testRequest_IncludesHeartbeatPayload_WhenHeartbeatsNeedSending() async throws { - // Given - let fakeHeartbeatLogger = FakeHeartbeatLogger() - let requestConfiguration = AuthRequestConfiguration(apiKey: kFakeAPIKey, - appID: kFakeAppID, - heartbeatLogger: fakeHeartbeatLogger) - - let request = FakeRequest(withRequestBody: [:], requestConfiguration: requestConfiguration) - - // When - let nonEmptyHeartbeatsPayload = HeartbeatLoggingTestUtils.nonEmptyHeartbeatsPayload - fakeHeartbeatLogger.onFlushHeartbeatsIntoPayloadHandler = { - nonEmptyHeartbeatsPayload + func flushHeartbeatsIntoPayload() -> FirebaseCoreInternal._ObjC_HeartbeatsPayload { + guard let handler = onFlushHeartbeatsIntoPayloadHandler else { + fatalError("Missing Handler") } - rpcIssuer.respondBlock = { - // Force return from async post - try self.rpcIssuer.respond(withJSON: [:]) - } - _ = try? await rpcImplementation.call(with: request) + return handler() + } - // Then - let expectedHeader = FIRHeaderValueFromHeartbeatsPayload( - HeartbeatLoggingTestUtils.nonEmptyHeartbeatsPayload - ) - let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest) - let headerValue = completeRequest.value(forHTTPHeaderField: "X-Firebase-Client") - XCTAssertEqual(headerValue, expectedHeader) + func heartbeatCodeForToday() -> FIRDailyHeartbeatCode { + // This API should not be used by the below tests because the Auth + // SDK uses only the V2 heartbeat API (`flushHeartbeatsIntoPayload`) for + // getting heartbeats. + return FIRDailyHeartbeatCode.none + } + } + + /** @fn testRequest_IncludesHeartbeatPayload_WhenHeartbeatsNeedSending + @brief This test checks the behavior of @c postWithRequest:response:callback: + to verify that a heartbeats payload is attached as a header to an + outgoing request when there are stored heartbeats that need sending. + */ + func testRequest_IncludesHeartbeatPayload_WhenHeartbeatsNeedSending() async throws { + // Given + let fakeHeartbeatLogger = FakeHeartbeatLogger() + let requestConfiguration = AuthRequestConfiguration(apiKey: kFakeAPIKey, + appID: kFakeAppID, + heartbeatLogger: fakeHeartbeatLogger) + + let request = FakeRequest(withRequestBody: [:], requestConfiguration: requestConfiguration) + + // When + let nonEmptyHeartbeatsPayload = HeartbeatLoggingTestUtils.nonEmptyHeartbeatsPayload + fakeHeartbeatLogger.onFlushHeartbeatsIntoPayloadHandler = { + nonEmptyHeartbeatsPayload + } + rpcIssuer.respondBlock = { + // Force return from async post + try self.rpcIssuer.respond(withJSON: [:]) } - #endif + _ = try? await rpcImplementation.call(with: request) + + // Then + let expectedHeader = HeartbeatLoggingTestUtils.nonEmptyHeartbeatsPayload.headerValue() + let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest) + let headerValue = completeRequest.value(forHTTPHeaderField: "X-Firebase-Client") + XCTAssertEqual(headerValue, expectedHeader) + } /** @fn testRequest_IncludesAppCheckHeader @brief This test checks the behavior of @c postWithRequest:response:callback: @@ -614,38 +617,35 @@ class AuthBackendRPCImplementationTests: RPCBaseTests { XCTAssertEqual(headerValue, fakeAppCheck.fakeAppCheckToken) } - // TODO: enable for SPM - #if COCOAPODS - /** @fn testRequest_DoesNotIncludeAHeartbeatPayload_WhenNoHeartbeatsNeedSending - @brief This test checks the behavior of @c postWithRequest:response:callback: - to verify that a request header does not contain heartbeat data in the - case that there are no stored heartbeats that need sending. - */ - func testRequest_DoesNotIncludeAHeartbeatPayload_WhenNoHeartbeatsNeedSending() async throws { - // Given - let fakeHeartbeatLogger = FakeHeartbeatLogger() - let requestConfiguration = AuthRequestConfiguration(apiKey: kFakeAPIKey, - appID: kFakeAppID, - heartbeatLogger: fakeHeartbeatLogger) - - let request = FakeRequest(withRequestBody: [:], requestConfiguration: requestConfiguration) - - // When - let emptyHeartbeatsPayload = HeartbeatLoggingTestUtils.emptyHeartbeatsPayload - fakeHeartbeatLogger.onFlushHeartbeatsIntoPayloadHandler = { - emptyHeartbeatsPayload - } - rpcIssuer.respondBlock = { - // Force return from async post - try self.rpcIssuer.respond(withJSON: [:]) - } - _ = try? await rpcImplementation.call(with: request) + /** @fn testRequest_DoesNotIncludeAHeartbeatPayload_WhenNoHeartbeatsNeedSending + @brief This test checks the behavior of @c postWithRequest:response:callback: + to verify that a request header does not contain heartbeat data in the + case that there are no stored heartbeats that need sending. + */ + func testRequest_DoesNotIncludeAHeartbeatPayload_WhenNoHeartbeatsNeedSending() async throws { + // Given + let fakeHeartbeatLogger = FakeHeartbeatLogger() + let requestConfiguration = AuthRequestConfiguration(apiKey: kFakeAPIKey, + appID: kFakeAppID, + heartbeatLogger: fakeHeartbeatLogger) + + let request = FakeRequest(withRequestBody: [:], requestConfiguration: requestConfiguration) - // Then - let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest) - XCTAssertNil(completeRequest.value(forHTTPHeaderField: "X-Firebase-Client")) + // When + let emptyHeartbeatsPayload = HeartbeatLoggingTestUtils.emptyHeartbeatsPayload + fakeHeartbeatLogger.onFlushHeartbeatsIntoPayloadHandler = { + emptyHeartbeatsPayload } - #endif + rpcIssuer.respondBlock = { + // Force return from async post + try self.rpcIssuer.respond(withJSON: [:]) + } + _ = try? await rpcImplementation.call(with: request) + + // Then + let completeRequest = try XCTUnwrap(rpcIssuer.completeRequest) + XCTAssertNil(completeRequest.value(forHTTPHeaderField: "X-Firebase-Client")) + } private class FakeRequest: AuthRPCRequest { typealias Response = FakeResponse diff --git a/FirebaseCore/Extension/FIRHeartbeatLogger.h b/FirebaseCore/Extension/FIRHeartbeatLogger.h index 0f39ad94817..6314f504f13 100644 --- a/FirebaseCore/Extension/FIRHeartbeatLogger.h +++ b/FirebaseCore/Extension/FIRHeartbeatLogger.h @@ -36,8 +36,8 @@ typedef NS_ENUM(NSInteger, FIRDailyHeartbeatCode) { - (void)log; #ifndef FIREBASE_BUILD_CMAKE -/// Flushes heartbeats from storage into a structured payload of heartbeats. -- (FIRHeartbeatsPayload *)flushHeartbeatsIntoPayload; +/// Return the headerValue for the HeartbeatLogger. +- (NSString *_Nullable)headerValue; #endif // FIREBASE_BUILD_CMAKE /// Gets the heartbeat code for today. diff --git a/FirebaseCore/Sources/FIRHeartbeatLogger.m b/FirebaseCore/Sources/FIRHeartbeatLogger.m index 5b0c309ee38..08cd396a99e 100644 --- a/FirebaseCore/Sources/FIRHeartbeatLogger.m +++ b/FirebaseCore/Sources/FIRHeartbeatLogger.m @@ -70,6 +70,10 @@ - (void)log { } #ifndef FIREBASE_BUILD_CMAKE +- (NSString *_Nullable)headerValue { + return FIRHeaderValueFromHeartbeatsPayload([self flushHeartbeatsIntoPayload]); +} + - (FIRHeartbeatsPayload *)flushHeartbeatsIntoPayload { FIRHeartbeatsPayload *payload = [_heartbeatController flush]; return payload; diff --git a/FirebaseInstallations/Source/Library/InstallationsAPI/FIRInstallationsAPIService.m b/FirebaseInstallations/Source/Library/InstallationsAPI/FIRInstallationsAPIService.m index 77408bbed8b..99dd21583ec 100644 --- a/FirebaseInstallations/Source/Library/InstallationsAPI/FIRInstallationsAPIService.m +++ b/FirebaseInstallations/Source/Library/InstallationsAPI/FIRInstallationsAPIService.m @@ -278,8 +278,7 @@ - (instancetype)initWithURLSession:(NSURLSession *)URLSession [request setValue:authHeader forHTTPHeaderField:@"Authorization"]; } // Heartbeat Header. - [request setValue:FIRHeaderValueFromHeartbeatsPayload( - [self.heartbeatLogger flushHeartbeatsIntoPayload]) + [request setValue:[self.heartbeatLogger headerValue] forHTTPHeaderField:kFIRInstallationsHeartbeatKey]; [additionalHeaders diff --git a/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m b/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m index 51552998b9f..076d011bdd2 100644 --- a/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m +++ b/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m @@ -95,8 +95,7 @@ - (void)testGetFID { // from the above Installations API call. [self addTeardownBlock:^{ FBLWaitForPromisesWithTimeout(20); - XCTAssertNil(FIRHeaderValueFromHeartbeatsPayload( - [FIRApp.defaultApp.heartbeatLogger flushHeartbeatsIntoPayload])); + XCTAssertNil(FIRHeaderValueFromHeartbeatsPayload([self.heartbeatLogger headerValue])); }]; } @@ -126,8 +125,7 @@ - (void)testAuthToken { // from the above Installations API call. [self addTeardownBlock:^{ FBLWaitForPromisesWithTimeout(20); - XCTAssertNil(FIRHeaderValueFromHeartbeatsPayload( - [FIRApp.defaultApp.heartbeatLogger flushHeartbeatsIntoPayload])); + XCTAssertNil([FIRApp.defaultApp.heartbeatLogger headerValue]); }]; } @@ -158,8 +156,7 @@ - (void)testDeleteInstallation { // from the above Installations API call. [self addTeardownBlock:^{ FBLWaitForPromisesWithTimeout(20); - XCTAssertNil(FIRHeaderValueFromHeartbeatsPayload( - [FIRApp.defaultApp.heartbeatLogger flushHeartbeatsIntoPayload])); + XCTAssertNil([FIRApp.defaultApp.heartbeatLogger headerValue]); }]; } From ab72260ada02cfcd12ef39e9fb3da531ec95cd87 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 3 Jan 2024 17:18:20 -0800 Subject: [PATCH 2/3] fix --- .../Source/Tests/Integration/FIRInstallationsIntegrationTests.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m b/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m index 076d011bdd2..0600602260a 100644 --- a/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m +++ b/FirebaseInstallations/Source/Tests/Integration/FIRInstallationsIntegrationTests.m @@ -95,7 +95,7 @@ - (void)testGetFID { // from the above Installations API call. [self addTeardownBlock:^{ FBLWaitForPromisesWithTimeout(20); - XCTAssertNil(FIRHeaderValueFromHeartbeatsPayload([self.heartbeatLogger headerValue])); + XCTAssertNil([FIRApp.defaultApp.heartbeatLogger headerValue]); }]; } From ca5ab97ab54a2d6c335fa547be3f1359d98634a7 Mon Sep 17 00:00:00 2001 From: Paul Beusterien Date: Wed, 3 Jan 2024 19:45:31 -0800 Subject: [PATCH 3/3] fix --- .../Source/Tests/Unit/FIRInstallationsAPIServiceTests.m | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsAPIServiceTests.m b/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsAPIServiceTests.m index fe8ae5137be..532a1515e03 100644 --- a/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsAPIServiceTests.m +++ b/FirebaseInstallations/Source/Tests/Unit/FIRInstallationsAPIServiceTests.m @@ -65,6 +65,10 @@ - (void)log { [self doesNotRecognizeSelector:_cmd]; } +- (NSString *_Nullable)headerValue { + return FIRHeaderValueFromHeartbeatsPayload([self flushHeartbeatsIntoPayload]); +} + @end #pragma mark - FIRInstallationsAPIService + Internal