Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 2ce05edd authored by Fahim M. Choudhury's avatar Fahim M. Choudhury
Browse files

Merge branch '4235-fix-update-cancellation-behaviour' into 'main'

fix(updates): resolve Update All button's state on update cancellation

See merge request !779
parents dbd24552 95fd8e0d
Loading
Loading
Loading
Loading
Loading
+9 −0
Original line number Diff line number Diff line
@@ -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)
    }
}
+17 −2
Original line number Diff line number Diff line
@@ -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,9 +69,20 @@ class InstallAppWorker @AssistedInject constructor(
        }

        val isPackageUpdate = params.inputData.getBoolean(IS_UPDATE_WORK, false)
        val response = appInstallationFacade.processInstall(fusedDownloadId, isPackageUpdate) { 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) {
            Result.success()
+74 −1
Original line number Diff line number Diff line
@@ -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<AppLoungePackageManager>(relaxed = true)
    private val appPreferencesRepository = mockk<AppPreferencesRepository>(relaxed = true)
    private val fDroidRepository = mockk<FDroidRepository>(relaxed = true)
    private val contentRatingDao = mockk<ContentRatingDao>(relaxed = true)
    private val downloadChannel = mockk<NotificationChannel>(relaxed = true)
    private val updateChannel = mockk<NotificationChannel>(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<String>? = null
    ) = AppInstall(
        id = "121",
        id = id,
        status = Status.AWAITING,
        downloadURLList = downloadUrlList ?: mutableListOf("apk1", "apk2"),
        packageName = packageName ?: "com.unit.test"
+28 −0
Original line number Diff line number Diff line
@@ -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<Context>(relaxed = true)
        val appManagerWrapper = mockk<AppManager>(relaxed = true)
        val downloadManager = mockk<DownloadManager>(relaxed = true)
        val storageNotificationManager = mockk<StorageNotificationManager>(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()) }
    }
}
+22 −0
Original line number Diff line number Diff line
@@ -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()) }
    }
}
Loading