diff --git a/app/src/main/java/foundation/e/apps/data/install/core/AppInstallationFacade.kt b/app/src/main/java/foundation/e/apps/data/install/core/AppInstallationFacade.kt index e4b9e287b2ff42b5d50df9ef8f0b6fa38710ed07..22b5de1d65402f9a9258d1f86040be08fd0091c6 100644 --- a/app/src/main/java/foundation/e/apps/data/install/core/AppInstallationFacade.kt +++ b/app/src/main/java/foundation/e/apps/data/install/core/AppInstallationFacade.kt @@ -104,4 +104,13 @@ class AppInstallationFacade @Inject constructor( } } } + + suspend fun cancelInstall(downloadId: String) { + if (downloadId.isBlank()) { + error("App download ID can't be blank.") + } + + val appInstall = appManager.getDownloadById(AppInstall(id = downloadId)) ?: return + appManager.cancelDownload(appInstall) + } } diff --git a/app/src/main/java/foundation/e/apps/data/install/workmanager/InstallAppWorker.kt b/app/src/main/java/foundation/e/apps/data/install/workmanager/InstallAppWorker.kt index b095fb55f5d8740fe3f28a7623ff5500d565a2bc..8ffe5f4fd2da637f858f2a926686da2cae9acd46 100644 --- a/app/src/main/java/foundation/e/apps/data/install/workmanager/InstallAppWorker.kt +++ b/app/src/main/java/foundation/e/apps/data/install/workmanager/InstallAppWorker.kt @@ -33,6 +33,10 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import foundation.e.apps.R import foundation.e.apps.data.install.core.AppInstallationFacade +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.withContext +import timber.log.Timber import java.util.concurrent.atomic.AtomicInteger @HiltWorker @@ -65,8 +69,19 @@ class InstallAppWorker @AssistedInject constructor( } val isPackageUpdate = params.inputData.getBoolean(IS_UPDATE_WORK, false) - val response = appInstallationFacade.processInstall(fusedDownloadId, isPackageUpdate) { title -> - setForeground(createForegroundInfo("${context.getString(R.string.installing)} $title")) + val response = try { + appInstallationFacade.processInstall(fusedDownloadId, isPackageUpdate) { title -> + setForeground(createForegroundInfo("${context.getString(R.string.installing)} $title")) + } + } catch (exception: CancellationException) { + runCatching { + withContext(NonCancellable) { + appInstallationFacade.cancelInstall(fusedDownloadId) + } + }.onFailure { throwable -> + Timber.w(throwable, "Failed to clean up cancelled install %s", fusedDownloadId) + } + throw exception } return if (response.isSuccess) { diff --git a/app/src/test/java/foundation/e/apps/data/install/AppManagerImplTest.kt b/app/src/test/java/foundation/e/apps/data/install/AppManagerImplTest.kt index 6c2275492ca488de7edfc89eb37106e5704484cc..9ebe81c8e8eefed6767c1739c74d6b4e9c742570 100644 --- a/app/src/test/java/foundation/e/apps/data/install/AppManagerImplTest.kt +++ b/app/src/test/java/foundation/e/apps/data/install/AppManagerImplTest.kt @@ -35,6 +35,7 @@ import foundation.e.apps.data.install.workmanager.InstallWorkManager import foundation.e.apps.data.installation.model.AppInstall import foundation.e.apps.data.installation.model.SharedLib import foundation.e.apps.data.installation.repository.AppInstallRepository +import foundation.e.apps.data.parentalcontrol.ContentRatingDao import foundation.e.apps.domain.model.install.Status import foundation.e.apps.domain.preferences.AppPreferencesRepository import foundation.e.apps.installProcessor.FakeAppInstallDAO @@ -73,6 +74,7 @@ class AppManagerImplTest { private val appLoungePackageManager = mockk(relaxed = true) private val appPreferencesRepository = mockk(relaxed = true) private val fDroidRepository = mockk(relaxed = true) + private val contentRatingDao = mockk(relaxed = true) private val downloadChannel = mockk(relaxed = true) private val updateChannel = mockk(relaxed = true) private val context: Context = ApplicationProvider.getApplicationContext() @@ -99,6 +101,7 @@ class AppManagerImplTest { context, fDroidRepository, ) + appManager.contentRatingDao = contentRatingDao appManager.appPreferencesRepository = appPreferencesRepository } @@ -267,6 +270,75 @@ class AppManagerImplTest { assertTrue(isSuccessful) } + @Test + fun cancelDownload_removesTrackedDownloadsAndDeletesPendingAppInstall() = runTest { + val appInstall = createAppInstall().apply { + status = Status.DOWNLOADING + downloadIdMap = mutableMapOf(11L to false, 12L to true) + } + appInstallDAO.addDownload(appInstall) + createApk(appInstall.packageName, "cached.apk") + DownloadProgressLD.setDownloadId(11L) + + appManager.cancelDownload(appInstall) + + verify { downloadManager.remove(11L) } + verify { downloadManager.remove(12L) } + assertThat(appInstallDAO.getDownloadById(appInstall.id)).isNull() + assertThat(File(tempDir, appInstall.packageName).exists()).isFalse() + assertThat(DownloadProgressLD.downloadId).isEmpty() + } + + @Test + fun getFusedDownload_returnsEmptyAfterCancelRemovesTrackedIds() = runTest { + val appInstall = createAppInstall().apply { + status = Status.INSTALLING + downloadIdMap = mutableMapOf(21L to true) + } + appInstallDAO.addDownload(appInstall) + + appManager.cancelDownload(appInstall) + + assertThat(appManager.getFusedDownload(21L).id).isEmpty() + } + + @Test + fun cancelDownload_keepsOtherActiveAppsUntouched() = runTest { + val canceledApp = createAppInstall(packageName = "com.example.cancel").apply { + status = Status.DOWNLOADING + downloadIdMap = mutableMapOf(31L to false) + } + val otherApp = createAppInstall(id = "122", packageName = "com.example.other").apply { + status = Status.INSTALLING + downloadIdMap = mutableMapOf(41L to true) + } + appInstallDAO.addDownload(canceledApp) + appInstallDAO.addDownload(otherApp) + + appManager.cancelDownload(canceledApp) + + assertThat(appInstallDAO.getDownloadById(canceledApp.id)).isNull() + assertThat(appInstallDAO.getDownloadById(otherApp.id)?.packageName) + .isEqualTo(otherApp.packageName) + verify(exactly = 1) { downloadManager.remove(31L) } + verify(exactly = 0) { downloadManager.remove(41L) } + } + + @Test + fun cancelDownload_isIdempotentWhenAppAlreadyRemoved() = runTest { + val appInstall = createAppInstall(packageName = "com.example.repeat").apply { + status = Status.DOWNLOADING + downloadIdMap = mutableMapOf(51L to false) + } + appInstallDAO.addDownload(appInstall) + + appManager.cancelDownload(appInstall) + appManager.cancelDownload(appInstall) + + assertThat(appInstallDAO.getDownloadById(appInstall.id)).isNull() + verify(exactly = 2) { downloadManager.remove(51L) } + } + private fun initTest(hasAnyExistingWork: Boolean = false): AppInstall { mockkObject(InstallWorkManager) every { InstallWorkManager.checkWorkIsAlreadyAvailable(any(), any()) } returns hasAnyExistingWork @@ -274,10 +346,11 @@ class AppManagerImplTest { } private fun createAppInstall( + id: String = "121", packageName: String? = null, downloadUrlList: MutableList? = null ) = AppInstall( - id = "121", + id = id, status = Status.AWAITING, downloadURLList = downloadUrlList ?: mutableListOf("apk1", "apk2"), packageName = packageName ?: "com.unit.test" diff --git a/app/src/test/java/foundation/e/apps/data/install/download/DownloadManagerUtilsTest.kt b/app/src/test/java/foundation/e/apps/data/install/download/DownloadManagerUtilsTest.kt index 0e5f347a9797554e374df3f45b8b562393402a0d..2c9c5d7107a330b218d937e0b9e16f880c8a7dd1 100644 --- a/app/src/test/java/foundation/e/apps/data/install/download/DownloadManagerUtilsTest.kt +++ b/app/src/test/java/foundation/e/apps/data/install/download/DownloadManagerUtilsTest.kt @@ -31,6 +31,7 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import io.mockk.verify +import kotlinx.coroutines.DelicateCoroutinesApi import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runCurrent @@ -97,4 +98,31 @@ class DownloadManagerUtilsTest { verify(exactly = 1) { appManagerWrapper.moveOBBFilesToOBBDirectory(appInstall) } coVerify(atLeast = 3) { appManagerWrapper.updateAppInstall(appInstall) } } + + @OptIn(DelicateCoroutinesApi::class) + @Test + fun updateDownloadStatus_ignoresLateCallbackWhenCanceledAppIsAlreadyRemoved() = runTest { + val context = mockk(relaxed = true) + val appManagerWrapper = mockk(relaxed = true) + val downloadManager = mockk(relaxed = true) + val storageNotificationManager = mockk(relaxed = true) + val utils = DownloadManagerUtils( + context, + appManagerWrapper, + downloadManager, + storageNotificationManager, + backgroundScope + ) + + coEvery { appManagerWrapper.getFusedDownload(99L, any()) } returns AppInstall() + + utils.updateDownloadStatus(99L) + advanceTimeBy(1500) + runCurrent() + + coVerify { appManagerWrapper.getFusedDownload(99L, any()) } + coVerify(exactly = 0) { appManagerWrapper.updateAppInstall(any()) } + coVerify(exactly = 0) { appManagerWrapper.reportInstallationIssue(any()) } + coVerify(exactly = 0) { appManagerWrapper.cancelDownload(any(), any()) } + } } diff --git a/app/src/test/java/foundation/e/apps/installProcessor/AppInstallationFacadeTest.kt b/app/src/test/java/foundation/e/apps/installProcessor/AppInstallationFacadeTest.kt index 74f6b894e2ba815b1dd184968a5c8009143d246f..af538bf6149ebefac657a3813701596ac6852a9e 100644 --- a/app/src/test/java/foundation/e/apps/installProcessor/AppInstallationFacadeTest.kt +++ b/app/src/test/java/foundation/e/apps/installProcessor/AppInstallationFacadeTest.kt @@ -131,4 +131,26 @@ class AppInstallationFacadeTest { assertEquals(ResultStatus.OK, result.getOrNull()) coVerify { installationProcessor.processInstall("123", false, any()) } } + + @Test + fun cancelInstall_cancelsExistingAppInstall() = runTest { + val appInstall = AppInstall(id = "123", packageName = "com.example.app") + coEvery { appManager.getDownloadById(match { it.id == "123" }) } returns appInstall + + appInstallationFacade.cancelInstall("123") + + coVerify { appManager.getDownloadById(match { it.id == "123" }) } + coVerify { appManager.cancelDownload(appInstall) } + coVerify(exactly = 0) { appManager.reportInstallationIssue(any()) } + } + + @Test + fun cancelInstall_ignoresMissingAppInstall() = runTest { + coEvery { appManager.getDownloadById(match { it.id == "missing" }) } returns null + + appInstallationFacade.cancelInstall("missing") + + coVerify { appManager.getDownloadById(match { it.id == "missing" }) } + coVerify(exactly = 0) { appManager.cancelDownload(any()) } + } } diff --git a/app/src/test/java/foundation/e/apps/installProcessor/InstallAppWorkerTest.kt b/app/src/test/java/foundation/e/apps/installProcessor/InstallAppWorkerTest.kt index 953424eb5aae7b7fae24e2bd5d3165923dae8108..41833f0b0b689ad87a30b63e8334698b7876fec8 100644 --- a/app/src/test/java/foundation/e/apps/installProcessor/InstallAppWorkerTest.kt +++ b/app/src/test/java/foundation/e/apps/installProcessor/InstallAppWorkerTest.kt @@ -30,12 +30,15 @@ import io.mockk.coVerify import io.mockk.every import io.mockk.mockk import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.delay import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner import org.robolectric.annotation.Config +import kotlin.test.assertFailsWith @RunWith(RobolectricTestRunner::class) @Config(sdk = [Build.VERSION_CODES.R]) @@ -94,6 +97,31 @@ class InstallAppWorkerTest { coVerify { appInstallationFacade.processInstall("123", false, any()) } } + @Test + fun doWork_cleansUpTargetedInstallWhenCancelled() = runTest { + coEvery { + appInstallationFacade.processInstall("123", false, any()) + } throws CancellationException("cancelled") + var cleanupCompleted = false + coEvery { appInstallationFacade.cancelInstall("123") } coAnswers { + delay(1) + cleanupCompleted = true + } + val worker = createWorker( + Data.Builder() + .putString(InstallAppWorker.INPUT_DATA_FUSED_DOWNLOAD, "123") + .putBoolean(InstallAppWorker.IS_UPDATE_WORK, false) + .build() + ) + + assertFailsWith { + worker.doWork() + } + + coVerify { appInstallationFacade.cancelInstall("123") } + assertThat(cleanupCompleted).isTrue() + } + private fun createWorker(inputData: Data): InstallAppWorker { val params = mockk(relaxed = true) every { params.inputData } returns inputData