Migrate the trash index forward instead of dropping it

SqliteTrashIndex.onUpgrade emptied the table and recreated it, which was defensible only while nothing had shipped. It loses more than a cache: the staged files are still on disk, but only this table knows where they came from, so emptying it turns every staged delete into a permanent one, silently, with no error raised anywhere.

onCreate and onUpgrade now delegate to a create/upgrade pair on the companion, which is internal so a test can drive it against a throwaway database. Version 1 is the first version this app has shipped, so the loop has no steps yet and DATABASE_VERSION stays 1 — the shape is the point. When a version 2 arrives the step for version 1 becomes the ALTER TABLE plus backfill, or the copy-and-swap for a change SQLite cannot make in place. A version with no step now reports itself with error() rather than wiping the table, so the failure mode of forgetting a migration is a loud message instead of quiet data loss.

Two tests pin it against real SQLite: a bump to a version with no migration must report itself and leave the rows intact (the old code would have returned normally with an empty table, which is exactly what this guards), and an already current schema is a no-op. 206 JVM tests, lint clean.

app/src/main/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndex.kt

@@ -85,7 +85,33 @@ class SqliteTrashIndex(context: Context) : TrashIndex {
     private class Helper(context: Context) :
         SQLiteOpenHelper(context, DATABASE_NAME, null, DATABASE_VERSION) {
 
-        override fun onCreate(db: SQLiteDatabase) {
+        override fun onCreate(db: SQLiteDatabase) = create(db)
+
+        override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) =
+            upgrade(db, oldVersion, newVersion)
+    }
+
+    /**
+     * The table's shape, and how that shape moves forward.
+     *
+     * `internal` rather than `private` so a test can drive `create` and `upgrade` against a
+     * throwaway database, which is the only way to prove the upgrade path without waiting for
+     * a real version 2 to exist.
+     */
+    internal companion object {
+        const val DATABASE_NAME = "trash-index.db"
+        const val DATABASE_VERSION = 1
+        const val TABLE = "trashed"
+        const val COLUMN_STAGED_PATH = "staged_path"
+        const val COLUMN_BATCH_ID = "batch_id"
+        const val COLUMN_BATCH_FOLDER = "batch_folder"
+        const val COLUMN_ORIGINAL_PATH = "original_path"
+        const val COLUMN_NAME = "name"
+        const val COLUMN_IS_DIRECTORY = "is_directory"
+        const val COLUMN_SIZE_BYTES = "size_bytes"
+        const val COLUMN_STAGED_AT = "staged_at"
+
+        fun create(db: SQLiteDatabase) {
             db.execSQL(
                 """
                 CREATE TABLE $TABLE (
@@ -104,27 +130,33 @@ class SqliteTrashIndex(context: Context) : TrashIndex {
             db.execSQL("CREATE INDEX trashed_staged_at ON $TABLE ($COLUMN_STAGED_AT)")
         }
 
-        override fun onUpgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+        /**
-            // Dropping rather than migrating, which is only acceptable while nothing has
+         * Bring [db] from [oldVersion] up to [newVersion], one version at a time.
-            // shipped: unlike every other kind of cache here, this one cannot be rebuilt
+         *
-            // from the device, because the device never recorded where the items came from.
+         * Version 1 is the first version this app has shipped, so there is nothing to migrate
-            // A released version needs a real migration.
+         * yet — the loop is empty and [DATABASE_VERSION] is still 1. The *shape* is the point.
-            db.execSQL("DROP TABLE IF EXISTS $TABLE")
+         * When version 2 arrives, the step for version 1 becomes the work that gets there: an
-            onCreate(db)
+         * `ALTER TABLE` plus a backfill for an added column, or a copy into a new table and a
+         * swap for a change SQLite cannot make in place.
+         *
+         * **A bare `DROP` must never appear here.** This is the one cache in the app that
+         * cannot be rebuilt from the device. The staged files are still on the disk, but only
+         * this table knows where they came from, so emptying it does not lose a cache — it
+         * turns every staged delete into a permanent one, silently, with every row still
+         * present on disk and no error raised anywhere. The old code did exactly that, which
+         * was defensible only while nothing had shipped.
+         *
+         * A version with no step is therefore a bug to be reported, not an excuse to wipe the
+         * table, which is why this ends in an `error` rather than in [create].
+         */
+        fun upgrade(db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
+            for (version in oldVersion until newVersion) {
+                error(
+                    "No migration from trash-index schema version $version to ${version + 1}. " +
+                        "Add one to SqliteTrashIndex.upgrade and bump DATABASE_VERSION; " +
+                        "do not drop the table.",
+                )
             }
         }
-
-    private companion object {
-        const val DATABASE_NAME = "trash-index.db"
-        const val DATABASE_VERSION = 1
-        const val TABLE = "trashed"
-        const val COLUMN_STAGED_PATH = "staged_path"
-        const val COLUMN_BATCH_ID = "batch_id"
-        const val COLUMN_BATCH_FOLDER = "batch_folder"
-        const val COLUMN_ORIGINAL_PATH = "original_path"
-        const val COLUMN_NAME = "name"
-        const val COLUMN_IS_DIRECTORY = "is_directory"
-        const val COLUMN_SIZE_BYTES = "size_bytes"
-        const val COLUMN_STAGED_AT = "staged_at"
     }
 }

app/src/test/java/de/softwareapp_hb/filemanager/data/SqliteTrashIndexTest.kt

@@ -1,6 +1,8 @@
 package de.softwareapp_hb.filemanager.data
 
+import android.database.sqlite.SQLiteDatabase
 import de.softwareapp_hb.filemanager.core.StoragePath
+import java.io.File
 import kotlinx.coroutines.runBlocking
 import org.junit.Assert.assertEquals
 import org.junit.Assert.assertTrue
@@ -85,4 +87,58 @@ class SqliteTrashIndexTest {
     fun `an empty index reads back empty rather than throwing`() = runBlocking {
         assertTrue(index().records().isEmpty())
     }
+
+    /**
+     * The upgrade path used to `DROP` the table and recreate it. That loses more than a cache:
+     * the staged files stay on disk, but nothing knows where they came from any more, so every
+     * staged delete silently becomes permanent — which is the one thing the trash exists to
+     * prevent. A version with no migration must therefore report itself and leave the rows be.
+     *
+     * Version 2 does not exist yet, which is exactly why this is worth pinning: the guard has
+     * to hold *before* the first real migration is written, not after.
+     */
+    @Test
+    fun `a schema bump with no migration reports itself instead of emptying the index`() {
+        val db = scratchDatabase("migration-undefined")
+        SqliteTrashIndex.create(db)
+        insertOneRow(db)
+
+        val failure = runCatching { SqliteTrashIndex.upgrade(db, 1, 2) }
+
+        assertTrue("expected the missing step to be reported", failure.isFailure)
+        assertEquals("a version bump must not empty the index", 1, rowCount(db))
+    }
+
+    @Test
+    fun `an already current schema is left alone`() {
+        val db = scratchDatabase("migration-current")
+        SqliteTrashIndex.create(db)
+        insertOneRow(db)
+
+        SqliteTrashIndex.upgrade(db, SqliteTrashIndex.DATABASE_VERSION, SqliteTrashIndex.DATABASE_VERSION)
+
+        assertEquals(1, rowCount(db))
+    }
+
+    /** A throwaway database, so a migration can be driven without disturbing the real index. */
+    private fun scratchDatabase(name: String): SQLiteDatabase =
+        SQLiteDatabase.openOrCreateDatabase(
+            File(RuntimeEnvironment.getApplication().cacheDir, "$name.db").apply { delete() },
+            null,
+        )
+
+    private fun insertOneRow(db: SQLiteDatabase) {
+        db.execSQL(
+            "INSERT INTO ${SqliteTrashIndex.TABLE} " +
+                "(${SqliteTrashIndex.COLUMN_STAGED_PATH}, ${SqliteTrashIndex.COLUMN_BATCH_ID}, " +
+                "${SqliteTrashIndex.COLUMN_BATCH_FOLDER}, ${SqliteTrashIndex.COLUMN_ORIGINAL_PATH}, " +
+                "${SqliteTrashIndex.COLUMN_NAME}, ${SqliteTrashIndex.COLUMN_IS_DIRECTORY}, " +
+                "${SqliteTrashIndex.COLUMN_SIZE_BYTES}, ${SqliteTrashIndex.COLUMN_STAGED_AT}) " +
+                "VALUES ('/t/batch-1/0/a.txt', 'batch-1', '/t/batch-1', " +
+                "'/storage/emulated/0/Documents/a.txt', 'a.txt', 0, 42, 1000)",
+        )
+    }
+
+    private fun rowCount(db: SQLiteDatabase): Int =
+        db.query(SqliteTrashIndex.TABLE, null, null, null, null, null, null).use { it.count }
 }