diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt index 61a3c11368011a9e30387a568b32cbe9e4661fb5..b0c270e62abf366fd24bf2dcf23f00e0d5fe95c9 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/CoronaWarnApplication.kt @@ -40,10 +40,8 @@ import de.rki.coronawarnapp.util.device.ForegroundState import de.rki.coronawarnapp.util.di.AppInjector import de.rki.coronawarnapp.util.di.ApplicationComponent import kotlinx.coroutines.GlobalScope -import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.runBlocking import org.conscrypt.Conscrypt import timber.log.Timber import java.security.Security @@ -114,17 +112,12 @@ class CoronaWarnApplication : Application(), HasAndroidInjector { .launchIn(GlobalScope) if (onboardingSettings.isOnboarded) { - // TODO this is on the main thread, not very nice... - runBlocking { - val isAllowedToSubmitKeys = coronaTestRepository.coronaTests.first().any { it.isSubmissionAllowed } - if (!isAllowedToSubmitKeys) { - deadmanNotificationScheduler.schedulePeriodic() - } - } - contactDiaryWorkScheduler.schedulePeriodic() } + Timber.v("Setting up deadman notification scheduler") + deadmanNotificationScheduler.setup() + Timber.v("Setting up risk work schedulers.") exposureWindowRiskWorkScheduler.setup() presenceTracingRiskWorkScheduler.setup() diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt index 03dc9df0aa1f22fbc3a0feaec0263e6465d3b636..9c01a64e92c3555b3bb0e79c6bae69fe73345203 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessor.kt @@ -20,7 +20,6 @@ import de.rki.coronawarnapp.coronatest.type.CoronaTestProcessor import de.rki.coronawarnapp.coronatest.type.CoronaTestService import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector import de.rki.coronawarnapp.datadonation.analytics.modules.registeredtest.TestResultDataCollector -import de.rki.coronawarnapp.deadman.DeadmanNotificationScheduler import de.rki.coronawarnapp.exception.ExceptionCategory import de.rki.coronawarnapp.exception.http.CwaWebException import de.rki.coronawarnapp.exception.reporting.report @@ -36,8 +35,7 @@ class PCRProcessor @Inject constructor( private val timeStamper: TimeStamper, private val submissionService: CoronaTestService, private val analyticsKeySubmissionCollector: AnalyticsKeySubmissionCollector, - private val testResultDataCollector: TestResultDataCollector, - private val deadmanNotificationScheduler: DeadmanNotificationScheduler, + private val testResultDataCollector: TestResultDataCollector ) : CoronaTestProcessor { override val type: CoronaTest.Type = CoronaTest.Type.PCR @@ -83,7 +81,6 @@ class PCRProcessor @Inject constructor( if (testResult == PCR_POSITIVE) { analyticsKeySubmissionCollector.reportPositiveTestResultReceived() - deadmanNotificationScheduler.cancelScheduledWork() } analyticsKeySubmissionCollector.reportTestRegistered() @@ -119,7 +116,6 @@ class PCRProcessor @Inject constructor( if (newTestResult == PCR_POSITIVE) { analyticsKeySubmissionCollector.reportPositiveTestResultReceived() - deadmanNotificationScheduler.cancelScheduledWork() } test.copy( diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationScheduler.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationScheduler.kt index cdde640fa1b309a7905d53da77aca3bbfc415cae..1936efa408c3049819bf315f031f49611a249994 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationScheduler.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationScheduler.kt @@ -4,16 +4,55 @@ import androidx.work.ExistingPeriodicWorkPolicy import androidx.work.ExistingWorkPolicy import androidx.work.WorkManager import dagger.Reusable +import de.rki.coronawarnapp.coronatest.CoronaTestRepository +import de.rki.coronawarnapp.nearby.ENFClient +import de.rki.coronawarnapp.storage.OnboardingSettings +import de.rki.coronawarnapp.util.coroutine.AppScope +import de.rki.coronawarnapp.util.flow.combine +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import timber.log.Timber import javax.inject.Inject @Reusable class DeadmanNotificationScheduler @Inject constructor( + @AppScope val appScope: CoroutineScope, val timeCalculation: DeadmanNotificationTimeCalculation, val workManager: WorkManager, - val workBuilder: DeadmanNotificationWorkBuilder + val workBuilder: DeadmanNotificationWorkBuilder, + val onboardingSettings: OnboardingSettings, + val enfClient: ENFClient, + val coronaTestRepository: CoronaTestRepository ) { + fun setup() { + Timber.i("setup() DeadmanNotificationScheduler") + + combine( + onboardingSettings.isOnboardedFlow, + coronaTestRepository.coronaTests, + enfClient.isTracingEnabled + ) { isOnboarded, coronaTests, isTracingEnabled -> + val noPositiveTestRegistered = coronaTests.none { it.isPositive } + Timber.d( + "isOnboarded = $isOnboarded, " + + "noPositiveTestRegistered = $noPositiveTestRegistered, " + + "isTracingEnabled = $isTracingEnabled" + ) + isOnboarded && noPositiveTestRegistered && isTracingEnabled + } + .onEach { shouldSchedulePeriodic -> + Timber.d("shouldSchedulePeriodic: $shouldSchedulePeriodic") + if (shouldSchedulePeriodic) { + schedulePeriodic() + } else { + cancelScheduledWork() + } + } + .launchIn(appScope) + } + /** * Enqueue background deadman notification onetime work * Replace with new if older work exists. @@ -25,6 +64,7 @@ class DeadmanNotificationScheduler @Inject constructor( if (delay < 0) { return } else { + Timber.d("DeadmanNotification will be scheduled for $delay minutes in the future") // Create unique work and enqueue workManager.enqueueUniqueWork( ONE_TIME_WORK_NAME, diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationTimeCalculation.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationTimeCalculation.kt index 48e6d3a8eba598fb4121e761684c7595230f544b..a7820554419bb452b385378f8df864c85bdd6c74 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationTimeCalculation.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/deadman/DeadmanNotificationTimeCalculation.kt @@ -7,6 +7,7 @@ import kotlinx.coroutines.flow.first import org.joda.time.DateTimeConstants import org.joda.time.Hours import org.joda.time.Instant +import timber.log.Timber import javax.inject.Inject @Reusable @@ -29,6 +30,7 @@ class DeadmanNotificationTimeCalculation @Inject constructor( */ suspend fun getDelay(): Long { val lastSuccess = enfClient.lastSuccessfulTrackedExposureDetection().first()?.finishedAt + Timber.d("enfClient.lastSuccessfulTrackedExposureDetection: $lastSuccess") return if (lastSuccess != null) { getHoursDiff(lastSuccess).toLong() } else { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt index 1b669bb181d70987acf682fa200fcc0a62a2d311..09f6ce0b1374dcdcbbfcbc16691ef79d723ee091 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivity.kt @@ -195,7 +195,6 @@ class MainActivity : AppCompatActivity(), HasAndroidInjector { vm.doBackgroundNoiseCheck() contactDiaryWorkScheduler.schedulePeriodic() dataDonationAnalyticsScheduler.schedulePeriodic() - vm.checkDeadMan() } private fun showEnergyOptimizedEnabledForBackground() { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivityViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivityViewModel.kt index 1949db08144a868cd025f9a46b64e90fbab4a9d4..f8fcb04d1a3005d290195b5ccd8db41580c0ef30 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivityViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/MainActivityViewModel.kt @@ -21,7 +21,6 @@ import de.rki.coronawarnapp.util.viewmodel.CWAViewModel import de.rki.coronawarnapp.util.viewmodel.SimpleCWAViewModelFactory import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.map -import timber.log.Timber @Suppress("LongParameterList") class MainActivityViewModel @AssistedInject constructor( @@ -97,16 +96,6 @@ class MainActivityViewModel @AssistedInject constructor( } } - fun checkDeadMan() { - launch { - val isAllowedToSubmitKeys = coronaTestRepository.coronaTests.first().any { it.isSubmissionAllowed } - if (!isAllowedToSubmitKeys) { - Timber.v("We are not allowed to submit keys, scheduling deadman.") - deadmanScheduler.schedulePeriodic() - } - } - } - @AssistedFactory interface Factory : SimpleCWAViewModelFactory<MainActivityViewModel> } diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt index 9f07dc1959de84bd42eb0760ac77689f9abf301d..7a2c51d7dc38bb10a0b02aa99f2ee7b29e9a0060 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/coronatest/type/pcr/PCRProcessorTest.kt @@ -17,7 +17,6 @@ import de.rki.coronawarnapp.coronatest.tan.CoronaTestTAN import de.rki.coronawarnapp.coronatest.type.CoronaTestService import de.rki.coronawarnapp.datadonation.analytics.modules.keysubmission.AnalyticsKeySubmissionCollector import de.rki.coronawarnapp.datadonation.analytics.modules.registeredtest.TestResultDataCollector -import de.rki.coronawarnapp.deadman.DeadmanNotificationScheduler import de.rki.coronawarnapp.util.TimeStamper import io.kotest.matchers.shouldBe import io.mockk.MockKAnnotations @@ -39,7 +38,6 @@ class PCRProcessorTest : BaseTest() { @MockK lateinit var submissionService: CoronaTestService @MockK lateinit var analyticsKeySubmissionCollector: AnalyticsKeySubmissionCollector @MockK lateinit var testResultDataCollector: TestResultDataCollector - @MockK lateinit var deadmanNotificationScheduler: DeadmanNotificationScheduler private val nowUTC = Instant.parse("2021-03-15T05:45:00.000Z") @@ -71,17 +69,13 @@ class PCRProcessorTest : BaseTest() { coEvery { updatePendingTestResultReceivedTime(any()) } just Runs coEvery { saveTestResultAnalyticsSettings(any()) } just Runs } - deadmanNotificationScheduler.apply { - every { cancelScheduledWork() } just Runs - } } fun createInstance() = PCRProcessor( timeStamper = timeStamper, submissionService = submissionService, analyticsKeySubmissionCollector = analyticsKeySubmissionCollector, - testResultDataCollector = testResultDataCollector, - deadmanNotificationScheduler = deadmanNotificationScheduler, + testResultDataCollector = testResultDataCollector ) @Test diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/deadman/DeadmanNotificationSchedulerTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/deadman/DeadmanNotificationSchedulerTest.kt index 27cb96a8fa562b02fb40c6561e53bdfe3397c7bf..e207eee48763c3edfaf20a85d1db815b9be04914 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/deadman/DeadmanNotificationSchedulerTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/deadman/DeadmanNotificationSchedulerTest.kt @@ -6,12 +6,19 @@ import androidx.work.OneTimeWorkRequest import androidx.work.Operation import androidx.work.PeriodicWorkRequest import androidx.work.WorkManager +import de.rki.coronawarnapp.coronatest.CoronaTestRepository +import de.rki.coronawarnapp.coronatest.type.CoronaTest +import de.rki.coronawarnapp.nearby.ENFClient +import de.rki.coronawarnapp.storage.OnboardingSettings import io.mockk.MockKAnnotations import io.mockk.coEvery import io.mockk.every import io.mockk.impl.annotations.MockK +import io.mockk.mockk import io.mockk.verify import io.mockk.verifySequence +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.test.runBlockingTest import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test @@ -25,12 +32,17 @@ class DeadmanNotificationSchedulerTest : BaseTest() { @MockK lateinit var workBuilder: DeadmanNotificationWorkBuilder @MockK lateinit var periodicWorkRequest: PeriodicWorkRequest @MockK lateinit var oneTimeWorkRequest: OneTimeWorkRequest + @MockK lateinit var onboardingSettings: OnboardingSettings + @MockK lateinit var enfClient: ENFClient + @MockK lateinit var coronaTestRepository: CoronaTestRepository @BeforeEach fun setup() { MockKAnnotations.init(this) every { workBuilder.buildPeriodicWork() } returns periodicWorkRequest every { workBuilder.buildOneTimeWork(any()) } returns oneTimeWorkRequest + every { workManager.cancelUniqueWork(DeadmanNotificationScheduler.PERIODIC_WORK_NAME) } returns operation + every { workManager.cancelUniqueWork(DeadmanNotificationScheduler.ONE_TIME_WORK_NAME) } returns operation every { workManager.enqueueUniquePeriodicWork( DeadmanNotificationScheduler.PERIODIC_WORK_NAME, @@ -46,19 +58,27 @@ class DeadmanNotificationSchedulerTest : BaseTest() { oneTimeWorkRequest ) } returns operation + + every { onboardingSettings.isOnboardedFlow } returns flowOf(true) + every { coronaTestRepository.coronaTests } returns flowOf(emptySet()) + every { enfClient.isTracingEnabled } returns flowOf(true) } - private fun createScheduler() = DeadmanNotificationScheduler( + private fun createScheduler(scope: CoroutineScope) = DeadmanNotificationScheduler( + appScope = scope, timeCalculation = timeCalculation, workManager = workManager, - workBuilder = workBuilder + workBuilder = workBuilder, + onboardingSettings = onboardingSettings, + enfClient = enfClient, + coronaTestRepository = coronaTestRepository ) @Test fun `one time work was scheduled`() = runBlockingTest { coEvery { timeCalculation.getDelay() } returns 10L - createScheduler().scheduleOneTime() + createScheduler(this).scheduleOneTime() verifySequence { workManager.enqueueUniqueWork( @@ -73,7 +93,7 @@ class DeadmanNotificationSchedulerTest : BaseTest() { fun `one time work was not scheduled`() = runBlockingTest { coEvery { timeCalculation.getDelay() } returns -10L - createScheduler().scheduleOneTime() + createScheduler(this).scheduleOneTime() verify(exactly = 0) { workManager.enqueueUniqueWork( @@ -93,10 +113,76 @@ class DeadmanNotificationSchedulerTest : BaseTest() { } @Test - fun `test periodic work was scheduled`() { - createScheduler().schedulePeriodic() + fun `test periodic work was scheduled`() = runBlockingTest { + createScheduler(this).schedulePeriodic() - verifySequence { + verifyPeriodicWorkScheduled() + } + + @Test + fun `scheduled work should be cancelled if onboarding wasn't yet done `() = runBlockingTest { + every { onboardingSettings.isOnboardedFlow } returns flowOf(false) + + createScheduler(this).apply { setup() } + + verifyCancelScheduledWork(exactly = 1) + verifyPeriodicWorkScheduled(exactly = 0) + } + + @Test + fun `work should be scheduled if no test is registered`() = runBlockingTest { + every { coronaTestRepository.coronaTests } returns flowOf(emptySet()) + + createScheduler(this).apply { setup() } + + verifyCancelScheduledWork(exactly = 0) + verifyPeriodicWorkScheduled(exactly = 1) + } + + @Test + fun `work should be scheduled if only negative test is registered`() = runBlockingTest { + every { coronaTestRepository.coronaTests } returns flowOf( + setOf( + mockk<CoronaTest>().apply { + every { isPositive } returns false + } + ) + ) + + createScheduler(this).apply { setup() } + + verifyCancelScheduledWork(exactly = 0) + verifyPeriodicWorkScheduled(exactly = 1) + } + + @Test + fun `scheduled work should be cancelled if positive test is registered`() = runBlockingTest { + every { coronaTestRepository.coronaTests } returns flowOf( + setOf( + mockk<CoronaTest>().apply { + every { isPositive } returns true + } + ) + ) + + createScheduler(this).apply { setup() } + + verifyCancelScheduledWork(exactly = 1) + verifyPeriodicWorkScheduled(exactly = 0) + } + + @Test + fun `scheduled work should be cancelled if tracing is disabled`() = runBlockingTest { + every { enfClient.isTracingEnabled } returns flowOf(false) + + createScheduler(this).apply { setup() } + + verifyCancelScheduledWork(exactly = 1) + verifyPeriodicWorkScheduled(exactly = 0) + } + + private fun verifyPeriodicWorkScheduled(exactly: Int = 1) { + verify(exactly = exactly) { workManager.enqueueUniquePeriodicWork( DeadmanNotificationScheduler.PERIODIC_WORK_NAME, ExistingPeriodicWorkPolicy.KEEP, @@ -104,4 +190,13 @@ class DeadmanNotificationSchedulerTest : BaseTest() { ) } } + + private fun verifyCancelScheduledWork(exactly: Int = 1) { + verify(exactly = exactly) { + workManager.cancelUniqueWork(DeadmanNotificationScheduler.PERIODIC_WORK_NAME) + } + verify(exactly = exactly) { + workManager.cancelUniqueWork(DeadmanNotificationScheduler.ONE_TIME_WORK_NAME) + } + } }