From 2cc53c64ac1369681c153a72998eadb78260cb2e Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Wed, 19 Mar 2025 10:36:26 -0400 Subject: [PATCH 01/10] [Swift 6] Add Swift 6 testing for Sessions --- .../FirebaseSessionsTests+BaseBehaviors.swift | 2 +- .../Tests/Unit/InitiatorTests.swift | 2 +- .../Unit/Library/LifecycleNotifications.swift | 26 ++----------------- 3 files changed, 4 insertions(+), 26 deletions(-) diff --git a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift index 2c453a4518a..c1b02cc38aa 100644 --- a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift +++ b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift @@ -87,7 +87,7 @@ final class FirebaseSessionsTestsBase_BaseBehaviors: FirebaseSessionsTestsBase { // We wanted to make sure that since we've introduced promises, // once the promise has been fulfilled, that .then'ing on the promise // in future initiations still results in a log - func test_multipleInitiations_logsSessionEventEachInitiation() { + @MainActor func test_multipleInitiations_logsSessionEventEachInitiation() { var loggedCount = 0 var lastLoggedSessionID = "" let loggedTwiceExpectation = expectation(description: "Sessions SDK logged events twice") diff --git a/FirebaseSessions/Tests/Unit/InitiatorTests.swift b/FirebaseSessions/Tests/Unit/InitiatorTests.swift index c3b17fb3dde..ebb6be2b5bc 100644 --- a/FirebaseSessions/Tests/Unit/InitiatorTests.swift +++ b/FirebaseSessions/Tests/Unit/InitiatorTests.swift @@ -58,7 +58,7 @@ class InitiatorTests: XCTestCase { XCTAssert(initiateCalled) } - func test_appForegrounded_initiatesNewSession() throws { + @MainActor func test_appForegrounded_initiatesNewSession() throws { // Given var pausedClock = date let initiator = SessionInitiator( diff --git a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift index 80c6f7c38f9..e2ea9d54e9d 100644 --- a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift +++ b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift @@ -37,18 +37,7 @@ import Dispatch #endif // swift(>=5.9) extension XCTestCase { - func postBackgroundedNotification() { - // On Catalyst, the notifications can only be called on a the main thread - if Thread.isMainThread { - postBackgroundedNotificationInternal() - } else { - DispatchQueue.main.sync { - self.postBackgroundedNotificationInternal() - } - } - } - - private func postBackgroundedNotificationInternal() { + @MainActor func postBackgroundedNotification() { let notificationCenter = NotificationCenter.default #if os(iOS) || os(tvOS) notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) @@ -74,18 +63,7 @@ extension XCTestCase { #endif // swift(>=5.9) } - func postForegroundedNotification() { - // On Catalyst, the notifications can only be called on a the main thread - if Thread.isMainThread { - postForegroundedNotificationInternal() - } else { - DispatchQueue.main.sync { - self.postForegroundedNotificationInternal() - } - } - } - - private func postForegroundedNotificationInternal() { + @MainActor func postForegroundedNotification() { let notificationCenter = NotificationCenter.default #if os(iOS) || os(tvOS) notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) From 5fd7a3100a288cd64bd2f17f1293f9fad25fdb6b Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Wed, 19 Mar 2025 10:41:09 -0400 Subject: [PATCH 02/10] [Swift 6] Add regression tests in CI --- .github/workflows/sessions.yml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.github/workflows/sessions.yml b/.github/workflows/sessions.yml index e30510d6fdd..668a1b674db 100644 --- a/.github/workflows/sessions.yml +++ b/.github/workflows/sessions.yml @@ -28,10 +28,17 @@ jobs: - os: macos-14 xcode: Xcode_15.3 tests: + swift_version: 5.9 # Flaky tests on CI - os: macos-15 xcode: Xcode_16.2 tests: --skip-tests + swift_version: 5.9 + # Flaky tests on CI + - os: macos-15 + xcode: Xcode_16.2 + tests: --skip-tests + swift_version: 6.0 runs-on: ${{ matrix.build-env.os }} steps: - uses: actions/checkout@v4 @@ -40,6 +47,8 @@ jobs: run: scripts/setup_bundler.sh - name: Xcode run: sudo xcode-select -s /Applications/${{ matrix.build-env.xcode }}.app/Contents/Developer + - name: Set Swift swift_version + run: sed -i "" "s/s.swift_version[[:space:]]*=[[:space:]]*'5.9'/s.swift_version = '${{ matrix.build-env.swift_version }}'/" FirebaseSessions.podspec - uses: nick-fields/retry@v3 with: timeout_minutes: 120 From cf7155e106eba9c037e2cf1b50ddc4f20cd405f3 Mon Sep 17 00:00:00 2001 From: Morgan Chen Date: Wed, 14 May 2025 05:57:09 -0400 Subject: [PATCH 03/10] Morgan's work from mc/messaging --- .../Sources/ApplicationInfo.swift | 9 +- .../Sources/FirebaseSessions.swift | 29 ++++- .../Sources/FirebaseSessionsError.swift | 2 +- .../Installations+InstallationsProtocol.swift | 2 +- FirebaseSessions/Sources/NetworkInfo.swift | 4 +- .../Sources/Public/SessionsDependencies.swift | 10 +- .../Sources/Public/SessionsSubscriber.swift | 2 +- .../Sources/SessionCoordinator.swift | 8 +- .../Sources/SessionInitiator.swift | 6 +- .../Sources/Settings/RemoteSettings.swift | 64 ++++++---- .../Settings/SettingsCacheClient.swift | 5 +- .../Settings/SettingsDownloadClient.swift | 10 +- .../FirebaseSessionsTests+BaseBehaviors.swift | 4 +- ...FirebaseSessionsTests+DataCollection.swift | 8 +- .../FirebaseSessionsTests+Subscribers.swift | 10 +- .../Tests/Unit/InitiatorTests.swift | 2 +- .../Library/FirebaseSessionsTestsBase.swift | 23 ++-- .../Unit/Library/LifecycleNotifications.swift | 112 +++++++++++------- .../Unit/Mocks/MockApplicationInfo.swift | 2 +- .../Mocks/MockInstallationsProtocol.swift | 2 +- .../Tests/Unit/Mocks/MockNetworkInfo.swift | 2 +- .../Unit/Mocks/MockSessionCoordinator.swift | 2 +- .../Unit/Mocks/MockSettingsDownloader.swift | 2 +- .../Tests/Unit/Mocks/MockSubscriber.swift | 2 +- .../Tests/Unit/SessionGeneratorTests.swift | 1 + 25 files changed, 193 insertions(+), 130 deletions(-) diff --git a/FirebaseSessions/Sources/ApplicationInfo.swift b/FirebaseSessions/Sources/ApplicationInfo.swift index b298bd5550a..85c42a9ef9d 100644 --- a/FirebaseSessions/Sources/ApplicationInfo.swift +++ b/FirebaseSessions/Sources/ApplicationInfo.swift @@ -34,7 +34,7 @@ enum DevEnvironment: String { case autopush // Autopush environment } -protocol ApplicationInfoProtocol { +protocol ApplicationInfoProtocol: Sendable { /// Google App ID / GMP App ID var appID: String { get } @@ -62,12 +62,15 @@ protocol ApplicationInfoProtocol { var osDisplayVersion: String { get } } -class ApplicationInfo: ApplicationInfoProtocol { +final class ApplicationInfo: ApplicationInfoProtocol { let appID: String private let networkInformation: NetworkInfoProtocol private let envParams: [String: String] - private let infoDict: [String: Any]? + + // Used to hold bundle info, so the `Any` params should also + // be Sendable. + private nonisolated(unsafe) let infoDict: [String: Any]? init(appID: String, networkInfo: NetworkInfoProtocol = NetworkInfo(), envParams: [String: String] = ProcessInfo.processInfo.environment, diff --git a/FirebaseSessions/Sources/FirebaseSessions.swift b/FirebaseSessions/Sources/FirebaseSessions.swift index be057cfc50f..31550ae4872 100644 --- a/FirebaseSessions/Sources/FirebaseSessions.swift +++ b/FirebaseSessions/Sources/FirebaseSessions.swift @@ -135,10 +135,10 @@ private enum GoogleDataTransportConfig { } // Initializes the SDK and begins the process of listening for lifecycle events and logging - // events + // events. `logEventCallback` is invoked on a global background queue. init(appID: String, sessionGenerator: SessionGenerator, coordinator: SessionCoordinatorProtocol, initiator: SessionInitiator, appInfo: ApplicationInfoProtocol, settings: SettingsProtocol, - loggedEventCallback: @escaping (Result) -> Void) { + loggedEventCallback: @escaping @Sendable (Result) -> Void) { self.appID = appID self.sessionGenerator = sessionGenerator @@ -247,18 +247,39 @@ private enum GoogleDataTransportConfig { return SessionDetails(sessionId: sessionGenerator.currentSession?.sessionId) } + // This type is not actually sendable, but works around an issue below. + // It's safe only if executed on the main actor. + private struct MainActorNotificationCallback: @unchecked Sendable { + private let callback: (Notification) -> Void + + init(_ callback: @escaping (Notification) -> Void) { + self.callback = callback + } + + func invoke(notification: Notification) { + dispatchPrecondition(condition: .onQueue(.main)) + callback(notification) + } + } + func register(subscriber: SessionsSubscriber) { Logger .logDebug( "Registering Sessions SDK subscriber with name: \(subscriber.sessionsSubscriberName), data collection enabled: \(subscriber.isDataCollectionEnabled)" ) + // After bumping to iOS 13, this hack should be replaced with `Task { @MainActor in }` + let callback = MainActorNotificationCallback { notification in + subscriber.onSessionChanged(self.currentSessionDetails) + } + + // Guaranteed to execute its callback on the main queue because of the queue parameter. notificationCenter.addObserver( forName: Sessions.SessionIDChangedNotificationName, object: nil, - queue: nil + queue: OperationQueue.main ) { notification in - subscriber.onSessionChanged(self.currentSessionDetails) + callback.invoke(notification: notification) } // Immediately call the callback because the Sessions SDK starts // before subscribers, so subscribers will miss the first Notification diff --git a/FirebaseSessions/Sources/FirebaseSessionsError.swift b/FirebaseSessions/Sources/FirebaseSessionsError.swift index 12ed1fb139b..6bfae66b56d 100644 --- a/FirebaseSessions/Sources/FirebaseSessionsError.swift +++ b/FirebaseSessions/Sources/FirebaseSessionsError.swift @@ -15,7 +15,7 @@ import Foundation /// Contains the list of errors that are localized for Firebase Sessions Library -enum FirebaseSessionsError: Error { +enum FirebaseSessionsError: Error, Sendable { /// Event sampling related error case SessionSamplingError /// Firebase Installation ID related error diff --git a/FirebaseSessions/Sources/Installations+InstallationsProtocol.swift b/FirebaseSessions/Sources/Installations+InstallationsProtocol.swift index 04f30b3ad75..98f54771411 100644 --- a/FirebaseSessions/Sources/Installations+InstallationsProtocol.swift +++ b/FirebaseSessions/Sources/Installations+InstallationsProtocol.swift @@ -17,7 +17,7 @@ import Foundation internal import FirebaseInstallations -protocol InstallationsProtocol { +protocol InstallationsProtocol: Sendable { var installationsWaitTimeInSecond: Int { get } /// Override Installation function for testing diff --git a/FirebaseSessions/Sources/NetworkInfo.swift b/FirebaseSessions/Sources/NetworkInfo.swift index a197f3d75ff..8d7e4110571 100644 --- a/FirebaseSessions/Sources/NetworkInfo.swift +++ b/FirebaseSessions/Sources/NetworkInfo.swift @@ -25,13 +25,13 @@ import Foundation internal import GoogleUtilities #endif // SWIFT_PACKAGE -protocol NetworkInfoProtocol { +protocol NetworkInfoProtocol: Sendable { var networkType: GULNetworkType { get } var mobileSubtype: String { get } } -class NetworkInfo: NetworkInfoProtocol { +final class NetworkInfo: NetworkInfoProtocol { var networkType: GULNetworkType { return GULNetworkInfo.getNetworkType() } diff --git a/FirebaseSessions/Sources/Public/SessionsDependencies.swift b/FirebaseSessions/Sources/Public/SessionsDependencies.swift index 4f366df9ba4..e929fff98d4 100644 --- a/FirebaseSessions/Sources/Public/SessionsDependencies.swift +++ b/FirebaseSessions/Sources/Public/SessionsDependencies.swift @@ -46,16 +46,10 @@ private final class AtomicBox { /// dependent SDKs @objc(FIRSessionsDependencies) public class SessionsDependencies: NSObject { - #if compiler(>=6) - private nonisolated(unsafe) static let _dependencies: AtomicBox> = - AtomicBox( - Set() - ) - #else - private static let _dependencies: AtomicBox> = AtomicBox( + private nonisolated(unsafe) static let _dependencies: AtomicBox> = + AtomicBox( Set() ) - #endif static var dependencies: Set { _dependencies.value() diff --git a/FirebaseSessions/Sources/Public/SessionsSubscriber.swift b/FirebaseSessions/Sources/Public/SessionsSubscriber.swift index 54b1b3fcba4..6d053b83f5c 100644 --- a/FirebaseSessions/Sources/Public/SessionsSubscriber.swift +++ b/FirebaseSessions/Sources/Public/SessionsSubscriber.swift @@ -18,7 +18,7 @@ import Foundation /// Sessions Subscriber is an interface that dependent SDKs /// must implement. @objc(FIRSessionsSubscriber) -public protocol SessionsSubscriber { +public protocol SessionsSubscriber: Sendable { func onSessionChanged(_ session: SessionDetails) var isDataCollectionEnabled: Bool { get } var sessionsSubscriberName: SessionsSubscriberName { get } diff --git a/FirebaseSessions/Sources/SessionCoordinator.swift b/FirebaseSessions/Sources/SessionCoordinator.swift index 3d4cec1b072..5811f9c7768 100644 --- a/FirebaseSessions/Sources/SessionCoordinator.swift +++ b/FirebaseSessions/Sources/SessionCoordinator.swift @@ -14,7 +14,7 @@ import Foundation -protocol SessionCoordinatorProtocol { +protocol SessionCoordinatorProtocol: Sendable { func attemptLoggingSessionStart(event: SessionStartEvent, callback: @escaping (Result) -> Void) } @@ -23,9 +23,11 @@ protocol SessionCoordinatorProtocol { /// SessionCoordinator is responsible for coordinating the systems in this SDK /// involved with sending a Session Start event. /// -class SessionCoordinator: SessionCoordinatorProtocol { +final class SessionCoordinator: SessionCoordinatorProtocol { let installations: InstallationsProtocol - let fireLogger: EventGDTLoggerProtocol + + // TODO: Make this type sendable + nonisolated(unsafe) let fireLogger: EventGDTLoggerProtocol init(installations: InstallationsProtocol, fireLogger: EventGDTLoggerProtocol) { diff --git a/FirebaseSessions/Sources/SessionInitiator.swift b/FirebaseSessions/Sources/SessionInitiator.swift index 77cd8eeda50..4a58fa9cea8 100644 --- a/FirebaseSessions/Sources/SessionInitiator.swift +++ b/FirebaseSessions/Sources/SessionInitiator.swift @@ -41,9 +41,9 @@ import Foundation /// class SessionInitiator { let currentTime: () -> Date - var settings: SettingsProtocol - var backgroundTime = Date.distantFuture - var initiateSessionStart: () -> Void = {} + let settings: SettingsProtocol + private var backgroundTime = Date.distantFuture + private var initiateSessionStart: () -> Void = {} init(settings: SettingsProtocol, currentTimeProvider: @escaping () -> Date = Date.init) { currentTime = currentTimeProvider diff --git a/FirebaseSessions/Sources/Settings/RemoteSettings.swift b/FirebaseSessions/Sources/Settings/RemoteSettings.swift index 15dffb18e43..08ce367bdeb 100644 --- a/FirebaseSessions/Sources/Settings/RemoteSettings.swift +++ b/FirebaseSessions/Sources/Settings/RemoteSettings.swift @@ -14,6 +14,7 @@ // limitations under the License. import Foundation +internal import FirebaseCoreInternal /// Extends ApplicationInfoProtocol to string-format a combined appDisplayVersion and /// appBuildVersion @@ -21,7 +22,7 @@ extension ApplicationInfoProtocol { var synthesizedVersion: String { return "\(appDisplayVersion) (\(appBuildVersion))" } } -class RemoteSettings: SettingsProvider { +final class RemoteSettings: SettingsProvider, Sendable { private static let cacheDurationSecondsDefault: TimeInterval = 60 * 60 private static let flagSessionsEnabled = "sessions_enabled" private static let flagSamplingRate = "sampling_rate" @@ -30,47 +31,58 @@ class RemoteSettings: SettingsProvider { private static let flagSessionsCache = "app_quality" private let appInfo: ApplicationInfoProtocol private let downloader: SettingsDownloadClient - private var cache: SettingsCacheClient + private let cache: FIRAllocatedUnfairLock - private var cacheDurationSeconds: TimeInterval { + private func cacheDuration(_ cache: SettingsCacheClient) -> TimeInterval { guard let duration = cache.cacheContent[RemoteSettings.flagCacheDuration] as? Double else { return RemoteSettings.cacheDurationSecondsDefault } + print("Duration: \(duration)") return duration } private var sessionsCache: [String: Any] { - return cache.cacheContent[RemoteSettings.flagSessionsCache] as? [String: Any] ?? [:] + cache.withLock { cache in + cache.cacheContent[RemoteSettings.flagSessionsCache] as? [String: Any] ?? [:] + } } init(appInfo: ApplicationInfoProtocol, downloader: SettingsDownloadClient, cache: SettingsCacheClient = SettingsCache()) { self.appInfo = appInfo - self.cache = cache + self.cache = FIRAllocatedUnfairLock(initialState: cache) self.downloader = downloader } private func fetchAndCacheSettings(currentTime: Date) { - // Only fetch if cache is expired, otherwise do nothing - guard isCacheExpired(time: currentTime) else { - Logger.logDebug("[Settings] Cache is not expired, no fetch will be made.") - return + let shouldFetch = cache.withLock { cache in + // Only fetch if cache is expired, otherwise do nothing + guard isCacheExpired(cache, time: currentTime) else { + Logger.logDebug("[Settings] Cache is not expired, no fetch will be made.") + return false + } + return true } - downloader.fetch { result in - switch result { - case let .success(dictionary): - // Saves all newly fetched Settings to cache - self.cache.cacheContent = dictionary - // Saves a "cache-key" which carries TTL metadata about current cache - self.cache.cacheKey = CacheKey( - createdAt: currentTime, - googleAppID: self.appInfo.appID, - appVersion: self.appInfo.synthesizedVersion - ) - case let .failure(error): - Logger.logError("[Settings] Fetching newest settings failed with error: \(error)") + if shouldFetch { + downloader.fetch { result in + + switch result { + case let .success(dictionary): + self.cache.withLock { cache in + // Saves all newly fetched Settings to cache + cache.cacheContent = dictionary + // Saves a "cache-key" which carries TTL metadata about current cache + cache.cacheKey = CacheKey( + createdAt: currentTime, + googleAppID: self.appInfo.appID, + appVersion: self.appInfo.synthesizedVersion + ) + } + case let .failure(error): + Logger.logError("[Settings] Fetching newest settings failed with error: \(error)") + } } } } @@ -102,10 +114,12 @@ extension RemoteSettingsConfigurations { } func isSettingsStale() -> Bool { - return isCacheExpired(time: Date()) + cache.withLock { cache in + isCacheExpired(cache, time: Date()) + } } - private func isCacheExpired(time: Date) -> Bool { + private func isCacheExpired(_ cache: SettingsCacheClient, time: Date) -> Bool { guard !cache.cacheContent.isEmpty else { cache.removeCache() return true @@ -121,7 +135,7 @@ extension RemoteSettingsConfigurations { cache.removeCache() return true } - if time.timeIntervalSince(cacheKey.createdAt) > cacheDurationSeconds { + if time.timeIntervalSince(cacheKey.createdAt) > cacheDuration(cache) { Logger.logDebug("[Settings] Cache TTL expired") return true } diff --git a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift index 52d222e46f7..d2218ea7d94 100644 --- a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift +++ b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift @@ -15,6 +15,7 @@ import Foundation +// TODO: sendable (remove preconcurrency) #if SWIFT_PACKAGE internal import GoogleUtilities_UserDefaults #else @@ -30,7 +31,7 @@ struct CacheKey: Codable { } /// SettingsCacheClient is responsible for accessing the cache that Settings are stored in. -protocol SettingsCacheClient { +protocol SettingsCacheClient: Sendable { /// Returns in-memory cache content in O(1) time. Returns empty dictionary if it does not exist in /// cache. var cacheContent: [String: Any] { get set } @@ -45,7 +46,7 @@ protocol SettingsCacheClient { /// when accessing Settings values during run-time. This is because UserDefaults encapsulates both /// in-memory and persisted-on-disk storage, allowing fast synchronous access in-app while hiding /// away the complexity of managing persistence asynchronously. -class SettingsCache: SettingsCacheClient { +final class SettingsCache: SettingsCacheClient { private static let settingsVersion: Int = 1 private enum UserDefaultsKeys { static let forContent = "firebase-sessions-settings" diff --git a/FirebaseSessions/Sources/Settings/SettingsDownloadClient.swift b/FirebaseSessions/Sources/Settings/SettingsDownloadClient.swift index dbfd7b2a666..a1e1deaad82 100644 --- a/FirebaseSessions/Sources/Settings/SettingsDownloadClient.swift +++ b/FirebaseSessions/Sources/Settings/SettingsDownloadClient.swift @@ -21,8 +21,9 @@ import Foundation internal import GoogleUtilities #endif // SWIFT_PACKAGE -protocol SettingsDownloadClient { - func fetch(completion: @escaping (Result<[String: Any], SettingsDownloaderError>) -> Void) +protocol SettingsDownloadClient: Sendable { + func fetch(completion: @Sendable @escaping (Result<[String: Any], SettingsDownloaderError>) + -> Void) } enum SettingsDownloaderError: Error { @@ -36,7 +37,7 @@ enum SettingsDownloaderError: Error { case InstallationIDError(String) } -class SettingsDownloader: SettingsDownloadClient { +final class SettingsDownloader: SettingsDownloadClient { private let appInfo: ApplicationInfoProtocol private let installations: InstallationsProtocol @@ -45,7 +46,8 @@ class SettingsDownloader: SettingsDownloadClient { self.installations = installations } - func fetch(completion: @escaping (Result<[String: Any], SettingsDownloaderError>) -> Void) { + func fetch(completion: @Sendable @escaping (Result<[String: Any], SettingsDownloaderError>) + -> Void) { guard let validURL = url else { completion(.failure(.URLError("Invalid URL"))) return diff --git a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift index c1b02cc38aa..a7659cebc5f 100644 --- a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift +++ b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift @@ -24,7 +24,7 @@ import XCTest final class FirebaseSessionsTestsBase_BaseBehaviors: FirebaseSessionsTestsBase { // MARK: - Test Settings & Sampling - func test_settingsDisabled_doesNotLogSessionEventButDoesFetchSettings() { + @MainActor func test_settingsDisabled_doesNotLogSessionEventButDoesFetchSettings() { runSessionsSDK( subscriberSDKs: [ mockPerformanceSubscriber, @@ -49,7 +49,7 @@ final class FirebaseSessionsTestsBase_BaseBehaviors: FirebaseSessionsTestsBase { ) } - func test_sessionSampled_doesNotLogSessionEventButDoesFetchSettings() { + @MainActor func test_sessionSampled_doesNotLogSessionEventButDoesFetchSettings() { runSessionsSDK( subscriberSDKs: [ mockPerformanceSubscriber, diff --git a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+DataCollection.swift b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+DataCollection.swift index 3b4ef30c05c..5a2c5d1da97 100644 --- a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+DataCollection.swift +++ b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+DataCollection.swift @@ -81,7 +81,7 @@ final class FirebaseSessionsTestsBase_DataCollection: FirebaseSessionsTestsBase // MARK: - Test Data Collection - func test_subscriberWithDataCollectionEnabled_logsSessionEvent() { + @MainActor func test_subscriberWithDataCollectionEnabled_logsSessionEvent() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, @@ -105,7 +105,7 @@ final class FirebaseSessionsTestsBase_DataCollection: FirebaseSessionsTestsBase ) } - func test_subscribersSomeDataCollectionDisabled_logsSessionEvent() { + @MainActor func test_subscribersSomeDataCollectionDisabled_logsSessionEvent() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, @@ -132,7 +132,7 @@ final class FirebaseSessionsTestsBase_DataCollection: FirebaseSessionsTestsBase ) } - func test_subscribersAllDataCollectionDisabled_doesNotLogSessionEvent() { + @MainActor func test_subscribersAllDataCollectionDisabled_doesNotLogSessionEvent() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, @@ -159,7 +159,7 @@ final class FirebaseSessionsTestsBase_DataCollection: FirebaseSessionsTestsBase ) } - func test_defaultSamplingRate_isSetInProto() { + @MainActor func test_defaultSamplingRate_isSetInProto() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, diff --git a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+Subscribers.swift b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+Subscribers.swift index a4166207dd2..139a8826e0e 100644 --- a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+Subscribers.swift +++ b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+Subscribers.swift @@ -25,7 +25,7 @@ final class FirebaseSessionsTestsBase_Subscribers: FirebaseSessionsTestsBase { // Check that the Session ID that was passed to the Subscriber SDK // matches the Session ID that the Sessions SDK logged, and ensure // both are not empty. - func assertValidChangedSessionID() { + @MainActor func assertValidChangedSessionID() { let expectedSessionID = sessions.currentSessionDetails.sessionId XCTAssert(expectedSessionID!.count > 0) for mock in [mockCrashlyticsSubscriber, mockPerformanceSubscriber] { @@ -37,7 +37,7 @@ final class FirebaseSessionsTestsBase_Subscribers: FirebaseSessionsTestsBase { // MARK: - Test Subscriber Callbacks - func test_registerSubscriber_callsOnSessionChanged() { + @MainActor func test_registerSubscriber_callsOnSessionChanged() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, @@ -61,7 +61,7 @@ final class FirebaseSessionsTestsBase_Subscribers: FirebaseSessionsTestsBase { // Make sure that even if the Sessions SDK is disabled, and data collection // is disabled, the Sessions SDK still generates Session IDs and provides // them to Subscribers - func test_subscribersDataCollectionDisabled_callsOnSessionChanged() { + @MainActor func test_subscribersDataCollectionDisabled_callsOnSessionChanged() { runSessionsSDK( subscriberSDKs: [ mockCrashlyticsSubscriber, @@ -86,7 +86,7 @@ final class FirebaseSessionsTestsBase_Subscribers: FirebaseSessionsTestsBase { ) } - func test_noDependencies_doesNotLogSessionEvent() { + @MainActor func test_noDependencies_doesNotLogSessionEvent() { runSessionsSDK( subscriberSDKs: [], preSessionsInit: { _ in @@ -102,7 +102,7 @@ final class FirebaseSessionsTestsBase_Subscribers: FirebaseSessionsTestsBase { ) } - func test_noSubscribersWithRegistrations_doesNotCrash() { + @MainActor func test_noSubscribersWithRegistrations_doesNotCrash() { runSessionsSDK( subscriberSDKs: [], preSessionsInit: { _ in diff --git a/FirebaseSessions/Tests/Unit/InitiatorTests.swift b/FirebaseSessions/Tests/Unit/InitiatorTests.swift index ebb6be2b5bc..c3b17fb3dde 100644 --- a/FirebaseSessions/Tests/Unit/InitiatorTests.swift +++ b/FirebaseSessions/Tests/Unit/InitiatorTests.swift @@ -58,7 +58,7 @@ class InitiatorTests: XCTestCase { XCTAssert(initiateCalled) } - @MainActor func test_appForegrounded_initiatesNewSession() throws { + func test_appForegrounded_initiatesNewSession() throws { // Given var pausedClock = date let initiator = SessionInitiator( diff --git a/FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift b/FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift index fed12144691..86d1c24131c 100644 --- a/FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift +++ b/FirebaseSessions/Tests/Unit/Library/FirebaseSessionsTestsBase.swift @@ -71,11 +71,13 @@ class FirebaseSessionsTestsBase: XCTestCase { /// is a good place for Subscribers to call register on the Sessions SDK /// - `postLogEvent` is called whenever an event is logged via the Sessions SDK. This is where /// most assertions will happen. - func runSessionsSDK(subscriberSDKs: [SessionsSubscriber], - preSessionsInit: (MockSettingsProtocol) -> Void, - postSessionsInit: () -> Void, - postLogEvent: @escaping (Result, - [SessionsSubscriber]) -> Void) { + @MainActor func runSessionsSDK(subscriberSDKs: [SessionsSubscriber], + preSessionsInit: (MockSettingsProtocol) -> Void, + postSessionsInit: () -> Void, + postLogEvent: @escaping @MainActor (Result, + [SessionsSubscriber]) + -> Void) { // This class is static, so we need to clear global state SessionsDependencies.removeAll() @@ -109,12 +111,13 @@ class FirebaseSessionsTestsBase: XCTestCase { initiator: initiator, appInfo: mockAppInfo, settings: mockSettings) { result in + DispatchQueue.main.async { + // Provide the result for tests to test against + postLogEvent(result, subscriberSDKs) - // Provide the result for tests to test against - postLogEvent(result, subscriberSDKs) - - // Fulfil the expectation so the test can continue - loggedEventExpectation.fulfill() + // Fulfil the expectation so the test can continue + loggedEventExpectation.fulfill() + } } // Execute test cases after Sessions is initialized. This is a good diff --git a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift index e2ea9d54e9d..b3de14e5d56 100644 --- a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift +++ b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift @@ -36,56 +36,78 @@ import Dispatch #endif // os(visionOS) #endif // swift(>=5.9) -extension XCTestCase { - @MainActor func postBackgroundedNotification() { - let notificationCenter = NotificationCenter.default - #if os(iOS) || os(tvOS) +private func _postBackgroundedNotificationInternal() { + let notificationCenter = NotificationCenter.default + #if os(iOS) || os(tvOS) + notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) + #elseif os(macOS) + notificationCenter.post(name: NSApplication.didResignActiveNotification, object: nil) + #elseif os(watchOS) + if #available(watchOSApplicationExtension 7.0, *) { + notificationCenter.post( + name: WKExtension.applicationDidEnterBackgroundNotification, + object: nil + ) + } + #endif // os(iOS) || os(tvOS) + + // swift(>=5.9) implies Xcode 15+ + // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. + // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above + // when Xcode 15 is the minimum supported by Firebase. + #if swift(>=5.9) + #if os(visionOS) notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) - #elseif os(macOS) - notificationCenter.post(name: NSApplication.didResignActiveNotification, object: nil) - #elseif os(watchOS) - if #available(watchOSApplicationExtension 7.0, *) { - notificationCenter.post( - name: WKExtension.applicationDidEnterBackgroundNotification, - object: nil - ) - } - #endif // os(iOS) || os(tvOS) + #endif // os(visionOS) + #endif // swift(>=5.9) +} - // swift(>=5.9) implies Xcode 15+ - // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. - // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above - // when Xcode 15 is the minimum supported by Firebase. - #if swift(>=5.9) - #if os(visionOS) - notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) - #endif // os(visionOS) - #endif // swift(>=5.9) - } +private func _postForegroundedNotificationInternal() { + let notificationCenter = NotificationCenter.default + #if os(iOS) || os(tvOS) + notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) + #elseif os(macOS) + notificationCenter.post(name: NSApplication.didBecomeActiveNotification, object: nil) + #elseif os(watchOS) + if #available(watchOSApplicationExtension 7.0, *) { + notificationCenter.post( + name: WKExtension.applicationDidBecomeActiveNotification, + object: nil + ) + } + #endif // os(iOS) || os(tvOS) - @MainActor func postForegroundedNotification() { - let notificationCenter = NotificationCenter.default - #if os(iOS) || os(tvOS) + // swift(>=5.9) implies Xcode 15+ + // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. + // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above + // when Xcode 15 is the minimum supported by Firebase. + #if swift(>=5.9) + #if os(visionOS) notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) - #elseif os(macOS) - notificationCenter.post(name: NSApplication.didBecomeActiveNotification, object: nil) - #elseif os(watchOS) - if #available(watchOSApplicationExtension 7.0, *) { - notificationCenter.post( - name: WKExtension.applicationDidBecomeActiveNotification, - object: nil - ) + #endif // os(visionOS) + #endif // swift(>=5.9) +} + +extension XCTestCase { + func postBackgroundedNotification() { + // On Catalyst, the notifications can only be called on a the main thread + if Thread.isMainThread { + _postBackgroundedNotificationInternal() + } else { + DispatchQueue.main.sync { + _postBackgroundedNotificationInternal() } - #endif // os(iOS) || os(tvOS) + } + } - // swift(>=5.9) implies Xcode 15+ - // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. - // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above - // when Xcode 15 is the minimum supported by Firebase. - #if swift(>=5.9) - #if os(visionOS) - notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) - #endif // os(visionOS) - #endif // swift(>=5.9) + func postForegroundedNotification() { + // On Catalyst, the notifications can only be called on a the main thread + if Thread.isMainThread { + _postForegroundedNotificationInternal() + } else { + DispatchQueue.main.sync { + _postForegroundedNotificationInternal() + } + } } } diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockApplicationInfo.swift b/FirebaseSessions/Tests/Unit/Mocks/MockApplicationInfo.swift index fea2bfc5cfd..8c5e3e5d07c 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockApplicationInfo.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockApplicationInfo.swift @@ -23,7 +23,7 @@ import Foundation @testable import FirebaseSessions -class MockApplicationInfo: ApplicationInfoProtocol { +class MockApplicationInfo: ApplicationInfoProtocol, @unchecked Sendable { var appID: String = "" var bundleID: String = "" diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockInstallationsProtocol.swift b/FirebaseSessions/Tests/Unit/Mocks/MockInstallationsProtocol.swift index e61f492e8df..112605e7aeb 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockInstallationsProtocol.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockInstallationsProtocol.swift @@ -17,7 +17,7 @@ internal import FirebaseInstallations @testable import FirebaseSessions -class MockInstallationsProtocol: InstallationsProtocol { +class MockInstallationsProtocol: InstallationsProtocol, @unchecked Sendable { static let testInstallationId = "testInstallationId" static let testAuthToken = "testAuthToken" var result: Result<(String, String), Error> = .success((testInstallationId, testAuthToken)) diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockNetworkInfo.swift b/FirebaseSessions/Tests/Unit/Mocks/MockNetworkInfo.swift index 103434179dc..9413c3951b0 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockNetworkInfo.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockNetworkInfo.swift @@ -23,7 +23,7 @@ import Foundation @testable import FirebaseSessions -class MockNetworkInfo: NetworkInfoProtocol { +class MockNetworkInfo: NetworkInfoProtocol, @unchecked Sendable { var mobileCountryCode: String? var mobileNetworkCode: String? var networkType: GULNetworkType = .WIFI diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockSessionCoordinator.swift b/FirebaseSessions/Tests/Unit/Mocks/MockSessionCoordinator.swift index 19672f70b6a..3bfad146882 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockSessionCoordinator.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockSessionCoordinator.swift @@ -16,7 +16,7 @@ @testable import FirebaseSessions import XCTest -class MockSessionCoordinator: SessionCoordinatorProtocol { +class MockSessionCoordinator: SessionCoordinatorProtocol, @unchecked Sendable { var loggedEvent: FirebaseSessions.SessionStartEvent? func attemptLoggingSessionStart(event: FirebaseSessions.SessionStartEvent, diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockSettingsDownloader.swift b/FirebaseSessions/Tests/Unit/Mocks/MockSettingsDownloader.swift index 3785f52e54e..df8c10fa3d8 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockSettingsDownloader.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockSettingsDownloader.swift @@ -17,7 +17,7 @@ import Foundation @testable import FirebaseSessions -class MockSettingsDownloader: SettingsDownloadClient { +class MockSettingsDownloader: SettingsDownloadClient, @unchecked Sendable { public var shouldSucceed: Bool = true public var successResponse: [String: Any] diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift b/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift index 7840809f586..9a3f6a0d9c1 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift @@ -16,7 +16,7 @@ @testable import FirebaseSessions import Foundation -final class MockSubscriber: SessionsSubscriber { +final class MockSubscriber: SessionsSubscriber, @unchecked Sendable /* not actually sendable */ { var sessionThatChanged: FirebaseSessions.SessionDetails? init(name: SessionsSubscriberName) { diff --git a/FirebaseSessions/Tests/Unit/SessionGeneratorTests.swift b/FirebaseSessions/Tests/Unit/SessionGeneratorTests.swift index e71215c8aa2..9d64940bca2 100644 --- a/FirebaseSessions/Tests/Unit/SessionGeneratorTests.swift +++ b/FirebaseSessions/Tests/Unit/SessionGeneratorTests.swift @@ -53,6 +53,7 @@ class SessionGeneratorTests: XCTestCase { localOverrides: localOverrideSettings, remoteSettings: remoteSettings ) + generator = SessionGenerator(collectEvents: Sessions .shouldCollectEvents(settings: sessionSettings)) } From b637a2dfedfb976fa0c93a18fb78e07a4abdba3d Mon Sep 17 00:00:00 2001 From: Morgan Chen Date: Wed, 14 May 2025 14:28:42 -0400 Subject: [PATCH 04/10] Morgan's work from mc/messaging --- .../Library/Public/FirebaseInstallations/FIRInstallations.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/FirebaseInstallations/Source/Library/Public/FirebaseInstallations/FIRInstallations.h b/FirebaseInstallations/Source/Library/Public/FirebaseInstallations/FIRInstallations.h index 1811d2bbdf1..7670d40b301 100644 --- a/FirebaseInstallations/Source/Library/Public/FirebaseInstallations/FIRInstallations.h +++ b/FirebaseInstallations/Source/Library/Public/FirebaseInstallations/FIRInstallations.h @@ -57,8 +57,7 @@ typedef void (^FIRInstallationsTokenHandler)( * as the ability to delete it. A Firebase Installation is unique by `FirebaseApp.name` and * `FirebaseApp.options.googleAppID` . */ -NS_SWIFT_NAME(Installations) -@interface FIRInstallations : NSObject +NS_SWIFT_NAME(Installations) NS_SWIFT_SENDABLE @interface FIRInstallations : NSObject - (instancetype)init NS_UNAVAILABLE; From dbb8ba863f79c6ecb376934dcdf8af01806be1fe Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Wed, 14 May 2025 14:44:44 -0400 Subject: [PATCH 05/10] Add preconcurrency attribute to imports --- FirebaseSessions/Sources/Settings/SettingsCacheClient.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift index d2218ea7d94..cee72dab883 100644 --- a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift +++ b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift @@ -17,9 +17,9 @@ import Foundation // TODO: sendable (remove preconcurrency) #if SWIFT_PACKAGE - internal import GoogleUtilities_UserDefaults + @preconcurrency internal import GoogleUtilities_UserDefaults #else - internal import GoogleUtilities + @preconcurrency internal import GoogleUtilities #endif // SWIFT_PACKAGE /// CacheKey is like a "key" to a "safe". It provides necessary metadata about the current cache to From 556ea368c2ad75fe5761851edb2db5d49fd7f764 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Wed, 14 May 2025 17:30:21 -0400 Subject: [PATCH 06/10] Remove comment --- .../Development/DevEventConsoleLogger.swift | 2 +- FirebaseSessions/Sources/EventGDTLogger.swift | 4 +-- .../Sources/FirebaseSessions.swift | 4 +-- ...ransport+GoogleDataTransportProtocol.swift | 25 +++++++++++++++---- .../Sources/SessionCoordinator.swift | 3 +-- .../Tests/Unit/Mocks/MockGDTLogger.swift | 3 ++- 6 files changed, 28 insertions(+), 13 deletions(-) diff --git a/FirebaseSessions/Sources/Development/DevEventConsoleLogger.swift b/FirebaseSessions/Sources/Development/DevEventConsoleLogger.swift index 488fca7efc6..8eeb17739dd 100644 --- a/FirebaseSessions/Sources/Development/DevEventConsoleLogger.swift +++ b/FirebaseSessions/Sources/Development/DevEventConsoleLogger.swift @@ -19,7 +19,7 @@ import Foundation import FirebaseSessionsObjC #endif // SWIFT_PACKAGE -class DevEventConsoleLogger: EventGDTLoggerProtocol { +final class DevEventConsoleLogger: EventGDTLoggerProtocol { private let commandLineArgument = "-FIRSessionsDebugEvents" func logEvent(event: SessionStartEvent, completion: @escaping (Result) -> Void) { diff --git a/FirebaseSessions/Sources/EventGDTLogger.swift b/FirebaseSessions/Sources/EventGDTLogger.swift index 20070706ae5..bb96c7753bc 100644 --- a/FirebaseSessions/Sources/EventGDTLogger.swift +++ b/FirebaseSessions/Sources/EventGDTLogger.swift @@ -17,7 +17,7 @@ import Foundation internal import GoogleDataTransport -protocol EventGDTLoggerProtocol { +protocol EventGDTLoggerProtocol: Sendable { func logEvent(event: SessionStartEvent, completion: @escaping (Result) -> Void) } @@ -26,7 +26,7 @@ protocol EventGDTLoggerProtocol { /// 1) Creating GDT Events and logging them to the GoogleDataTransport SDK /// 2) Handling debugging situations (eg. running in Simulator or printing the event to console) /// -class EventGDTLogger: EventGDTLoggerProtocol { +final class EventGDTLogger: EventGDTLoggerProtocol { let googleDataTransport: GoogleDataTransportProtocol let devEventConsoleLogger: EventGDTLoggerProtocol diff --git a/FirebaseSessions/Sources/FirebaseSessions.swift b/FirebaseSessions/Sources/FirebaseSessions.swift index 31550ae4872..149e56c7167 100644 --- a/FirebaseSessions/Sources/FirebaseSessions.swift +++ b/FirebaseSessions/Sources/FirebaseSessions.swift @@ -62,13 +62,13 @@ private enum GoogleDataTransportConfig { // Initializes the SDK and top-level classes required convenience init(appID: String, installations: InstallationsProtocol) { - let googleDataTransport = GDTCORTransport( + let googleDataTransport = GoogleDataTransporter( mappingID: GoogleDataTransportConfig.sessionsLogSource, transformers: nil, target: GoogleDataTransportConfig.sessionsTarget ) - let fireLogger = EventGDTLogger(googleDataTransport: googleDataTransport!) + let fireLogger = EventGDTLogger(googleDataTransport: googleDataTransport) let appInfo = ApplicationInfo(appID: appID) let settings = SessionsSettings( diff --git a/FirebaseSessions/Sources/GoogleDataTransport+GoogleDataTransportProtocol.swift b/FirebaseSessions/Sources/GoogleDataTransport+GoogleDataTransportProtocol.swift index b194b736ba4..9c244fe6fe4 100644 --- a/FirebaseSessions/Sources/GoogleDataTransport+GoogleDataTransportProtocol.swift +++ b/FirebaseSessions/Sources/GoogleDataTransport+GoogleDataTransportProtocol.swift @@ -15,20 +15,31 @@ import Foundation -internal import GoogleDataTransport +@preconcurrency internal import GoogleDataTransport enum GoogleDataTransportProtocolErrors: Error { case writeFailure } -protocol GoogleDataTransportProtocol { +protocol GoogleDataTransportProtocol: Sendable { func logGDTEvent(event: GDTCOREvent, completion: @escaping (Result) -> Void) func eventForTransport() -> GDTCOREvent } -extension GDTCORTransport: GoogleDataTransportProtocol { - func logGDTEvent(event: GDTCOREvent, completion: @escaping (Result) -> Void) { - sendDataEvent(event) { wasWritten, error in +/// Workaround in combo with preconcurrency import of GDT. When GDT's +/// `GDTCORTransport`type conforms to Sendable within the GDT module, +/// this can be removed. +final class GoogleDataTransporter: GoogleDataTransportProtocol { + private let transporter: GDTCORTransport + + init(mappingID: String, + transformers: [any GDTCOREventTransformer]?, + target: GDTCORTarget) { + transporter = GDTCORTransport(mappingID: mappingID, transformers: transformers, target: target)! + } + + func logGDTEvent(event: GDTCOREvent, completion: @escaping (Result) -> Void) { + transporter.sendDataEvent(event) { wasWritten, error in if let error { completion(.failure(error)) } else if !wasWritten { @@ -38,4 +49,8 @@ extension GDTCORTransport: GoogleDataTransportProtocol { } } } + + func eventForTransport() -> GDTCOREvent { + transporter.eventForTransport() + } } diff --git a/FirebaseSessions/Sources/SessionCoordinator.swift b/FirebaseSessions/Sources/SessionCoordinator.swift index 5811f9c7768..ab2d8898f67 100644 --- a/FirebaseSessions/Sources/SessionCoordinator.swift +++ b/FirebaseSessions/Sources/SessionCoordinator.swift @@ -26,8 +26,7 @@ protocol SessionCoordinatorProtocol: Sendable { final class SessionCoordinator: SessionCoordinatorProtocol { let installations: InstallationsProtocol - // TODO: Make this type sendable - nonisolated(unsafe) let fireLogger: EventGDTLoggerProtocol + let fireLogger: EventGDTLoggerProtocol init(installations: InstallationsProtocol, fireLogger: EventGDTLoggerProtocol) { diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockGDTLogger.swift b/FirebaseSessions/Tests/Unit/Mocks/MockGDTLogger.swift index a44a2dd0147..732470e5c8d 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockGDTLogger.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockGDTLogger.swift @@ -17,7 +17,8 @@ import Foundation @testable import FirebaseSessions -class MockGDTLogger: EventGDTLoggerProtocol { +// TODO(Swift 6): Add checked Sendable support. +final class MockGDTLogger: EventGDTLoggerProtocol, @unchecked Sendable { var loggedEvent: SessionStartEvent? var result: Result = .success(()) From 268a3061e29b98061b18160460044413540afeb2 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Wed, 14 May 2025 17:34:46 -0400 Subject: [PATCH 07/10] add todo --- FirebaseSessions/Sources/FirebaseSessions.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/FirebaseSessions/Sources/FirebaseSessions.swift b/FirebaseSessions/Sources/FirebaseSessions.swift index 149e56c7167..0894bf2a028 100644 --- a/FirebaseSessions/Sources/FirebaseSessions.swift +++ b/FirebaseSessions/Sources/FirebaseSessions.swift @@ -268,7 +268,8 @@ private enum GoogleDataTransportConfig { "Registering Sessions SDK subscriber with name: \(subscriber.sessionsSubscriberName), data collection enabled: \(subscriber.isDataCollectionEnabled)" ) - // After bumping to iOS 13, this hack should be replaced with `Task { @MainActor in }` + // TODO(Firebase 12): After bumping to iOS 13, this hack should be replaced + // with `Task { @MainActor in }`. let callback = MainActorNotificationCallback { notification in subscriber.onSessionChanged(self.currentSessionDetails) } From 1c3797a5a7065242e93df758dee422fb11a0c32a Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 15 May 2025 16:40:25 -0400 Subject: [PATCH 08/10] Refactor to avoid unsynchronized access --- .../Sources/Settings/RemoteSettings.swift | 41 +------------------ .../Settings/SettingsCacheClient.swift | 39 ++++++++++++++++++ 2 files changed, 41 insertions(+), 39 deletions(-) diff --git a/FirebaseSessions/Sources/Settings/RemoteSettings.swift b/FirebaseSessions/Sources/Settings/RemoteSettings.swift index 08ce367bdeb..9f57b348b88 100644 --- a/FirebaseSessions/Sources/Settings/RemoteSettings.swift +++ b/FirebaseSessions/Sources/Settings/RemoteSettings.swift @@ -23,24 +23,14 @@ extension ApplicationInfoProtocol { } final class RemoteSettings: SettingsProvider, Sendable { - private static let cacheDurationSecondsDefault: TimeInterval = 60 * 60 private static let flagSessionsEnabled = "sessions_enabled" private static let flagSamplingRate = "sampling_rate" private static let flagSessionTimeout = "session_timeout_seconds" - private static let flagCacheDuration = "cache_duration" private static let flagSessionsCache = "app_quality" private let appInfo: ApplicationInfoProtocol private let downloader: SettingsDownloadClient private let cache: FIRAllocatedUnfairLock - private func cacheDuration(_ cache: SettingsCacheClient) -> TimeInterval { - guard let duration = cache.cacheContent[RemoteSettings.flagCacheDuration] as? Double else { - return RemoteSettings.cacheDurationSecondsDefault - } - print("Duration: \(duration)") - return duration - } - private var sessionsCache: [String: Any] { cache.withLock { cache in cache.cacheContent[RemoteSettings.flagSessionsCache] as? [String: Any] ?? [:] @@ -58,7 +48,7 @@ final class RemoteSettings: SettingsProvider, Sendable { private func fetchAndCacheSettings(currentTime: Date) { let shouldFetch = cache.withLock { cache in // Only fetch if cache is expired, otherwise do nothing - guard isCacheExpired(cache, time: currentTime) else { + guard cache.isExpired(for: appInfo, time: currentTime) else { Logger.logDebug("[Settings] Cache is not expired, no fetch will be made.") return false } @@ -115,34 +105,7 @@ extension RemoteSettingsConfigurations { func isSettingsStale() -> Bool { cache.withLock { cache in - isCacheExpired(cache, time: Date()) - } - } - - private func isCacheExpired(_ cache: SettingsCacheClient, time: Date) -> Bool { - guard !cache.cacheContent.isEmpty else { - cache.removeCache() - return true - } - guard let cacheKey = cache.cacheKey else { - Logger.logError("[Settings] Could not load settings cache key") - cache.removeCache() - return true - } - guard cacheKey.googleAppID == appInfo.appID else { - Logger - .logDebug("[Settings] Cache expired because Google App ID changed") - cache.removeCache() - return true - } - if time.timeIntervalSince(cacheKey.createdAt) > cacheDuration(cache) { - Logger.logDebug("[Settings] Cache TTL expired") - return true - } - if appInfo.synthesizedVersion != cacheKey.appVersion { - Logger.logDebug("[Settings] Cache expired because app version changed") - return true + cache.isExpired(for: appInfo, time: Date()) } - return false } } diff --git a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift index cee72dab883..18f9bcefbff 100644 --- a/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift +++ b/FirebaseSessions/Sources/Settings/SettingsCacheClient.swift @@ -40,6 +40,8 @@ protocol SettingsCacheClient: Sendable { var cacheKey: CacheKey? { get set } /// Removes all cache content and cache-key func removeCache() + /// Returns whether the cache is expired for the given app info structure and time. + func isExpired(for appInfo: ApplicationInfoProtocol, time: Date) -> Bool } /// SettingsCache uses UserDefaults to store Settings on-disk, but also directly query UserDefaults @@ -47,6 +49,8 @@ protocol SettingsCacheClient: Sendable { /// in-memory and persisted-on-disk storage, allowing fast synchronous access in-app while hiding /// away the complexity of managing persistence asynchronously. final class SettingsCache: SettingsCacheClient { + private static let cacheDurationSecondsDefault: TimeInterval = 60 * 60 + private static let flagCacheDuration = "cache_duration" private static let settingsVersion: Int = 1 private enum UserDefaultsKeys { static let forContent = "firebase-sessions-settings" @@ -93,4 +97,39 @@ final class SettingsCache: SettingsCacheClient { cache.setObject(nil, forKey: UserDefaultsKeys.forContent) cache.setObject(nil, forKey: UserDefaultsKeys.forCacheKey) } + + func isExpired(for appInfo: ApplicationInfoProtocol, time: Date) -> Bool { + guard !cacheContent.isEmpty else { + removeCache() + return true + } + guard let cacheKey = cacheKey else { + Logger.logError("[Settings] Could not load settings cache key") + removeCache() + return true + } + guard cacheKey.googleAppID == appInfo.appID else { + Logger + .logDebug("[Settings] Cache expired because Google App ID changed") + removeCache() + return true + } + if time.timeIntervalSince(cacheKey.createdAt) > cacheDuration() { + Logger.logDebug("[Settings] Cache TTL expired") + return true + } + if appInfo.synthesizedVersion != cacheKey.appVersion { + Logger.logDebug("[Settings] Cache expired because app version changed") + return true + } + return false + } + + private func cacheDuration() -> TimeInterval { + guard let duration = cacheContent[Self.flagCacheDuration] as? Double else { + return Self.cacheDurationSecondsDefault + } + print("Duration: \(duration)") + return duration + } } From 021fb99635b9dc856e8bd205bd06b284f43cd4ad Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 15 May 2025 18:00:50 -0400 Subject: [PATCH 09/10] review --- .../FirebaseSessionsTests+BaseBehaviors.swift | 4 +- .../Tests/Unit/InitiatorTests.swift | 10 +-- .../Unit/Library/LifecycleNotifications.swift | 66 ++----------------- 3 files changed, 14 insertions(+), 66 deletions(-) diff --git a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift index a7659cebc5f..462be23ead3 100644 --- a/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift +++ b/FirebaseSessions/Tests/Unit/FirebaseSessionsTests+BaseBehaviors.swift @@ -128,9 +128,9 @@ final class FirebaseSessionsTestsBase_BaseBehaviors: FirebaseSessionsTestsBase { // then bring the app to the foreground to generate another session. // // This postLogEvent callback will be called again after this - self.postBackgroundedNotification() + postBackgroundedNotification() self.pausedClock.addTimeInterval(30 * 60 + 1) - self.postForegroundedNotification() + postForegroundedNotification() } else { loggedTwiceExpectation.fulfill() diff --git a/FirebaseSessions/Tests/Unit/InitiatorTests.swift b/FirebaseSessions/Tests/Unit/InitiatorTests.swift index c3b17fb3dde..0f6ca6b2f5e 100644 --- a/FirebaseSessions/Tests/Unit/InitiatorTests.swift +++ b/FirebaseSessions/Tests/Unit/InitiatorTests.swift @@ -58,7 +58,7 @@ class InitiatorTests: XCTestCase { XCTAssert(initiateCalled) } - func test_appForegrounded_initiatesNewSession() throws { + func test_appForegrounded_initiatesNewSession() async throws { // Given var pausedClock = date let initiator = SessionInitiator( @@ -73,18 +73,18 @@ class InitiatorTests: XCTestCase { // When // Background, advance time by 30 minutes + 1 second, then foreground - postBackgroundedNotification() + await postBackgroundedNotification() pausedClock.addTimeInterval(30 * 60 + 1) - postForegroundedNotification() + await postForegroundedNotification() // Then // Session count increases because time spent in background > 30 minutes XCTAssert(sessionCount == 2) // When // Background, advance time by exactly 30 minutes, then foreground - postBackgroundedNotification() + await postBackgroundedNotification() pausedClock.addTimeInterval(30 * 60) - postForegroundedNotification() + await postForegroundedNotification() // Then // Session count doesn't increase because time spent in background <= 30 minutes XCTAssert(sessionCount == 2) diff --git a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift index b3de14e5d56..b406c32ece5 100644 --- a/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift +++ b/FirebaseSessions/Tests/Unit/Library/LifecycleNotifications.swift @@ -17,7 +17,7 @@ import XCTest import Dispatch -#if os(iOS) || os(tvOS) +#if os(iOS) || os(tvOS) || os(visionOS) import UIKit #elseif os(macOS) import AppKit @@ -26,19 +26,10 @@ import Dispatch import WatchKit #endif // os(iOS) || os(tvOS) -// swift(>=5.9) implies Xcode 15+ -// Need to have this Swift version check to use os(visionOS) macro, VisionOS support. -// TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above -// when Xcode 15 is the minimum supported by Firebase. -#if swift(>=5.9) - #if os(visionOS) - import UIKit - #endif // os(visionOS) -#endif // swift(>=5.9) - -private func _postBackgroundedNotificationInternal() { +@MainActor func postBackgroundedNotification() { + // On Catalyst, the notifications can only be called on the main thread let notificationCenter = NotificationCenter.default - #if os(iOS) || os(tvOS) + #if os(iOS) || os(tvOS) || os(visionOS) notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) #elseif os(macOS) notificationCenter.post(name: NSApplication.didResignActiveNotification, object: nil) @@ -50,21 +41,12 @@ private func _postBackgroundedNotificationInternal() { ) } #endif // os(iOS) || os(tvOS) - - // swift(>=5.9) implies Xcode 15+ - // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. - // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above - // when Xcode 15 is the minimum supported by Firebase. - #if swift(>=5.9) - #if os(visionOS) - notificationCenter.post(name: UIApplication.didEnterBackgroundNotification, object: nil) - #endif // os(visionOS) - #endif // swift(>=5.9) } -private func _postForegroundedNotificationInternal() { +@MainActor func postForegroundedNotification() { + // On Catalyst, the notifications can only be called on a the main thread let notificationCenter = NotificationCenter.default - #if os(iOS) || os(tvOS) + #if os(iOS) || os(tvOS) || os(visionOS) notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) #elseif os(macOS) notificationCenter.post(name: NSApplication.didBecomeActiveNotification, object: nil) @@ -76,38 +58,4 @@ private func _postForegroundedNotificationInternal() { ) } #endif // os(iOS) || os(tvOS) - - // swift(>=5.9) implies Xcode 15+ - // Need to have this Swift version check to use os(visionOS) macro, VisionOS support. - // TODO: Remove this check and add `os(visionOS)` to the `os(iOS) || os(tvOS)` conditional above - // when Xcode 15 is the minimum supported by Firebase. - #if swift(>=5.9) - #if os(visionOS) - notificationCenter.post(name: UIApplication.didBecomeActiveNotification, object: nil) - #endif // os(visionOS) - #endif // swift(>=5.9) -} - -extension XCTestCase { - func postBackgroundedNotification() { - // On Catalyst, the notifications can only be called on a the main thread - if Thread.isMainThread { - _postBackgroundedNotificationInternal() - } else { - DispatchQueue.main.sync { - _postBackgroundedNotificationInternal() - } - } - } - - func postForegroundedNotification() { - // On Catalyst, the notifications can only be called on a the main thread - if Thread.isMainThread { - _postForegroundedNotificationInternal() - } else { - DispatchQueue.main.sync { - _postForegroundedNotificationInternal() - } - } - } } From 1c5ebff5311f770ab3fdfef957d598088dab32b8 Mon Sep 17 00:00:00 2001 From: Nick Cooke Date: Thu, 15 May 2025 18:09:46 -0400 Subject: [PATCH 10/10] style and more fixes --- .../Sources/Public/SessionsSubscriber.swift | 2 +- .../Tests/Unit/Mocks/MockSubscriber.swift | 26 ++++++++++++++----- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/FirebaseSessions/Sources/Public/SessionsSubscriber.swift b/FirebaseSessions/Sources/Public/SessionsSubscriber.swift index 6d053b83f5c..bc15106e3c5 100644 --- a/FirebaseSessions/Sources/Public/SessionsSubscriber.swift +++ b/FirebaseSessions/Sources/Public/SessionsSubscriber.swift @@ -38,7 +38,7 @@ public class SessionDetails: NSObject { /// Session Subscriber Names are used for identifying subscribers @objc(FIRSessionsSubscriberName) -public enum SessionsSubscriberName: Int, CustomStringConvertible { +public enum SessionsSubscriberName: Int, CustomStringConvertible, Sendable { case Unknown case Crashlytics case Performance diff --git a/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift b/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift index 9a3f6a0d9c1..2f3845e70bc 100644 --- a/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift +++ b/FirebaseSessions/Tests/Unit/Mocks/MockSubscriber.swift @@ -13,21 +13,33 @@ // See the License for the specific language governing permissions and // limitations under the License. +import FirebaseCoreInternal @testable import FirebaseSessions import Foundation -final class MockSubscriber: SessionsSubscriber, @unchecked Sendable /* not actually sendable */ { - var sessionThatChanged: FirebaseSessions.SessionDetails? +final class MockSubscriber: SessionsSubscriber, Sendable { + let sessionsSubscriberName: FirebaseSessions.SessionsSubscriberName + + var sessionThatChanged: FirebaseSessions.SessionDetails? { + get { _sessionThatChanged.value() } + set { _sessionThatChanged.withLock { $0 = newValue } } + } + + var isDataCollectionEnabled: Bool { + get { _isDataCollectionEnabled.value() } + set { _isDataCollectionEnabled.withLock { $0 = newValue } } + } + + private let _sessionThatChanged = FIRAllocatedUnfairLock( + initialState: nil + ) + private let _isDataCollectionEnabled = FIRAllocatedUnfairLock(initialState: true) init(name: SessionsSubscriberName) { sessionsSubscriberName = name } func onSessionChanged(_ session: FirebaseSessions.SessionDetails) { - sessionThatChanged = session + _sessionThatChanged.withLock { $0 = session } } - - var isDataCollectionEnabled: Bool = true - - var sessionsSubscriberName: FirebaseSessions.SessionsSubscriberName }