Skip to content

Commit c94d583

Browse files
polbinspolbins
authored
fix: deprecate on enter / exit foreground (#286)
* deprecate onEnter/onExit foreground * refactor timeline to make it more readable
1 parent 30a2082 commit c94d583

File tree

6 files changed

+172
-101
lines changed

6 files changed

+172
-101
lines changed

‎android/src/main/java/com/amplitude/android/Amplitude.kt

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import com.amplitude.android.plugins.AndroidNetworkConnectivityCheckerPlugin
1111
import com.amplitude.android.storage.AndroidStorageContextV3
1212
import com.amplitude.android.utilities.ActivityLifecycleObserver
1313
import com.amplitude.core.State
14-
import com.amplitude.core.events.BaseEvent
1514
import com.amplitude.core.platform.plugins.AmplitudeDestination
1615
import com.amplitude.core.platform.plugins.GetAmpliExtrasPlugin
1716
import com.amplitude.id.IdentityConfiguration
@@ -126,22 +125,14 @@ open class Amplitude internal constructor(
126125
return this
127126
}
128127

128+
@Deprecated("This method is deprecated and a no-op.")
129129
fun onEnterForeground(timestamp: Long) {
130-
val dummyEnterForegroundEvent = BaseEvent()
131-
dummyEnterForegroundEvent.eventType = DUMMY_ENTER_FOREGROUND_EVENT
132-
dummyEnterForegroundEvent.timestamp = timestamp
133-
timeline.process(dummyEnterForegroundEvent)
130+
// no-op
134131
}
135132

133+
@Deprecated("This method is deprecated and a no-op.")
136134
fun onExitForeground(timestamp: Long) {
137-
val dummyExitForegroundEvent = BaseEvent()
138-
dummyExitForegroundEvent.eventType = DUMMY_EXIT_FOREGROUND_EVENT
139-
dummyExitForegroundEvent.timestamp = timestamp
140-
timeline.process(dummyExitForegroundEvent)
141-
142-
if ((configuration as Configuration).flushEventsOnClose) {
143-
flush()
144-
}
135+
// no-op
145136
}
146137

147138
private fun registerShutdownHook() {

‎android/src/main/java/com/amplitude/android/Timeline.kt

Lines changed: 76 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
package com.amplitude.android
22

3+
import com.amplitude.android.Amplitude.Companion.DUMMY_ENTER_FOREGROUND_EVENT
4+
import com.amplitude.android.Amplitude.Companion.DUMMY_EXIT_FOREGROUND_EVENT
5+
import com.amplitude.android.Amplitude.Companion.END_SESSION_EVENT
6+
import com.amplitude.android.Amplitude.Companion.START_SESSION_EVENT
37
import com.amplitude.core.Storage
8+
import com.amplitude.core.Storage.Constants
9+
import com.amplitude.core.Storage.Constants.LAST_EVENT_ID
10+
import com.amplitude.core.Storage.Constants.LAST_EVENT_TIME
11+
import com.amplitude.core.Storage.Constants.PREVIOUS_SESSION_ID
412
import com.amplitude.core.events.BaseEvent
513
import com.amplitude.core.platform.Timeline
614
import kotlinx.coroutines.channels.Channel
@@ -23,21 +31,20 @@ class Timeline(
2331
var lastEventTime: Long = -1L
2432

2533
internal fun start() {
26-
amplitude.amplitudeScope.launch(amplitude.storageIODispatcher) {
27-
// Wait until build (including possible legacy data migration) is finished.
28-
amplitude.isBuilt.await()
29-
30-
if (initialSessionId == null) {
31-
_sessionId.set(
32-
amplitude.storage.read(Storage.Constants.PREVIOUS_SESSION_ID)?.toLongOrNull()
33-
?: -1
34-
)
35-
}
36-
lastEventId = amplitude.storage.read(Storage.Constants.LAST_EVENT_ID)?.toLongOrNull() ?: 0
37-
lastEventTime = amplitude.storage.read(Storage.Constants.LAST_EVENT_TIME)?.toLongOrNull() ?: -1
34+
with(amplitude) {
35+
amplitudeScope.launch(storageIODispatcher) {
36+
// Wait until build (including possible legacy data migration) is finished.
37+
isBuilt.await()
38+
39+
if (initialSessionId == null) {
40+
_sessionId.set(storage.readLong(PREVIOUS_SESSION_ID, -1))
41+
}
42+
lastEventId = storage.readLong(LAST_EVENT_ID, 0)
43+
lastEventTime = storage.readLong(LAST_EVENT_TIME, -1)
3844

39-
for (message in eventMessageChannel) {
40-
processEventMessage(message)
45+
for (message in eventMessageChannel) {
46+
processEventMessage(message)
47+
}
4148
}
4249
}
4350
}
@@ -56,93 +63,89 @@ class Timeline(
5663

5764
private suspend fun processEventMessage(message: EventQueueMessage) {
5865
val event = message.event
59-
var sessionEvents: Iterable<BaseEvent>? = null
60-
val eventTimestamp = event.timestamp!!
66+
val eventTimestamp = event.timestamp!! // Guaranteed non-null by process()
6167
val eventSessionId = event.sessionId
62-
var skipEvent = false
63-
64-
if (event.eventType == Amplitude.START_SESSION_EVENT) {
65-
setSessionId(eventSessionId ?: eventTimestamp)
66-
refreshSessionTime(eventTimestamp)
67-
} else if (event.eventType == Amplitude.END_SESSION_EVENT) {
68-
// do nothing
69-
} else if (event.eventType == Amplitude.DUMMY_ENTER_FOREGROUND_EVENT) {
70-
skipEvent = true
71-
sessionEvents = startNewSessionIfNeeded(eventTimestamp)
72-
_foreground = true
73-
} else if (event.eventType == Amplitude.DUMMY_EXIT_FOREGROUND_EVENT) {
74-
skipEvent = true
75-
refreshSessionTime(eventTimestamp)
76-
_foreground = false
77-
} else {
78-
if (!_foreground) {
79-
sessionEvents = startNewSessionIfNeeded(eventTimestamp)
80-
} else {
68+
69+
var localSessionEvents: List<BaseEvent> = emptyList()
70+
71+
when (event.eventType) {
72+
START_SESSION_EVENT -> {
73+
setSessionId(eventSessionId ?: eventTimestamp)
8174
refreshSessionTime(eventTimestamp)
8275
}
83-
}
8476

85-
if (!skipEvent && event.sessionId == null) {
86-
event.sessionId = sessionId
87-
}
77+
END_SESSION_EVENT -> {
78+
// No specific action needed before processing this event type
79+
}
8880

89-
val savedLastEventId = lastEventId
81+
DUMMY_ENTER_FOREGROUND_EVENT -> {
82+
localSessionEvents = startNewSessionIfNeeded(eventTimestamp)
83+
_foreground = true
84+
}
9085

91-
sessionEvents?.let {
92-
it.forEach { e ->
93-
e.eventId ?: let {
94-
val newEventId = lastEventId + 1
95-
e.eventId = newEventId
96-
lastEventId = newEventId
97-
}
86+
DUMMY_EXIT_FOREGROUND_EVENT -> {
87+
refreshSessionTime(eventTimestamp)
88+
_foreground = false
9889
}
99-
}
10090

101-
if (!skipEvent) {
102-
event.eventId ?: let {
103-
val newEventId = lastEventId + 1
104-
event.eventId = newEventId
105-
lastEventId = newEventId
91+
else -> {
92+
// Regular event
93+
if (!_foreground) {
94+
localSessionEvents = startNewSessionIfNeeded(eventTimestamp)
95+
} else {
96+
refreshSessionTime(eventTimestamp)
97+
}
10698
}
10799
}
108100

109-
if (lastEventId > savedLastEventId) {
110-
amplitude.storage.write(Storage.Constants.LAST_EVENT_ID, lastEventId.toString())
101+
val initialLastEventId = lastEventId
102+
103+
// Process any local session events first
104+
for (sessionEvent in localSessionEvents) {
105+
sessionEvent.eventId = sessionEvent.eventId ?: ++lastEventId
106+
super.process(sessionEvent)
111107
}
112108

113-
sessionEvents?.let {
114-
it.forEach { e ->
115-
super.process(e)
109+
// Process the incoming event
110+
val dummyEvent = event.eventType == DUMMY_ENTER_FOREGROUND_EVENT ||
111+
event.eventType == DUMMY_EXIT_FOREGROUND_EVENT
112+
if (!dummyEvent) {
113+
event.eventId = event.eventId ?: ++lastEventId
114+
// Assign sessionId to the current event if it's not a dummy event and doesn't have one
115+
if (event.sessionId == null) {
116+
event.sessionId = this.sessionId // Use this.sessionId for clarity
116117
}
118+
super.process(event)
117119
}
118120

119-
if (!skipEvent) {
120-
super.process(event)
121+
// Persist lastEventId if it changed
122+
if (lastEventId > initialLastEventId) {
123+
amplitude.storage.write(LAST_EVENT_ID, lastEventId.toString())
121124
}
122125
}
123126

124-
private suspend fun startNewSessionIfNeeded(timestamp: Long): Iterable<BaseEvent>? {
127+
private suspend fun startNewSessionIfNeeded(timestamp: Long): List<BaseEvent> {
125128
if (inSession() && isWithinMinTimeBetweenSessions(timestamp)) {
126129
refreshSessionTime(timestamp)
127-
return null
130+
return emptyList()
128131
}
129132
return startNewSession(timestamp)
130133
}
131134

132135
private suspend fun setSessionId(timestamp: Long) {
133136
_sessionId.set(timestamp)
134-
amplitude.storage.write(Storage.Constants.PREVIOUS_SESSION_ID, sessionId.toString())
137+
amplitude.storage.write(PREVIOUS_SESSION_ID, sessionId.toString())
135138
}
136139

137-
private suspend fun startNewSession(timestamp: Long): Iterable<BaseEvent> {
140+
private suspend fun startNewSession(timestamp: Long): List<BaseEvent> {
138141
val sessionEvents = mutableListOf<BaseEvent>()
139142
val configuration = amplitude.configuration as Configuration
140143
val trackingSessionEvents = AutocaptureOption.SESSIONS in configuration.autocapture
141144

142145
// end previous session
143146
if (trackingSessionEvents && inSession()) {
144147
val sessionEndEvent = BaseEvent()
145-
sessionEndEvent.eventType = Amplitude.END_SESSION_EVENT
148+
sessionEndEvent.eventType = END_SESSION_EVENT
146149
sessionEndEvent.timestamp = if (lastEventTime > 0) lastEventTime else null
147150
sessionEndEvent.sessionId = sessionId
148151
sessionEvents.add(sessionEndEvent)
@@ -153,7 +156,7 @@ class Timeline(
153156
refreshSessionTime(timestamp)
154157
if (trackingSessionEvents) {
155158
val sessionStartEvent = BaseEvent()
156-
sessionStartEvent.eventType = Amplitude.START_SESSION_EVENT
159+
sessionStartEvent.eventType = START_SESSION_EVENT
157160
sessionStartEvent.timestamp = timestamp
158161
sessionStartEvent.sessionId = sessionId
159162
sessionEvents.add(sessionStartEvent)
@@ -167,17 +170,22 @@ class Timeline(
167170
return
168171
}
169172
lastEventTime = timestamp
170-
amplitude.storage.write(Storage.Constants.LAST_EVENT_TIME, lastEventTime.toString())
173+
amplitude.storage.write(LAST_EVENT_TIME, lastEventTime.toString())
171174
}
172175

173176
private fun isWithinMinTimeBetweenSessions(timestamp: Long): Boolean {
174-
val sessionLimit: Long = (amplitude.configuration as Configuration).minTimeBetweenSessionsMillis
177+
val sessionLimit: Long =
178+
(amplitude.configuration as Configuration).minTimeBetweenSessionsMillis
175179
return timestamp - lastEventTime < sessionLimit
176180
}
177181

178182
private fun inSession(): Boolean {
179183
return sessionId >= 0
180184
}
185+
186+
private fun Storage.readLong(key: Constants, default: Long): Long {
187+
return read(key)?.toLongOrNull() ?: default
188+
}
181189
}
182190

183191
data class EventQueueMessage(

‎android/src/main/java/com/amplitude/android/plugins/AndroidLifecyclePlugin.kt

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import com.amplitude.android.utilities.ActivityCallbackType
1717
import com.amplitude.android.utilities.ActivityLifecycleObserver
1818
import com.amplitude.android.utilities.DefaultEventUtils
1919
import com.amplitude.core.Amplitude
20+
import com.amplitude.core.events.BaseEvent
2021
import com.amplitude.core.platform.Plugin
2122
import kotlinx.coroutines.Dispatchers
2223
import kotlinx.coroutines.Job
@@ -112,7 +113,14 @@ class AndroidLifecyclePlugin(
112113
}
113114

114115
override fun onActivityResumed(activity: Activity) {
115-
androidAmplitude.onEnterForeground(getCurrentTimeMillis())
116+
with(androidAmplitude) {
117+
timeline.process(
118+
BaseEvent().apply {
119+
eventType = AndroidAmplitude.DUMMY_ENTER_FOREGROUND_EVENT
120+
timestamp = System.currentTimeMillis()
121+
}
122+
)
123+
}
116124

117125
@OptIn(ExperimentalAmplitudeFeature::class)
118126
if (ELEMENT_INTERACTIONS in autocapture) {
@@ -121,7 +129,18 @@ class AndroidLifecyclePlugin(
121129
}
122130

123131
override fun onActivityPaused(activity: Activity) {
124-
androidAmplitude.onExitForeground(getCurrentTimeMillis())
132+
with(androidAmplitude) {
133+
timeline.process(
134+
BaseEvent().apply {
135+
eventType = AndroidAmplitude.DUMMY_EXIT_FOREGROUND_EVENT
136+
timestamp = System.currentTimeMillis()
137+
}
138+
)
139+
140+
if ((configuration as Configuration).flushEventsOnClose) {
141+
flush()
142+
}
143+
}
125144

126145
@OptIn(ExperimentalAmplitudeFeature::class)
127146
if (ELEMENT_INTERACTIONS in autocapture) {
@@ -153,11 +172,4 @@ class AndroidLifecyclePlugin(
153172
super.teardown()
154173
eventJob?.cancel()
155174
}
156-
157-
companion object {
158-
@JvmStatic
159-
fun getCurrentTimeMillis(): Long {
160-
return System.currentTimeMillis()
161-
}
162-
}
163175
}

‎android/src/test/kotlin/com/amplitude/android/AmplitudeRobolectricTests.kt

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,14 @@ class AmplitudeRobolectricTests {
5454
event1.timestamp = 1000
5555
amplitudeInstance.track(event1)
5656

57-
amplitudeInstance.onEnterForeground(1500)
57+
enterForeground(amplitudeInstance, 1500)
5858

5959
val event2 = BaseEvent()
6060
event2.eventType = "test event 2"
6161
event2.timestamp = 1700
6262
amplitudeInstance.track(event2)
6363

64-
amplitudeInstance.onExitForeground(2000)
64+
exitForeground(amplitudeInstance, 2000)
6565

6666
assertTrue { fakeEventPlugin.trackedEvents.isEmpty() }
6767
}
@@ -82,4 +82,24 @@ class AmplitudeRobolectricTests {
8282
?: Configuration.MIN_TIME_BETWEEN_SESSIONS_MILLIS,
8383
)
8484
}
85+
86+
// simulates the dummy event for android lifecycle onActivityResumed
87+
private fun enterForeground(amplitude: Amplitude, timestamp: Long) {
88+
amplitude.timeline.process(
89+
BaseEvent().apply {
90+
eventType = Amplitude.DUMMY_ENTER_FOREGROUND_EVENT
91+
this.timestamp = timestamp
92+
}
93+
)
94+
}
95+
96+
// simulates the dummy event for android lifecycle onActivityPaused
97+
private fun exitForeground(amplitude: Amplitude, timestamp: Long) {
98+
amplitude.timeline.process(
99+
BaseEvent().apply {
100+
eventType = Amplitude.DUMMY_EXIT_FOREGROUND_EVENT
101+
this.timestamp = timestamp
102+
}
103+
)
104+
}
85105
}

0 commit comments

Comments
 (0)