Upgrade Drift to 2.23.0; start using new features

lib/model/database.dart

@@ -1,12 +1,9 @@
-import 'dart:io';
-
 import 'package:drift/drift.dart';
-import 'package:drift/native.dart';
 import 'package:drift/remote.dart';
-import 'package:path/path.dart' as path;
-import 'package:path_provider/path_provider.dart';
 import 'package:sqlite3/common.dart';
 
+import 'schema_versions.g.dart';
+
 part 'database.g.dart';
 
 /// The table of [Account] records in the app's database.
@@ -52,30 +49,40 @@ class UriConverter extends TypeConverter<Uri, String> {
   @override Uri fromSql(String fromDb) => Uri.parse(fromDb);
 }
 
-LazyDatabase _openConnection() {
-  return LazyDatabase(() async {
-    // TODO decide if this path is the right one to use
-    final dbFolder = await getApplicationDocumentsDirectory();
-    final file = File(path.join(dbFolder.path, 'db.sqlite'));
-    return NativeDatabase.createInBackground(file);
-  });
-}
-
 @DriftDatabase(tables: [Accounts])
 class AppDatabase extends _$AppDatabase {
   AppDatabase(super.e);
 
-  AppDatabase.live() : this(_openConnection());
-
   // When updating the schema:
   //  * Make the change in the table classes, and bump schemaVersion.
-  //  * Export the new schema and generate test migrations:
+  //  * Export the new schema and generate test migrations with drift:
   //    $ tools/check --fix drift
+  //    and generate database code with build_runner.
   //  * Write a migration in `onUpgrade` below.
   //  * Write tests.
   @override
   int get schemaVersion => 2; // See note.
 
+  Future<void> _resetDatabase(Migrator m) async {
+    // This should only ever happen in dev.  As a dev convenience,
+    // drop everything from the database and start over.
+    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, https://www.sqlite.org/fileformat2.html#intschema
+        // https://github.com/simolus3/drift/blob/0901c984a987e54ba0b67e227adbfdcf3c08eeb4/drift_dev/lib/src/services/schema/verifier_common.dart#L9-L22
+        if (tableName.startsWith('sqlite_')) continue;
+        // SQL injection could be a concern, but the database would've been
+        // compromised if the table name is corrupted.
+        await m.database.customStatement('DROP TABLE $tableName');
+      }
+      await m.createAll();
+    });
+  }
+
   @override
   MigrationStrategy get migration {
     return MigrationStrategy(
@@ -85,24 +92,18 @@ 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();
+          await _resetDatabase(m);
           return;
         }
         assert(1 <= from && from <= to && to <= schemaVersion);
 
-        if (from < 2 && 2 <= to) {
-          await m.addColumn(accounts, accounts.ackedPushToken);
-        }
-        // New migrations go here.
-      }
-    );
+        await m.runMigrationSteps(from: from, to: to,
+          steps: migrationSteps(
+            from1To2: (m, schema) async {
+              await m.addColumn(schema.accounts, schema.accounts.ackedPushToken);
+            },
+          ));
+      });
   }
 
   Future<int> createAccount(AccountsCompanion values) async {

lib/model/schema_versions.g.dart

@@ -0,0 +1,112 @@
+// dart format width=80
+import 'package:drift/internal/versioned_schema.dart' as i0;
+import 'package:drift/drift.dart' as i1;
+import 'package:drift/drift.dart'; // ignore_for_file: type=lint,unused_import
+
+// GENERATED BY drift_dev, DO NOT MODIFY.
+final class Schema2 extends i0.VersionedSchema {
+  Schema2({required super.database}) : super(version: 2);
+  @override
+  late final List<i1.DatabaseSchemaEntity> entities = [
+    accounts,
+  ];
+  late final Shape0 accounts = Shape0(
+      source: i0.VersionedTable(
+        entityName: 'accounts',
+        withoutRowId: false,
+        isStrict: false,
+        tableConstraints: [
+          'UNIQUE(realm_url, user_id)',
+          'UNIQUE(realm_url, email)',
+        ],
+        columns: [
+          _column_0,
+          _column_1,
+          _column_2,
+          _column_3,
+          _column_4,
+          _column_5,
+          _column_6,
+          _column_7,
+          _column_8,
+        ],
+        attachedDatabase: database,
+      ),
+      alias: null);
+}
+
+class Shape0 extends i0.VersionedTable {
+  Shape0({required super.source, required super.alias}) : super.aliased();
+  i1.GeneratedColumn<int> get id =>
+      columnsByName['id']! as i1.GeneratedColumn<int>;
+  i1.GeneratedColumn<String> get realmUrl =>
+      columnsByName['realm_url']! as i1.GeneratedColumn<String>;
+  i1.GeneratedColumn<int> get userId =>
+      columnsByName['user_id']! as i1.GeneratedColumn<int>;
+  i1.GeneratedColumn<String> get email =>
+      columnsByName['email']! as i1.GeneratedColumn<String>;
+  i1.GeneratedColumn<String> get apiKey =>
+      columnsByName['api_key']! as i1.GeneratedColumn<String>;
+  i1.GeneratedColumn<String> get zulipVersion =>
+      columnsByName['zulip_version']! as i1.GeneratedColumn<String>;
+  i1.GeneratedColumn<String> get zulipMergeBase =>
+      columnsByName['zulip_merge_base']! as i1.GeneratedColumn<String>;
+  i1.GeneratedColumn<int> get zulipFeatureLevel =>
+      columnsByName['zulip_feature_level']! as i1.GeneratedColumn<int>;
+  i1.GeneratedColumn<String> get ackedPushToken =>
+      columnsByName['acked_push_token']! as i1.GeneratedColumn<String>;
+}
+
+i1.GeneratedColumn<int> _column_0(String aliasedName) =>
+    i1.GeneratedColumn<int>('id', aliasedName, false,
+        hasAutoIncrement: true,
+        type: i1.DriftSqlType.int,
+        defaultConstraints:
+            i1.GeneratedColumn.constraintIsAlways('PRIMARY KEY AUTOINCREMENT'));
+i1.GeneratedColumn<String> _column_1(String aliasedName) =>
+    i1.GeneratedColumn<String>('realm_url', aliasedName, false,
+        type: i1.DriftSqlType.string);
+i1.GeneratedColumn<int> _column_2(String aliasedName) =>
+    i1.GeneratedColumn<int>('user_id', aliasedName, false,
+        type: i1.DriftSqlType.int);
+i1.GeneratedColumn<String> _column_3(String aliasedName) =>
+    i1.GeneratedColumn<String>('email', aliasedName, false,
+        type: i1.DriftSqlType.string);
+i1.GeneratedColumn<String> _column_4(String aliasedName) =>
+    i1.GeneratedColumn<String>('api_key', aliasedName, false,
+        type: i1.DriftSqlType.string);
+i1.GeneratedColumn<String> _column_5(String aliasedName) =>
+    i1.GeneratedColumn<String>('zulip_version', aliasedName, false,
+        type: i1.DriftSqlType.string);
+i1.GeneratedColumn<String> _column_6(String aliasedName) =>
+    i1.GeneratedColumn<String>('zulip_merge_base', aliasedName, true,
+        type: i1.DriftSqlType.string);
+i1.GeneratedColumn<int> _column_7(String aliasedName) =>
+    i1.GeneratedColumn<int>('zulip_feature_level', aliasedName, false,
+        type: i1.DriftSqlType.int);
+i1.GeneratedColumn<String> _column_8(String aliasedName) =>
+    i1.GeneratedColumn<String>('acked_push_token', aliasedName, true,
+        type: i1.DriftSqlType.string);
+i0.MigrationStepWithVersion migrationSteps({
+  required Future<void> Function(i1.Migrator m, Schema2 schema) from1To2,
+}) {
+  return (currentVersion, database) async {
+    switch (currentVersion) {
+      case 1:
+        final schema = Schema2(database: database);
+        final migrator = i1.Migrator(database, schema);
+        await from1To2(migrator, schema);
+        return 2;
+      default:
+        throw ArgumentError.value('Unknown migration from $currentVersion');
+    }
+  };
+}
+
+i1.OnUpgrade stepByStep({
+  required Future<void> Function(i1.Migrator m, Schema2 schema) from1To2,
+}) =>
+    i0.VersionedSchema.stepByStepHelper(
+        step: migrationSteps(
+      from1To2: from1To2,
+    ));

lib/model/store.dart

@@ -778,19 +778,22 @@ class LiveGlobalStore extends GlobalStore {
 
   /// The file path to use for the app database.
   static Future<File> _dbFile() async {
-    // What directory should we use?
-    //   path_provider's getApplicationSupportDirectory:
-    //     on Android, -> Flutter's PathUtils.getFilesDir -> https://developer.android.com/reference/android/content/Context#getFilesDir()
-    //       -> empirically /data/data/com.zulip.flutter/files/
-    //     on iOS, -> "Library/Application Support" via https://developer.apple.com/documentation/foundation/nssearchpathdirectory/nsapplicationsupportdirectory
-    //     on Linux, -> "${XDG_DATA_HOME:-~/.local/share}/com.zulip.flutter/"
-    //     All seem reasonable.
-    //   path_provider's getApplicationDocumentsDirectory:
-    //     on Android, -> Flutter's PathUtils.getDataDirectory -> https://developer.android.com/reference/android/content/Context#getDir(java.lang.String,%20int)
-    //       with https://developer.android.com/reference/android/content/Context#MODE_PRIVATE
-    //     on iOS, "Document directory" via https://developer.apple.com/documentation/foundation/nssearchpathdirectory/nsdocumentdirectory
-    //     on Linux, -> `xdg-user-dir DOCUMENTS` -> e.g. ~/Documents
-    //     That Linux answer is definitely not a fit.  Harder to tell about the rest.
+    // path_provider's getApplicationSupportDirectory:
+    //   on Android, -> Flutter's PathUtils.getFilesDir -> https://developer.android.com/reference/android/content/Context#getFilesDir()
+    //     -> empirically /data/data/com.zulip.flutter/files/
+    //   on iOS, -> "Library/Application Support" via https://developer.apple.com/documentation/foundation/nssearchpathdirectory/nsapplicationsupportdirectory
+    //   on Linux, -> "${XDG_DATA_HOME:-~/.local/share}/com.zulip.flutter/"
+    //
+    // This is reasonable for iOS per Apple's recommendation:
+    // > Use ["Library/Application Support"] to store all app data files except
+    // > those associated with the user’s documents. For example, you might use
+    // > this directory to store app-created data files, configuration files,
+    // > templates, or other fixed or modifiable resources that are managed by
+    // > the app.
+    // See: https://developer.apple.com/library/archive/documentation/FileManagement/Conceptual/FileSystemProgrammingGuide/FileSystemOverview/FileSystemOverview.html#//apple_ref/doc/uid/TP40010672-CH2-SW2
+    //
+    // The paths are reasonable for both Android and Linux, compared to the
+    // ones from using path_provider's getApplicationDocumentsDirectory.
     final dir = await getApplicationSupportDirectory();
     return File(p.join(dir.path, 'zulip.db'));
   }

pubspec.lock

@@ -30,14 +30,6 @@ packages:
       url: "https://pub.dev"
     source: hosted
     version: "6.11.0"
-  analyzer_plugin:
-    dependency: transitive
-    description:
-      name: analyzer_plugin
-      sha256: "9661b30b13a685efaee9f02e5d01ed9f2b423bd889d28a304d02d704aee69161"
-      url: "https://pub.dev"
-    source: hosted
-    version: "0.11.3"
   app_settings:
     dependency: "direct main"
     description:
@@ -283,18 +275,18 @@ packages:
     dependency: "direct main"
     description:
       name: drift
-      sha256: c2d073d35ad441730812f4ea05b5dd031fb81c5f9786a4f5fb77ecd6307b6f74
+      sha256: cc593c8acaccaf4a5750fac034e06289f572038a38391d4343739789591e6b01
       url: "https://pub.dev"
     source: hosted
-    version: "2.22.1"
+    version: "2.23.0"
   drift_dev:
     dependency: "direct dev"
     description:
       name: drift_dev
-      sha256: f4ab5d6976b1e31551ceb82ff597a505bda7818ff4f7be08a1da9d55eb6e730c
+      sha256: fde4fcb5776c2048ecdf672de396e3fdad8e93bf5148b69dd401cf6767a16e0d
       url: "https://pub.dev"
     source: hosted
-    version: "2.22.1"
+    version: "2.23.0"
   fake_async:
     dependency: "direct dev"
     description:

pubspec.yaml

@@ -40,7 +40,7 @@ dependencies:
   convert: ^3.1.1
   crypto: ^3.0.3
   device_info_plus: ^10.0.1
-  drift: ^2.5.0
+  drift: ^2.23.0
   file_picker: ^8.0.0+1
   firebase_core: ^3.3.0
   firebase_messaging: ^15.0.1

test/model/database_test.dart

@@ -98,37 +98,62 @@ void main() {
       verifier = SchemaVerifier(GeneratedHelper());
     });
 
-    test('upgrade to v2, empty', () async {
-      final connection = await verifier.startAt(1);
+    test('downgrading', () async {
+      final connection = await verifier.startAt(2);
       final db = AppDatabase(connection);
-      await verifier.migrateAndValidate(db, 2);
+      await verifier.migrateAndValidate(db, 1);
       await db.close();
-    });
-
-    test('upgrade to v2, with data', () async {
-      final schema = await verifier.schemaAt(1);
-      final before = v1.DatabaseAtV1(schema.newConnection());
-      await before.into(before.accounts).insert(v1.AccountsCompanion.insert(
-        realmUrl: 'https://chat.example/',
-        userId: 1,
-        email: '[email protected]',
-        apiKey: '1234',
-        zulipVersion: '6.0',
-        zulipMergeBase: const Value('6.0'),
-        zulipFeatureLevel: 42,
-      ));
-      final accountV1 = await before.select(before.accounts).watchSingle().first;
-      await before.close();
+    }, skip: true); // TODO(#1172): unskip this
 
-      final db = AppDatabase(schema.newConnection());
-      await verifier.migrateAndValidate(db, 2);
-      await db.close();
+    group('migrate without data', () {
+      // These simple tests verify all possible schema updates with a simple (no
+      // data) migration. This is a quick way to ensure that written database
+      // migrations properly alter the schema.
+      const versions = GeneratedHelper.versions;
+      for (final (i, fromVersion) in versions.indexed) {
+        group('from $fromVersion', () {
+          for (final toVersion in versions.skip(i + 1)) {
+            test('to $toVersion', () async {
+              final schema = await verifier.schemaAt(fromVersion);
+              final db = AppDatabase(schema.newConnection());
+              await verifier.migrateAndValidate(db, toVersion);
+              await db.close();
+            });
+          }
+        });
+      }
+    });
 
-      final after = v2.DatabaseAtV2(schema.newConnection());
-      final account = await after.select(after.accounts).getSingle();
-      check(account.toJson()).deepEquals({
-        ...accountV1.toJson(),
-        'ackedPushToken': null,
+    // Testing this can be useful for migrations that change existing columns
+    // (e.g. by alterating their type or constraints). Migrations that only add
+    // tables or columns typically don't need these advanced tests. For more
+    // information, see https://drift.simonbinder.eu/migrations/tests/#verifying-data-integrity
+    group('migrate with data', () {
+      test('upgrade to v2', () async {
+        late final v1.AccountsData oldAccountData;
+        await verifier.testWithDataIntegrity(
+          oldVersion: 1, createOld: v1.DatabaseAtV1.new,
+          newVersion: 2, createNew: v2.DatabaseAtV2.new,
+          openTestedDatabase: AppDatabase.new,
+          createItems: (batch, oldDb) async {
+            await oldDb.into(oldDb.accounts).insert(v1.AccountsCompanion.insert(
+              realmUrl: 'https://chat.example/',
+              userId: 1,
+              email: '[email protected]',
+              apiKey: '1234',
+              zulipVersion: '6.0',
+              zulipMergeBase: const Value('6.0'),
+              zulipFeatureLevel: 42,
+            ));
+            oldAccountData = await oldDb.select(oldDb.accounts).watchSingle().first;
+          },
+          validateItems: (newDb) async {
+            final account = await newDb.select(newDb.accounts).getSingle();
+            check(account.toJson()).deepEquals({
+              ...oldAccountData.toJson(),
+              'ackedPushToken': null,
+            });
+          });
       });
     });
   });