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.

The test for this relies on some undocumented drift internals to
modify the schema version it sees when running the migration.
References:
  https://github.com/simolus3/drift/blob/18cede15/drift/lib/src/runtime/executor/helpers/engines.dart#L459-L495
  https://github.com/simolus3/drift/blob/18cede15/drift/lib/src/sqlite3/database.dart#L198-L211
  https://github.com/simolus3/sqlite3.dart/blob/4de46afd/sqlite3/lib/src/implementation/database.dart#L69-L85

Fixes: zulip#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(drift): 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);
@@ -60,11 +75,37 @@ class AppDatabase extends _$AppDatabase {
   //    and generate database code with build_runner.
   //    See ../../README.md#generated-files for more
   //    information on using the build_runner.
+  //  * Update [_getSchema] to handle the new schemaVersion.
   //  * Write a migration in `onUpgrade` below.
   //  * Write tests.
   @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 +114,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

@@ -98,6 +98,29 @@ void main() {
       verifier = SchemaVerifier(GeneratedHelper());
     });
 
+    test('downgrading', () async {
+      final schema = await verifier.schemaAt(2);
+
+      // This simulates the scenario during development when running the app
+      // with a future schema version that has additional tables and columns.
+      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>();
+      // Override the schema version by modifying the underlying value
+      // drift internally keeps track of in the database.
+      // TODO(drift): Expose a better interface for testing this.
+      await before.customStatement('PRAGMA user_version = 999;');
+      await before.close();
+
+      // Simulate starting up the app, with an older schema version that
+      // does not have the extra tables and columns.
+      final after = AppDatabase(schema.newConnection());
+      await verifier.migrateAndValidate(after, 2, validateDropped: true);
+      await after.close();
+    });
+
     group('migrate without data', () {
       const versions = GeneratedHelper.versions;
       final latestVersion = versions.last;