From f8284112f3129409ca3425961c8a2b42817eac6c Mon Sep 17 00:00:00 2001 From: Rituraj Sambherao <54317407+ritsam@users.noreply.github.com> Date: Thu, 5 Nov 2020 15:53:40 +0000 Subject: [PATCH] Harmonize behavior scanning expired QR (EXPOUREAPP-2700) (#1493) * Harmonization of the expired QR-Code * implmentation and refactoring of the asyncRegisterDevice method * fix detekt pipeline issue * test failing pipeline fix * QRCode Exceotion, observevent and timestamp improvements as per code reviews * removed deregistration logic from Fragment to viewModel UI Thread protection as this is important logic and should run on ViewModel * Code conflict resolution * ktlint fixes * changin the method deregisterTestFromDevice() to private * scanStatus SingleLIveEvent and relevant test cases added Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../service/submission/SubmissionService.kt | 27 ++++------- .../scan/SubmissionQRCodeScanFragment.kt | 40 +++++++++------- .../scan/SubmissionQRCodeScanViewModel.kt | 48 ++++++++++++++----- .../submission/tan/SubmissionTanViewModel.kt | 2 +- .../src/main/res/values-de/strings.xml | 4 +- .../submission/SubmissionServiceTest.kt | 12 +---- .../scan/SubmissionQRCodeScanViewModelTest.kt | 12 +++-- 7 files changed, 82 insertions(+), 63 deletions(-) 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 cfd3f3f40..0961774da 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 @@ -1,11 +1,11 @@ package de.rki.coronawarnapp.service.submission -import de.rki.coronawarnapp.exception.NoGUIDOrTANSetException import de.rki.coronawarnapp.exception.NoRegistrationTokenSetException import de.rki.coronawarnapp.playbook.BackgroundNoise import de.rki.coronawarnapp.playbook.Playbook import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.storage.SubmissionRepository +import de.rki.coronawarnapp.util.TimeStamper import de.rki.coronawarnapp.util.di.AppInjector import de.rki.coronawarnapp.util.formatter.TestResult import de.rki.coronawarnapp.verification.server.VerificationKeyType @@ -16,41 +16,34 @@ object SubmissionService { private val playbook: Playbook get() = AppInjector.component.playbook - suspend fun asyncRegisterDevice() { - val testGUID = LocalData.testGUID() - val testTAN = LocalData.teletan() + private val timeStamper: TimeStamper + get() = TimeStamper() - when { - testGUID != null -> asyncRegisterDeviceViaGUID(testGUID) - testTAN != null -> asyncRegisterDeviceViaTAN(testTAN) - else -> throw NoGUIDOrTANSetException() - } - LocalData.devicePairingSuccessfulTimestamp(System.currentTimeMillis()) - BackgroundNoise.getInstance().scheduleDummyPattern() - } - - private suspend fun asyncRegisterDeviceViaGUID(guid: String) { + suspend fun asyncRegisterDeviceViaGUID(guid: String): TestResult { val (registrationToken, testResult) = playbook.initialRegistration( guid, VerificationKeyType.GUID ) - LocalData.registrationToken(registrationToken) deleteTestGUID() SubmissionRepository.updateTestResult(testResult) + LocalData.devicePairingSuccessfulTimestamp(timeStamper.nowUTC.millis) + BackgroundNoise.getInstance().scheduleDummyPattern() + return testResult } - private suspend fun asyncRegisterDeviceViaTAN(tan: String) { + suspend fun asyncRegisterDeviceViaTAN(tan: String) { val (registrationToken, testResult) = playbook.initialRegistration( tan, VerificationKeyType.TELETAN ) - LocalData.registrationToken(registrationToken) deleteTeleTAN() SubmissionRepository.updateTestResult(testResult) + LocalData.devicePairingSuccessfulTimestamp(timeStamper.nowUTC.millis) + BackgroundNoise.getInstance().scheduleDummyPattern() } suspend fun asyncRequestTestResult(): TestResult { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanFragment.kt index 1ecb64ba8..b9b618c30 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanFragment.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanFragment.kt @@ -21,7 +21,6 @@ import de.rki.coronawarnapp.ui.submission.viewmodel.SubmissionNavigationEvents import de.rki.coronawarnapp.util.CameraPermissionHelper import de.rki.coronawarnapp.util.DialogHelper import de.rki.coronawarnapp.util.di.AutoInject -import de.rki.coronawarnapp.util.observeEvent import de.rki.coronawarnapp.util.ui.doNavigate import de.rki.coronawarnapp.util.ui.observe2 import de.rki.coronawarnapp.util.ui.viewBindingLazy @@ -32,7 +31,8 @@ import javax.inject.Inject /** * A simple [Fragment] subclass. */ -class SubmissionQRCodeScanFragment : Fragment(R.layout.fragment_submission_qr_code_scan), AutoInject { +class SubmissionQRCodeScanFragment : Fragment(R.layout.fragment_submission_qr_code_scan), + AutoInject { @Inject lateinit var viewModelFactory: CWAViewModelFactoryProvider.Factory private val viewModel: SubmissionQRCodeScanViewModel by cwaViewModels { viewModelFactory } @@ -58,22 +58,29 @@ class SubmissionQRCodeScanFragment : Fragment(R.layout.fragment_submission_qr_co binding.submissionQrCodeScanViewfinderView.setCameraPreview(binding.submissionQrCodeScanPreview) - viewModel.scanStatus.observeEvent(viewLifecycleOwner) { - if (ScanStatus.SUCCESS == it) { - viewModel.doDeviceRegistration() - } - + viewModel.scanStatusValue.observe2(this) { if (ScanStatus.INVALID == it) { showInvalidScanDialog() } } + viewModel.showRedeemedTokenWarning.observe2(this) { + val dialog = DialogHelper.DialogInstance( + requireActivity(), + R.string.submission_error_dialog_web_tan_redeemed_title, + R.string.submission_error_dialog_web_tan_redeemed_body, + R.string.submission_error_dialog_web_tan_redeemed_button_positive + ) + + DialogHelper.showDialog(dialog) + goBack() + } + viewModel.registrationState.observe2(this) { binding.submissionQrCodeScanSpinner.visibility = when (it) { ApiRequestState.STARTED -> View.VISIBLE else -> View.GONE } - if (ApiRequestState.SUCCESS == it) { doNavigate( SubmissionQRCodeScanFragmentDirections @@ -98,7 +105,7 @@ class SubmissionQRCodeScanFragment : Fragment(R.layout.fragment_submission_qr_co private fun startDecode() { binding.submissionQrCodeScanPreview.decodeSingle { - viewModel.validateAndStoreTestGUID(it.text) + viewModel.validateTestGUID(it.text) } } @@ -165,13 +172,14 @@ class SubmissionQRCodeScanFragment : Fragment(R.layout.fragment_submission_qr_co ) { // if permission was denied if (requestCode == REQUEST_CAMERA_PERMISSION_CODE && - (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_DENIED)) { - if (shouldShowRequestPermissionRationale(Manifest.permission.CAMERA)) { - showCameraPermissionRationaleDialog() - } else { - // user permanently denied access to the camera - showCameraPermissionDeniedDialog() - } + (grantResults.isNotEmpty() && grantResults[0] == PackageManager.PERMISSION_DENIED) + ) { + if (shouldShowRequestPermissionRationale(Manifest.permission.CAMERA)) { + showCameraPermissionRationaleDialog() + } else { + // user permanently denied access to the camera + showCameraPermissionDeniedDialog() + } } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt index b4330c73b..ec4ade17f 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModel.kt @@ -1,6 +1,5 @@ package de.rki.coronawarnapp.ui.submission.qrcode.scan -import androidx.lifecycle.LiveData import androidx.lifecycle.MutableLiveData import com.squareup.inject.assisted.AssistedInject import de.rki.coronawarnapp.exception.ExceptionCategory @@ -9,38 +8,41 @@ import de.rki.coronawarnapp.exception.http.CwaWebException import de.rki.coronawarnapp.exception.reporting.report import de.rki.coronawarnapp.service.submission.QRScanResult import de.rki.coronawarnapp.service.submission.SubmissionService +import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.ui.submission.ApiRequestState import de.rki.coronawarnapp.ui.submission.ScanStatus import de.rki.coronawarnapp.ui.submission.viewmodel.SubmissionNavigationEvents -import de.rki.coronawarnapp.util.Event +import de.rki.coronawarnapp.util.formatter.TestResult import de.rki.coronawarnapp.util.ui.SingleLiveEvent import de.rki.coronawarnapp.util.viewmodel.CWAViewModel import de.rki.coronawarnapp.util.viewmodel.SimpleCWAViewModelFactory +import timber.log.Timber -class SubmissionQRCodeScanViewModel @AssistedInject constructor() : CWAViewModel() { - +class SubmissionQRCodeScanViewModel @AssistedInject constructor() : + CWAViewModel() { val routeToScreen: SingleLiveEvent<SubmissionNavigationEvents> = SingleLiveEvent() - private val _scanStatus = MutableLiveData(Event(ScanStatus.STARTED)) + val showRedeemedTokenWarning = SingleLiveEvent<Unit>() + val scanStatusValue = SingleLiveEvent<ScanStatus>() - val scanStatus: LiveData<Event<ScanStatus>> = _scanStatus + open class InvalidQRCodeException : Exception("error in qr code") - fun validateAndStoreTestGUID(rawResult: String) { + fun validateTestGUID(rawResult: String) { val scanResult = QRScanResult(rawResult) if (scanResult.isValid) { - SubmissionService.storeTestGUID(scanResult.guid!!) - _scanStatus.value = Event(ScanStatus.SUCCESS) + scanStatusValue.postValue(ScanStatus.SUCCESS) + doDeviceRegistration(scanResult) } else { - _scanStatus.value = Event(ScanStatus.INVALID) + scanStatusValue.postValue(ScanStatus.INVALID) } } val registrationState = MutableLiveData(ApiRequestState.IDLE) val registrationError = SingleLiveEvent<CwaWebException>() - fun doDeviceRegistration() = launch { + private fun doDeviceRegistration(scanResult: QRScanResult) = launch { try { registrationState.postValue(ApiRequestState.STARTED) - SubmissionService.asyncRegisterDevice() + checkTestResult(SubmissionService.asyncRegisterDeviceViaGUID(scanResult.guid!!)) registrationState.postValue(ApiRequestState.SUCCESS) } catch (err: CwaWebException) { registrationState.postValue(ApiRequestState.FAILED) @@ -52,12 +54,34 @@ class SubmissionQRCodeScanViewModel @AssistedInject constructor() : CWAViewModel err.report(ExceptionCategory.INTERNAL) } registrationState.postValue(ApiRequestState.FAILED) + } catch (err: InvalidQRCodeException) { + registrationState.postValue(ApiRequestState.FAILED) + deregisterTestFromDevice() + showRedeemedTokenWarning.postValue(Unit) } catch (err: Exception) { registrationState.postValue(ApiRequestState.FAILED) err.report(ExceptionCategory.INTERNAL) } } + private fun checkTestResult(testResult: TestResult) { + if (testResult == TestResult.REDEEMED) { + throw InvalidQRCodeException() + } + } + + private fun deregisterTestFromDevice() { + launch { + Timber.d("deregisterTestFromDevice()") + SubmissionService.deleteTestGUID() + SubmissionService.deleteRegistrationToken() + LocalData.isAllowedToSubmitDiagnosisKeys(false) + LocalData.initialTestResultReceivedTimestamp(0L) + + routeToScreen.postValue(SubmissionNavigationEvents.NavigateToMainActivity) + } + } + fun onBackPressed() { routeToScreen.postValue(SubmissionNavigationEvents.NavigateToQRInfo) } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/tan/SubmissionTanViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/tan/SubmissionTanViewModel.kt index b2fcfe37d..c947e09a9 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/tan/SubmissionTanViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/tan/SubmissionTanViewModel.kt @@ -51,7 +51,7 @@ class SubmissionTanViewModel @AssistedInject constructor( launch { try { registrationState.postValue(ApiRequestState.STARTED) - SubmissionService.asyncRegisterDevice() + SubmissionService.asyncRegisterDeviceViaTAN(teletan.value) registrationState.postValue(ApiRequestState.SUCCESS) } catch (err: CwaWebException) { registrationState.postValue(ApiRequestState.FAILED) diff --git a/Corona-Warn-App/src/main/res/values-de/strings.xml b/Corona-Warn-App/src/main/res/values-de/strings.xml index c181cbe69..0ec0abc64 100644 --- a/Corona-Warn-App/src/main/res/values-de/strings.xml +++ b/Corona-Warn-App/src/main/res/values-de/strings.xml @@ -799,9 +799,9 @@ <string name="submission_error_dialog_web_tan_invalid_button_positive">"Zurück"</string> <!-- XHED: Dialog title for submission tan redeemed --> - <string name="submission_error_dialog_web_tan_redeemed_title">"Test fehlerhaft"</string> + <string name="submission_error_dialog_web_tan_redeemed_title">"QR-Code nicht mehr gültig"</string> <!-- XMSG: Dialog body for submission tan redeemed --> - <string name="submission_error_dialog_web_tan_redeemed_body">"Es gab ein Problem bei der Auswertung Ihres Tests. Ihr QR Code ist bereits abgelaufen."</string> + <string name="submission_error_dialog_web_tan_redeemed_body">"Ihr Test liegt länger als 21 Tage zurück und kann nicht mehr in der App registriert werden. Bitte stellen Sie bei zukünftigen Tests sicher, dass Sie den QR-Code scannen, sobald er Ihnen vorliegt."</string> <!-- XBUT: Positive button for submission tan redeemed --> <string name="submission_error_dialog_web_tan_redeemed_button_positive">"OK"</string> 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 553c5c3a6..5645c8f49 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 @@ -1,6 +1,5 @@ package de.rki.coronawarnapp.service.submission -import de.rki.coronawarnapp.exception.NoGUIDOrTANSetException import de.rki.coronawarnapp.exception.NoRegistrationTokenSetException import de.rki.coronawarnapp.playbook.BackgroundNoise import de.rki.coronawarnapp.playbook.Playbook @@ -66,13 +65,6 @@ class SubmissionServiceTest { clearAllMocks() } - @Test - fun registerDeviceWithoutTANOrGUIDFails(): Unit = runBlocking { - shouldThrow<NoGUIDOrTANSetException> { - SubmissionService.asyncRegisterDevice() - } - } - @Test fun registrationWithGUIDSucceeds() { every { LocalData.testGUID() } returns guid @@ -89,7 +81,7 @@ class SubmissionServiceTest { every { backgroundNoise.scheduleDummyPattern() } just Runs runBlocking { - SubmissionService.asyncRegisterDevice() + SubmissionService.asyncRegisterDeviceViaGUID(guid) } verify(exactly = 1) { @@ -117,7 +109,7 @@ class SubmissionServiceTest { every { backgroundNoise.scheduleDummyPattern() } just Runs runBlocking { - SubmissionService.asyncRegisterDevice() + SubmissionService.asyncRegisterDeviceViaTAN(guid) } verify(exactly = 1) { diff --git a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModelTest.kt b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModelTest.kt index b518ae31a..d11445344 100644 --- a/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModelTest.kt +++ b/Corona-Warn-App/src/test/java/de/rki/coronawarnapp/ui/submission/qrcode/scan/SubmissionQRCodeScanViewModelTest.kt @@ -40,15 +40,17 @@ class SubmissionQRCodeScanViewModelTest : BaseTest() { val viewModel = createViewModel() // start - viewModel.scanStatus.value!!.getContent() shouldBe ScanStatus.STARTED + viewModel.scanStatusValue.value = ScanStatus.STARTED + + viewModel.scanStatusValue.value shouldBe ScanStatus.STARTED // valid guid val guid = "123456-12345678-1234-4DA7-B166-B86D85475064" - viewModel.validateAndStoreTestGUID("https://localhost/?$guid") - viewModel.scanStatus.value?.getContent().let { Assert.assertEquals(ScanStatus.SUCCESS, it) } + viewModel.validateTestGUID("https://localhost/?$guid") + viewModel.scanStatusValue.let { Assert.assertEquals(ScanStatus.SUCCESS, it.value) } // invalid guid - viewModel.validateAndStoreTestGUID("https://no-guid-here") - viewModel.scanStatus.value?.getContent().let { Assert.assertEquals(ScanStatus.INVALID, it) } + viewModel.validateTestGUID("https://no-guid-here") + viewModel.scanStatusValue.let { Assert.assertEquals(ScanStatus.INVALID, it.value) } } } -- GitLab