From bb93699a8d49dc8dfdc5e900c10cb80715f490c7 Mon Sep 17 00:00:00 2001 From: Mohamed <mohamed.metwalli@sap.com> Date: Tue, 4 May 2021 16:35:42 +0200 Subject: [PATCH] Test not found exception (EXPOSUREAPP-6870) (#3067) * Improve logging * Catch exception and split into smaller functions * Remove finally clause * Remove inner launch Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../ShareTestResultNotification.kt | 8 +- .../ShareTestResultNotificationService.kt | 114 +++++++++++------- .../notification/GeneralNotifications.kt | 9 +- .../SubmissionDeletionWarningViewModel.kt | 3 - .../submission/tan/SubmissionTanViewModel.kt | 3 - ...tPositiveOtherWarningNoConsentViewModel.kt | 16 ++- 6 files changed, 89 insertions(+), 64 deletions(-) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotification.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotification.kt index 8c2a92737..7148ec4c4 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotification.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotification.kt @@ -34,7 +34,7 @@ class ShareTestResultNotification @Inject constructor( } fun showSharePositiveTestResultNotification(notificationId: Int, testType: CoronaTest.Type) { - Timber.d("showSharePositiveTestResultNotification(notificationId=$notificationId)") + Timber.tag(TAG).d("showSharePositiveTestResultNotification(notificationId=$notificationId)") val args = Bundle().apply { putSerializable("testType", testType) } @@ -59,6 +59,10 @@ class ShareTestResultNotification @Inject constructor( fun cancelSharePositiveTestResultNotification(testType: CoronaTest.Type) { notificationHelper.cancelFutureNotifications(POSITIVE_RESULT_NOTIFICATION_ID, testType) - Timber.v("Future positive test result notifications have been canceled") + Timber.tag(TAG).v("Future positive test result notifications have been canceled") + } + + companion object { + private val TAG = ShareTestResultNotification::class.simpleName } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotificationService.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotificationService.kt index 155b74a70..8167c7550 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotificationService.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/coronatest/notification/ShareTestResultNotificationService.kt @@ -10,7 +10,6 @@ import de.rki.coronawarnapp.main.CWASettings import de.rki.coronawarnapp.notification.NotificationConstants.POSITIVE_RESULT_NOTIFICATION_TOTAL_COUNT import de.rki.coronawarnapp.util.coroutine.AppScope import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.flow.catch import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import timber.log.Timber @@ -27,48 +26,20 @@ class ShareTestResultNotificationService @Inject constructor( fun setup() { Timber.d("setup()") - - coronaTestRepository.coronaTests - .onEach { tests -> - - // schedule reminder if test wasn't submitted - tests.filter { test -> - test.isSubmissionAllowed && !test.isSubmitted - }.forEach { test -> - maybeScheduleSharePositiveTestResultReminder(test.type) - } - - // cancel the reminder when test is submitted - tests - .filter { it.isSubmitted } - .forEach { notification.cancelSharePositiveTestResultNotification(it.type) } - } - .catch { Timber.e(it, "Failed to schedule positive test result reminder.") } - .launchIn(appScope) - + // Schedule positive test reminders + schedulePositiveTestsReminder() // if no PCR test is stored or if it was deleted, we reset the reminder - coronaTestRepository.latestPCRT - .onEach { - if (it == null) { - resetSharePositiveTestResultNotification(PCR) - } - } - .catch { Timber.e(it, "Failed to reset positive test result reminder for PCR test.") } - .launchIn(appScope) - + resetPositivePCRTestReminder() // if no RAT test is stored or if it was deleted, we reset the reminder - coronaTestRepository.latestRAT - .onEach { - if (it == null) { - resetSharePositiveTestResultNotification(RAPID_ANTIGEN) - } - } - .catch { Timber.e(it, "Failed to reset positive test result reminder for RAT test.") } - .launchIn(appScope) + resetPositiveRATTestReminder() } fun maybeShowSharePositiveTestResultNotification(notificationId: Int, testType: CoronaTest.Type) { - Timber.d("maybeShowSharePositiveTestResultNotification(notificationId=$notificationId)") + Timber.tag(TAG).d( + "maybeShowSharePositiveTestResultNotification(notificationId=%s,testType=%s)", + notificationId, + testType + ) if (testType == PCR) { if (cwaSettings.numberOfRemainingSharePositiveTestResultRemindersPcr > 0) { cwaSettings.numberOfRemainingSharePositiveTestResultRemindersPcr -= 1 @@ -86,32 +57,85 @@ class ShareTestResultNotificationService @Inject constructor( } } + private fun resetPositivePCRTestReminder() { + Timber.tag(TAG).v("resetPositivePCRTestReminder") + try { + coronaTestRepository.latestPCRT + .onEach { + if (it == null) { + resetSharePositiveTestResultNotification(PCR) + } + } + .launchIn(appScope) + } catch (e: Exception) { + Timber.tag(TAG).e(e, "Failed to reset positive test result reminder for PCR test.") + } + } + + private fun resetPositiveRATTestReminder() { + Timber.tag(TAG).v("resetPositiveRATTestReminder") + try { + coronaTestRepository.latestRAT + .onEach { + if (it == null) { + resetSharePositiveTestResultNotification(RAPID_ANTIGEN) + } + } + .launchIn(appScope) + } catch (e: Exception) { + Timber.tag(TAG).e(e, "Failed to reset positive test result reminder for RAT test.") + } + } + + private fun schedulePositiveTestsReminder() { + Timber.tag(TAG).v("schedulePositiveTestsReminder") + try { + coronaTestRepository.coronaTests + .onEach { tests -> + // schedule reminder if test wasn't submitted + tests.filter { test -> + test.isSubmissionAllowed && !test.isSubmitted + }.forEach { test -> + maybeScheduleSharePositiveTestResultReminder(test.type) + } + + // cancel the reminder when test is submitted + tests.filter { it.isSubmitted } + .forEach { notification.cancelSharePositiveTestResultNotification(it.type) } + } + .launchIn(appScope) + } catch (e: Exception) { + Timber.e(e, "Failed to schedule positive test result reminder.") + } + } + private fun maybeScheduleSharePositiveTestResultReminder(testType: CoronaTest.Type) { when (testType) { PCR -> { if (cwaSettings.numberOfRemainingSharePositiveTestResultRemindersPcr < 0) { - Timber.v("Schedule positive test result notification for PCR test") + Timber.tag(TAG).v("Schedule positive test result notification for PCR test") cwaSettings.numberOfRemainingSharePositiveTestResultRemindersPcr = POSITIVE_RESULT_NOTIFICATION_TOTAL_COUNT notification.scheduleSharePositiveTestResultReminder(testType) } else { - Timber.v("Positive test result notification for PCR test has already been scheduled") + Timber.tag(TAG).v("Positive test result notification for PCR test has already been scheduled") } } RAPID_ANTIGEN -> { if (cwaSettings.numberOfRemainingSharePositiveTestResultRemindersRat < 0) { - Timber.v("Schedule positive test result notification for RAT test") + Timber.tag(TAG).v("Schedule positive test result notification for RAT test") cwaSettings.numberOfRemainingSharePositiveTestResultRemindersRat = POSITIVE_RESULT_NOTIFICATION_TOTAL_COUNT notification.scheduleSharePositiveTestResultReminder(testType) } else { - Timber.v("Positive test result notification for RAT test has already been scheduled") + Timber.tag(TAG).v("Positive test result notification for RAT test has already been scheduled") } } } } private fun resetSharePositiveTestResultNotification(testType: CoronaTest.Type) { + Timber.tag(TAG).v("resetSharePositiveTestResultNotification(testType=%s)", testType) notification.cancelSharePositiveTestResultNotification(testType) when (testType) { @@ -119,6 +143,10 @@ class ShareTestResultNotificationService @Inject constructor( RAPID_ANTIGEN -> cwaSettings.numberOfRemainingSharePositiveTestResultRemindersRat = Int.MIN_VALUE } - Timber.v("Share positive test result notification counter has been reset for all test types") + Timber.tag(TAG).v("Share positive test result notification counter has been reset for all test types") + } + + companion object { + private val TAG = ShareTestResultNotificationService::class.simpleName } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/notification/GeneralNotifications.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/notification/GeneralNotifications.kt index 9bffd29f9..7361b2843 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/notification/GeneralNotifications.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/notification/GeneralNotifications.kt @@ -44,7 +44,7 @@ class GeneralNotifications @Inject constructor( @TargetApi(Build.VERSION_CODES.O) private fun setupNotificationChannel() { - Timber.d("setupChannel()") + Timber.tag(TAG).d("setupChannel()") val channel = NotificationChannel( MAIN_CHANNEL_ID, @@ -70,12 +70,12 @@ class GeneralNotifications @Inject constructor( val manager = context.getSystemService(Context.ALARM_SERVICE) as AlarmManager manager.cancel(pendingIntent) - Timber.v("Canceled future notifications with id: %s", notificationId) + Timber.tag(TAG).v("Canceled future notifications with id: %s", notificationId) } fun cancelCurrentNotification(notificationId: Int) { NotificationManagerCompat.from(context).cancel(notificationId) - Timber.v("Canceled notifications with id: %s", notificationId) + Timber.tag(TAG).v("Canceled notifications with id: %s", notificationId) } fun scheduleRepeatingNotification( @@ -138,12 +138,13 @@ class GeneralNotifications @Inject constructor( isNotificationChannelSetup = true setupNotificationChannel() } - Timber.i("Showing notification for ID=$notificationId: %s", notification) + Timber.tag(TAG).i("Showing notification for ID=$notificationId: %s", notification) notificationManagerCompat.notify(notificationId, notification) } companion object { @VisibleForTesting(otherwise = VisibleForTesting.PRIVATE) internal const val MAIN_CHANNEL_ID = "de.rki.coronawarnapp.notification.exposureNotificationChannelId" + private const val TAG = "GeneralNotifications" } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/deletionwarning/SubmissionDeletionWarningViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/deletionwarning/SubmissionDeletionWarningViewModel.kt index 7d0738a27..9a46d82ae 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/deletionwarning/SubmissionDeletionWarningViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/deletionwarning/SubmissionDeletionWarningViewModel.kt @@ -89,9 +89,6 @@ class SubmissionDeletionWarningViewModel @AssistedInject constructor( } catch (err: Exception) { mutableRegistrationState.postValue(RegistrationState(ApiRequestState.FAILED)) err.report(ExceptionCategory.INTERNAL) - } finally { - // TODO Should not be necessary? What new data would we - submissionRepository.refreshTest(type = CoronaTest.Type.PCR) } } 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 b55a94539..de7b107fc 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 @@ -89,9 +89,6 @@ class SubmissionTanViewModel @AssistedInject constructor( } catch (err: Exception) { registrationState.postValue(ApiRequestState.FAILED) err.report(ExceptionCategory.INTERNAL) - } finally { - // TODO Should not be necessary? What new data would we - submissionRepository.refreshTest(type = CoronaTest.Type.PCR) } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/warnothers/SubmissionResultPositiveOtherWarningNoConsentViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/warnothers/SubmissionResultPositiveOtherWarningNoConsentViewModel.kt index 14ac3802e..e17e0f40b 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/warnothers/SubmissionResultPositiveOtherWarningNoConsentViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/warnothers/SubmissionResultPositiveOtherWarningNoConsentViewModel.kt @@ -114,15 +114,13 @@ class SubmissionResultPositiveOtherWarningNoConsentViewModel @AssistedInject con fun onConsentButtonClicked() = launch { showKeysRetrievalProgress.postValue(true) submissionRepository.giveConsentToSubmission(type = testType) - launch { - if (enfClient.isTracingEnabled.first()) { - Timber.tag(TAG).d("tekHistoryUpdater.updateTEKHistoryOrRequestPermission()") - tekHistoryUpdater.updateTEKHistoryOrRequestPermission() - } else { - Timber.tag(TAG).d("showEnableTracingEvent:Unit") - showKeysRetrievalProgress.postValue(false) - showEnableTracingEvent.postValue(Unit) - } + if (enfClient.isTracingEnabled.first()) { + Timber.tag(TAG).d("tekHistoryUpdater.updateTEKHistoryOrRequestPermission()") + tekHistoryUpdater.updateTEKHistoryOrRequestPermission() + } else { + Timber.tag(TAG).d("showEnableTracingEvent:Unit") + showKeysRetrievalProgress.postValue(false) + showEnableTracingEvent.postValue(Unit) } } -- GitLab