From 7489f8466be469ad3bba39df912f451c66bc65a5 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 5 Apr 2023 17:34:36 -0300 Subject: [PATCH 1/3] [db] Rework AppDao#clearAll() method to fix critical bug This is a breaking change. The new method is in FDroidDatabase and called clearAllAppData(). It now also resets repository timestamps, so we will not try to apply a diff when we should use a full index. --- .../org/fdroid/database/FDroidDatabaseTest.kt | 44 +++++++++++++++++++ .../main/java/org/fdroid/database/AppDao.kt | 7 +-- .../org/fdroid/database/FDroidDatabase.kt | 14 ++++++ .../java/org/fdroid/database/RepositoryDao.kt | 7 +++ 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 libs/database/src/dbTest/java/org/fdroid/database/FDroidDatabaseTest.kt diff --git a/libs/database/src/dbTest/java/org/fdroid/database/FDroidDatabaseTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/FDroidDatabaseTest.kt new file mode 100644 index 000000000..072977fb5 --- /dev/null +++ b/libs/database/src/dbTest/java/org/fdroid/database/FDroidDatabaseTest.kt @@ -0,0 +1,44 @@ +package org.fdroid.database + +import androidx.test.ext.junit.runners.AndroidJUnit4 +import org.fdroid.test.TestRepoUtils +import org.junit.Test +import org.junit.runner.RunWith +import kotlin.test.assertEquals +import kotlin.test.assertNotEquals + +@RunWith(AndroidJUnit4::class) +internal class FDroidDatabaseTest : AppTest() { + + @Test + fun testClearAllAppData() { + // insert three apps in two repos + val repoId2 = repoDao.insertOrReplace(TestRepoUtils.getRandomRepo()) + val repoId1 = repoDao.insertOrReplace(TestRepoUtils.getRandomRepo()) + appDao.insert(repoId1, packageName1, app1, locales) + appDao.insert(repoId2, packageName2, app2, locales) + appDao.insert(repoId2, packageName3, app3, locales) + + // assert that both repos and all three apps made it into the DB + assertEquals(2, repoDao.getRepositories().size) + assertEquals(3, appDao.countApps()) + + // assert that repo timestamps are recent, not reset + repoDao.getRepositories().forEach { repo -> + assertNotEquals(-1, repo.timestamp) + } + + // clear all app data + db.clearAllAppData() + + // assert that both repos survived, but all apps are gone + assertEquals(2, repoDao.getRepositories().size) + assertEquals(0, appDao.countApps()) + + // assert that repo timestamps got reset + repoDao.getRepositories().forEach { repo -> + assertEquals(-1, repo.timestamp) + } + } + +} 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 eea1b0517..66c877080 100644 --- a/libs/database/src/main/java/org/fdroid/database/AppDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/AppDao.kt @@ -111,11 +111,6 @@ public interface AppDao { public fun getNumberOfAppsInCategory(category: String): Int public fun getNumberOfAppsInRepository(repoId: Long): Int - - /** - * Removes all apps and associated data from all repositories. - */ - public fun clearAll() } public enum class AppListSortOrder { @@ -598,5 +593,5 @@ internal interface AppDaoInt : AppDao { fun countLocalizedFileLists(): Int @Query("DELETE FROM ${AppMetadata.TABLE}") - override fun clearAll() + fun clearAll() } diff --git a/libs/database/src/main/java/org/fdroid/database/FDroidDatabase.kt b/libs/database/src/main/java/org/fdroid/database/FDroidDatabase.kt index 7d32c8d2c..5631d6a56 100644 --- a/libs/database/src/main/java/org/fdroid/database/FDroidDatabase.kt +++ b/libs/database/src/main/java/org/fdroid/database/FDroidDatabase.kt @@ -69,6 +69,13 @@ internal abstract class FDroidDatabaseInt internal constructor() : RoomDatabase( fun afterUpdatingRepo(repoId: Long) { getAppDao().updateCompatibility(repoId) } + + override fun clearAllAppData() { + runInTransaction { + getAppDao().clearAll() + getRepositoryDao().resetTimestamps() + } + } } /** @@ -93,4 +100,11 @@ public interface FDroidDatabase { * Please run as little code as possible to keep the time the database is blocked minimal. */ public fun runInTransaction(body: Runnable) + + /** + * Removes all apps and associated data (such as versions) from all repositories. + * The repository data and app preferences are kept as-is. + * Only the timestamp of the last repo update gets reset, so we won't try to apply diffs. + */ + public fun clearAllAppData() } 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 1dc36c887..0605db02b 100644 --- a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt @@ -378,6 +378,13 @@ internal interface RepositoryDaoInt : RepositoryDao { @Query("DELETE FROM ${ReleaseChannel.TABLE} WHERE repoId = :repoId AND id = :id") fun deleteReleaseChannel(repoId: Long, id: String) + /** + * Resets timestamps for *all* repos in the database. + * This will cause + */ + @Query("UPDATE ${CoreRepository.TABLE} SET timestamp = -1") + fun resetTimestamps() + /** * Use when replacing an existing repo with a full index. * This removes all existing index data associated with this repo from the database, From 8f02240419d6903cac48eafa3d8032972823bf4c Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Wed, 5 Apr 2023 17:35:38 -0300 Subject: [PATCH 2/3] [app] Switch to new method for clearing all app data to avoid stuck repo updates when doing major Android upgrades --- app/src/main/java/org/fdroid/fdroid/data/DBHelper.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java index d8fa3ed3d..8610c1182 100644 --- a/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java +++ b/app/src/main/java/org/fdroid/fdroid/data/DBHelper.java @@ -103,7 +103,7 @@ public class DBHelper { @AnyThread public static void resetTransient(Context context) { FDroidDatabase db = getDb(context); - Utils.runOffUiThread(() -> db.getAppDao().clearAll()); + Utils.runOffUiThread(db::clearAllAppData); } @AnyThread From d7b52abf43aa7f04bc56d2cfdcc6fd9d0ac6ae85 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Thu, 6 Apr 2023 08:34:02 -0300 Subject: [PATCH 3/3] [db] Clarify method documentation for clearing/resetting --- libs/database/src/main/java/org/fdroid/database/AppDao.kt | 5 +++++ .../src/main/java/org/fdroid/database/RepositoryDao.kt | 4 +++- 2 files changed, 8 insertions(+), 1 deletion(-) 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 66c877080..ef6f20719 100644 --- a/libs/database/src/main/java/org/fdroid/database/AppDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/AppDao.kt @@ -592,6 +592,11 @@ internal interface AppDaoInt : AppDao { @Query("SELECT COUNT(*) FROM ${LocalizedFileList.TABLE}") fun countLocalizedFileLists(): Int + /** + * Removes all apps and associated data such as versions from the database. + * Careful: Doing this without other measures such as calling [RepositoryDaoInt.resetTimestamps] + * will cause application of diffs to fail. + */ @Query("DELETE FROM ${AppMetadata.TABLE}") fun clearAll() } 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 0605db02b..db78e78a5 100644 --- a/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt +++ b/libs/database/src/main/java/org/fdroid/database/RepositoryDao.kt @@ -15,6 +15,7 @@ import org.fdroid.database.DbDiffUtils.diffAndUpdateListTable import org.fdroid.database.DbDiffUtils.diffAndUpdateTable import org.fdroid.index.IndexFormatVersion import org.fdroid.index.IndexParser.json +import org.fdroid.index.v2.IndexV2Updater import org.fdroid.index.v2.MirrorV2 import org.fdroid.index.v2.ReflectionDiffer.applyDiff import org.fdroid.index.v2.RepoV2 @@ -380,7 +381,8 @@ internal interface RepositoryDaoInt : RepositoryDao { /** * Resets timestamps for *all* repos in the database. - * This will cause + * This will use a full index instead of diffs + * when updating the repository via [IndexV2Updater]. */ @Query("UPDATE ${CoreRepository.TABLE} SET timestamp = -1") fun resetTimestamps()