From 3db92539af8bfd83f0cbf9794a369fbcf5bf1ef5 Mon Sep 17 00:00:00 2001 From: Fabian-K <fabian.kajzar@sap.com> Date: Thu, 27 Aug 2020 12:21:29 +0200 Subject: [PATCH] [EXPOSUREAPP-2314] Include test result retrieval in playbook for initial registration (#1073) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit For plausible deniability, the test result retrieval after the initial registration (via QR or TeleTAN) needs to be part of the same playbook and not two playbooks executed sequentially. For this, this commit: - updates the initial registration playbook to fetch the test result instead of a dummy request and returns the registration token as well as the test result - adds an argument "skipInitialTestResultRefresh" to the SubmissionTestResultFragment to optionally skip the initial test result refresh - includes the argument "skipInitialTestResultRefresh" when navigating from the QR Code scan or the TeleTAN input screen to the SubmissionTestResultFragment - adds "updateTestResult" to the SubmissionRepository to be able to set the test result without having to refresh it every time from the server - updates the SubmissionService to set the test result coming from the initial registration playbook Co-authored-by: Jakob Möller <jakob.moeller@sap.com> --- .../res/navigation/nav_graph.xml | 23 ++++--- .../coronawarnapp/http/playbook/Playbook.kt | 2 +- .../http/playbook/PlaybookImpl.kt | 28 ++++++--- .../service/submission/SubmissionService.kt | 7 ++- .../storage/SubmissionRepository.kt | 62 ++++++++++++------- .../SubmissionTestResultFragment.kt | 10 ++- .../ui/viewmodel/SubmissionViewModel.kt | 4 +- .../src/main/res/navigation/nav_graph.xml | 18 +++++- .../http/playbook/PlaybookImplTest.kt | 29 ++++++++- .../submission/SubmissionServiceTest.kt | 11 ++++ 10 files changed, 145 insertions(+), 49 deletions(-) diff --git a/Corona-Warn-App/src/deviceForTesters/res/navigation/nav_graph.xml b/Corona-Warn-App/src/deviceForTesters/res/navigation/nav_graph.xml index 58d6e0bd7..2612e3950 100644 --- a/Corona-Warn-App/src/deviceForTesters/res/navigation/nav_graph.xml +++ b/Corona-Warn-App/src/deviceForTesters/res/navigation/nav_graph.xml @@ -224,6 +224,10 @@ android:name="de.rki.coronawarnapp.ui.submission.SubmissionTestResultFragment" android:label="fragment_submission_result" tools:layout="@layout/fragment_submission_test_result"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="false" + app:argType="boolean" /> <action android:id="@+id/action_submissionResultFragment_to_mainFragment" app:destination="@id/mainFragment" @@ -239,16 +243,16 @@ android:name="de.rki.coronawarnapp.ui.submission.SubmissionTanFragment" android:label="fragment_submission_tan" tools:layout="@layout/fragment_submission_tan"> - <action - android:id="@+id/action_submissionTanFragment_to_submissionDispatcherFragment" - app:destination="@id/submissionDispatcherFragment" - app:popUpTo="@id/submissionDispatcherFragment" - app:popUpToInclusive="true" /> <action android:id="@+id/action_submissionTanFragment_to_submissionResultFragment" app:destination="@id/submissionResultFragment" app:popUpTo="@id/submissionResultFragment" - app:popUpToInclusive="true" /> + app:popUpToInclusive="true"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="true" + app:argType="boolean" /> + </action> </fragment> <fragment @@ -284,7 +288,12 @@ <action android:id="@+id/action_submissionQRCodeScanFragment_to_submissionResultFragment" app:destination="@id/submissionResultFragment" - app:popUpTo="@id/submissionResultFragment" /> + app:popUpTo="@id/submissionResultFragment"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="true" + app:argType="boolean" /> + </action> </fragment> <fragment android:id="@+id/submissionDoneFragment" diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/Playbook.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/Playbook.kt index 66ab1f957..5d37af5df 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/Playbook.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/Playbook.kt @@ -14,7 +14,7 @@ interface Playbook { suspend fun initialRegistration( key: String, keyType: KeyType - ): String /* registration token */ + ): Pair<String, TestResult> /* registration token & test result*/ suspend fun testResult( registrationToken: String diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/PlaybookImpl.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/PlaybookImpl.kt index 3012a1139..e5b3d78ca 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/PlaybookImpl.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/http/playbook/PlaybookImpl.kt @@ -20,22 +20,36 @@ class PlaybookImpl( private val uid = UUID.randomUUID().toString() private val coroutineScope: CoroutineScope = CoroutineScope(Dispatchers.IO) - override suspend fun initialRegistration(key: String, keyType: KeyType): String { + override suspend fun initialRegistration( + key: String, + keyType: KeyType + ): Pair<String, TestResult> { Timber.i("[$uid] New Initial Registration Playbook") // real registration - val (registrationToken, exception) = + val (registrationToken, registrationException) = executeCapturingExceptions { webRequestBuilder.asyncGetRegistrationToken(key, keyType) } - // fake verification - ignoreExceptions { webRequestBuilder.asyncFakeVerification() } + // if the registration succeeded continue with the real test result retrieval + // if it failed, execute a dummy request to satisfy the required playbook pattern + val (testResult, testResultException) = if (registrationToken != null) { + executeCapturingExceptions { webRequestBuilder.asyncGetTestResult(registrationToken) } + } else { + ignoreExceptions { webRequestBuilder.asyncFakeVerification() } + null to null + } // fake submission ignoreExceptions { webRequestBuilder.asyncFakeSubmission() } coroutineScope.launch { followUpPlaybooks() } - return registrationToken ?: propagateException(exception) + // if registration and test result retrieval succeeded, return the result + if (registrationToken != null && testResult != null) + return registrationToken to TestResult.fromInt(testResult) + + // else propagate the exception of either the first or the second step + propagateException(registrationException, testResultException) } override suspend fun testResult(registrationToken: String): TestResult { @@ -137,7 +151,7 @@ class PlaybookImpl( } } - private fun propagateException(exception: Exception?): Nothing { - throw exception ?: IllegalStateException() + private fun propagateException(vararg exceptions: Exception?): Nothing { + throw exceptions.filterNotNull().firstOrNull() ?: IllegalStateException() } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionService.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionService.kt index 10dc0ec34..defce20ed 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionService.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/service/submission/SubmissionService.kt @@ -7,6 +7,7 @@ import de.rki.coronawarnapp.http.WebRequestBuilder import de.rki.coronawarnapp.http.playbook.BackgroundNoise import de.rki.coronawarnapp.http.playbook.PlaybookImpl import de.rki.coronawarnapp.storage.LocalData +import de.rki.coronawarnapp.storage.SubmissionRepository import de.rki.coronawarnapp.transaction.SubmitDiagnosisKeysTransaction import de.rki.coronawarnapp.util.formatter.TestResult import de.rki.coronawarnapp.worker.BackgroundWorkScheduler @@ -27,7 +28,7 @@ object SubmissionService { } private suspend fun asyncRegisterDeviceViaGUID(guid: String) { - val registrationToken = + val (registrationToken, testResult) = PlaybookImpl(WebRequestBuilder.getInstance()).initialRegistration( guid, KeyType.GUID @@ -35,10 +36,11 @@ object SubmissionService { LocalData.registrationToken(registrationToken) deleteTestGUID() + SubmissionRepository.updateTestResult(testResult) } private suspend fun asyncRegisterDeviceViaTAN(tan: String) { - val registrationToken = + val (registrationToken, testResult) = PlaybookImpl(WebRequestBuilder.getInstance()).initialRegistration( tan, KeyType.TELETAN @@ -46,6 +48,7 @@ object SubmissionService { LocalData.registrationToken(registrationToken) deleteTeleTAN() + SubmissionRepository.updateTestResult(testResult) } suspend fun asyncSubmitExposureKeys(keys: List<TemporaryExposureKey>) { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/SubmissionRepository.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/SubmissionRepository.kt index eb9694415..e241304e6 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/SubmissionRepository.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/storage/SubmissionRepository.kt @@ -13,8 +13,9 @@ object SubmissionRepository { val testResultReceivedDate = MutableLiveData(Date()) val deviceUIState = MutableLiveData(DeviceUIState.UNPAIRED) + private val testResult = MutableLiveData<TestResult?>(null) - suspend fun refreshUIState() { + suspend fun refreshUIState(refreshTestResult: Boolean) { var uiState = DeviceUIState.UNPAIRED if (LocalData.numberOfSuccessfulSubmissions() == 1) { @@ -25,7 +26,10 @@ object SubmissionRepository { LocalData.isAllowedToSubmitDiagnosisKeys() == true -> { DeviceUIState.PAIRED_POSITIVE } - else -> fetchTestResult() + refreshTestResult -> fetchTestResult() + else -> { + deriveUiState(testResult.value) + } } } } @@ -35,32 +39,42 @@ object SubmissionRepository { private suspend fun fetchTestResult(): DeviceUIState { try { val testResult = SubmissionService.asyncRequestTestResult() - if (testResult == TestResult.POSITIVE) { - LocalData.isAllowedToSubmitDiagnosisKeys(true) - } + updateTestResult(testResult) + return deriveUiState(testResult) + } catch (err: NoRegistrationTokenSetException) { + return DeviceUIState.UNPAIRED + } + } - val initialTestResultReceivedTimestamp = LocalData.initialTestResultReceivedTimestamp() + fun updateTestResult(testResult: TestResult) { + this.testResult.value = testResult - if (initialTestResultReceivedTimestamp == null) { - val currentTime = System.currentTimeMillis() - LocalData.initialTestResultReceivedTimestamp(currentTime) - testResultReceivedDate.value = Date(currentTime) - if (testResult == TestResult.PENDING) { - BackgroundWorkScheduler.startWorkScheduler() - } - } else { - testResultReceivedDate.value = Date(initialTestResultReceivedTimestamp) - } + if (testResult == TestResult.POSITIVE) { + LocalData.isAllowedToSubmitDiagnosisKeys(true) + } + + val initialTestResultReceivedTimestamp = LocalData.initialTestResultReceivedTimestamp() - return when (testResult) { - TestResult.NEGATIVE -> DeviceUIState.PAIRED_NEGATIVE - TestResult.POSITIVE -> DeviceUIState.PAIRED_POSITIVE - TestResult.PENDING -> DeviceUIState.PAIRED_NO_RESULT - TestResult.INVALID -> DeviceUIState.PAIRED_ERROR - TestResult.REDEEMED -> DeviceUIState.PAIRED_REDEEMED + if (initialTestResultReceivedTimestamp == null) { + val currentTime = System.currentTimeMillis() + LocalData.initialTestResultReceivedTimestamp(currentTime) + testResultReceivedDate.value = Date(currentTime) + if (testResult == TestResult.PENDING) { + BackgroundWorkScheduler.startWorkScheduler() } - } catch (err: NoRegistrationTokenSetException) { - return DeviceUIState.UNPAIRED + } else { + testResultReceivedDate.value = Date(initialTestResultReceivedTimestamp) + } + } + + private fun deriveUiState(testResult: TestResult?): DeviceUIState { + return when (testResult) { + TestResult.NEGATIVE -> DeviceUIState.PAIRED_NEGATIVE + TestResult.POSITIVE -> DeviceUIState.PAIRED_POSITIVE + TestResult.PENDING -> DeviceUIState.PAIRED_NO_RESULT + TestResult.REDEEMED -> DeviceUIState.PAIRED_REDEEMED + TestResult.INVALID -> DeviceUIState.PAIRED_ERROR + null -> DeviceUIState.UNPAIRED } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionTestResultFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionTestResultFragment.kt index 03aecc8b2..34e9cbc98 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionTestResultFragment.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/SubmissionTestResultFragment.kt @@ -37,6 +37,8 @@ class SubmissionTestResultFragment : Fragment() { private var _binding: FragmentSubmissionTestResultBinding? = null private val binding: FragmentSubmissionTestResultBinding get() = _binding!! + private var skipInitialTestResultRefresh = false + // Overrides default back behaviour private val backCallback: OnBackPressedCallback = object : OnBackPressedCallback(true) { @@ -59,6 +61,10 @@ class SubmissionTestResultFragment : Fragment() { // registers callback when the os level back is pressed requireActivity().onBackPressedDispatcher.addCallback(viewLifecycleOwner, backCallback) // Inflate the layout for this fragment + + skipInitialTestResultRefresh = + arguments?.getBoolean("skipInitialTestResultRefresh") ?: false + return binding.root } @@ -139,8 +145,10 @@ class SubmissionTestResultFragment : Fragment() { override fun onResume() { super.onResume() binding.submissionTestResultContainer.sendAccessibilityEvent(AccessibilityEvent.TYPE_ANNOUNCEMENT) - submissionViewModel.refreshDeviceUIState() + submissionViewModel.refreshDeviceUIState(refreshTestResult = !skipInitialTestResultRefresh) tracingViewModel.refreshIsTracingEnabled() + + skipInitialTestResultRefresh = false } private fun setButtonOnClickListener() { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt index 1964d9169..2fced7642 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/viewmodel/SubmissionViewModel.kt @@ -91,9 +91,9 @@ class SubmissionViewModel : ViewModel() { } } - fun refreshDeviceUIState() = + fun refreshDeviceUIState(refreshTestResult: Boolean = true) = executeRequestWithState( - SubmissionRepository::refreshUIState, + { SubmissionRepository.refreshUIState(refreshTestResult) }, _uiStateState, _uiStateError ) diff --git a/Corona-Warn-App/src/main/res/navigation/nav_graph.xml b/Corona-Warn-App/src/main/res/navigation/nav_graph.xml index 13ffd4246..decb93bee 100644 --- a/Corona-Warn-App/src/main/res/navigation/nav_graph.xml +++ b/Corona-Warn-App/src/main/res/navigation/nav_graph.xml @@ -215,6 +215,10 @@ android:name="de.rki.coronawarnapp.ui.submission.SubmissionTestResultFragment" android:label="fragment_submission_result" tools:layout="@layout/fragment_submission_test_result"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="false" + app:argType="boolean" /> <action android:id="@+id/action_submissionResultFragment_to_mainFragment" app:destination="@id/mainFragment" @@ -234,7 +238,12 @@ android:id="@+id/action_submissionTanFragment_to_submissionResultFragment" app:destination="@id/submissionResultFragment" app:popUpTo="@id/submissionResultFragment" - app:popUpToInclusive="true" /> + app:popUpToInclusive="true"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="true" + app:argType="boolean" /> + </action> </fragment> <fragment @@ -270,7 +279,12 @@ <action android:id="@+id/action_submissionQRCodeScanFragment_to_submissionResultFragment" app:destination="@id/submissionResultFragment" - app:popUpTo="@id/submissionResultFragment" /> + app:popUpTo="@id/submissionResultFragment"> + <argument + android:name="skipInitialTestResultRefresh" + android:defaultValue="true" + app:argType="boolean" /> + </action> </fragment> <fragment android:id="@+id/submissionDoneFragment" diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/http/playbook/PlaybookImplTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/http/playbook/PlaybookImplTest.kt index 60cf0a738..508ed2d00 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/http/playbook/PlaybookImplTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/http/playbook/PlaybookImplTest.kt @@ -2,6 +2,7 @@ package de.rki.coronawarnapp.http.playbook import de.rki.coronawarnapp.exception.http.InternalServerErrorException import de.rki.coronawarnapp.service.submission.KeyType +import de.rki.coronawarnapp.util.formatter.TestResult import de.rki.coronawarnapp.util.newWebRequestBuilder import kotlinx.coroutines.runBlocking import okhttp3.mockwebserver.MockResponse @@ -84,18 +85,20 @@ class PlaybookImplTest { server.start() val expectedRegistrationToken = "token" + val expectedTestResult = TestResult.PENDING server.enqueue(MockResponse().setBody("""{"registrationToken":"$expectedRegistrationToken"}""")) - server.enqueue(MockResponse().setResponseCode(500)) + server.enqueue(MockResponse().setBody("""{"testResult":${expectedTestResult.value}}""")) server.enqueue(MockResponse().setResponseCode(500)) - val registrationToken = PlaybookImpl(server.newWebRequestBuilder()) + val (registrationToken, testResult) = PlaybookImpl(server.newWebRequestBuilder()) .initialRegistration("key", KeyType.GUID) assertThat(registrationToken, equalTo(expectedRegistrationToken)) + assertThat(testResult, equalTo(expectedTestResult)) } @Test - fun hasRequestPatternWhenRealRequestFails_initialRegistration(): Unit = runBlocking { + fun hasRequestPatternWhenRealRequestFails_initialRegistrationFirst(): Unit = runBlocking { val server = MockWebServer() server.start() @@ -115,6 +118,26 @@ class PlaybookImplTest { assertRequestPattern(server) } + @Test + fun hasRequestPatternWhenRealRequestFails_initialRegistrationSecond(): Unit = runBlocking { + val server = MockWebServer() + server.start() + + server.enqueue(MockResponse().setBody("""{"registrationToken":"response"}""")) + server.enqueue(MockResponse().setResponseCode(500)) + server.enqueue(MockResponse().setBody("{}")) + + try { + PlaybookImpl(server.newWebRequestBuilder()) + .initialRegistration("9A3B578UMG", KeyType.TELETAN) + fail("exception propagation expected") + } catch (e: InternalServerErrorException) { + } + + // ensure request order is 2x verification and 1x submission + assertRequestPattern(server) + } + @Test fun hasRequestPatternWhenRealRequestFails_testResult(): Unit = runBlocking { val server = MockWebServer() diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionServiceTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionServiceTest.kt index 9d1f958c6..fef637070 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionServiceTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/service/submission/SubmissionServiceTest.kt @@ -5,6 +5,7 @@ import de.rki.coronawarnapp.exception.NoRegistrationTokenSetException import de.rki.coronawarnapp.http.WebRequestBuilder import de.rki.coronawarnapp.http.playbook.BackgroundNoise import de.rki.coronawarnapp.storage.LocalData +import de.rki.coronawarnapp.storage.SubmissionRepository import de.rki.coronawarnapp.transaction.SubmitDiagnosisKeysTransaction import de.rki.coronawarnapp.util.formatter.TestResult import io.mockk.MockKAnnotations @@ -24,6 +25,7 @@ import org.junit.Test class SubmissionServiceTest { private val guid = "123456-12345678-1234-4DA7-B166-B86D85475064" private val registrationToken = "asdjnskjfdniuewbheboqudnsojdff" + private val testResult = TestResult.PENDING @MockK private lateinit var webRequestBuilder: WebRequestBuilder @@ -43,6 +45,9 @@ class SubmissionServiceTest { mockkObject(SubmitDiagnosisKeysTransaction) mockkObject(LocalData) + mockkObject(SubmissionRepository) + every { SubmissionRepository.updateTestResult(any()) } just Runs + every { LocalData.teletan() } returns null every { LocalData.testGUID() } returns null every { LocalData.registrationToken() } returns null @@ -66,6 +71,8 @@ class SubmissionServiceTest { coEvery { webRequestBuilder.asyncGetRegistrationToken(any(), KeyType.GUID) } returns registrationToken + coEvery { webRequestBuilder.asyncGetTestResult(registrationToken) } returns testResult.value + every { backgroundNoise.scheduleDummyPattern() } just Runs runBlocking { @@ -77,6 +84,7 @@ class SubmissionServiceTest { LocalData.devicePairingSuccessfulTimestamp(any()) LocalData.testGUID(null) backgroundNoise.scheduleDummyPattern() + SubmissionRepository.updateTestResult(testResult) } } @@ -91,6 +99,8 @@ class SubmissionServiceTest { coEvery { webRequestBuilder.asyncGetRegistrationToken(any(), KeyType.TELETAN) } returns registrationToken + coEvery { webRequestBuilder.asyncGetTestResult(registrationToken) } returns testResult.value + every { backgroundNoise.scheduleDummyPattern() } just Runs runBlocking { @@ -102,6 +112,7 @@ class SubmissionServiceTest { LocalData.devicePairingSuccessfulTimestamp(any()) LocalData.teletan(null) backgroundNoise.scheduleDummyPattern() + SubmissionRepository.updateTestResult(testResult) } } -- GitLab