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

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

fix(update): hide non-owned updates during discovery

Filter update candidates through the ownership eligibility checker before they reach UI and update counts.

Log hidden packages so developers can understand why some updates are suppressed.
parent 20a20728
Loading
Loading
Loading
Loading
+15 −12
Original line number Diff line number Diff line
@@ -351,26 +351,26 @@ Hide blocked apps before they reach update UI, update counts, or update-all flow
### Tasks

#### Task 3.1 - Integrate the checker into `UpdatesManagerImpl`
- [ ] Keep existing source/backend selection logic separate from eligibility logic.
- [ ] After candidate update apps are gathered, run the eligibility checker on each installed package.
- [ ] Keep only eligible apps in the list returned to UI.
- [x] Keep existing source/backend selection logic separate from eligibility logic.
- [x] After candidate update apps are gathered, run the eligibility checker on each installed package.
- [x] Keep only eligible apps in the list returned to UI.

#### Task 3.2 - Add structured logging for hidden apps
- [ ] Log package name and reason code when an app is hidden.
- [ ] Log install-source fields needed for developer debugging:
- [x] Log package name and reason code when an app is hidden.
- [x] Log install-source fields needed for developer debugging:
  - `initiatingPackageName`
  - `installingPackageName`
  - `updateOwnerPackageName`
- [ ] Avoid logging unrelated private data.
- [x] Avoid logging unrelated private data.

#### Task 3.3 - Make future UX switching easy
- [ ] Extract the final decision step into a small method or helper.
- [ ] Add a comment that future product work may replace hiding with disabled presentation.
- [ ] Do not add disabled UI in this phase.
- [x] Extract the final decision step into a small method or helper.
- [x] Add a comment that future product work may replace hiding with disabled presentation.
- [x] Do not add disabled UI in this phase.

#### Task 3.4 - Update tests
- [ ] Extend `UpdateManagerImptTest.kt`.
- [ ] Cover:
- [x] Extend `UpdateManagerImptTest.kt`.
- [x] Cover:
  - eligible apps remain visible
  - Aurora-owned apps are hidden
  - F-Droid-owned apps are hidden
@@ -403,7 +403,10 @@ Body:

### Iteration Notes

- Add notes here after the phase is done.
- `UpdatesManagerImpl` now applies ownership filtering only after source-specific discovery and faulty-app filtering, so backend/source selection remains separate from ownership policy.
- Hidden updates are logged with package name, reason code, and the three install-source fields needed to debug ownership decisions.
- The final visibility decision now lives in a small helper with a comment that future product work can swap hidden items for disabled presentation without rewriting discovery flow.
- `UpdateManagerImptTest` now covers eligible visibility, Aurora/F-Droid/plain adb/null-installer hiding, legacy FakeStore visibility, and the reduced actionable update count when blocked items are removed.

## Phase 4 - Apply Eligibility In Execution Paths

