diff --git a/Apps/CocoaPods-FCM/Podfile b/Apps/CocoaPods-FCM/Podfile index 2ee0d74a0..e4ce5c836 100644 --- a/Apps/CocoaPods-FCM/Podfile +++ b/Apps/CocoaPods-FCM/Podfile @@ -28,4 +28,6 @@ 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 diff --git a/Apps/CocoaPods-FCM/Rich Push Notification Service Extension/NotificationService.swift b/Apps/CocoaPods-FCM/Rich Push Notification Service Extension/NotificationService.swift index add92b7fd..2ecae7f6b 100644 --- a/Apps/CocoaPods-FCM/Rich Push Notification Service Extension/NotificationService.swift +++ b/Apps/CocoaPods-FCM/Rich Push Notification Service Extension/NotificationService.swift @@ -1,14 +1,26 @@ import CioMessagingPushFCM import CioTracking +import SampleAppsCommon import UserNotifications class NotificationService: UNNotificationServiceExtension { override func didReceive(_ request: UNNotificationRequest, withContentHandler contentHandler: @escaping (UNNotificationContent) -> Void) { - // 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 + // 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) } + // Then, send the request to the Customer.io SDK for processing. MessagingPush.shared.didReceive(request, withContentHandler: contentHandler) } diff --git a/Apps/CocoaPods-FCM/src/AppDelegate.swift b/Apps/CocoaPods-FCM/src/AppDelegate.swift index a0b45d3b3..3aa52a75a 100644 --- a/Apps/CocoaPods-FCM/src/AppDelegate.swift +++ b/Apps/CocoaPods-FCM/src/AppDelegate.swift @@ -16,6 +16,8 @@ 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 @@ -58,6 +60,22 @@ 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 { diff --git a/Apps/Common/Source/Model/CioSettings.swift b/Apps/Common/Source/Model/CioSettings.swift index ba56b6188..cb47bb49f 100644 --- a/Apps/Common/Source/Model/CioSettings.swift +++ b/Apps/Common/Source/Model/CioSettings.swift @@ -25,6 +25,15 @@ 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! diff --git a/Sources/Common/DIGraph.swift b/Sources/Common/DIGraph.swift index cba95ba79..f52d8a234 100644 --- a/Sources/Common/DIGraph.swift +++ b/Sources/Common/DIGraph.swift @@ -22,6 +22,10 @@ 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? { diff --git a/Sources/Common/Util/Lock.swift b/Sources/Common/Util/Lock.swift index 984a460ad..cf163ba74 100644 --- a/Sources/Common/Util/Lock.swift +++ b/Sources/Common/Util/Lock.swift @@ -16,11 +16,11 @@ public class Lock { private init() {} - func lock() { + public func lock() { _lock.lock() } - func unlock() { + public func unlock() { _lock.unlock() } } diff --git a/Sources/Tracking/CustomerIO.swift b/Sources/Tracking/CustomerIO.swift index e38abf942..63ee4eb3b 100644 --- a/Sources/Tracking/CustomerIO.swift +++ b/Sources/Tracking/CustomerIO.swift @@ -108,6 +108,11 @@ 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. @@ -117,9 +122,6 @@ 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() {} @@ -143,9 +145,7 @@ public class CustomerIO: CustomerIOInstance { internal static func initializeIntegrationTests( diGraph: DIGraph ) { - let implementation = CustomerIOImplementation(diGraph: diGraph) - Self.shared = CustomerIO(implementation: implementation, diGraph: diGraph) - Self.shared.postInitialize(diGraph: diGraph) + Self.shared.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph)) } /** @@ -165,7 +165,7 @@ public class CustomerIO: CustomerIOInstance { configureHandler(&newSdkConfig) } - Self.initialize(config: newSdkConfig) + Self.shared.commonInitialize(config: newSdkConfig) if newSdkConfig.autoTrackScreenViews { // Setting up screen view tracking is not available for rich push (Notification Service Extension). @@ -192,44 +192,51 @@ public class CustomerIO: CustomerIOInstance { configureHandler(&newSdkConfig) } - Self.initialize(config: newSdkConfig.toSdkConfig()) + Self.shared.commonInitialize(config: newSdkConfig.toSdkConfig()) } - // private shared logic initialize to avoid copy/paste between the different - // public initialize functions. - private static func initialize( - config: SdkConfig - ) { + // convenience method that takes a new config object + internal func commonInitialize(config: SdkConfig) { let newDiGraph = DIGraph(sdkConfig: config) - Self.shared.diGraph = newDiGraph - Self.shared.implementation = CustomerIOImplementation(diGraph: newDiGraph) - - Self.shared.postInitialize(diGraph: newDiGraph) + commonInitialize(newDiGraph: newDiGraph, newImplementation: CustomerIOImplementation(diGraph: newDiGraph)) } - // 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 + // 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). + + // 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() - cleanupRepository = diGraph.cleanupRepository + diGraph = newDiGraph + implementation = newImplementation + + let hooks = newDiGraph.hooksManager + let threadUtil = newDiGraph.threadUtil + let logger = newDiGraph.logger + let siteId = newDiGraph.sdkConfig.siteId + + let cleanupRepository = newDiGraph.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 = diGraph.globalDataStore + let globalDataStore = newDiGraph.globalDataStore if let token = globalDataStore.pushDeviceToken { registerDeviceToken(token) } // run cleanup in background to prevent locking the UI thread - threadUtil.runBackground { [weak self] in - self?.cleanupRepository?.cleanup() + 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() } logger @@ -396,4 +403,4 @@ public class CustomerIO: CustomerIOInstance { ) { implementation?.trackMetric(deliveryID: deliveryID, event: event, deviceToken: deviceToken) } -} +} // swiftlint:disable:this file_length diff --git a/Tests/MessagingInApp/MessagingInAppIntegrationTest.swift b/Tests/MessagingInApp/MessagingInAppIntegrationTest.swift index 07b13a83a..fd2c519a6 100644 --- a/Tests/MessagingInApp/MessagingInAppIntegrationTest.swift +++ b/Tests/MessagingInApp/MessagingInAppIntegrationTest.swift @@ -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() { - tearDown() // re-initialize with different configuration options. + uninitializeSDK() // re-initialize with different configuration options. CustomerIO.initialize(siteId: testSiteId, apiKey: .random, region: .EU) { $0.logLevel = .debug diff --git a/Tests/Shared/Stub/ThreadUtilStub.swift b/Tests/Shared/Stub/ThreadUtilStub.swift index 9b931938e..95c1ec44f 100644 --- a/Tests/Shared/Stub/ThreadUtilStub.swift +++ b/Tests/Shared/Stub/ThreadUtilStub.swift @@ -2,11 +2,22 @@ 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() } } diff --git a/Tests/Tracking/CustomerIOIntegrationTests.swift b/Tests/Tracking/CustomerIOIntegrationTests.swift index d9a55aedd..3d9ae26df 100644 --- a/Tests/Tracking/CustomerIOIntegrationTests.swift +++ b/Tests/Tracking/CustomerIOIntegrationTests.swift @@ -156,4 +156,45 @@ 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) + } } diff --git a/Tests/Tracking/CustomerIOTest.swift b/Tests/Tracking/CustomerIOTest.swift index 4461425ef..b7f21d051 100644 --- a/Tests/Tracking/CustomerIOTest.swift +++ b/Tests/Tracking/CustomerIOTest.swift @@ -23,7 +23,7 @@ class CustomerIOTest: UnitTest { } func test_initialize_expectAddModuleHooks_expectRunCleanup() { - customerIO.postInitialize(diGraph: diGraph) + customerIO.commonInitialize(newDiGraph: diGraph, newImplementation: CustomerIOImplementation(diGraph: diGraph)) XCTAssertEqual(hooksMock.addCallsCount, 1) XCTAssertEqual(hooksMock.addReceivedArguments?.key, .tracking)