Skip to content

Commit

Permalink
fix: Set inForeground to true after a new session has been started (#124
Browse files Browse the repository at this point in the history 
)

Set inForeground to true only after a new session has been started. If another event is processed between inForeground = true and session start, it can unintentionally extend an old session
  • Loading branch information
@izaaz
izaaz authored Mar 4, 2024
1 parent 00e29c7 commit 3780c44
Show file tree
Hide file tree
Showing 3 changed files with 186 additions and 120 deletions.
3 changes: 2 additions & 1 deletion Sources/Amplitude/Amplitude.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) }
}

Expand Down
284 changes: 169 additions & 115 deletions Tests/AmplitudeTests/AmplitudeTests.swift
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import XCTest
import AnalyticsConnector
import XCTest

@testable import AmplitudeSwift

Expand Down Expand Up @@ -184,12 +184,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")

Expand All @@ -216,7 +217,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")
Expand All @@ -233,27 +235,30 @@ 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"
let expectation = XCTestExpectation()
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)
Expand Down Expand Up @@ -293,10 +298,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 {
Expand All @@ -314,9 +321,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 {
Expand All @@ -334,9 +343,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 {
Expand All @@ -354,9 +365,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() {
Expand All @@ -376,6 +389,43 @@ final class AmplitudeTests: XCTestCase {
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"
let config = Configuration(
Expand All @@ -392,15 +442,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()

Expand Down Expand Up @@ -441,13 +492,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
Expand All @@ -458,81 +509,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
)
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) ?? ""
}

// Create storages using instance name only
let legacyEventStorage = FakePersistentStorageAppSandboxEnabled(storagePrefix: "storage-\(config.getNormalizeInstanceName())")
let legacyIdentityStorage = FakePersistentStorageAppSandboxEnabled(storagePrefix: "identify-\(config.getNormalizeInstanceName())")
XCTAssertEqual(legacyEventFiles?.count ?? 0, 1)

// 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 amplitude = FakeAmplitudeWithSandboxEnabled(configuration: config)
let deviceId = amplitude.getDeviceId()
let userId = amplitude.getUserId()

let legacyDeviceId = legacyStorageAmplitude.getDeviceId()
guard let eventFiles: [URL]? = amplitude.storage.read(key: StorageKey.EVENTS) else { return }

// set userId
legacyStorageAmplitude.setUserId(userId: legacyUserId)
XCTAssertEqual(legacyUserId, legacyStorageAmplitude.getUserId())
var eventsString = ""
eventFiles?.forEach { file in
eventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? ""
}

// track events to legacy storage
legacyStorageAmplitude.identify(identify: Identify().set(property: "user-prop", value: true))
legacyStorageAmplitude.track(event: BaseEvent(eventType: "Legacy Storage Event"))
XCTAssertEqual(legacyDeviceId != nil, true)
XCTAssertEqual(deviceId != nil, true)
XCTAssertEqual(legacyDeviceId, deviceId)

guard let legacyEventFiles: [URL]? = legacyEventStorage.read(key: StorageKey.EVENTS) else { return }
XCTAssertEqual(legacyUserId, userId)

var legacyEventsString = ""
legacyEventFiles?.forEach { file in
legacyEventsString = legacyEventStorage.getEventsString(eventBlock: file) ?? ""
}
XCTAssertNotNil(legacyEventsString)

XCTAssertEqual(legacyEventFiles?.count ?? 0, 1)
// Transfer event data in sandboxed apps
XCTAssertTrue(eventsString == "")
XCTAssertNotEqual(legacyEventsString, eventsString)
XCTAssertEqual(eventFiles?.count ?? 0, 0)

let amplitude = FakeAmplitudeWithSandboxEnabled(configuration: config)
let deviceId = amplitude.getDeviceId()
let userId = amplitude.getUserId()

guard let eventFiles: [URL]? = amplitude.storage.read(key: StorageKey.EVENTS) else { return }

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 testRemnantDataNotMigratedInNonSandboxedApps() throws {
Expand Down
Loading

0 comments on commit 3780c44

Please sign in to comment.