From aad7e22e460a42d670ec78348e090aeda5bae6e6 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Mon, 19 Feb 2024 11:05:03 -0800 Subject: [PATCH 1/4] bug: Set inForeground to true after a new session has been started if needed --- Sources/Amplitude/Amplitude.swift | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Sources/Amplitude/Amplitude.swift b/Sources/Amplitude/Amplitude.swift index 686c04a3..248eb0b6 100644 --- a/Sources/Amplitude/Amplitude.swift +++ b/Sources/Amplitude/Amplitude.swift @@ -319,12 +319,13 @@ public class Amplitude { } func onEnterForeground(timestamp: Int64) { - inForeground = true let dummySessionStartEvent = BaseEvent( timestamp: timestamp, eventType: Constants.AMP_SESSION_START_EVENT ) let events = sessions.processEvent(event: dummySessionStartEvent, inForeground: false) + // Set inForeground to true only after we have successfully started a new session if needed. + inForeground = true events.forEach { e in timeline.processEvent(event: e) } } From df2b20afac1d0006a604c676332569d4829d9d32 Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Mon, 4 Mar 2024 13:17:07 -0800 Subject: [PATCH 2/4] patch: Adding tests for session tracking --- Tests/AmplitudeTests/AmplitudeTests.swift | 37 +++++++++++++++++++ .../Supports/TestUtilities.swift | 9 +++++ 2 files changed, 46 insertions(+) diff --git a/Tests/AmplitudeTests/AmplitudeTests.swift b/Tests/AmplitudeTests/AmplitudeTests.swift index de7cc66f..ebb662c2 100644 --- a/Tests/AmplitudeTests/AmplitudeTests.swift +++ b/Tests/AmplitudeTests/AmplitudeTests.swift @@ -340,6 +340,43 @@ final class AmplitudeTests: XCTestCase { XCTAssertEqual(events[0].eventType, eventType) XCTAssertEqual(events[0].sessionId, -1) } + + func testEventProcessingBeforeOnEnterForeground() async { + let configuration = Configuration( + apiKey: "api-key", + storageProvider: storageMem, + identifyStorageProvider: interceptStorageMem, + defaultTracking: DefaultTrackingOptions(sessions: false) + ) + let amplitude = Amplitude(configuration: configuration) + amplitude.sessions = SessionsWithDelayedEventStartProcessing(amplitude: amplitude) + let timestamp = Int64(NSDate().timeIntervalSince1970 * 1000) + + let oneHourEarlierTimestamp = timestamp - (1 * 60 * 60 * 1000) + amplitude.setSessionId(timestamp: oneHourEarlierTimestamp) + + @Sendable + func processStartSessionEvent() async { + amplitude.onEnterForeground(timestamp: timestamp) + } + + func processRegularEvent() async { + amplitude.track(eventType: "test_event") + } + + // We process the session start event first. The session class will wait for 3 seconds before it processes + // the event + async let task = processStartSessionEvent(); + // Sleep for 1 second and process a regular event. This is to try the case where an event gets processed + // before the session start event + sleep(1) + await processRegularEvent(); + await task; + + // We want to make sure that a new session was started + XCTAssertTrue(amplitude.getSessionId() > oneHourEarlierTimestamp) + + } func testMigrationToApiKeyAndInstanceNameStorage() throws { let legacyUserId = "legacy-user-id" diff --git a/Tests/AmplitudeTests/Supports/TestUtilities.swift b/Tests/AmplitudeTests/Supports/TestUtilities.swift index d16bd923..3b2d09fa 100644 --- a/Tests/AmplitudeTests/Supports/TestUtilities.swift +++ b/Tests/AmplitudeTests/Supports/TestUtilities.swift @@ -289,3 +289,12 @@ final class MockPathCreation: PathCreationProtocol { subject.send(networkPath) } } + +class SessionsWithDelayedEventStartProcessing: Sessions { + override func processEvent(event: BaseEvent, inForeground: Bool) -> [BaseEvent] { + if (event.eventType == Constants.AMP_SESSION_START_EVENT) { + sleep(3) + } + return super.processEvent(event: event, inForeground: inForeground) + } +} From 2e972c6f46af1030e42da99d85b95f875d2b960e Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Mon, 4 Mar 2024 14:38:10 -0800 Subject: [PATCH 3/4] chore: Fixing lint issues --- Tests/AmplitudeTests/AmplitudeTests.swift | 265 ++++++++++++---------- 1 file changed, 141 insertions(+), 124 deletions(-) diff --git a/Tests/AmplitudeTests/AmplitudeTests.swift b/Tests/AmplitudeTests/AmplitudeTests.swift index ebb662c2..17e2fae6 100644 --- a/Tests/AmplitudeTests/AmplitudeTests.swift +++ b/Tests/AmplitudeTests/AmplitudeTests.swift @@ -1,5 +1,5 @@ -import XCTest import AnalyticsConnector +import XCTest @testable import AmplitudeSwift @@ -149,12 +149,13 @@ final class AmplitudeTests: XCTestCase { let apiKey = "testApiKeyPersist" storageTest = TestPersistentStorage(storagePrefix: "storage") interceptStorageTest = TestPersistentStorage(storagePrefix: "identify") - let amplitude = Amplitude(configuration: Configuration( - apiKey: apiKey, - storageProvider: storageTest, - identifyStorageProvider: interceptStorageTest, - defaultTracking: DefaultTrackingOptions.NONE - )) + let amplitude = Amplitude( + configuration: Configuration( + apiKey: apiKey, + storageProvider: storageTest, + identifyStorageProvider: interceptStorageTest, + defaultTracking: DefaultTrackingOptions.NONE + )) amplitude.setUserId(userId: "test-user") @@ -181,7 +182,8 @@ final class AmplitudeTests: XCTestCase { XCTAssertEqual(e1.eventType, "$identify") XCTAssertNil(e1.groups) XCTAssertNotNil(e1.userProperties) - XCTAssertTrue(getDictionary(e1.userProperties!).isEqual(to: ["$set": ["key-1": "value-1", "key-2": "value-2"]])) + XCTAssertTrue( + getDictionary(e1.userProperties!).isEqual(to: ["$set": ["key-1": "value-1", "key-2": "value-2"]])) let e2 = events[1] XCTAssertEqual(e2.eventType, "$identify") @@ -198,12 +200,13 @@ final class AmplitudeTests: XCTestCase { func testAnalyticsConnector() { let apiKey = "test-api-key" let instanceName = "test-instance" - let amplitude = Amplitude(configuration: Configuration( - apiKey: apiKey, - instanceName: instanceName, - storageProvider: storageMem, - identifyStorageProvider: interceptStorageMem - )) + let amplitude = Amplitude( + configuration: Configuration( + apiKey: apiKey, + instanceName: instanceName, + storageProvider: storageMem, + identifyStorageProvider: interceptStorageMem + )) let userId = "some-user" let deviceId = "some-device" @@ -211,14 +214,16 @@ final class AmplitudeTests: XCTestCase { var identitySet = false let connector = AnalyticsConnector.getInstance(instanceName) - connector.identityStore.addIdentityListener(key: "test-analytics-connector", { identity in - if identitySet { - XCTAssertEqual(identity.userId, userId) - XCTAssertEqual(identity.deviceId, deviceId) - XCTAssertEqual(identity.userProperties, ["prop-A": 123]) - expectation.fulfill() - } - }) + connector.identityStore.addIdentityListener( + key: "test-analytics-connector", + { identity in + if identitySet { + XCTAssertEqual(identity.userId, userId) + XCTAssertEqual(identity.deviceId, deviceId) + XCTAssertEqual(identity.userProperties, ["prop-A": 123]) + expectation.fulfill() + } + }) amplitude.setUserId(userId: userId) amplitude.setDeviceId(deviceId: deviceId) @@ -258,10 +263,12 @@ final class AmplitudeTests: XCTestCase { let events = storageMem.events() XCTAssertEqual(events.count, 1) XCTAssertEqual(events[0].eventType, Constants.AMP_DEEP_LINK_OPENED_EVENT) - XCTAssertEqual(getDictionary(events[0].eventProperties!), [ - Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com", - Constants.AMP_APP_LINK_REFERRER_PROPERTY: "https://test-referrer.com" - ]) + XCTAssertEqual( + getDictionary(events[0].eventProperties!), + [ + Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com", + Constants.AMP_APP_LINK_REFERRER_PROPERTY: "https://test-referrer.com", + ]) } func testTrackURLOpened() throws { @@ -279,9 +286,11 @@ final class AmplitudeTests: XCTestCase { let events = storageMem.events() XCTAssertEqual(events.count, 1) XCTAssertEqual(events[0].eventType, Constants.AMP_DEEP_LINK_OPENED_EVENT) - XCTAssertEqual(getDictionary(events[0].eventProperties!), [ - Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com" - ]) + XCTAssertEqual( + getDictionary(events[0].eventProperties!), + [ + Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com" + ]) } func testTrackNSURLOpened() throws { @@ -299,9 +308,11 @@ final class AmplitudeTests: XCTestCase { let events = storageMem.events() XCTAssertEqual(events.count, 1) XCTAssertEqual(events[0].eventType, Constants.AMP_DEEP_LINK_OPENED_EVENT) - XCTAssertEqual(getDictionary(events[0].eventProperties!), [ - Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com" - ]) + XCTAssertEqual( + getDictionary(events[0].eventProperties!), + [ + Constants.AMP_APP_LINK_URL_PROPERTY: "https://test-app.com" + ]) } func testTrackScreenView() throws { @@ -319,9 +330,11 @@ final class AmplitudeTests: XCTestCase { let events = storageMem.events() XCTAssertEqual(events.count, 1) XCTAssertEqual(events[0].eventType, Constants.AMP_SCREEN_VIEWED_EVENT) - XCTAssertEqual(getDictionary(events[0].eventProperties!), [ - Constants.AMP_APP_SCREEN_NAME_PROPERTY: "main view" - ]) + XCTAssertEqual( + getDictionary(events[0].eventProperties!), + [ + Constants.AMP_APP_SCREEN_NAME_PROPERTY: "main view" + ]) } func testOutOfSessionEvent() { @@ -340,7 +353,7 @@ final class AmplitudeTests: XCTestCase { XCTAssertEqual(events[0].eventType, eventType) XCTAssertEqual(events[0].sessionId, -1) } - + func testEventProcessingBeforeOnEnterForeground() async { let configuration = Configuration( apiKey: "api-key", @@ -351,7 +364,7 @@ final class AmplitudeTests: XCTestCase { let amplitude = Amplitude(configuration: configuration) amplitude.sessions = SessionsWithDelayedEventStartProcessing(amplitude: amplitude) let timestamp = Int64(NSDate().timeIntervalSince1970 * 1000) - + let oneHourEarlierTimestamp = timestamp - (1 * 60 * 60 * 1000) amplitude.setSessionId(timestamp: oneHourEarlierTimestamp) @@ -359,23 +372,23 @@ final class AmplitudeTests: XCTestCase { func processStartSessionEvent() async { amplitude.onEnterForeground(timestamp: timestamp) } - + func processRegularEvent() async { amplitude.track(eventType: "test_event") } - + // We process the session start event first. The session class will wait for 3 seconds before it processes // the event - async let task = processStartSessionEvent(); + async let task = processStartSessionEvent() // Sleep for 1 second and process a regular event. This is to try the case where an event gets processed // before the session start event sleep(1) - await processRegularEvent(); - await task; - + await processRegularEvent() + await task + // We want to make sure that a new session was started XCTAssertTrue(amplitude.getSessionId() > oneHourEarlierTimestamp) - + } func testMigrationToApiKeyAndInstanceNameStorage() throws { @@ -394,15 +407,16 @@ final class AmplitudeTests: XCTestCase { let legacyIdentityStorage = PersistentStorage(storagePrefix: "identify-\(config.getNormalizeInstanceName())") // Init Amplitude using legacy storage - let legacyStorageAmplitude = FakeAmplitudeWithNoInstNameOnlyMigration(configuration: Configuration( - apiKey: config.apiKey, - flushQueueSize: config.flushQueueSize, - flushIntervalMillis: config.flushIntervalMillis, - storageProvider: legacyEventStorage, - identifyStorageProvider: legacyIdentityStorage, - logLevel: config.logLevel, - defaultTracking: config.defaultTracking - )) + let legacyStorageAmplitude = FakeAmplitudeWithNoInstNameOnlyMigration( + configuration: Configuration( + apiKey: config.apiKey, + flushQueueSize: config.flushQueueSize, + flushIntervalMillis: config.flushIntervalMillis, + storageProvider: legacyEventStorage, + identifyStorageProvider: legacyIdentityStorage, + logLevel: config.logLevel, + defaultTracking: config.defaultTracking + )) let legacyDeviceId = legacyStorageAmplitude.getDeviceId() @@ -443,13 +457,13 @@ final class AmplitudeTests: XCTestCase { XCTAssertNotNil(legacyEventsString) #if os(macOS) - // We don't want to transfer event data in non-sanboxed apps - XCTAssertFalse(amplitude.isSandboxEnabled()) - XCTAssertEqual(eventFiles?.count ?? 0, 0) + // We don't want to transfer event data in non-sanboxed apps + XCTAssertFalse(amplitude.isSandboxEnabled()) + XCTAssertEqual(eventFiles?.count ?? 0, 0) #else - XCTAssertTrue(eventsString != "") - XCTAssertEqual(legacyEventsString, eventsString) - XCTAssertEqual(eventFiles?.count ?? 0, 1) + XCTAssertTrue(eventsString != "") + XCTAssertEqual(legacyEventsString, eventsString) + XCTAssertEqual(eventFiles?.count ?? 0, 1) #endif // clear storage @@ -460,81 +474,84 @@ final class AmplitudeTests: XCTestCase { } #if os(macOS) - func testMigrationToApiKeyAndInstanceNameStorageMacSandboxEnabled() throws { - let legacyUserId = "legacy-user-id" - let config = Configuration( - apiKey: "amp-mac-migration-api-key", - // don't transfer any events - flushQueueSize: 1000, - flushIntervalMillis: 99999, - logLevel: LogLevelEnum.DEBUG, - defaultTracking: DefaultTrackingOptions.NONE - ) - - // Create storages using instance name only - let legacyEventStorage = FakePersistentStorageAppSandboxEnabled(storagePrefix: "storage-\(config.getNormalizeInstanceName())") - let legacyIdentityStorage = FakePersistentStorageAppSandboxEnabled(storagePrefix: "identify-\(config.getNormalizeInstanceName())") - - // Init Amplitude using legacy storage - let legacyStorageAmplitude = FakeAmplitudeWithNoInstNameOnlyMigration(configuration: Configuration( - apiKey: config.apiKey, - flushQueueSize: config.flushQueueSize, - flushIntervalMillis: config.flushIntervalMillis, - storageProvider: legacyEventStorage, - identifyStorageProvider: legacyIdentityStorage, - logLevel: config.logLevel, - defaultTracking: config.defaultTracking - )) + func testMigrationToApiKeyAndInstanceNameStorageMacSandboxEnabled() throws { + let legacyUserId = "legacy-user-id" + let config = Configuration( + apiKey: "amp-mac-migration-api-key", + // don't transfer any events + flushQueueSize: 1000, + flushIntervalMillis: 99999, + logLevel: LogLevelEnum.DEBUG, + defaultTracking: DefaultTrackingOptions.NONE + ) + + // Create storages using instance name only + let legacyEventStorage = FakePersistentStorageAppSandboxEnabled( + storagePrefix: "storage-\(config.getNormalizeInstanceName())") + let legacyIdentityStorage = FakePersistentStorageAppSandboxEnabled( + storagePrefix: "identify-\(config.getNormalizeInstanceName())") + + // Init Amplitude using legacy storage + let legacyStorageAmplitude = FakeAmplitudeWithNoInstNameOnlyMigration( + configuration: Configuration( + apiKey: config.apiKey, + flushQueueSize: config.flushQueueSize, + flushIntervalMillis: config.flushIntervalMillis, + storageProvider: legacyEventStorage, + identifyStorageProvider: legacyIdentityStorage, + logLevel: config.logLevel, + defaultTracking: config.defaultTracking + )) + + let legacyDeviceId = legacyStorageAmplitude.getDeviceId() + + // set userId + legacyStorageAmplitude.setUserId(userId: legacyUserId) + XCTAssertEqual(legacyUserId, legacyStorageAmplitude.getUserId()) + + // track events to legacy storage + legacyStorageAmplitude.identify(identify: Identify().set(property: "user-prop", value: true)) + legacyStorageAmplitude.track(event: BaseEvent(eventType: "Legacy Storage Event")) + + guard let legacyEventFiles: [URL]? = legacyEventStorage.read(key: StorageKey.EVENTS) else { return } + + var legacyEventsString = "" + legacyEventFiles?.forEach { file in + legacyEventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? "" + } - let legacyDeviceId = legacyStorageAmplitude.getDeviceId() + XCTAssertEqual(legacyEventFiles?.count ?? 0, 1) - // set userId - legacyStorageAmplitude.setUserId(userId: legacyUserId) - XCTAssertEqual(legacyUserId, legacyStorageAmplitude.getUserId()) + let amplitude = FakeAmplitudeWithSandboxEnabled(configuration: config) + let deviceId = amplitude.getDeviceId() + let userId = amplitude.getUserId() - // track events to legacy storage - legacyStorageAmplitude.identify(identify: Identify().set(property: "user-prop", value: true)) - legacyStorageAmplitude.track(event: BaseEvent(eventType: "Legacy Storage Event")) + guard let eventFiles: [URL]? = amplitude.storage.read(key: StorageKey.EVENTS) else { return } - guard let legacyEventFiles: [URL]? = legacyEventStorage.read(key: StorageKey.EVENTS) else { return } + var eventsString = "" + eventFiles?.forEach { file in + eventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? "" + } - var legacyEventsString = "" - legacyEventFiles?.forEach { file in - legacyEventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? "" - } + XCTAssertEqual(legacyDeviceId != nil, true) + XCTAssertEqual(deviceId != nil, true) + XCTAssertEqual(legacyDeviceId, deviceId) - XCTAssertEqual(legacyEventFiles?.count ?? 0, 1) + XCTAssertEqual(legacyUserId, userId) - let amplitude = FakeAmplitudeWithSandboxEnabled(configuration: config) - let deviceId = amplitude.getDeviceId() - let userId = amplitude.getUserId() + XCTAssertNotNil(legacyEventsString) - guard let eventFiles: [URL]? = amplitude.storage.read(key: StorageKey.EVENTS) else { return } + // Transfer event data in sandboxed apps + XCTAssertTrue(eventsString == "") + XCTAssertNotEqual(legacyEventsString, eventsString) + XCTAssertEqual(eventFiles?.count ?? 0, 0) - var eventsString = "" - eventFiles?.forEach { file in - eventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? "" + // clear storage + amplitude.storage.reset() + amplitude.identifyStorage.reset() + legacyStorageAmplitude.storage.reset() + legacyStorageAmplitude.identifyStorage.reset() } - - XCTAssertEqual(legacyDeviceId != nil, true) - XCTAssertEqual(deviceId != nil, true) - XCTAssertEqual(legacyDeviceId, deviceId) - - XCTAssertEqual(legacyUserId, userId) - - XCTAssertNotNil(legacyEventsString) - - // Transfer event data in sandboxed apps - XCTAssertTrue(eventsString == "") - XCTAssertNotEqual(legacyEventsString, eventsString) - XCTAssertEqual(eventFiles?.count ?? 0, 0) - - // clear storage - amplitude.storage.reset() - amplitude.identifyStorage.reset() - legacyStorageAmplitude.storage.reset() - legacyStorageAmplitude.identifyStorage.reset() - } #endif func testInit_Offline() { From 429bf36a693bd736817ca2c21e15e907cab7652a Mon Sep 17 00:00:00 2001 From: "izaaz.yunus" Date: Mon, 4 Mar 2024 14:41:00 -0800 Subject: [PATCH 4/4] chore: Fixing lint issues --- Tests/AmplitudeTests/Supports/TestUtilities.swift | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/Tests/AmplitudeTests/Supports/TestUtilities.swift b/Tests/AmplitudeTests/Supports/TestUtilities.swift index 3b2d09fa..4fc677bd 100644 --- a/Tests/AmplitudeTests/Supports/TestUtilities.swift +++ b/Tests/AmplitudeTests/Supports/TestUtilities.swift @@ -1,7 +1,7 @@ +import Combine import Foundation -import XCTest import Network -import Combine +import XCTest @testable import AmplitudeSwift @@ -111,7 +111,9 @@ class FakeInMemoryStorage: Storage { eventBlock: EventBlock, eventsString: String ) -> ResponseHandler { - FakeResponseHandler(configuration: configuration, storage: self, eventPipeline: eventPipeline, eventBlock: eventBlock, eventsString: eventsString) + FakeResponseHandler( + configuration: configuration, storage: self, eventPipeline: eventPipeline, eventBlock: eventBlock, + eventsString: eventsString) } } @@ -134,7 +136,7 @@ class FakeHttpClient: HttpClient { override func getDate() -> Date { // timestamp of 2023-10-24T18:16:24.000 in UTC time zone - return Date(timeIntervalSince1970: 1698171384) + return Date(timeIntervalSince1970: 1_698_171_384) } } @@ -292,7 +294,7 @@ final class MockPathCreation: PathCreationProtocol { class SessionsWithDelayedEventStartProcessing: Sessions { override func processEvent(event: BaseEvent, inForeground: Bool) -> [BaseEvent] { - if (event.eventType == Constants.AMP_SESSION_START_EVENT) { + if event.eventType == Constants.AMP_SESSION_START_EVENT { sleep(3) } return super.processEvent(event: event, inForeground: inForeground)