From 4dae99c7685c27d45571cd86dd4fe6a684b097ac Mon Sep 17 00:00:00 2001 From: Lucas Date: Tue, 7 Nov 2023 13:08:19 -0300 Subject: [PATCH] Move Delivery Retry logic outside EmbraceApiService. --- .../embracesdk/comms/api/EmbraceApiService.kt | 188 +---------- .../comms/delivery/DeliveryRetryManager.kt | 19 ++ .../delivery/EmbraceDeliveryRetryManager.kt | 217 +++++++++++++ .../injection/EssentialServiceModule.kt | 12 + .../comms/api/EmbraceApiServiceTest.kt | 255 +-------------- .../EmbraceDeliveryRetryManagerTest.kt | 302 ++++++++++++++++++ .../fakes/FakeDeliveryRetryManager.kt | 14 + .../injection/FakeEssentialServiceModule.kt | 3 + 8 files changed, 586 insertions(+), 424 deletions(-) create mode 100644 embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/DeliveryRetryManager.kt create mode 100644 embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManager.kt create mode 100644 embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManagerTest.kt create mode 100644 embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeDeliveryRetryManager.kt diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt index 7f8a6172aa..3c73da0c3b 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/api/EmbraceApiService.kt @@ -6,8 +6,7 @@ import io.embrace.android.embracesdk.EmbraceEvent import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityListener import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityService import io.embrace.android.embracesdk.comms.delivery.DeliveryCacheManager -import io.embrace.android.embracesdk.comms.delivery.DeliveryFailedApiCall -import io.embrace.android.embracesdk.comms.delivery.DeliveryFailedApiCalls +import io.embrace.android.embracesdk.comms.delivery.DeliveryRetryManager import io.embrace.android.embracesdk.comms.delivery.NetworkStatus import io.embrace.android.embracesdk.config.remote.RemoteConfig import io.embrace.android.embracesdk.internal.EmbraceSerializer @@ -20,11 +19,8 @@ import io.embrace.android.embracesdk.utils.exceptions.Unchecked import java.io.StringReader import java.net.HttpURLConnection import java.util.concurrent.Future -import java.util.concurrent.RejectedExecutionException import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.ScheduledFuture import java.util.concurrent.TimeUnit -import kotlin.math.max internal class EmbraceApiService( private val apiClient: ApiClient, @@ -35,22 +31,18 @@ internal class EmbraceApiService( private val scheduledExecutorService: ScheduledExecutorService, networkConnectivityService: NetworkConnectivityService, private val cacheManager: DeliveryCacheManager, + private val deliveryRetryManager: DeliveryRetryManager, private val lazyDeviceId: Lazy, private val appId: String ) : ApiService, NetworkConnectivityListener { - private val retryQueue: DeliveryFailedApiCalls by lazy { cacheManager.loadFailedApiCalls() } - private var lastRetryTask: ScheduledFuture<*>? = null private var lastNetworkStatus: NetworkStatus = NetworkStatus.UNKNOWN init { logger.logDeveloper(TAG, "start") - networkConnectivityService.addNetworkConnectivityListener(this) lastNetworkStatus = networkConnectivityService.getCurrentNetworkStatus() - scheduledExecutorService.submit( - this::scheduleFailedApiCallsRetry - ) + deliveryRetryManager.setPostExecutor(this::executePost) } /** @@ -118,27 +110,6 @@ internal class EmbraceApiService( override fun onNetworkConnectivityStatusChanged(status: NetworkStatus) { lastNetworkStatus = status - logger.logDebug("Network status is now: $lastNetworkStatus") - when (status) { - NetworkStatus.UNKNOWN, - NetworkStatus.WIFI, - NetworkStatus.WAN -> { - scheduleFailedApiCallsRetry() - } - - NetworkStatus.NOT_REACHABLE -> { - synchronized(this) { - lastRetryTask?.let { task -> - if (task.cancel(false)) { - logger.logDebug("Failed Calls Retry Action was stopped because there is no connection. ") - lastRetryTask = null - } else { - logger.logError("Failed Calls Retry Action could not be stopped.") - } - } - } - } - } } /** @@ -263,19 +234,6 @@ internal class EmbraceApiService( return postOnExecutor(sessionPayload, request, true, onFinish) } - /** - * Returns true if there is an active pending retry task - */ - fun isRetryTaskActive(): Boolean = - lastRetryTask?.let { task -> - !task.isCancelled && !task.isDone - } ?: false - - /** - * Returns the number of failed API calls that will be retried - */ - fun pendingRetriesCount() = retryQueue.size - private fun createRequest(eventMessage: EventMessage): ApiRequest { logger.logDeveloper(TAG, "sendEvent") checkNotNull(eventMessage.event) { "event must be set" } @@ -355,17 +313,17 @@ internal class EmbraceApiService( try { if (lastNetworkStatus != NetworkStatus.NOT_REACHABLE) { if (compress) { - apiClient.post(request, payload) + executePost(request, payload) } else { - apiClient.rawPost(request, payload) + executeRawPost(request, payload) } } else { - scheduleForRetry(request, payload) + deliveryRetryManager.scheduleForRetry(request, payload) logger.logWarning("No connection available. Request was queued to retry later.") } } catch (ex: Exception) { logger.logWarning("Failed to post Embrace API call. Will retry.", ex) - scheduleForRetry(request, payload) + deliveryRetryManager.scheduleForRetry(request, payload) throw ex } finally { onComplete?.invoke() @@ -400,136 +358,14 @@ internal class EmbraceApiService( return "$abbreviation:$stories" } - private fun scheduleForRetry(request: ApiRequest, payload: ByteArray) { - logger.logDeveloper(TAG, "Scheduling api call for retry") - if (pendingRetriesCount() < MAX_FAILED_CALLS) { - val scheduleJob = retryQueue.isEmpty() - val cachedPayloadName = cacheManager.savePayload(payload) - val failedApiCall = DeliveryFailedApiCall(request, cachedPayloadName) - retryQueue.add(failedApiCall) - cacheManager.saveFailedApiCalls(retryQueue) - - // By default there are no scheduled retry jobs pending. If the retry queue was initially empty, try to schedule a retry. - if (scheduleJob) { - scheduleFailedApiCallsRetry(RETRY_PERIOD) - } - } - } - - /** - * Return true if the conditions are met that a retry should be scheduled - */ - private fun shouldScheduleRetry(): Boolean { - return !isRetryTaskActive() && retryQueue.isNotEmpty() - } - - /** - * Schedules an action to retry failed API calls. If the retry doesn't send all the failed API requests, it will recursively schedule - * itself with an exponential backoff delay, starting with [RETRY_PERIOD], doubling after that until - * [MAX_EXPONENTIAL_RETRY_PERIOD] is reached, after which case it stops trying until the next cold start. - */ - private fun scheduleFailedApiCallsRetry(delayInSeconds: Long = 0L) { - try { - synchronized(this) { - if (shouldScheduleRetry()) { - lastRetryTask = scheduledExecutorService.schedule( - { - var noFailedRetries = true - if (lastNetworkStatus != NetworkStatus.NOT_REACHABLE) { - try { - logger.logInfo("Retrying failed API calls") - logger.logDeveloper(TAG, "Retrying failed API calls") - val retries = pendingRetriesCount() - repeat(retries) { - retryQueue.poll()?.let { failedApiCall -> - val callSucceeded = retryFailedApiCall(failedApiCall) - if (callSucceeded) { - // if the retry succeeded, save the modified queue in cache. - cacheManager.saveFailedApiCalls(retryQueue) - } else { - // if the retry failed, add the call back to the queue. - retryQueue.add(failedApiCall) - noFailedRetries = false - } - } - } - } catch (ex: Exception) { - logger.logDebug("Error when retrying failed API call", ex) - } - if (retryQueue.isNotEmpty()) { - scheduledExecutorService.submit { - scheduleNextFailedApiCallsRetry( - noFailedRetries, - delayInSeconds - ) - } - } - } else { - logger.logInfo( - "Did not retry network calls as scheduled because the network is not reachable" - ) - } - }, - delayInSeconds, - TimeUnit.SECONDS - ) - logger.logInfo( - "Scheduled failed API calls to retry ${if (delayInSeconds == 0L) "now" else "in $delayInSeconds seconds"}" - ) - } - } - } catch (e: RejectedExecutionException) { - // This happens if the executor has shutdown previous to the schedule call - logger.logError("Cannot schedule retry failed calls.", e) - } + private fun executePost(request: ApiRequest, payload: ByteArray) { + apiClient.post(request, payload) } - /** - * Executes the network call for a DeliveryFailedApiCall. - */ - private fun retryFailedApiCall(call: DeliveryFailedApiCall): Boolean { - val payload = cacheManager.loadPayload(call.cachedPayload) - if (payload != null) { - try { - logger.logDeveloper(TAG, "Retrying failed API call") - apiClient.post(call.apiRequest, payload) - cacheManager.deletePayload(call.cachedPayload) - } catch (ex: Exception) { - logger.logDeveloper( - TAG, - "retried call but fail again, scheduling to retry later", - ex - ) - return false - } - } else { - logger.logError("Could not retrieve cached api payload") - // If payload is null, the file could have been removed. - // We don't need to retry sending in the future as we'd get the same result. - // That's the reason for returning true. - } - return true - } - - /** - * Schedules the next call to retry sending the failed_api_calls again. The delay will be extended if the previous retry yielded - * at least one failed request. - */ - private fun scheduleNextFailedApiCallsRetry(noFailedRetries: Boolean, delay: Long) { - val nextDelay = if (noFailedRetries) { - RETRY_PERIOD - } else { - // if a network call failed, the retries will use exponential backoff - max(RETRY_PERIOD, delay * 2) - } - if (nextDelay <= MAX_EXPONENTIAL_RETRY_PERIOD) { - scheduleFailedApiCallsRetry(nextDelay) - } + private fun executeRawPost(request: ApiRequest, payload: ByteArray) { + apiClient.rawPost(request, payload) } } -private const val TAG = "DeliveryNetworkManager" +private const val TAG = "EmbraceApiService" private const val CRASH_TIMEOUT = 1L // Seconds to wait before timing out when sending a crash -private const val RETRY_PERIOD = 120L // In seconds -private const val MAX_EXPONENTIAL_RETRY_PERIOD = 3600 // In seconds -private const val MAX_FAILED_CALLS = 200 // Max number of failed calls that will be cached for retry diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/DeliveryRetryManager.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/DeliveryRetryManager.kt new file mode 100644 index 0000000000..5622ec4081 --- /dev/null +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/DeliveryRetryManager.kt @@ -0,0 +1,19 @@ +package io.embrace.android.embracesdk.comms.delivery + +import io.embrace.android.embracesdk.comms.api.ApiRequest + +/** + * Manages the retrying of failed API calls. + */ +internal interface DeliveryRetryManager { + + /** + * Schedules a failed API call for retry. + */ + fun scheduleForRetry(request: ApiRequest, payload: ByteArray) + + /** + * Sets the executor that will be used to retry failed API calls. + */ + fun setPostExecutor(postExecutor: (request: ApiRequest, payload: ByteArray) -> Unit) +} diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManager.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManager.kt new file mode 100644 index 0000000000..a12b1152f9 --- /dev/null +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManager.kt @@ -0,0 +1,217 @@ +package io.embrace.android.embracesdk.comms.delivery + +import androidx.annotation.VisibleForTesting +import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityListener +import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityService +import io.embrace.android.embracesdk.comms.api.ApiRequest +import io.embrace.android.embracesdk.logging.InternalStaticEmbraceLogger.Companion.logger +import java.util.concurrent.RejectedExecutionException +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.ScheduledFuture +import java.util.concurrent.TimeUnit +import kotlin.math.max + +internal class EmbraceDeliveryRetryManager( + networkConnectivityService: NetworkConnectivityService, + private val scheduledExecutorService: ScheduledExecutorService, + private val cacheManager: DeliveryCacheManager +) : DeliveryRetryManager, NetworkConnectivityListener { + + private val retryQueue: DeliveryFailedApiCalls by lazy { cacheManager.loadFailedApiCalls() } + private var lastRetryTask: ScheduledFuture<*>? = null + private var lastNetworkStatus: NetworkStatus = NetworkStatus.UNKNOWN + private lateinit var postExecutor: (request: ApiRequest, payload: ByteArray) -> Unit + + init { + logger.logDeveloper(TAG, "Starting DeliveryRetryManager") + networkConnectivityService.addNetworkConnectivityListener(this) + lastNetworkStatus = networkConnectivityService.getCurrentNetworkStatus() + scheduledExecutorService.submit( + this::scheduleFailedApiCallsRetry + ) + } + + /** + * Sets the executor that will be used to retry failed API calls. + */ + override fun setPostExecutor(postExecutor: (request: ApiRequest, payload: ByteArray) -> Unit) { + this.postExecutor = postExecutor + } + + /** + * Schedules a failed API call for retry. + */ + override fun scheduleForRetry(request: ApiRequest, payload: ByteArray) { + logger.logDeveloper(TAG, "Scheduling api call for retry") + if (pendingRetriesCount() < MAX_FAILED_CALLS) { + val scheduleJob = retryQueue.isEmpty() + val cachedPayloadName = cacheManager.savePayload(payload) + val failedApiCall = DeliveryFailedApiCall(request, cachedPayloadName) + retryQueue.add(failedApiCall) + cacheManager.saveFailedApiCalls(retryQueue) + + // By default there are no scheduled retry jobs pending. If the retry queue was initially empty, try to schedule a retry. + if (scheduleJob) { + scheduleFailedApiCallsRetry(RETRY_PERIOD) + } + } + } + + /** + * Called when the network status has changed. + */ + override fun onNetworkConnectivityStatusChanged(status: NetworkStatus) { + lastNetworkStatus = status + when (status) { + NetworkStatus.UNKNOWN, + NetworkStatus.WIFI, + NetworkStatus.WAN -> { + scheduleFailedApiCallsRetry() + } + + NetworkStatus.NOT_REACHABLE -> { + synchronized(this) { + lastRetryTask?.let { task -> + if (task.cancel(false)) { + logger.logDebug("Failed Calls Retry Action was stopped because there is no connection. ") + lastRetryTask = null + } else { + logger.logError("Failed Calls Retry Action could not be stopped.") + } + } + } + } + } + } + + /** + * Returns true if there is an active pending retry task + */ + @VisibleForTesting + fun isRetryTaskActive(): Boolean = + lastRetryTask?.let { task -> + !task.isCancelled && !task.isDone + } ?: false + + /** + * Returns the number of failed API calls that will be retried + */ + @VisibleForTesting + fun pendingRetriesCount() = retryQueue.size + + /** + * Return true if the conditions are met that a retry should be scheduled + */ + private fun shouldScheduleRetry(): Boolean { + return !isRetryTaskActive() && retryQueue.isNotEmpty() + } + + /** + * Schedules an action to retry failed API calls. If the retry doesn't send all the failed API requests, it will recursively schedule + * itself with an exponential backoff delay, starting with [RETRY_PERIOD], doubling after that until + * [MAX_EXPONENTIAL_RETRY_PERIOD] is reached, after which case it stops trying until the next cold start. + */ + private fun scheduleFailedApiCallsRetry(delayInSeconds: Long = 0L) { + try { + synchronized(this) { + if (shouldScheduleRetry()) { + lastRetryTask = scheduledExecutorService.schedule( + { + var noFailedRetries = true + if (lastNetworkStatus != NetworkStatus.NOT_REACHABLE) { + try { + logger.logInfo("Retrying failed API calls") + logger.logDeveloper(TAG, "Retrying failed API calls") + val retries = pendingRetriesCount() + repeat(retries) { + retryQueue.poll()?.let { failedApiCall -> + val callSucceeded = retryFailedApiCall(failedApiCall) + if (callSucceeded) { + // if the retry succeeded, save the modified queue in cache. + cacheManager.saveFailedApiCalls(retryQueue) + } else { + // if the retry failed, add the call back to the queue. + retryQueue.add(failedApiCall) + noFailedRetries = false + } + } + } + } catch (ex: Exception) { + logger.logDebug("Error when retrying failed API call", ex) + } + if (retryQueue.isNotEmpty()) { + scheduledExecutorService.submit { + scheduleNextFailedApiCallsRetry( + noFailedRetries, + delayInSeconds + ) + } + } + } else { + logger.logInfo( + "Did not retry network calls as scheduled because the network is not reachable" + ) + } + }, + delayInSeconds, + TimeUnit.SECONDS + ) + logger.logInfo( + "Scheduled failed API calls to retry ${if (delayInSeconds == 0L) "now" else "in $delayInSeconds seconds"}" + ) + } + } + } catch (e: RejectedExecutionException) { + // This happens if the executor has shutdown previous to the schedule call + logger.logError("Cannot schedule retry failed calls.", e) + } + } + + /** + * Executes the network call for a DeliveryFailedApiCall. + */ + private fun retryFailedApiCall(call: DeliveryFailedApiCall): Boolean { + val payload = cacheManager.loadPayload(call.cachedPayload) + if (payload != null) { + try { + logger.logDeveloper(TAG, "Retrying failed API call") + postExecutor(call.apiRequest, payload) + cacheManager.deletePayload(call.cachedPayload) + } catch (ex: Exception) { + logger.logDeveloper( + TAG, + "retried call but fail again, scheduling to retry later", + ex + ) + return false + } + } else { + logger.logError("Could not retrieve cached api payload") + // If payload is null, the file could have been removed. + // We don't need to retry sending in the future as we'd get the same result. + // That's the reason for returning true. + } + return true + } + + /** + * Schedules the next call to retry sending the failed_api_calls again. The delay will be extended if the previous retry yielded + * at least one failed request. + */ + private fun scheduleNextFailedApiCallsRetry(noFailedRetries: Boolean, delay: Long) { + val nextDelay = if (noFailedRetries) { + RETRY_PERIOD + } else { + // if a network call failed, the retries will use exponential backoff + max(RETRY_PERIOD, delay * 2) + } + if (nextDelay <= MAX_EXPONENTIAL_RETRY_PERIOD) { + scheduleFailedApiCallsRetry(nextDelay) + } + } +} + +private const val TAG = "DeliveryRetryManager" +private const val RETRY_PERIOD = 120L // In seconds +private const val MAX_EXPONENTIAL_RETRY_PERIOD = 3600 // In seconds +private const val MAX_FAILED_CALLS = 200 // Max number of failed calls that will be cached for retry diff --git a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/EssentialServiceModule.kt b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/EssentialServiceModule.kt index 00a98d4ab4..4a428b0320 100644 --- a/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/EssentialServiceModule.kt +++ b/embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/injection/EssentialServiceModule.kt @@ -20,8 +20,10 @@ import io.embrace.android.embracesdk.comms.api.EmbraceApiService import io.embrace.android.embracesdk.comms.api.EmbraceApiUrlBuilder import io.embrace.android.embracesdk.comms.delivery.CacheService import io.embrace.android.embracesdk.comms.delivery.DeliveryCacheManager +import io.embrace.android.embracesdk.comms.delivery.DeliveryRetryManager import io.embrace.android.embracesdk.comms.delivery.EmbraceCacheService import io.embrace.android.embracesdk.comms.delivery.EmbraceDeliveryCacheManager +import io.embrace.android.embracesdk.comms.delivery.EmbraceDeliveryRetryManager import io.embrace.android.embracesdk.config.ConfigService import io.embrace.android.embracesdk.config.EmbraceConfigService import io.embrace.android.embracesdk.config.behavior.AutoDataCaptureBehavior @@ -65,6 +67,7 @@ internal interface EssentialServiceModule { val networkConnectivityService: NetworkConnectivityService val cacheService: CacheService val deliveryCacheManager: DeliveryCacheManager + val deliveryRetryManager: DeliveryRetryManager } internal class EssentialServiceModuleImpl( @@ -263,6 +266,14 @@ internal class EssentialServiceModuleImpl( ) } + override val deliveryRetryManager: DeliveryRetryManager by singleton { + EmbraceDeliveryRetryManager( + networkConnectivityService, + apiRetryExecutor, + deliveryCacheManager + ) + } + override val apiService: ApiService by singleton { EmbraceApiService( apiClient, @@ -273,6 +284,7 @@ internal class EssentialServiceModuleImpl( apiRetryExecutor, networkConnectivityService, deliveryCacheManager, + deliveryRetryManager, lazyDeviceId, appId ) diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt index ffa419295b..1887f70d4d 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/api/EmbraceApiServiceTest.kt @@ -3,8 +3,6 @@ package io.embrace.android.embracesdk.comms.api import io.embrace.android.embracesdk.ResourceReader import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityService import io.embrace.android.embracesdk.comms.delivery.DeliveryCacheManager -import io.embrace.android.embracesdk.comms.delivery.DeliveryFailedApiCall -import io.embrace.android.embracesdk.comms.delivery.DeliveryFailedApiCalls import io.embrace.android.embracesdk.comms.delivery.NetworkStatus import io.embrace.android.embracesdk.concurrency.BlockingScheduledExecutorService import io.embrace.android.embracesdk.config.remote.RemoteConfig @@ -13,7 +11,6 @@ import io.mockk.clearMocks import io.mockk.every import io.mockk.mockk import io.mockk.unmockkAll -import io.mockk.verify import org.junit.After import org.junit.AfterClass import org.junit.Assert.assertEquals @@ -24,21 +21,16 @@ import org.junit.Before import org.junit.BeforeClass import org.junit.Test import java.util.concurrent.ScheduledExecutorService -import java.util.concurrent.TimeUnit internal class EmbraceApiServiceTest { companion object { - private val connectedNetworkStatuses = - NetworkStatus.values().filter { it != NetworkStatus.NOT_REACHABLE } - private lateinit var mockApiUrlBuilder: ApiUrlBuilder private lateinit var mockApiClient: ApiClient private lateinit var mockCacheManager: DeliveryCacheManager private lateinit var blockingScheduledExecutorService: BlockingScheduledExecutorService private lateinit var testScheduledExecutor: ScheduledExecutorService private lateinit var networkConnectivityService: NetworkConnectivityService - private lateinit var failedApiCalls: DeliveryFailedApiCalls private lateinit var cachedConfig: CachedConfig private lateinit var apiService: EmbraceApiService @@ -50,6 +42,8 @@ internal class EmbraceApiServiceTest { every { getConfigUrl() } returns "https://config.url" } networkConnectivityService = mockk(relaxUnitFun = true) + blockingScheduledExecutorService = BlockingScheduledExecutorService() + testScheduledExecutor = blockingScheduledExecutorService } /** @@ -71,12 +65,7 @@ internal class EmbraceApiServiceTest { config = null, eTag = null ) - failedApiCalls = DeliveryFailedApiCalls() - clearApiPipeline() mockCacheManager = mockk(relaxUnitFun = true) - every { mockCacheManager.loadPayload("cached_payload_1") } returns "{payload 1}".toByteArray() - every { mockCacheManager.loadFailedApiCalls() } returns failedApiCalls - every { mockCacheManager.savePayload(any()) } returns "fake_cache" } @After @@ -94,8 +83,7 @@ internal class EmbraceApiServiceTest { headers = emptyMap() ) initApiService( - status = NetworkStatus.NOT_REACHABLE, - runRetryJobAfterScheduling = true + status = NetworkStatus.NOT_REACHABLE ) val remoteConfig = apiService.getConfig() @@ -110,8 +98,7 @@ internal class EmbraceApiServiceTest { fun `test getConfig rethrows an exception thrown by apiClient`() { every { mockApiClient.executeGet(any()) } throws IllegalStateException("Test exception message") initApiService( - status = NetworkStatus.NOT_REACHABLE, - runRetryJobAfterScheduling = true + status = NetworkStatus.NOT_REACHABLE ) // exception will be thrown and caught by this test's annotation apiService.getConfig() @@ -126,213 +113,12 @@ internal class EmbraceApiServiceTest { body = "", headers = emptyMap() ) - initApiService( - status = NetworkStatus.NOT_REACHABLE, - runRetryJobAfterScheduling = true - ) + initApiService() val remoteConfig = apiService.getConfig() assertSame(cfg, remoteConfig) } - @Test - fun `scheduled retry job active at init time`() { - connectedNetworkStatuses.forEach { status -> - initApiService(status = status, runRetryJobAfterScheduling = true) - retryTaskActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask is not active and doesn't run if there are no failed API requests`() { - connectedNetworkStatuses.forEach { status -> - initApiService( - status = status, - loadFailedRequest = false, - runRetryJobAfterScheduling = true - ) - retryTaskNotActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkNoApiRequestSent() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask is active and runs after init if network is connected`() { - connectedNetworkStatuses.forEach { status -> - initApiService(status = status, runRetryJobAfterScheduling = true) - retryTaskActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkRequestSendAttempt() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask will be scheduled again if retry fails`() { - connectedNetworkStatuses.forEach { status -> - every { mockApiClient.post(any(), any()) } throws Exception() - initApiService(status = status, runRetryJobAfterScheduling = true) - retryTaskActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkRequestSendAttempt() - retryTaskNotActive(status) - // Previous failed attempt will queue another retry. Let it run so a new retry task is active - blockingScheduledExecutorService.runCurrentlyBlocked() - retryTaskActive(status) - - // First failure will result in another retry in 120 seconds - // Go most of the way to check it didn't run - blockingScheduledExecutorService.moveForwardAndRunBlocked( - TimeUnit.SECONDS.toMillis(119L) - ) - retryTaskActive(status) - checkRequestSendAttempt() - - // Go the full 120 seconds and check that the retry runs and fails - blockingScheduledExecutorService.moveForwardAndRunBlocked( - TimeUnit.SECONDS.toMillis(1L) - ) - checkRequestSendAttempt(count = 2) - - // Previous failed attempt will queue another retry. Let it run - blockingScheduledExecutorService.runCurrentlyBlocked() - - // Let the next retry succeed - every { mockApiClient.post(any(), any()) } returns "" - - // Second failure will result in another retry in double the last time, 240 seconds - // Go most of the way to check it didn't run, then go all the way to check that it did. - blockingScheduledExecutorService.moveForwardAndRunBlocked( - TimeUnit.SECONDS.toMillis(239L) - ) - retryTaskActive(status) - checkRequestSendAttempt(count = 2) - blockingScheduledExecutorService.moveForwardAndRunBlocked( - TimeUnit.SECONDS.toMillis(1L) - ) - retryTaskNotActive(status) - checkRequestSendAttempt(count = 3) - clearApiPipeline() - } - } - - @Test - fun `retryTask is not active and doesn't run after init if network not reachable`() { - initApiService( - status = NetworkStatus.NOT_REACHABLE, - runRetryJobAfterScheduling = true - ) - blockingScheduledExecutorService.runCurrentlyBlocked() - retryTaskNotActive(NetworkStatus.NOT_REACHABLE) - checkNoApiRequestSent() - } - - @Test - fun `retryTask isn't active and won't run if there are no failed requests after getting a connection before retry job is scheduled`() { - connectedNetworkStatuses.forEach { status -> - initApiService( - status = NetworkStatus.NOT_REACHABLE, - loadFailedRequest = false, - runRetryJobAfterScheduling = true - ) - blockingScheduledExecutorService.runCurrentlyBlocked() - apiService.onNetworkConnectivityStatusChanged(status) - retryTaskNotActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkNoApiRequestSent() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask is active and runs after connection changes from not reachable to connected after retry job runs`() { - connectedNetworkStatuses.forEach { status -> - initApiService( - status = NetworkStatus.NOT_REACHABLE, - runRetryJobAfterScheduling = true - ) - blockingScheduledExecutorService.runCurrentlyBlocked() - apiService.onNetworkConnectivityStatusChanged(status) - retryTaskActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkRequestSendAttempt() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask isn't active and doesn't run if there are no failed request after getting a connection before retry job is scheduled`() { - connectedNetworkStatuses.forEach { status -> - initApiService( - status = NetworkStatus.NOT_REACHABLE, - loadFailedRequest = false - ) - apiService.onNetworkConnectivityStatusChanged(status) - retryTaskNotActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkNoApiRequestSent() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask is active and runs after connection changes from not reachable to connected before retry job is scheduled`() { - connectedNetworkStatuses.forEach { status -> - initApiService(status = NetworkStatus.NOT_REACHABLE) - apiService.onNetworkConnectivityStatusChanged(status) - retryTaskActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkRequestSendAttempt() - retryTaskNotActive(status) - clearApiPipeline() - } - } - - @Test - fun `retryTask is not active and doesn't run after connection changes from connected to not reachable before retry job is scheduled`() { - connectedNetworkStatuses.forEach { status -> - initApiService(status = status) - apiService.onNetworkConnectivityStatusChanged(NetworkStatus.NOT_REACHABLE) - retryTaskNotActive(status) - blockingScheduledExecutorService.runCurrentlyBlocked() - checkNoApiRequestSent() - } - } - - @Test - fun `queue size should be bounded`() { - initApiService(status = NetworkStatus.WIFI, loadFailedRequest = false) - every { mockApiClient.post(any(), any()) } throws Exception() - - assertEquals(0, apiService.pendingRetriesCount()) - - repeat(201) { - apiService.sendSession("{ dummy_session }".toByteArray(), null) - blockingScheduledExecutorService.runCurrentlyBlocked() - } - assertEquals(200, apiService.pendingRetriesCount()) - } - - private fun clearApiPipeline() { - clearMocks(mockApiClient, answers = false) - failedApiCalls.clear() - blockingScheduledExecutorService = BlockingScheduledExecutorService() - testScheduledExecutor = - blockingScheduledExecutorService - } - - private fun initApiService( - status: NetworkStatus, - loadFailedRequest: Boolean = true, - runRetryJobAfterScheduling: Boolean = false - ) { + private fun initApiService(status: NetworkStatus = NetworkStatus.NOT_REACHABLE) { every { networkConnectivityService.getCurrentNetworkStatus() } returns status apiService = EmbraceApiService( @@ -346,34 +132,7 @@ internal class EmbraceApiServiceTest { cacheManager = mockCacheManager, lazyDeviceId = lazy { "07D85B44E4E245F4A30E559BFC0D07FF" }, appId = "o0o0o", + deliveryRetryManager = mockk(relaxed = true) ) - - failedApiCalls.clear() - if (loadFailedRequest) { - failedApiCalls.add(DeliveryFailedApiCall(mockk(), "cached_payload_1")) - } - - if (runRetryJobAfterScheduling) { - blockingScheduledExecutorService.runCurrentlyBlocked() - } - } - - private fun retryTaskActive(status: NetworkStatus) { - assertTrue("Failed for network status = $status", apiService.isRetryTaskActive()) - } - - private fun retryTaskNotActive(status: NetworkStatus) { - assertFalse( - "Failed for network status = $status", - apiService.isRetryTaskActive() - ) - } - - private fun checkRequestSendAttempt(count: Int = 1) { - verify(exactly = count) { mockApiClient.post(any(), "{payload 1}".toByteArray()) } - } - - private fun checkNoApiRequestSent() { - verify(exactly = 0) { mockApiClient.post(any(), any()) } } } diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManagerTest.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManagerTest.kt new file mode 100644 index 0000000000..c6af61e40c --- /dev/null +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/comms/delivery/EmbraceDeliveryRetryManagerTest.kt @@ -0,0 +1,302 @@ +package io.embrace.android.embracesdk.comms.delivery + +import io.embrace.android.embracesdk.capture.connectivity.NetworkConnectivityService +import io.embrace.android.embracesdk.comms.api.ApiRequest +import io.embrace.android.embracesdk.concurrency.BlockingScheduledExecutorService +import io.mockk.clearMocks +import io.mockk.every +import io.mockk.mockk +import io.mockk.unmockkAll +import io.mockk.verify +import org.junit.After +import org.junit.AfterClass +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.BeforeClass +import org.junit.Test +import java.util.concurrent.ScheduledExecutorService +import java.util.concurrent.TimeUnit + +internal class EmbraceDeliveryRetryManagerTest { + + companion object { + private val connectedNetworkStatuses = + NetworkStatus.values().filter { it != NetworkStatus.NOT_REACHABLE } + + private lateinit var networkConnectivityService: NetworkConnectivityService + private lateinit var blockingScheduledExecutorService: BlockingScheduledExecutorService + private lateinit var mockCacheManager: DeliveryCacheManager + private lateinit var testScheduledExecutor: ScheduledExecutorService + private lateinit var failedApiCalls: DeliveryFailedApiCalls + private lateinit var deliveryRetryManager: EmbraceDeliveryRetryManager + private lateinit var mockPostExecutor: (request: ApiRequest, payload: ByteArray) -> Unit + + @BeforeClass + @JvmStatic + fun setupBeforeAll() { + networkConnectivityService = mockk(relaxUnitFun = true) + } + + /** + * Setup after all tests get executed. Un-mock all here. + */ + @AfterClass + @JvmStatic + fun tearDownAfterAll() { + unmockkAll() + } + } + + @Before + fun setUp() { + blockingScheduledExecutorService = BlockingScheduledExecutorService() + testScheduledExecutor = blockingScheduledExecutorService + failedApiCalls = DeliveryFailedApiCalls() + mockPostExecutor = mockk(relaxUnitFun = true) + clearApiPipeline() + mockCacheManager = mockk(relaxUnitFun = true) + every { mockCacheManager.loadPayload("cached_payload_1") } returns "{payload 1}".toByteArray() + every { mockCacheManager.loadFailedApiCalls() } returns failedApiCalls + every { mockCacheManager.savePayload(any()) } returns "fake_cache" + } + + @After + fun tearDown() { + clearMocks(mockCacheManager) + } + + @Test + fun `scheduled retry job active at init time`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager(status = status, runRetryJobAfterScheduling = true) + retryTaskActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask is not active and doesn't run if there are no failed API requests`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager( + status = status, loadFailedRequest = false, runRetryJobAfterScheduling = true + ) + retryTaskNotActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkNoApiRequestSent() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask is active and runs after init if network is connected`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager(status = status, runRetryJobAfterScheduling = true) + retryTaskActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkRequestSendAttempt() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask will be scheduled again if retry fails`() { + connectedNetworkStatuses.forEach { status -> + every { mockPostExecutor(any(), any()) } throws Exception() + initDeliveryRetryManager(status = status, runRetryJobAfterScheduling = true) + retryTaskActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkRequestSendAttempt() + retryTaskNotActive(status) + // Previous failed attempt will queue another retry. Let it run so a new retry task is active + blockingScheduledExecutorService.runCurrentlyBlocked() + retryTaskActive(status) + + // First failure will result in another retry in 120 seconds + // Go most of the way to check it didn't run + blockingScheduledExecutorService.moveForwardAndRunBlocked( + TimeUnit.SECONDS.toMillis(119L) + ) + retryTaskActive(status) + checkRequestSendAttempt() + + // Go the full 120 seconds and check that the retry runs and fails + blockingScheduledExecutorService.moveForwardAndRunBlocked( + TimeUnit.SECONDS.toMillis(1L) + ) + checkRequestSendAttempt(count = 2) + + // Previous failed attempt will queue another retry. Let it run + blockingScheduledExecutorService.runCurrentlyBlocked() + + // Let the next retry succeed + every { mockPostExecutor(any(), any()) } returns Unit + + // Second failure will result in another retry in double the last time, 240 seconds + // Go most of the way to check it didn't run, then go all the way to check that it did. + blockingScheduledExecutorService.moveForwardAndRunBlocked( + TimeUnit.SECONDS.toMillis(239L) + ) + retryTaskActive(status) + checkRequestSendAttempt(count = 2) + blockingScheduledExecutorService.moveForwardAndRunBlocked( + TimeUnit.SECONDS.toMillis(1L) + ) + retryTaskNotActive(status) + checkRequestSendAttempt(count = 3) + clearApiPipeline() + } + } + + @Test + fun `retryTask is not active and doesn't run after init if network not reachable`() { + initDeliveryRetryManager( + status = NetworkStatus.NOT_REACHABLE, runRetryJobAfterScheduling = true + ) + blockingScheduledExecutorService.runCurrentlyBlocked() + retryTaskNotActive(NetworkStatus.NOT_REACHABLE) + checkNoApiRequestSent() + } + + @Test + fun `retryTask isn't active and won't run if there are no failed requests after getting a connection before retry job is scheduled`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager( + status = NetworkStatus.NOT_REACHABLE, + loadFailedRequest = false, + runRetryJobAfterScheduling = true + ) + blockingScheduledExecutorService.runCurrentlyBlocked() + deliveryRetryManager.onNetworkConnectivityStatusChanged(status) + retryTaskNotActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkNoApiRequestSent() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask is active and runs after connection changes from not reachable to connected after retry job runs`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager( + status = NetworkStatus.NOT_REACHABLE, runRetryJobAfterScheduling = true + ) + blockingScheduledExecutorService.runCurrentlyBlocked() + deliveryRetryManager.onNetworkConnectivityStatusChanged(status) + retryTaskActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkRequestSendAttempt() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask isn't active and doesn't run if there are no failed request after getting a connection before retry job is scheduled`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager( + status = NetworkStatus.NOT_REACHABLE, loadFailedRequest = false + ) + deliveryRetryManager.onNetworkConnectivityStatusChanged(status) + retryTaskNotActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkNoApiRequestSent() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask is active and runs after connection changes from not reachable to connected before retry job is scheduled`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager(status = NetworkStatus.NOT_REACHABLE) + deliveryRetryManager.onNetworkConnectivityStatusChanged(status) + retryTaskActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkRequestSendAttempt() + retryTaskNotActive(status) + clearApiPipeline() + } + } + + @Test + fun `retryTask is not active and doesn't run after connection changes from connected to not reachable before retry job is scheduled`() { + connectedNetworkStatuses.forEach { status -> + initDeliveryRetryManager(status = status) + deliveryRetryManager.onNetworkConnectivityStatusChanged(NetworkStatus.NOT_REACHABLE) + retryTaskNotActive(status) + blockingScheduledExecutorService.runCurrentlyBlocked() + checkNoApiRequestSent() + } + } + + @Test + fun `queue size should be bounded`() { + initDeliveryRetryManager(status = NetworkStatus.WIFI, loadFailedRequest = false) + every { mockPostExecutor(any(), any()) } throws Exception() + + assertEquals(0, deliveryRetryManager.pendingRetriesCount()) + + val mockApiRequest = mockk() + repeat(201) { + deliveryRetryManager.scheduleForRetry(mockApiRequest, "{ dummy_payload }".toByteArray()) + blockingScheduledExecutorService.runCurrentlyBlocked() + } + assertEquals(200, deliveryRetryManager.pendingRetriesCount()) + } + + private fun clearApiPipeline() { + clearMocks(mockPostExecutor, answers = false) + failedApiCalls.clear() + blockingScheduledExecutorService = BlockingScheduledExecutorService() + testScheduledExecutor = blockingScheduledExecutorService + } + + private fun initDeliveryRetryManager( + status: NetworkStatus, + loadFailedRequest: Boolean = true, + runRetryJobAfterScheduling: Boolean = false, + ) { + every { networkConnectivityService.getCurrentNetworkStatus() } returns status + + deliveryRetryManager = EmbraceDeliveryRetryManager( + scheduledExecutorService = testScheduledExecutor, + networkConnectivityService = networkConnectivityService, + cacheManager = mockCacheManager + ) + + deliveryRetryManager.setPostExecutor(mockPostExecutor) + + failedApiCalls.clear() + + if (loadFailedRequest) { + failedApiCalls.add(DeliveryFailedApiCall(mockk(), "cached_payload_1")) + } + + if (runRetryJobAfterScheduling) { + blockingScheduledExecutorService.runCurrentlyBlocked() + } + } + + private fun retryTaskActive(status: NetworkStatus) { + assertTrue("Failed for network status = $status", deliveryRetryManager.isRetryTaskActive()) + } + + private fun retryTaskNotActive(status: NetworkStatus) { + assertFalse( + "Failed for network status = $status", deliveryRetryManager.isRetryTaskActive() + ) + } + + private fun checkRequestSendAttempt(count: Int = 1) { + verify(exactly = count) { mockPostExecutor(any(), "{payload 1}".toByteArray()) } + } + + private fun checkNoApiRequestSent() { + verify(exactly = 0) { mockPostExecutor(any(), any()) } + } +} diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeDeliveryRetryManager.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeDeliveryRetryManager.kt new file mode 100644 index 0000000000..5360112806 --- /dev/null +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/FakeDeliveryRetryManager.kt @@ -0,0 +1,14 @@ +package io.embrace.android.embracesdk.fakes + +import io.embrace.android.embracesdk.comms.api.ApiRequest +import io.embrace.android.embracesdk.comms.delivery.DeliveryRetryManager + +internal class FakeDeliveryRetryManager : DeliveryRetryManager { + override fun scheduleForRetry(request: ApiRequest, payload: ByteArray) { + TODO("Not yet implemented") + } + + override fun setPostExecutor(postExecutor: (request: ApiRequest, payload: ByteArray) -> Unit) { + TODO("Not yet implemented") + } +} diff --git a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/injection/FakeEssentialServiceModule.kt b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/injection/FakeEssentialServiceModule.kt index da918b0b98..b1f3ab0436 100644 --- a/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/injection/FakeEssentialServiceModule.kt +++ b/embrace-android-sdk/src/test/java/io/embrace/android/embracesdk/fakes/injection/FakeEssentialServiceModule.kt @@ -13,6 +13,7 @@ import io.embrace.android.embracesdk.comms.api.ApiService import io.embrace.android.embracesdk.comms.api.ApiUrlBuilder import io.embrace.android.embracesdk.comms.delivery.CacheService import io.embrace.android.embracesdk.comms.delivery.DeliveryCacheManager +import io.embrace.android.embracesdk.comms.delivery.DeliveryRetryManager import io.embrace.android.embracesdk.config.ConfigService import io.embrace.android.embracesdk.fakes.FakeActivityService import io.embrace.android.embracesdk.fakes.FakeAndroidMetadataService @@ -22,6 +23,7 @@ import io.embrace.android.embracesdk.fakes.FakeCacheService import io.embrace.android.embracesdk.fakes.FakeConfigService import io.embrace.android.embracesdk.fakes.FakeCpuInfoDelegate import io.embrace.android.embracesdk.fakes.FakeDeliveryCacheManager +import io.embrace.android.embracesdk.fakes.FakeDeliveryRetryManager import io.embrace.android.embracesdk.fakes.FakeDeviceArchitecture import io.embrace.android.embracesdk.fakes.FakeGatingService import io.embrace.android.embracesdk.fakes.FakeMemoryCleanerService @@ -51,6 +53,7 @@ internal class FakeEssentialServiceModule( override val networkConnectivityService: NetworkConnectivityService = NoOpNetworkConnectivityService(), override val cacheService: CacheService = FakeCacheService(), override val deliveryCacheManager: DeliveryCacheManager = FakeDeliveryCacheManager(), + override val deliveryRetryManager: DeliveryRetryManager = FakeDeliveryRetryManager(), override val urlBuilder: ApiUrlBuilder = FakeApiUrlBuilder() ) : EssentialServiceModule {