diff --git a/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/session/MomentBoundaryTest.kt b/embrace-android-sdk/src/integrationTest/kotlin/io/embrace/android/embracesdk/session/MomentBoundaryTest.kt new file mode 100644 index 000000000..aae02471a --- /dev/null +++ b/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 +} diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt index eaa883e6d..7c4f33e3a 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EmbraceEventService.kt +++ b/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 = ConcurrentSkipListMap() - private val eventIdsCache = CacheableValue> { eventIds.size } + private val eventIds = ConcurrentLinkedQueue() /** * 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 { - logDeveloper("EmbraceEventService", "findEventIdsForSession") - return eventIdsCache.value { ArrayList(eventIds.subMap(startTime, endTime).values) } - } + override fun findEventIdsForSession(): List = + eventIds.toList() override fun getActiveEventIds(): List { val ids: MutableList = 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) + } } /** diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EventService.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EventService.kt index cfa73feb4..58b96d004 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/event/EventService.kt +++ b/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 + fun findEventIdsForSession(): List /** * Gets all of the IDs of the currently active moments. diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/message/PayloadMessageCollator.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/message/PayloadMessageCollator.kt index df77a08f3..c9fd5ecfe 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/message/PayloadMessageCollator.kt +++ b/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 { diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/event/EmbraceEventServiceTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/event/EmbraceEventServiceTest.kt index 5306edb8e..f5afb6e6c 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/event/EmbraceEventServiceTest.kt +++ b/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 diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeEventService.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeEventService.kt index 1af0d5347..c861b4b9c 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeEventService.kt +++ b/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 { + override fun findEventIdsForSession(): List { return emptyList() }