Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 8 commits into from
Oct 30, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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
Expand All @@ -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)
}
Expand All @@ -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()) }
}
Expand All @@ -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.
Expand Down Expand Up @@ -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
)
}
Expand All @@ -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
)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
29 changes: 17 additions & 12 deletions FirebaseCore/Internal/Sources/HeartbeatLogging/Storage.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
Expand Down Expand Up @@ -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
)
Expand All @@ -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
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
23 changes: 23 additions & 0 deletions FirebaseCore/Internal/Tests/Common/AdjustableDate.swift
Original file line number Diff line number Diff line change
@@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
Expand Down
37 changes: 20 additions & 17 deletions FirebaseCore/Internal/Tests/Unit/HeartbeatControllerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")

Expand Down Expand Up @@ -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")

Expand All @@ -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
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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?) {
Expand Down
Loading
Loading