Merge pull request #541 from embrace-io/fix-moment-boundary

Preserve moments across session boundaries

embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/session/MomentBoundaryTest.kt

@@ -0,0 +1,108 @@
+package io.embrace.android.embracesdk.session
+
+import androidx.test.ext.junit.runners.AndroidJUnit4
+import io.embrace.android.embracesdk.EventType
+import io.embrace.android.embracesdk.IntegrationTestRule
+import io.embrace.android.embracesdk.recordSession
+import org.junit.Assert.assertEquals
+import org.junit.Rule
+import org.junit.Test
+import org.junit.runner.RunWith
+
+/**
+ * Asserts that moments (including startup moments) remain between session boundaries.
+ */
+@RunWith(AndroidJUnit4::class)
+internal class MomentBoundaryTest {
+
+ companion object {
+ private const val MOMENT_NAME = "my_moment"
+ }
+
+ @Rule
+ @JvmField
+ val testRule: IntegrationTestRule = IntegrationTestRule()
+
+ @Test
+ fun `startup moment completes within one session`() {
+ with(testRule) {
+ val message = harness.recordSession {
+ embrace.endAppStartup()
+ embrace.startMoment(MOMENT_NAME)
+ embrace.endMoment(MOMENT_NAME)
+ }
+ checkNotNull(message)
+
+ val moments = fetchDeliveredEvents()
+ assertEquals(4, moments.size)
+ val startMoment = moments[0]
+ val endMoment = moments[1]
+ val myStartMoment = moments[2]
+ val myEndMoment = moments[3]
+
+ assertEquals("_startup", startMoment.event.name)
+ assertEquals(EventType.START, startMoment.event.type)
+ assertEquals(message.session.sessionId, startMoment.event.sessionId)
+
+ assertEquals("_startup", endMoment.event.name)
+ assertEquals(EventType.END, endMoment.event.type)
+ assertEquals(message.session.sessionId, endMoment.event.sessionId)
+
+ assertEquals(MOMENT_NAME, myStartMoment.event.name)
+ assertEquals(EventType.START, myStartMoment.event.type)
+ assertEquals(message.session.sessionId, myStartMoment.event.sessionId)
+
+ assertEquals(MOMENT_NAME, myEndMoment.event.name)
+ assertEquals(EventType.END, myEndMoment.event.type)
+ assertEquals(message.session.sessionId, myEndMoment.event.sessionId)
+
+ val expectedEventIds = listOf(startMoment.event.eventId, myStartMoment.event.eventId)
+ assertEquals(expectedEventIds, message.session.eventIds)
+ }
+ }
+
+ @Test
+ fun `startup moment completes within two sessions`() {
+ with(testRule) {
+ val firstMessage = harness.recordSession {
+ embrace.startMoment(MOMENT_NAME)
+ }
+ val secondMessage = harness.recordSession {
+ embrace.endAppStartup()
+ embrace.endMoment(MOMENT_NAME)
+ }
+ checkNotNull(firstMessage)
+ checkNotNull(secondMessage)
+
+ val moments = fetchDeliveredEvents()
+ assertEquals(4, moments.size)
+ val startMoment = moments[0]
+ val myStartMoment = moments[1]
+ val endMoment = moments[2]
+ val myEndMoment = moments[3]
+
+ assertEquals("_startup", startMoment.event.name)
+ assertEquals(EventType.START, startMoment.event.type)
+ assertEquals(firstMessage.session.sessionId, startMoment.event.sessionId)
+
+ assertEquals(MOMENT_NAME, myStartMoment.event.name)
+ assertEquals(EventType.START, myStartMoment.event.type)
+ assertEquals(firstMessage.session.sessionId, myStartMoment.event.sessionId)
+
+ assertEquals("_startup", endMoment.event.name)
+ assertEquals(EventType.END, endMoment.event.type)
+ assertEquals(secondMessage.session.sessionId, endMoment.event.sessionId)
+
+ assertEquals(MOMENT_NAME, myEndMoment.event.name)
+ assertEquals(EventType.END, myEndMoment.event.type)
+ assertEquals(secondMessage.session.sessionId, myEndMoment.event.sessionId)
+
+ val expectedEventIds = listOf(startMoment.event.eventId, myStartMoment.event.eventId)
+ assertEquals(expectedEventIds, firstMessage.session.eventIds)
+ assertEquals(expectedEventIds, secondMessage.session.eventIds)
+ }
+ }
+
+ private fun IntegrationTestRule.fetchDeliveredEvents() =
+ harness.fakeDeliveryModule.deliveryService.sentMoments
+}

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt

