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

Fix bug where AnalyticsServices fires same event multiple times #1210

Merged
merged 11 commits into from
Mar 11, 2024
4 changes: 3 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,13 +1,15 @@
# Braintree iOS SDK Release Notes

## unreleased
* Send `paypal_context_id` in `batch_params` to PayPal's analytics service (FPTI) when available
* BraintreeVenmo
* Add `isFinalAmount` to `BTVenmoRequest`
* Add `BTVenmoRequest.fallbackToWeb`
* If set to `true` customers will fallback to a web based Venmo flow if the Venmo app is not installed
* This method uses Universal Links instead of URL Schemes
* BraintreeCore
* Send `paypal_context_id` in `batch_params` to PayPal's analytics service (FPTI) when available
* Send `link_type` in `event_params` to PayPal's analytics service (FPTI)
* Fix bug where FPTI analytic events were being sent multiple times

## 6.12.0 (2024-01-18)
* BraintreePayPal
Expand Down
158 changes: 35 additions & 123 deletions Sources/BraintreeCore/Analytics/BTAnalyticsService.swift
Original file line number Diff line number Diff line change
@@ -1,33 +1,11 @@
import Foundation

/// Encapsulates analytics events for a given session
struct BTAnalyticsSession {

let sessionID: String
var events: [FPTIBatchData.Event] = []

init(with sessionID: String) {
self.sessionID = sessionID
}
}

