Use Hilt for explicit Migrations

bitfireAT · Dec 31, 2024 · 891f3fe · 891f3fe
1 parent 139c5f1
commit 891f3fe
Show file tree

Hide file tree

Showing 16 changed files with 172 additions and 83 deletions.
diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/db/AppDatabaseTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/db/AppDatabaseTest.kt
@@ -7,6 +7,7 @@ package at.bitfire.davdroid.db
 import android.content.Context
 import androidx.room.Room
 import androidx.room.migration.AutoMigrationSpec
+import androidx.room.migration.Migration
 import androidx.room.testing.MigrationTestHelper
 import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
 import androidx.test.platform.app.InstrumentationRegistry
@@ -34,6 +35,9 @@ class AppDatabaseTest {
     @Inject
     lateinit var logger: Logger
 
+    @Inject
+    lateinit var manualMigrations: Set<@JvmSuppressWildcards Migration>
+
     @Before
     fun setup() {
         hiltRule.inject()
@@ -56,7 +60,7 @@ class AppDatabaseTest {
         // open and migrate (to current version) database
         Room.databaseBuilder(context, AppDatabase::class.java, TEST_DB)
             // manual migrations
-            .addMigrations(*AppDatabase.manualMigrations)
+            .addMigrations(*manualMigrations.toTypedArray())
             // auto-migrations that need to be specified explicitly
             .apply {
                 for (spec in autoMigrations)

diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/db/migration/AutoMigration16Test.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/db/migration/AutoMigration16Test.kt
@@ -15,74 +15,68 @@ import org.junit.Test
 class AutoMigration16Test: DatabaseMigrationTest(toVersion = 16) {
 
     @Test
-    fun testMigrate_WithTimeZone() {
-        testMigration(
-            prepare = { db ->
-                val minimalVTimezone = """
-                    BEGIN:VCALENDAR
-                    VERSION:2.0
-                    PRODID:DAVx5
-                    BEGIN:VTIMEZONE
-                    TZID:America/New_York
-                    END:VTIMEZONE
-                    END:VCALENDAR
-                """.trimIndent()
-                db.execSQL(
-                    "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
-                    arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
-                )
-                db.execSQL(
-                    "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync, timezone) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
-                    arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false, minimalVTimezone)
-                )
-            }
-        ) { db ->
-            db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
-                cursor.moveToFirst()
-                assertEquals("America/New_York", cursor.getString(0))
-            }
+    fun testMigrate_WithTimeZone() = testMigration(
+        prepare = { db ->
+            val minimalVTimezone = """
+                BEGIN:VCALENDAR
+                VERSION:2.0
+                PRODID:DAVx5
+                BEGIN:VTIMEZONE
+                TZID:America/New_York
+                END:VTIMEZONE
+                END:VCALENDAR
+            """.trimIndent()
+            db.execSQL(
+                "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
+                arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
+            )
+            db.execSQL(
+                "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync, timezone) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
+                arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false, minimalVTimezone)
+            )
+        }
+    ) { db ->
+        db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
+            cursor.moveToFirst()
+            assertEquals("America/New_York", cursor.getString(0))
         }
     }
 
     @Test
-    fun testMigrate_WithTimeZone_Unparseable() {
-        testMigration(
-            prepare = { db ->
-                db.execSQL(
-                    "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
-                    arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
-                )
-                db.execSQL(
-                    "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync, timezone) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
-                    arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false, "Some Garbage Content")
-                )
-            }
-        ) { db ->
-            db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
-                cursor.moveToFirst()
-                assertNull(cursor.getString(0))
-            }
+    fun testMigrate_WithTimeZone_Unparseable() = testMigration(
+        prepare = { db ->
+            db.execSQL(
+                "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
+                arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
+            )
+            db.execSQL(
+                "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync, timezone) VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?)",
+                arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false, "Some Garbage Content")
+            )
+        }
+    ) { db ->
+        db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
+            cursor.moveToFirst()
+            assertNull(cursor.getString(0))
         }
     }
 
     @Test
-    fun testMigrate_WithoutTimezone() {
-        testMigration(
-            prepare = { db ->
-                db.execSQL(
-                    "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
-                    arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
-                )
-                db.execSQL(
-                    "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync) VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
-                    arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false)
-                )
-            }
-        ) { db ->
-            db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
-                cursor.moveToFirst()
-                assertNull(cursor.getString(0))
-            }
+    fun testMigrate_WithoutTimezone() = testMigration(
+        prepare = { db ->
+            db.execSQL(
+                "INSERT INTO service (id, accountName, type) VALUES (?, ?, ?)",
+                arrayOf(1, "test", Service.Companion.TYPE_CALDAV)
+            )
+            db.execSQL(
+                "INSERT INTO collection (id, serviceId, type, url, privWriteContent, privUnbind, forceReadOnly, sync) VALUES (?, ?, ?, ?, ?, ?, ?, ?)",
+                arrayOf(1, 1, TYPE_CALENDAR, "https://example.com", true, true, false, false)
+            )
+        }
+    ) { db ->
+        db.query("SELECT timezoneId FROM collection WHERE id=1").use { cursor ->
+            cursor.moveToFirst()
+            assertNull(cursor.getString(0))
         }
     }
 

diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/db/migration/DatabaseMigrationTest.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/db/migration/DatabaseMigrationTest.kt
@@ -5,6 +5,7 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.AutoMigrationSpec
+import androidx.room.migration.Migration
 import androidx.room.testing.MigrationTestHelper
 import androidx.sqlite.db.SupportSQLiteDatabase
 import androidx.sqlite.db.framework.FrameworkSQLiteOpenHelperFactory
@@ -15,13 +16,21 @@ import org.junit.Before
 import org.junit.Rule
 import javax.inject.Inject
 
+/**
+ * Helper for testing the database migration from [toVersion] - 1 to [toVersion].
+ *
+ * @param toVersion     The target version to migrate to.
+ */
 abstract class DatabaseMigrationTest(
     private val toVersion: Int
 ) {
 
     @Inject
     lateinit var autoMigrations: Set<@JvmSuppressWildcards AutoMigrationSpec>
 
+    @Inject
+    lateinit var manualMigrations: Set<@JvmSuppressWildcards Migration>
+
     @get:Rule
     val hiltRule = HiltAndroidRule(this)
 
@@ -35,7 +44,7 @@ abstract class DatabaseMigrationTest(
     /**
      * Used for testing the migration process from [toVersion]-1 to [toVersion].
      *
-     * @param prepare      Callback to prepare the database. Will be run with database schema in version [fromVersion].
+     * @param prepare      Callback to prepare the database. Will be run with database schema in version [toVersion] - 1.
      * @param validate     Callback to validate the migration result. Will be run with database schema in version [toVersion].
      */
     protected fun testMigration(
@@ -61,7 +70,7 @@ abstract class DatabaseMigrationTest(
             name = dbName,
             version = toVersion,
             validateDroppedTables = true,
-            migrations = AppDatabase.manualMigrations
+            migrations = manualMigrations.toTypedArray()
         )
 
         validate(db)

diff --git a/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt b/app/src/androidTest/kotlin/at/bitfire/davdroid/sync/account/TestAccountAuthenticator.kt
@@ -42,7 +42,7 @@ class TestAccountAuthenticator: Service() {
 
     companion object {
 
-        val context by lazy { InstrumentationRegistry.getInstrumentation().context }
+        private val context by lazy { InstrumentationRegistry.getInstrumentation().context }
         val counter = AtomicInteger(0)
 
         /**

diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/AppDatabase.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/AppDatabase.kt
@@ -61,10 +61,11 @@ abstract class AppDatabase: RoomDatabase() {
         fun appDatabase(
             autoMigrations: Set<@JvmSuppressWildcards AutoMigrationSpec>,
             @ApplicationContext context: Context,
+            manualMigrations: Set<@JvmSuppressWildcards Migration>,
             notificationRegistry: NotificationRegistry
         ): AppDatabase = Room
             .databaseBuilder(context, AppDatabase::class.java, "services.db")
-            .addMigrations(*manualMigrations)
+            .addMigrations(*manualMigrations.toTypedArray())
             .apply {
                 for (spec in autoMigrations)
                     addAutoMigrationSpec(spec)
@@ -95,22 +96,6 @@ abstract class AppDatabase: RoomDatabase() {
     }
 
 
-    companion object {
-
-        val manualMigrations: Array<Migration> = arrayOf(
-            Migration9,
-            Migration8,
-            Migration7,
-            Migration6,
-            Migration5,
-            Migration4,
-            Migration3,
-            Migration2
-        )
-
-    }
-
-
     // DAOs
 
     abstract fun serviceDao(): ServiceDao

diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/AutoMigration12.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/AutoMigration12.kt
@@ -39,7 +39,7 @@ class AutoMigration12 @Inject constructor(
 
     @Module
     @InstallIn(SingletonComponent::class)
-    abstract class SettingsMigrationModule {
+    abstract class AutoMigrationModule {
         @Binds @IntoSet
         abstract fun provide(impl: AutoMigration12): AutoMigrationSpec
     }

diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/AutoMigration16.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/AutoMigration16.kt
@@ -39,7 +39,7 @@ class AutoMigration16 @Inject constructor(): AutoMigrationSpec {
 
     @Module
     @InstallIn(SingletonComponent::class)
-    abstract class SettingsMigrationModule {
+    abstract class AutoMigrationModule {
         @Binds @IntoSet
         abstract fun provide(impl: AutoMigration16): AutoMigrationSpec
     }

diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration2.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration2.kt
@@ -5,6 +5,11 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.Migration
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
 
 val Migration2 = Migration(1, 2) { db ->
     db.execSQL("ALTER TABLE collections ADD COLUMN type TEXT NOT NULL DEFAULT ''")
@@ -14,4 +19,11 @@ val Migration2 = Migration(1, 2) { db ->
             "FROM services WHERE _id=collections.serviceID" +
             ")",
         arrayOf("caldav", "CALENDAR", "ADDRESS_BOOK"))
+}
+
+@Module
+@InstallIn(SingletonComponent::class)
+internal object Migration2Module {
+    @Provides @IntoSet
+    fun provide(): Migration = Migration2
 }
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration3.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration3.kt
@@ -5,10 +5,22 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.Migration
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
 import java.util.logging.Logger
 
 val Migration3 = Migration(2, 3) { db ->
     // We don't have access to the context in a Room migration now, so
     // we will just drop those settings from old DAVx5 versions.
     Logger.getGlobal().warning("Dropping settings distrustSystemCerts and overrideProxy*")
+}
+
+@Module
+@InstallIn(SingletonComponent::class)
+internal object Migration3Module {
+    @Provides @IntoSet
+    fun provide(): Migration = Migration3
 }
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration4.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration4.kt
@@ -5,7 +5,19 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.Migration
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
 
 val Migration4 = Migration(3, 4) { db ->
     db.execSQL("ALTER TABLE collections ADD COLUMN forceReadOnly INTEGER DEFAULT 0 NOT NULL")
+}
+
+@Module
+@InstallIn(SingletonComponent::class)
+internal object Migration4Module {
+    @Provides @IntoSet
+    fun provide(): Migration = Migration4
 }
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration5.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration5.kt
@@ -5,6 +5,11 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.Migration
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
 
 val Migration5 = Migration(4, 5) { db ->
     db.execSQL("ALTER TABLE collections ADD COLUMN privWriteContent INTEGER DEFAULT 0 NOT NULL")
@@ -14,4 +19,11 @@ val Migration5 = Migration(4, 5) { db ->
     db.execSQL("UPDATE collections SET privUnbind=NOT readOnly")
 
     // there's no DROP COLUMN in SQLite, so just keep the "readOnly" column
+}
+
+@Module
+@InstallIn(SingletonComponent::class)
+internal object Migration5Module {
+    @Provides @IntoSet
+    fun provide(): Migration = Migration5
 }
diff --git a/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration6.kt b/app/src/main/kotlin/at/bitfire/davdroid/db/migration/Migration6.kt
@@ -5,6 +5,11 @@
 package at.bitfire.davdroid.db.migration
 
 import androidx.room.migration.Migration
+import dagger.Module
+import dagger.Provides
+import dagger.hilt.InstallIn
+import dagger.hilt.components.SingletonComponent
+import dagger.multibindings.IntoSet
 
 val Migration6 = Migration(5, 6) { db ->
     val sql = arrayOf(
@@ -56,4 +61,11 @@ val Migration6 = Migration(5, 6) { db ->
         "DROP TABLE collections"
     )
     sql.forEach { db.execSQL(it) }
+}
+
+@Module
+@InstallIn(SingletonComponent::class)
+internal object Migration6Module {
+    @Provides @IntoSet
+    fun provide(): Migration = Migration6
 }