From 3bf05851d0d2ef0626f735c33c0e4ce206d91931 Mon Sep 17 00:00:00 2001 From: Justin Fiedler Date: Fri, 22 Mar 2024 17:23:18 +0000 Subject: [PATCH] chore: move lastEventId logic back to timeline (remove from session) --- .../java/com/amplitude/android/Timeline.kt | 34 ++++++++++++- .../amplitude/android/utilities/Session.kt | 19 +------ .../amplitude/android/AmplitudeSessionTest.kt | 49 ++++++++++--------- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/android/src/main/java/com/amplitude/android/Timeline.kt b/android/src/main/java/com/amplitude/android/Timeline.kt index 8b5a942f..e54ea306 100644 --- a/android/src/main/java/com/amplitude/android/Timeline.kt +++ b/android/src/main/java/com/amplitude/android/Timeline.kt @@ -2,16 +2,27 @@ package com.amplitude.android import com.amplitude.android.utilities.Session import com.amplitude.android.utilities.SystemTime +import com.amplitude.core.Storage import com.amplitude.core.events.BaseEvent import com.amplitude.core.platform.Timeline import kotlinx.coroutines.channels.Channel import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import java.util.concurrent.atomic.AtomicLong class Timeline : Timeline() { + companion object { + const val DEFAULT_LAST_EVENT_ID = 0L + } + private val eventMessageChannel: Channel = Channel(Channel.UNLIMITED) internal lateinit var session: Session + private val _lastEventId = AtomicLong(DEFAULT_LAST_EVENT_ID) + + internal var lastEventId: Long = DEFAULT_LAST_EVENT_ID + get() = _lastEventId.get() + internal var sessionId: Long = Session.EMPTY_SESSION_ID get() = if (session == null) Session.EMPTY_SESSION_ID else session.sessionId @@ -27,6 +38,8 @@ class Timeline : Timeline() { amplitude.configuration.sessionId ) + loadLastEventId() + amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) { // Wait until build (including possible legacy data migration) is finished. amplitude.isBuilt.await() @@ -90,14 +103,14 @@ class Timeline : Timeline() { sessionEvents?.let { it.forEach { e -> e.eventId ?: let { - e.eventId = session.getAndSetNextEventId() + e.eventId = getAndSetNextEventId() } } } if (!skipEvent) { event.eventId ?: let { - event.eventId = session.getAndSetNextEventId() + event.eventId = getAndSetNextEventId() } } @@ -111,6 +124,23 @@ class Timeline : Timeline() { super.process(event) } } + + private fun loadLastEventId() { + val lastEventId = amplitude.storage.read(Storage.Constants.LAST_EVENT_ID)?.toLongOrNull() + ?: DEFAULT_LAST_EVENT_ID + _lastEventId.set(lastEventId) + } + + private suspend fun writeLastEventId(lastEventId: Long) { + amplitude.storage.write(Storage.Constants.LAST_EVENT_ID, lastEventId.toString()) + } + + private suspend fun getAndSetNextEventId(): Long { + val nextEventId = _lastEventId.incrementAndGet() + writeLastEventId(nextEventId) + + return nextEventId + } } data class EventQueueMessage( diff --git a/android/src/main/java/com/amplitude/android/utilities/Session.kt b/android/src/main/java/com/amplitude/android/utilities/Session.kt index 373278e2..1310cb17 100644 --- a/android/src/main/java/com/amplitude/android/utilities/Session.kt +++ b/android/src/main/java/com/amplitude/android/utilities/Session.kt @@ -15,11 +15,9 @@ class Session( companion object { const val EMPTY_SESSION_ID = -1L const val DEFAULT_LAST_EVENT_TIME = -1L - const val DEFAULT_LAST_EVENT_ID = 0L } private val _sessionId = AtomicLong(EMPTY_SESSION_ID) - private val _lastEventId = AtomicLong(DEFAULT_LAST_EVENT_ID) private val _lastEventTime = AtomicLong(DEFAULT_LAST_EVENT_TIME) val sessionId: Long @@ -27,8 +25,6 @@ class Session( return _sessionId.get() } - internal var lastEventId: Long = DEFAULT_LAST_EVENT_ID - get() = _lastEventId.get() var lastEventTime: Long = DEFAULT_LAST_EVENT_TIME get() = _lastEventTime.get() @@ -38,7 +34,6 @@ class Session( private fun loadFromStorage() { _sessionId.set(storage?.read(Storage.Constants.PREVIOUS_SESSION_ID)?.toLongOrNull() ?: EMPTY_SESSION_ID) - _lastEventId.set(storage?.read(Storage.Constants.LAST_EVENT_ID)?.toLongOrNull() ?: DEFAULT_LAST_EVENT_ID) _lastEventTime.set(storage?.read(Storage.Constants.LAST_EVENT_TIME)?.toLongOrNull() ?: DEFAULT_LAST_EVENT_TIME) } @@ -65,18 +60,6 @@ class Session( state?.sessionId = timestamp } - suspend fun setLastEventId(lastEventId: Long) { - _lastEventId.set(lastEventId) - storage?.write(Storage.Constants.LAST_EVENT_ID, lastEventId.toString()) - } - - suspend fun getAndSetNextEventId(): Long { - val nextEventId = _lastEventId.incrementAndGet() - storage?.write(Storage.Constants.LAST_EVENT_ID, lastEventId.toString()) - - return nextEventId - } - private suspend fun startNewSession(timestamp: Long, sessionId: Long? = null): Iterable { val _sessionId = sessionId ?: timestamp val sessionEvents = mutableListOf() @@ -125,6 +108,6 @@ class Session( } override fun toString(): String { - return "Session(sessionId=$sessionId, lastEventId=$lastEventId, lastEventTime=$lastEventTime)" + return "Session(sessionId=$sessionId, lastEventTime=$lastEventTime)" } } diff --git a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt index 35d82c50..f37ff445 100644 --- a/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt +++ b/android/src/test/java/com/amplitude/android/AmplitudeSessionTest.kt @@ -445,11 +445,10 @@ class AmplitudeSessionTest { advanceUntilIdle() Thread.sleep(100) - val session1 = (amplitude1.timeline as Timeline).session - - Assertions.assertEquals(StartTime, session1.sessionId) - Assertions.assertEquals(StartTime, session1.lastEventTime) - Assertions.assertEquals(1, session1.lastEventId) + val timeline1 = getAndroidTimeline(amplitude1) + Assertions.assertEquals(StartTime, timeline1.session.sessionId) + Assertions.assertEquals(StartTime, timeline1.session.lastEventTime) + Assertions.assertEquals(1, timeline1.lastEventId) val event1Time = mockSystemTime(StartTime + 200) amplitude1.track(createEvent(event1Time, "test event 1")) @@ -457,9 +456,9 @@ class AmplitudeSessionTest { advanceUntilIdle() Thread.sleep(100) - Assertions.assertEquals(StartTime, session1.sessionId) - Assertions.assertEquals(event1Time, session1.lastEventTime) - Assertions.assertEquals(2, session1.lastEventId) + Assertions.assertEquals(StartTime, timeline1.session.sessionId) + Assertions.assertEquals(event1Time, timeline1.session.lastEventTime) + Assertions.assertEquals(2, timeline1.lastEventId) // Inc time by 50ms val instance2CreationTime = mockSystemTime(StartTime + 250) @@ -468,11 +467,11 @@ class AmplitudeSessionTest { setDispatcher(amplitude2, testScheduler) amplitude2.isBuilt.await() - val session2 = (amplitude2.timeline as Timeline).session - Assertions.assertEquals(StartTime, session2.sessionId) + val timeline2 = getAndroidTimeline(amplitude2) + Assertions.assertEquals(StartTime, timeline2.session.sessionId) // Last event time is the SDK creation time (1250) - Assertions.assertEquals(instance2CreationTime, session2.lastEventTime) - Assertions.assertEquals(2, session2.lastEventId) + Assertions.assertEquals(instance2CreationTime, timeline2.session.lastEventTime) + Assertions.assertEquals(2, timeline2.lastEventId) } @Test @@ -653,10 +652,10 @@ class AmplitudeSessionTest { advanceUntilIdle() Thread.sleep(100) - val session1 = (amplitude1.timeline as Timeline).session - Assertions.assertEquals(enterForegroundTime, session1.sessionId) - Assertions.assertEquals(enterForegroundTime, session1.lastEventTime) - Assertions.assertEquals(1, session1.lastEventId) + val timeline1 = getAndroidTimeline(amplitude1) + Assertions.assertEquals(enterForegroundTime, timeline1.session.sessionId) + Assertions.assertEquals(enterForegroundTime, timeline1.session.lastEventTime) + Assertions.assertEquals(1, timeline1.lastEventId) // track event (set last event time) (3) val event1Time = mockSystemTime(enterForegroundTime + 200) @@ -666,9 +665,9 @@ class AmplitudeSessionTest { Thread.sleep(100) // valid session and last event time - Assertions.assertEquals(startTime, session1.sessionId) - Assertions.assertEquals(event1Time, session1.lastEventTime) - Assertions.assertEquals(2, session1.lastEventId) + Assertions.assertEquals(startTime, timeline1.session.sessionId) + Assertions.assertEquals(event1Time, timeline1.session.lastEventTime) + Assertions.assertEquals(2, timeline1.lastEventId) // exit foreground (4) val exitForegroundTime = mockSystemTime(event1Time + 100) @@ -689,11 +688,11 @@ class AmplitudeSessionTest { advanceUntilIdle() Thread.sleep(100) - val session2 = (amplitude2.timeline as Timeline).session - Assertions.assertEquals(newSessionTime, session2.sessionId) - Assertions.assertEquals(newSessionTime, session2.lastEventTime) + val timeline2 = getAndroidTimeline(amplitude2) + Assertions.assertEquals(newSessionTime, timeline2.session.sessionId) + Assertions.assertEquals(newSessionTime, timeline2.session.lastEventTime) // 6 events = session_start, enter foreground, track, exit foreground, session_end, session_start - Assertions.assertEquals(6, session2.lastEventId) + Assertions.assertEquals(6, timeline2.lastEventId) } private fun createEvent(timestamp: Long, eventType: String, sessionId: Long? = null): BaseEvent { @@ -705,6 +704,10 @@ class AmplitudeSessionTest { return event } + private fun getAndroidTimeline(amplitude: Amplitude): Timeline { + return amplitude.timeline as Timeline + } + companion object { const val instanceName = "testInstance" private const val StartTime: Long = 1000