@@ -5,7 +5,6 @@ import io.embrace.android.embracesdk.capture.metadata.MetadataService
 import io.embrace.android.embracesdk.capture.user.UserService
 import io.embrace.android.embracesdk.comms.delivery.DeliveryService
 import io.embrace.android.embracesdk.config.ConfigService
-import io.embrace.android.embracesdk.internal.CacheableValue
 import io.embrace.android.embracesdk.internal.EventDescription
 import io.embrace.android.embracesdk.internal.StartupEventInfo
 import io.embrace.android.embracesdk.internal.clock.Clock
@@ -23,10 +22,9 @@ import io.embrace.android.embracesdk.utils.stream
 import io.embrace.android.embracesdk.worker.BackgroundWorker
 import io.embrace.android.embracesdk.worker.WorkerName
 import io.embrace.android.embracesdk.worker.WorkerThreadModule
-import java.util.NavigableMap
 import java.util.concurrent.ConcurrentHashMap
+import java.util.concurrent.ConcurrentLinkedQueue
 import java.util.concurrent.ConcurrentMap
-import java.util.concurrent.ConcurrentSkipListMap
 
 /**
  * Handles the lifecycle of events (moments).
@@ -54,8 +52,7 @@ internal class EmbraceEventService(
  /**
  * Timeseries of event IDs, keyed on the start time of the event.
  */
- private val eventIds: NavigableMap<Long, String> = ConcurrentSkipListMap()
- private val eventIdsCache = CacheableValue<List<String>> { eventIds.size }
+ private val eventIds = ConcurrentLinkedQueue<String>()
 
  /**
  * Map of active events, keyed on their event ID (event name + identifier).
@@ -166,7 +163,7 @@ internal class EmbraceEventService(
  sanitizedStartTime = now
  }
  val eventId = getEmbUuid()
- eventIds[now] = eventId
+ eventIds.add(eventId)
  val eventDescription = eventHandler.onEventStarted(
  eventId,
  name,
@@ -251,10 +248,8 @@ internal class EmbraceEventService(
  }
  }
 
- override fun findEventIdsForSession(startTime: Long, endTime: Long): List<String> {
- logDeveloper("EmbraceEventService", "findEventIdsForSession")
- return eventIdsCache.value { ArrayList(eventIds.subMap(startTime, endTime).values) }
- }
+ override fun findEventIdsForSession(): List<String> =
+ eventIds.toList()
 
  override fun getActiveEventIds(): List<String> {
  val ids: MutableList<String> = ArrayList()
@@ -268,13 +263,13 @@ internal class EmbraceEventService(
 
  override fun close() {
  cleanCollections()
- logDeveloper("EmbraceEventService", "close")
  }
 
  override fun cleanCollections() {
- eventIds.clear()
- activeEvents.clear()
- logDeveloper("EmbraceEventService", "collections cleaned")
+ val activeEventIds = activeEvents.values.map { it.event.eventId }
+ eventIds.removeAll {
+ !activeEventIds.contains(it)
+ }
  }
 
  /**

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EventService.kt

@@ -96,11 +96,9 @@ internal interface EventService : Closeable {
  /**
  * Finds all event IDs (event UUIDs) within the given time window.
  *
- * @param startTime the start time of the window to search
- * @param endTime the end time of the window to search
  * @return the list of story IDs within the specified range
  */
