db: Drop all tables on downgrade

We previously missed tables that are not known to the schema.  This
becomes an issue if a new table is added at a newer schema level. When
we go back to an earlier schema, that new table remains in the database,
so subsequent attempts to upgrade to the later schema level that adds
the table will fail, because it already exists.

Fixes: #1172

Signed-off-by: Zixuan James Li <[email protected]>

Author: PIG208

lib/model/database.dart

@@ -1,7 +1,9 @@
 import 'package:drift/drift.dart';
+import 'package:drift/internal/versioned_schema.dart';
 import 'package:drift/remote.dart';
 import 'package:sqlite3/common.dart';
 
+import '../log.dart';
 import 'schema_versions.g.dart';
 
 part 'database.g.dart';
@@ -49,6 +51,19 @@ class UriConverter extends TypeConverter<Uri, String> {
   @override Uri fromSql(String fromDb) => Uri.parse(fromDb);
 }
 
+// TODO(upstream): generate this
+VersionedSchema _getSchema({
+  required DatabaseConnectionUser database,
+  required int schemaVersion,
+}) {
+  switch (schemaVersion) {
+    case 2:
+      return Schema2(database: database);
+    default:
+      throw Exception('unknown schema version: $schemaVersion');
+  }
+}
+
 @DriftDatabase(tables: [Accounts])
 class AppDatabase extends _$AppDatabase {
   AppDatabase(super.e);
@@ -65,6 +80,31 @@ class AppDatabase extends _$AppDatabase {
   @override
   int get schemaVersion => 2; // See note.
 
+  Future<void> _dropAndCreateAll(Migrator m, {
+    required int schemaVersion,
+  }) async {
+    await m.database.transaction(() async {
+      final query = m.database.customSelect(
+        "SELECT name FROM sqlite_master WHERE type='table'");
+      for (final row in await query.get()) {
+        final data = row.data;
+        final tableName = data['name'] as String;
+        // Skip sqlite-internal tables.  See for comparison:
+        //   https://www.sqlite.org/fileformat2.html#intschema
+        //   https://github.com/simolus3/drift/blob/0901c984a/drift_dev/lib/src/services/schema/verifier_common.dart#L9-L22
+        if (tableName.startsWith('sqlite_')) continue;
+        // No need to worry about SQL injection; this table name
+        // was already a table name in the database, not something
+        // that should be affected by user data.
+        await m.database.customStatement('DROP TABLE $tableName');
+      }
+      final schema = _getSchema(database: m.database, schemaVersion: schemaVersion);
+      for (final entity in schema.entities) {
+        await m.create(entity);
+      }
+    });
+  }
+
   @override
   MigrationStrategy get migration {
     return MigrationStrategy(
@@ -73,15 +113,11 @@ class AppDatabase extends _$AppDatabase {
       },
       onUpgrade: (Migrator m, int from, int to) async {
         if (from > to) {
-          // TODO(log): log schema downgrade as an error
           // This should only ever happen in dev.  As a dev convenience,
           // drop everything from the database and start over.
-          for (final entity in allSchemaEntities) {
-            // This will miss any entire tables (or indexes, etc.) that
-            // don't exist at this version.  For a dev-only feature, that's OK.
-            await m.drop(entity);
-          }
-          await m.createAll();
+          // TODO(log): log schema downgrade as an error
+          assert(debugLog('Downgrading schema from v$from to v$to.'));
+          await _dropAndCreateAll(m, schemaVersion: to);
           return;
         }
         assert(1 <= from && from <= to && to <= schemaVersion);

test/model/database_test.dart

@@ -1,4 +1,5 @@
 import 'package:checks/checks.dart';
+import 'package:drift/backends.dart';
 import 'package:drift/drift.dart';
 import 'package:drift/native.dart';
 import 'package:drift_dev/api/migrations_native.dart';
@@ -98,6 +99,37 @@ void main() {
       verifier = SchemaVerifier(GeneratedHelper());
     });
 
+    test('downgrading', () async {
+      final schema = await verifier.schemaAt(2);
+
+      // Prepare the database by creating extra tables and columns unknown to
+      // us.  This represent the scenario during developement when checking out
+      // a branch that does not contain some changes to the schema on another
+      // branch.
+      final before = AppDatabase(schema.newConnection());
+      await before.customStatement('CREATE TABLE test_extra (num int)');
+      await before.customStatement('ALTER TABLE accounts ADD extra_column int');
+      await check(verifier.migrateAndValidate(
+        before, 2, validateDropped: true)).throws<SchemaMismatch>();
+      await before.close();
+
+      // We need a new connection because migrations are only run when the
+      // database is first opened (see [DelegatedDatabase.ensureOpen]), and
+      // running custom statements opens it.
+      final after = AppDatabase(schema.newConnection());
+      // Trick the drift into believing that the schema version is
+      // higher than it actually is, so that a downgrade migration is triggered.
+      // This relies on some relevant implementation details:
+      // - [DelegatedDatabase._runMigrations]
+      // - [VerifierImplementation.migrateAndValidate]
+      // TODO(upstream): Expose a better interface for testing this.
+      await ((after.executor as DelegatedDatabase)
+        .delegate.versionDelegate as DynamicVersionDelegate).setSchemaVersion(999);
+
+      await verifier.migrateAndValidate(after, 2, validateDropped: true);
+      await after.close();
+    });
+
     group('migrate without data', () {
       const versions = GeneratedHelper.versions;
       final latestVersion = versions.last;