Skip to content

Commit

Permalink
Fix dispatchSource (#1344)
Browse files Browse the repository at this point in the history
* Fix Constraints Error in `PayPalWebCheckoutViewController` (#1342)

* fix constraints warning by adding leading and trailing constraints for nested stackViews
* remove print statement

* Add RepeatingTimer class

* Setup default timeInterval value on RepeatingTimer

* Refactor BTAnalyticsService

* Add RepeatingTimer Tests

* Move RepeatingTimer files

* Fix UT

* Rename UTs

* Remove default value on RepeatingTimer init

* Address PR comment

* Fix tests

* Address PR comments

* Update CHANGELOG

* Update docstring

* Remove unnecessary test

* PR Feedback - move RepeatingTimer into Analytics/ directory

* Add Pragma marks to RepeatingTimer

---------

Co-authored-by: Jax DesMarais-Leder <[email protected]>
Co-authored-by: Sammy Cannillo <[email protected]>
  • Loading branch information
3 people authored Jul 2, 2024
1 parent 80e324c commit 5b14e06
Show file tree
Hide file tree
Showing 6 changed files with 161 additions and 42 deletions.
10 changes: 9 additions & 1 deletion Braintree.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@
428F976626727333001042E1 /* BTMockOpenURLContext.m in Sources */ = {isa = PBXBuildFile; fileRef = 428F976526727333001042E1 /* BTMockOpenURLContext.m */; };
42FC218B25CDE0290047C49A /* BTPayPalRequest_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42FC218A25CDE0290047C49A /* BTPayPalRequest_Tests.swift */; };
42FC237125CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 42FC237025CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift */; };
457D7FC82C29CEC300EF6523 /* RepeatingTimer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */; };
457D7FCA2C2A250E00EF6523 /* RepeatingTimer_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */; };
460C0C220F594AE8EE205E57 /* Pods_Tests_BraintreeCoreTests.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 9239C9FE850C3587DE61A3A2 /* Pods_Tests_BraintreeCoreTests.framework */; };
5708E0A628809AD9007946B9 /* BTJSON.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5708E0A528809AD9007946B9 /* BTJSON.swift */; };
5708E0A828809BC6007946B9 /* BTJSONError.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5708E0A728809BC6007946B9 /* BTJSONError.swift */; };
Expand Down Expand Up @@ -772,6 +774,8 @@
42F75E5A24D48138007DC5E7 /* BTThreeDSecureClient_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTThreeDSecureClient_Tests.swift; sourceTree = "<group>"; };
42FC218A25CDE0290047C49A /* BTPayPalRequest_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalRequest_Tests.swift; sourceTree = "<group>"; };
42FC237025CE0E110047C49A /* BTPayPalCheckoutRequest_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTPayPalCheckoutRequest_Tests.swift; sourceTree = "<group>"; };
457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RepeatingTimer.swift; sourceTree = "<group>"; };
457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RepeatingTimer_Tests.swift; sourceTree = "<group>"; };
463DED22C0F426A474E6D7E2 /* Pods-Tests-BraintreeCoreTests.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests-BraintreeCoreTests.release.xcconfig"; path = "Target Support Files/Pods-Tests-BraintreeCoreTests/Pods-Tests-BraintreeCoreTests.release.xcconfig"; sourceTree = "<group>"; };
541AEE40A1F01913E0638CC9 /* Pods-Tests-BraintreeCoreTests.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Tests-BraintreeCoreTests.debug.xcconfig"; path = "Target Support Files/Pods-Tests-BraintreeCoreTests/Pods-Tests-BraintreeCoreTests.debug.xcconfig"; sourceTree = "<group>"; };
5708E0A528809AD9007946B9 /* BTJSON.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTJSON.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1489,8 +1493,9 @@
isa = PBXGroup;
children = (
BEE2E4A628FDB64400C03FDD /* BTAnalyticsService_Tests.swift */,
8053F05329FAB6B00076F988 /* FPTIBatchData_Tests.swift */,
BEDA919F28EDDE64007441D9 /* FakeAnalyticsService.swift */,
8053F05329FAB6B00076F988 /* FPTIBatchData_Tests.swift */,
457D7FC92C2A250E00EF6523 /* RepeatingTimer_Tests.swift */,
);
path = Analytics;
sourceTree = "<group>";
Expand Down Expand Up @@ -1556,6 +1561,7 @@
BEE2E4E5290080BD00C03FDD /* BTAnalyticsServiceError.swift */,
BEF388C02BE52CD2000965C8 /* BTCoreAnalytics.swift */,
800E78C329E0DD5300D1B0FC /* FPTIBatchData.swift */,
457D7FC72C29CEC300EF6523 /* RepeatingTimer.swift */,
);
path = Analytics;
sourceTree = "<group>";
Expand Down Expand Up @@ -3245,6 +3251,7 @@
804DC45D2B2D08FF00F17A15 /* BTConfigurationRequest.swift in Sources */,
BED00CB228A57AD400D74AEC /* BTClientTokenError.swift in Sources */,
BE24C67328E73E810067B11A /* BTAPIClientHTTPType.swift in Sources */,
457D7FC82C29CEC300EF6523 /* RepeatingTimer.swift in Sources */,
BE9EC0982899CF040022EC63 /* BTAPIPinnedCertificates.swift in Sources */,
BEF1089029019CAE003B2FDA /* BTAnalyticsService.swift in Sources */,
BEF388C32BE9340C000965C8 /* BTAPITimingDelegate.swift in Sources */,
Expand Down Expand Up @@ -3564,6 +3571,7 @@
A9E5C23424FD6FAE00EE691F /* BTAPIClient_Tests.swift in Sources */,
8053F05729FAD4790076F988 /* Encodable+Dictionary_Tests.swift in Sources */,
BEE2E4A728FDB64400C03FDD /* BTAnalyticsService_Tests.swift in Sources */,
457D7FCA2C2A250E00EF6523 /* RepeatingTimer_Tests.swift in Sources */,
A95229C424FD8EEE006F7D25 /* BTURLUtils_Tests.swift in Sources */,
A908436324FD82C0004134CA /* BTBinData_Tests.swift in Sources */,
A908436224FD82A9004134CA /* BTAppContextSwitcher_Tests.swift in Sources */,
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* BraintreeCore
* For analytics, only call `fetchOrReturnRemoteConfig()` when batch uploading, not on each analytic event enqueue
* For analytics, add additional metrics on networking timing
* Fix bug causing random crashes in `BTAnalyticsService`
* BraintreePayPalNativeCheckout (DEPRECATED)
* **Note:** This module is deprecated and will be removed in a future version of the SDK
* Add deprecated warning message to all public classes and methods
Expand Down
55 changes: 18 additions & 37 deletions Sources/BraintreeCore/Analytics/BTAnalyticsService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,35 @@ class BTAnalyticsService: Equatable {
var shouldBypassTimerQueue = false

// MARK: - Private Properties

private static let events = BTAnalyticsEventsStorage()

private let apiClient: BTAPIClient

/// Amount of time, in seconds, between batch API requests sent to FPTI
private let timerInterval = 15
private static let timeInterval = 15

private static let timer = RepeatingTimer(timeInterval: timeInterval)

private static var timer: DispatchSourceTimer?
private let apiClient: BTAPIClient

// MARK: - Initializer

init(apiClient: BTAPIClient) {
self.apiClient = apiClient
self.http = BTHTTP(authorization: apiClient.authorization, customBaseURL: Self.url)

Self.timer.eventHandler = { [weak self] in
guard let self else { return }
Task {
await self.sendQueuedAnalyticsEvents()
}
}
Self.timer.resume()
}

// MARK: - Deinit

deinit {
BTAnalyticsService.timer?.cancel()
BTAnalyticsService.timer = nil
Self.timer.suspend()
}

// MARK: - Internal Methods
Expand Down Expand Up @@ -111,36 +120,8 @@ class BTAnalyticsService: Equatable {

await BTAnalyticsService.events.append(event)

// TODO: - Refactor to make HTTP non-optional property and instantiate in init()
if self.http == nil {
self.http = BTHTTP(authorization: self.apiClient.authorization, customBaseURL: BTAnalyticsService.url)
}

// A special value passed in by unit tests to prevent BTHTTP from actually posting
if let http = self.http, http.customBaseURL?.absoluteString == "test://do-not-send.url" {
return
}

if shouldBypassTimerQueue {
await self.sendQueuedAnalyticsEvents()
return
}

if BTAnalyticsService.timer == nil {
BTAnalyticsService.timer = DispatchSource.makeTimerSource(queue: self.http?.dispatchQueue)
BTAnalyticsService.timer?.schedule(
deadline: .now() + .seconds(self.timerInterval),
repeating: .seconds(self.timerInterval),
leeway: .seconds(1)
)

BTAnalyticsService.timer?.setEventHandler {
Task {
await self.sendQueuedAnalyticsEvents()
}
}

BTAnalyticsService.timer?.resume()
}
}

Expand All @@ -150,9 +131,9 @@ class BTAnalyticsService: Equatable {
if await !BTAnalyticsService.events.isEmpty {
do {
let configuration = try await apiClient.fetchConfiguration()
let postParameters = await createAnalyticsEvent(config: configuration, sessionID: apiClient.metadata.sessionID, events: BTAnalyticsService.events.allValues)
let postParameters = await createAnalyticsEvent(config: configuration, sessionID: apiClient.metadata.sessionID, events: Self.events.allValues)
http?.post("v1/tracking/batch/events", parameters: postParameters) { _, _, _ in }
await BTAnalyticsService.events.removeAll()
await Self.events.removeAll()
} catch {
return
}
Expand Down
77 changes: 77 additions & 0 deletions Sources/BraintreeCore/Analytics/RepeatingTimer.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
import Foundation

final class RepeatingTimer {

// MARK: - Private Properties

/// Amount of time, in seconds.
private let timeInterval: Int

private lazy var timer: DispatchSourceTimer = {
let timerSource = DispatchSource.makeTimerSource(queue: DispatchQueue.main)
timerSource.schedule(
deadline: .now() + .seconds(timeInterval),
repeating: .seconds(timeInterval),
leeway: .seconds(1)
)
timerSource.setEventHandler { [weak self] in
self?.eventHandler?()
}
return timerSource
}()

private var state: State = .suspended

private enum State {
case suspended
case resumed
}

// MARK: - Internal Properties

var eventHandler: (() -> Void)?

// MARK: - Initializer

init(timeInterval: Int) {
self.timeInterval = timeInterval
}

deinit {
timer.setEventHandler { }
timer.cancel()
// If the timer is suspended, calling cancel without resuming afterwards
// triggers a crash. This is documented here https://forums.developer.apple.com/thread/15902
resume()
eventHandler = nil
}

// MARK: - Internal Methods

/*
GCD timers are sensitive to errors. It is crucial to maintain
balance between calls to `dispatch_suspend` and `dispatch_resume`. Failure to do so
results in crashes with errors similar to:

BUG IN CLIENT OF LIBDISPATCH: Over-resume of an object

Such errors indicate an attempt to resume an already resumed timer. According to the
documentation, each call to `dispatch_suspend` must be matched with a corresponding
call to `dispatch_resume` to ensure proper event delivery and avoid crashes.
For more information: https://developer.apple.com/library/archive/documentation/General/Conceptual/ConcurrencyProgrammingGuide/GCDWorkQueues/GCDWorkQueues.html#//apple_ref/doc/uid/TP40008091-CH103-SW8
*/

func resume() {
guard state != .resumed else { return }

state = .resumed
timer.resume()
}

func suspend() {
guard state != .suspended else { return }

state = .suspended
timer.suspend()
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ final class BTAnalyticsService_Tests: XCTestCase {

XCTAssertEqual(mockAnalyticsHTTP.lastRequestEndpoint, "v1/tracking/batch/events")

let timestamp = self.parseTimestamp(mockAnalyticsHTTP.lastRequestParameters)!
let eventName = self.parseEventName(mockAnalyticsHTTP.lastRequestParameters)
let timestamp = parseTimestamp(mockAnalyticsHTTP.lastRequestParameters)!
let eventName = parseEventName(mockAnalyticsHTTP.lastRequestParameters)

XCTAssertEqual(eventName!, "any.analytics.event")
XCTAssertGreaterThanOrEqual(timestamp, self.currentTime)
XCTAssertLessThanOrEqual(timestamp, self.oneSecondLater)
XCTAssertGreaterThanOrEqual(timestamp, currentTime)
XCTAssertLessThanOrEqual(timestamp, oneSecondLater)
XCTAssertEqual(mockAnalyticsHTTP.POSTRequestCount, 1)

self.validateMetadataParameters(mockAnalyticsHTTP.lastRequestParameters)
Expand Down
52 changes: 52 additions & 0 deletions UnitTests/BraintreeCoreTests/Analytics/RepeatingTimer_Tests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import XCTest
@testable import BraintreeCore

class RepeatingTimerTests: XCTestCase {

func testResume_callsEventHandler() {
let sut = RepeatingTimer(timeInterval: 2)
let expectation = expectation(description: "Timer should fire")
var handlerCalled = false

sut.eventHandler = {
handlerCalled = true
expectation.fulfill()
}

sut.resume()
waitForExpectations(timeout: 3, handler: nil)
XCTAssertTrue(handlerCalled, "Event handler should be called after timer resumes")
}

func testSuspend_preventsEventHandler() {
let sut = RepeatingTimer(timeInterval: 2)
let expectation = expectation(description: "Timer should not fire after suspension")
var handlerCalled = false

sut.eventHandler = {
handlerCalled = true
}

sut.resume()
sut.suspend()

DispatchQueue.main.asyncAfter(deadline: .now() + 3) {
expectation.fulfill()
}

waitForExpectations(timeout: 4, handler: nil)
XCTAssertFalse(handlerCalled, "Event handler should not be called after timer is suspended")
}

func testDeinit() {
var sut: RepeatingTimer? = RepeatingTimer(timeInterval: 1)
sut?.resume()

addTeardownBlock {
sut?.suspend()
}

sut = nil
XCTAssertNil(sut, "Timer should be deallocated")
}
}

0 comments on commit 5b14e06

Please sign in to comment.