Skip to content

Commit

Permalink
Bugfix - send analytics even when BTAPIClient deallocated (#1379)
Browse files Browse the repository at this point in the history 
- Move `BTAnalyticsService` from "faux" singleton setup in `BTAPIClient`, to actual singleton
    - Use protocol abstraction to be able to mock BTAnalyticsService in our tests
    - Make `RepeatingTimer` and `AnalyticsEventStorage `properties non-static
- Refactor `BTAnalyticsService.sendAnalyticsEvent()` method to require single `FPTIBatchData.Event` property vs 12 individual properties

---------
Signed-off-by: Jax DesMarais-Leder <[email protected]>
Signed-off-by: Rich Herrera <[email protected]>
  • Loading branch information
@scannillo
scannillo authored Jul 31, 2024
1 parent 824efa4 commit abdd1e9
Show file tree
Hide file tree
Showing 10 changed files with 123 additions and 162 deletions.
4 changes: 4 additions & 0 deletions Braintree.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,7 @@
8075CBEE2B1B735200CA6265 /* BTAPIRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8075CBED2B1B735200CA6265 /* BTAPIRequest.swift */; };
80842DA72B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy in Resources */ = {isa = PBXBuildFile; fileRef = 80842DA62B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy */; };
8087C10F2BFBACCA0020FC2E /* TokenizationKey_Tests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 8087C10E2BFBACCA0020FC2E /* TokenizationKey_Tests.swift */; };
808E4A162C581CD40006A737 /* AnalyticsSendable.swift in Sources */ = {isa = PBXBuildFile; fileRef = 808E4A152C581CD40006A737 /* AnalyticsSendable.swift */; };
80A6C6192B21205900416D50 /* UIApplication+URLOpener.swift in Sources */ = {isa = PBXBuildFile; fileRef = 80A6C6182B21205900416D50 /* UIApplication+URLOpener.swift */; };
80AB424F2B27CAC200249218 /* BraintreeTestShared.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = A903E1A624F9D34000C314E1 /* BraintreeTestShared.framework */; platformFilter = ios; };
80AB42542B27DF5B00249218 /* BraintreeCore.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 570B93AC285397520041BAFE /* BraintreeCore.framework */; platformFilter = ios; };
Expand Down Expand Up @@ -860,6 +861,7 @@
8075CBED2B1B735200CA6265 /* BTAPIRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTAPIRequest.swift; sourceTree = "<group>"; };
80842DA62B8E49EF00A5CD92 /* PrivacyInfo.xcprivacy */ = {isa = PBXFileReference; lastKnownFileType = text.xml; path = PrivacyInfo.xcprivacy; sourceTree = "<group>"; };
8087C10E2BFBACCA0020FC2E /* TokenizationKey_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TokenizationKey_Tests.swift; sourceTree = "<group>"; };
808E4A152C581CD40006A737 /* AnalyticsSendable.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AnalyticsSendable.swift; sourceTree = "<group>"; };
80A1EE3D2236AAC600F6218B /* BTThreeDSecureAdditionalInformation_Tests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BTThreeDSecureAdditionalInformation_Tests.swift; sourceTree = "<group>"; };
80A6C6182B21205900416D50 /* UIApplication+URLOpener.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "UIApplication+URLOpener.swift"; sourceTree = "<group>"; };
80AD35F12BFBB1DD00BF890E /* TokenizationKeyError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TokenizationKeyError.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1591,6 +1593,7 @@
80F4F4D529F8A628003EACB1 /* Analytics */ = {
isa = PBXGroup;
children = (
808E4A152C581CD40006A737 /* AnalyticsSendable.swift */,
BE549F132BF6576300B6F441 /* BTAnalyticsEventsStorage.swift */,
BEE2E4E329007FF100C03FDD /* BTAnalyticsService.swift */,
BEE2E4E5290080BD00C03FDD /* BTAnalyticsServiceError.swift */,
Expand Down Expand Up @@ -3318,6 +3321,7 @@
574891E9286F7D300020DA36 /* BTClientMetadataSource.swift in Sources */,
BED7493628579BAC0074C818 /* BTURLUtils.swift in Sources */,
BEBD05222A1FE8BE0003C15C /* BTWebAuthenticationSession.swift in Sources */,
808E4A162C581CD40006A737 /* AnalyticsSendable.swift in Sources */,
BE698EA028AA8DCB001D9B10 /* BTHTTP.swift in Sources */,
BC17F9B428D23C5C004B18CC /* BTGraphQLMultiErrorNode.swift in Sources */,
5708E0A828809BC6007946B9 /* BTJSONError.swift in Sources */,
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Braintree iOS SDK Release Notes

## unreleased
* BraintreeCore
* Fix bug where some analytics wouldn't send if `BTAPIClient` instantiated on button click

## 6.23.2 (2024-07-30)
* BraintreePayPal
* Fix bug where `BTPayPalCheckoutRequest` was not passing the correct data causing issues with some transaction attempts
Expand Down
7 changes: 7 additions & 0 deletions Sources/BraintreeCore/Analytics/AnalyticsSendable.swift
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
/// Describes a class that batches and sends analytics events.
/// - Note: Specifically created to mock the `BTAnalyticsService` singleton.
protocol AnalyticsSendable: AnyObject {

func sendAnalyticsEvent(_ event: FPTIBatchData.Event)
func setAPIClient(_ apiClient: BTAPIClient)
}
125 changes: 30 additions & 95 deletions Sources/BraintreeCore/Analytics/BTAnalyticsService.swift
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import Foundation

class BTAnalyticsService: Equatable {
final class BTAnalyticsService: AnalyticsSendable {

// MARK: - Internal Properties

static let shared = BTAnalyticsService()

// swiftlint:disable force_unwrapping
/// The FPTI URL to post all analytic events.
Expand All @@ -17,156 +19,89 @@ class BTAnalyticsService: Equatable {

// MARK: - Private Properties

private static let events = BTAnalyticsEventsStorage()
private let events = BTAnalyticsEventsStorage()

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

private static let timer = RepeatingTimer(timeInterval: timeInterval)
private let timer = RepeatingTimer(timeInterval: timeInterval)

private let apiClient: BTAPIClient

private weak var apiClient: BTAPIClient?
// MARK: - Initializer

init(apiClient: BTAPIClient) {

private init() { }

/// Used to inject `BTAPIClient` dependency into `BTAnalyticsService` singleton
func setAPIClient(_ apiClient: BTAPIClient) {
self.apiClient = apiClient
self.http = BTHTTP(authorization: apiClient.authorization, customBaseURL: Self.url)

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

timer.resume()
}

// MARK: - Deinit

deinit {
Self.timer.suspend()
timer.suspend()
}

// MARK: - Internal Methods

/// Sends analytics event to https://api.paypal.com/v1/tracking/batch/events/ via a background task.
/// - Parameters:
/// - eventName: Name of analytic event.
/// - correlationID: Optional. CorrelationID associated with the checkout session.
/// - endpoint: Optional. The endpoint of the API request send during networking requests.
/// - endTime: Optional. The end time of the roundtrip networking request.
/// - errorDescription: Optional. Full error description returned to merchant.
/// - isVaultRequest: Optional. If the Venmo or PayPal request is being vaulted.
/// - linkType: Optional. The type of link the SDK will be handling, currently deeplink or universal.
/// - payPalContextID: Optional. PayPal Context ID associated with the checkout session.
/// - startTime: Optional. The start time of the networking request.
func sendAnalyticsEvent(
_ eventName: String,
connectionStartTime: Int? = nil,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
isConfigFromCache: Bool? = nil,
isVaultRequest: Bool? = nil,
linkType: String? = nil,
payPalContextID: String? = nil,
requestStartTime: Int? = nil,
startTime: Int? = nil
) {
/// - Parameter event: A single `FPTIBatchData.Event`
func sendAnalyticsEvent(_ event: FPTIBatchData.Event) {
Task(priority: .background) {
await performEventRequest(
eventName,
connectionStartTime: connectionStartTime,
correlationID: correlationID,
endpoint: endpoint,
endTime: endTime,
errorDescription: errorDescription,
isConfigFromCache: isConfigFromCache,
isVaultRequest: isVaultRequest,
linkType: linkType,
payPalContextID: payPalContextID,
requestStartTime: requestStartTime,
startTime: startTime
)
await performEventRequest(with: event)
}
}

/// Exposed to be able to execute this function synchronously in unit tests
func performEventRequest(
_ eventName: String,
connectionStartTime: Int? = nil,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
isConfigFromCache: Bool? = nil,
isVaultRequest: Bool? = nil,
linkType: String? = nil,
payPalContextID: String? = nil,
requestStartTime: Int? = nil,
startTime: Int? = nil
) async {
let timestampInMilliseconds = Date().utcTimestampMilliseconds
let event = FPTIBatchData.Event(
connectionStartTime: connectionStartTime,
correlationID: correlationID,
endpoint: endpoint,
endTime: endTime,
errorDescription: errorDescription,
eventName: eventName,
isConfigFromCache: isConfigFromCache,
isVaultRequest: isVaultRequest,
linkType: linkType,
payPalContextID: payPalContextID,
requestStartTime: requestStartTime,
startTime: startTime,
timestamp: String(timestampInMilliseconds)
)

await BTAnalyticsService.events.append(event)
func performEventRequest(with event: FPTIBatchData.Event) async {
await events.append(event)

if shouldBypassTimerQueue {
await self.sendQueuedAnalyticsEvents()
}
}

// MARK: - Helpers
// MARK: - Private Methods

func sendQueuedAnalyticsEvents() async {
if await !BTAnalyticsService.events.isEmpty {
private func sendQueuedAnalyticsEvents() async {
if await !events.isEmpty, let apiClient {
do {
let configuration = try await apiClient.fetchConfiguration()
let postParameters = await createAnalyticsEvent(
config: configuration,
sessionID: apiClient.metadata.sessionID,
events: Self.events.allValues
events: events.allValues
)
http?.post("v1/tracking/batch/events", parameters: postParameters) { _, _, _ in }
await Self.events.removeAll()
await events.removeAll()
} catch {
return
}
}
}

/// Constructs POST params to be sent to FPTI
func createAnalyticsEvent(config: BTConfiguration, sessionID: String, events: [FPTIBatchData.Event]) -> Codable {
private func createAnalyticsEvent(config: BTConfiguration, sessionID: String, events: [FPTIBatchData.Event]) -> Codable {
let batchMetadata = FPTIBatchData.Metadata(
authorizationFingerprint: apiClient.authorization.type == .clientToken ? apiClient.authorization.bearer : nil,
authorizationFingerprint: apiClient?.authorization.type == .clientToken ? apiClient?.authorization.bearer : nil,
environment: config.fptiEnvironment,
integrationType: apiClient.metadata.integration.stringValue,
integrationType: apiClient?.metadata.integration.stringValue ?? BTClientMetadataIntegration.custom.stringValue,
merchantID: config.merchantID,
sessionID: sessionID,
tokenizationKey: apiClient.authorization.type == .tokenizationKey ? apiClient.authorization.originalValue : nil
tokenizationKey: apiClient?.authorization.type == .tokenizationKey ? apiClient?.authorization.originalValue : nil
)

return FPTIBatchData(metadata: batchMetadata, events: events)
}

// MARK: Equitable Protocol Conformance

static func == (lhs: BTAnalyticsService, rhs: BTAnalyticsService) -> Bool {
lhs.http == rhs.http && lhs.apiClient == rhs.apiClient
}
}
30 changes: 29 additions & 1 deletion Sources/BraintreeCore/Analytics/FPTIBatchData.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,37 @@ struct FPTIBatchData: Codable {
let requestStartTime: Int?
/// UTC millisecond timestamp when a networking task initiated.
let startTime: Int?
let timestamp: String
let timestamp = String(Date().utcTimestampMilliseconds)
let tenantName: String = "Braintree"
let venmoInstalled: Bool = application.isVenmoAppInstalled()

init(
connectionStartTime: Int? = nil,
correlationID: String? = nil,
endpoint: String? = nil,
endTime: Int? = nil,
errorDescription: String? = nil,
eventName: String,
isConfigFromCache: Bool? = nil,
isVaultRequest: Bool? = nil,
linkType: String? = nil,
payPalContextID: String? = nil,
requestStartTime: Int? = nil,
startTime: Int? = nil
) {
self.connectionStartTime = connectionStartTime
self.correlationID = correlationID
self.endpoint = endpoint
self.endTime = endTime
self.errorDescription = errorDescription
self.eventName = eventName
self.isConfigFromCache = isConfigFromCache
self.isVaultRequest = isVaultRequest
self.linkType = linkType
self.payPalContextID = payPalContextID
self.requestStartTime = requestStartTime
self.startTime = startTime
}

enum CodingKeys: String, CodingKey {
case connectionStartTime = "connect_start_time"
Expand Down
47 changes: 22 additions & 25 deletions Sources/BraintreeCore/BTAPIClient.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,7 @@ import Foundation
var configurationLoader: ConfigurationLoader

/// Exposed for testing analytics
/// By default, the `BTAnalyticsService` instance is static/shared so that only one queue of events exists.
/// The "singleton" is managed here because the analytics service depends on `BTAPIClient`.
weak var analyticsService: BTAnalyticsService? {
get { BTAPIClient._analyticsService }
set { BTAPIClient._analyticsService = newValue }
}

private static var _analyticsService: BTAnalyticsService?
var analyticsService: AnalyticsSendable = BTAnalyticsService.shared

// MARK: - Initializers

Expand Down Expand Up @@ -65,7 +58,7 @@ import Foundation
configurationLoader = ConfigurationLoader(http: btHttp)

super.init()
BTAPIClient._analyticsService = BTAnalyticsService(apiClient: self)
analyticsService.setAPIClient(self)
http?.networkTimingDelegate = self

// Kickoff the background request to fetch the config
Expand Down Expand Up @@ -320,14 +313,16 @@ import Foundation
linkType: String? = nil,
payPalContextID: String? = nil
) {
analyticsService?.sendAnalyticsEvent(
eventName,
correlationID: correlationID,
errorDescription: errorDescription,
isConfigFromCache: isConfigFromCache,
isVaultRequest: isVaultRequest,
linkType: linkType,
payPalContextID: payPalContextID
analyticsService.sendAnalyticsEvent(
FPTIBatchData.Event(
correlationID: correlationID,
errorDescription: errorDescription,
eventName: eventName,
isConfigFromCache: isConfigFromCache,
isVaultRequest: isVaultRequest,
linkType: linkType,
payPalContextID: payPalContextID
)
)
}

Expand Down Expand Up @@ -409,15 +404,17 @@ import Foundation
with: "payment_methods/three_d_secure",
options: .regularExpression
)

if cleanedPath != "/v1/tracking/batch/events" {
analyticsService?.sendAnalyticsEvent(
BTCoreAnalytics.apiRequestLatency,
connectionStartTime: connectionStartTime,
endpoint: cleanedPath,
endTime: endTime,
requestStartTime: requestStartTime,
startTime: startTime
analyticsService.sendAnalyticsEvent(
FPTIBatchData.Event(
connectionStartTime: connectionStartTime,
endpoint: cleanedPath,
endTime: endTime,
eventName: BTCoreAnalytics.apiRequestLatency,
requestStartTime: requestStartTime,
startTime: startTime
)
)
}
}
Expand Down
18 changes: 10 additions & 8 deletions UnitTests/BraintreeCoreTests/Analytics/BTAnalyticsService_Tests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,25 @@ final class BTAnalyticsService_Tests: XCTestCase {

func testSendAnalyticsEvent_whenConfigFetchCompletes_setsUpAnalyticsHTTPToUseBaseURL() async {
let stubAPIClient: MockAPIClient = stubbedAPIClientWithAnalyticsURL("test://do-not-send.url")
let analyticsService = BTAnalyticsService(apiClient: stubAPIClient)
let sut = BTAnalyticsService.shared
sut.setAPIClient(stubAPIClient)

await analyticsService.performEventRequest("any.analytics.event")
await sut.performEventRequest(with: FPTIBatchData.Event(eventName: "any.analytics.event"))

XCTAssertEqual(analyticsService.http?.customBaseURL?.absoluteString, "https://api.paypal.com")
XCTAssertEqual(sut.http?.customBaseURL?.absoluteString, "https://api.paypal.com")
}

func testSendAnalyticsEvent_sendsAnalyticsEvent() async {
let stubAPIClient: MockAPIClient = stubbedAPIClientWithAnalyticsURL("test://do-not-send.url")
let mockAnalyticsHTTP = FakeHTTP.fakeHTTP()
let analyticsService = BTAnalyticsService(apiClient: stubAPIClient)
analyticsService.shouldBypassTimerQueue = true
let sut = BTAnalyticsService.shared
sut.setAPIClient(stubAPIClient)
sut.shouldBypassTimerQueue = true

analyticsService.http = mockAnalyticsHTTP

await analyticsService.performEventRequest("any.analytics.event")
sut.http = mockAnalyticsHTTP

await sut.performEventRequest(with: FPTIBatchData.Event(eventName: "any.analytics.event"))

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

let timestamp = parseTimestamp(mockAnalyticsHTTP.lastRequestParameters)!
Expand Down
Loading

0 comments on commit abdd1e9

Please sign in to comment.