From dfcd405309b7264701d97d600017cc9b6b92de39 Mon Sep 17 00:00:00 2001 From: Sayantan Roychowdhury Date: Thu, 7 Sep 2023 02:42:31 +0530 Subject: [PATCH 1/5] simplify getAppFilterLevel() function --- .../e/apps/data/fused/FusedApiImpl.kt | 75 ++++++++----------- 1 file changed, 33 insertions(+), 42 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt index e08bb6ded..3f61dabf6 100644 --- a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt +++ b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt @@ -714,52 +714,43 @@ class FusedApiImpl @Inject constructor( * Issue: https://gitlab.e.foundation/e/backlog/-/issues/5720 */ override suspend fun getAppFilterLevel(fusedApp: FusedApp, authData: AuthData?): FilterLevel { - if (fusedApp.package_name.isBlank()) { - return FilterLevel.UNKNOWN - } - if (fusedApp.origin == Origin.CLEANAPK) { - /* - * Whitelist all open source apps. - * Issue: https://gitlab.e.foundation/e/backlog/-/issues/5785 - */ - return FilterLevel.NONE - } - if (authData == null) { - return if (fusedApp.origin == Origin.GPLAY) FilterLevel.UNKNOWN - else FilterLevel.NONE + return when { + fusedApp.package_name.isBlank() -> FilterLevel.UNKNOWN + fusedApp.originalSize == 0L -> FilterLevel.UI + !fusedApp.isFree && fusedApp.price.isBlank() -> FilterLevel.UI + fusedApp.origin == Origin.CLEANAPK -> FilterLevel.NONE + authData == null -> FilterLevel.UNKNOWN // cannot determine for gplay app + !isRestricted(fusedApp) -> FilterLevel.NONE + !isApplicationVisible(fusedApp) -> FilterLevel.DATA + !isDownloadable(fusedApp) -> FilterLevel.UI + else -> FilterLevel.NONE } + } - if (!fusedApp.isFree && fusedApp.price.isBlank()) { - return FilterLevel.UI - } + /** + * Some apps are simply not visible. + * Example: com.skype.m2 + */ + private suspend fun isApplicationVisible(fusedApp: FusedApp): Boolean { + return kotlin.runCatching { gplayRepository.getAppDetails(fusedApp.package_name) }.isSuccess + } - if (fusedApp.restriction != Constants.Restriction.NOT_RESTRICTED) { - /* - * Check if app details can be shown. If not then remove the app from lists. - */ - try { - gplayRepository.getAppDetails(fusedApp.package_name) - } catch (e: Exception) { - return FilterLevel.DATA - } + /** + * Some apps are visible but not downloadable. + * Example: com.riotgames.league.wildrift + */ + private suspend fun isDownloadable(fusedApp: FusedApp): Boolean { + return kotlin.runCatching { + gplayRepository.getDownloadInfo( + fusedApp.package_name, + fusedApp.latest_version_code, + fusedApp.offer_type, + ) + }.isSuccess + } - /* - * If the app can be shown, check if the app is downloadable. - * If not then change "Install" button to "N/A" - */ - try { - gplayRepository.getDownloadInfo( - fusedApp.package_name, - fusedApp.latest_version_code, - fusedApp.offer_type, - ) - } catch (e: Exception) { - return FilterLevel.UI - } - } else if (fusedApp.originalSize == 0L) { - return FilterLevel.UI - } - return FilterLevel.NONE + private fun isRestricted(fusedApp: FusedApp): Boolean { + return fusedApp.restriction != Constants.Restriction.NOT_RESTRICTED } /* -- GitLab From b20dbad1e61773c5c7a03926b77e01b547c170c6 Mon Sep 17 00:00:00 2001 From: Sayantan Roychowdhury Date: Thu, 7 Sep 2023 02:44:38 +0530 Subject: [PATCH 2/5] UpdatesManagerImpl - getGPlayUpdates - fetch individual apps information --- .../e/apps/data/fused/FusedApiImpl.kt | 1 + .../e/apps/data/updates/UpdatesManagerImpl.kt | 39 +++++++++++++++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt index 3f61dabf6..1c3dc83b5 100644 --- a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt +++ b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt @@ -574,6 +574,7 @@ class FusedApiImpl @Inject constructor( return Pair(fusedApp, result.getResultStatus()) } + // Warning - GPlay results may not have proper geo-restriction information. override suspend fun getApplicationDetails( packageNameList: List, authData: AuthData, diff --git a/app/src/main/java/foundation/e/apps/data/updates/UpdatesManagerImpl.kt b/app/src/main/java/foundation/e/apps/data/updates/UpdatesManagerImpl.kt index bc9708991..42e90174a 100644 --- a/app/src/main/java/foundation/e/apps/data/updates/UpdatesManagerImpl.kt +++ b/app/src/main/java/foundation/e/apps/data/updates/UpdatesManagerImpl.kt @@ -39,6 +39,9 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.withContext import timber.log.Timber import javax.inject.Inject +import kotlinx.coroutines.async +import kotlinx.coroutines.awaitAll +import kotlinx.coroutines.coroutineScope class UpdatesManagerImpl @Inject constructor( @ApplicationContext private val context: Context, @@ -106,10 +109,9 @@ class UpdatesManagerImpl @Inject constructor( ) { val gplayStatus = getUpdatesFromApi({ - fusedAPIRepository.getApplicationDetails( + getGPlayUpdates( gPlayInstalledApps, - authData, - Origin.GPLAY + authData ) }, updateList) @@ -222,6 +224,37 @@ class UpdatesManagerImpl @Inject constructor( return apiResult.second } + /** + * Bulk info from gplay api is not providing correct geo restriction status of apps. + * So we get all individual app information asynchronously. + * Example: in.startv.hotstar.dplus + * Issue: https://gitlab.e.foundation/e/backlog/-/issues/7135 + */ + private suspend fun getGPlayUpdates( + packageNames: List, + authData: AuthData + ): Pair, ResultStatus> { + + val appsResults = coroutineScope { + val deferredResults = packageNames.map { packageName -> + async { + fusedAPIRepository.getApplicationDetails( + "", + packageName, + authData, + Origin.GPLAY + ) + } + } + deferredResults.awaitAll() + } + + val status = appsResults.find { it.second != ResultStatus.OK }?.second ?: ResultStatus.OK + val appsList = appsResults.map { it.first } + + return Pair(appsList, status) + } + /** * Takes a list of package names and for the apps present on F-Droid, * returns key value pairs of package names and their signatures. -- GitLab From 695ac5de47205bc48ac0fc0c0713c8ac3b567015 Mon Sep 17 00:00:00 2001 From: Sayantan Roychowdhury Date: Wed, 13 Sep 2023 19:30:31 +0530 Subject: [PATCH 3/5] move size check --- app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt index 1c3dc83b5..d0a3ccb19 100644 --- a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt +++ b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt @@ -717,9 +717,9 @@ class FusedApiImpl @Inject constructor( override suspend fun getAppFilterLevel(fusedApp: FusedApp, authData: AuthData?): FilterLevel { return when { fusedApp.package_name.isBlank() -> FilterLevel.UNKNOWN - fusedApp.originalSize == 0L -> FilterLevel.UI !fusedApp.isFree && fusedApp.price.isBlank() -> FilterLevel.UI fusedApp.origin == Origin.CLEANAPK -> FilterLevel.NONE + fusedApp.originalSize == 0L -> FilterLevel.UI authData == null -> FilterLevel.UNKNOWN // cannot determine for gplay app !isRestricted(fusedApp) -> FilterLevel.NONE !isApplicationVisible(fusedApp) -> FilterLevel.DATA -- GitLab From b19b785dd679dcbc94de20c4745e6bc1cb0132dd Mon Sep 17 00:00:00 2001 From: Sayantan Roychowdhury Date: Wed, 13 Sep 2023 20:00:37 +0530 Subject: [PATCH 4/5] move some more checks --- .../main/java/foundation/e/apps/data/fused/FusedApiImpl.kt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt index d0a3ccb19..151aecf62 100644 --- a/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt +++ b/app/src/main/java/foundation/e/apps/data/fused/FusedApiImpl.kt @@ -719,10 +719,10 @@ class FusedApiImpl @Inject constructor( fusedApp.package_name.isBlank() -> FilterLevel.UNKNOWN !fusedApp.isFree && fusedApp.price.isBlank() -> FilterLevel.UI fusedApp.origin == Origin.CLEANAPK -> FilterLevel.NONE - fusedApp.originalSize == 0L -> FilterLevel.UI - authData == null -> FilterLevel.UNKNOWN // cannot determine for gplay app !isRestricted(fusedApp) -> FilterLevel.NONE + authData == null -> FilterLevel.UNKNOWN // cannot determine for gplay app !isApplicationVisible(fusedApp) -> FilterLevel.DATA + fusedApp.originalSize == 0L -> FilterLevel.UI !isDownloadable(fusedApp) -> FilterLevel.UI else -> FilterLevel.NONE } -- GitLab From 113107d1a14a916e506d1612372270254ced7bf1 Mon Sep 17 00:00:00 2001 From: Hasib Prince Date: Wed, 13 Sep 2023 23:54:33 +0600 Subject: [PATCH 5/5] fixed: unit tests --- .../foundation/e/apps/FusedApiImplTest.kt | 97 +++++++------------ .../e/apps/UpdateManagerImptTest.kt | 46 ++++++--- 2 files changed, 65 insertions(+), 78 deletions(-) diff --git a/app/src/test/java/foundation/e/apps/FusedApiImplTest.kt b/app/src/test/java/foundation/e/apps/FusedApiImplTest.kt index 3096c444c..f4b38ae2d 100644 --- a/app/src/test/java/foundation/e/apps/FusedApiImplTest.kt +++ b/app/src/test/java/foundation/e/apps/FusedApiImplTest.kt @@ -473,27 +473,26 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is CleanApk`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.CLEANAPK - ) + val fusedApp = getFusedAppForFilterLevelTest() val filterLevel = fusedAPIImpl.getAppFilterLevel(fusedApp, AUTH_DATA) assertEquals("getAppFilterLevel", FilterLevel.NONE, filterLevel) } + private fun getFusedAppForFilterLevelTest(isFree: Boolean = true) = FusedApp( + _id = "113", + name = "Demo Three", + package_name = "foundation.e.demothree", + latest_version_code = 123, + origin = Origin.CLEANAPK, + originalSize = -1, + isFree = isFree, + price = "" + ) + @Test fun `getAppFilterLevel when Authdata is NULL`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.CLEANAPK - ) + val fusedApp = getFusedAppForFilterLevelTest() val filterLevel = fusedAPIImpl.getAppFilterLevel(fusedApp, null) assertEquals("getAppFilterLevel", FilterLevel.NONE, filterLevel) @@ -501,16 +500,10 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is restricted and paid and no price`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.GPLAY, - restriction = Constants.Restriction.UNKNOWN, - isFree = false, - price = "" - ) + val fusedApp = getFusedAppForFilterLevelTest(false).apply { + this.origin = Origin.GPLAY + this.restriction = Constants.Restriction.UNKNOWN + } val filterLevel = fusedAPIImpl.getAppFilterLevel(fusedApp, AUTH_DATA) assertEquals("getAppFilterLevel", FilterLevel.UI, filterLevel) @@ -518,16 +511,10 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is not_restricted and paid and no price`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.GPLAY, - restriction = Constants.Restriction.NOT_RESTRICTED, - isFree = false, - price = "" - ) + val fusedApp = getFusedAppForFilterLevelTest(false).apply { + this.origin = Origin.GPLAY + this.restriction = Constants.Restriction.NOT_RESTRICTED + } val filterLevel = fusedAPIImpl.getAppFilterLevel(fusedApp, AUTH_DATA) assertEquals("getAppFilterLevel", FilterLevel.UI, filterLevel) @@ -536,16 +523,10 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is restricted and getAppDetails and getDownloadDetails returns success`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.GPLAY, - restriction = Constants.Restriction.UNKNOWN, - isFree = true, - price = "" - ) + val fusedApp = getFusedAppForFilterLevelTest().apply { + this.origin = Origin.GPLAY + this.restriction = Constants.Restriction.UNKNOWN + } Mockito.`when`(gPlayAPIRepository.getAppDetails(fusedApp.package_name)) .thenReturn(App(fusedApp.package_name)) @@ -564,16 +545,10 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is restricted and getAppDetails throws exception`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.GPLAY, - restriction = Constants.Restriction.UNKNOWN, - isFree = true, - price = "" - ) + val fusedApp = getFusedAppForFilterLevelTest().apply { + this.origin = Origin.GPLAY + this.restriction = Constants.Restriction.UNKNOWN + } Mockito.`when`(gPlayAPIRepository.getAppDetails(fusedApp.package_name)) .thenThrow(RuntimeException()) @@ -590,16 +565,10 @@ class FusedApiImplTest { @Test fun `getAppFilterLevel when app is restricted and getDownoadInfo throws exception`() = runTest { - val fusedApp = FusedApp( - _id = "113", - name = "Demo Three", - package_name = "foundation.e.demothree", - latest_version_code = 123, - origin = Origin.GPLAY, - restriction = Constants.Restriction.UNKNOWN, - isFree = true, - price = "" - ) + val fusedApp = getFusedAppForFilterLevelTest().apply { + this.origin = Origin.GPLAY + this.restriction = Constants.Restriction.UNKNOWN + } Mockito.`when`(gPlayAPIRepository.getAppDetails(fusedApp.package_name)) .thenReturn(App(fusedApp.package_name)) diff --git a/app/src/test/java/foundation/e/apps/UpdateManagerImptTest.kt b/app/src/test/java/foundation/e/apps/UpdateManagerImptTest.kt index 418299588..8bf8335f7 100644 --- a/app/src/test/java/foundation/e/apps/UpdateManagerImptTest.kt +++ b/app/src/test/java/foundation/e/apps/UpdateManagerImptTest.kt @@ -81,20 +81,21 @@ class UpdateManagerImptTest { val authData = AuthData("e@e.email", "AtadyMsIAtadyM") - val applicationInfo = mutableListOf( - ApplicationInfo().apply { this.packageName = "foundation.e.demoone" }, - ApplicationInfo().apply { this.packageName = "foundation.e.demotwo" }, - ApplicationInfo().apply { this.packageName = "foundation.e.demothree" } - ) - @Before fun setup() { MockitoAnnotations.openMocks(this) faultyAppRepository = FaultyAppRepository(FakeFaultyAppDao()) preferenceModule = FakePreferenceModule(context) pkgManagerModule = FakePkgManagerModule(context, getGplayApps()) - updatesManagerImpl = - UpdatesManagerImpl(context, pkgManagerModule, fusedAPIRepository, faultyAppRepository, preferenceModule, fdroidRepository, blockedAppRepository) + updatesManagerImpl = UpdatesManagerImpl( + context, + pkgManagerModule, + fusedAPIRepository, + faultyAppRepository, + preferenceModule, + fdroidRepository, + blockedAppRepository + ) } @Test @@ -330,14 +331,31 @@ class UpdateManagerImptTest { eq(Origin.CLEANAPK) ) ).thenReturn(openSourceUpdates) + Mockito.`when`(fusedAPIRepository.getApplicationCategoryPreference()) .thenReturn(selectedApplicationSources) - Mockito.`when`( - fusedAPIRepository.getApplicationDetails( - any(), - any(), - eq(Origin.GPLAY) + + if (gplayUpdates.first.isNotEmpty()) { + Mockito.`when`( + fusedAPIRepository.getApplicationDetails( + any(), + any(), + any(), + eq(Origin.GPLAY) + ) + ).thenReturn( + Pair(gplayUpdates.first.first(), ResultStatus.OK), + Pair(gplayUpdates.first[1], ResultStatus.OK) ) - ).thenReturn(gplayUpdates) + } else { + Mockito.`when`( + fusedAPIRepository.getApplicationDetails( + any(), + any(), + any(), + eq(Origin.GPLAY) + ) + ).thenReturn(Pair(FusedApp(), ResultStatus.TIMEOUT)) + } } } -- GitLab