diff --git a/Sources/Common/Util/LockManager.swift b/Sources/Common/Util/LockManager.swift index 85fc2614b..9a29ac660 100644 --- a/Sources/Common/Util/LockManager.swift +++ b/Sources/Common/Util/LockManager.swift @@ -32,6 +32,7 @@ public class LockManager { public enum LockReference: String { case queueStorage case pushHistory + case autoTrackScreenViewStore } // Dependency needs to be accessed by multiple graphs. diff --git a/Sources/DataPipeline/Plugins/AutoTrackingScreenViews.swift b/Sources/DataPipeline/Plugins/AutoTrackingScreenViews.swift index 31c6696ea..9804d2e13 100644 --- a/Sources/DataPipeline/Plugins/AutoTrackingScreenViews.swift +++ b/Sources/DataPipeline/Plugins/AutoTrackingScreenViews.swift @@ -12,7 +12,13 @@ public class AutoTrackingScreenViews: UtilityPlugin { public let type = PluginType.utility public var analytics: Segment.Analytics? - public var diGraph = DIGraphShared.shared + public var diGraph: DIGraphShared { + DIGraphShared.shared + } + + private var store: AutoTrackingScreenViewStore { + diGraph.autoTrackingScreenViewStore + } static let notificationName = Notification.Name(rawValue: "AutoTrackingScreenViewsNotification") static let screenNameKey = "name" @@ -36,7 +42,6 @@ public class AutoTrackingScreenViews: UtilityPlugin { public init( filterAutoScreenViewEvents: ((UIViewController) -> Bool)? = nil, - autoScreenViewBody: (() -> [String: Any])? = nil ) { self.filterAutoScreenViewEvents = filterAutoScreenViewEvents @@ -92,9 +97,9 @@ extension AutoTrackingScreenViews { } let filter = customerOverridenFilter ?? defaultSdkFilter - let shouldTrackEvent = filter(viewController) + let screenPassesFilter = filter(viewController) - guard shouldTrackEvent else { + guard screenPassesFilter else { let isUsingSdkDefaultFilter = customerOverridenFilter == nil diGraph.logger.debug( "automatic screenview ignored for, \(name):\(viewController.bundleIdOfView ?? ""). It was filtered out. Is using sdk default filter: \(isUsingSdkDefaultFilter)" @@ -102,6 +107,13 @@ extension AutoTrackingScreenViews { return // event has been filtered out. Ignore it. } + // Because of the automatic screenview tracking implementation, it's possible to see 2+ screenview events get tracked for 1 screen when the screen is viewed. To prevent this duplication, we only track the event if the screen the user is looking at has changed. + let isLastScreenTracked = store.isScreenLastScreenTracked(screenName: name) + guard !isLastScreenTracked else { + diGraph.logger.debug("automatic screenview ignored for, \(name):\(viewController.bundleIdOfView ?? ""). It was already tracked.") + return + } + let addionalScreenViewData = autoScreenViewBody?() ?? [:] analytics?.screen(title: name, properties: addionalScreenViewData) } @@ -211,3 +223,40 @@ extension UIViewController { return nil } } + +// Store for the automatic screenview feature. +protocol AutoTrackingScreenViewStore { + /* + Returns true if `screenName` is equal to the `screenName` the last time this function was called. + Use this return value to determine if the screenview event has already been tracked or not. + + This function will save `screenName` as the last screen tracked to be used the next time this function is called. + To make the store's thread-safety easier, all getting/setting in the store is encapsulated in 1 function call. + */ + func isScreenLastScreenTracked(screenName: String) -> Bool +} + +// in-memory store because the data stored is not relevant when the app is started. We purposely are not persisting the store's data. +// +// sourcery: InjectRegisterShared = "AutoTrackingScreenViewStore" +// sourcery: InjectSingleton +class InMemoryAutoTrackingScreenViewStore: AutoTrackingScreenViewStore { + let lock: Lock + + private var lastScreenTracked: String? + + init(lockManager: LockManager) { + self.lock = lockManager.getLock(id: .autoTrackScreenViewStore) + } + + func isScreenLastScreenTracked(screenName: String) -> Bool { + lock.lock() + defer { self.lock.unlock() } + + let isLastScreenTracked = lastScreenTracked == screenName + + lastScreenTracked = screenName + + return isLastScreenTracked + } +} diff --git a/Sources/DataPipeline/autogenerated/AutoDependencyInjection.generated.swift b/Sources/DataPipeline/autogenerated/AutoDependencyInjection.generated.swift index 42ea7cdde..d3bb4a9c6 100644 --- a/Sources/DataPipeline/autogenerated/AutoDependencyInjection.generated.swift +++ b/Sources/DataPipeline/autogenerated/AutoDependencyInjection.generated.swift @@ -78,6 +78,9 @@ extension DIGraphShared { func testDependenciesAbleToResolve() -> Int { var countDependenciesResolved = 0 + _ = autoTrackingScreenViewStore + countDependenciesResolved += 1 + _ = deviceAttributesProvider countDependenciesResolved += 1 @@ -85,6 +88,30 @@ extension DIGraphShared { } // Handle classes annotated with InjectRegisterShared + // AutoTrackingScreenViewStore (singleton) + var autoTrackingScreenViewStore: AutoTrackingScreenViewStore { + getOverriddenInstance() ?? + sharedAutoTrackingScreenViewStore + } + + var sharedAutoTrackingScreenViewStore: AutoTrackingScreenViewStore { + // Use a DispatchQueue to make singleton thread safe. You must create unique dispatchqueues instead of using 1 shared one or you will get a crash when trying + // to call DispatchQueue.sync{} while already inside another DispatchQueue.sync{} call. + DispatchQueue(label: "DIGraphShared_AutoTrackingScreenViewStore_singleton_access").sync { + if let overridenDep: AutoTrackingScreenViewStore = getOverriddenInstance() { + return overridenDep + } + let existingSingletonInstance = self.singletons[String(describing: AutoTrackingScreenViewStore.self)] as? AutoTrackingScreenViewStore + let instance = existingSingletonInstance ?? _get_autoTrackingScreenViewStore() + self.singletons[String(describing: AutoTrackingScreenViewStore.self)] = instance + return instance + } + } + + private func _get_autoTrackingScreenViewStore() -> AutoTrackingScreenViewStore { + InMemoryAutoTrackingScreenViewStore(lockManager: lockManager) + } + // DeviceAttributesProvider var deviceAttributesProvider: DeviceAttributesProvider { getOverriddenInstance() ?? diff --git a/Tests/DataPipeline/DataPipelineImplementation+ScreenViewsTest.swift b/Tests/DataPipeline/DataPipelineImplementation+ScreenViewsTest.swift index 55dd7a3ab..ed3a4c557 100644 --- a/Tests/DataPipeline/DataPipelineImplementation+ScreenViewsTest.swift +++ b/Tests/DataPipeline/DataPipelineImplementation+ScreenViewsTest.swift @@ -15,7 +15,11 @@ class DataPipelineImplementationScreenViewsTest: IntegrationTest { // setting up required plugins outputReader = (CustomerIO.shared.add(plugin: OutputReaderPlugin()) as! OutputReaderPlugin) - autoTrackingScreenViews = (CustomerIO.shared.add(plugin: AutoTrackingScreenViews()) as! AutoTrackingScreenViews) + autoTrackingScreenViews = getTrackingScreenViewsPlugin() + } + + private func getTrackingScreenViewsPlugin() -> AutoTrackingScreenViews { + (CustomerIO.shared.add(plugin: AutoTrackingScreenViews()) as! AutoTrackingScreenViews) } // MARK: performScreenTracking @@ -56,6 +60,53 @@ class DataPipelineImplementationScreenViewsTest: IntegrationTest { assertEventTracked() } + func test_performScreenTracking_givenViewSameScreenMultipleTimes_expectNoTrackingDuplicateEvents() { + class ViewInsideOfHostApp: UIViewController {} + class AnotherViewInsideOfHostApp: UIViewController {} + + // The first time that the screen is tracked, an event should be added + autoTrackingScreenViews.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 1) + + // If the screen is tracked again, ignore the event. + autoTrackingScreenViews.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 1) + + // Check that an event is added, if the next screen is not equal to the last screen tracked. + autoTrackingScreenViews.performScreenTracking(onViewController: AnotherViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 2) + } + + func test_performScreenTracking_givenChangeScreen_expectTrackNonDuplicateScreens() { + class ViewInsideOfHostApp: UIViewController {} + class AnotherViewInsideOfHostApp: UIViewController {} + + // The first time that the screen is tracked, an event should be added + autoTrackingScreenViews.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 1) + + // Change to a different screen, expect to track it. + autoTrackingScreenViews.performScreenTracking(onViewController: AnotherViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 2) + + // Re-visit the first screen again, expect to track it. + autoTrackingScreenViews.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 3) + } + + func test_performScreenTracking_givenMultiplePluginInstances_expectNoTrackingDuplicateEvents() { + class ViewInsideOfHostApp: UIViewController {} + + let plugin1 = getTrackingScreenViewsPlugin() + let plugin2 = getTrackingScreenViewsPlugin() + + plugin1.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 1) + + plugin2.performScreenTracking(onViewController: ViewInsideOfHostApp()) + assertEventTracked(numberOfEventsAdded: 1) + } + // MARK: getNameForAutomaticScreenViewTracking func test_getNameForAutomaticScreenViewTracking_givenViewWithNoTitle_expectNil() {