From 85c023dc39c9a1b569f167a21500ddfa9cae1b1f Mon Sep 17 00:00:00 2001 From: Matthias Urhahn <matthias.urhahn@sap.com> Date: Thu, 19 Nov 2020 12:11:17 +0100 Subject: [PATCH] Fix unguarded launch calls on UI scopes (DEV) (#1625) * Move doBackgroundNoiseCheck call into MainActivity viewmodel and use guarded launch method from viewmodel. * Move startStopTracing to viewmodel and use guarded launch method. * Move `schedulePositiveTestResultReminder` from being launched in the fragment to viewmodels (with guarded launch). * Good morning LINTer Co-authored-by: harambasicluka <64483219+harambasicluka@users.noreply.github.com> --- .../rki/coronawarnapp/ui/main/MainActivity.kt | 11 +--- .../ui/main/MainActivityViewModel.kt | 7 ++ .../ui/main/home/HomeFragment.kt | 4 +- .../ui/main/home/HomeFragmentViewModel.kt | 3 +- .../SubmissionTestResultFragment.kt | 4 +- .../SubmissionTestResultViewModel.kt | 3 +- .../settings/SettingsTracingFragment.kt | 64 +++++-------------- .../SettingsTracingFragmentViewModel.kt | 51 ++++++++++++++- .../util/DefaultBackgroundPrioritization.kt | 2 + 9 files changed, 83 insertions(+), 66 deletions(-) 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 efb78a5cb..fd0e01406 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 @@ -10,13 +10,11 @@ import androidx.appcompat.app.AppCompatActivity import androidx.fragment.app.Fragment import androidx.fragment.app.FragmentManager import androidx.lifecycle.ViewModelProviders -import androidx.lifecycle.lifecycleScope import dagger.android.AndroidInjector import dagger.android.DispatchingAndroidInjector import dagger.android.HasAndroidInjector import de.rki.coronawarnapp.R import de.rki.coronawarnapp.deadman.DeadmanNotificationScheduler -import de.rki.coronawarnapp.playbook.BackgroundNoise import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.ui.base.startActivitySafely import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel @@ -30,7 +28,6 @@ import de.rki.coronawarnapp.util.ui.observe2 import de.rki.coronawarnapp.util.viewmodel.CWAViewModelFactoryProvider import de.rki.coronawarnapp.util.viewmodel.cwaViewModels import de.rki.coronawarnapp.worker.BackgroundWorkScheduler -import kotlinx.coroutines.launch import javax.inject.Inject /** @@ -105,16 +102,10 @@ class MainActivity : AppCompatActivity(), HasAndroidInjector { settingsViewModel.updateBackgroundJobEnabled(ConnectivityHelper.autoModeEnabled(this)) scheduleWork() checkShouldDisplayBackgroundWarning() - doBackgroundNoiseCheck() + vm.doBackgroundNoiseCheck() deadmanScheduler.schedulePeriodic() } - private fun doBackgroundNoiseCheck() { - lifecycleScope.launch { - BackgroundNoise.getInstance().foregroundScheduleCheck() - } - } - private fun showEnergyOptimizedEnabledForBackground() { val dialog = DialogHelper.DialogInstance( this, 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 8345f33c5..02dbea80d 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 @@ -2,6 +2,7 @@ package de.rki.coronawarnapp.ui.main import com.squareup.inject.assisted.AssistedInject import de.rki.coronawarnapp.environment.EnvironmentSetup +import de.rki.coronawarnapp.playbook.BackgroundNoise import de.rki.coronawarnapp.util.CWADebug import de.rki.coronawarnapp.util.coroutine.DispatcherProvider import de.rki.coronawarnapp.util.ui.SingleLiveEvent @@ -28,6 +29,12 @@ class MainActivityViewModel @AssistedInject constructor( } } + fun doBackgroundNoiseCheck() { + launch { + BackgroundNoise.getInstance().foregroundScheduleCheck() + } + } + @AssistedInject.Factory interface Factory : SimpleCWAViewModelFactory<MainActivityViewModel> } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragment.kt index aba0e9850..af99a6380 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragment.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragment.kt @@ -5,7 +5,6 @@ import android.os.Bundle import android.view.View import android.view.accessibility.AccessibilityEvent import androidx.fragment.app.Fragment -import androidx.lifecycle.lifecycleScope import de.rki.coronawarnapp.R import de.rki.coronawarnapp.databinding.FragmentHomeBinding import de.rki.coronawarnapp.util.DialogHelper @@ -18,7 +17,6 @@ import de.rki.coronawarnapp.util.ui.observe2 import de.rki.coronawarnapp.util.ui.viewBindingLazy import de.rki.coronawarnapp.util.viewmodel.CWAViewModelFactoryProvider import de.rki.coronawarnapp.util.viewmodel.cwaViewModels -import kotlinx.coroutines.launch import javax.inject.Inject /** @@ -100,7 +98,7 @@ class HomeFragment : Fragment(R.layout.fragment_home), AutoInject { } } - lifecycleScope.launch { vm.observeTestResultToSchedulePositiveTestResultReminder() } + vm.observeTestResultToSchedulePositiveTestResultReminder() } override fun onResume() { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragmentViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragmentViewModel.kt index 0141c47ef..5d4c54347 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragmentViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/main/home/HomeFragmentViewModel.kt @@ -76,10 +76,11 @@ class HomeFragmentViewModel @AssistedInject constructor( private var isLoweredRiskLevelDialogBeingShown = false - suspend fun observeTestResultToSchedulePositiveTestResultReminder() = + fun observeTestResultToSchedulePositiveTestResultReminder() = launch { submissionCardsStateProvider.state .first { it.isPositiveSubmissionCardVisible() } .also { testResultNotificationService.schedulePositiveTestResultReminder() } + } // TODO only lazy to keep tests going which would break because of LocalData access val showLoweredRiskLevelDialog: LiveData<Boolean> by lazy { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultFragment.kt index 0de5f0efa..86d6b9bc9 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultFragment.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultFragment.kt @@ -6,7 +6,6 @@ import android.view.View import android.view.accessibility.AccessibilityEvent import androidx.activity.OnBackPressedCallback import androidx.fragment.app.Fragment -import androidx.lifecycle.lifecycleScope import de.rki.coronawarnapp.R import de.rki.coronawarnapp.databinding.FragmentSubmissionTestResultBinding import de.rki.coronawarnapp.exception.http.CwaClientError @@ -22,7 +21,6 @@ import de.rki.coronawarnapp.util.ui.observe2 import de.rki.coronawarnapp.util.ui.viewBindingLazy import de.rki.coronawarnapp.util.viewmodel.CWAViewModelFactoryProvider import de.rki.coronawarnapp.util.viewmodel.cwaViewModels -import kotlinx.coroutines.launch import javax.inject.Inject class SubmissionTestResultFragment : Fragment(R.layout.fragment_submission_test_result), @@ -136,7 +134,7 @@ class SubmissionTestResultFragment : Fragment(R.layout.fragment_submission_test_ } } - lifecycleScope.launch { viewModel.observeTestResultToSchedulePositiveTestResultReminder() } + viewModel.observeTestResultToSchedulePositiveTestResultReminder() } override fun onResume() { diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultViewModel.kt index 370ecbf29..ff12359c1 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/submission/testresult/SubmissionTestResultViewModel.kt @@ -57,7 +57,7 @@ class SubmissionTestResultViewModel @AssistedInject constructor( ).let { emit(it) } }.asLiveData(context = dispatcherProvider.Default) - suspend fun observeTestResultToSchedulePositiveTestResultReminder() = + fun observeTestResultToSchedulePositiveTestResultReminder() = launch { SubmissionRepository.deviceUIStateFlow .first { request -> request.withSuccess(false) { @@ -65,6 +65,7 @@ class SubmissionTestResultViewModel @AssistedInject constructor( } } .also { testResultNotificationService.schedulePositiveTestResultReminder() } + } fun onBackPressed() { routeToScreen.postValue(SubmissionNavigationEvents.NavigateToMainActivity) diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragment.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragment.kt index 56ef3c2d2..489f83c59 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragment.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragment.kt @@ -5,17 +5,14 @@ import android.os.Bundle import android.view.View import android.view.accessibility.AccessibilityEvent import androidx.fragment.app.Fragment -import androidx.lifecycle.lifecycleScope import androidx.navigation.fragment.findNavController import de.rki.coronawarnapp.R import de.rki.coronawarnapp.databinding.FragmentSettingsTracingBinding -import de.rki.coronawarnapp.exception.ExceptionCategory -import de.rki.coronawarnapp.exception.reporting.report import de.rki.coronawarnapp.nearby.InternalExposureNotificationClient import de.rki.coronawarnapp.nearby.InternalExposureNotificationPermissionHelper -import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.ui.doNavigate import de.rki.coronawarnapp.ui.main.MainActivity +import de.rki.coronawarnapp.ui.tracing.settings.SettingsTracingFragmentViewModel.Event import de.rki.coronawarnapp.ui.viewmodel.SettingsViewModel import de.rki.coronawarnapp.util.DialogHelper import de.rki.coronawarnapp.util.ExternalActionHelper @@ -25,7 +22,6 @@ import de.rki.coronawarnapp.util.ui.viewBindingLazy import de.rki.coronawarnapp.util.viewmodel.CWAViewModelFactoryProvider import de.rki.coronawarnapp.util.viewmodel.cwaViewModels import de.rki.coronawarnapp.worker.BackgroundWorkScheduler -import kotlinx.coroutines.launch import timber.log.Timber import javax.inject.Inject @@ -47,7 +43,7 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), private val binding: FragmentSettingsTracingBinding by viewBindingLazy() - private lateinit var internalExposureNotificationPermissionHelper: InternalExposureNotificationPermissionHelper + private lateinit var exposureNotificationPermissionHelper: InternalExposureNotificationPermissionHelper override fun onViewCreated(view: View, savedInstanceState: Bundle?) { super.onViewCreated(view, savedInstanceState) @@ -63,11 +59,21 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), TracingSettingsState.BluetoothDisabled, TracingSettingsState.LocationDisabled -> setOnClickListener(null) TracingSettingsState.TracingInActive, - TracingSettingsState.TracingActive -> setOnClickListener { startStopTracing() } + TracingSettingsState.TracingActive -> setOnClickListener { vm.startStopTracing() } } } } + exposureNotificationPermissionHelper = InternalExposureNotificationPermissionHelper(this, this) + + vm.events.observe2(this) { + when (it) { + Event.RequestPermissions -> exposureNotificationPermissionHelper.requestPermissionToStartTracing() + Event.ShowConsentDialog -> showConsentDialog() + Event.ManualCheckingDialog -> showManualCheckingRequiredDialog() + } + } + setButtonOnClickListener() } @@ -77,7 +83,7 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), } override fun onActivityResult(requestCode: Int, resultCode: Int, data: Intent?) { - internalExposureNotificationPermissionHelper.onResolutionComplete( + exposureNotificationPermissionHelper.onResolutionComplete( requestCode, resultCode ) @@ -98,13 +104,10 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), val location = binding.settingsTracingStatusLocation.tracingStatusCardButton val interoperability = binding.settingsInteroperabilityRow.settingsPlainRow - internalExposureNotificationPermissionHelper = - InternalExposureNotificationPermissionHelper(this, this) switch.setOnCheckedChangeListener { view, _ -> - // Make sure that listener is called by user interaction if (view.isPressed) { - startStopTracing() + vm.startStopTracing() // Focus on the body text after to announce the tracing status for accessibility reasons binding.settingsTracingSwitchRow.settingsSwitchRowHeaderBody .sendAccessibilityEvent(AccessibilityEvent.TYPE_VIEW_ACCESSIBILITY_FOCUSED) @@ -131,39 +134,6 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), ) } - private fun startStopTracing() { - // if tracing is enabled when listener is activated it should be disabled - lifecycleScope.launch { - try { - if (InternalExposureNotificationClient.asyncIsEnabled()) { - InternalExposureNotificationClient.asyncStop() - BackgroundWorkScheduler.stopWorkScheduler() - } else { - // tracing was already activated - if (LocalData.initialTracingActivationTimestamp() != null) { - internalExposureNotificationPermissionHelper.requestPermissionToStartTracing() - } else { - // tracing was never activated - // ask for consent via dialog for initial tracing activation when tracing was not - // activated during onboarding - showConsentDialog() - // check if background processing is switched off, if it is, show the manual calculation dialog explanation before turning on. - val activity = requireActivity() as MainActivity - if (!activity.backgroundPrioritization.isBackgroundActivityPrioritized) { - showManualCheckingRequiredDialog() - } - } - } - } catch (exception: Exception) { - exception.report( - ExceptionCategory.EXPOSURENOTIFICATION, - TAG, - null - ) - } - } - } - private fun showManualCheckingRequiredDialog() { val dialog = DialogHelper.DialogInstance( requireActivity(), @@ -186,7 +156,7 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), R.string.onboarding_button_enable, R.string.onboarding_button_cancel, true, { - internalExposureNotificationPermissionHelper.requestPermissionToStartTracing() + exposureNotificationPermissionHelper.requestPermissionToStartTracing() }, { // Declined }) @@ -194,6 +164,6 @@ class SettingsTracingFragment : Fragment(R.layout.fragment_settings_tracing), } companion object { - private val TAG: String? = SettingsTracingFragment::class.simpleName + internal val TAG: String? = SettingsTracingFragment::class.simpleName } } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragmentViewModel.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragmentViewModel.kt index c9a546293..6b2e57dfa 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragmentViewModel.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/ui/tracing/settings/SettingsTracingFragmentViewModel.kt @@ -4,13 +4,20 @@ import androidx.lifecycle.LiveData import androidx.lifecycle.asLiveData import androidx.lifecycle.viewModelScope import com.squareup.inject.assisted.AssistedInject +import de.rki.coronawarnapp.exception.ExceptionCategory +import de.rki.coronawarnapp.exception.reporting.report +import de.rki.coronawarnapp.nearby.InternalExposureNotificationClient +import de.rki.coronawarnapp.storage.LocalData import de.rki.coronawarnapp.tracing.GeneralTracingStatus import de.rki.coronawarnapp.ui.tracing.details.TracingDetailsState import de.rki.coronawarnapp.ui.tracing.details.TracingDetailsStateProvider +import de.rki.coronawarnapp.util.BackgroundPrioritization import de.rki.coronawarnapp.util.coroutine.DispatcherProvider import de.rki.coronawarnapp.util.flow.shareLatest +import de.rki.coronawarnapp.util.ui.SingleLiveEvent import de.rki.coronawarnapp.util.viewmodel.CWAViewModel import de.rki.coronawarnapp.util.viewmodel.SimpleCWAViewModelFactory +import de.rki.coronawarnapp.worker.BackgroundWorkScheduler import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach @@ -20,7 +27,8 @@ import timber.log.Timber class SettingsTracingFragmentViewModel @AssistedInject constructor( dispatcherProvider: DispatcherProvider, tracingDetailsStateProvider: TracingDetailsStateProvider, - tracingStatus: GeneralTracingStatus + tracingStatus: GeneralTracingStatus, + private val backgroundPrioritization: BackgroundPrioritization ) : CWAViewModel(dispatcherProvider = dispatcherProvider) { val tracingDetailsState: LiveData<TracingDetailsState> = tracingDetailsStateProvider.state @@ -36,6 +44,47 @@ class SettingsTracingFragmentViewModel @AssistedInject constructor( ) .asLiveData(dispatcherProvider.Main) + val events = SingleLiveEvent<Event>() + + fun startStopTracing() { + // if tracing is enabled when listener is activated it should be disabled + launch { + try { + if (InternalExposureNotificationClient.asyncIsEnabled()) { + InternalExposureNotificationClient.asyncStop() + BackgroundWorkScheduler.stopWorkScheduler() + } else { + // tracing was already activated + if (LocalData.initialTracingActivationTimestamp() != null) { + events.postValue(Event.RequestPermissions) + } else { + // tracing was never activated + // ask for consent via dialog for initial tracing activation when tracing was not + // activated during onboarding + events.postValue(Event.ShowConsentDialog) + // check if background processing is switched off, + // if it is, show the manual calculation dialog explanation before turning on. + if (!backgroundPrioritization.isBackgroundActivityPrioritized) { + events.postValue(Event.ManualCheckingDialog) + } + } + } + } catch (exception: Exception) { + exception.report( + ExceptionCategory.EXPOSURENOTIFICATION, + SettingsTracingFragment.TAG, + null + ) + } + } + } + + sealed class Event { + object RequestPermissions : Event() + object ShowConsentDialog : Event() + object ManualCheckingDialog : Event() + } + @AssistedInject.Factory interface Factory : SimpleCWAViewModelFactory<SettingsTracingFragmentViewModel> } diff --git a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DefaultBackgroundPrioritization.kt b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DefaultBackgroundPrioritization.kt index 7cfd0d07b..eab3573bc 100644 --- a/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DefaultBackgroundPrioritization.kt +++ b/Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/DefaultBackgroundPrioritization.kt @@ -1,8 +1,10 @@ package de.rki.coronawarnapp.util +import dagger.Reusable import de.rki.coronawarnapp.util.device.PowerManagement import javax.inject.Inject +@Reusable class DefaultBackgroundPrioritization @Inject constructor( private val powerManagement: PowerManagement ) : BackgroundPrioritization { -- GitLab