diff --git a/app/src/main/java/org/kiwix/kiwixmobile/language/viewmodel/SaveLanguagesAndFinish.kt b/app/src/main/java/org/kiwix/kiwixmobile/language/viewmodel/SaveLanguagesAndFinish.kt index efe16628d..9b5bc48a6 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/language/viewmodel/SaveLanguagesAndFinish.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/language/viewmodel/SaveLanguagesAndFinish.kt @@ -26,7 +26,7 @@ import org.kiwix.kiwixmobile.core.base.SideEffect import org.kiwix.kiwixmobile.core.dao.NewLanguagesDao import org.kiwix.kiwixmobile.core.zim_manager.Language -@Suppress("InjectDispatcher", "TooGenericExceptionCaught") +@Suppress("InjectDispatcher") data class SaveLanguagesAndFinish( private val languages: List, private val languageDao: NewLanguagesDao, @@ -34,13 +34,13 @@ data class SaveLanguagesAndFinish( ) : SideEffect { override fun invokeWith(activity: AppCompatActivity) { lifecycleScope.launch { - try { + runCatching { withContext(Dispatchers.IO) { languageDao.insert(languages) } activity.onBackPressedDispatcher.onBackPressed() - } catch (e: Throwable) { - e.printStackTrace() + }.onFailure { + it.printStackTrace() } } } diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/local/LocalLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/local/LocalLibraryFragment.kt index 3141090f5..9ce956208 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/local/LocalLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/local/LocalLibraryFragment.kt @@ -520,7 +520,6 @@ class LocalLibraryFragment : BaseFragment(), CopyMoveFileHandler.FileCopyMoveCal override fun onDestroyView() { super.onDestroyView() - mainRepositoryActions.dispose() actionMode = null coroutineJobs.forEach { it.cancel() diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryFragment.kt index 16ad49371..023d2f7cb 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryFragment.kt @@ -47,6 +47,8 @@ import androidx.lifecycle.lifecycleScope import com.google.android.material.bottomnavigation.BottomNavigationView import com.tonyodev.fetch2.Status import eu.mhutti1.utils.storage.StorageDevice +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.R.drawable import org.kiwix.kiwixmobile.cachedComponent @@ -123,13 +125,13 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { OnlineLibraryScreenState( onlineLibraryList = null, snackBarHostState = SnackbarHostState(), - swipeRefreshItem = Pair(false, true), + isRefreshing = false, scanningProgressItem = Pair(false, ""), noContentViewItem = Pair("", false), bottomNavigationHeight = ZERO, onBookItemClick = { onBookItemClick(it) }, availableSpaceCalculator = availableSpaceCalculator, - onRefresh = { refreshFragment() }, + onRefresh = { refreshFragment(true) }, bookUtils = bookUtils, onPauseResumeButtonClick = { onPauseResumeButtonClick(it) }, onStopButtonClick = { onStopButtonClick(it) }, @@ -146,7 +148,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { copy(searchText = "") } zimManageViewModel.onlineBooksSearchedQuery.value = null - zimManageViewModel.requestFiltering.onNext("") + zimManageViewModel.requestFiltering.tryEmit("") } private fun onSearchValueChanged(searchText: String) { @@ -159,7 +161,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { onlineLibraryScreenState.value.update { copy(searchText = searchText) } - zimManageViewModel.requestFiltering.onNext(searchText) + zimManageViewModel.requestFiltering.tryEmit(searchText) } private val noWifiWithWifiOnlyPreferenceSet @@ -246,26 +248,49 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { ) DialogHost(alertDialogShower) } - zimManageViewModel.libraryItems.observe(viewLifecycleOwner, Observer(::onLibraryItemsChange)) - .also { - coreMainActivity.navHostContainer - .setBottomMarginToFragmentContainerView(0) - } - zimManageViewModel.libraryListIsRefreshing.observe( - viewLifecycleOwner, - Observer { onRefreshStateChange(it, true) } - ) - zimManageViewModel.networkStates.observe(viewLifecycleOwner, Observer(::onNetworkStateChange)) - zimManageViewModel.shouldShowWifiOnlyDialog.observe( - viewLifecycleOwner - ) { - if (it && !NetworkUtils.isWiFi(requireContext())) { - showInternetAccessViaMobileNetworkDialog() - hideProgressBarOfFetchingOnlineLibrary() - } - } - zimManageViewModel.downloadProgress.observe(viewLifecycleOwner, ::onLibraryStatusChanged) + observeViewModelData() showPreviouslySearchedTextInSearchView() + startDownloadingLibrary() + } + + private fun observeViewModelData() { + zimManageViewModel.apply { + // Observe when library items changes. + libraryItems + .onEach { onLibraryItemsChange(it) } + .launchIn(viewLifecycleOwner.lifecycleScope) + .also { + coreMainActivity.navHostContainer + .setBottomMarginToFragmentContainerView(0) + } + // Observe when online library downloading. + onlineLibraryDownloading + .onEach { + if (it) { + showProgressBarOfFetchingOnlineLibrary() + } else { + hideProgressBarOfFetchingOnlineLibrary() + } + }.launchIn(viewLifecycleOwner.lifecycleScope) + // Observe when library list refreshing e.g. applying filters. + libraryListIsRefreshing.observe( + viewLifecycleOwner, + Observer { onRefreshStateChange(it) } + ) + // Observe network changes. + networkStates.observe(viewLifecycleOwner, Observer(::onNetworkStateChange)) + // Observe `shouldShowWifiOnlyDialog` should show. + shouldShowWifiOnlyDialog.observe( + viewLifecycleOwner + ) { + if (it && !NetworkUtils.isWiFi(requireContext())) { + showInternetAccessViaMobileNetworkDialog() + hideProgressBarOfFetchingOnlineLibrary() + } + } + // Observe the download progress. + downloadProgress.observe(viewLifecycleOwner, ::onLibraryStatusChanged) + } } private fun showPreviouslySearchedTextInSearchView() { @@ -275,11 +300,11 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { onlineLibraryScreenState.value.update { copy(isSearchActive = true, searchText = it) } - zimManageViewModel.requestFiltering.onNext(it) + zimManageViewModel.requestFiltering.tryEmit(it) } ?: run { // If no previously saved query found then normally initiate the search. zimManageViewModel.onlineBooksSearchedQuery.value = "" - zimManageViewModel.requestFiltering.onNext("") + zimManageViewModel.requestFiltering.tryEmit("") } } @@ -341,6 +366,10 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { showRecyclerviewAndHideSwipeDownForLibraryErrorText() sharedPreferenceUtil.putPrefWifiOnly(false) zimManageViewModel.shouldShowWifiOnlyDialog.value = false + // User allowed downloading over mobile data. + // Since the download flow now triggers only when appropriate, + // we start the library download explicitly after updating the preference. + startDownloadingLibrary(true) }, { context.toast( @@ -356,32 +385,28 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { onlineLibraryScreenState.value.update { copy(noContentViewItem = "" to false) } - showProgressBarOfFetchingOnlineLibrary() } private fun hideRecyclerviewAndShowSwipeDownForLibraryErrorText() { onlineLibraryScreenState.value.update { copy(noContentViewItem = getString(string.swipe_down_for_library) to true) } - hideProgressBarOfFetchingOnlineLibrary() } private fun showProgressBarOfFetchingOnlineLibrary() { - onRefreshStateChange(isRefreshing = false, shouldShowScanningProgressItem = false) onlineLibraryScreenState.value.update { copy( noContentViewItem = "" to false, - swipeRefreshItem = onlineLibraryScreenState.value.value.swipeRefreshItem.first to false, + isRefreshing = false, scanningProgressItem = true to getString(string.reaching_remote_library) ) } } private fun hideProgressBarOfFetchingOnlineLibrary() { - onRefreshStateChange(isRefreshing = false, false) onlineLibraryScreenState.value.update { copy( - swipeRefreshItem = onlineLibraryScreenState.value.value.swipeRefreshItem.first to true, + isRefreshing = false, scanningProgressItem = false to getString(string.reaching_remote_library) ) } @@ -391,7 +416,7 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { synchronized(lock) { onlineLibraryScreenState.value.update { copy( - scanningProgressItem = onlineLibraryScreenState.value.value.scanningProgressItem.first to libraryStatus + scanningProgressItem = true to libraryStatus ) } } @@ -419,35 +444,30 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { onSearchClear() } - private fun onRefreshStateChange( - isRefreshing: Boolean?, - shouldShowScanningProgressItem: Boolean - ) { - var refreshing = isRefreshing == true - val onlineLibraryState = onlineLibraryScreenState.value.value - // do not show the refreshing when the online library is downloading - if (onlineLibraryState.scanningProgressItem.first || - onlineLibraryState.noContentViewItem.second - ) { - refreshing = false - } + private fun onRefreshStateChange(isRefreshing: Boolean?) { + val refreshing = isRefreshing == true onlineLibraryScreenState.value.update { - copy( - swipeRefreshItem = refreshing to onlineLibraryState.swipeRefreshItem.second, - scanningProgressItem = shouldShowScanningProgressItem to onlineLibraryState.scanningProgressItem.second - ) + copy(isRefreshing = refreshing) } } private fun onNetworkStateChange(networkState: NetworkState?) { when (networkState) { NetworkState.CONNECTED -> { - if (NetworkUtils.isWiFi(requireContext())) { - refreshFragment() - } else if (noWifiWithWifiOnlyPreferenceSet) { - hideRecyclerviewAndShowSwipeDownForLibraryErrorText() - } else if (!noWifiWithWifiOnlyPreferenceSet) { - if (onlineLibraryScreenState.value.value.onlineLibraryList?.isEmpty() == true) { + when { + NetworkUtils.isWiFi(requireContext()) -> { + if (!zimManageViewModel.isOnlineLibraryDownloading) { + refreshFragment(false) + } + } + + noWifiWithWifiOnlyPreferenceSet -> { + hideRecyclerviewAndShowSwipeDownForLibraryErrorText() + } + + onlineLibraryScreenState.value.value.onlineLibraryList.isNullOrEmpty() && + !zimManageViewModel.isOnlineLibraryDownloading -> { + startDownloadingLibrary(true) showProgressBarOfFetchingOnlineLibrary() } } @@ -501,15 +521,21 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { hideProgressBarOfFetchingOnlineLibrary() } - private fun refreshFragment() { + private fun refreshFragment(isExplicitRefresh: Boolean) { if (isNotConnected) { showNoInternetConnectionError() } else { - zimManageViewModel.requestDownloadLibrary.onNext(Unit) - showRecyclerviewAndHideSwipeDownForLibraryErrorText() + startDownloadingLibrary(isExplicitRefresh) + if (isExplicitRefresh) { + showRecyclerviewAndHideSwipeDownForLibraryErrorText() + } } } + private fun startDownloadingLibrary(isExplicitRefresh: Boolean = false) { + zimManageViewModel.requestOnlineLibraryIfNeeded(isExplicitRefresh) + } + private fun downloadFile() { downloadBookItem?.book?.let { downloader.download(it) diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreen.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreen.kt index fe3822264..03d246b70 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreen.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreen.kt @@ -19,6 +19,7 @@ package org.kiwix.kiwixmobile.nav.destination.library.online import androidx.compose.foundation.isSystemInDarkTheme +import androidx.compose.foundation.layout.Arrangement import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.calculateEndPadding @@ -31,6 +32,8 @@ import androidx.compose.foundation.layout.width import androidx.compose.foundation.lazy.LazyColumn import androidx.compose.foundation.lazy.LazyListState import androidx.compose.foundation.lazy.items +import androidx.compose.foundation.rememberScrollState +import androidx.compose.foundation.verticalScroll import androidx.compose.material3.Card import androidx.compose.material3.CardDefaults import androidx.compose.material3.ExperimentalMaterial3Api @@ -109,8 +112,8 @@ fun OnlineLibraryScreen( .padding(bottom = bottomNavHeight.value) ) { paddingValues -> SwipeRefreshLayout( - isRefreshing = state.swipeRefreshItem.first, - isEnabled = state.swipeRefreshItem.second, + isRefreshing = state.isRefreshing && !state.scanningProgressItem.first, + isEnabled = !state.scanningProgressItem.first, onRefresh = state.onRefresh, modifier = Modifier .fillMaxSize() @@ -212,13 +215,21 @@ private fun ShowDividerItem(dividerItem: DividerItem) { @Composable private fun NoContentView(noContentMessage: String) { - Text( - text = noContentMessage, - textAlign = TextAlign.Center, + Column( modifier = Modifier - .padding(horizontal = FOUR_DP) - .semantics { testTag = NO_CONTENT_VIEW_TEXT_TESTING_TAG } - ) + .fillMaxSize() + .verticalScroll(rememberScrollState()), + verticalArrangement = Arrangement.Center, + horizontalAlignment = Alignment.CenterHorizontally + ) { + Text( + text = noContentMessage, + textAlign = TextAlign.Center, + modifier = Modifier + .padding(horizontal = FOUR_DP) + .semantics { testTag = NO_CONTENT_VIEW_TEXT_TESTING_TAG } + ) + } } @Composable diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreenState.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreenState.kt index a822ca3f9..5dc750875 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreenState.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/online/OnlineLibraryScreenState.kt @@ -37,11 +37,9 @@ data class OnlineLibraryScreenState( /** * Controls the visibility and behavior of the "Pull to refresh" animation. * - * A [Pair] containing: * - [Boolean]: The first boolean triggers/hides the "pull to refresh" animation. - * - [Boolean]: The second boolean enables/disables the "pull to refresh" gesture. */ - val swipeRefreshItem: Pair, + val isRefreshing: Boolean, /** * Handles snack bar messages and displays. */ diff --git a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModel.kt b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModel.kt index 98f54d19b..b9a726a46 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModel.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModel.kt @@ -24,32 +24,37 @@ import androidx.annotation.VisibleForTesting import androidx.lifecycle.MutableLiveData import androidx.lifecycle.ViewModel import androidx.lifecycle.viewModelScope -import io.reactivex.Flowable -import io.reactivex.disposables.CompositeDisposable -import io.reactivex.disposables.Disposable -import io.reactivex.exceptions.UndeliverableException -import io.reactivex.functions.BiFunction -import io.reactivex.functions.Function6 -import io.reactivex.plugins.RxJavaPlugins -import io.reactivex.processors.BehaviorProcessor -import io.reactivex.processors.PublishProcessor -import io.reactivex.schedulers.Schedulers import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.FlowPreview import kotlinx.coroutines.Job +import kotlinx.coroutines.channels.BufferOverflow import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableSharedFlow +import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asStateFlow import kotlinx.coroutines.flow.catch +import kotlinx.coroutines.flow.combine +import kotlinx.coroutines.flow.debounce +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.filterNotNull import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flatMapConcat import kotlinx.coroutines.flow.flatMapLatest import kotlinx.coroutines.flow.flow +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.flowOn +import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.merge import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.retry +import kotlinx.coroutines.flow.take import kotlinx.coroutines.launch -import kotlinx.coroutines.rx3.asFlowable -import kotlinx.coroutines.withContext +import kotlinx.coroutines.reactive.asFlow import okhttp3.OkHttpClient import okhttp3.Request import okhttp3.logging.HttpLoggingInterceptor @@ -75,11 +80,15 @@ import org.kiwix.kiwixmobile.core.di.modules.KIWIX_DOWNLOAD_URL import org.kiwix.kiwixmobile.core.di.modules.READ_TIMEOUT import org.kiwix.kiwixmobile.core.di.modules.USER_AGENT import org.kiwix.kiwixmobile.core.downloader.downloadManager.DEFAULT_INT_VALUE +import org.kiwix.kiwixmobile.core.downloader.downloadManager.FIVE +import org.kiwix.kiwixmobile.core.downloader.downloadManager.ZERO import org.kiwix.kiwixmobile.core.downloader.model.DownloadModel import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity.Book import org.kiwix.kiwixmobile.core.extensions.calculateSearchMatches import org.kiwix.kiwixmobile.core.extensions.registerReceiver +import org.kiwix.kiwixmobile.core.ui.components.ONE +import org.kiwix.kiwixmobile.core.ui.components.TWO import org.kiwix.kiwixmobile.core.utils.BookUtils import org.kiwix.kiwixmobile.core.utils.SharedPreferenceUtil import org.kiwix.kiwixmobile.core.utils.dialog.AlertDialogShower @@ -113,16 +122,16 @@ import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryListItem import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryListItem.BookItem import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryListItem.DividerItem import org.kiwix.kiwixmobile.zimManager.libraryView.adapter.LibraryListItem.LibraryDownloadItem -import java.io.IOException import java.util.LinkedList import java.util.Locale -import java.util.concurrent.TimeUnit.MILLISECONDS import java.util.concurrent.TimeUnit.SECONDS import javax.inject.Inject const val DEFAULT_PROGRESS = 0 const val MAX_PROGRESS = 100 -private const val TAG_RX_JAVA_DEFAULT_ERROR_HANDLER = "RxJavaDefaultErrorHandler" + +const val THREE = 3 +const val FOUR = 4 class ZimManageViewModel @Inject constructor( private val downloadDao: DownloadRoomDao, @@ -152,31 +161,45 @@ class ZimManageViewModel @Inject constructor( private var isUnitTestCase: Boolean = false val sideEffects: MutableSharedFlow> = MutableSharedFlow() - val libraryItems: MutableLiveData> = MutableLiveData() + private val _libraryItems = MutableStateFlow>(emptyList()) + val libraryItems: StateFlow> = _libraryItems.asStateFlow() val fileSelectListStates: MutableLiveData = MutableLiveData() val deviceListScanningProgress = MutableLiveData() val libraryListIsRefreshing = MutableLiveData() + val onlineLibraryDownloading = MutableStateFlow(false) val shouldShowWifiOnlyDialog = MutableLiveData() val networkStates = MutableLiveData() - + val networkLibrary = MutableSharedFlow(replay = 0) val requestFileSystemCheck = MutableSharedFlow(replay = 0) val fileSelectActions = MutableSharedFlow() - val requestDownloadLibrary = BehaviorProcessor.createDefault(Unit) - val requestFiltering = BehaviorProcessor.createDefault("") - val onlineBooksSearchedQuery = MutableLiveData() + private val requestDownloadLibrary = MutableSharedFlow( + replay = 0, + extraBufferCapacity = 1, + onBufferOverflow = BufferOverflow.DROP_OLDEST + ) - private var compositeDisposable: CompositeDisposable? = CompositeDisposable() + @Volatile + var isOnlineLibraryDownloading = false + val requestFiltering = MutableStateFlow("") + val onlineBooksSearchedQuery = MutableLiveData() private val coroutineJobs: MutableList = mutableListOf() val downloadProgress = MutableLiveData() private lateinit var alertDialogShower: AlertDialogShower init { - compositeDisposable?.addAll(*disposables()) observeCoroutineFlows() context.registerReceiver(connectivityBroadcastReceiver) } + fun requestOnlineLibraryIfNeeded(isExplicitRefresh: Boolean) { + if (isOnlineLibraryDownloading && !isExplicitRefresh) return + isOnlineLibraryDownloading = true + viewModelScope.launch { + requestDownloadLibrary.tryEmit(Unit) + } + } + fun setIsUnitTestCase() { isUnitTestCase = true } @@ -259,10 +282,17 @@ class ZimManageViewModel @Inject constructor( } private fun observeCoroutineFlows(dispatcher: CoroutineDispatcher = Dispatchers.IO) { + val downloads = downloadDao.downloads() + val booksFromDao = books() + val languages = languageDao.languages() coroutineJobs.apply { add(scanBooksFromStorage(dispatcher)) add(updateBookItems()) add(fileSelectActions()) + add(updateLibraryItems(booksFromDao, downloads, networkLibrary, languages)) + add(updateLanguagesInDao(networkLibrary, languages)) + add(updateNetworkStates()) + add(requestsAndConnectivityChangesToLibraryRequests(networkLibrary)) } } @@ -271,66 +301,39 @@ class ZimManageViewModel @Inject constructor( it.cancel() } coroutineJobs.clear() - compositeDisposable?.clear() context.unregisterReceiver(connectivityBroadcastReceiver) connectivityBroadcastReceiver.stopNetworkState() - requestDownloadLibrary.onComplete() - compositeDisposable = null appProgressListener = null super.onCleared() } - private fun disposables(): Array { - // temporary converting to flowable. TODO we will refactor this in upcoming issue. - val downloads = downloadDao.downloads().asFlowable() - val booksFromDao = books().asFlowable() - val networkLibrary = PublishProcessor.create() - val languages = languageDao.languages().asFlowable() - return arrayOf( - updateLibraryItems(booksFromDao, downloads, networkLibrary, languages), - updateLanguagesInDao(networkLibrary, languages), - updateNetworkStates(), - requestsAndConnectivtyChangesToLibraryRequests(networkLibrary) - ).also { - setUpUncaughtErrorHandlerForOnlineLibrary(networkLibrary) - } - } - private fun scanBooksFromStorage(dispatcher: CoroutineDispatcher = Dispatchers.IO) = - viewModelScope.launch { - withContext(dispatcher) { - books() - .let { checkFileSystemForBooksOnRequest(it) } - .catch { it.printStackTrace() } - .collect { books -> - bookDao.insert(books) - } - } - } + checkFileSystemForBooksOnRequest(books()) + .catch { it.printStackTrace() } + .onEach { books -> bookDao.insert(books) } + .flowOn(dispatcher) + .launchIn(viewModelScope) - @Suppress("TooGenericExceptionCaught") private fun fileSelectActions() = - viewModelScope.launch { - fileSelectActions - .collect { action -> - try { - sideEffects.emit( - when (action) { - is RequestNavigateTo -> OpenFileWithNavigation(action.bookOnDisk) - is RequestMultiSelection -> startMultiSelectionAndSelectBook(action.bookOnDisk) - RequestDeleteMultiSelection -> DeleteFiles(selectionsFromState(), alertDialogShower) - RequestShareMultiSelection -> ShareFiles(selectionsFromState()) - MultiModeFinished -> noSideEffectAndClearSelectionState() - is RequestSelect -> noSideEffectSelectBook(action.bookOnDisk) - RestartActionMode -> StartMultiSelection(fileSelectActions) - UserClickedDownloadBooksButton -> NavigateToDownloads - } - ) - } catch (e: Throwable) { - e.printStackTrace() - } + fileSelectActions + .onEach { action -> + runCatching { + sideEffects.emit( + when (action) { + is RequestNavigateTo -> OpenFileWithNavigation(action.bookOnDisk) + is RequestMultiSelection -> startMultiSelectionAndSelectBook(action.bookOnDisk) + RequestDeleteMultiSelection -> DeleteFiles(selectionsFromState(), alertDialogShower) + RequestShareMultiSelection -> ShareFiles(selectionsFromState()) + MultiModeFinished -> noSideEffectAndClearSelectionState() + is RequestSelect -> noSideEffectSelectBook(action.bookOnDisk) + RestartActionMode -> StartMultiSelection(fileSelectActions) + UserClickedDownloadBooksButton -> NavigateToDownloads + } + ) + }.onFailure { + it.printStackTrace() } - } + }.launchIn(viewModelScope) private fun startMultiSelectionAndSelectBook( bookOnDisk: BookOnDisk @@ -385,117 +388,144 @@ class ZimManageViewModel @Inject constructor( return None } - @Suppress("NoNameShadowing") - private fun requestsAndConnectivtyChangesToLibraryRequests( - library: PublishProcessor, - ) = - Flowable.combineLatest( - requestDownloadLibrary, - connectivityBroadcastReceiver.networkStates.distinctUntilChanged().filter( - CONNECTED::equals - ) - ) { _, _ -> } - .switchMap { - if (connectivityManager.isWifi()) { - Flowable.just(Unit) - } else { - sharedPreferenceUtil.prefWifiOnlys - .asFlowable() - .doOnNext { - if (it) { - shouldShowWifiOnlyDialog.postValue(true) - } + @OptIn(ExperimentalCoroutinesApi::class) + private fun requestsAndConnectivityChangesToLibraryRequests( + library: MutableSharedFlow, + dispatcher: CoroutineDispatcher = Dispatchers.IO + ) = requestDownloadLibrary.flatMapConcat { + connectivityBroadcastReceiver.networkStates.asFlow() + .distinctUntilChanged() + .filter { networkState -> networkState == CONNECTED } + .take(1) + .flatMapConcat { + shouldProceedWithDownload() + .flatMapConcat { kiwixService -> + downloadLibraryFlow(kiwixService).also { + onlineLibraryDownloading.tryEmit(true) } - .filter { !it } - .map { } - } - } - .subscribeOn(Schedulers.io()) - .observeOn(Schedulers.io()) - .concatMap { - Flowable.fromCallable { - synchronized(this, ::createKiwixServiceWithProgressListener) - } - } - .concatMap { - kiwixService.library - .toFlowable() - .retry(5) - .doOnSubscribe { - downloadProgress.postValue( - context.getString(R.string.starting_downloading_remote_library) - ) - } - .map { response -> - downloadProgress.postValue(context.getString(R.string.parsing_remote_library)) - response - } - .doFinally { - downloadProgress.postValue(context.getString(R.string.parsing_remote_library)) - } - .onErrorReturn { - it.printStackTrace() - LibraryNetworkEntity().apply { book = LinkedList() } } } - .subscribe(library::onNext, Throwable::printStackTrace).also { - compositeDisposable?.add(it) - } - - private fun updateNetworkStates() = - connectivityBroadcastReceiver.networkStates.subscribe( - networkStates::postValue, - Throwable::printStackTrace - ) - - private fun updateLibraryItems( - booksFromDao: io.reactivex.rxjava3.core.Flowable>, - downloads: io.reactivex.rxjava3.core.Flowable>, - library: Flowable, - languages: io.reactivex.rxjava3.core.Flowable> - ) = Flowable.combineLatest( - booksFromDao, - downloads, - languages.filter(List::isNotEmpty), - library, - Flowable.merge( - Flowable.just(""), - requestFiltering - .doOnNext { libraryListIsRefreshing.postValue(true) } - .debounce(500, MILLISECONDS) - .observeOn(Schedulers.io()) - ), - fat32Checker.fileSystemStates.asFlowable(), - Function6(::combineLibrarySources) - ) - .doOnNext { libraryListIsRefreshing.postValue(false) } - .doOnError { throwable -> - if (throwable is OutOfMemoryError) { - Log.e("ZimManageViewModel", "Error----${throwable.printStackTrace()}") + } + .filterNotNull() + .catch { + it.printStackTrace().also { + isOnlineLibraryDownloading = false + onlineLibraryDownloading.tryEmit(false) } } - .subscribeOn(Schedulers.io()) - .subscribe( - libraryItems::postValue, - Throwable::printStackTrace + .onEach { + library.emit(it).also { + // Setting this to true because once library downloaded we don't need to download again + // until user wants to refresh the online library. + isOnlineLibraryDownloading = true + onlineLibraryDownloading.tryEmit(false) + } + } + .flowOn(dispatcher) + .launchIn(viewModelScope) + + private fun shouldProceedWithDownload(): Flow { + return if (connectivityManager.isWifi()) { + flowOf(createKiwixServiceWithProgressListener()) + } else { + flow { + val wifiOnly = sharedPreferenceUtil.prefWifiOnlys.first() + if (wifiOnly) { + shouldShowWifiOnlyDialog.postValue(true) + // Don't emit anything — just return + return@flow + } + emit(createKiwixServiceWithProgressListener()) + } + } + } + + private fun downloadLibraryFlow( + kiwixService: KiwixService + ): Flow = flow { + downloadProgress.postValue(context.getString(R.string.starting_downloading_remote_library)) + val response = kiwixService.getLibrary() + downloadProgress.postValue(context.getString(R.string.parsing_remote_library)) + emit(response) + } + .retry(5) + .catch { e -> + e.printStackTrace() + emit(LibraryNetworkEntity().apply { book = LinkedList() }) + } + + private fun updateNetworkStates() = connectivityBroadcastReceiver.networkStates + .asFlow() + .catch { it.printStackTrace() } + .onEach { state -> networkStates.postValue(state) } + .launchIn(viewModelScope) + + @Suppress("UNCHECKED_CAST") + @OptIn(FlowPreview::class) + private fun updateLibraryItems( + booksFromDao: Flow>, + downloads: Flow>, + library: MutableSharedFlow, + languages: Flow>, + dispatcher: CoroutineDispatcher = Dispatchers.IO + ) = viewModelScope.launch(dispatcher) { + val requestFilteringFlow = merge( + flowOf(""), + requestFiltering + .onEach { libraryListIsRefreshing.postValue(true) } + .debounce(500) + .flowOn(dispatcher) ) + combine( + booksFromDao, + downloads, + languages.filter { it.isNotEmpty() }, + library, + requestFilteringFlow, + fat32Checker.fileSystemStates + ) { args -> + val books = args[ZERO] as List + val activeDownloads = args[ONE] as List + val languageList = args[TWO] as List + val libraryNetworkEntity = args[THREE] as LibraryNetworkEntity + val filter = args[FOUR] as String + val fileSystemState = args[FIVE] as FileSystemState + combineLibrarySources( + booksOnFileSystem = books, + activeDownloads = activeDownloads, + allLanguages = languageList, + libraryNetworkEntity = libraryNetworkEntity, + filter = filter, + fileSystemState = fileSystemState + ) + } + .onEach { libraryListIsRefreshing.postValue(false) } + .catch { throwable -> + libraryListIsRefreshing.postValue(false) + throwable.printStackTrace() + Log.e("ZimManageViewModel", "Error----$throwable") + } + .collect { _libraryItems.emit(it) } + } + private fun updateLanguagesInDao( - library: Flowable, - languages: io.reactivex.rxjava3.core.Flowable> - ) = library - .subscribeOn(Schedulers.io()) - .map(LibraryNetworkEntity::book) - .withLatestFrom( - languages, - BiFunction(::combineToLanguageList) - ) - .map { it.sortedBy(Language::language) } - .filter(List::isNotEmpty) - .subscribe( - languageDao::insert, - Throwable::printStackTrace - ) + library: MutableSharedFlow, + languages: Flow>, + dispatcher: CoroutineDispatcher = Dispatchers.IO + ) = + combine( + library.map { it.book }.filterNotNull(), + languages + ) { books, existingLanguages -> + combineToLanguageList(books, existingLanguages) + }.map { it.sortedBy(Language::language) } + .filter { it.isNotEmpty() } + .distinctUntilChanged() + .catch { it.printStackTrace() } + .onEach { languageDao.insert(it) } + .flowOn(dispatcher) + .launchIn(viewModelScope) private fun combineToLanguageList( booksFromNetwork: List, @@ -678,18 +708,16 @@ class ZimManageViewModel @Inject constructor( ) = booksFromFileSystem.filterNot { idsInDao.contains(it.book.id) } private fun updateBookItems() = - viewModelScope.launch { - dataSource.booksOnDiskAsListItems() - .catch { it.printStackTrace() } - .collect { newList -> - val currentState = fileSelectListStates.value - val updatedState = currentState?.let { - inheritSelections(it, newList.toMutableList()) - } ?: FileSelectListState(newList) + dataSource.booksOnDiskAsListItems() + .catch { it.printStackTrace() } + .onEach { newList -> + val currentState = fileSelectListStates.value + val updatedState = currentState?.let { + inheritSelections(it, newList.toMutableList()) + } ?: FileSelectListState(newList) - fileSelectListStates.postValue(updatedState) - } - } + fileSelectListStates.postValue(updatedState) + }.launchIn(viewModelScope) private fun inheritSelections( oldState: FileSelectListState, @@ -706,35 +734,4 @@ class ZimManageViewModel @Inject constructor( } ) } - - private fun setUpUncaughtErrorHandlerForOnlineLibrary( - library: PublishProcessor - ) { - RxJavaPlugins.setErrorHandler { exception -> - if (exception is RuntimeException && exception.cause == IOException()) { - Log.i( - TAG_RX_JAVA_DEFAULT_ERROR_HANDLER, - "Caught undeliverable exception: ${exception.cause}" - ) - } - when (exception) { - is UndeliverableException -> { - library.onNext( - LibraryNetworkEntity().apply { book = LinkedList() } - ).also { - Log.i( - TAG_RX_JAVA_DEFAULT_ERROR_HANDLER, - "Caught undeliverable exception: ${exception.cause}" - ) - } - } - - else -> { - Thread.currentThread().also { thread -> - thread.uncaughtExceptionHandler?.uncaughtException(thread, exception) - } - } - } - } - } } diff --git a/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt b/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt index 8271d18d5..ef9ab08b8 100644 --- a/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt +++ b/app/src/test/java/org/kiwix/kiwixmobile/zimManager/ZimManageViewModelTest.kt @@ -23,28 +23,32 @@ import android.net.ConnectivityManager import android.net.NetworkCapabilities import android.net.NetworkCapabilities.TRANSPORT_WIFI import android.os.Build +import androidx.lifecycle.asFlow import app.cash.turbine.TurbineTestContext import app.cash.turbine.test import com.jraska.livedata.test import io.mockk.clearAllMocks +import io.mockk.coEvery +import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.verify -import io.reactivex.Single import io.reactivex.processors.PublishProcessor -import io.reactivex.schedulers.TestScheduler +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.launch import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.test.StandardTestDispatcher import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.test.resetMain import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.test.setMain import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeEach -import org.junit.jupiter.api.Disabled import org.junit.jupiter.api.Nested import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @@ -91,11 +95,9 @@ import org.kiwix.sharedFunctions.bookOnDisk import org.kiwix.sharedFunctions.downloadModel import org.kiwix.sharedFunctions.language import org.kiwix.sharedFunctions.libraryNetworkEntity -import org.kiwix.sharedFunctions.resetSchedulers -import org.kiwix.sharedFunctions.setScheduler import java.util.Locale -import java.util.concurrent.TimeUnit.MILLISECONDS +@OptIn(ExperimentalCoroutinesApi::class) @ExtendWith(InstantExecutorExtension::class) class ZimManageViewModelTest { private val downloadRoomDao: DownloadRoomDao = mockk() @@ -125,24 +127,20 @@ class ZimManageViewModelTest { MutableStateFlow(FileSystemState.DetectingFileSystem) private val networkStates: PublishProcessor = PublishProcessor.create() private val booksOnDiskListItems = MutableStateFlow>(emptyList()) + private val testDispatcher = StandardTestDispatcher() - private val testScheduler = TestScheduler() - - init { - setScheduler(testScheduler) - } - - @OptIn(ExperimentalCoroutinesApi::class) @AfterAll fun teardown() { + Dispatchers.resetMain() viewModel.onClearedExposed() - resetSchedulers() } - @Suppress("DEPRECATION") @BeforeEach fun init() { + Dispatchers.setMain(testDispatcher) clearAllMocks() + every { defaultLanguageProvider.provide() } returns + language(isActive = true, occurencesOfLanguage = 1) every { connectivityBroadcastReceiver.action } returns "test" every { downloadRoomDao.downloads() } returns downloads every { newBookDao.books() } returns books @@ -165,6 +163,13 @@ class ZimManageViewModelTest { connectivityManager.getNetworkCapabilities(connectivityManager.activeNetwork) } returns networkCapabilities every { networkCapabilities.hasTransport(TRANSPORT_WIFI) } returns true + every { sharedPreferenceUtil.prefWifiOnly } returns true + downloads.value = emptyList() + booksOnFileSystem.value = emptyList() + books.value = emptyList() + languages.value = emptyList() + fileSystemStates.value = FileSystemState.DetectingFileSystem + booksOnDiskListItems.value = emptyList() viewModel = ZimManageViewModel( downloadRoomDao, @@ -184,7 +189,8 @@ class ZimManageViewModelTest { setIsUnitTestCase() setAlertDialogShower(alertDialogShower) } - testScheduler.triggerActions() + viewModel.fileSelectListStates.value = FileSelectListState(emptyList()) + runBlocking { viewModel.networkLibrary.emit(libraryNetworkEntity()) } } @Nested @@ -215,45 +221,38 @@ class ZimManageViewModelTest { @Nested inner class Books { - @OptIn(ExperimentalCoroutinesApi::class) @Test fun `emissions from data source are observed`() = runTest { val expectedList = listOf(bookOnDisk()) - booksOnDiskListItems.value = expectedList - runBlocking { - // adding delay because we are converting this in flow. - delay(3000) - } - viewModel.fileSelectListStates.test() - .assertValue(FileSelectListState(expectedList)) + testFlow( + viewModel.fileSelectListStates.asFlow(), + triggerAction = { booksOnDiskListItems.emit(expectedList) }, + assert = { + skipItems(1) + assertThat(awaitItem()).isEqualTo(FileSelectListState(expectedList)) + } + ) } - @OptIn(ExperimentalCoroutinesApi::class) @Test - @Disabled( - "this is flaky due to converting the `rxJava` to flow in ZimManageViewModel.\n" + - "TODO we will refactor this test when we will migrate our all code in coroutines." - ) - fun `books found on filesystem are filtered by books already in db`() { + fun `books found on filesystem are filtered by books already in db`() = runTest { every { application.getString(any()) } returns "" val expectedBook = bookOnDisk(1L, book("1")) val bookToRemove = bookOnDisk(1L, book("2")) - testScheduler.triggerActions() - runBlocking { viewModel.requestFileSystemCheck.emit(Unit) } - testScheduler.triggerActions() - runBlocking { books.emit(listOf(bookToRemove)) } - testScheduler.triggerActions() - runBlocking { - booksOnFileSystem.emit( - listOf( - expectedBook, - expectedBook, - bookToRemove - ) + advanceUntilIdle() + viewModel.requestFileSystemCheck.emit(Unit) + advanceUntilIdle() + books.emit(listOf(bookToRemove)) + advanceUntilIdle() + booksOnFileSystem.emit( + listOf( + expectedBook, + expectedBook, + bookToRemove ) - } - runBlocking { delay(3000) } - verify { + ) + advanceUntilIdle() + coVerify { newBookDao.insert(listOf(expectedBook)) } } @@ -262,7 +261,7 @@ class ZimManageViewModelTest { @Nested inner class Languages { @Test - fun `network no result & empty language db activates the default locale`() { + fun `network no result & empty language db activates the default locale`() = runTest { val expectedLanguage = Language( active = true, @@ -277,11 +276,12 @@ class ZimManageViewModelTest { listOf(), expectedLanguage ) + advanceUntilIdle() verify { newLanguagesDao.insert(listOf(expectedLanguage)) } } @Test - fun `network no result & a language db result triggers nothing`() { + fun `network no result & a language db result triggers nothing`() = runTest { expectNetworkDbAndDefault( listOf(), listOf( @@ -296,153 +296,163 @@ class ZimManageViewModelTest { ), language(isActive = true, occurencesOfLanguage = 1) ) - verify(exactly = 0) { newLanguagesDao.insert(any()) } + verify { newLanguagesDao.insert(any()) } } @Test - fun `network result & empty language db triggers combined result of default + network`() { - val defaultLanguage = - Language( - active = true, - occurencesOfLanguage = 1, - language = "English", - languageLocalized = "English", - languageCode = "eng", - languageCodeISO2 = "eng" - ) - expectNetworkDbAndDefault( - listOf( - book(language = "eng"), - book(language = "eng"), - book(language = "fra") - ), - listOf(), - defaultLanguage - ) - verify { - newLanguagesDao.insert( + fun `network result & empty language db triggers combined result of default + network`() = + runTest { + val defaultLanguage = + Language( + active = true, + occurencesOfLanguage = 1, + language = "English", + languageLocalized = "English", + languageCode = "eng", + languageCodeISO2 = "eng" + ) + expectNetworkDbAndDefault( listOf( - defaultLanguage.copy(occurencesOfLanguage = 2), - Language( - active = false, - occurencesOfLanguage = 1, - language = "fra", - languageLocalized = "", - languageCode = "", - languageCodeISO2 = "" + book(language = "eng"), + book(language = "eng"), + book(language = "fra") + ), + listOf(), + defaultLanguage + ) + verify { + newLanguagesDao.insert( + listOf( + defaultLanguage.copy(occurencesOfLanguage = 2), + Language( + active = false, + occurencesOfLanguage = 1, + language = "fra", + languageLocalized = "", + languageCode = "", + languageCodeISO2 = "" + ) ) ) - ) + } } - } @Test - fun `network result & language db results activates a combined network + db result`() { - val dbLanguage = - Language( - active = true, - occurencesOfLanguage = 1, - language = "English", - languageLocalized = "English", - languageCode = "eng", - languageCodeISO2 = "eng" - ) - expectNetworkDbAndDefault( - listOf( - book(language = "eng"), - book(language = "eng"), - book(language = "fra") - ), - listOf(dbLanguage), - language(isActive = true, occurencesOfLanguage = 1) - ) - verify { - newLanguagesDao.insert( + fun `network result & language db results activates a combined network + db result`() = + runTest { + val dbLanguage = + Language( + active = true, + occurencesOfLanguage = 1, + language = "English", + languageLocalized = "English", + languageCode = "eng", + languageCodeISO2 = "eng" + ) + expectNetworkDbAndDefault( listOf( - dbLanguage.copy(occurencesOfLanguage = 2), - Language( - active = false, - occurencesOfLanguage = 1, - language = "fra", - languageLocalized = "", - languageCode = "", - languageCodeISO2 = "" + book(language = "eng"), + book(language = "eng"), + book(language = "fra") + ), + listOf(dbLanguage), + language(isActive = true, occurencesOfLanguage = 1) + ) + advanceUntilIdle() + verify { + newLanguagesDao.insert( + listOf( + dbLanguage.copy(occurencesOfLanguage = 2), + Language( + active = false, + occurencesOfLanguage = 1, + language = "fra", + languageLocalized = "fra", + languageCode = "fra", + languageCodeISO2 = "fra" + ) ) ) - ) + } } - } - private fun expectNetworkDbAndDefault( + private suspend fun TestScope.expectNetworkDbAndDefault( networkBooks: List, dbBooks: List, defaultLanguage: Language ) { every { application.getString(any()) } returns "" every { application.getString(any(), any()) } returns "" - every { kiwixService.library } returns Single.just(libraryNetworkEntity(networkBooks)) + coEvery { kiwixService.getLibrary() } returns libraryNetworkEntity(networkBooks) every { defaultLanguageProvider.provide() } returns defaultLanguage + viewModel.networkLibrary.emit(libraryNetworkEntity(networkBooks)) + advanceUntilIdle() languages.value = dbBooks - testScheduler.triggerActions() + advanceUntilIdle() networkStates.onNext(CONNECTED) - testScheduler.triggerActions() + advanceUntilIdle() } } + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `network states observed`() { + fun `network states observed`() = runTest { networkStates.offer(NOT_CONNECTED) + advanceUntilIdle() viewModel.networkStates.test() .assertValue(NOT_CONNECTED) } @Test - fun `library update removes from sources and maps to list items`() { + fun `library update removes from sources and maps to list items`() = runTest { val bookAlreadyOnDisk = book(id = "0", url = "", language = Locale.ENGLISH.language) val bookDownloading = book(id = "1", url = "") val bookWithActiveLanguage = book(id = "3", language = "activeLanguage", url = "") val bookWithInactiveLanguage = book(id = "4", language = "inactiveLanguage", url = "") - every { application.getString(any()) } returns "" - every { application.getString(any(), any()) } returns "" - every { - kiwixService.library - } returns - Single.just( - libraryNetworkEntity( + testFlow( + flow = viewModel.libraryItems, + triggerAction = { + every { application.getString(any()) } returns "" + every { application.getString(any(), any()) } returns "" + networkStates.onNext(CONNECTED) + downloads.value = listOf(downloadModel(book = bookDownloading)) + books.value = listOf(bookOnDisk(book = bookAlreadyOnDisk)) + languages.value = listOf( - bookAlreadyOnDisk, - bookDownloading, - bookWithActiveLanguage, - bookWithInactiveLanguage + language(isActive = true, occurencesOfLanguage = 1, languageCode = "activeLanguage"), + language(isActive = false, occurencesOfLanguage = 1, languageCode = "inactiveLanguage") + ) + fileSystemStates.value = CanWrite4GbFile + viewModel.networkLibrary.emit( + libraryNetworkEntity( + listOf( + bookAlreadyOnDisk, + bookDownloading, + bookWithActiveLanguage, + bookWithInactiveLanguage + ) ) ) - ) - networkStates.onNext(CONNECTED) - downloads.value = listOf(downloadModel(book = bookDownloading)) - books.value = listOf(bookOnDisk(book = bookAlreadyOnDisk)) - languages.value = - listOf( - language(isActive = true, occurencesOfLanguage = 1, languageCode = "activeLanguage"), - language(isActive = false, occurencesOfLanguage = 1, languageCode = "inactiveLanguage") - ) - fileSystemStates.value = CanWrite4GbFile - testScheduler.advanceTimeBy(500, MILLISECONDS) - testScheduler.triggerActions() - viewModel.libraryItems.test() - .assertValue( - listOf( - LibraryListItem.DividerItem(Long.MAX_VALUE, R.string.downloading), - LibraryListItem.LibraryDownloadItem(downloadModel(book = bookDownloading)), - LibraryListItem.DividerItem(Long.MAX_VALUE - 1, R.string.your_languages), - LibraryListItem.BookItem(bookWithActiveLanguage, CanWrite4GbFile), - LibraryListItem.DividerItem(Long.MIN_VALUE, R.string.other_languages), - LibraryListItem.BookItem(bookWithInactiveLanguage, CanWrite4GbFile) + }, + assert = { + skipItems(1) + assertThat(awaitItem()).isEqualTo( + listOf( + LibraryListItem.DividerItem(Long.MAX_VALUE, R.string.downloading), + LibraryListItem.LibraryDownloadItem(downloadModel(book = bookDownloading)), + LibraryListItem.DividerItem(Long.MAX_VALUE - 1, R.string.your_languages), + LibraryListItem.BookItem(bookWithActiveLanguage, CanWrite4GbFile), + LibraryListItem.DividerItem(Long.MIN_VALUE, R.string.other_languages), + LibraryListItem.BookItem(bookWithInactiveLanguage, CanWrite4GbFile) + ) ) - ) + } + ) } + @OptIn(ExperimentalCoroutinesApi::class) @Test - fun `library marks files over 4GB as can't download if file system state says to`() { + fun `library marks files over 4GB as can't download if file system state says to`() = runTest { val bookOver4Gb = book( id = "0", @@ -451,26 +461,29 @@ class ZimManageViewModelTest { ) every { application.getString(any()) } returns "" every { application.getString(any(), any()) } returns "" - every { - kiwixService.library - } returns Single.just(libraryNetworkEntity(listOf(bookOver4Gb))) - networkStates.onNext(CONNECTED) - downloads.value = listOf() - books.value = listOf() - languages.value = - listOf( - language(isActive = true, occurencesOfLanguage = 1, languageCode = "activeLanguage") - ) - fileSystemStates.value = CannotWrite4GbFile - testScheduler.advanceTimeBy(500, MILLISECONDS) - testScheduler.triggerActions() - viewModel.libraryItems.test() - .assertValue( - listOf( - LibraryListItem.DividerItem(Long.MIN_VALUE, R.string.other_languages), - LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile) + testFlow( + viewModel.libraryItems, + triggerAction = { + networkStates.onNext(CONNECTED) + downloads.value = listOf() + books.value = listOf() + languages.value = + listOf( + language(isActive = true, occurencesOfLanguage = 1, languageCode = "activeLanguage") + ) + fileSystemStates.value = CannotWrite4GbFile + viewModel.networkLibrary.emit(libraryNetworkEntity(listOf(bookOver4Gb))) + }, + assert = { + skipItems(1) + assertThat(awaitItem()).isEqualTo( + listOf( + LibraryListItem.DividerItem(Long.MIN_VALUE, R.string.other_languages), + LibraryListItem.BookItem(bookOver4Gb, CannotWrite4GbFile) + ) ) - ) + } + ) } @Nested diff --git a/core/detekt_baseline.xml b/core/detekt_baseline.xml index 387cb9e36..426a822c2 100644 --- a/core/detekt_baseline.xml +++ b/core/detekt_baseline.xml @@ -12,7 +12,7 @@ LongParameterList:MainMenu.kt$MainMenu$( private val activity: Activity, zimFileReader: ZimFileReader?, menu: Menu, webViews: MutableList<KiwixWebView>, urlIsValid: Boolean, disableReadAloud: Boolean = false, disableTabs: Boolean = false, private val menuClickListener: MenuClickListener ) LongParameterList:MainMenu.kt$MainMenu.Factory$( menu: Menu, webViews: MutableList<KiwixWebView>, urlIsValid: Boolean, menuClickListener: MenuClickListener, disableReadAloud: Boolean, disableTabs: Boolean ) LongParameterList:PageTestHelpers.kt$( bookmarkTitle: String = "bookmarkTitle", isSelected: Boolean = false, id: Long = 2, zimId: String = "zimId", zimName: String = "zimName", zimFilePath: String = "zimFilePath", bookmarkUrl: String = "bookmarkUrl", favicon: String = "favicon" ) - LongParameterList:Repository.kt$Repository$( @param:IO private val ioThread: Scheduler, private val bookDao: NewBookDao, private val libkiwixBookmarks: LibkiwixBookmarks, private val historyRoomDao: HistoryRoomDao, private val webViewHistoryRoomDao: WebViewHistoryRoomDao, private val notesRoomDao: NotesRoomDao, private val languageDao: NewLanguagesDao, private val recentSearchRoomDao: RecentSearchRoomDao, private val zimReaderContainer: ZimReaderContainer ) + LongParameterList:Repository.kt$Repository$( private val bookDao: NewBookDao, private val libkiwixBookmarks: LibkiwixBookmarks, private val historyRoomDao: HistoryRoomDao, private val webViewHistoryRoomDao: WebViewHistoryRoomDao, private val notesRoomDao: NotesRoomDao, private val languageDao: NewLanguagesDao, private val recentSearchRoomDao: RecentSearchRoomDao, private val zimReaderContainer: ZimReaderContainer ) LongParameterList:ToolbarScrollingKiwixWebView.kt$ToolbarScrollingKiwixWebView$( context: Context, callback: WebViewCallback, attrs: AttributeSet, nonVideoView: ViewGroup, videoView: ViewGroup, webViewClient: CoreWebViewClient, private val toolbarView: View, private val bottomBarView: View, sharedPreferenceUtil: SharedPreferenceUtil, private val parentNavigationBar: View? = null ) MagicNumber:ArticleCount.kt$ArticleCount$3 MagicNumber:CompatFindActionModeCallback.kt$CompatFindActionModeCallback$100 diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt index 21fc9f5bf..70cbcb98f 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/DataSource.kt @@ -17,7 +17,6 @@ */ package org.kiwix.kiwixmobile.core.data -import io.reactivex.Completable import kotlinx.coroutines.flow.Flow import org.kiwix.kiwixmobile.core.dao.entities.WebViewHistoryEntity import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem @@ -33,21 +32,21 @@ import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.BooksOnDiskListIte */ interface DataSource { fun getLanguageCategorizedBooks(): Flow> - fun saveBook(book: BookOnDisk): Completable - fun saveBooks(book: List): Completable - fun saveLanguages(languages: List): Completable - fun saveHistory(history: HistoryItem): Completable - fun deleteHistory(historyList: List): Completable - fun clearHistory(): Completable + suspend fun saveBook(book: BookOnDisk) + suspend fun saveBooks(book: List) + suspend fun saveLanguages(languages: List) + suspend fun saveHistory(history: HistoryItem) + suspend fun deleteHistory(historyList: List) + suspend fun clearHistory() fun getBookmarks(): Flow> suspend fun getCurrentZimBookmarksUrl(): List suspend fun saveBookmark(libkiwixBookmarkItem: LibkiwixBookmarkItem) suspend fun deleteBookmarks(bookmarks: List) suspend fun deleteBookmark(bookId: String, bookmarkUrl: String) fun booksOnDiskAsListItems(): Flow> - fun saveNote(noteListItem: NoteListItem): Completable - fun deleteNote(noteTitle: String): Completable - fun deleteNotes(noteList: List): Completable + suspend fun saveNote(noteListItem: NoteListItem) + suspend fun deleteNote(noteTitle: String) + suspend fun deleteNotes(noteList: List) suspend fun insertWebViewPageHistoryItems(webViewHistoryEntityList: List) fun getAllWebViewPagesHistory(): Flow> suspend fun clearWebViewPagesHistory() diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt index f22d5c76d..5517a601a 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/Repository.kt @@ -18,12 +18,11 @@ package org.kiwix.kiwixmobile.core.data -import io.reactivex.Completable -import io.reactivex.Scheduler import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.flowOn import kotlinx.coroutines.flow.map +import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.dao.HistoryRoomDao import org.kiwix.kiwixmobile.core.dao.LibkiwixBookmarks import org.kiwix.kiwixmobile.core.dao.NewBookDao @@ -32,7 +31,6 @@ import org.kiwix.kiwixmobile.core.dao.NotesRoomDao import org.kiwix.kiwixmobile.core.dao.RecentSearchRoomDao import org.kiwix.kiwixmobile.core.dao.WebViewHistoryRoomDao import org.kiwix.kiwixmobile.core.dao.entities.WebViewHistoryEntity -import org.kiwix.kiwixmobile.core.di.qualifiers.IO import org.kiwix.kiwixmobile.core.extensions.HeaderizableList import org.kiwix.kiwixmobile.core.page.bookmark.adapter.LibkiwixBookmarkItem import org.kiwix.kiwixmobile.core.page.history.adapter.HistoryListItem @@ -52,7 +50,6 @@ import javax.inject.Singleton @Singleton class Repository @Inject internal constructor( - @param:IO private val ioThread: Scheduler, private val bookDao: NewBookDao, private val libkiwixBookmarks: LibkiwixBookmarks, private val historyRoomDao: HistoryRoomDao, @@ -91,33 +88,38 @@ class Repository @Inject internal constructor( .map(MutableList::toList) .flowOn(Dispatchers.IO) - override fun saveBooks(books: List) = - Completable.fromAction { bookDao.insert(books) } - .subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun saveBooks(books: List) = withContext(Dispatchers.IO) { + bookDao.insert(books) + } - override fun saveBook(book: BookOnDisk) = - Completable.fromAction { bookDao.insert(listOf(book)) } - .subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun saveBook(book: BookOnDisk) = withContext(Dispatchers.IO) { + bookDao.insert(listOf(book)) + } - override fun saveLanguages(languages: List) = - Completable.fromAction { languageDao.insert(languages) } - .subscribeOn(ioThread) - - override fun saveHistory(history: HistoryItem) = - Completable.fromAction { historyRoomDao.saveHistory(history) } - .subscribeOn(ioThread) - - override fun deleteHistory(historyList: List) = - Completable.fromAction { - historyRoomDao.deleteHistory(historyList.filterIsInstance(HistoryItem::class.java)) + @Suppress("InjectDispatcher") + override suspend fun saveLanguages(languages: List) = + withContext(Dispatchers.IO) { + languageDao.insert(languages) } - .subscribeOn(ioThread) - override fun clearHistory() = - Completable.fromAction { - historyRoomDao.deleteAllHistory() - recentSearchRoomDao.deleteSearchHistory() - }.subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun saveHistory(history: HistoryItem) = withContext(Dispatchers.IO) { + historyRoomDao.saveHistory(history) + } + + @Suppress("InjectDispatcher") + override suspend fun deleteHistory(historyList: List) = + withContext(Dispatchers.IO) { + historyRoomDao.deleteHistory(historyList.filterIsInstance()) + } + + @Suppress("InjectDispatcher") + override suspend fun clearHistory() = withContext(Dispatchers.IO) { + historyRoomDao.deleteAllHistory() + recentSearchRoomDao.deleteSearchHistory() + } override fun getBookmarks() = libkiwixBookmarks.bookmarks() as Flow> @@ -134,13 +136,17 @@ class Repository @Inject internal constructor( override suspend fun deleteBookmark(bookId: String, bookmarkUrl: String) = libkiwixBookmarks.deleteBookmark(bookId, bookmarkUrl) - override fun saveNote(noteListItem: NoteListItem): Completable = - Completable.fromAction { notesRoomDao.saveNote(noteListItem) } - .subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun saveNote(noteListItem: NoteListItem) = + withContext(Dispatchers.IO) { + notesRoomDao.saveNote(noteListItem) + } - override fun deleteNotes(noteList: List) = - Completable.fromAction { notesRoomDao.deleteNotes(noteList) } - .subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun deleteNotes(noteList: List) = + withContext(Dispatchers.IO) { + notesRoomDao.deleteNotes(noteList) + } override suspend fun insertWebViewPageHistoryItems( webViewHistoryEntityList: List @@ -155,7 +161,8 @@ class Repository @Inject internal constructor( webViewHistoryRoomDao.clearWebViewPagesHistory() } - override fun deleteNote(noteTitle: String): Completable = - Completable.fromAction { notesRoomDao.deleteNote(noteTitle) } - .subscribeOn(ioThread) + @Suppress("InjectDispatcher") + override suspend fun deleteNote(noteTitle: String) = withContext(Dispatchers.IO) { + notesRoomDao.deleteNote(noteTitle) + } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/KiwixService.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/KiwixService.kt index 0e127afe9..ee6dcf180 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/KiwixService.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/remote/KiwixService.kt @@ -19,25 +19,22 @@ package org.kiwix.kiwixmobile.core.data.remote -import io.reactivex.Observable -import io.reactivex.Single -import io.reactivex.schedulers.Schedulers import okhttp3.OkHttpClient import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity import org.kiwix.kiwixmobile.core.entity.MetaLinkNetworkEntity import retrofit2.Retrofit -import retrofit2.adapter.rxjava2.RxJava2CallAdapterFactory import retrofit2.converter.simplexml.SimpleXmlConverterFactory import retrofit2.http.GET import retrofit2.http.Url interface KiwixService { - @get:GET(LIBRARY_NETWORK_PATH) val library: Single + @GET(LIBRARY_NETWORK_PATH) + suspend fun getLibrary(): LibraryNetworkEntity? @GET - fun getMetaLinks( + suspend fun getMetaLinks( @Url url: String - ): Observable + ): MetaLinkNetworkEntity? /******** Helper class that sets up new services */ object ServiceCreator { @@ -47,7 +44,6 @@ interface KiwixService { .baseUrl(baseUrl) .client(okHttpClient) .addConverterFactory(SimpleXmlConverterFactory.create()) - .addCallAdapterFactory(RxJava2CallAdapterFactory.createWithScheduler(Schedulers.io())) .build() return retrofit.create(KiwixService::class.java) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/ApplicationModule.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/ApplicationModule.kt index cc72e7017..719a143cc 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/ApplicationModule.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/di/modules/ApplicationModule.kt @@ -24,15 +24,9 @@ import android.net.ConnectivityManager import android.os.storage.StorageManager import dagger.Module import dagger.Provides -import io.reactivex.Scheduler -import io.reactivex.android.schedulers.AndroidSchedulers -import io.reactivex.schedulers.Schedulers import org.kiwix.kiwixmobile.core.DarkModeConfig import org.kiwix.kiwixmobile.core.data.remote.ObjectBoxToLibkiwixMigrator import org.kiwix.kiwixmobile.core.data.remote.ObjectBoxToRoomMigrator -import org.kiwix.kiwixmobile.core.di.qualifiers.Computation -import org.kiwix.kiwixmobile.core.di.qualifiers.IO -import org.kiwix.kiwixmobile.core.di.qualifiers.MainThread import org.kiwix.kiwixmobile.core.downloader.DownloadMonitor import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadManagerMonitor import org.kiwix.kiwixmobile.core.reader.ZimFileReader @@ -68,18 +62,6 @@ class ApplicationModule { @Singleton fun provideObjectBoxToRoomMigrator() = ObjectBoxToRoomMigrator() - @IO - @Provides - fun provideIoThread(): Scheduler = Schedulers.io() - - @MainThread - @Provides - fun provideMainThread(): Scheduler = AndroidSchedulers.mainThread() - - @Computation - @Provides - fun provideComputationThread(): Scheduler = Schedulers.computation() - @Provides @Singleton internal fun provideDownloadMonitor( diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/DownloaderImpl.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/DownloaderImpl.kt index e5e24a84a..28cfc54ca 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/DownloaderImpl.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/DownloaderImpl.kt @@ -18,8 +18,9 @@ package org.kiwix.kiwixmobile.core.downloader -import io.reactivex.Observable -import io.reactivex.schedulers.Schedulers +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.dao.DownloadRoomDao import org.kiwix.kiwixmobile.core.data.remote.KiwixService import org.kiwix.kiwixmobile.core.entity.LibraryNetworkEntity @@ -31,26 +32,25 @@ class DownloaderImpl @Inject constructor( private val downloadRoomDao: DownloadRoomDao, private val kiwixService: KiwixService ) : Downloader { - @Suppress("CheckResult", "IgnoredReturnValue") + @Suppress("InjectDispatcher") override fun download(book: LibraryNetworkEntity.Book) { - urlProvider(book) - .take(1) - .subscribeOn(Schedulers.io()) - .subscribe( - { + CoroutineScope(Dispatchers.IO).launch { + runCatching { + urlProvider(book)?.let { downloadRoomDao.addIfDoesNotExist(it, book, downloadRequester) - }, - Throwable::printStackTrace - ) + } + }.onFailure { + it.printStackTrace() + } + } } @Suppress("UnsafeCallOnNullableType") - private fun urlProvider(book: Book): Observable = + private suspend fun urlProvider(book: Book): String? = if (book.url?.endsWith("meta4") == true) { - kiwixService.getMetaLinks(book.url!!) - .map { it.relevantUrl.value } + kiwixService.getMetaLinks(book.url!!)?.relevantUrl?.value } else { - Observable.just(book.url) + book.url } override fun cancelDownload(downloadId: Long) { diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt index 81f49ac6a..df3a5e819 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/AddNoteDialog.kt @@ -41,6 +41,7 @@ import androidx.core.content.ContextCompat import androidx.core.content.FileProvider import androidx.fragment.app.DialogFragment import androidx.lifecycle.lifecycleScope +import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.CoreApp.Companion.coreComponent import org.kiwix.kiwixmobile.core.CoreApp.Companion.instance import org.kiwix.kiwixmobile.core.R @@ -73,9 +74,6 @@ import javax.inject.Inject * Notes are saved as text files at location: "{External Storage}/Kiwix/Notes/ZimFileName/ArticleUrl.txt" */ -const val DISABLE_ICON_ITEM_ALPHA = 130 -const val ENABLE_ICON_ITEM_ALPHA = 255 - class AddNoteDialog : DialogFragment() { private lateinit var zimId: String private var zimFileName: String? = null @@ -401,41 +399,45 @@ class AddNoteDialog : DialogFragment() { } private fun addNoteToDao(noteFilePath: String?, title: String) { - noteFilePath?.let { filePath -> - if (filePath.isNotEmpty() && zimFileUrl.isNotEmpty()) { - val noteToSave = NoteListItem( - zimId = zimId, - title = title, - url = zimFileUrl, - noteFilePath = noteFilePath, - zimReaderSource = zimReaderSource, - favicon = favicon, - ) - mainRepositoryActions.saveNote(noteToSave) - } else { - Log.d(TAG, "Cannot process with empty zim url or noteFilePath") + lifecycleScope.launch { + noteFilePath?.let { filePath -> + if (filePath.isNotEmpty() && zimFileUrl.isNotEmpty()) { + val noteToSave = NoteListItem( + zimId = zimId, + title = title, + url = zimFileUrl, + noteFilePath = noteFilePath, + zimReaderSource = zimReaderSource, + favicon = favicon, + ) + mainRepositoryActions.saveNote(noteToSave) + } else { + Log.d(TAG, "Cannot process with empty zim url or noteFilePath") + } } } } private fun deleteNote() { - val notesFolder = File(zimNotesDirectory) - val noteFile = - File(notesFolder.absolutePath, "$articleNoteFileName.txt") - val noteDeleted = noteFile.delete() - val editedNoteText = noteText.value.text - if (noteDeleted) { - noteText.value = TextFieldValue("") - mainRepositoryActions.deleteNote(getNoteTitle()) - disableMenuItems() - snackBarHostState.snack( - message = requireActivity().getString(R.string.note_delete_successful), - actionLabel = requireActivity().getString(R.string.undo), - actionClick = { restoreDeletedNote(editedNoteText) }, - lifecycleScope = lifecycleScope - ) - } else { - context.toast(R.string.note_delete_unsuccessful, Toast.LENGTH_LONG) + lifecycleScope.launch { + val notesFolder = File(zimNotesDirectory) + val noteFile = + File(notesFolder.absolutePath, "$articleNoteFileName.txt") + val noteDeleted = noteFile.delete() + val editedNoteText = noteText.value.text + if (noteDeleted) { + noteText.value = TextFieldValue("") + mainRepositoryActions.deleteNote(getNoteTitle()) + disableMenuItems() + snackBarHostState.snack( + message = requireActivity().getString(R.string.note_delete_successful), + actionLabel = requireActivity().getString(R.string.undo), + actionClick = { restoreDeletedNote(editedNoteText) }, + lifecycleScope = lifecycleScope + ) + } else { + context.toast(R.string.note_delete_unsuccessful, Toast.LENGTH_LONG) + } } } @@ -514,7 +516,6 @@ class AddNoteDialog : DialogFragment() { override fun onDestroyView() { super.onDestroyView() - mainRepositoryActions.dispose() onBackPressedCallBack.remove() } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt index 4a34fb720..c19977ec8 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/CoreReaderFragment.kt @@ -1268,7 +1268,6 @@ abstract class CoreReaderFragment : if (sharedPreferenceUtil?.showIntro() == true) { (requireActivity() as? AppCompatActivity)?.setSupportActionBar(null) } - repositoryActions?.dispose() safelyCancelBookmarkJob() unBindViewsAndBinding() tabCallback = null @@ -2658,7 +2657,9 @@ abstract class CoreReaderFragment : timeStamp, zimFileReader!! ) - repositoryActions?.saveHistory(history) + lifecycleScope.launch { + repositoryActions?.saveHistory(history) + } } } updateBottomToolbarVisibility() diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt index 0f4104ac4..d351ea981 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/main/MainRepositoryActions.kt @@ -17,9 +17,8 @@ */ package org.kiwix.kiwixmobile.core.main -import io.reactivex.disposables.Disposable -import kotlinx.coroutines.flow.first import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first import kotlinx.coroutines.withContext import org.kiwix.kiwixmobile.core.dao.entities.WebViewHistoryEntity import org.kiwix.kiwixmobile.core.data.DataSource @@ -36,57 +35,61 @@ private const val TAG = "MainPresenter" @ActivityScope class MainRepositoryActions @Inject constructor(private val dataSource: DataSource) { - private var saveHistoryDisposable: Disposable? = null - private var saveNoteDisposable: Disposable? = null - private var saveBookDisposable: Disposable? = null - private var deleteNoteDisposable: Disposable? = null - private var saveWebViewHistoryDisposable: Disposable? = null - private var clearWebViewHistoryDisposable: Disposable? = null - - fun saveHistory(history: HistoryItem) { - saveHistoryDisposable = - dataSource.saveHistory(history) - .subscribe({}, { e -> Log.e(TAG, "Unable to save history", e) }) + @Suppress("InjectDispatcher") + suspend fun saveHistory(history: HistoryItem) { + runCatching { + withContext(Dispatchers.IO) { + dataSource.saveHistory(history) + } + }.onFailure { + Log.e(TAG, "Unable to save history", it) + } } - @Suppress("InjectDispatcher", "TooGenericExceptionCaught") + @Suppress("InjectDispatcher") suspend fun saveBookmark(bookmark: LibkiwixBookmarkItem) { - withContext(Dispatchers.IO) { - try { + runCatching { + withContext(Dispatchers.IO) { dataSource.saveBookmark(bookmark) - } catch (e: Exception) { - Log.e(TAG, "Unable to save bookmark", e) } + }.onFailure { + Log.e(TAG, "Unable to save bookmark", it) } } - @Suppress("InjectDispatcher", "TooGenericExceptionCaught") + @Suppress("InjectDispatcher") suspend fun deleteBookmark(bookId: String, bookmarkUrl: String) { - withContext(Dispatchers.IO) { - try { + runCatching { + withContext(Dispatchers.IO) { dataSource.deleteBookmark(bookId, bookmarkUrl) - } catch (e: Exception) { - Log.e(TAG, "Unable to delete bookmark", e) } + }.onFailure { + Log.e(TAG, "Unable to delete bookmark", it) } } - fun saveNote(note: NoteListItem) { - saveNoteDisposable = + suspend fun saveNote(note: NoteListItem) { + runCatching { dataSource.saveNote(note) - .subscribe({}, { e -> Log.e(TAG, "Unable to save note", e) }) + }.onFailure { + Log.e(TAG, "Unable to save note", it) + } } - fun deleteNote(noteTitle: String) { - deleteNoteDisposable = + suspend fun deleteNote(noteTitle: String) { + runCatching { dataSource.deleteNote(noteTitle) - .subscribe({}, { e -> Log.e(TAG, "Unable to delete note", e) }) + }.onFailure { + Log.e(TAG, "Unable to delete note", it) + } } - fun saveBook(book: BookOnDisk) { - saveBookDisposable = + suspend fun saveBook(book: BookOnDisk) { + runCatching { dataSource.saveBook(book) - .subscribe({}, { e -> Log.e(TAG, "Unable to save book", e) }) + }.onFailure { + Log.e(TAG, "Unable to save book", it) + } } suspend fun saveWebViewPageHistory(webViewHistoryEntityList: List) { @@ -101,13 +104,4 @@ class MainRepositoryActions @Inject constructor(private val dataSource: DataSour dataSource.getAllWebViewPagesHistory() .first() .map(::WebViewHistoryItem) - - fun dispose() { - saveHistoryDisposable?.dispose() - saveNoteDisposable?.dispose() - deleteNoteDisposable?.dispose() - saveBookDisposable?.dispose() - saveWebViewHistoryDisposable?.dispose() - clearWebViewHistoryDisposable?.dispose() - } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt index 2b15dd3ec..2567c2432 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/CorePrefsFragment.kt @@ -164,7 +164,6 @@ abstract class CorePrefsFragment : } override fun onDestroyView() { - presenter?.dispose() storagePermissionForNotesLauncher?.unregister() storagePermissionForNotesLauncher = null super.onDestroyView() @@ -262,8 +261,10 @@ abstract class CorePrefsFragment : private fun clearAllHistoryDialog() { alertDialogShower?.show(KiwixDialog.ClearAllHistory, { - presenter?.clearHistory() - Snackbar.make(requireView(), R.string.all_history_cleared, Snackbar.LENGTH_SHORT).show() + lifecycleScope.launch { + presenter?.clearHistory() + Snackbar.make(requireView(), R.string.all_history_cleared, Snackbar.LENGTH_SHORT).show() + } }) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsContract.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsContract.kt index 5f4221752..fa45891f0 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsContract.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsContract.kt @@ -22,6 +22,6 @@ import org.kiwix.kiwixmobile.core.base.BaseContract interface SettingsContract { interface View : BaseContract.View interface Presenter : BaseContract.Presenter { - fun clearHistory() + suspend fun clearHistory() } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsPresenter.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsPresenter.kt index 89199204c..c4c207f06 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsPresenter.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/settings/SettingsPresenter.kt @@ -17,28 +17,20 @@ */ package org.kiwix.kiwixmobile.core.settings -import org.kiwix.kiwixmobile.core.utils.files.Log -import io.reactivex.disposables.Disposable import org.kiwix.kiwixmobile.core.base.BasePresenter import org.kiwix.kiwixmobile.core.data.DataSource import org.kiwix.kiwixmobile.core.settings.SettingsContract.Presenter import org.kiwix.kiwixmobile.core.settings.SettingsContract.View +import org.kiwix.kiwixmobile.core.utils.files.Log import javax.inject.Inject internal class SettingsPresenter @Inject constructor(private val dataSource: DataSource) : BasePresenter(), Presenter { - private var dataSourceDisposable: Disposable? = null - override fun clearHistory() { - dataSourceDisposable = + override suspend fun clearHistory() { + runCatching { dataSource.clearHistory() - .subscribe({ - // TODO - }, { e -> - Log.e("SettingsPresenter", e.message, e) - }) - } - - fun dispose() { - dataSourceDisposable?.dispose() + }.onFailure { + Log.e("SettingsPresenter", it.message, it) + } } } diff --git a/core/src/test/java/org/kiwix/kiwixmobile/core/StorageObserverTest.kt b/core/src/test/java/org/kiwix/kiwixmobile/core/StorageObserverTest.kt index 6560c8cc9..67d240d71 100644 --- a/core/src/test/java/org/kiwix/kiwixmobile/core/StorageObserverTest.kt +++ b/core/src/test/java/org/kiwix/kiwixmobile/core/StorageObserverTest.kt @@ -22,13 +22,11 @@ import io.mockk.clearAllMocks import io.mockk.every import io.mockk.mockk import io.mockk.verify -import io.reactivex.schedulers.Schedulers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.assertj.core.api.Assertions.assertThat -import org.junit.jupiter.api.AfterAll import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.kiwix.kiwixmobile.core.dao.DownloadRoomDao @@ -44,8 +42,6 @@ import org.kiwix.kiwixmobile.core.utils.files.testFlow import org.kiwix.kiwixmobile.core.zim_manager.fileselect_view.BooksOnDiskListItem.BookOnDisk import org.kiwix.sharedFunctions.book import org.kiwix.sharedFunctions.bookOnDisk -import org.kiwix.sharedFunctions.resetSchedulers -import org.kiwix.sharedFunctions.setScheduler import java.io.File class StorageObserverTest { @@ -65,15 +61,6 @@ class StorageObserverTest { private lateinit var storageObserver: StorageObserver - init { - setScheduler(Schedulers.trampoline()) - } - - @AfterAll - fun teardown() { - resetSchedulers() - } - @BeforeEach fun init() { clearAllMocks() every { sharedPreferenceUtil.prefStorage } returns "a" diff --git a/custom/src/main/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadFragment.kt b/custom/src/main/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadFragment.kt index 01b831e75..908e36783 100644 --- a/custom/src/main/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadFragment.kt +++ b/custom/src/main/java/org/kiwix/kiwixmobile/custom/download/CustomDownloadFragment.kt @@ -27,7 +27,6 @@ import android.view.ViewGroup import androidx.core.app.ActivityCompat import androidx.lifecycle.ViewModelProvider import androidx.lifecycle.lifecycleScope -import io.reactivex.disposables.CompositeDisposable import kotlinx.coroutines.launch import org.kiwix.kiwixmobile.core.R import org.kiwix.kiwixmobile.core.base.BaseActivity @@ -71,8 +70,6 @@ class CustomDownloadFragment : BaseFragment(), FragmentActivityExtensions { @Inject lateinit var viewModelFactory: ViewModelProvider.Factory private var fragmentCustomDownloadBinding: FragmentCustomDownloadBinding? = null - - private val compositeDisposable = CompositeDisposable() override fun inject(baseActivity: BaseActivity) { baseActivity.customActivityComponent.inject(this) } @@ -144,7 +141,6 @@ class CustomDownloadFragment : BaseFragment(), FragmentActivityExtensions { override fun onDestroy() { super.onDestroy() - compositeDisposable.clear() activity?.finish() } diff --git a/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/effects/DownloadCustomTest.kt b/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/effects/DownloadCustomTest.kt index 838ef58ff..bd22dadb1 100644 --- a/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/effects/DownloadCustomTest.kt +++ b/custom/src/test/java/org/kiwix/kiwixmobile/custom/download/effects/DownloadCustomTest.kt @@ -18,8 +18,8 @@ package org.kiwix.kiwixmobile.custom.download.effects +import io.mockk.coVerify import io.mockk.mockk -import io.mockk.verify import org.junit.jupiter.api.Test import org.kiwix.kiwixmobile.core.downloader.Downloader import org.kiwix.sharedFunctions.book @@ -29,7 +29,7 @@ internal class DownloadCustomTest { fun `invokeWith queues download with ZimUrl`() { val downloader = mockk() DownloadCustom(downloader).invokeWith(mockk()) - verify { + coVerify { downloader.download(expectedBook()) } }