From 362acd6cab8c6b8a1f28ff9c8dd4f6d14860adb4 Mon Sep 17 00:00:00 2001 From: hayaksi1 <193020925+hayaksi1@users.noreply.github.com> Date: Tue, 25 Aug 2026 21:41:17 +0300 Subject: [PATCH] Run the database migrations again when a database is upgraded Upgrading over an older install crashed before the login screen with `SQLiteException: no such column: SessionData.position`. The session, cache and push databases were opened at their new schema version but their tables were never migrated. `SqlCipherDriverFactory` overrides `AndroidSqliteDriver.Callback.onUpgrade` so the database key can be replaced, and never called through to the base implementation - which is the only thing that runs `schema.migrate`. Every SQLDelight migration in the app was skipped as a result. The override now calls `super.onUpgrade` after the re-key, so a database that crosses the re-key version is still re-encrypted before its tables are migrated. It moved out of an anonymous object into a named `MigrationCallback` so the behaviour can be tested. Part of #7509 --- libraries/encrypted-db/build.gradle.kts | 4 ++ .../encrypteddb/SqlCipherDriverFactory.kt | 30 +++++++--- .../encrypteddb/MigrationCallbackTest.kt | 55 +++++++++++++++++++ 3 files changed, 82 insertions(+), 7 deletions(-) create mode 100644 libraries/encrypted-db/src/test/kotlin/io/element/encrypteddb/MigrationCallbackTest.kt diff --git a/libraries/encrypted-db/build.gradle.kts b/libraries/encrypted-db/build.gradle.kts index a2150945a5..d48d4aefd1 100644 --- a/libraries/encrypted-db/build.gradle.kts +++ b/libraries/encrypted-db/build.gradle.kts @@ -5,6 +5,8 @@ * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. * Please see LICENSE files in the repository root for full details. */ +import extension.testCommonDependencies + plugins { id("io.element.android-library") } @@ -27,4 +29,6 @@ dependencies { implementation(libs.google.tink) implementation(projects.libraries.androidutils) + + testCommonDependencies(libs) } diff --git a/libraries/encrypted-db/src/main/kotlin/io/element/encrypteddb/SqlCipherDriverFactory.kt b/libraries/encrypted-db/src/main/kotlin/io/element/encrypteddb/SqlCipherDriverFactory.kt index 84ab2d2a18..0f2e954b1f 100644 --- a/libraries/encrypted-db/src/main/kotlin/io/element/encrypteddb/SqlCipherDriverFactory.kt +++ b/libraries/encrypted-db/src/main/kotlin/io/element/encrypteddb/SqlCipherDriverFactory.kt @@ -51,12 +51,28 @@ class SqlCipherDriverFactory( is ClientSecret.Passphrase -> secret.formattedAsString().hexToByteArray() } val factory = SupportOpenHelperFactory(key) - return AndroidSqliteDriver(schema = schema, context = context, name = name, factory = factory, callback = object : AndroidSqliteDriver.Callback( - schema - ) { - override fun onUpgrade(db: SupportSQLiteDatabase, oldVersion: Int, newVersion: Int) { - onUpgradeCallback?.invoke(db, oldVersion, newVersion) - } - }) + return AndroidSqliteDriver( + schema = schema, + context = context, + name = name, + factory = factory, + callback = MigrationCallback(schema, onUpgradeCallback), + ) + } +} + +/** + * Runs [onUpgradeCallback] and then the schema migrations themselves. + * + * @param schema The SQLite DB schema, whose migrations are applied by [AndroidSqliteDriver.Callback.onUpgrade]. + * @param onUpgradeCallback Called before the migrations run, so that the database key can be replaced first. + */ +internal class MigrationCallback( + schema: SqlSchema>, + private val onUpgradeCallback: ((driver: SupportSQLiteDatabase, oldVersion: Int, newVersion: Int) -> Unit)?, +) : AndroidSqliteDriver.Callback(schema) { + override fun onUpgrade(db: SupportSQLiteDatabase, oldVersion: Int, newVersion: Int) { + onUpgradeCallback?.invoke(db, oldVersion, newVersion) + super.onUpgrade(db, oldVersion, newVersion) } } diff --git a/libraries/encrypted-db/src/test/kotlin/io/element/encrypteddb/MigrationCallbackTest.kt b/libraries/encrypted-db/src/test/kotlin/io/element/encrypteddb/MigrationCallbackTest.kt new file mode 100644 index 0000000000..5e16184951 --- /dev/null +++ b/libraries/encrypted-db/src/test/kotlin/io/element/encrypteddb/MigrationCallbackTest.kt @@ -0,0 +1,55 @@ +/* + * Copyright (c) 2026 Element Creations Ltd. + * + * SPDX-License-Identifier: AGPL-3.0-only OR LicenseRef-Element-Commercial. + * Please see LICENSE files in the repository root for full details. + */ + +package io.element.encrypteddb + +import app.cash.sqldelight.db.AfterVersion +import app.cash.sqldelight.db.QueryResult +import app.cash.sqldelight.db.SqlDriver +import app.cash.sqldelight.db.SqlSchema +import com.google.common.truth.Truth.assertThat +import io.mockk.mockk +import org.junit.Test + +class MigrationCallbackTest { + @Test + fun `onUpgrade migrates the schema`() { + val schema = RecordingSchema() + MigrationCallback(schema, onUpgradeCallback = null).onUpgrade(mockk(relaxed = true), 9, 11) + assertThat(schema.migrations).containsExactly(9L to 11L) + } + + @Test + fun `onUpgrade replaces the database key before migrating the schema`() { + val order = mutableListOf() + val schema = RecordingSchema(onMigrate = { order.add("migrate") }) + MigrationCallback(schema, onUpgradeCallback = { _, _, _ -> order.add("rekey") }) + .onUpgrade(mockk(relaxed = true), 9, 11) + assertThat(order).containsExactly("rekey", "migrate").inOrder() + } +} + +private class RecordingSchema( + private val onMigrate: () -> Unit = {}, +) : SqlSchema> { + val migrations = mutableListOf>() + + override val version = 11L + + override fun create(driver: SqlDriver) = QueryResult.Unit + + override fun migrate( + driver: SqlDriver, + oldVersion: Long, + newVersion: Long, + vararg callbacks: AfterVersion, + ): QueryResult.Value { + migrations.add(oldVersion to newVersion) + onMigrate() + return QueryResult.Unit + } +}