Fixed: Application crash caused by "Input dispatching timed out" while retrieving storageDeviceList information in the online library.

* Moved the storage device retrieval to the IO thread.
* Updated the StorageDeviceUtils.getWritableStorage method to a suspend function to ensure execution on the IO thread.
* Refactored CopyMoveFileHandler, OnlineLibraryFragment, KiwixPrefsFragment, and LocalLibraryFragment to adapt to this change.
* Optimized storage device retrieval by caching it in the main activity and reusing it across fragments to enhance performance, especially with large SD cards.
* Refactored the CopyMoveFileHandlerTest.
* Improved the showing of storage device list.
This commit is contained in:
MohitMaliFtechiz 2025-01-06 21:34:14 +05:30 committed by MohitMaliFtechiz
parent 3c4614e3d7
commit 7f6f0d4347
8 changed files with 110 additions and 88 deletions

View File

@ -40,6 +40,7 @@ import androidx.navigation.fragment.NavHostFragment
import androidx.navigation.ui.NavigationUI
import androidx.navigation.ui.setupWithNavController
import com.google.android.material.navigation.NavigationView
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.BuildConfig
@ -115,6 +116,7 @@ class KiwixMainActivity : CoreMainActivity() {
NavController.OnDestinationChangedListener { _, _, _ ->
actionMode?.finish()
}
private val storageDeviceList = arrayListOf<StorageDevice>()
override fun onCreate(savedInstanceState: Bundle?) {
cachedComponent.inject(this)
@ -142,7 +144,7 @@ class KiwixMainActivity : CoreMainActivity() {
private suspend fun migrateInternalToPublicAppDirectory() {
if (!sharedPreferenceUtil.prefIsAppDirectoryMigrated) {
val storagePath = StorageDeviceUtils.getWritableStorage(this)
val storagePath = getStorageDeviceList()
.getOrNull(sharedPreferenceUtil.storagePosition)
?.name
storagePath?.let {
@ -152,6 +154,23 @@ class KiwixMainActivity : CoreMainActivity() {
}
}
/**
* Fetches the storage device list once in the main activity and reuses it across all fragments.
* This is necessary because retrieving the storage device list, especially on devices with large SD cards,
* is a resource-intensive operation. Performing this operation repeatedly in fragments can negatively
* affect the user experience, as it takes time and can block the UI.
*
* If a fragment is destroyed and we need to retrieve the device list again, performing the operation
* repeatedly leads to inefficiency. To optimize this, we fetch the storage device list once and reuse
* it in all fragments, thereby reducing redundant processing and improving performance.
*/
suspend fun getStorageDeviceList(): List<StorageDevice> {
if (storageDeviceList.isEmpty()) {
storageDeviceList.addAll(StorageDeviceUtils.getWritableStorage(this))
}
return storageDeviceList
}
override fun onConfigurationChanged(newConfig: Configuration) {
super.onConfigurationChanged(newConfig)
if (::activityKiwixMainBinding.isInitialized) {

View File

@ -33,7 +33,6 @@ import androidx.documentfile.provider.DocumentFile
import androidx.fragment.app.FragmentManager
import eu.mhutti1.utils.storage.STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
@ -53,6 +52,7 @@ import org.kiwix.kiwixmobile.core.utils.INTERNAL_SELECT_POSITION
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower
import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.Fat32Checker
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.Companion.FOUR_GIGABYTES_IN_KILOBYTES
import org.kiwix.kiwixmobile.zimManager.Fat32Checker.FileSystemState.CannotWrite4GbFile
@ -83,9 +83,6 @@ class CopyMoveFileHandler @Inject constructor(
var shouldValidateZimFile: Boolean = false
private var fileSystemDisposable: Disposable? = null
private lateinit var fragmentManager: FragmentManager
val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(activity)
}
private val copyMoveTitle: String by lazy {
if (isMoveOperation) {
@ -111,18 +108,23 @@ class CopyMoveFileHandler @Inject constructor(
this.shouldValidateZimFile = shouldValidateZimFile
this.fragmentManager = fragmentManager
setSelectedFileAndUri(uri, documentFile)
if (sharedPreferenceUtil.shouldShowStorageSelectionDialog && storageDeviceList.size > 1) {
if (getStorageDeviceList().isEmpty()) {
showPreparingCopyMoveDialog()
}
if (sharedPreferenceUtil.shouldShowStorageSelectionDialog && getStorageDeviceList().size > 1) {
// Show dialog to select storage if more than one storage device is available, and user
// have not configured the storage yet.
hidePreparingCopyMoveDialog()
showCopyMoveDialog(true)
} else {
if (storageDeviceList.size == 1) {
if (getStorageDeviceList().size == 1) {
// If only internal storage is currently available, set shouldShowStorageSelectionDialog
// to true. This allows the storage configuration dialog to be shown again if the
// user removes an external storage device (like an SD card) and then reinserts it.
// This ensures they are prompted to configure storage settings upon SD card reinsertion.
sharedPreferenceUtil.shouldShowStorageSelectionDialog = true
}
hidePreparingCopyMoveDialog()
if (validateZimFileCanCopyOrMove()) {
showCopyMoveDialog()
}
@ -142,17 +144,15 @@ class CopyMoveFileHandler @Inject constructor(
lifecycleScope = coroutineScope
}
fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::copyMoveZIMFileInSelectedStorage
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(
fragmentManager,
activity.getString(R.string.choose_storage_to_copy_move_zim_file)
)
fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::copyMoveZIMFileInSelectedStorage
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(fragmentManager, activity.getString(R.string.choose_storage_to_copy_move_zim_file))
fun copyMoveZIMFileInSelectedStorage(storageDevice: StorageDevice) {
lifecycleScope?.launch {
@ -259,19 +259,23 @@ class CopyMoveFileHandler @Inject constructor(
fun performCopyOperation(showStorageSelectionDialog: Boolean = false) {
isMoveOperation = false
if (showStorageSelectionDialog) {
showStorageSelectDialog()
} else {
copyZimFileToPublicAppDirectory()
lifecycleScope?.launch {
if (showStorageSelectionDialog) {
showStorageSelectDialog(getStorageDeviceList())
} else {
copyZimFileToPublicAppDirectory()
}
}
}
fun performMoveOperation(showStorageSelectionDialog: Boolean = false) {
isMoveOperation = true
if (showStorageSelectionDialog) {
showStorageSelectDialog()
} else {
moveZimFileToPublicAppDirectory()
lifecycleScope?.launch {
if (showStorageSelectionDialog) {
showStorageSelectDialog(getStorageDeviceList())
} else {
moveZimFileToPublicAppDirectory()
}
}
}
@ -538,6 +542,9 @@ class CopyMoveFileHandler @Inject constructor(
}
}
suspend fun getStorageDeviceList() =
(activity as KiwixMainActivity).getStorageDeviceList()
fun dispose() {
fileSystemDisposable?.dispose()
setFileCopyMoveCallback(null)

View File

@ -58,7 +58,6 @@ import androidx.recyclerview.widget.RecyclerView
import com.google.android.material.bottomnavigation.BottomNavigationView
import eu.mhutti1.utils.storage.Bytes
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.CompositeDisposable
@ -99,6 +98,7 @@ import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDis
import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem
import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.adapter.BooksOnDiskListItem.BookOnDisk
import org.kiwix.kiwixmobile.databinding.FragmentDestinationLibraryBinding
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.MAX_PROGRESS
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel.FileSelectActions
@ -135,9 +135,6 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
private val zimManageViewModel by lazy {
requireActivity().viewModel<ZimManageViewModel>(viewModelFactory)
}
private val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(requireActivity())
}
private var storagePermissionLauncher: ActivityResultLauncher<Array<String>>? =
registerForActivityResult(
@ -665,17 +662,22 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal
message,
requireActivity().findViewById(R.id.bottom_nav_view),
string.download_change_storage,
::showStorageSelectDialog
{
lifecycleScope.launch {
showStorageSelectDialog((requireActivity() as KiwixMainActivity).getStorageDeviceList())
}
}
)
}
private fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(true)
}
.show(parentFragmentManager, getString(string.pref_storage))
private fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(true)
}
.show(parentFragmentManager, getString(string.pref_storage))
private fun storeDeviceInPreferences(
storageDevice: StorageDevice

View File

@ -53,7 +53,6 @@ import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import eu.mhutti1.utils.storage.STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import eu.mhutti1.utils.storage.StorageSelectDialog
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.R
@ -93,6 +92,7 @@ import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog
import org.kiwix.kiwixmobile.core.utils.dialog.KiwixDialog.YesNoDialog.WifiOnly
import org.kiwix.kiwixmobile.core.zim_manager.NetworkState
import org.kiwix.kiwixmobile.databinding.FragmentDestinationDownloadBinding
import org.kiwix.kiwixmobile.main.KiwixMainActivity
import org.kiwix.kiwixmobile.zimManager.ZimManageViewModel
import org.kiwix.kiwixmobile.zimManager.libraryView.AvailableSpaceCalculator
import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryAdapter
@ -116,9 +116,6 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
private val zimManageViewModel by lazy {
requireActivity().viewModel<ZimManageViewModel>(viewModelFactory)
}
private val storageDeviceList by lazy {
StorageDeviceUtils.getWritableStorage(requireActivity())
}
@VisibleForTesting
fun getOnlineLibraryList() = libraryAdapter.items
@ -550,8 +547,8 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
else -> if (sharedPreferenceUtil.showStorageOption) {
// Show the storage selection dialog for configuration if there is an SD card available.
if (storageDeviceList.size > 1) {
showStorageSelectDialog()
if (getStorageDeviceList().size > 1) {
showStorageSelectDialog(getStorageDeviceList())
} else {
// If only internal storage is available, proceed with the ZIM file download directly.
// Displaying a configuration dialog is unnecessary in this case.
@ -575,7 +572,11 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
""".trimIndent(),
requireActivity().findViewById(R.id.bottom_nav_view),
string.download_change_storage,
::showStorageSelectDialog
{
lifecycleScope.launch {
showStorageSelectDialog(getStorageDeviceList())
}
}
)
}
)
@ -588,14 +589,15 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
}
}
private fun showStorageSelectDialog() = StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(parentFragmentManager, getString(string.choose_storage_to_download_book))
private fun showStorageSelectDialog(storageDeviceList: List<StorageDevice>) =
StorageSelectDialog()
.apply {
onSelectAction = ::storeDeviceInPreferences
titleSize = STORAGE_SELECT_STORAGE_TITLE_TEXTVIEW_SIZE
setStorageDeviceList(storageDeviceList)
setShouldShowCheckboxSelected(false)
}
.show(parentFragmentManager, getString(string.choose_storage_to_download_book))
private fun clickOnBookItem() {
if (!requireActivity().isManageExternalStoragePermissionGranted(sharedPreferenceUtil)) {
@ -615,4 +617,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions {
)
}
}
private suspend fun getStorageDeviceList() =
(activity as KiwixMainActivity).getStorageDeviceList()
}

View File

@ -26,11 +26,6 @@ import androidx.lifecycle.lifecycleScope
import androidx.preference.Preference
import androidx.preference.PreferenceCategory
import eu.mhutti1.utils.storage.StorageDevice
import eu.mhutti1.utils.storage.StorageDeviceUtils
import io.reactivex.Flowable
import io.reactivex.android.schedulers.AndroidSchedulers
import io.reactivex.disposables.Disposable
import io.reactivex.schedulers.Schedulers
import kotlinx.coroutines.launch
import org.kiwix.kiwixmobile.core.R
import org.kiwix.kiwixmobile.core.extensions.getFreeSpace
@ -44,11 +39,11 @@ import org.kiwix.kiwixmobile.core.settings.StorageRadioButtonPreference
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.Companion.PREF_EXTERNAL_STORAGE
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil.Companion.PREF_INTERNAL_STORAGE
import org.kiwix.kiwixmobile.main.KiwixMainActivity
const val PREF_STORAGE_PROGRESSBAR = "storage_progressbar"
class KiwixPrefsFragment : CorePrefsFragment() {
private var storageDisposable: Disposable? = null
private var storageDeviceList: List<StorageDevice> = listOf()
override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
@ -65,20 +60,13 @@ class KiwixPrefsFragment : CorePrefsFragment() {
return@setStorage
}
showHideProgressBarWhileFetchingStorageInfo(true)
storageDisposable =
Flowable.fromCallable { StorageDeviceUtils.getWritableStorage(requireActivity()) }
.subscribeOn(Schedulers.io())
.observeOn(AndroidSchedulers.mainThread())
.subscribe(
{ storageList ->
storageDeviceList = storageList
showHideProgressBarWhileFetchingStorageInfo(false)
showInternalStoragePreferece()
showExternalPreferenceIfAvailable()
setUpStoragePreference(it)
},
Throwable::printStackTrace
)
lifecycleScope.launch {
storageDeviceList = (requireActivity() as KiwixMainActivity).getStorageDeviceList()
showHideProgressBarWhileFetchingStorageInfo(false)
showInternalStoragePreferece()
showExternalPreferenceIfAvailable()
setUpStoragePreference(it)
}
}
}
@ -157,12 +145,6 @@ class KiwixPrefsFragment : CorePrefsFragment() {
preferenceCategory?.isVisible = true
}
override fun onDestroyView() {
storageDisposable?.dispose()
storageDisposable = null
super.onDestroyView()
}
companion object {
const val PREF_MANAGE_EXTERNAL_STORAGE_PERMISSION =
"pref_manage_external_storage"

View File

@ -22,6 +22,7 @@ import android.app.Activity
import android.net.Uri
import androidx.documentfile.provider.DocumentFile
import androidx.fragment.app.FragmentManager
import eu.mhutti1.utils.storage.StorageDevice
import io.mockk.Runs
import io.mockk.clearAllMocks
import io.mockk.coEvery
@ -207,9 +208,11 @@ class CopyMoveFileHandlerTest {
@Test
fun showStorageConfigureDialogAtFirstLaunch() = runBlocking {
fileHandler = spyk(fileHandler)
every { fileHandler.showStorageSelectDialog() } just Runs
val storageDeviceList = listOf<StorageDevice>(mockk(), mockk())
every { fileHandler.showStorageSelectDialog(storageDeviceList) } just Runs
every { sharedPreferenceUtil.shouldShowStorageSelectionDialog } returns true
every { fileHandler.storageDeviceList } returns listOf(mockk(), mockk())
coEvery { fileHandler.getStorageDeviceList() } returns storageDeviceList
val positiveButtonClickSlot = slot<() -> Unit>()
every {
alertDialogShower.show(
@ -221,14 +224,15 @@ class CopyMoveFileHandlerTest {
fileHandler.showMoveFileToPublicDirectoryDialog(fragmentManager = fragmentManager)
coEvery { fileHandler.validateZimFileCanCopyOrMove() } returns true
positiveButtonClickSlot.captured.invoke()
verify { fileHandler.showStorageSelectDialog() }
testDispatcher.scheduler.advanceUntilIdle()
coVerify { fileHandler.showStorageSelectDialog(storageDeviceList) }
}
@Test
fun shouldNotShowStorageConfigureDialogWhenThereIsOnlyInternalAvailable() = runBlocking {
fileHandler = spyk(fileHandler)
every { sharedPreferenceUtil.shouldShowStorageSelectionDialog } returns true
every { fileHandler.storageDeviceList } returns listOf(mockk())
coEvery { fileHandler.getStorageDeviceList() } returns listOf(mockk())
val positiveButtonClickSlot = slot<() -> Unit>()
every {
alertDialogShower.show(
@ -240,14 +244,14 @@ class CopyMoveFileHandlerTest {
coEvery { fileHandler.validateZimFileCanCopyOrMove() } returns true
fileHandler.showMoveFileToPublicDirectoryDialog(fragmentManager = fragmentManager)
positiveButtonClickSlot.captured.invoke()
verify(exactly = 0) { fileHandler.showStorageSelectDialog() }
verify(exactly = 0) { fileHandler.showStorageSelectDialog(listOf(mockk())) }
}
@Test
fun showDirectlyCopyMoveDialogAfterFirstLaunch() = runBlocking {
fileHandler = spyk(fileHandler)
every { sharedPreferenceUtil.shouldShowStorageSelectionDialog } returns false
every { fileHandler.storageDeviceList } returns listOf(mockk(), mockk())
coEvery { fileHandler.getStorageDeviceList() } returns listOf(mockk(), mockk())
coEvery { fileHandler.validateZimFileCanCopyOrMove() } returns true
prepareFileSystemAndFileForMockk()
every { alertDialogShower.show(any(), any(), any()) } just Runs
@ -267,7 +271,7 @@ class CopyMoveFileHandlerTest {
val positiveButtonClickSlot = slot<() -> Unit>()
val negativeButtonClickSlot = slot<() -> Unit>()
fileHandler = spyk(fileHandler)
every { fileHandler.storageDeviceList } returns listOf(mockk(), mockk())
coEvery { fileHandler.getStorageDeviceList() } returns listOf(mockk(), mockk())
every { sharedPreferenceUtil.shouldShowStorageSelectionDialog } returns false
every {
alertDialogShower.show(

View File

@ -21,6 +21,8 @@ package eu.mhutti1.utils.storage
import android.content.Context
import android.content.ContextWrapper
import android.os.Environment
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.withContext
import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil
import java.io.File
import java.io.FileFilter
@ -28,8 +30,9 @@ import java.io.RandomAccessFile
object StorageDeviceUtils {
@JvmStatic
fun getWritableStorage(context: Context) =
suspend fun getWritableStorage(context: Context) = withContext(Dispatchers.IO) {
validate(externalMediaFilesDirsDevices(context), true)
}
@JvmStatic
fun getReadableStorage(context: Context): List<StorageDevice> {

View File

@ -55,7 +55,7 @@
android:layout_marginTop="8dp"
android:indeterminate="false"
android:max="100"
android:progress="50"
android:progress="0"
android:progressDrawable="@drawable/progress_bar_state"
app:layout_constraintEnd_toEndOf="parent"
app:layout_constraintStart_toEndOf="@id/radioButton"