+39 −2
Original line number Diff line number Diff line
@@ -32,6 +32,8 @@ import foundation.e.apps.data.fdroid.FDroidRepository
import foundation.e.apps.data.gitlab.SystemAppsUpdatesRepository
import foundation.e.apps.data.handleNetworkResult
import foundation.e.apps.data.install.pkg.AppLoungePackageManager
import foundation.e.apps.data.installation.model.UpdateEligibilityResult
import foundation.e.apps.data.installation.port.UpdateEligibilityChecker
import foundation.e.apps.domain.model.install.Status
import foundation.e.apps.domain.preferences.AppPreferencesRepository
import kotlinx.coroutines.Dispatchers
@@ -48,6 +50,7 @@ class UpdatesManagerImpl @Inject constructor(
    private val appPreferencesRepository: AppPreferencesRepository,
    private val fDroidRepository: FDroidRepository,
    private val blockedAppRepository: BlockedAppRepository,
    private val updateEligibilityChecker: UpdateEligibilityChecker,
    private val systemAppsUpdatesRepository: SystemAppsUpdatesRepository,
) {
    companion object {
@@ -117,8 +120,9 @@ class UpdatesManagerImpl @Inject constructor(

        val systemApps = getSystemAppUpdates()
        val nonFaultyUpdateList = faultyAppRepository.removeFaultyApps(updateList)
        val eligibleUpdateList = filterVisibleUpdates(nonFaultyUpdateList)

        addSystemApps(updateList, nonFaultyUpdateList, systemApps)
        addSystemApps(updateList, eligibleUpdateList, systemApps)

        return Pair(updateList, status)
    }
@@ -154,12 +158,45 @@ class UpdatesManagerImpl @Inject constructor(

        val systemApps = getSystemAppUpdates()
        val nonFaultyUpdateList = faultyAppRepository.removeFaultyApps(updateList)
        val eligibleUpdateList = filterVisibleUpdates(nonFaultyUpdateList)

        addSystemApps(updateList, nonFaultyUpdateList, systemApps)
        addSystemApps(updateList, eligibleUpdateList, systemApps)

        return Pair(updateList, status)
    }

    private fun filterVisibleUpdates(candidateUpdates: List<Application>): List<Application> {
        return candidateUpdates.filter(::shouldShowDiscoveredUpdate)
    }

    private fun shouldShowDiscoveredUpdate(application: Application): Boolean {
        val eligibilityResult = updateEligibilityChecker.getUpdateEligibility(application.package_name)
        if (eligibilityResult.isEligible) {
            return true
        }

        logHiddenUpdate(application.package_name, eligibilityResult)

        // Future product work may replace hiding with disabled presentation plus a reason.
        return false
    }

    private fun logHiddenUpdate(
        packageName: String,
        eligibilityResult: UpdateEligibilityResult,
    ) {
        val installSourceDetails = eligibilityResult.installSourceDetails
        Timber.w(
            "Hide update by ownership policy: packageName=%s reasonCode=%s " +
                "initiatingPackageName=%s installingPackageName=%s updateOwnerPackageName=%s",
            packageName,
            eligibilityResult.reasonCode,
            installSourceDetails.initiatingPackageName,
            installSourceDetails.installingPackageName,
            installSourceDetails.updateOwnerPackageName,
        )
    }

    private suspend fun getSystemAppUpdates(): List<Application> {
        val systemApps = mutableListOf<Application>()
        getUpdatesFromApi({
+163 −0
Original line number Diff line number Diff line
@@ -30,12 +30,16 @@ import foundation.e.apps.domain.model.install.Status
import foundation.e.apps.data.faultyApps.FaultyAppRepository
import foundation.e.apps.data.fdroid.FDroidRepository
import foundation.e.apps.data.gitlab.SystemAppsUpdatesRepository
import foundation.e.apps.data.installation.model.InstallSourceDetails
import foundation.e.apps.data.installation.model.UpdateEligibilityResult
import foundation.e.apps.data.installation.port.UpdateEligibilityChecker
import foundation.e.apps.data.updates.UpdatesManagerImpl
import foundation.e.apps.util.MainCoroutineRule
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.test.runTest
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertTrue
import org.junit.Before
import org.junit.Rule
import org.junit.Test
@@ -81,6 +85,8 @@ class UpdateManagerImptTest {
    @Mock
    private lateinit var systemAppsUpdatesRepository: SystemAppsUpdatesRepository

    private lateinit var updateEligibilityChecker: FakeUpdateEligibilityChecker

    val authData = AuthData("e@e.email", "AtadyMsIAtadyM")

    @Before
@@ -89,6 +95,7 @@ class UpdateManagerImptTest {
        faultyAppRepository = FaultyAppRepository(FakeFaultyAppDao())
        preferenceModule = FakeAppLoungePreference()
        pkgManagerModule = FakeAppLoungePackageManager(context, getGplayApps())
        updateEligibilityChecker = FakeUpdateEligibilityChecker()
        updatesManagerImpl = UpdatesManagerImpl(
            context,
            pkgManagerModule,
@@ -97,6 +104,7 @@ class UpdateManagerImptTest {
            preferenceModule,
            fDroidRepository,
            blockedAppRepository,
            updateEligibilityChecker,
            systemAppsUpdatesRepository,
        )
    }
@@ -132,6 +140,128 @@ class UpdateManagerImptTest {
        assertEquals("fetchUpdate", 3, updateResult.first.size)
    }

    @Test
    fun getUpdates_keepsEligibleAppsVisible() = runTest {
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertTrue(updateResult.first.any { it.package_name == "foundation.e.demoone" })
        assertTrue(updateResult.first.any { it.package_name == "foundation.e.demothree" })
    }

    @Test
    fun getUpdates_hidesAuroraOwnedApps() = runTest {
        updateEligibilityChecker.block(
            packageName = "foundation.e.demoone",
            reasonCode = UpdateEligibilityResult.BLOCK_OTHER_UPDATE_OWNER,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertFalse(updateResult.first.any { it.package_name == "foundation.e.demoone" })
    }

    @Test
    fun getUpdates_hidesFDroidOwnedApps() = runTest {
        updateEligibilityChecker.block(
            packageName = "foundation.e.demothree",
            reasonCode = UpdateEligibilityResult.BLOCK_OTHER_INSTALLER,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertFalse(updateResult.first.any { it.package_name == "foundation.e.demothree" })
    }

    @Test
    fun getUpdates_hidesPlainAdbInstalledApps() = runTest {
        updateEligibilityChecker.block(
            packageName = "foundation.e.demoone",
            reasonCode = UpdateEligibilityResult.BLOCK_OTHER_INSTALLER,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertFalse(updateResult.first.any { it.package_name == "foundation.e.demoone" })
    }

    @Test
    fun getUpdates_hidesAppLoungeInitiatedNullInstallerApps() = runTest {
        updateEligibilityChecker.block(
            packageName = "foundation.e.demoone",
            reasonCode = UpdateEligibilityResult.BLOCK_APP_LOUNGE_INITIATED_NULL_INSTALLER,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertFalse(updateResult.first.any { it.package_name == "foundation.e.demoone" })
    }

    @Test
    fun getUpdates_keepsLegacyFakeStoreAppsVisible() = runTest {
        updateEligibilityChecker.allow(
            packageName = "foundation.e.demoone",
            reasonCode = UpdateEligibilityResult.ALLOW_LEGACY_FAKESTORE_MIGRATION,
            isLegacyMigrationCase = true,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertTrue(updateResult.first.any { it.package_name == "foundation.e.demoone" })
    }

    @Test
    fun getUpdates_doesNotCountHiddenAppsAsActionableUpdates() = runTest {
        updateEligibilityChecker.block(
            packageName = "foundation.e.demoone",
            reasonCode = UpdateEligibilityResult.BLOCK_OTHER_UPDATE_OWNER,
        )
        updateEligibilityChecker.block(
            packageName = "foundation.e.demothree",
            reasonCode = UpdateEligibilityResult.BLOCK_OTHER_INSTALLER,
        )
        setupMockingForFetchingUpdates(
            Pair(getOpenSourceApps(Status.UPDATABLE), ResultStatus.OK),
            Pair(getGplayApps(Status.UPDATABLE), ResultStatus.OK),
            getSystemApps(),
        )

        val updateResult = updatesManagerImpl.getUpdates()

        assertEquals(1, updateResult.first.size)
        assertEquals(Source.SYSTEM_APP, updateResult.first.first().source)
    }

    private fun getGplayApps(status: Status = Status.UPDATABLE) = mutableListOf<Application>(
        Application(
            _id = "111",
@@ -420,4 +550,37 @@ class UpdateManagerImptTest {
        Mockito.`when`(systemAppsUpdatesRepository.getSystemUpdates())
            .thenReturn(systemAppUpdates)
    }

    private class FakeUpdateEligibilityChecker : UpdateEligibilityChecker {
        private val overrides = mutableMapOf<String, UpdateEligibilityResult>()

        override fun getUpdateEligibility(packageName: String): UpdateEligibilityResult {
            return overrides[packageName] ?: UpdateEligibilityResult(
                isEligible = true,
                reasonCode = UpdateEligibilityResult.ALLOW_INSTALLER_OF_RECORD,
                installSourceDetails = InstallSourceDetails(readSucceeded = true),
            )
        }

        fun allow(
            packageName: String,
            reasonCode: String = UpdateEligibilityResult.ALLOW_INSTALLER_OF_RECORD,
            isLegacyMigrationCase: Boolean = false,
        ) {
            overrides[packageName] = UpdateEligibilityResult(
                isEligible = true,
                reasonCode = reasonCode,
                installSourceDetails = InstallSourceDetails(readSucceeded = true),
                isLegacyMigrationCase = isLegacyMigrationCase,
            )
        }

        fun block(packageName: String, reasonCode: String) {
            overrides[packageName] = UpdateEligibilityResult(
                isEligible = false,
                reasonCode = reasonCode,
                installSourceDetails = InstallSourceDetails(readSucceeded = true),
            )
        }
    }
}