From f091c81aafa208c4f4f9dc8fc808cabaa706314b Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <darken@darken.eu> Date: Tue, 1 Sep 2020 14:15:54 +0200 Subject: [PATCH] Prevent UI scopes from canceling transactions (EXPOSUREAPP-2312) (#1093) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Don't execute the transactions as part of any view scopes. We don't want to cancel them if the viewscope is cleared. This commit introduces distinct scopes for each transaction. * Mock injection helper behavior on existing tests. * Simplify to one CoroutineScope for all Transaction. Switch context when awaiting the transaction codeblock to end, otherwise a cancellation still affects us. * Add test for timeout * Remove unused code. * Expand comment on unexpected test behavior * Even without actual timeout, runBlockingTest causes flaky test behavior. Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com> Co-authored-by: Jakob Möller <jakob.moeller@sap.com> --- Corona-Warn-App/build.gradle | 1 + .../RetrieveDiagnosisInjectionHelper.kt | 10 ++ .../RetrieveDiagnosisKeysTransaction.kt | 9 +- .../transaction/RiskLevelInjectionHelper.kt | 10 ++ .../transaction/RiskLevelTransaction.kt | 7 +- .../SubmitDiagnosisInjectionHelper.kt | 10 ++ .../SubmitDiagnosisKeysTransaction.kt | 9 +- .../coronawarnapp/transaction/Transaction.kt | 88 +++++++------- .../transaction/TransactionCoroutineScope.kt | 13 ++ .../rki/coronawarnapp/util/di/AppInjector.kt | 7 +- .../util/di/ApplicationComponent.kt | 13 +- .../RetrieveDiagnosisKeysTransactionTest.kt | 10 ++ .../transaction/RiskLevelTransactionTest.kt | 11 ++ .../SubmitDiagnosisKeysTransactionTest.kt | 11 ++ .../transaction/TransactionTest.kt | 113 ++++++++++++++++++ 15 files changed, 270 insertions(+), 52 deletions(-) create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisInjectionHelper.kt create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelInjectionHelper.kt create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisInjectionHelper.kt create mode 100644 Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/TransactionCoroutineScope.kt create mode 100644 Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/TransactionTest.kt diff --git a/Corona-Warn-App/build.gradle b/Corona-Warn-App/build.gradle index 5f9be1008..c303fb301 100644 --- a/Corona-Warn-App/build.gradle +++ b/Corona-Warn-App/build.gradle @@ -263,6 +263,7 @@ dependencies { testImplementation "io.mockk:mockk:1.10.0" testImplementation "com.squareup.okhttp3:mockwebserver:4.8.0" testImplementation 'org.hamcrest:hamcrest-library:2.2' + testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.3.9' // Testing - jUnit4 testImplementation 'junit:junit:4.13' diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisInjectionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisInjectionHelper.kt new file mode 100644 index 000000000..ee6c36d9f --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisInjectionHelper.kt @@ -0,0 +1,10 @@ +package de.rki.coronawarnapp.transaction + +import javax.inject.Inject +import javax.inject.Singleton + +// TODO Remove once we have refactored the transaction and it's no longer a singleton +@Singleton +data class RetrieveDiagnosisInjectionHelper @Inject constructor( + val transactionScope: TransactionCoroutineScope +) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransaction.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransaction.kt index 10b990387..a1c454595 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransaction.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransaction.kt @@ -36,6 +36,7 @@ import de.rki.coronawarnapp.transaction.RetrieveDiagnosisKeysTransaction.Retriev import de.rki.coronawarnapp.transaction.RetrieveDiagnosisKeysTransaction.rollback import de.rki.coronawarnapp.transaction.RetrieveDiagnosisKeysTransaction.start import de.rki.coronawarnapp.util.CachedKeyFileHolder +import de.rki.coronawarnapp.util.di.AppInjector import de.rki.coronawarnapp.worker.BackgroundWorkHelper import org.joda.time.DateTime import org.joda.time.DateTimeZone @@ -118,6 +119,10 @@ object RetrieveDiagnosisKeysTransaction : Transaction() { /** atomic reference for the rollback value for created files during the transaction */ private val exportFilesForRollback = AtomicReference<List<File>>() + private val transactionScope: TransactionCoroutineScope by lazy { + AppInjector.component.transRetrieveKeysInjection.transactionScope + } + suspend fun startWithConstraints() { val currentDate = DateTime(Instant.now(), DateTimeZone.getDefault()) val lastFetch = DateTime( @@ -136,7 +141,7 @@ object RetrieveDiagnosisKeysTransaction : Transaction() { } /** initiates the transaction. This suspend function guarantees a successful transaction once completed. */ - suspend fun start() = lockAndExecuteUnique { + suspend fun start() = lockAndExecute(unique = true, scope = transactionScope) { /** * Handles the case when the ENClient got disabled but the Transaction is still scheduled * in a background job. Also it acts as a failure catch in case the orchestration code did @@ -145,7 +150,7 @@ object RetrieveDiagnosisKeysTransaction : Transaction() { if (!InternalExposureNotificationClient.asyncIsEnabled()) { Timber.tag(TAG).w("EN is not enabled, skipping RetrieveDiagnosisKeys") executeClose() - return@lockAndExecuteUnique + return@lockAndExecute } /**************************************************** * INIT TRANSACTION diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelInjectionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelInjectionHelper.kt new file mode 100644 index 000000000..f5e6689c6 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelInjectionHelper.kt @@ -0,0 +1,10 @@ +package de.rki.coronawarnapp.transaction + +import javax.inject.Inject +import javax.inject.Singleton + +// TODO Remove once we have refactored the transaction and it's no longer a singleton +@Singleton +data class RiskLevelInjectionHelper @Inject constructor( + val transactionScope: TransactionCoroutineScope +) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelTransaction.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelTransaction.kt index 8bd531250..4629e0eb7 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelTransaction.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/RiskLevelTransaction.kt @@ -37,6 +37,7 @@ import de.rki.coronawarnapp.transaction.RiskLevelTransaction.RiskLevelTransactio import de.rki.coronawarnapp.transaction.RiskLevelTransaction.RiskLevelTransactionState.UPDATE_RISK_LEVEL import de.rki.coronawarnapp.util.ConnectivityHelper import de.rki.coronawarnapp.util.TimeAndDateExtensions.millisecondsToHours +import de.rki.coronawarnapp.util.di.AppInjector import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import timber.log.Timber @@ -184,8 +185,12 @@ object RiskLevelTransaction : Transaction() { /** atomic reference for the rollback value for date of last risk level calculation */ private val lastCalculatedRiskLevelDate = AtomicReference<Long>() + private val transactionScope: TransactionCoroutineScope by lazy { + AppInjector.component.transRiskLevelInjection.transactionScope + } + /** initiates the transaction. This suspend function guarantees a successful transaction once completed. */ - suspend fun start() = lockAndExecute { + suspend fun start() = lockAndExecute(scope = transactionScope) { /**************************************************** * CHECK [NO_CALCULATION_POSSIBLE_TRACING_OFF] CONDITIONS ****************************************************/ diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisInjectionHelper.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisInjectionHelper.kt new file mode 100644 index 000000000..7ddfa7fd1 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisInjectionHelper.kt @@ -0,0 +1,10 @@ +package de.rki.coronawarnapp.transaction + +import javax.inject.Inject +import javax.inject.Singleton + +// TODO Remove once we have refactored the transaction and it's no longer a singleton +@Singleton +data class SubmitDiagnosisInjectionHelper @Inject constructor( + val transactionScope: TransactionCoroutineScope +) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransaction.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransaction.kt index 326cffe11..dddc37982 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransaction.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransaction.kt @@ -10,6 +10,7 @@ import de.rki.coronawarnapp.transaction.SubmitDiagnosisKeysTransaction.SubmitDia import de.rki.coronawarnapp.transaction.SubmitDiagnosisKeysTransaction.SubmitDiagnosisKeysTransactionState.STORE_SUCCESS import de.rki.coronawarnapp.util.ProtoFormatConverterExtensions.limitKeyCount import de.rki.coronawarnapp.util.ProtoFormatConverterExtensions.transformKeyHistoryToExternalFormat +import de.rki.coronawarnapp.util.di.AppInjector /** * The SubmitDiagnosisKeysTransaction is used to define an atomic Transaction for Key Reports. Its states allow an @@ -47,16 +48,20 @@ object SubmitDiagnosisKeysTransaction : Transaction() { CLOSE } + private val transactionScope: TransactionCoroutineScope by lazy { + AppInjector.component.transSubmitDiagnosisInjection.transactionScope + } + /** initiates the transaction. This suspend function guarantees a successful transaction once completed. */ suspend fun start( registrationToken: String, keys: List<TemporaryExposureKey> - ) = lockAndExecuteUnique { + ) = lockAndExecute(unique = true, scope = transactionScope) { /**************************************************** * RETRIEVE TEMPORARY EXPOSURE KEY HISTORY ****************************************************/ val temporaryExposureKeyList = executeState(RETRIEVE_TEMPORARY_EXPOSURE_KEY_HISTORY) { - keys.limitKeyCount() + keys.limitKeyCount() .transformKeyHistoryToExternalFormat() } /**************************************************** diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/Transaction.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/Transaction.kt index 54e5d219f..a732d93fb 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/Transaction.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/Transaction.kt @@ -26,6 +26,7 @@ import de.rki.coronawarnapp.risk.TimeVariables import de.rki.coronawarnapp.transaction.Transaction.InternalTransactionStates.INIT import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.async import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -53,11 +54,11 @@ abstract class Transaction { * Transaction Timeout in Milliseconds, used to cancel any Transactions that run into never ending execution * (e.g. due to a coroutine not being cancelled properly or an exception leading to unchecked behavior) */ - private val TRANSACTION_TIMEOUT_MS = - TimeVariables.getTransactionTimeout() + private val TRANSACTION_TIMEOUT_MS: Long + get() = TimeVariables.getTransactionTimeout() } - @Suppress("VariableNaming") // Done as the Convention is TAG for every class + @Suppress("VariableNaming", "PropertyName") // Done as the Convention is TAG for every class abstract val TAG: String? /** @@ -173,26 +174,6 @@ abstract class Transaction { ): T = executeState(Dispatchers.Default, state, block) - /** - * Executes the transaction as Unique. This results in the next execution being omitted in case of a race towards - * the lock. Please see [lockAndExecute] for more details - * - * @param T Optional Return Type in case the Transaction should return a value from the coroutine. - * @param block the suspending function that should be used to execute the transaction. - */ - protected suspend fun <T> lockAndExecuteUnique(block: suspend CoroutineScope.() -> T) = - lockAndExecute(true, block) - - /** - * Executes the transaction as non-unique. This results in the next execution being queued in case of a race towards - * the lock. Please see [lockAndExecute] for more details - * - * @param T Optional Return Type in case the Transaction should return a value from the coroutine. - * @param block the suspending function that should be used to execute the transaction. - */ - protected suspend fun <T> lockAndExecute(block: suspend CoroutineScope.() -> T) = - lockAndExecute(false, block) - /** * Attempts to go into the internal lock context (mutual exclusion coroutine) and executes the given suspending * function. Standard Logging is executed to inform about the transaction status. @@ -206,37 +187,52 @@ abstract class Transaction { * * In an error scenario, during the handling of the transaction error, a rollback will be executed on best-effort basis. * - * @param T Optional Return Type in case the Transaction should return a value from the coroutine. + * @param unique Executes the transaction as Unique. This results in the next execution being omitted in case of a race towards the lock. * @param block the suspending function that should be used to execute the transaction. * @throws TransactionException the exception that wraps around any error that occurs inside the lock. * * @see executeState * @see executedStatesStack */ - private suspend fun <T> lockAndExecute(unique: Boolean, block: suspend CoroutineScope.() -> T) { + suspend fun lockAndExecute( + unique: Boolean = false, + scope: CoroutineScope, + block: suspend CoroutineScope.() -> Unit + ) { + if (unique && internalMutualExclusionLock.isLocked) { - val runningString = "TRANSACTION WITH ID $transactionId ALREADY RUNNING " + - "($currentTransactionState) AS UNIQUE, SKIPPING EXECUTION." - Timber.tag(TAG).w(runningString) + Timber.tag(TAG).w( + "TRANSACTION WITH ID %s ALREADY RUNNING (%s) AS UNIQUE, SKIPPING EXECUTION.", + transactionId, currentTransactionState + ) return } - try { - return internalMutualExclusionLock.withLock { + + val deferred = scope.async { + internalMutualExclusionLock.withLock { executeState(INIT) { transactionId.set(UUID.randomUUID()) } - measureTimeMillis { + + val duration = measureTimeMillis { withTimeout(TRANSACTION_TIMEOUT_MS) { block.invoke(this) } - }.also { - val completedString = - "TRANSACTION $transactionId COMPLETED (${System.currentTimeMillis()}) " + - "in $it ms, STATES EXECUTED: ${getExecutedStates()}" - Timber.tag(TAG).i(completedString) } + + Timber.tag(TAG).i( + "TRANSACTION %s COMPLETED (%d) in %d ms, STATES EXECUTED: %s", + transactionId, System.currentTimeMillis(), duration, getExecutedStates() + ) + resetExecutedStateStack() } - } catch (e: Exception) { - handleTransactionError(e) + } + + withContext(scope.coroutineContext) { + try { + deferred.await() + } catch (e: Exception) { + handleTransactionError(e) + } } } @@ -253,14 +249,18 @@ abstract class Transaction { * @param error the error that lead to an error case in the transaction that cannot be handled inside the * transaction but has to be caught from the exception caller */ - protected open suspend fun handleTransactionError(error: Throwable?): Nothing { - rollback() - resetExecutedStateStack() - throw TransactionException( + protected open suspend fun handleTransactionError(error: Throwable): Nothing { + val wrap = TransactionException( transactionId.get(), currentTransactionState.toString(), error ) + Timber.tag(TAG).e(wrap) + + rollback() + resetExecutedStateStack() + + throw wrap } /** @@ -285,10 +285,12 @@ abstract class Transaction { * @param error the error that lead to an error case in the rollback */ protected open fun handleRollbackError(error: Throwable?): Nothing { - throw RollbackException( + val wrap = RollbackException( transactionId.get(), currentTransactionState.toString(), error ) + Timber.tag(TAG).e(wrap) + throw wrap } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/TransactionCoroutineScope.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/TransactionCoroutineScope.kt new file mode 100644 index 000000000..0e27712b8 --- /dev/null +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/transaction/TransactionCoroutineScope.kt @@ -0,0 +1,13 @@ +package de.rki.coronawarnapp.transaction + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import javax.inject.Inject +import javax.inject.Singleton +import kotlin.coroutines.CoroutineContext + +@Singleton +class TransactionCoroutineScope @Inject constructor() : CoroutineScope { + override val coroutineContext: CoroutineContext = SupervisorJob() + Dispatchers.Default +} diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/AppInjector.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/AppInjector.kt index dbebf2871..daa9dc01a 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/AppInjector.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/AppInjector.kt @@ -3,9 +3,10 @@ package de.rki.coronawarnapp.util.di import de.rki.coronawarnapp.CoronaWarnApplication object AppInjector { + lateinit var component: ApplicationComponent + fun init(app: CoronaWarnApplication) { - DaggerApplicationComponent.factory() - .create(app) - .inject(app) + component = DaggerApplicationComponent.factory().create(app) + component.inject(app) } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt index 914a6a9f5..691b28bd7 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/ApplicationComponent.kt @@ -1,5 +1,6 @@ package de.rki.coronawarnapp.util.di +import dagger.BindsInstance import dagger.Component import dagger.android.AndroidInjector import dagger.android.support.AndroidSupportInjectionModule @@ -7,6 +8,9 @@ import de.rki.coronawarnapp.CoronaWarnApplication import de.rki.coronawarnapp.receiver.ReceiverBinder import de.rki.coronawarnapp.risk.RiskModule import de.rki.coronawarnapp.service.ServiceBinder +import de.rki.coronawarnapp.transaction.RetrieveDiagnosisInjectionHelper +import de.rki.coronawarnapp.transaction.RiskLevelInjectionHelper +import de.rki.coronawarnapp.transaction.SubmitDiagnosisInjectionHelper import de.rki.coronawarnapp.ui.ActivityBinder import de.rki.coronawarnapp.util.device.DeviceModule import javax.inject.Singleton @@ -24,6 +28,13 @@ import javax.inject.Singleton ] ) interface ApplicationComponent : AndroidInjector<CoronaWarnApplication> { + + val transRetrieveKeysInjection: RetrieveDiagnosisInjectionHelper + val transRiskLevelInjection: RiskLevelInjectionHelper + val transSubmitDiagnosisInjection: SubmitDiagnosisInjectionHelper + @Component.Factory - interface Factory : AndroidInjector.Factory<CoronaWarnApplication> + interface Factory { + fun create(@BindsInstance app: CoronaWarnApplication): ApplicationComponent + } } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransactionTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransactionTest.kt index e8c59395e..893a93928 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransactionTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransactionTest.kt @@ -4,6 +4,8 @@ import com.google.android.gms.nearby.exposurenotification.ExposureConfiguration import de.rki.coronawarnapp.nearby.InternalExposureNotificationClient import de.rki.coronawarnapp.service.applicationconfiguration.ApplicationConfigurationService import de.rki.coronawarnapp.storage.LocalData +import de.rki.coronawarnapp.util.di.AppInjector +import de.rki.coronawarnapp.util.di.ApplicationComponent import io.mockk.Runs import io.mockk.coEvery import io.mockk.coVerifyOrder @@ -28,6 +30,14 @@ class RetrieveDiagnosisKeysTransactionTest { @Before fun setUp() { + mockkObject(AppInjector) + val appComponent = mockk<ApplicationComponent>().apply { + every { transRetrieveKeysInjection } returns RetrieveDiagnosisInjectionHelper( + TransactionCoroutineScope() + ) + } + every { AppInjector.component } returns appComponent + mockkObject(InternalExposureNotificationClient) mockkObject(ApplicationConfigurationService) mockkObject(RetrieveDiagnosisKeysTransaction) diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RiskLevelTransactionTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RiskLevelTransactionTest.kt index 212a34ee6..363492631 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RiskLevelTransactionTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RiskLevelTransactionTest.kt @@ -23,6 +23,8 @@ import de.rki.coronawarnapp.storage.ExposureSummaryRepository import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.storage.RiskLevelRepository import de.rki.coronawarnapp.util.ConnectivityHelper +import de.rki.coronawarnapp.util.di.AppInjector +import de.rki.coronawarnapp.util.di.ApplicationComponent import io.mockk.MockKAnnotations import io.mockk.Runs import io.mockk.coEvery @@ -30,6 +32,7 @@ import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.just +import io.mockk.mockk import io.mockk.mockkObject import io.mockk.unmockkAll import kotlinx.coroutines.runBlocking @@ -52,6 +55,14 @@ class RiskLevelTransactionTest { fun setUp() { MockKAnnotations.init(this) + mockkObject(AppInjector) + val appComponent = mockk<ApplicationComponent>().apply { + every { transRiskLevelInjection } returns RiskLevelInjectionHelper( + TransactionCoroutineScope() + ) + } + every { AppInjector.component } returns appComponent + mockkObject(InternalExposureNotificationClient) mockkObject(ApplicationConfigurationService) mockkObject(LocalData) diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransactionTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransactionTest.kt index c43d6b9eb..25d61722c 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransactionTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/SubmitDiagnosisKeysTransactionTest.kt @@ -7,6 +7,8 @@ import de.rki.coronawarnapp.http.playbook.BackgroundNoise import de.rki.coronawarnapp.nearby.InternalExposureNotificationClient import de.rki.coronawarnapp.service.submission.SubmissionService import de.rki.coronawarnapp.storage.LocalData +import de.rki.coronawarnapp.util.di.AppInjector +import de.rki.coronawarnapp.util.di.ApplicationComponent import de.rki.coronawarnapp.worker.BackgroundWorkScheduler import io.mockk.MockKAnnotations import io.mockk.Runs @@ -15,6 +17,7 @@ import io.mockk.coVerifyOrder import io.mockk.every import io.mockk.impl.annotations.MockK import io.mockk.just +import io.mockk.mockk import io.mockk.mockkObject import io.mockk.slot import io.mockk.unmockkAll @@ -40,6 +43,14 @@ class SubmitDiagnosisKeysTransactionTest { fun setUp() { MockKAnnotations.init(this) + mockkObject(AppInjector) + val appComponent = mockk<ApplicationComponent>().apply { + every { transSubmitDiagnosisInjection } returns SubmitDiagnosisInjectionHelper( + TransactionCoroutineScope() + ) + } + every { AppInjector.component } returns appComponent + mockkObject(WebRequestBuilder.Companion) every { WebRequestBuilder.getInstance() } returns webRequestBuilder diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/TransactionTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/TransactionTest.kt new file mode 100644 index 000000000..5ba62a995 --- /dev/null +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/TransactionTest.kt @@ -0,0 +1,113 @@ +package de.rki.coronawarnapp.transaction + +import de.rki.coronawarnapp.exception.RollbackException +import de.rki.coronawarnapp.exception.TransactionException +import de.rki.coronawarnapp.risk.TimeVariables +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.should +import io.kotest.matchers.types.beInstanceOf +import io.mockk.clearAllMocks +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockkObject +import io.mockk.spyk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.delay +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.TestCoroutineScope +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import testhelpers.BaseTest +import java.io.IOException + +@ExperimentalCoroutinesApi +class TransactionTest : BaseTest() { + + @BeforeEach + fun setup() { + mockkObject(TimeVariables) + } + + @AfterEach + fun tearDown() { + clearAllMocks() + } + + @Suppress("UNREACHABLE_CODE") + private class TestTransaction( + val errorOnRollBack: Exception? = null + ) : Transaction() { + override val TAG: String = "TestTag" + + public override suspend fun rollback() { + errorOnRollBack?.let { handleRollbackError(it) } + super.rollback() + } + + public override suspend fun handleTransactionError(error: Throwable): Nothing { + return super.handleTransactionError(error) + } + + public override fun handleRollbackError(error: Throwable?): Nothing { + return super.handleRollbackError(error) + } + } + + @Test + fun `transaction error handler is called`() { + val testScope = TestCoroutineScope() + val testTransaction = spyk(TestTransaction()) + shouldThrow<TransactionException> { + runBlocking { + testTransaction.lockAndExecute(scope = testScope) { + throw IOException() + } + } + } + + coVerify { testTransaction.handleTransactionError(any()) } + coVerify { testTransaction.rollback() } + } + + @Test + fun `rollback error handler is called`() { + val testScope = TestCoroutineScope() + val testTransaction = spyk( + TestTransaction( + errorOnRollBack = IllegalAccessException() + ) + ) + shouldThrow<RollbackException> { + runBlocking { + testTransaction.lockAndExecute(scope = testScope) { + throw IOException() + } + } + } + + coVerify { testTransaction.handleTransactionError(ofType<IOException>()) } + coVerify { testTransaction.rollback() } + coVerify { testTransaction.handleRollbackError(ofType<IllegalAccessException>()) } + } + + @Test + fun `transactions can timeout`() { + /** + * TODO use runBlockingTest & advanceTime, which currently does not work + * https://github.com/Kotlin/kotlinx.coroutines/issues/1204 + */ + every { TimeVariables.getTransactionTimeout() } returns 0L + + val testTransaction = TestTransaction() + val exception = shouldThrow<TransactionException> { + runBlocking { + testTransaction.lockAndExecute(scope = this) { + delay(TimeVariables.getTransactionTimeout()) + } + } + } + exception.cause should beInstanceOf<TimeoutCancellationException>() + } +} -- GitLab