Skip to content

Commit

Permalink
Improved handling of scenarios where download progress was interrupte…
Browse files Browse the repository at this point in the history 
…d due to network errors (e.g., network fluctuations). The application now correctly retrieves download progress from the DownloadManager and, if necessary, automatically resumes paused downloads without requiring user intervention.

* Downloads paused due to network errors like "Waiting to Retry" are now resumed automatically when the network becomes available.
* For downloads configured to only proceed on Wi-Fi, the application will resume progress when a Wi-Fi connection is re-established. Similarly, downloads queued for mobile networks will resume when the mobile network reconnects.
  • Loading branch information
@MohitMaliDeveloper @MohitMaliFtechiz
MohitMaliDeveloper authored and MohitMaliFtechiz committed Dec 11, 2024
1 parent d56d722 commit 4859313
Show file tree
Hide file tree
Showing 2 changed files with 178 additions and 50 deletions.
54 changes: 46 additions & 8 deletions ...main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadManagerMonitor.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -62,10 +63,6 @@ class DownloadManagerMonitor @Inject constructor(
// Download Manager takes some time to update the download status.
// In such cases, the foreground service may stop prematurely due to
// a lack of active downloads during this update delay.
Log.e(
"DOWNLOAD_MONITOR",
"startMonitoringDownloads: monitor ${shouldStartDownloadMonitorService()}"
)
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.
Expand Down Expand Up @@ -98,10 +95,51 @@ class DownloadManagerMonitor @Inject constructor(
!context.isServiceRunning(DownloadMonitorService::class.java)

private fun getActiveDownloads(): List<DownloadRoomEntity> =
downloadRoomDao.downloadRoomEntity().blockingFirst().filter {
(it.status != Status.PAUSED || it.error == Error.WAITING_TO_RETRY) &&
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
) &&
NetworkUtils.isNetworkAvailable(context)
}

/**
* 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) {
Expand Down
174 changes: 132 additions & 42 deletions ...main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -137,7 +138,6 @@ class DownloadMonitorService : Service() {
{
try {
synchronized(lock) {
Log.e("DOWNLOAD_MONITOR", "startMonitoringDownloads: service")
if (downloadRoomDao.downloads().blockingFirst().isNotEmpty()) {
checkDownloads()
} else {
Expand Down Expand Up @@ -386,11 +386,6 @@ class DownloadMonitorService : Service() {
) {
synchronized(lock) {
updater.onNext {
Log.e(
"DOWNLOAD_MONITOR",
"updateDownloadStatus: status = $status\n" +
"error = $error\n bytesDownloaded = $bytesDownloaded"
)
downloadRoomDao.getEntityForDownloadId(downloadId)?.let { downloadEntity ->
if (shouldUpdateDownloadStatus(downloadEntity)) {
val downloadModel = DownloadModel(downloadEntity).apply {
Expand Down Expand Up @@ -426,10 +421,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.
Expand All @@ -442,38 +436,94 @@ 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@shouldUpdateDownloadStatus when {
// Check if the download is paused and was previously queued.
isPausedAndQueued(status, downloadRoomEntity) ->
handlePausedAndQueuedDownload(error, downloadRoomEntity)

// 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
}
} else {
true
// Check if the download is paused and retryable due to network availability.
isPausedAndRetryable(status, error) -> handleRetryablePausedDownload(downloadRoomEntity)

// 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.
* @return `true` if the download is paused and retryable, `false` otherwise.
*/
private fun isPausedAndRetryable(status: Status, error: Error): Boolean {
return status == Status.PAUSED &&
error == Error.WAITING_TO_RETRY &&
NetworkUtils.isNetworkAvailable(this)
}

/**
* 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

Check warning on line 509 in core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt

View check run for this annotation

Codecov / codecov/patch

core/src/main/java/org/kiwix/kiwixmobile/core/downloader/downloadManager/DownloadMonitorService.kt#L509 

Added line #L509 was not covered by tests
}
}

/**
* 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()
Expand Down Expand Up @@ -501,10 +551,51 @@ class DownloadMonitorService : Service() {
}

private fun getActiveDownloads(): List<DownloadRoomEntity> =
downloadRoomDao.downloadRoomEntity().blockingFirst().filter {
(it.status != Status.PAUSED || it.error == Error.WAITING_TO_RETRY) &&
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
) &&
NetworkUtils.isNetworkAvailable(this)
}

/**
* 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,
Expand Down Expand Up @@ -619,7 +710,6 @@ class DownloadMonitorService : Service() {
}

private fun stopForegroundServiceForDownloads() {
Log.e("DOWNLOAD_MONITOR", "stopForegroundServiceForDownloads: service")
foreGroundServiceInformation = true to DEFAULT_INT_VALUE
monitoringDisposable?.dispose()
stopForeground(STOP_FOREGROUND_REMOVE)
Expand Down

0 comments on commit 4859313

Please sign in to comment.