diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift index e3ab505ef..7b60875cd 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift @@ -72,6 +72,9 @@ public struct OSSessionRecord: Codable, Equatable { public internal(set) var subscriptionId: String? /// Nil until the sessions API creates this session. public internal(set) var serverSessionId: String? + /// Seconds since 1970 when the app last left the foreground, which becomes the session's end + /// time. Nil until then. + public internal(set) var lastUnfocusTime: TimeInterval? } /// Owns the persisted record of the current session. Nothing reads the record while @@ -164,9 +167,12 @@ public final class OSSessionService: NSObject { /// Leaves any open foreground interval running: the tracker reports focus before the session /// starts asynchronously, and that visit belongs to the new session. A launch in the /// background has no interval open, so nothing counts until the app becomes active. - /// With the sessions API, the create is enqueued here. The executor holds it until the - /// session's IDs are assigned. + /// With the sessions API, the previous session's end and the new session's create are enqueued + /// here. The executor holds them until their IDs are assigned. public func startNewSession(userProvider: OSSessionUserProvider) { + // Set first so the previous session's IDs can be filled before it is ended. + stateLock.withLock { self.userProvider = userProvider } + let previousIds = pinnableIds() let user = userProvider.sessionCurrentUser let newRecord = OSSessionRecord( sessionId: UUID().uuidString, @@ -178,11 +184,18 @@ public final class OSSessionService: NSObject { subscriptionId: user.pushSubscriptionId, serverSessionId: nil ) - stateLock.withLock { - self.userProvider = userProvider - loaded = true + let previous: OSSessionRecord? = stateLock.withLock { + loadIfNeeded() + fillPinnedIds(previousIds) + let previous = record record = newRecord persist() + return previous + } + if let previous, previous.usesSessionsApi { + // A session killed before its first unfocus ends at its start. Its foreground time was + // never added to the duration either. + userProvider.enqueueSessionUpdate(previous, endTime: previous.lastUnfocusTime ?? previous.startTime) } if newRecord.usesSessionsApi { userProvider.enqueueSessionCreate(newRecord, directAttributionId: nil) @@ -196,6 +209,7 @@ public final class OSSessionService: NSObject { func onUnfocus() { let now = monotonicNow() + let wallNow = wallNow() let ids = pinnableIds() stateLock.withLock { guard let start = focusedAt else { @@ -207,6 +221,7 @@ public final class OSSessionService: NSObject { return } record?.activeDuration += max(0, now - start) + record?.lastUnfocusTime = wallNow fillPinnedIds(ids) persist() } diff --git a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSSessionServiceTests.swift b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSSessionServiceTests.swift index 3aad2f398..cc2ea5d3a 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSSessionServiceTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalOSCoreTests/OSSessionServiceTests.swift @@ -49,12 +49,19 @@ private final class FakeUserProvider: OSSessionUserProvider { } private(set) var createdSessions: [OSSessionRecord] = [] + private(set) var updatedSessions: [(record: OSSessionRecord, endTime: TimeInterval?)] = [] + /// Local session IDs in the order their requests were enqueued. + private(set) var enqueuedSessionIds: [String] = [] func enqueueSessionCreate(_ record: OSSessionRecord, directAttributionId: String?) { createdSessions.append(record) + enqueuedSessionIds.append(record.sessionId) } - func enqueueSessionUpdate(_ record: OSSessionRecord, endTime: TimeInterval?) {} + func enqueueSessionUpdate(_ record: OSSessionRecord, endTime: TimeInterval?) { + updatedSessions.append((record, endTime)) + enqueuedSessionIds.append(record.sessionId) + } } final class OSSessionServiceTests: XCTestCase { @@ -62,6 +69,7 @@ final class OSSessionServiceTests: XCTestCase { private var flagsStore: OSFeatureFlagsStore! private var featureManager: OSFeatureManager! private var monotonicNow: TimeInterval = 1_000 + private var wallNow: TimeInterval = 5_000 private var user: FakeUserProvider! override func setUp() { @@ -71,6 +79,7 @@ final class OSSessionServiceTests: XCTestCase { flagsStore = OSFeatureFlagsStore() featureManager = nil monotonicNow = 1_000 + wallNow = 5_000 user = FakeUserProvider() user.identityModelId = "model-a" user.onesignalIds = ["model-a": "onesignal-a"] @@ -90,7 +99,7 @@ final class OSSessionServiceTests: XCTestCase { return OSSessionService( featureManager: { manager }, monotonicNow: { [unowned self] in self.monotonicNow }, - wallNow: { 5_000 } + wallNow: { [unowned self] in self.wallNow } ) } @@ -141,6 +150,7 @@ final class OSSessionServiceTests: XCTestCase { XCTAssertEqual(record?.startTime, 5_000) XCTAssertEqual(record?.activeDuration, 0) XCTAssertNil(record?.serverSessionId) + XCTAssertNil(record?.lastUnfocusTime) } func testEachNewSessionGetsANewSessionId() { @@ -199,6 +209,7 @@ final class OSSessionServiceTests: XCTestCase { XCTAssertEqual(record?.onesignalId, "onesignal-a") XCTAssertEqual(record?.subscriptionId, "subscription-a") XCTAssertNil(record?.serverSessionId) + XCTAssertNil(record?.lastUnfocusTime) } func testClearingTheStoredRecordDropsIt() { @@ -293,6 +304,7 @@ final class OSSessionServiceTests: XCTestCase { service.onUnfocus() XCTAssertEqual(service.currentRecord?.activeDuration, 15) + XCTAssertEqual(service.currentRecord?.lastUnfocusTime, 5_000) } /// Resuming past the new-session threshold: the visit that started the session counts from @@ -345,6 +357,7 @@ final class OSSessionServiceTests: XCTestCase { restarted.onUnfocus() XCTAssertEqual(restarted.currentRecord?.activeDuration, 0) + XCTAssertNil(restarted.currentRecord?.lastUnfocusTime) } // MARK: - Session start @@ -388,4 +401,71 @@ final class OSSessionServiceTests: XCTestCase { XCTAssertTrue(user.createdSessions.isEmpty) } + + // MARK: - Session end + + /// A foreground visit of `duration` seconds that leaves the foreground at `unfocusAt`. + private func visit(_ service: OSSessionService, duration: TimeInterval, unfocusAt: TimeInterval) { + service.onFocus() + monotonicNow += duration + wallNow = unfocusAt + service.onUnfocus() + } + + func testNewSessionEndsThePreviousOneAtItsLastUnfocusBeforeTheNextCreate() { + flagsStore.applyRemoteFlags([flagKey], metadata: nil) + let service = makeService() + service.startNewSession(userProvider: user) + let first = service.currentRecord! + visit(service, duration: 30, unfocusAt: 6_000) + visit(service, duration: 12, unfocusAt: 7_000) + + wallNow = 9_000 + service.startNewSession(userProvider: user) + + let end = user.updatedSessions.last + XCTAssertEqual(end?.record.sessionId, first.sessionId) + XCTAssertEqual(end?.record.activeDuration, 42) + XCTAssertEqual(end?.endTime, 7_000) + XCTAssertEqual(user.enqueuedSessionIds, [first.sessionId, first.sessionId, service.currentRecord!.sessionId]) + } + + func testSessionKilledInTheForegroundEndsAtItsStartTime() { + flagsStore.applyRemoteFlags([flagKey], metadata: nil) + let first = makeForegroundSession().currentRecord! + monotonicNow += 30 + + wallNow = 9_000 + makeService().startNewSession(userProvider: user) + + let end = user.updatedSessions.last + XCTAssertEqual(end?.record.sessionId, first.sessionId) + XCTAssertEqual(end?.record.activeDuration, 0) + XCTAssertEqual(end?.endTime, first.startTime) + } + + /// The previous process ended before the user was created, and nothing set the user provider + /// in this one before the next session started. + func testSessionWithIncompleteIdsFromAnEarlierProcessIsEndedWithItsIds() { + flagsStore.applyRemoteFlags([flagKey], metadata: nil) + user.onesignalIds = [:] + makeService().startNewSession(userProvider: user) + + user.onesignalIds = ["model-a": "onesignal-a"] + makeService().startNewSession(userProvider: user) + + XCTAssertEqual(user.updatedSessions.last?.record.onesignalId, "onesignal-a") + } + + func testLegacySessionIsNotEndedThroughTheSessionsApi() { + let service = makeService() + service.startNewSession(userProvider: user) + visit(service, duration: 10, unfocusAt: 6_000) + + flagsStore.applyRemoteFlags([flagKey], metadata: nil) + service.startNewSession(userProvider: user) + + XCTAssertTrue(user.updatedSessions.isEmpty) + XCTAssertEqual(user.createdSessions.count, 1) + } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl+Session.swift b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl+Session.swift index 2009c6f9a..8532fbf44 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl+Session.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl+Session.swift @@ -59,8 +59,7 @@ extension OneSignalUserManagerImpl: OSSessionUserProvider { private func enqueueSessionDelta(_ name: String, record: OSSessionRecord, value: [String: Any]) { guard record.usesSessionsApi, - let identityModelId = record.identityModelId, - let identityModel = identityModelRepo.get(modelId: identityModelId), + let identityModel = sessionIdentityModel(for: record), let appId = OneSignalIdentifiers.currentAppId else { return @@ -71,7 +70,7 @@ extension OneSignalUserManagerImpl: OSSessionUserProvider { value[OSSessionDeltaKey.subscriptionId] = record.subscriptionId let delta = OSDelta( name: name, - identityModelId: identityModelId, + identityModelId: identityModel.modelId, externalId: identityModel.externalId, model: identityModel, property: record.sessionId, @@ -79,4 +78,19 @@ extension OneSignalUserManagerImpl: OSSessionUserProvider { ) operationRepo.enqueueDelta(delta) } + + /// The session's user. One no longer loaded, such as the user before a login in an earlier + /// process, is restored from the session's pinned `onesignal_id` so its requests can still be sent. + private func sessionIdentityModel(for record: OSSessionRecord) -> OSIdentityModel? { + if let identityModelId = record.identityModelId, let identityModel = identityModelRepo.get(modelId: identityModelId) { + return identityModel + } + guard let onesignalId = record.onesignalId else { + OneSignalLog.onesignalLog(.LL_WARN, message: "OneSignalUserManagerImpl dropping a request for session \(record.sessionId), its user is gone and was never created") + return nil + } + let identityModel = OSIdentityModel(aliases: [OS_ONESIGNAL_ID: onesignalId], changeNotifier: OSEventProducer()) + addIdentityModelToRepo(identityModel) + return identityModel + } } diff --git a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/OSSessionOperationExecutorTests.swift b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/OSSessionOperationExecutorTests.swift index 65419b8f3..d7130931b 100644 --- a/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/OSSessionOperationExecutorTests.swift +++ b/iOS_SDK/OneSignalSDK/OneSignalUserTests/Executors/OSSessionOperationExecutorTests.swift @@ -448,4 +448,29 @@ final class OSSessionOperationExecutorTests: XCTestCase { XCTAssertEqual(sessionDeltas.first?.property, "session-1") XCTAssertEqual(sessionDeltas.first?.externalId, externalId) } + + func testSessionWhoseUserIsNoLongerLoadedIsSentWithItsPinnedOnesignalId() throws { + let manager = OneSignalUserManagerImpl.sharedInstance + let record = { (onesignalId: String?) in + OSSessionRecord( + sessionId: onesignalId ?? "never-created", + startTime: 1_700_000_000, + activeDuration: 12, + usesSessionsApi: true, + identityModelId: "unloaded-model", + onesignalId: onesignalId, + subscriptionId: nil, + serverSessionId: "server-1" + ) + } + + manager.enqueueSessionUpdate(record("pinned-onesignal-id"), endTime: 1_700_000_100) + manager.enqueueSessionUpdate(record(nil), endTime: 1_700_000_100) + manager.operationRepo.dispatchQueue.sync {} + + let sessionDeltas = manager.operationRepo.deltaQueue.filter { $0.name == OS_UPDATE_SESSION_DELTA } + XCTAssertEqual(sessionDeltas.map(\.property), ["pinned-onesignal-id"]) + let identityModel = try XCTUnwrap(manager.getIdentityModel(sessionDeltas[0].identityModelId)) + XCTAssertEqual(identityModel.onesignalId, "pinned-onesignal-id") + } }