From ef4e72e6d4bd556e75fb4ce7d546f72bbc10abfa Mon Sep 17 00:00:00 2001
From: Torsten Grote <t@grobox.de>
Date: Fri, 5 Apr 2024 15:28:28 -0300
Subject: [PATCH] [db] handle broken repos without certificate

On versions before 1.18.0, adding a repo would add it as "empty" to the DB without a certificate right away and if there was an issue with it (e.g. typo in URL), it would stay in the DB until manually removed.
Our migration code did not consider such repos.
---
 .../fdroid/database/MultiRepoMigrationTest.kt | 38 +++++++++++++++++++
 .../java/org/fdroid/database/Migrations.kt    |  4 +-
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt b/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt
index c5b89696e..b252299b4 100644
--- a/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt
+++ b/libs/database/src/dbTest/java/org/fdroid/database/MultiRepoMigrationTest.kt
@@ -248,6 +248,44 @@ internal class MultiRepoMigrationTest {
             }
     }
 
+    @Test
+    fun repoWithoutCertificate() {
+        helper.createDatabase(TEST_DB, 1).use { db ->
+            // Database has schema version 1. Insert some data using SQL queries.
+            // We can't use DAO classes because they expect the latest schema.
+            val repoId = db.insert(CoreRepository.TABLE, CONFLICT_FAIL, ContentValues().apply {
+                put("name", localizedTextV2toString(mapOf("en-US" to fdroidRepo.name)))
+                put(
+                    "description",
+                    localizedTextV2toString(mapOf("en-US" to fdroidRepo.description))
+                )
+                put("address", fdroidRepo.address)
+                put("timestamp", -1)
+            })
+            db.insert(RepositoryPreferences.TABLE, CONFLICT_FAIL, ContentValues().apply {
+                put("repoId", repoId)
+                put("enabled", fdroidRepo.enabled)
+                put("weight", fdroidRepo.weight)
+            })
+        }
+
+        // Re-open the database with version 2, auto-migrations are applied automatically
+        helper.runMigrationsAndValidate(TEST_DB, 2, true).close()
+
+        // now get the Room DB, so we can use our DAOs for verifying the migration
+        databaseBuilder(getApplicationContext(), FDroidDatabaseInt::class.java, TEST_DB)
+            .allowMainThreadQueries()
+            .build().use { db ->
+                // repo without cert did not get migrated
+                assertEquals(1, db.getRepositoryDao().getRepositories().size)
+                val repo = db.getRepositoryDao().getRepositories()[0]
+                // cert is still null
+                assertNull(repo.certificate)
+                // address still the same
+                assertEquals(fdroidRepo.address, repo.address)
+            }
+    }
+
     private fun runRepoMigration(
         repos: List<InitialRepository>,
         check: (FDroidDatabaseInt) -> Unit,
diff --git a/libs/database/src/main/java/org/fdroid/database/Migrations.kt b/libs/database/src/main/java/org/fdroid/database/Migrations.kt
index 3e326eb9d..8dc70cdef 100644
--- a/libs/database/src/main/java/org/fdroid/database/Migrations.kt
+++ b/libs/database/src/main/java/org/fdroid/database/Migrations.kt
@@ -38,7 +38,7 @@ internal class MultiRepoMigration : AutoMigrationSpec {
             while (cursor.moveToNext()) {
                 val repo = getRepo(cursor)
                 log.error { repo.toString() }
-                if (repo.isArchive()) {
+                if (repo.isArchive() && repo.certificate != null) {
                     if (archiveMap.containsKey(repo.certificate)) {
                         log.error { "More than two repos with certificate of ${repo.address}" }
                         // still migrating this as a normal repo then
@@ -98,7 +98,7 @@ internal class MultiRepoMigration : AutoMigrationSpec {
     private data class Repo(
         val repoId: Long,
         val address: String,
-        val certificate: String,
+        val certificate: String?,
         val weight: Int,
     ) {
         fun isArchive(): Boolean = address.trimEnd('/').endsWith("/archive")