diff --git a/lib/model/database.dart b/lib/model/database.dart index 6ca2aa3726..ae50ab2765 100644 --- a/lib/model/database.dart +++ b/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,42 @@ class UriConverter extends TypeConverter { @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. + // See ../../README.md#generated-files for more + // information on using the build_runner. // * Write a migration in `onUpgrade` below. // * Write tests. @override int get schemaVersion => 2; // See note. + Future _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 +94,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 createAccount(AccountsCompanion values) async { diff --git a/lib/model/schema_versions.g.dart b/lib/model/schema_versions.g.dart new file mode 100644 index 0000000000..300813c53e --- /dev/null +++ b/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 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 get id => + columnsByName['id']! as i1.GeneratedColumn; + i1.GeneratedColumn get realmUrl => + columnsByName['realm_url']! as i1.GeneratedColumn; + i1.GeneratedColumn get userId => + columnsByName['user_id']! as i1.GeneratedColumn; + i1.GeneratedColumn get email => + columnsByName['email']! as i1.GeneratedColumn; + i1.GeneratedColumn get apiKey => + columnsByName['api_key']! as i1.GeneratedColumn; + i1.GeneratedColumn get zulipVersion => + columnsByName['zulip_version']! as i1.GeneratedColumn; + i1.GeneratedColumn get zulipMergeBase => + columnsByName['zulip_merge_base']! as i1.GeneratedColumn; + i1.GeneratedColumn get zulipFeatureLevel => + columnsByName['zulip_feature_level']! as i1.GeneratedColumn; + i1.GeneratedColumn get ackedPushToken => + columnsByName['acked_push_token']! as i1.GeneratedColumn; +} + +i1.GeneratedColumn _column_0(String aliasedName) => + i1.GeneratedColumn('id', aliasedName, false, + hasAutoIncrement: true, + type: i1.DriftSqlType.int, + defaultConstraints: + i1.GeneratedColumn.constraintIsAlways('PRIMARY KEY AUTOINCREMENT')); +i1.GeneratedColumn _column_1(String aliasedName) => + i1.GeneratedColumn('realm_url', aliasedName, false, + type: i1.DriftSqlType.string); +i1.GeneratedColumn _column_2(String aliasedName) => + i1.GeneratedColumn('user_id', aliasedName, false, + type: i1.DriftSqlType.int); +i1.GeneratedColumn _column_3(String aliasedName) => + i1.GeneratedColumn('email', aliasedName, false, + type: i1.DriftSqlType.string); +i1.GeneratedColumn _column_4(String aliasedName) => + i1.GeneratedColumn('api_key', aliasedName, false, + type: i1.DriftSqlType.string); +i1.GeneratedColumn _column_5(String aliasedName) => + i1.GeneratedColumn('zulip_version', aliasedName, false, + type: i1.DriftSqlType.string); +i1.GeneratedColumn _column_6(String aliasedName) => + i1.GeneratedColumn('zulip_merge_base', aliasedName, true, + type: i1.DriftSqlType.string); +i1.GeneratedColumn _column_7(String aliasedName) => + i1.GeneratedColumn('zulip_feature_level', aliasedName, false, + type: i1.DriftSqlType.int); +i1.GeneratedColumn _column_8(String aliasedName) => + i1.GeneratedColumn('acked_push_token', aliasedName, true, + type: i1.DriftSqlType.string); +i0.MigrationStepWithVersion migrationSteps({ + required Future 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 Function(i1.Migrator m, Schema2 schema) from1To2, +}) => + i0.VersionedSchema.stepByStepHelper( + step: migrationSteps( + from1To2: from1To2, + )); diff --git a/pubspec.yaml b/pubspec.yaml index cc1d93cca8..c17bebe704 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -40,7 +40,7 @@ dependencies: convert: ^3.1.1 crypto: ^3.0.3 device_info_plus: ^11.2.0 - drift: ^2.5.0 + drift: ^2.23.0 file_picker: ^8.0.0+1 firebase_core: ^3.3.0 firebase_messaging: ^15.0.1 diff --git a/test/model/database_test.dart b/test/model/database_test.dart index cb3a7d299b..85bfd9c232 100644 --- a/test/model/database_test.dart +++ b/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: 'asdf@example.org', - 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: 'asdf@example.org', + 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, + }); + }); }); }); }); diff --git a/tools/check b/tools/check index fefcd514ed..392c11d634 100755 --- a/tools/check +++ b/tools/check @@ -378,6 +378,7 @@ run_l10n() { run_drift() { local schema_dir=test/model/schemas/ + local migration_helper_path=lib/model/schema_versions.g.dart # Omitted from this check: # pubspec.{yaml,lock} tools/check @@ -386,6 +387,8 @@ run_drift() { check_no_uncommitted_or_untracked "${schema_dir}" \ || return + check_no_uncommitted_or_untracked "${migration_helper_path}" \ + || return dart run drift_dev schema dump \ lib/model/database.dart "${schema_dir}" \ @@ -393,8 +396,12 @@ run_drift() { dart run drift_dev schema generate --data-classes --companions \ "${schema_dir}" "${schema_dir}" \ || return + dart run drift_dev schema steps \ + "${schema_dir}" "${migration_helper_path}" \ + || return check_no_changes "schema updates" "${schema_dir}" + check_no_changes "migration helper updates" "${migration_helper_path}" } filter_flutter_pub_run_output() {