diff --git a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt index 9999f9ca47..732a77be89 100644 --- a/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt +++ b/app/src/main/java/org/kiwix/kiwixmobile/nav/destination/library/OnlineLibraryFragment.kt @@ -148,6 +148,10 @@ class OnlineLibraryFragment : BaseFragment(), FragmentActivityExtensions { }, { context?.let { context -> + if (isNotConnected) { + noInternetSnackbar() + return@let + } downloader.pauseResumeDownload( it.downloadId, it.downloadState.toReadableState(context).contains(getString(string.paused_state)) diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/DownloadRoomEntity.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/DownloadRoomEntity.kt index 1a049bae46..dff17d8740 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/DownloadRoomEntity.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/dao/entities/DownloadRoomEntity.kt @@ -54,7 +54,8 @@ data class DownloadRoomEntity( val size: String, val name: String?, val favIcon: String, - val tags: String? = null + val tags: String? = null, + var pausedByUser: Boolean = false ) { constructor(downloadUrl: String, downloadId: Long, book: Book, file: String?) : this( file = file, @@ -99,7 +100,8 @@ data class DownloadRoomEntity( totalSizeOfDownload = download.totalSizeOfDownload, status = download.state, error = download.error, - progress = download.progress + progress = download.progress, + pausedByUser = download.pausedByUser ) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt index 58a89a6c70..a5561b5786 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/data/KiwixRoomDatabase.kt @@ -44,7 +44,7 @@ import org.kiwix.kiwixmobile.core.dao.entities.ZimSourceRoomConverter NotesRoomEntity::class, DownloadRoomEntity::class ], - version = 5, + version = 6, exportSchema = false ) @TypeConverters(HistoryRoomDaoCoverts::class, ZimSourceRoomConverter::class) @@ -62,7 +62,13 @@ abstract class KiwixRoomDatabase : RoomDatabase() { ?: Room.databaseBuilder(context, KiwixRoomDatabase::class.java, "KiwixRoom.db") // We have already database name called kiwix.db in order to avoid complexity we named // as kiwixRoom.db - .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5) + .addMigrations( + MIGRATION_1_2, + MIGRATION_2_3, + MIGRATION_3_4, + MIGRATION_4_5, + MIGRATION_5_6 + ) .build().also { db = it } } } @@ -202,6 +208,15 @@ abstract class KiwixRoomDatabase : RoomDatabase() { } } + @Suppress("MagicNumber") + private val MIGRATION_5_6 = object : Migration(5, 6) { + override fun migrate(database: SupportSQLiteDatabase) { + database.execSQL( + "ALTER TABLE DownloadRoomEntity ADD COLUMN pausedByUser INTEGER NOT NULL DEFAULT 0" + ) + } + } + fun destroyInstance() { db = null } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadManagerMonitor.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadManagerMonitor.kt index 4d41202155..fc633d2b2e 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadManagerMonitor.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadManagerMonitor.kt @@ -32,6 +32,7 @@ import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadNotificatio import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadNotificationManager.Companion.ACTION_QUERY_DOWNLOAD_STATUS import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadNotificationManager.Companion.ACTION_RESUME import org.kiwix.kiwixmobile.core.extensions.isServiceRunning +import org.kiwix.kiwixmobile.core.utils.NetworkUtils import org.kiwix.kiwixmobile.core.utils.files.Log import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -65,8 +66,8 @@ class DownloadManagerMonitor @Inject constructor( if (downloadRoomDao.downloads().blockingFirst().isNotEmpty()) { // Check if there are active downloads and the service is not running. // If so, start the DownloadMonitorService to properly track download progress. - if (shouldStartService()) { - startService() + if (shouldStartDownloadMonitorService()) { + startDownloadMonitorService() } else { // Do nothing; it is for fixing the error when "if" is used as an expression. } @@ -89,32 +90,85 @@ class DownloadManagerMonitor @Inject constructor( * Determines if the DownloadMonitorService should be started. * Checks if there are active downloads and if the service is not already running. */ - private fun shouldStartService(): Boolean = + private fun shouldStartDownloadMonitorService(): Boolean = getActiveDownloads().isNotEmpty() && !context.isServiceRunning(DownloadMonitorService::class.java) private fun getActiveDownloads(): List = - downloadRoomDao.downloadRoomEntity().blockingFirst().filter { - it.status != Status.PAUSED && it.status != Status.CANCELLED - } + downloadRoomDao.downloadRoomEntity().blockingFirst().filter(::isActiveDownload) + + /** + * Determines if a given download is considered active. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is active, false otherwise. + */ + private fun isActiveDownload(download: DownloadRoomEntity): Boolean = + (download.status != Status.PAUSED || isPausedAndRetryable(download)) && + download.status != Status.CANCELLED + + /** + * Checks if a paused download is eligible for retry based on its error status and network conditions. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the paused download is retryable, false otherwise. + */ + private fun isPausedAndRetryable(download: DownloadRoomEntity): Boolean { + return download.status == Status.PAUSED && + ( + isQueuedForWiFiAndConnected(download) || + isQueuedForNetwork(download) || + download.error == Error.WAITING_TO_RETRY || + download.error == Error.PAUSED_UNKNOWN + ) && + NetworkUtils.isNetworkAvailable(context) && + !download.pausedByUser + } + + /** + * Checks if the download is queued for Wi-Fi and the device is connected to Wi-Fi. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is queued for Wi-Fi and connected, false otherwise. + */ + private fun isQueuedForWiFiAndConnected(download: DownloadRoomEntity): Boolean = + download.error == Error.QUEUED_FOR_WIFI && NetworkUtils.isWiFi(context) + + /** + * Checks if the download is waiting for a network connection and the network is now available. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is waiting for a network and connected, false otherwise. + */ + private fun isQueuedForNetwork(download: DownloadRoomEntity): Boolean = + download.error == Error.WAITING_FOR_NETWORK && NetworkUtils.isNetworkAvailable(context) override fun downloadCompleteOrCancelled(intent: Intent) { synchronized(lock) { intent.extras?.let { - val downloadId = it.getLong(DownloadManager.EXTRA_DOWNLOAD_ID, -1L) - if (downloadId != -1L) { - context.startService( - getDownloadMonitorIntent( - ACTION_QUERY_DOWNLOAD_STATUS, - downloadId.toInt() + val downloadId = it.getLong(DownloadManager.EXTRA_DOWNLOAD_ID, DEFAULT_INT_VALUE.toLong()) + if (downloadId != DEFAULT_INT_VALUE.toLong()) { + try { + context.startService( + getDownloadMonitorIntent( + ACTION_QUERY_DOWNLOAD_STATUS, + downloadId.toInt() + ) ) - ) + } catch (ignore: Exception) { + // Catching exception if application is not in foreground. + // Since we can not start the foreground services from background. + Log.e( + "DOWNLOAD_MONITOR", + "Couldn't start download service. Original exception = $ignore" + ) + } } } } } - private fun startService() { + private fun startDownloadMonitorService() { context.startService(Intent(context, DownloadMonitorService::class.java)) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt index 403db07dfb..19f76b8711 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt @@ -40,6 +40,7 @@ import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadNotificatio import org.kiwix.kiwixmobile.core.downloader.downloadManager.DownloadNotificationManager.Companion.ACTION_RESUME import org.kiwix.kiwixmobile.core.downloader.model.DownloadModel import org.kiwix.kiwixmobile.core.downloader.model.DownloadState +import org.kiwix.kiwixmobile.core.utils.NetworkUtils import org.kiwix.kiwixmobile.core.utils.files.Log import java.util.concurrent.TimeUnit import javax.inject.Inject @@ -381,13 +382,18 @@ class DownloadMonitorService : Service() { progress: Int = DEFAULT_INT_VALUE, etaInMilliSeconds: Long = DEFAULT_INT_VALUE.toLong(), bytesDownloaded: Long = DEFAULT_INT_VALUE.toLong(), - totalSizeOfDownload: Long = DEFAULT_INT_VALUE.toLong() + totalSizeOfDownload: Long = DEFAULT_INT_VALUE.toLong(), + pausedByUser: Boolean? = null ) { synchronized(lock) { updater.onNext { downloadRoomDao.getEntityForDownloadId(downloadId)?.let { downloadEntity -> if (shouldUpdateDownloadStatus(downloadEntity)) { val downloadModel = DownloadModel(downloadEntity).apply { + pausedByUser?.let { + this.pausedByUser = it + downloadEntity.pausedByUser = it + } if (shouldUpdateDownloadStatus(status, error, downloadEntity)) { state = status } @@ -420,10 +426,9 @@ class DownloadMonitorService : Service() { /** * Determines whether the download status should be updated based on the current status and error. * - * This method checks the current download status and error, and decides whether to update the status - * of the download entity. Specifically, it handles the case where a download is paused but has been - * queued for resumption. In such cases, it ensures that the download manager is instructed to resume - * the download, and prevents the status from being prematurely updated to "Paused". + * This method evaluates the current download status and error conditions, ensuring proper handling + * for paused downloads, queued downloads, and network-related retries. It coordinates with the + * Download Manager to resume downloads when necessary and prevents premature status updates. * * @param status The current status of the download. * @param error The current error state of the download. @@ -436,38 +441,102 @@ class DownloadMonitorService : Service() { downloadRoomEntity: DownloadRoomEntity ): Boolean { synchronized(lock) { - return@shouldUpdateDownloadStatus if ( - status == Status.PAUSED && - downloadRoomEntity.status == Status.QUEUED - ) { - // Check if the user has resumed the download. - // Do not update the download status immediately since the download manager - // takes some time to actually resume the download. During this time, - // it will still return the paused state. - // By not updating the status right away, we ensure that the user - // sees the "Pending" state, indicating that the download is in the process - // of resuming. - when (error) { - // When the pause reason is unknown or waiting to retry, and the user - // resumes the download, attempt to resume the download if it was not resumed - // due to some reason. - Error.PAUSED_UNKNOWN, - Error.WAITING_TO_RETRY -> { - resumeDownload(downloadRoomEntity.downloadId) - false - } - - // Return true to update the status of the download if there is any other status, - // e.g., WAITING_FOR_WIFI, WAITING_FOR_NETWORK, or any other pause reason - // to inform the user. - else -> true + return@shouldUpdateDownloadStatus when { + // Check if the download is paused and was previously queued. + isPausedAndQueued(status, downloadRoomEntity) -> + handlePausedAndQueuedDownload(error, downloadRoomEntity) + + // Check if the download is paused and retryable due to network availability. + isPausedAndRetryable( + status, + error, + downloadRoomEntity.pausedByUser + ) -> { + handleRetryablePausedDownload(downloadRoomEntity) } - } else { - true + + // Default case: update the status. + else -> true + } + } + } + + /** + * Checks if the download is paused and was previously queued. + * + * Specifically, it evaluates whether the current status is "Paused" while the previous status + * was "Queued", indicating that the user might have initiated a resume action. + * + * @param status The current status of the download. + * @param downloadRoomEntity The download entity to evaluate. + * @return `true` if the download is paused and queued, `false` otherwise. + */ + private fun isPausedAndQueued(status: Status, downloadRoomEntity: DownloadRoomEntity): Boolean = + status == Status.PAUSED && downloadRoomEntity.status == Status.QUEUED + + /** + * Checks if the download is paused and retryable based on the error and network conditions. + * + * This evaluates whether the download can be resumed, considering its paused state, + * error condition (e.g., waiting for retry), and the availability of a network connection. + * + * @param status The current status of the download. + * @param error The current error state of the download. + * @param pausedByUser To identify if the download paused by user or downloadManager. + * @return `true` if the download is paused and retryable, `false` otherwise. + */ + private fun isPausedAndRetryable(status: Status, error: Error, pausedByUser: Boolean): Boolean { + return status == Status.PAUSED && + (error == Error.WAITING_TO_RETRY || error == Error.PAUSED_UNKNOWN) && + NetworkUtils.isNetworkAvailable(this) && + !pausedByUser + } + + /** + * Handles the case where a paused download was previously queued. + * + * This ensures that the download manager is instructed to resume the download and prevents + * the status from being prematurely updated to "Paused". Instead, the user will see the "Pending" + * state, indicating that the download is in the process of resuming. + * + * @param error The current error state of the download. + * @param downloadRoomEntity The download entity to evaluate. + * @return `true` if the status should be updated, `false` otherwise. + */ + private fun handlePausedAndQueuedDownload( + error: Error, + downloadRoomEntity: DownloadRoomEntity + ): Boolean { + return when (error) { + // When the pause reason is unknown or waiting to retry, and the user + // resumes the download, attempt to resume the download if it was not resumed + // due to some reason. + Error.PAUSED_UNKNOWN, + Error.WAITING_TO_RETRY -> { + resumeDownload(downloadRoomEntity.downloadId) + false } + + // For any other error state, update the status to reflect the current state + // and provide feedback to the user. + else -> true } } + /** + * Handles the case where a paused download is retryable due to network availability. + * + * If the download manager is waiting to retry due to a network error caused by fluctuations, + * this method resumes the download and ensures the status reflects the resumption process. + * + * @param downloadRoomEntity The download entity to evaluate. + * @return `true` to update the status and attempt to resume the download. + */ + private fun handleRetryablePausedDownload(downloadRoomEntity: DownloadRoomEntity): Boolean { + resumeDownload(downloadRoomEntity.downloadId) + return true + } + private fun cancelNotificationAndAssignNewNotificationToForegroundService(downloadId: Long) { downloadNotificationManager.cancelNotification(downloadId.toInt()) updateForegroundNotificationOrStopService() @@ -495,9 +564,53 @@ class DownloadMonitorService : Service() { } private fun getActiveDownloads(): List = - downloadRoomDao.downloadRoomEntity().blockingFirst().filter { - it.status != Status.PAUSED && it.status != Status.CANCELLED - } + downloadRoomDao.downloadRoomEntity().blockingFirst().filter(::isActiveDownload) + + /** + * Determines if a given download is considered active. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is active, false otherwise. + */ + private fun isActiveDownload(download: DownloadRoomEntity): Boolean = + (download.status != Status.PAUSED || isPausedAndRetryable(download)) && + download.status != Status.CANCELLED + + /** + * Checks if a paused download is eligible for retry based on its error status and network conditions. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the paused download is retryable, false otherwise. + */ + private fun isPausedAndRetryable(download: DownloadRoomEntity): Boolean { + return download.status == Status.PAUSED && + ( + isQueuedForWiFiAndConnected(download) || + isQueuedForNetwork(download) || + download.error == Error.WAITING_TO_RETRY || + download.error == Error.PAUSED_UNKNOWN + ) && + NetworkUtils.isNetworkAvailable(this) && + !download.pausedByUser + } + + /** + * Checks if the download is queued for Wi-Fi and the device is connected to Wi-Fi. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is queued for Wi-Fi and connected, false otherwise. + */ + private fun isQueuedForWiFiAndConnected(download: DownloadRoomEntity): Boolean = + download.error == Error.QUEUED_FOR_WIFI && NetworkUtils.isWiFi(this) + + /** + * Checks if the download is waiting for a network connection and the network is now available. + * + * @param download The DownloadRoomEntity to evaluate. + * @return True if the download is waiting for a network and connected, false otherwise. + */ + private fun isQueuedForNetwork(download: DownloadRoomEntity): Boolean = + download.error == Error.WAITING_FOR_NETWORK && NetworkUtils.isNetworkAvailable(this) private fun updateNotification( downloadModel: DownloadModel, @@ -550,7 +663,8 @@ class DownloadMonitorService : Service() { STATUS_PAUSED_BY_APP ) ) { - updateDownloadStatus(downloadId, Status.PAUSED, Error.NONE) + // pass true when user paused the download to not retry the download automatically. + updateDownloadStatus(downloadId, Status.PAUSED, Error.NONE, pausedByUser = true) } } } @@ -565,7 +679,8 @@ class DownloadMonitorService : Service() { STATUS_RUNNING ) ) { - updateDownloadStatus(downloadId, Status.QUEUED, Error.NONE) + // pass false when user resumed the download to proceed with further checks. + updateDownloadStatus(downloadId, Status.QUEUED, Error.NONE, pausedByUser = false) } } } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt index e76caf8847..d9db90c596 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/downloader/model/DownloadModel.kt @@ -33,7 +33,8 @@ data class DownloadModel( var state: Status, var error: Error, var progress: Int, - val book: Book + val book: Book, + var pausedByUser: Boolean ) { val bytesRemaining: Long by lazy { totalSizeOfDownload - bytesDownloaded } val fileNameFromUrl: String by lazy { StorageUtils.getFileNameFromUrl(book.url) } @@ -48,6 +49,7 @@ data class DownloadModel( downloadEntity.status, downloadEntity.error, downloadEntity.progress, - downloadEntity.toBook() + downloadEntity.toBook(), + downloadEntity.pausedByUser ) } diff --git a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt index d3bc7d308c..a028247d2b 100644 --- a/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt +++ b/core/src/main/java/org/kiwix/kiwixmobile/core/search/SearchFragment.kt @@ -192,11 +192,12 @@ class SearchFragment : BaseFragment() { ) } + @Suppress("UnnecessarySafeCall") private fun setupToolbar(view: View) { view.post { - with(requireActivity() as CoreMainActivity) { - setSupportActionBar(view.findViewById(R.id.toolbar)) - supportActionBar?.apply { + with(activity as? CoreMainActivity) { + this?.setSupportActionBar(view.findViewById(R.id.toolbar)) + this?.supportActionBar?.apply { setHomeButtonEnabled(true) title = getString(R.string.menu_search_in_text) } diff --git a/core/src/sharedTestFunctions/java/org/kiwix/sharedFunctions/TestModelFunctions.kt b/core/src/sharedTestFunctions/java/org/kiwix/sharedFunctions/TestModelFunctions.kt index a8fae21a4b..e6955b2757 100644 --- a/core/src/sharedTestFunctions/java/org/kiwix/sharedFunctions/TestModelFunctions.kt +++ b/core/src/sharedTestFunctions/java/org/kiwix/sharedFunctions/TestModelFunctions.kt @@ -58,7 +58,7 @@ fun downloadModel( book: Book = book() ) = DownloadModel( databaseId, downloadId, file, etaInMilliSeconds, bytesDownloaded, totalSizeOfDownload, - status, error, progress, book + status, error, progress, book, false ) fun downloadItem(