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: apps initializing sdk multiple times in short amount of time may make SDK ignore requests #360

Merged
merged 1 commit into from
Jul 21, 2023
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions Apps/CocoaPods-FCM/Podfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,4 @@ target 'Rich Push Notification Service Extension' do
pod 'CustomerIOTracking', :path => sdk_local_path
pod 'CustomerIOMessagingPush', :path => sdk_local_path
pod 'CustomerIOMessagingPushFCM', :path => sdk_local_path

pod 'SampleAppsCommon', :path => File.join(File.dirname(__FILE__), '../Common')
end
Original file line number Diff line number Diff line change
@@ -1,26 +1,14 @@
import CioMessagingPushFCM
import CioTracking
import SampleAppsCommon
import UserNotifications

class NotificationService: UNNotificationServiceExtension {
override func didReceive(_ request: UNNotificationRequest, withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) {
// This code is internal to Customer.io for testing purposes. Your app will be unique
// in how you decide to provide the siteId, apiKey, and region to the SDK.
let appSetSettings = CioSettingsManager().appSetSettings
let siteId = appSetSettings?.siteId ?? BuildEnvironment.CustomerIO.siteId
let apiKey = appSetSettings?.apiKey ?? BuildEnvironment.CustomerIO.apiKey

// You must initialize the Customer.io SDK inside of the Notification Service.
CustomerIO.initialize(siteId: siteId, apiKey: apiKey, region: .US) { config in
// Modify properties in the config object to configure the Customer.io SDK.
// config.logLevel = .debug // Uncomment this line to enable debug logging.

// This line of code is internal to Customer.io for testing purposes. Do not add this code to your app.
appSetSettings?.configureCioSdk(config: &config)
// we are only using this sample app for testing it can compile so providing a siteid and apikey is not useful at the moment.
CustomerIO.initialize(siteId: "", apiKey: "", region: .US) { config in
config.logLevel = .debug
}

// Then, send the request to the Customer.io SDK for processing.
MessagingPush.shared.didReceive(request, withContentHandler: contentHandler)
}

Expand Down
18 changes: 0 additions & 18 deletions Apps/CocoaPods-FCM/src/AppDelegate.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ class AppDelegate: NSObject, UIApplicationDelegate {
// Initialize the Firebase SDK.
FirebaseApp.configure()

// This code is internal to Customer.io for testing purposes. Your app will be unique
// in how you decide to provide the siteId, apiKey, and region to the SDK.
let appSetSettings = CioSettingsManager().appSetSettings
let siteId = appSetSettings?.siteId ?? BuildEnvironment.CustomerIO.siteId
let apiKey = appSetSettings?.apiKey ?? BuildEnvironment.CustomerIO.apiKey
Expand Down Expand Up @@ -60,22 +58,6 @@ extension AppDelegate: UNUserNotificationCenterDelegate {
) {
completionHandler([.list, .banner, .badge, .sound])
}

func userNotificationCenter(_ center: UNUserNotificationCenter, didReceive response: UNNotificationResponse, withCompletionHandler completionHandler: @escaping () -> Void) {
// Send Customer.io SDK click event to process. This enables features such as
// push metrics and deep links.
let handled = MessagingPush.shared.userNotificationCenter(
center,
didReceive: response,
withCompletionHandler: completionHandler
)

// If the Customer.io SDK does not handle the push, it's up to you to handle it and call the
// completion handler. If the SDK did handle it, it called the completion handler for you.
if !handled {
completionHandler()
}
}
}

extension AppDelegate: InAppEventListener {
Expand Down
9 changes: 0 additions & 9 deletions Apps/Common/Source/Model/CioSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,6 @@ public struct CioSettings: Codable {
}
}

public func configureCioSdk(config: inout CioNotificationServiceExtensionSdkConfig) {
config.trackingApiUrl = trackUrl
config.autoTrackDeviceAttributes = trackDeviceAttributes

if debugSdkMode {
config.logLevel = .debug
}
}

public static func getFromCioSdk() -> CioSettings {
let sdkConfig = CustomerIO.shared.config!

Expand Down
4 changes: 0 additions & 4 deletions Sources/Common/DIGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,6 @@ public class DIGraph {
overrides[String(describing: type)] = value
}

public func resetOverride(forType type: Any.Type) {
overrides[String(describing: type)] = nil
}

// Retrieves an overridden instance of a specified type from the `overrides` dictionary.
// If an overridden instance exists and can be cast to the specified type, it is returned; otherwise, nil is returned.
public func getOverriddenInstance<T: Any>() -> T? {
Expand Down
4 changes: 2 additions & 2 deletions Sources/Common/Util/Lock.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,11 @@ public class Lock {

private init() {}

public func lock() {
func lock() {
_lock.lock()
}

public func unlock() {
func unlock() {
_lock.unlock()
}
}
63 changes: 28 additions & 35 deletions Sources/Tracking/CustomerIO.swift
Original file line number Diff line number Diff line change
Expand Up @@ -108,11 +108,6 @@ public class CustomerIO: CustomerIOInstance {
*/
@Atomic public private(set) static var shared = CustomerIO()

// mutex to safely initialize the SDK multiple times in short amount of time.
// Not using LockManager because 'shared' instance will keep a singleton instance of this lock
// through entire lifecycle of SDK.
@Atomic private var initializeLock = Lock.unsafeInit()

// Only assign a value to this *when the SDK is initialzied*.
// It's assumed that if this instance is not-nil, the SDK has been initialized.
// Tip: Use `SdkInitializedUtil` in modules to see if the SDK has been initialized and get data it needs.
Expand All @@ -122,6 +117,9 @@ public class CustomerIO: CustomerIOInstance {
// Exposed for `SdkInitializedUtil`. Not recommended to use this property directly.
internal var diGraph: DIGraph?

// strong reference to repository to prevent garbage collection as it runs tasks in async.
private var cleanupRepository: CleanupRepository?

// private constructor to force use of singleton API
private init() {}

Expand All @@ -145,7 +143,9 @@ public class CustomerIO: CustomerIOInstance {
internal static func initializeIntegrationTests(
diGraph: DIGraph
) {
Self.shared.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph))
let implementation = CustomerIOImplementation(diGraph: diGraph)
Self.shared = CustomerIO(implementation: implementation, diGraph: diGraph)
Self.shared.postInitialize(diGraph: diGraph)
}

/**
Expand All @@ -165,7 +165,7 @@ public class CustomerIO: CustomerIOInstance {
configureHandler(&newSdkConfig)
}

Self.shared.commonInitialize(config: newSdkConfig)
Self.initialize(config: newSdkConfig)

if newSdkConfig.autoTrackScreenViews {
// Setting up screen view tracking is not available for rich push (Notification Service Extension).
Expand All @@ -192,51 +192,44 @@ public class CustomerIO: CustomerIOInstance {
configureHandler(&newSdkConfig)
}

Self.shared.commonInitialize(config: newSdkConfig.toSdkConfig())
Self.initialize(config: newSdkConfig.toSdkConfig())
}

// convenience method that takes a new config object
internal func commonInitialize(config: SdkConfig) {
// private shared logic initialize to avoid copy/paste between the different
// public initialize functions.
private static func initialize(
config: SdkConfig
) {
let newDiGraph = DIGraph(sdkConfig: config)

commonInitialize(newDiGraph: newDiGraph, newImplementation: CustomerIOImplementation(diGraph: newDiGraph))
}

// Contains all logic shared between all of the initialize() functions, including integration tests.
internal func commonInitialize(newDiGraph: DIGraph, newImplementation: CustomerIOImplementation) {
// SDK initialization could be called multiple times within a close amount of time (example: app launch in a SDK wrapper app).
Self.shared.diGraph = newDiGraph
Self.shared.implementation = CustomerIOImplementation(diGraph: newDiGraph)

// lock before setting new property values such as diGraph. Unlock after it's safe to re-set dependency graph if SDK is being initialized multiple times.
initializeLock.lock()

diGraph = newDiGraph
implementation = newImplementation
Self.shared.postInitialize(diGraph: newDiGraph)
}

let hooks = newDiGraph.hooksManager
let threadUtil = newDiGraph.threadUtil
let logger = newDiGraph.logger
let siteId = newDiGraph.sdkConfig.siteId
// Contains all logic shared between all of the initialize() functions.
internal func postInitialize(diGraph: DIGraph) {
let hooks = diGraph.hooksManager
let threadUtil = diGraph.threadUtil
let logger = diGraph.logger
let siteId = diGraph.sdkConfig.siteId

let cleanupRepository = newDiGraph.cleanupRepository
cleanupRepository = diGraph.cleanupRepository

// Register Tracking module hooks now that the module is being initialized.
hooks.add(key: .tracking, provider: TrackingModuleHookProvider())

// Register the device token during SDK initialization to address device registration issues
// arising from lifecycle differences between wrapper SDKs and native SDK.
let globalDataStore = newDiGraph.globalDataStore
let globalDataStore = diGraph.globalDataStore
if let token = globalDataStore.pushDeviceToken {
registerDeviceToken(token)
}

// run cleanup in background to prevent locking the UI thread
threadUtil.runBackground {
// keep a strong reference to self and all dependencies in callback.
// memory access exceptions can occur if dependencies get deinitialized before the callback gets called.
cleanupRepository.cleanup()

// unlock only after it's safe for dependencies to be re-created in the SDK.
self.initializeLock.unlock()
threadUtil.runBackground { [weak self] in
self?.cleanupRepository?.cleanup()
}

logger
Expand Down Expand Up @@ -403,4 +396,4 @@ public class CustomerIO: CustomerIOInstance {
) {
implementation?.trackMetric(deliveryID: deliveryID, event: event, deviceToken: deviceToken)
}
} // swiftlint:disable:this file_length
}
2 changes: 1 addition & 1 deletion Tests/MessagingInApp/MessagingInAppIntegrationTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ class MessagingInAppIntegrationTests: IntegrationTest {
// Bug reported when: using in-app messaging, debug logging enabled, and using iOS version 1.2.8.
// Report: https://github.com/customerio/customerio-ios/issues/242
func test_initialize_enableDebugLogs_assertNotCrash() {
uninitializeSDK() // re-initialize with different configuration options.
tearDown() // re-initialize with different configuration options.

CustomerIO.initialize(siteId: testSiteId, apiKey: .random, region: .EU) {
$0.logLevel = .debug
Expand Down
11 changes: 0 additions & 11 deletions Tests/Shared/Stub/ThreadUtilStub.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,22 +2,11 @@ import CioInternalCommon
import Foundation

public class ThreadUtilStub: ThreadUtil {
@Atomic public var runMainCallsCount = 0
@Atomic public var runBackgroundCallsCount = 0

public var runCount: Int {
runMainCallsCount + runBackgroundCallsCount
}

public func runMain(_ block: @escaping () -> Void) {
runMainCallsCount += 1

block()
}

public func runBackground(_ block: @escaping () -> Void) {
runBackgroundCallsCount += 1

block()
}
}
41 changes: 0 additions & 41 deletions Tests/Tracking/CustomerIOIntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -156,45 +156,4 @@ class CustomerIOIntegrationTests: IntegrationTest {
XCTAssertEqual(CustomerIO.shared.config!.siteId, givenSiteId)
XCTAssertEqual(CustomerIO.shared.config!.apiKey, givenApiKey)
}

// Reproduces edge case where SDK is initialized multiple times in a short amount of time (such as during app launch in a SDK wrapper app).
// Learn more: https://github.com/customerio/opsbugs/issues/3618
func test_initializeSdk_givenCallMultipleTimes_expectNoExceptions() {
// It's very important that this test runs using real background threads to make SDK initialization async.

diGraph.resetOverride(forType: ThreadUtil.self)
let threadRunCountBeforeTest = threadUtilStub.runCount // test class may have already initialized the SDK once. Get count now to see if it's modified after test.

// 1000 is arbitrary. From running, exception is thrown consistently between index 100 and index 500.
for _ in 0 ..< 1000 {
CustomerIO.initialize(siteId: .random, apiKey: .random, region: .US, configure: nil)
}

// Assert that test executed using real background threads.
XCTAssertEqual(threadUtilStub.runCount, threadRunCountBeforeTest)
}

// The SDK initialization involves async operations. Assert that the async operations all exectute and do not get skipped with multiple requests.
func test_initializeSdk_givenCallMultipleTimes_expectFinishAsyncInitialize() {
let givenNumberOfInitCalls = 100
let expectCleanupMultipleTimes = expectation(description: "cleanup multiple times")
expectCleanupMultipleTimes.expectedFulfillmentCount = givenNumberOfInitCalls

// The cleanup repository is the async operation that executes during SDK initialization.
// Assert that cleanup executes as many times as the SDK is initialized.
let cleanupRepositoryMock = CleanupRepositoryMock()
cleanupRepositoryMock.cleanupClosure = {
Thread.sleep(forTimeInterval: 0.1) // sleep for 100 milliseconds to simulate async operation
expectCleanupMultipleTimes.fulfill()
}

diGraph.resetOverride(forType: ThreadUtil.self) // use real background threads to assert we are running async operations.
diGraph.override(value: cleanupRepositoryMock, forType: CleanupRepository.self)

for _ in 0 ..< givenNumberOfInitCalls {
CustomerIO.initializeIntegrationTests(diGraph: diGraph)
}

waitForExpectations(timeout: 3)
}
}
2 changes: 1 addition & 1 deletion Tests/Tracking/CustomerIOTest.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ class CustomerIOTest: UnitTest {
}

func test_initialize_expectAddModuleHooks_expectRunCleanup() {
customerIO.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph))
customerIO.postInitialize(diGraph: diGraph)

XCTAssertEqual(hooksMock.addCallsCount, 1)
XCTAssertEqual(hooksMock.addReceivedArguments?.key, .tracking)
Expand Down