From 1a01f96f08475599ef1b152c5933563bdb3c0116 Mon Sep 17 00:00:00 2001 From: Aayush Gupta Date: Wed, 21 Feb 2024 18:39:17 +0530 Subject: [PATCH] DownloadWorkerUtil: Rework cancelAll logic to only cancel request downloads We don't want to cancel all downloads when user requests to cancel only updates. Also, update the download's status to cancelled atomically. Signed-off-by: Aayush Gupta --- .../store/data/room/download/DownloadDao.kt | 11 ++--- .../aurora/store/data/work/DownloadWorker.kt | 40 +++++++++---------- .../aurora/store/util/DownloadWorkerUtil.kt | 14 +++---- .../store/viewmodel/all/UpdatesViewModel.kt | 2 +- 4 files changed, 31 insertions(+), 36 deletions(-) diff --git a/app/src/main/java/com/aurora/store/data/room/download/DownloadDao.kt b/app/src/main/java/com/aurora/store/data/room/download/DownloadDao.kt index 1412d6ac1..de2b795d0 100644 --- a/app/src/main/java/com/aurora/store/data/room/download/DownloadDao.kt +++ b/app/src/main/java/com/aurora/store/data/room/download/DownloadDao.kt @@ -4,7 +4,6 @@ import androidx.room.Dao import androidx.room.Insert import androidx.room.OnConflictStrategy import androidx.room.Query -import androidx.room.Update import com.aurora.gplayapi.data.models.File import com.aurora.store.data.model.DownloadStatus import kotlinx.coroutines.flow.Flow @@ -15,9 +14,6 @@ interface DownloadDao { @Insert(onConflict = OnConflictStrategy.REPLACE) suspend fun insert(download: Download) - @Update - suspend fun update(download: Download) - @Query("UPDATE download SET downloadStatus=:downloadStatus WHERE packageName=:packageName") suspend fun updateStatus(packageName: String, downloadStatus: DownloadStatus) @@ -30,13 +26,12 @@ interface DownloadDao { @Query( """ UPDATE download - SET downloadStatus=:downloadStatus, progress=:progress, speed=:speed, timeRemaining=:timeRemaining + SET progress=:progress, speed=:speed, timeRemaining=:timeRemaining WHERE packageName=:packageName """ ) - suspend fun updateStatusProgress( + suspend fun updateProgress( packageName: String, - downloadStatus: DownloadStatus, progress: Int, speed: Long, timeRemaining: Long @@ -46,7 +41,7 @@ interface DownloadDao { fun downloads(): Flow> @Query("SELECT * FROM download WHERE packageName = :packageName") - suspend fun getDownload(packageName: String): Download? + suspend fun getDownload(packageName: String): Download @Query("DELETE FROM download WHERE packageName = :packageName") suspend fun delete(packageName: String) diff --git a/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt b/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt index 4ca6b1b2c..da9992e13 100644 --- a/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt +++ b/app/src/main/java/com/aurora/store/data/work/DownloadWorker.kt @@ -43,7 +43,6 @@ import java.io.File import java.net.URL import java.security.DigestInputStream import java.security.MessageDigest -import kotlinx.coroutines.MainScope import kotlinx.coroutines.NonCancellable import kotlin.properties.Delegates import com.aurora.gplayapi.data.models.File as GPlayFile @@ -136,6 +135,7 @@ class DownloadWorker @AssistedInject constructor( downloadDao.updateSharedLibs(download.packageName, download.sharedLibs) // Download and verify all files exists + notifyStatus(DownloadStatus.DOWNLOADING) requestList.forEach { request -> downloading = true runCatching { downloadFile(request); download.downloadedFiles++ } @@ -148,7 +148,8 @@ class DownloadWorker @AssistedInject constructor( } while (downloading) { delay(1000) - if (isStopped) { + val d = downloadDao.getDownload(download.packageName) + if (isStopped || d.downloadStatus == DownloadStatus.CANCELLED) { onFailure() break } @@ -180,13 +181,12 @@ class DownloadWorker @AssistedInject constructor( withContext(NonCancellable) { Log.i(TAG, "Cleaning up!") val cancelReasons = listOf(STOP_REASON_USER, STOP_REASON_CANCELLED_BY_APP) - val status = if (isSAndAbove() && stopReason in cancelReasons) { - DownloadStatus.CANCELLED + if (isSAndAbove() && stopReason in cancelReasons) { + notifyStatus(DownloadStatus.CANCELLED) } else { - DownloadStatus.FAILED + notifyStatus(DownloadStatus.FAILED) } - notifyStatus(status) PathUtil.getAppDownloadDir(appContext, download.packageName, download.versionCode) .deleteRecursively() with(appContext.getSystemService(Service.NOTIFICATION_SERVICE) as NotificationManager) { @@ -282,14 +282,12 @@ class DownloadWorker @AssistedInject constructor( } download.apply { - this.downloadStatus = DownloadStatus.DOWNLOADING this.progress = progress this.speed = downloadInfo.speed this.timeRemaining = bytesRemaining / speed * 1000 } - downloadDao.updateStatusProgress( + downloadDao.updateProgress( download.packageName, - download.downloadStatus, download.progress, download.speed, download.timeRemaining @@ -315,18 +313,20 @@ class DownloadWorker @AssistedInject constructor( } private suspend fun notifyStatus(status: DownloadStatus, dID: Int = -1) { - // Update database for all status except downloading which is handled onProgress - if (status != DownloadStatus.DOWNLOADING) { - download.downloadStatus = status - if (download.downloadStatus == DownloadStatus.COMPLETED) { - download.progress = 100 - downloadDao.updateStatusProgress(download.packageName, status, 100, 0, 0) - } else { - downloadDao.updateStatus(download.packageName, status) - } - } + // Update status in database + download.downloadStatus = status + downloadDao.updateStatus(download.packageName, status) - if (status == DownloadStatus.CANCELLED) return + when (status) { + DownloadStatus.DOWNLOADING, DownloadStatus.CANCELLED -> return + DownloadStatus.COMPLETED -> { + // Mark progress as 100 manually to avoid race conditions + download.progress = 100 + downloadDao.updateProgress(download.packageName, 100, 0, 0) + } + + else -> {} + } val notification = NotificationUtil.getDownloadNotification(appContext, download, id, icon) val notificationID = if (dID != -1) dID else download.packageName.hashCode() diff --git a/app/src/main/java/com/aurora/store/util/DownloadWorkerUtil.kt b/app/src/main/java/com/aurora/store/util/DownloadWorkerUtil.kt index 355605d25..9685ff524 100644 --- a/app/src/main/java/com/aurora/store/util/DownloadWorkerUtil.kt +++ b/app/src/main/java/com/aurora/store/util/DownloadWorkerUtil.kt @@ -107,15 +107,15 @@ class DownloadWorkerUtil @Inject constructor( } } - suspend fun cancelAll(downloads: Boolean = true, updates: Boolean = true) { + suspend fun cancelAll(updatesOnly: Boolean = false) { // Cancel all enqueued downloads first to avoid triggering re-download - downloadsList.value.filter { it.downloadStatus == DownloadStatus.QUEUED }.forEach { - downloadDao.update(it.copy(downloadStatus = DownloadStatus.CANCELLED)) - } + downloadsList.value.filter { it.downloadStatus == DownloadStatus.QUEUED } + .filter { if (updatesOnly) it.isInstalled else true }.forEach { + downloadDao.updateStatus(it.packageName, DownloadStatus.CANCELLED) + } - val workManager = WorkManager.getInstance(context) - if (downloads) workManager.cancelAllWorkByTag(DOWNLOAD_APP) - if (updates) workManager.cancelAllWorkByTag(DOWNLOAD_UPDATE) + WorkManager.getInstance(context) + .cancelAllWorkByTag(if (updatesOnly) DOWNLOAD_UPDATE else DOWNLOAD_APP) } private fun trigger(download: Download) { diff --git a/app/src/main/java/com/aurora/store/viewmodel/all/UpdatesViewModel.kt b/app/src/main/java/com/aurora/store/viewmodel/all/UpdatesViewModel.kt index 73dc93cfa..6e63708ce 100644 --- a/app/src/main/java/com/aurora/store/viewmodel/all/UpdatesViewModel.kt +++ b/app/src/main/java/com/aurora/store/viewmodel/all/UpdatesViewModel.kt @@ -96,6 +96,6 @@ class UpdatesViewModel @Inject constructor( } fun cancelAll() { - viewModelScope.launch { downloadWorkerUtil.cancelAll(downloads = false) } + viewModelScope.launch { downloadWorkerUtil.cancelAll(true) } } }