- fun findEventIdsForSession(startTime: Long, endTime: Long): List<String>
+ fun findEventIdsForSession(): List<String>
 
  /**
  * Gets all of the IDs of the currently active moments.

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/message/PayloadMessageCollator.kt

@@ -121,10 +121,7 @@ internal class PayloadMessageCollator(
  return initial.copy(
  endTime = endTime,
  eventIds = captureDataSafely {
- eventService.findEventIdsForSession(
- startTime,
- endTime
- )
+ eventService.findEventIdsForSession()
  },
  infoLogIds = captureDataSafely { logMessageService.findInfoLogIds(startTime, endTime) },
  warningLogIds = captureDataSafely {

embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/event/EmbraceEventServiceTest.kt

@@ -353,64 +353,28 @@ internal class EmbraceEventServiceTest {
  assertNull(eventService.getStartupMomentInfo())
  }
 
- @Test
- fun `verify find event ids using findEventIdsForSession()`() {
- // Simulate the session moving forward in time, and having new Moments added and us retrieving the eventIds given the new time range
- // Note that because of how the underlying cache works, if the size of the eventIds collection didn't change from the last time
- // this method was invoked, the previously cached value will be returned. So while calling this method with arbitrary start/end
- // times can return wrong values, how it is being used, that won't happen. This test will simulate the EXPECTED usage rather than
- // the arbitrary usage.
-
- val sessionBeginTime = 100L
- fakeClock.setCurrentTime(sessionBeginTime)
- eventService.startEvent("first")
- fakeClock.setCurrentTime(fakeClock.now() + 1L)
-
- // after a new Moment is logged and the time ticks forward, we should see it reflected in the cache
- assertEquals(1, eventService.findEventIdsForSession(sessionBeginTime, fakeClock.now()).size)
- fakeClock.setCurrentTime(fakeClock.now() + 50L)
- eventService.startEvent("second")
- fakeClock.setCurrentTime(fakeClock.now() + 1L)
- eventService.startEvent("third")
-
- // the new time range will only return 2 of the logged moments because the clock hasn't ticked forward
- assertEquals(2, eventService.findEventIdsForSession(sessionBeginTime, fakeClock.now()).size)
- fakeClock.setCurrentTime(fakeClock.now() + 1L)
-
- // After the clock ticks forward, because of the caching, we will still only return 2. This is a perf optimization that will be
- // OK in practice because we should only bust the cache if there's a new moment - this check is just to verify the caching works
- // because it won't really happen in practice
- assertEquals(2, eventService.findEventIdsForSession(sessionBeginTime, fakeClock.now()).size)
-
- // After logging another moment, the cache is busted so after the clock ticks forward, we get all 4 moments
- eventService.startEvent("fourth")
- fakeClock.setCurrentTime(fakeClock.now() + 1L)
- assertEquals(4, eventService.findEventIdsForSession(sessionBeginTime, fakeClock.now()).size)
- }
-
- @Test
- fun `verify close clears the existing events`() {
- eventService.startEvent("event-yeah")
- eventService.close()
- assertTrue(eventService.activeEvents.isEmpty())
- }
-
  @Test
  fun `verify clean collections`() {
  val time = 123L
  fakeClock.setCurrentTime(time)
  val eventName = "event-to-start"
+ val secondEventName = "another-event-to-start"
  val identifier = "identifier"
 
  eventService.startEvent(eventName, identifier)
+ eventService.startEvent(secondEventName, identifier)
+ eventService.endEvent(secondEventName, identifier)
  // assert that active events is not empty
  assertTrue(eventService.activeEvents.isNotEmpty())
- assertTrue(eventService.findEventIdsForSession(time - 1, time + 1).isNotEmpty())
+ assertTrue(eventService.findEventIdsForSession().isNotEmpty())
 
- eventService.cleanCollections()
+ eventService.close()
+ assertEquals("event-to-start#identifier", eventService.activeEvents.keys.single())
+ eventService.endEvent(eventName, identifier)
 
+ eventService.close()
  assertTrue(eventService.activeEvents.isEmpty())
- assertTrue(eventService.findEventIdsForSession(time - 1, time + 1).isEmpty())
+ assertTrue(eventService.findEventIdsForSession().isEmpty())
  }
 
  @Test

embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeEventService.kt

@@ -41,7 +41,7 @@ internal class FakeEventService : EventService {
  TODO("Not yet implemented")
  }
 
- override fun findEventIdsForSession(startTime: Long, endTime: Long): List<String> {
+ override fun findEventIdsForSession(): List<String> {
  return emptyList()
  }