From b993da8db8fb053e6d73d2676af23845f44fe9eb Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Fri, 3 Nov 2023 11:22:42 -0300 Subject: [PATCH] [db] New repos now get lower weight than older ones so they do not override the information from older repos anymore. This is especially an issue for the official repo which historically had the lowest priority while it should have the highest. --- .../java/org/fdroid/database/AppDaoTest.kt | 10 +++---- .../org/fdroid/database/AppListItemsTest.kt | 15 ++++------- .../fdroid/database/AppOverviewItemsTest.kt | 27 +++++++++++++++---- .../org/fdroid/database/AppPrefsDaoTest.kt | 4 +-- .../fdroid/database/DbUpdateCheckerTest.kt | 6 ++--- .../org/fdroid/database/RepositoryDaoTest.kt | 13 ++++++++- .../main/java/org/fdroid/database/AppDao.kt | 2 +- .../java/org/fdroid/database/RepositoryDao.kt | 16 +++++------ .../java/org/fdroid/repo/RepoAdderTest.kt | 2 +- 9 files changed, 59 insertions(+), 36 deletions(-) diff --git a/libs/database/src/dbTest/java/org/fdroid/database/AppDaoTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/AppDaoTest.kt index 1b19435db..0c1ebe25a 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/AppDaoTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/AppDaoTest.kt @@ -35,9 +35,9 @@ internal class AppDaoTest : AppTest() { @Test fun testGetSameAppFromTwoRepos() { // insert same app into three repos (repoId1 has highest weight) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) - val repoId3 = repoDao.insertOrReplace(getRandomRepo()) val repoId1 = repoDao.insertOrReplace(getRandomRepo()) + val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId2, packageName, app2, locales) appDao.insert(repoId3, packageName, app3, locales) @@ -67,9 +67,9 @@ internal class AppDaoTest : AppTest() { @Test fun testAppRepoPref() { // insert same app into three repos (repoId1 has highest weight) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) - val repoId3 = repoDao.insertOrReplace(getRandomRepo()) val repoId1 = repoDao.insertOrReplace(getRandomRepo()) + val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId2, packageName, app2, locales) appDao.insert(repoId3, packageName, app3, locales) @@ -93,8 +93,8 @@ internal class AppDaoTest : AppTest() { @Test fun testGetSameAppFromTwoReposOneDisabled() { // insert same app into two repos (repoId2 has highest weight) - val repoId1 = repoDao.insertOrReplace(getRandomRepo()) val repoId2 = repoDao.insertOrReplace(getRandomRepo()) + val repoId1 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId2, packageName, app2, locales) diff --git a/libs/database/src/dbTest/java/org/fdroid/database/AppListItemsTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/AppListItemsTest.kt index b7dce5948..f06839e93 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/AppListItemsTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/AppListItemsTest.kt @@ -25,6 +25,7 @@ import kotlin.test.assertNull import kotlin.test.assertTrue import kotlin.test.fail +@Suppress("DEPRECATION") @RunWith(AndroidJUnit4::class) internal class AppListItemsTest : AppTest() { @@ -48,7 +49,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId, packageName2, app2, locales) // one of the apps is installed - @Suppress("DEPRECATION") val packageInfo2 = PackageInfo().apply { packageName = packageName2 versionName = getRandomString() @@ -108,7 +108,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId, packageName2, app2, locales) // one of the apps is installed - @Suppress("DEPRECATION") val packageInfo2 = PackageInfo().apply { packageName = packageName2 versionName = getRandomString() @@ -177,7 +176,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId3, packageName3, app3b, locales) // one of the apps is installed - @Suppress("DEPRECATION") val packageInfo2 = PackageInfo().apply { packageName = packageName2 versionName = getRandomString() @@ -306,7 +304,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId, packageName2, app2, locales) // one of the apps is installed - @Suppress("DEPRECATION") val packageInfo2 = PackageInfo().apply { packageName = packageName2 versionName = getRandomString() @@ -429,9 +426,9 @@ internal class AppListItemsTest : AppTest() { @Test fun testFromRepoWithHighestWeight() { // insert same app into three repos (repoId1 has highest weight) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) - val repoId3 = repoDao.insertOrReplace(getRandomRepo()) val repoId1 = repoDao.insertOrReplace(getRandomRepo()) + val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId2, packageName, app2, locales) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId3, packageName, app3, locales) @@ -454,9 +451,9 @@ internal class AppListItemsTest : AppTest() { @Test fun testFromRepoFromAppPrefs() { // insert same app into three repos (repoId1 has highest weight) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) - val repoId3 = repoDao.insertOrReplace(getRandomRepo()) val repoId1 = repoDao.insertOrReplace(getRandomRepo()) + val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId2, packageName, app2, locales) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId3, packageName, app3, locales) @@ -539,7 +536,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId, packageName3, app3, locales) // define packageInfo for each test - @Suppress("DEPRECATION") val packageInfo1 = PackageInfo().apply { packageName = packageName1 versionName = getRandomString() @@ -581,7 +577,6 @@ internal class AppListItemsTest : AppTest() { appDao.insert(repoId, packageName, app1, locales) val packageInfoCreator = { name: String -> - @Suppress("DEPRECATION") PackageInfo().apply { packageName = name versionName = name diff --git a/libs/database/src/dbTest/java/org/fdroid/database/AppOverviewItemsTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/AppOverviewItemsTest.kt index 25fc58f1a..b99234465 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/AppOverviewItemsTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/AppOverviewItemsTest.kt @@ -71,7 +71,14 @@ internal class AppOverviewItemsTest : AppTest() { val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId2, packageName, app2, locales) - // now icon is returned from app in second repo + // app is still returned as before + appDao.getAppOverviewItems().getOrFail().let { apps -> + assertEquals(1, apps.size) + assertEquals(app1.icon.getBestLocale(locales), apps[0].getIcon(locales)) + } + + // after preferring second repo, icon is returned from app in second repo + appPrefsDao.update(AppPrefs(packageName, preferredRepoId = repoId2)) appDao.getAppOverviewItems().getOrFail().let { apps -> assertEquals(1, apps.size) assertEquals(app2.icon.getBestLocale(locales), apps[0].getIcon(locales)) @@ -152,7 +159,14 @@ internal class AppOverviewItemsTest : AppTest() { val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId2, packageName, app2, locales) - // now second app from second repo is returned + // app is still returned as before, new repo doesn't override old one + appDao.getAppOverviewItems().getOrFail().let { apps -> + assertEquals(1, apps.size) + assertEquals(app1, apps[0]) + } + + // now second app from second repo is returned after preferring it explicitly + appPrefsDao.update(AppPrefs(packageName, preferredRepoId = repoId2)) appDao.getAppOverviewItems().getOrFail().let { apps -> assertEquals(1, apps.size) assertEquals(app2, apps[0]) @@ -162,9 +176,9 @@ internal class AppOverviewItemsTest : AppTest() { @Test fun testGetByRepoPref() { // insert same app into three repos (repoId1 has highest weight) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) - val repoId3 = repoDao.insertOrReplace(getRandomRepo()) val repoId1 = repoDao.insertOrReplace(getRandomRepo()) + val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId2, packageName, app2, locales) appDao.insert(repoId3, packageName, app3, locales) @@ -255,8 +269,10 @@ internal class AppOverviewItemsTest : AppTest() { assertEquals(3, appDao.getAppOverviewItems().getOrFail().size) // app3b is the same as app3, but has an icon, so is not last anymore + // after we prefer that repo for this app val app3b = app3.copy(icon = icons2) appDao.insert(repoId2, packageName3, app3b) + appPrefsDao.update(AppPrefs(packageName3, preferredRepoId = repoId2)) // note that we don't insert a version here appDao.getAppOverviewItems().getOrFail().let { apps -> assertEquals(3, apps.size) @@ -307,9 +323,10 @@ internal class AppOverviewItemsTest : AppTest() { // note that we don't insert a version here assertEquals(3, appDao.getAppOverviewItems("A").getOrFail().size) - // app3b is the same as app3, but has an icon, so is not last anymore + // app3b is the same as app3, but has an icon and is preferred, so is not last anymore val app3b = app3.copy(icon = icons2) appDao.insert(repoId2, packageName3, app3b) + appPrefsDao.update(AppPrefs(packageName3, preferredRepoId = repoId2)) // note that we don't insert a version here appDao.getAppOverviewItems("A").getOrFail().let { apps -> assertEquals(3, apps.size) diff --git a/libs/database/src/dbTest/java/org/fdroid/database/AppPrefsDaoTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/AppPrefsDaoTest.kt index b9d51c206..da767aa8b 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/AppPrefsDaoTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/AppPrefsDaoTest.kt @@ -55,9 +55,9 @@ internal class AppPrefsDaoTest : AppTest() { @Test fun testGetPreferredRepos() { // insert three apps, the third is in repo2 and repo3 - val repoId1 = repoDao.insertOrReplace(getRandomRepo()) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) + val repoId1 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName1, app1, locales) appDao.insert(repoId2, packageName2, app2, locales) appDao.insert(repoId2, packageName3, app3, locales) diff --git a/libs/database/src/dbTest/java/org/fdroid/database/DbUpdateCheckerTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/DbUpdateCheckerTest.kt index 2f83e87ae..b846c3b90 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/DbUpdateCheckerTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/DbUpdateCheckerTest.kt @@ -136,8 +136,8 @@ internal class DbUpdateCheckerTest : AppTest() { @Test fun testSuggestedVersionOnlyFromPreferredRepo() { // insert the same app into two repos - val repoId1 = repoDao.insertOrReplace(getRandomRepo()) val repoId2 = repoDao.insertOrReplace(getRandomRepo()) + val repoId1 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageName, app1, locales) appDao.insert(repoId2, packageName, app2, locales) @@ -225,9 +225,9 @@ internal class DbUpdateCheckerTest : AppTest() { @Test fun testGetUpdatableAppsOnlyFromPreferredRepo() { // insert the same app into three repos - val repoId1 = repoDao.insertOrReplace(getRandomRepo()) - val repoId2 = repoDao.insertOrReplace(getRandomRepo()) val repoId3 = repoDao.insertOrReplace(getRandomRepo()) + val repoId2 = repoDao.insertOrReplace(getRandomRepo()) + val repoId1 = repoDao.insertOrReplace(getRandomRepo()) appDao.insert(repoId1, packageInfo.packageName, app1, locales) appDao.insert(repoId2, packageInfo.packageName, app2, locales) appDao.insert(repoId3, packageInfo.packageName, app3, locales) diff --git a/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt index cde57bddf..5b337e02d 100644 --- a/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt +++ b/libs/database/src/dbTest/java/org/fdroid/database/RepositoryDaoTest.kt @@ -110,7 +110,7 @@ internal class RepositoryDaoTest : DbTest() { val repositoryPreferences2 = repoDao.getRepositoryPreferences(repoId2) assertEquals(repoId2, repositoryPreferences2?.repoId) // second repo has one weight point more than first repo - assertEquals(repositoryPreferences1?.weight?.plus(1), repositoryPreferences2?.weight) + assertEquals(repositoryPreferences1?.weight?.minus(2), repositoryPreferences2?.weight) // remove first repo and check that the database only returns one repoDao.deleteRepository(repoId1) @@ -277,4 +277,15 @@ internal class RepositoryDaoTest : DbTest() { assertEquals(1, repoDao.getRepositories().size) assertEquals(cert, repoDao.getRepositories()[0].certificate) } + + @Test + fun testGetMinRepositoryWeight() { + assertEquals(Int.MAX_VALUE, repoDao.getMinRepositoryWeight()) + + repoDao.insertOrReplace(getRandomRepo()) + assertEquals(Int.MAX_VALUE - 2, repoDao.getMinRepositoryWeight()) + + repoDao.insertOrReplace(getRandomRepo()) + assertEquals(Int.MAX_VALUE - 4, repoDao.getMinRepositoryWeight()) + } } diff --git a/libs/database/src/main/java/org/fdroid/database/AppDao.kt b/libs/database/src/main/java/org/fdroid/database/AppDao.kt index 92c2240cd..a5ada080a 100644 --- a/libs/database/src/main/java/org/fdroid/database/AppDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/AppDao.kt @@ -354,7 +354,7 @@ internal interface AppDaoInt : AppDao { JOIN ${RepositoryPreferences.TABLE} AS pref USING (repoId) LEFT JOIN ${HighestVersion.TABLE} AS version USING (repoId, packageName) LEFT JOIN ${LocalizedIcon.TABLE} AS icon USING (repoId, packageName) - LEFT JOIN AppPrefs USING (packageName) + LEFT JOIN ${AppPrefs.TABLE} USING (packageName) WHERE pref.enabled = 1 AND COALESCE(preferredRepoId, repoId) = repoId GROUP BY packageName HAVING MAX(pref.weight) ORDER BY localizedName IS NULL ASC, icon.packageName IS NULL ASC, diff --git a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt index 281d68122..7e2f9df97 100644 --- a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt @@ -151,10 +151,10 @@ internal interface RepositoryDaoInt : RepositoryDao { certificate = newRepository.certificate, ) val repoId = insertOrReplace(repo) - val currentMaxWeight = getMaxRepositoryWeight() + val currentMinWeight = getMinRepositoryWeight() val repositoryPreferences = RepositoryPreferences( repoId = repoId, - weight = currentMaxWeight + 1, + weight = currentMinWeight - 2, lastUpdated = null, username = newRepository.username, password = newRepository.password, @@ -181,10 +181,10 @@ internal interface RepositoryDaoInt : RepositoryDao { certificate = null, ) val repoId = insertOrReplace(repo) - val currentMaxWeight = getMaxRepositoryWeight() + val currentMinWeight = getMinRepositoryWeight() val repositoryPreferences = RepositoryPreferences( repoId = repoId, - weight = currentMaxWeight + 1, + weight = currentMinWeight - 2, lastUpdated = null, username = username, password = password, @@ -197,15 +197,15 @@ internal interface RepositoryDaoInt : RepositoryDao { @VisibleForTesting fun insertOrReplace(repository: RepoV2, version: Long = 0): Long { val repoId = insertOrReplace(repository.toCoreRepository(version = version)) - val currentMaxWeight = getMaxRepositoryWeight() - val repositoryPreferences = RepositoryPreferences(repoId, currentMaxWeight + 1) + val currentMinWeight = getMinRepositoryWeight() + val repositoryPreferences = RepositoryPreferences(repoId, currentMinWeight - 2) insert(repositoryPreferences) insertRepoTables(repoId, repository) return repoId } - @Query("SELECT MAX(weight) FROM ${RepositoryPreferences.TABLE}") - fun getMaxRepositoryWeight(): Int + @Query("SELECT COALESCE(MIN(weight), ${Int.MAX_VALUE}) FROM ${RepositoryPreferences.TABLE}") + fun getMinRepositoryWeight(): Int @Transaction @Query("SELECT * FROM ${CoreRepository.TABLE} WHERE repoId = :repoId") diff --git a/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt b/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt index 714f7b5a2..eb58d8864 100644 --- a/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt +++ b/libs/database/src/test/java/org/fdroid/repo/RepoAdderTest.kt @@ -100,7 +100,7 @@ internal class RepoAdderTest { val userManager = mockk() val repoAdder = RepoAdder(context, db, tempFileProvider, downloaderFactory, httpManager) - every { context.getSystemService("user") } returns userManager + every { context.getSystemService(UserManager::class.java) } returns userManager every { userManager.hasUserRestriction(DISALLOW_INSTALL_UNKNOWN_SOURCES) } returns true