Merge pull request #67 from embrace-io/gate-session-async

Perform session gating asynchronously
embrace-io · Nov 14, 2023 · 1fd4228 · 1fd4228
2 parents 5168910 + 967e318
commit 1fd4228
Show file tree

Hide file tree

Showing 6 changed files with 61 additions and 63 deletions.
diff --git a/...-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryService.kt b/...-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryService.kt
@@ -1,6 +1,7 @@
 package io.embrace.android.embracesdk.comms.delivery
 
 import io.embrace.android.embracesdk.comms.api.ApiService
+import io.embrace.android.embracesdk.gating.GatingService
 import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
 import io.embrace.android.embracesdk.ndk.NdkService
 import io.embrace.android.embracesdk.payload.BackgroundActivityMessage
@@ -15,6 +16,7 @@ import java.util.concurrent.TimeUnit
 internal class EmbraceDeliveryService(
     private val cacheManager: DeliveryCacheManager,
     private val apiService: ApiService,
+    private val gatingService: GatingService,
     private val cachedSessionsExecutorService: ExecutorService,
     private val sendSessionsExecutorService: ExecutorService,
     private val logger: InternalEmbraceLogger
@@ -33,11 +35,13 @@ internal class EmbraceDeliveryService(
      * point.
      */
     override fun saveSession(sessionMessage: SessionMessage) {
-        cacheManager.saveSession(sessionMessage)
+        val sanitizedSessionMessage = gatingService.gateSessionMessage(sessionMessage)
+        cacheManager.saveSession(sanitizedSessionMessage)
     }
 
     override fun saveSessionOnCrash(sessionMessage: SessionMessage) {
-        cacheManager.saveSessionOnCrash(sessionMessage)
+        val sanitizedSessionMessage = gatingService.gateSessionMessage(sessionMessage)
+        cacheManager.saveSessionOnCrash(sanitizedSessionMessage)
     }
 
     /**
@@ -50,40 +54,50 @@ internal class EmbraceDeliveryService(
         logger.logDeveloper(TAG, "Sending session message")
 
         sendSessionsExecutorService.submit {
-            logger.logDeveloper(TAG, "Sending session message - background job started")
-            val sessionBytes = cacheManager.saveSession(sessionMessage)
+            // sanitize start session message before send it to backend
+            val sanitizedSession = gatingService.gateSessionMessage(sessionMessage)
+            logger.logDebug("Session successfully sanitized.")
+            sendSessionImpl(sanitizedSession, state)
+        }
+    }
 
-            sessionBytes?.also { session ->
-                logger.logDeveloper(TAG, "Serialized session message ready to be sent")
+    private fun sendSessionImpl(
+        sessionMessage: SessionMessage,
+        state: SessionMessageState
+    ) {
+        logger.logDeveloper(TAG, "Sending session message - background job started")
+        val sessionBytes = cacheManager.saveSession(sessionMessage)
 
-                try {
-                    var onFinish: (() -> Unit)? = null
-                    if (state == SessionMessageState.END || state == SessionMessageState.END_WITH_CRASH) {
-                        onFinish = { cacheManager.deleteSession(sessionMessage.session.sessionId) }
-                    }
+        sessionBytes?.also { session ->
+            logger.logDeveloper(TAG, "Serialized session message ready to be sent")
 
-                    if (state == SessionMessageState.END_WITH_CRASH) {
-                        // perform session request synchronously
-                        apiService.sendSession(
-                            session,
-                            onFinish
-                        )?.get(SEND_SESSION_TIMEOUT, TimeUnit.SECONDS)
-                        logger.logDeveloper(TAG, "Session message sent.")
-                    } else {
-                        // perform session request asynchronously
-                        apiService.sendSession(session, onFinish)
-                        logger.logDeveloper(TAG, "Session message queued to be sent.")
-                    }
-                    logger.logDeveloper(
-                        TAG,
-                        "Current session has been successfully removed from cache."
-                    )
-                } catch (ex: Exception) {
-                    logger.logInfo(
-                        "Failed to send session end message. Embrace will store the " +
-                            "session message and attempt to deliver it at a future date."
-                    )
+            try {
+                var onFinish: (() -> Unit)? = null
+                if (state == SessionMessageState.END || state == SessionMessageState.END_WITH_CRASH) {
+                    onFinish = { cacheManager.deleteSession(sessionMessage.session.sessionId) }
+                }
+
+                if (state == SessionMessageState.END_WITH_CRASH) {
+                    // perform session request synchronously
+                    apiService.sendSession(
+                        session,
+                        onFinish
+                    )?.get(SEND_SESSION_TIMEOUT, TimeUnit.SECONDS)
+                    logger.logDeveloper(TAG, "Session message sent.")
+                } else {
+                    // perform session request asynchronously
+                    apiService.sendSession(session, onFinish)
+                    logger.logDeveloper(TAG, "Session message queued to be sent.")
                 }
+                logger.logDeveloper(
+                    TAG,
+                    "Current session has been successfully removed from cache."
+                )
+            } catch (ex: Exception) {
+                logger.logInfo(
+                    "Failed to send session end message. Embrace will store the " +
+                        "session message and attempt to deliver it at a future date."
+                )
             }
         }
     }

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/DeliveryModule.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/DeliveryModule.kt
@@ -22,6 +22,7 @@ internal class DeliveryModuleImpl(
         EmbraceDeliveryService(
             essentialServiceModule.deliveryCacheManager,
             essentialServiceModule.apiService,
+            essentialServiceModule.gatingService,
             cachedSessionsExecutorService,
             sendSessionsExecutorService,
             coreModule.logger

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/SessionModule.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/SessionModule.kt
@@ -39,7 +39,6 @@ internal class SessionModuleImpl(
             essentialServiceModule.userService,
             essentialServiceModule.networkConnectivityService,
             essentialServiceModule.metadataService,
-            essentialServiceModule.gatingService,
             dataCaptureServiceModule.breadcrumbService,
             essentialServiceModule.activityService,
             nativeModule.ndkService,

diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/SessionHandler.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/session/SessionHandler.kt
@@ -15,7 +15,6 @@ import io.embrace.android.embracesdk.comms.delivery.SessionMessageState
 import io.embrace.android.embracesdk.config.ConfigService
 import io.embrace.android.embracesdk.event.EmbraceRemoteLogger
 import io.embrace.android.embracesdk.event.EventService
-import io.embrace.android.embracesdk.gating.GatingService
 import io.embrace.android.embracesdk.internal.MessageType
 import io.embrace.android.embracesdk.internal.clock.Clock
 import io.embrace.android.embracesdk.internal.spans.EmbraceSpanData
@@ -43,7 +42,6 @@ internal class SessionHandler(
     private val userService: UserService,
     private val networkConnectivityService: NetworkConnectivityService,
     private val metadataService: MetadataService,
-    private val gatingService: GatingService,
     private val breadcrumbService: BreadcrumbService,
     private val activityService: ActivityService,
     private val ndkService: NdkService,
@@ -100,11 +98,7 @@ internal class SessionHandler(
 
         metadataService.setActiveSessionId(session.sessionId)
 
-        // sanitize start session message before send it to backend
-        val sanitizedSession = gatingService.gateSessionMessage(sessionMessage)
-        logger.logDebug("Start session successfully sanitized.")
-
-        deliveryService.sendSession(sanitizedSession, SessionMessageState.START)
+        deliveryService.sendSession(sessionMessage, SessionMessageState.START)
         logger.logDebug("Start session successfully sent.")
 
         handleAutomaticSessionStopper(automaticSessionCloserCallback)
@@ -412,16 +406,9 @@ internal class SessionHandler(
         metadataService.removeActiveSessionId(originSession.sessionId)
         logger.logDebug("Services collections successfully cleaned.")
 
-        // Sanitize session message
-        val sanitizedSessionMessage = gatingService.gateSessionMessage(fullEndSessionMessage)
-        logger.logDeveloper(
-            "SessionHandler",
-            "Sanitized End session message=$sanitizedSessionMessage"
-        )
-
         sessionProperties.clearTemporary()
         logger.logDebug("Session properties successfully temporary cleared.")
-        deliveryService.sendSession(sanitizedSessionMessage, SessionMessageState.END)
+        deliveryService.sendSession(fullEndSessionMessage, SessionMessageState.END)
     }
 
     /**
@@ -463,14 +450,7 @@ internal class SessionHandler(
             completedSpans
         )
         logger.logDeveloper("SessionHandler", "End session message=$fullEndSessionMessage")
-
-        // Sanitize session message
-        val sanitizedSessionMessage = gatingService.gateSessionMessage(fullEndSessionMessage)
-        logger.logDeveloper(
-            "SessionHandler",
-            "Sanitized End session message=$sanitizedSessionMessage"
-        )
-        deliveryService.saveSessionOnCrash(sanitizedSessionMessage)
+        deliveryService.saveSessionOnCrash(fullEndSessionMessage)
     }
 
     /**

diff --git a/.../src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryServiceTest.kt b/.../src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryServiceTest.kt
@@ -4,6 +4,7 @@ import com.google.common.util.concurrent.MoreExecutors
 import io.embrace.android.embracesdk.EmbraceEvent
 import io.embrace.android.embracesdk.comms.api.ApiService
 import io.embrace.android.embracesdk.fakeBackgroundActivity
+import io.embrace.android.embracesdk.fakes.FakeGatingService
 import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
 import io.embrace.android.embracesdk.ndk.NdkService
 import io.embrace.android.embracesdk.payload.Event
@@ -15,6 +16,7 @@ import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
 import org.junit.After
+import org.junit.Assert.assertEquals
 import org.junit.Assert.assertNotNull
 import org.junit.BeforeClass
 import org.junit.Test
@@ -29,6 +31,7 @@ internal class EmbraceDeliveryServiceTest {
     companion object {
         private lateinit var mockDeliveryCacheManager: EmbraceDeliveryCacheManager
         private lateinit var apiService: ApiService
+        private lateinit var gatingService: FakeGatingService
         private lateinit var logger: InternalEmbraceLogger
 
         @BeforeClass
@@ -38,6 +41,7 @@ internal class EmbraceDeliveryServiceTest {
             every { mockDeliveryCacheManager.loadCrash() } returns null
             every { mockDeliveryCacheManager.getAllCachedSessionIds() } returns emptyList()
             apiService = mockk(relaxed = true)
+            gatingService = FakeGatingService()
             logger = InternalEmbraceLogger()
         }
     }
@@ -46,12 +50,14 @@ internal class EmbraceDeliveryServiceTest {
     fun after() {
         clearAllMocks()
         executor.shutdown()
+        gatingService.sessionMessagesFiltered.clear()
     }
 
     private fun initializeDeliveryService() {
         deliveryService = EmbraceDeliveryService(
             mockDeliveryCacheManager,
             apiService,
+            gatingService,
             executor,
             executor,
             logger
@@ -68,7 +74,8 @@ internal class EmbraceDeliveryServiceTest {
 
         deliveryService.saveSession(mockSessionMessage)
 
-        verify { mockDeliveryCacheManager.saveSession(mockSessionMessage) }
+        verify(exactly = 1) { mockDeliveryCacheManager.saveSession(mockSessionMessage) }
+        assertEquals(1, gatingService.sessionMessagesFiltered.size)
     }
 
     @Test
@@ -164,6 +171,7 @@ internal class EmbraceDeliveryServiceTest {
             )
         }
         verify { mockFuture wasNot Called }
+        assertEquals(1, gatingService.sessionMessagesFiltered.size)
     }
 
     @Test
@@ -188,6 +196,7 @@ internal class EmbraceDeliveryServiceTest {
             )
         }
         verify { mockFuture wasNot Called }
+        assertEquals(1, gatingService.sessionMessagesFiltered.size)
     }
 
     @Test
@@ -214,6 +223,7 @@ internal class EmbraceDeliveryServiceTest {
         verify(exactly = 1) {
             mockFuture.get(1L, TimeUnit.SECONDS)
         }
+        assertEquals(1, gatingService.sessionMessagesFiltered.size)
     }
 
     @Test

diff --git a/...ace-android-sdk/src/test/java/io/embrace/android/embracesdk/session/SessionHandlerTest.kt b/...ace-android-sdk/src/test/java/io/embrace/android/embracesdk/session/SessionHandlerTest.kt
@@ -155,7 +155,6 @@ internal class SessionHandlerTest {
             mockUserService,
             mockNetworkConnectivityService,
             metadataService,
-            gatingService,
             mockBreadcrumbService,
             activityService,
             mockNdkService,
@@ -205,8 +204,6 @@ internal class SessionHandlerTest {
         verify { mockNetworkConnectivityService.networkStatusOnSessionStarted(now) }
         // verify active session is set
         assertEquals(sessionUuid, metadataService.activeSessionId)
-        // verify session is being sanitized
-        assertEquals(1, gatingService.sessionMessagesFiltered.size)
         // verify automatic session stopper has been scheduled
         verify {
             mockAutomaticSessionStopper.scheduleWithFixedDelay(
@@ -366,8 +363,6 @@ internal class SessionHandlerTest {
                 mockExceptionService
             )
         }
-        // verify we are sanitizing session message
-        assertEquals(1, gatingService.sessionMessagesFiltered.size)
         // verify current session is removed from cache
         verify { mockSessionProperties.clearTemporary() }
     }
@@ -497,7 +492,6 @@ internal class SessionHandlerTest {
         verify { mockMemoryCleanerService wasNot Called }
         verify(exactly = 0) { mockSessionProperties.clearTemporary() }
         assertTrue(deliveryService.lastSentSessions.isEmpty())
-        assertEquals(1, gatingService.sessionMessagesFiltered.size)
 
         val session = checkNotNull(deliveryService.lastSavedSession).session