From ac94ce208b76631f115f5002480d253d53db3d46 Mon Sep 17 00:00:00 2001 From: Michael Troger <11340859+michaeltroger@users.noreply.github.com> Date: Tue, 30 Apr 2024 22:21:23 +0200 Subject: [PATCH] Feature/fix file import crash (#248) * Make snakbar reusable * Move use case to View model * Show error instead of crashing when copying fails * Refactor * Add new line * Refactor * Suppress detekt --- .../michaeltroger/gruenerpass/MainActivity.kt | 75 ++++++++++++------- .../gruenerpass/MainViewModel.kt | 29 +++++-- .../CertificateDetailsFragment.kt | 2 +- .../certificates/CertificatesFragment.kt | 20 ++--- .../certificates/dialogs/CertificateErrors.kt | 16 ++++ .../certificates/dialogs/di/DialogsModule.kt | 13 +++- .../CertificatesListFragment.kt | 21 +++--- .../gruenerpass/pdfimporter/PdfImporter.kt | 15 +++- 8 files changed, 128 insertions(+), 63 deletions(-) create mode 100644 app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/CertificateErrors.kt diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/MainActivity.kt b/app/src/main/java/com/michaeltroger/gruenerpass/MainActivity.kt index 0d2f64ca..b6be68a1 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/MainActivity.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/MainActivity.kt @@ -14,6 +14,7 @@ import androidx.navigation.NavController import androidx.navigation.fragment.NavHostFragment import androidx.navigation.ui.AppBarConfiguration import androidx.navigation.ui.setupActionBarWithNavController +import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateErrors import com.michaeltroger.gruenerpass.extensions.getUri import com.michaeltroger.gruenerpass.navigation.GetAutoRedirectDestinationUseCase import com.michaeltroger.gruenerpass.navigation.GetStartDestinationUseCase @@ -25,6 +26,7 @@ import kotlinx.coroutines.launch private const val INTERACTION_TIMEOUT_MS = 5 * 60 * 1000L private const val PDF_MIME_TYPE = "application/pdf" +@Suppress("TooManyFunctions") @AndroidEntryPoint class MainActivity : AppCompatActivity(R.layout.activity_main), AddFile { @@ -32,12 +34,10 @@ class MainActivity : AppCompatActivity(R.layout.activity_main), AddFile { @Inject lateinit var preferenceUtil: PreferenceUtil - @Inject - lateinit var getStartDestinationUseCase: GetStartDestinationUseCase - + lateinit var certificateErrors: CertificateErrors @Inject - lateinit var getAutoRedirectDestinationUseCase: GetAutoRedirectDestinationUseCase + lateinit var getStartDestinationUseCase: GetStartDestinationUseCase private val timeoutHandler: Handler = Handler(Looper.getMainLooper()) private lateinit var interactionTimeoutRunnable: Runnable @@ -69,36 +69,57 @@ class MainActivity : AppCompatActivity(R.layout.activity_main), AddFile { setUpNavigation() repeatOnLifecycle(Lifecycle.State.STARTED) { - getAutoRedirectDestinationUseCase(navController!!).collect { navDestination -> - when (navDestination) { - GetAutoRedirectDestinationUseCase.Result.NavigateBack -> { - navController?.popBackStack() - } - is GetAutoRedirectDestinationUseCase.Result.NavigateTo -> { - navController?.navigate(navDestination.navDirections) - } - GetAutoRedirectDestinationUseCase.Result.NothingTodo -> { - // nothing to do - } - } + vm.getAutoRedirectDestination(navController!!).collect { + handleTargetDestination(it) + } + } + } + lifecycleScope.launch { + repeatOnLifecycle(Lifecycle.State.STARTED) { + vm.viewEvent.collect { + handleEvent(it) } } } } - private suspend fun setUpNavigation() { - val navHostFragment = supportFragmentManager.findFragmentById(R.id.nav_host_fragment) as NavHostFragment - val graphInflater = navHostFragment.navController.navInflater - val navGraph = graphInflater.inflate(R.navigation.nav_graph) - navController = navHostFragment.navController + private fun handleEvent(it: ViewEvent) { + when (it) { + ViewEvent.ShowParsingFileError -> { + certificateErrors.showFileErrorSnackbar(window.decorView.rootView) + } + } + } + + private fun handleTargetDestination(navDestination: GetAutoRedirectDestinationUseCase.Result) { + when (navDestination) { + GetAutoRedirectDestinationUseCase.Result.NavigateBack -> { + navController?.popBackStack() + } - navGraph.setStartDestination(getStartDestinationUseCase()) - navController!!.graph = navGraph + is GetAutoRedirectDestinationUseCase.Result.NavigateTo -> { + navController?.navigate(navDestination.navDirections) + } + + GetAutoRedirectDestinationUseCase.Result.NothingTodo -> { + // nothing to do + } + } + } - setupActionBarWithNavController( - navController = navController!!, - configuration = appBarConfiguration.build() - ) + private suspend fun setUpNavigation() { + val navHostFragment = supportFragmentManager.findFragmentById(R.id.nav_host_fragment) as NavHostFragment + navController = navHostFragment.navController.apply { + val navGraph = navInflater.inflate(R.navigation.nav_graph).apply { + setStartDestination(getStartDestinationUseCase()) + } + graph = navGraph + }.also { + setupActionBarWithNavController( + navController = it, + configuration = appBarConfiguration.build() + ) + } } private fun updateSettings() { diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/MainViewModel.kt b/app/src/main/java/com/michaeltroger/gruenerpass/MainViewModel.kt index 715e72e7..c1258a1c 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/MainViewModel.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/MainViewModel.kt @@ -3,9 +3,14 @@ package com.michaeltroger.gruenerpass import android.net.Uri import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope +import androidx.navigation.NavController import com.michaeltroger.gruenerpass.lock.AppLockedRepo +import com.michaeltroger.gruenerpass.navigation.GetAutoRedirectDestinationUseCase +import com.michaeltroger.gruenerpass.pdfimporter.PdfImportResult import com.michaeltroger.gruenerpass.pdfimporter.PdfImporter import dagger.hilt.android.lifecycle.HiltViewModel +import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.launch import javax.inject.Inject @@ -14,21 +19,31 @@ import javax.inject.Inject class MainViewModel @Inject constructor( private val pdfImporter: PdfImporter, private val lockedRepo: AppLockedRepo, + private val getAutoRedirectDestinationUseCase: GetAutoRedirectDestinationUseCase ): ViewModel() { - fun setPendingFile(uri: Uri) { - viewModelScope.launch { - pdfImporter.preparePendingFile(uri) + private val _viewEvent = MutableSharedFlow(extraBufferCapacity = 1) + val viewEvent: SharedFlow = _viewEvent + + fun getAutoRedirectDestination(navController: NavController) + = getAutoRedirectDestinationUseCase(navController) + + fun setPendingFile(uri: Uri) = viewModelScope.launch { + when (pdfImporter.preparePendingFile(uri)) { + is PdfImportResult.ParsingError -> _viewEvent.emit(ViewEvent.ShowParsingFileError) + else -> Unit } } - fun onInteractionTimeout() { - viewModelScope.launch { - lockedRepo.lockApp() - } + fun onInteractionTimeout() = viewModelScope.launch { + lockedRepo.lockApp() } override fun onCleared() { pdfImporter.deletePendingFile() } } + +sealed class ViewEvent { + data object ShowParsingFileError : ViewEvent() +} diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/certificatedetails/CertificateDetailsFragment.kt b/app/src/main/java/com/michaeltroger/gruenerpass/certificatedetails/CertificateDetailsFragment.kt index 02591559..951d7e27 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/certificatedetails/CertificateDetailsFragment.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/certificatedetails/CertificateDetailsFragment.kt @@ -117,7 +117,7 @@ class CertificateDetailsFragment : Fragment(R.layout.fragment_certificate_detail } override fun onDestroyView() { - binding!!.certificateFullscreen.adapter = null + binding?.certificateFullscreen?.adapter = null binding = null super.onDestroyView() } diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/certificates/CertificatesFragment.kt b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/CertificatesFragment.kt index 3cdbb24c..791ff785 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/certificates/CertificatesFragment.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/CertificatesFragment.kt @@ -11,11 +11,11 @@ import androidx.lifecycle.repeatOnLifecycle import androidx.navigation.fragment.findNavController import androidx.recyclerview.widget.PagerSnapHelper import androidx.recyclerview.widget.RecyclerView -import com.google.android.material.snackbar.Snackbar import com.michaeltroger.gruenerpass.AddFile import com.michaeltroger.gruenerpass.R import com.michaeltroger.gruenerpass.barcode.BarcodeRenderer import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateDialogs +import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateErrors import com.michaeltroger.gruenerpass.certificates.pager.item.CertificateItem import com.michaeltroger.gruenerpass.certificates.sharing.PdfSharing import com.michaeltroger.gruenerpass.certificates.states.ViewEvent @@ -51,6 +51,8 @@ class CertificatesFragment : Fragment(R.layout.fragment_certificates) { @Inject lateinit var certificateDialogs: CertificateDialogs @Inject + lateinit var certificateErrors: CertificateErrors + @Inject lateinit var barcodeRenderer: BarcodeRenderer private lateinit var menuProvider: CertificatesMenuProvider @@ -106,7 +108,11 @@ class CertificatesFragment : Fragment(R.layout.fragment_certificates) { onCancelled = vm::onPasswordDialogAborted ) - ViewEvent.ShowParsingFileError -> showFileCanNotBeReadError() + ViewEvent.ShowParsingFileError -> { + binding?.root?.let { + certificateErrors.showFileErrorSnackbar(it) + } + } is ViewEvent.GoToCertificate -> goToCertificate(it) is ViewEvent.ShareMultiple -> { pdfSharing.openShareAllFilePicker( @@ -170,14 +176,14 @@ class CertificatesFragment : Fragment(R.layout.fragment_certificates) { } override fun onDestroyView() { - binding!!.certificates.adapter = null + binding?.certificates?.adapter = null binding = null super.onDestroyView() } private fun updateState(state: ViewState) { menuProvider.updateMenuState(state) - binding!!.addButton.isVisible = state.showAddButton + binding?.addButton?.isVisible = state.showAddButton when (state) { is ViewState.Initial -> {} // nothing to do is ViewState.Empty -> { @@ -226,10 +232,4 @@ class CertificatesFragment : Fragment(R.layout.fragment_certificates) { binding?.certificates?.smoothScrollToPosition(event.position) } } - - private fun showFileCanNotBeReadError() { - binding!!.root.let { - Snackbar.make(it, R.string.error_reading_pdf, Snackbar.LENGTH_LONG).show() - } - } } diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/CertificateErrors.kt b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/CertificateErrors.kt new file mode 100644 index 00000000..eafeca5e --- /dev/null +++ b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/CertificateErrors.kt @@ -0,0 +1,16 @@ +package com.michaeltroger.gruenerpass.certificates.dialogs + +import android.view.View +import com.google.android.material.snackbar.Snackbar +import com.michaeltroger.gruenerpass.R +import javax.inject.Inject + +interface CertificateErrors { + fun showFileErrorSnackbar(view: View) +} + +class CertificateErrorsImpl @Inject constructor() : CertificateErrors { + override fun showFileErrorSnackbar(view: View) { + Snackbar.make(view, R.string.error_reading_pdf, Snackbar.LENGTH_LONG).show() + } +} diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/di/DialogsModule.kt b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/di/DialogsModule.kt index 19f5b469..6f936835 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/di/DialogsModule.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/certificates/dialogs/di/DialogsModule.kt @@ -3,17 +3,24 @@ package com.michaeltroger.gruenerpass.certificates.dialogs.di; import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateDialogs import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateDialogsImpl +import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateErrors +import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateErrorsImpl import dagger.Binds import dagger.Module import dagger.hilt.InstallIn -import dagger.hilt.android.components.FragmentComponent +import dagger.hilt.android.components.ActivityComponent @Module -@InstallIn(FragmentComponent::class) +@InstallIn(ActivityComponent::class) abstract class DialogsModule { @Binds abstract fun bindCertificateDialogs( - certificateDialogsImpl: CertificateDialogsImpl + impl: CertificateDialogsImpl ): CertificateDialogs + + @Binds + abstract fun bindCertificateErrors( + impl: CertificateErrorsImpl + ): CertificateErrors } diff --git a/app/src/main/java/com/michaeltroger/gruenerpass/certificateslist/CertificatesListFragment.kt b/app/src/main/java/com/michaeltroger/gruenerpass/certificateslist/CertificatesListFragment.kt index 15c01cb6..3cbde1bc 100644 --- a/app/src/main/java/com/michaeltroger/gruenerpass/certificateslist/CertificatesListFragment.kt +++ b/app/src/main/java/com/michaeltroger/gruenerpass/certificateslist/CertificatesListFragment.kt @@ -11,12 +11,12 @@ import androidx.lifecycle.repeatOnLifecycle import androidx.lifecycle.withStarted import androidx.navigation.fragment.findNavController import androidx.recyclerview.widget.DividerItemDecoration -import com.google.android.material.snackbar.Snackbar import com.michaeltroger.gruenerpass.AddFile import com.michaeltroger.gruenerpass.R import com.michaeltroger.gruenerpass.certificates.CertificatesMenuProvider import com.michaeltroger.gruenerpass.certificates.CertificatesViewModel import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateDialogs +import com.michaeltroger.gruenerpass.certificates.dialogs.CertificateErrors import com.michaeltroger.gruenerpass.certificates.sharing.PdfSharing import com.michaeltroger.gruenerpass.certificates.states.ViewEvent import com.michaeltroger.gruenerpass.certificates.states.ViewState @@ -41,6 +41,8 @@ class CertificatesListFragment : Fragment(R.layout.fragment_certificates_list) { lateinit var pdfSharing: PdfSharing @Inject lateinit var certificateDialogs: CertificateDialogs + @Inject + lateinit var certificateErrors: CertificateErrors private lateinit var menuProvider: CertificatesMenuProvider @@ -92,8 +94,11 @@ class CertificatesListFragment : Fragment(R.layout.fragment_certificates_list) { onPasswordEntered = vm::onPasswordEntered, onCancelled = vm::onPasswordDialogAborted ) - - ViewEvent.ShowParsingFileError -> showFileCanNotBeReadError() + ViewEvent.ShowParsingFileError -> { + binding?.root?.let { + certificateErrors.showFileErrorSnackbar(it) + } + } is ViewEvent.GoToCertificate -> goToCertificate(it) is ViewEvent.ShareMultiple -> { pdfSharing.openShareAllFilePicker( @@ -157,14 +162,14 @@ class CertificatesListFragment : Fragment(R.layout.fragment_certificates_list) { } override fun onDestroyView() { - binding!!.certificates.adapter = null + binding?.certificates?.adapter = null binding = null super.onDestroyView() } private fun updateState(state: ViewState) { menuProvider.updateMenuState(state) - binding!!.addButton.isVisible = state.showAddButton + binding?.addButton?.isVisible = state.showAddButton when (state) { is ViewState.Initial -> {} // nothing to do is ViewState.Empty -> { @@ -217,10 +222,4 @@ class CertificatesListFragment : Fragment(R.layout.fragment_certificates_list) { } } } - - private fun showFileCanNotBeReadError() { - binding!!.root.let { - Snackbar.make(it, R.string.error_reading_pdf, Snackbar.LENGTH_LONG).show() - } - } } diff --git a/pdfimporter/src/main/java/com/michaeltroger/gruenerpass/pdfimporter/PdfImporter.kt b/pdfimporter/src/main/java/com/michaeltroger/gruenerpass/pdfimporter/PdfImporter.kt index 83ca4740..44a94075 100644 --- a/pdfimporter/src/main/java/com/michaeltroger/gruenerpass/pdfimporter/PdfImporter.kt +++ b/pdfimporter/src/main/java/com/michaeltroger/gruenerpass/pdfimporter/PdfImporter.kt @@ -14,7 +14,7 @@ import kotlinx.coroutines.flow.map import javax.inject.Inject public interface PdfImporter { - public suspend fun preparePendingFile(uri: Uri) + public suspend fun preparePendingFile(uri: Uri): PdfImportResult public fun hasPendingFile(): Flow public fun deletePendingFile() public suspend fun importPendingFile(password: String?): PdfImportResult @@ -32,10 +32,17 @@ internal class PdfImporterImpl @Inject constructor( ) private val pendingFile: StateFlow = _pendingFile - override suspend fun preparePendingFile(uri: Uri) { + @Suppress("TooGenericExceptionCaught") + override suspend fun preparePendingFile(uri: Uri): PdfImportResult { deletePendingFile() - _pendingFile.value = fileRepo.copyToApp(uri) - logger.logDebug(pendingFile.value) + return try { + _pendingFile.value = fileRepo.copyToApp(uri) + logger.logDebug(pendingFile.value) + PdfImportResult.Success(pendingFile.value!!) + } catch (e: Exception) { + logger.logError(e.toString()) + PdfImportResult.ParsingError + } } override fun hasPendingFile(): Flow = pendingFile.map {