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

Verified Commit 95fd8e0d authored by Fahim M. Choudhury's avatar Fahim M. Choudhury
Browse files

fix(install): clean up canceled installs 

Cancel tracked downloads when install work is interrupted so stale install records do not survive worker cancellation. Ignore late download callbacks once the canceled app install has already been removed.
parent dbd24552

app/src/main/java/foundation/e/apps/data/install/core/AppInstallationFacade.kt

+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)

    }

}

app/src/main/java/foundation/e/apps/data/install/workmanager/InstallAppWorker.kt

+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()

app/src/test/java/foundation/e/apps/data/install/AppManagerImplTest.kt

+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"

app/src/test/java/foundation/e/apps/data/install/download/DownloadManagerUtilsTest.kt

+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()) }

    }

}

app/src/test/java/foundation/e/apps/installProcessor/AppInstallationFacadeTest.kt

+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()) }

    }

}