class BTAnalyticsService: Equatable {

// MARK: - Internal Properties

/// A serial dispatch queue that synchronizes access to `analyticsSessions`
let sessionsQueue: DispatchQueue = DispatchQueue(label: "com.braintreepayments.BTAnalyticsService")

/// The HTTP client for communication with the analytics service endpoint. Exposed for testing.
var http: BTHTTP?

/// Defaults to 1, can be overridden
var flushThreshold: Int

/// Dictionary of analytics sessions, keyed by session ID. The analytics service requires that batched events
/// are sent from only one session. In practice, BTAPIClient.metadata.sessionID should never change, so this
/// is defensive.
var analyticsSessions: [String: BTAnalyticsSession] = [:]

/// The FPTI URL to post all analytic events.
static let url = URL(string: "https://api-m.paypal.com")!
Expand All @@ -38,65 +16,58 @@ class BTAnalyticsService: Equatable {

// MARK: - Initializer

init(apiClient: BTAPIClient, flushThreshold: Int = 1) {
init(apiClient: BTAPIClient) {
self.apiClient = apiClient
self.flushThreshold = flushThreshold
}

// MARK: - Internal Methods

/// Tracks an event.
///
/// Events are queued and sent in batches to the analytics service, based on the status of the app and
/// the number of queued events. After exiting this method, there is no guarantee that the event has been sent.
/// - Parameter eventName: String representing the event name

/// Sends analytics event to https://api.paypal.com/v1/tracking/batch/events/ via a background task.
/// - Parameters:
/// - eventName: Name of analytic event.
/// - errorDescription: Optional. Full error description returned to merchant.
/// - correlationID: Optional. CorrelationID associated with the checkout session.
/// - payPalContextID: Optional. PayPal Context ID associated with the checkout session.
/// - linkType: Optional. The type of link the SDK will be handling, currently deeplink or universal.
func sendAnalyticsEvent(
_ eventName: String,
errorDescription: String? = nil,
correlationID: String? = nil,
payPalContextID: String? = nil,
linkType: String? = nil
) {
DispatchQueue.main.async {
self.payPalContextID = payPalContextID
self.enqueueEvent(
Task(priority: .background) {
await performEventRequest(
eventName,
errorDescription: errorDescription,
correlationID: correlationID,
linkType: linkType
linkType: linkType,
payPalContextID: payPalContextID
)
self.flushIfAtThreshold()
}
}

/// Sends request to FPTI immediately, without checking number of events in queue against flush threshold
func sendAnalyticsEvent(
/// Exposed to be able to execute this function synchronously in unit tests
func performEventRequest(
_ eventName: String,
errorDescription: String? = nil,
correlationID: String? = nil,
payPalContextID: String? = nil,
linkType: String? = nil,
completion: @escaping (Error?) -> Void = { _ in }
) {
DispatchQueue.main.async {
self.payPalContextID = payPalContextID
self.enqueueEvent(
eventName,
errorDescription: errorDescription,
correlationID: correlationID,
linkType: linkType
)
self.flush(completion)
}
}

/// Executes API request to FPTI
func flush(_ completion: @escaping (Error?) -> Void = { _ in }) {
payPalContextID: String? = nil
) async {
self.payPalContextID = payPalContextID

let timestampInMilliseconds = UInt64(Date().timeIntervalSince1970 * 1000)
let event = FPTIBatchData.Event(
correlationID: correlationID,
errorDescription: errorDescription,
eventName: eventName,
linkType: linkType,
timestamp: String(timestampInMilliseconds)
)

apiClient.fetchOrReturnRemoteConfiguration { configuration, error in
guard let configuration, error == nil else {
if let error {
completion(error)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This completion was always ignored, so we removed it entirely

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Curious why this is happening, thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTAPIClient was always no-oping the result of this completion. I don't think we would ever want to return an error to the merchant and stop the flow if there is an issue with analytics since that's just for us, so this is safe for us to remove since we just discarded the result.

}
return
}

Expand All @@ -107,81 +78,24 @@ class BTAnalyticsService: Equatable {
} else if let tokenizationKey = self.apiClient.tokenizationKey {
self.http = BTHTTP(url: BTAnalyticsService.url, tokenizationKey: tokenizationKey)
} else {
completion(BTAnalyticsServiceError.invalidAPIClient)
return
}
}

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

self.sessionsQueue.async {
if self.analyticsSessions.count == 0 {
completion(nil)
return
}

self.analyticsSessions.keys.forEach { sessionID in
let postParameters = self.createAnalyticsEvent(config: configuration, sessionID: sessionID)
self.http?.post("v1/tracking/batch/events", parameters: postParameters) { body, response, error in
if let error {
completion(error)
}
}
}
completion(nil)
}
let postParameters = self.createAnalyticsEvent(config: configuration, sessionID: self.apiClient.metadata.sessionID, event: event)
self.http?.post("v1/tracking/batch/events", parameters: postParameters) { _, _, _ in }
}
}

// MARK: - Helpers

/// Adds an event to the queue
func enqueueEvent(
_ eventName: String,
errorDescription: String?,
correlationID: String?,
linkType: String?
) {
let timestampInMilliseconds = UInt64(Date().timeIntervalSince1970 * 1000)
let event = FPTIBatchData.Event(
correlationID: correlationID,
errorDescription: errorDescription,
eventName: eventName,
linkType: linkType,
timestamp: String(timestampInMilliseconds)
)
let session = BTAnalyticsSession(with: apiClient.metadata.sessionID)

sessionsQueue.async {
if self.analyticsSessions[session.sessionID] == nil {
self.analyticsSessions[session.sessionID] = session
}

self.analyticsSessions[session.sessionID]?.events.append(event)
}
}

/// Checks queued event count to determine if ready to fire API request
func flushIfAtThreshold() {
var eventCount = 0

sessionsQueue.sync {
analyticsSessions.values.forEach { analyticsSession in
eventCount += analyticsSession.events.count
}
}

if eventCount >= flushThreshold {
flush()
}
}

/// Constructs POST params to be sent to FPTI from the queued events in the session
func createAnalyticsEvent(config: BTConfiguration, sessionID: String) -> Codable {
/// Constructs POST params to be sent to FPTI
func createAnalyticsEvent(config: BTConfiguration, sessionID: String, event: FPTIBatchData.Event) -> Codable {
let batchMetadata = FPTIBatchData.Metadata(
authorizationFingerprint: apiClient.clientToken?.authorizationFingerprint,
environment: config.fptiEnvironment,
Expand All @@ -192,14 +106,12 @@ class BTAnalyticsService: Equatable {
tokenizationKey: apiClient.tokenizationKey
)

let session = self.analyticsSessions[sessionID]

return FPTIBatchData(metadata: batchMetadata, events: session?.events)
return FPTIBatchData(metadata: batchMetadata, events: [event])
}

// MARK: Equitable Protocol Conformance

static func == (lhs: BTAnalyticsService, rhs: BTAnalyticsService) -> Bool {
lhs.http == rhs.http && lhs.flushThreshold == rhs.flushThreshold && lhs.apiClient == rhs.apiClient
lhs.http == rhs.http && lhs.apiClient == rhs.apiClient
}
}
5 changes: 2 additions & 3 deletions Sources/BraintreeCore/BTAPIClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import Foundation
self.metadata = BTClientMetadata()

super.init()
BTAPIClient._analyticsService = BTAnalyticsService(apiClient: self, flushThreshold: 5)
BTAPIClient._analyticsService = BTAnalyticsService(apiClient: self)
guard let authorizationType: BTAPIClientAuthorization = Self.authorizationType(forAuthorization: authorization) else { return nil }

let errorString = BTLogLevelDescription.string(for: .error)
Expand Down Expand Up @@ -351,8 +351,7 @@ import Foundation
errorDescription: errorDescription,
correlationID: correlationID,
payPalContextID: payPalContextID,
linkType: linkType,
completion: { _ in }
linkType: linkType
)
}

Expand Down
Loading
Loading