[Core] Make storage class conform to Sendable and subsequent changes (#13939)

Author: ncooke3

FirebaseCore/Internal/Sources/HeartbeatLogging/HeartbeatController.swift

@@ -15,7 +15,7 @@
 import Foundation
 
 /// An object that provides API to log and flush heartbeats from a synchronized storage container.
-public final class HeartbeatController {
+public final class HeartbeatController: Sendable {
   /// Used for standardizing dates for calendar-day comparison.
   private enum DateStandardizer {
     private static let calendar: Calendar = {
@@ -31,18 +31,18 @@ public final class HeartbeatController {
   }
 
   /// The thread-safe storage object to log and flush heartbeats from.
-  private let storage: HeartbeatStorageProtocol
+  private let storage: any HeartbeatStorageProtocol
   /// The max capacity of heartbeats to store in storage.
-  private let heartbeatsStorageCapacity: Int = 30
+  private static let heartbeatsStorageCapacity: Int = 30
   /// Current date provider. It is used for testability.
-  private let dateProvider: () -> Date
+  private let dateProvider: @Sendable () -> Date
   /// Used for standardizing dates for calendar-day comparison.
   private static let dateStandardizer = DateStandardizer.self
 
   /// Public initializer.
   /// - Parameter id: The `id` to associate this controller's heartbeat storage with.
   public convenience init(id: String) {
-    self.init(id: id, dateProvider: Date.init)
+    self.init(id: id, dateProvider: { Date() })
   }
 
   /// Convenience initializer. Mirrors the semantics of the public initializer with the added
@@ -51,7 +51,7 @@ public final class HeartbeatController {
   /// - Parameters:
   ///   - id: The id to associate this controller's heartbeat storage with.
   ///   - dateProvider: A date provider.
-  convenience init(id: String, dateProvider: @escaping () -> Date) {
+  convenience init(id: String, dateProvider: @escaping @Sendable () -> Date) {
     let storage = HeartbeatStorage.getInstance(id: id)
     self.init(storage: storage, dateProvider: dateProvider)
   }
@@ -61,7 +61,7 @@ public final class HeartbeatController {
   ///   - storage: A heartbeat storage container.
   ///   - dateProvider: A date provider. Defaults to providing the current date.
   init(storage: HeartbeatStorageProtocol,
-       dateProvider: @escaping () -> Date = Date.init) {
+       dateProvider: @escaping @Sendable () -> Date = { Date() }) {
     self.storage = storage
     self.dateProvider = { Self.dateStandardizer.standardize(dateProvider()) }
   }
@@ -76,7 +76,7 @@ public final class HeartbeatController {
 
     storage.readAndWriteAsync { heartbeatsBundle in
       var heartbeatsBundle = heartbeatsBundle ??
-        HeartbeatsBundle(capacity: self.heartbeatsStorageCapacity)
+        HeartbeatsBundle(capacity: Self.heartbeatsStorageCapacity)
 
       // Filter for the time periods where the last heartbeat to be logged for
       // that time period was logged more than one time period (i.e. day) ago.
@@ -109,7 +109,7 @@ public final class HeartbeatController {
       // The new value that's stored will use the old's cache to prevent the
       // logging of duplicates after flushing.
       return HeartbeatsBundle(
-        capacity: self.heartbeatsStorageCapacity,
+        capacity: Self.heartbeatsStorageCapacity,
         cache: oldHeartbeatsBundle.lastAddedHeartbeatDates
       )
     }
@@ -126,15 +126,15 @@ public final class HeartbeatController {
     }
   }
 
-  public func flushAsync(completionHandler: @escaping (HeartbeatsPayload) -> Void) {
-    let resetTransform = { (heartbeatsBundle: HeartbeatsBundle?) -> HeartbeatsBundle? in
+  public func flushAsync(completionHandler: @escaping @Sendable (HeartbeatsPayload) -> Void) {
+    let resetTransform = { @Sendable (heartbeatsBundle: HeartbeatsBundle?) -> HeartbeatsBundle? in
       guard let oldHeartbeatsBundle = heartbeatsBundle else {
         return nil // Storage was empty.
       }
       // The new value that's stored will use the old's cache to prevent the
       // logging of duplicates after flushing.
       return HeartbeatsBundle(
-        capacity: self.heartbeatsStorageCapacity,
+        capacity: Self.heartbeatsStorageCapacity,
         cache: oldHeartbeatsBundle.lastAddedHeartbeatDates
       )
     }

FirebaseCore/Internal/Sources/HeartbeatLogging/HeartbeatStorage.swift

@@ -15,21 +15,22 @@
 import Foundation
 
 /// A type that can perform atomic operations using block-based transformations.
-protocol HeartbeatStorageProtocol {
+protocol HeartbeatStorageProtocol: Sendable {
   func readAndWriteSync(using transform: (HeartbeatsBundle?) -> HeartbeatsBundle?)
-  func readAndWriteAsync(using transform: @escaping (HeartbeatsBundle?) -> HeartbeatsBundle?)
+  func readAndWriteAsync(using transform: @escaping @Sendable (HeartbeatsBundle?)
+    -> HeartbeatsBundle?)
   func getAndSet(using transform: (HeartbeatsBundle?) -> HeartbeatsBundle?) throws
     -> HeartbeatsBundle?
-  func getAndSetAsync(using transform: @escaping (HeartbeatsBundle?) -> HeartbeatsBundle?,
-                      completion: @escaping (Result<HeartbeatsBundle?, Error>) -> Void)
+  func getAndSetAsync(using transform: @escaping @Sendable (HeartbeatsBundle?) -> HeartbeatsBundle?,
+                      completion: @escaping @Sendable (Result<HeartbeatsBundle?, Error>) -> Void)
 }
 
 /// Thread-safe storage object designed for transforming heartbeat data that is persisted to disk.
-final class HeartbeatStorage: HeartbeatStorageProtocol {
+final class HeartbeatStorage: Sendable, HeartbeatStorageProtocol {
   /// The identifier used to differentiate instances.
   private let id: String
   /// The underlying storage container to read from and write to.
-  private let storage: Storage
+  private let storage: any Storage
   /// The encoder used for encoding heartbeat data.
   private let encoder: JSONEncoder = .init()
   /// The decoder used for decoding heartbeat data.
@@ -130,7 +131,8 @@ final class HeartbeatStorage: HeartbeatStorageProtocol {
   /// Asynchronously reads from and writes to storage using the given transform block.
   /// - Parameter transform: A block to transform the currently stored heartbeats bundle to a new
   /// heartbeats bundle value.
-  func readAndWriteAsync(using transform: @escaping (HeartbeatsBundle?) -> HeartbeatsBundle?) {
+  func readAndWriteAsync(using transform: @escaping @Sendable (HeartbeatsBundle?)
+    -> HeartbeatsBundle?) {
     queue.async { [self] in
       let oldHeartbeatsBundle = try? load(from: storage)
       let newHeartbeatsBundle = transform(oldHeartbeatsBundle)
@@ -166,8 +168,8 @@ final class HeartbeatStorage: HeartbeatStorageProtocol {
   ///   - completion: An escaping block used to process the heartbeat data that
   ///   was stored (before the `transform` was applied); otherwise, the error
   ///   that occurred.
-  func getAndSetAsync(using transform: @escaping (HeartbeatsBundle?) -> HeartbeatsBundle?,
-                      completion: @escaping (Result<HeartbeatsBundle?, Error>) -> Void) {
+  func getAndSetAsync(using transform: @escaping @Sendable (HeartbeatsBundle?) -> HeartbeatsBundle?,
+                      completion: @escaping @Sendable (Result<HeartbeatsBundle?, Error>) -> Void) {
     queue.async {
       do {
         let oldHeartbeatsBundle = try? self.load(from: self.storage)

FirebaseCore/Internal/Sources/HeartbeatLogging/Storage.swift

@@ -15,7 +15,7 @@
 import Foundation
 
 /// A type that reads from and writes to an underlying storage container.
-protocol Storage {
+protocol Storage: Sendable {
   /// Reads and returns the data stored by this storage type.
   /// - Returns: The data read from storage.
   /// - Throws: An error if the read failed.
@@ -38,16 +38,12 @@ enum StorageError: Error {
 final class FileStorage: Storage {
   /// A  file system URL to the underlying file resource.
   private let url: URL
-  /// The file manager used to perform file system operations.
-  private let fileManager: FileManager
 
   /// Designated initializer.
   /// - Parameters:
   ///   - url: A file system URL for the underlying file resource.
-  ///   - fileManager: A file manager. Defaults to `default` manager.
-  init(url: URL, fileManager: FileManager = .default) {
+  init(url: URL) {
     self.url = url
-    self.fileManager = fileManager
   }
 
   /// Reads and returns the data from this object's associated file resource.
@@ -90,7 +86,7 @@ final class FileStorage: Storage {
   /// - Parameter url: The URL to create directories in.
   private func createDirectories(in url: URL) throws {
     do {
-      try fileManager.createDirectory(
+      try FileManager.default.createDirectory(
         at: url,
         withIntermediateDirectories: true
       )
@@ -104,17 +100,26 @@ final class FileStorage: Storage {
 
 /// A object that provides API for reading and writing to a user defaults resource.
 final class UserDefaultsStorage: Storage {
-  /// The underlying defaults container.
-  private let defaults: UserDefaults
+  /// The suite name for the underlying defaults container.
+  private let suiteName: String
+
   /// The key mapping to the object's associated resource in `defaults`.
   private let key: String
 
+  /// The underlying defaults container.
+  private var defaults: UserDefaults {
+    // It's safe to force unwrap the below defaults instance because the
+    // initializer only returns `nil` when the bundle id or `globalDomain`
+    // is passed in as the `suiteName`.
+    UserDefaults(suiteName: suiteName)!
+  }
+
   /// Designated initializer.
   /// - Parameters:
-  ///   - defaults: The defaults container.
+  ///   - suiteName: The suite name for the defaults container.
   ///   - key: The key mapping to the value stored in the defaults container.
-  init(defaults: UserDefaults, key: String) {
-    self.defaults = defaults
+  init(suiteName: String, key: String) {
+    self.suiteName = suiteName
     self.key = key
   }
 

FirebaseCore/Internal/Sources/HeartbeatLogging/StorageFactory.swift

@@ -56,11 +56,7 @@ extension FileManager {
 extension UserDefaultsStorage: StorageFactory {
   static func makeStorage(id: String) -> Storage {
     let suiteName = Constants.heartbeatUserDefaultsSuiteName
-    // It's safe to force unwrap the below defaults instance because the
-    // initializer only returns `nil` when the bundle id or `globalDomain`
-    // is passed in as the `suiteName`.
-    let defaults = UserDefaults(suiteName: suiteName)!
     let key = "heartbeats-\(id)"
-    return UserDefaultsStorage(defaults: defaults, key: key)
+    return UserDefaultsStorage(suiteName: suiteName, key: key)
   }
 }

FirebaseCore/Internal/Sources/HeartbeatLogging/_ObjC_HeartbeatController.swift

@@ -49,7 +49,7 @@ public class _ObjC_HeartbeatController: NSObject {
   ///
   /// - Note: This API is thread-safe.
   /// - Returns: A heartbeats payload for the flushed heartbeat(s).
-  public func flushAsync(completionHandler: @escaping (_ObjC_HeartbeatsPayload) -> Void) {
+  public func flushAsync(completionHandler: @escaping @Sendable (_ObjC_HeartbeatsPayload) -> Void) {
     // TODO: When minimum version moves to iOS 13.0, restore the async version
     // removed in #13952.
     heartbeatController.flushAsync { heartbeatsPayload in

FirebaseCore/Internal/Tests/Common/AdjustableDate.swift

@@ -0,0 +1,23 @@
+// Copyright 2024 Google LLC
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//      http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import Foundation
+
+/// Used to manipulate a date across multiple concurrent contexts for simulation purposes.
+final class AdjustableDate: @unchecked Sendable {
+  var date: Date
+  init(date: Date) {
+    self.date = date
+  }
+}

FirebaseCore/Internal/Tests/Integration/HeartbeatLoggingIntegrationTests.swift

@@ -232,8 +232,8 @@ class HeartbeatLoggingIntegrationTests: XCTestCase {
 
   func testLogRepeatedly_WithoutFlushing_LimitsOnWrite() throws {
     // Given
-    var testdate = date
-    let heartbeatController = HeartbeatController(id: #function, dateProvider: { testdate })
+    let testDate = AdjustableDate(date: date)
+    let heartbeatController = HeartbeatController(id: #function, dateProvider: { testDate.date })
 
     // When
     // Iterate over 35 days and log a heartbeat each day.
@@ -252,7 +252,7 @@ class HeartbeatLoggingIntegrationTests: XCTestCase {
         heartbeatController.log("dummy_agent_3")
       }
 
-      testdate.addTimeInterval(60 * 60 * 24) // Advance the test date by 1 day.
+      testDate.date.addTimeInterval(60 * 60 * 24) // Advance the test date by 1 day.
     }
 
     // Then

FirebaseCore/Internal/Tests/Unit/HeartbeatControllerTests.swift

@@ -99,11 +99,11 @@ class HeartbeatControllerTests: XCTestCase {
 
   func testLogAtEndOfTimePeriodAndAcceptAtStartOfNextOne() throws {
     // Given
-    var testDate = date
+    let testDate = AdjustableDate(date: date)
 
     let controller = HeartbeatController(
       storage: HeartbeatStorageFake(),
-      dateProvider: { testDate }
+      dateProvider: { testDate.date }
     )
 
     assertHeartbeatControllerFlushesEmptyPayload(controller)
@@ -113,12 +113,12 @@ class HeartbeatControllerTests: XCTestCase {
     controller.log("dummy_agent")
 
     // - Advance to 2021-11-01 @ 23:59:59 (EST)
-    testDate.addTimeInterval(60 * 60 * 24 - 1)
+    testDate.date.addTimeInterval(60 * 60 * 24 - 1)
 
     controller.log("dummy_agent")
 
     // - Advance to 2021-11-02 @ 00:00:00 (EST)
-    testDate.addTimeInterval(1)
+    testDate.date.addTimeInterval(1)
 
     controller.log("dummy_agent")
 
@@ -271,18 +271,18 @@ class HeartbeatControllerTests: XCTestCase {
       ).date // 2021-11-02 @ 11 PM (Tokyo time zone)
     )
 
-    var testDate = newYorkDate
+    let testDate = AdjustableDate(date: newYorkDate)
 
     let heartbeatController = HeartbeatController(
       storage: HeartbeatStorageFake(),
-      dateProvider: { testDate }
+      dateProvider: { testDate.date }
     )
 
     // When
     heartbeatController.log("dummy_agent")
 
     // Device travels from NYC to Tokyo.
-    testDate = tokyoDate
+    testDate.date = tokyoDate
 
     heartbeatController.log("dummy_agent")
 
@@ -308,22 +308,22 @@ class HeartbeatControllerTests: XCTestCase {
 
   func testLoggingDependsOnDateNotUserAgent() throws {
     // Given
-    var testDate = date
+    let testDate = AdjustableDate(date: date)
     let heartbeatController = HeartbeatController(
       storage: HeartbeatStorageFake(),
-      dateProvider: { testDate }
+      dateProvider: { testDate.date }
     )
 
     // When
     // - Day 1
     heartbeatController.log("dummy_agent")
 
     // - Day 2
-    testDate.addTimeInterval(60 * 60 * 24)
+    testDate.date.addTimeInterval(60 * 60 * 24)
     heartbeatController.log("some_other_agent")
 
     // - Day 3
-    testDate.addTimeInterval(60 * 60 * 24)
+    testDate.date.addTimeInterval(60 * 60 * 24)
     heartbeatController.log("dummy_agent")
 
     // Then
@@ -359,20 +359,20 @@ class HeartbeatControllerTests: XCTestCase {
     let todaysDate = date
     let tomorrowsDate = date.addingTimeInterval(60 * 60 * 24)
 
-    var testDate = yesterdaysDate
+    let testDate = AdjustableDate(date: yesterdaysDate)
 
     let heartbeatController = HeartbeatController(
       storage: HeartbeatStorageFake(),
-      dateProvider: { testDate }
+      dateProvider: { testDate.date }
     )
 
     // When
     heartbeatController.log("yesterdays_dummy_agent")
-    testDate = todaysDate
+    testDate.date = todaysDate
     heartbeatController.log("todays_dummy_agent")
-    testDate = tomorrowsDate
+    testDate.date = tomorrowsDate
     heartbeatController.log("tomorrows_dummy_agent")
-    testDate = todaysDate
+    testDate.date = todaysDate
 
     // Then
     let payload = heartbeatController.flushHeartbeatFromToday()
@@ -426,7 +426,10 @@ class HeartbeatControllerTests: XCTestCase {
 
 // MARK: - Fakes
 
-private class HeartbeatStorageFake: HeartbeatStorageProtocol {
+private final class HeartbeatStorageFake: HeartbeatStorageProtocol, @unchecked Sendable {
+  // The unchecked Sendable conformance is used to prevent warnings for the below var, which
+  // violates the class's Sendable conformance. Ignoring this violation should be okay for
+  // testing purposes.
   private var heartbeatsBundle: HeartbeatsBundle?
 
   func readAndWriteSync(using transform: (HeartbeatsBundle?) -> HeartbeatsBundle?) {