From 7e88e74a6c128273e89e9b7314d03a89d71dc15c Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Fri, 16 Oct 2020 17:16:45 +0200 Subject: [PATCH] Add tests for schema verification, fix inconsistencies --- .../analyzer/moor/create_table_reader.dart | 17 ++-- .../lib/src/model/declarations/tables.dart | 25 ++++-- moor_generator/lib/src/model/table.dart | 3 +- .../src/services/schema/find_differences.dart | 59 ++++++++++++-- .../lib/src/services/schema/schema_files.dart | 9 ++- .../src/services/schema/verifier_impl.dart | 2 +- .../lib/src/writer/tables/table_writer.dart | 6 +- .../schema/find_differences_test.dart | 81 +++++++++++++++++++ .../test/services/schema/writer_test.dart | 71 +++++++++++++--- 9 files changed, 231 insertions(+), 42 deletions(-) create mode 100644 moor_generator/test/services/schema/find_differences_test.dart diff --git a/moor_generator/lib/src/analyzer/moor/create_table_reader.dart b/moor_generator/lib/src/analyzer/moor/create_table_reader.dart index fdc854e1..297e42ec 100644 --- a/moor_generator/lib/src/analyzer/moor/create_table_reader.dart +++ b/moor_generator/lib/src/analyzer/moor/create_table_reader.dart @@ -40,10 +40,9 @@ class CreateTableReader { } final foundColumns = {}; - final primaryKey = {}; + Set primaryKeyFromTableConstraint; for (final column in table.resultColumns) { - var isPrimaryKey = false; final features = []; final sqlName = column.name; final dartName = ReCase(sqlName).camelCase; @@ -87,7 +86,6 @@ class CreateTableReader { : const Iterable.empty(); for (final constraint in constraints) { if (constraint is PrimaryKeyColumn) { - isPrimaryKey = true; features.add(const PrimaryKey()); if (constraint.autoIncrement) { features.add(AutoIncrement()); @@ -153,9 +151,6 @@ class CreateTableReader { ); foundColumns[column.name] = parsed; - if (isPrimaryKey) { - primaryKey.add(parsed); - } } final tableName = table.name; @@ -167,9 +162,11 @@ class CreateTableReader { for (final keyConstraint in table.tableConstraints.whereType()) { if (keyConstraint.isPrimaryKey) { - primaryKey.addAll(keyConstraint.indexedColumns - .map((r) => foundColumns[r.columnName]) - .where((c) => c != null)); + primaryKeyFromTableConstraint = { + for (final column in keyConstraint.indexedColumns) + if (foundColumns.containsKey(column.columnName)) + foundColumns[column.columnName] + }; } } @@ -179,7 +176,7 @@ class CreateTableReader { sqlName: table.name, dartTypeName: dataClassName, overriddenName: dartTableName, - primaryKey: primaryKey, + primaryKey: primaryKeyFromTableConstraint, overrideWithoutRowId: table.withoutRowId ? true : null, overrideTableConstraints: constraints.isNotEmpty ? constraints : null, // we take care of writing the primary key ourselves diff --git a/moor_generator/lib/src/model/declarations/tables.dart b/moor_generator/lib/src/model/declarations/tables.dart index 31f78008..ed446ed2 100644 --- a/moor_generator/lib/src/model/declarations/tables.dart +++ b/moor_generator/lib/src/model/declarations/tables.dart @@ -8,6 +8,9 @@ abstract class TableDeclaration extends Declaration { abstract class TableDeclarationWithSql implements TableDeclaration { /// The `CREATE TABLE` statement used to create this table. String get createSql; + + /// The parsed statement creating this table. + TableInducingStatement get creatingStatement; } class DartTableDeclaration implements TableDeclaration, DartDeclaration { @@ -38,12 +41,6 @@ class MoorTableDeclaration @override final TableInducingStatement node; - @override - bool get isVirtual => node is CreateVirtualTableStatement; - - @override - String get createSql => node.span.text; - MoorTableDeclaration._(this.declaration, this.node); factory MoorTableDeclaration(TableInducingStatement node, FoundFile file) { @@ -52,13 +49,22 @@ class MoorTableDeclaration node, ); } + + @override + bool get isVirtual => node is CreateVirtualTableStatement; + + @override + String get createSql => node.span.text; + + @override + TableInducingStatement get creatingStatement => node; } class CustomVirtualTableDeclaration implements TableDeclarationWithSql { @override - final String createSql; + final CreateVirtualTableStatement creatingStatement; - CustomVirtualTableDeclaration(this.createSql); + CustomVirtualTableDeclaration(this.creatingStatement); @override SourceRange get declaration { @@ -67,6 +73,9 @@ class CustomVirtualTableDeclaration implements TableDeclarationWithSql { @override bool get isVirtual => true; + + @override + String get createSql => creatingStatement.span.text; } class CustomTableDeclaration implements TableDeclaration { diff --git a/moor_generator/lib/src/model/table.dart b/moor_generator/lib/src/model/table.dart index 4d0dc1a0..80abb437 100644 --- a/moor_generator/lib/src/model/table.dart +++ b/moor_generator/lib/src/model/table.dart @@ -114,8 +114,7 @@ class MoorTable implements MoorSchemaEntity { String get createVirtual { if (!isVirtualTable) return null; - final node = (declaration as MoorTableDeclaration).node; - return (node as CreateVirtualTableStatement).span.text; + return (declaration as TableDeclarationWithSql).createSql; } MoorTable({ diff --git a/moor_generator/lib/src/services/schema/find_differences.dart b/moor_generator/lib/src/services/schema/find_differences.dart index c16d2345..d3983a33 100644 --- a/moor_generator/lib/src/services/schema/find_differences.dart +++ b/moor_generator/lib/src/services/schema/find_differences.dart @@ -111,6 +111,22 @@ class FindSchemaDifferences { CreateTableStatement ref, CreateTableStatement act) { final results = {}; + results['columns'] = _compareColumns(ref.columns, act.columns); + + // We're currently comparing table constraints by their exact order. + if (ref.tableConstraints.length != act.tableConstraints.length) { + results['constraints'] = FoundDifference( + 'Expected the table to have ${ref.tableConstraints.length} table ' + 'constraints, it actually has ${act.tableConstraints.length}.'); + } else { + for (var i = 0; i < ref.tableConstraints.length; i++) { + final refConstraint = ref.tableConstraints[i]; + final actConstraint = act.tableConstraints[i]; + + results['constraints_$i'] = _compareByAst(refConstraint, actConstraint); + } + } + if (ref.withoutRowId != act.withoutRowId) { final expectedWithout = ref.withoutRowId; results['rowid'] = FoundDifference(expectedWithout @@ -118,7 +134,35 @@ class FindSchemaDifferences { : 'Did not expect the table to have a WITHOUT ROWID clause.'); } - return const Success(); + return MultiResult(results); + } + + CompareResult _compareColumns( + List ref, List act) { + final results = {}; + + final actByName = {for (final column in act) column.columnName: column}; + // Additional columns in act that ref doesn't have. Built by iterating over + // ref. + final additionalColumns = actByName.keys.toSet(); + + for (final refColumn in ref) { + final name = refColumn.columnName; + final actColumn = actByName[name]; + + if (actColumn == null) { + results[name] = FoundDifference('Missing in schema'); + } else { + results[name] = _compareByAst(refColumn, actColumn); + additionalColumns.remove(name); + } + } + + for (final additional in additionalColumns) { + results[additional] = FoundDifference('Additional unexpected column'); + } + + return MultiResult(results); } CompareResult _compareByAst(AstNode a, AstNode b) { @@ -153,7 +197,8 @@ abstract class CompareResult { bool get noChanges; - String describe(int indent); + String describe() => _describe(0); + String _describe(int indent); } class Success extends CompareResult { @@ -163,7 +208,7 @@ class Success extends CompareResult { bool get noChanges => true; @override - String describe(int indent) => '${' ' * indent}matches schema ✓'; + String _describe(int indent) => '${' ' * indent}matches schema ✓'; } class FoundDifference extends CompareResult { @@ -175,7 +220,7 @@ class FoundDifference extends CompareResult { bool get noChanges => false; @override - String describe(int indent) => ' ' * indent + description; + String _describe(int indent) => ' ' * indent + description; } class MultiResult extends CompareResult { @@ -187,14 +232,16 @@ class MultiResult extends CompareResult { bool get noChanges => nestedResults.values.every((e) => e.noChanges); @override - String describe(int indent) { + String _describe(int indent) { final buffer = StringBuffer(); final indentStr = ' ' * indent; for (final result in nestedResults.entries) { + if (result.value.noChanges) continue; + buffer ..writeln('$indentStr${result.key}:') - ..writeln(result.value.describe(indent + 1)); + ..writeln(result.value._describe(indent + 1)); } return buffer.toString(); diff --git a/moor_generator/lib/src/services/schema/schema_files.dart b/moor_generator/lib/src/services/schema/schema_files.dart index f1f91564..0c5ca8bd 100644 --- a/moor_generator/lib/src/services/schema/schema_files.dart +++ b/moor_generator/lib/src/services/schema/schema_files.dart @@ -1,5 +1,6 @@ import 'package:moor_generator/moor_generator.dart'; import 'package:recase/recase.dart'; +import 'package:sqlparser/sqlparser.dart'; const _infoVersion = '0.1.0-dev-preview'; @@ -129,6 +130,8 @@ class SchemaReader { final Set _currentlyProcessing = {}; + final SqlEngine _engine = SqlEngine(); + SchemaReader._(); factory SchemaReader.readJson(Map json) { @@ -221,11 +224,13 @@ class SchemaReader { if (isVirtual) { final create = content['create_virtual_stmt'] as String; + final parsed = + _engine.parse(create).rootNode as CreateVirtualTableStatement; return MoorTable( sqlName: sqlName, overriddenName: sqlName, - declaration: CustomVirtualTableDeclaration(create), + declaration: CustomVirtualTableDeclaration(parsed), overrideWithoutRowId: withoutRowId, ); } @@ -244,7 +249,7 @@ class SchemaReader { if (content.containsKey('explicit_pk')) { explicitPk = { for (final columnName in content['explicit_pk'] as List) - columns.singleWhere((c) => c.name == columnName) + columns.singleWhere((c) => c.name.name == columnName) }; } diff --git a/moor_generator/lib/src/services/schema/verifier_impl.dart b/moor_generator/lib/src/services/schema/verifier_impl.dart index ff742e29..a3ffb854 100644 --- a/moor_generator/lib/src/services/schema/verifier_impl.dart +++ b/moor_generator/lib/src/services/schema/verifier_impl.dart @@ -29,7 +29,7 @@ class VerifierImplementation implements SchemaVerifier { .compare(); if (!result.noChanges) { - throw SchemaMismatch(result.describe(0)); + throw SchemaMismatch(result.describe()); } } diff --git a/moor_generator/lib/src/writer/tables/table_writer.dart b/moor_generator/lib/src/writer/tables/table_writer.dart index a8b295f8..e9456715 100644 --- a/moor_generator/lib/src/writer/tables/table_writer.dart +++ b/moor_generator/lib/src/writer/tables/table_writer.dart @@ -246,7 +246,7 @@ class TableWriter { void _writePrimaryKeyOverride() { _buffer.write('@override\nSet get \$primaryKey => '); - var primaryKey = table.primaryKey; + var primaryKey = table.fullPrimaryKey; // If there is an auto increment column, that forms the primary key. The // PK returned by table.primaryKey only contains column that have been @@ -306,8 +306,8 @@ class TableWriter { } if (table.isVirtualTable) { - final declaration = table.declaration as MoorTableDeclaration; - final stmt = declaration.node as CreateVirtualTableStatement; + final declaration = table.declaration as TableDeclarationWithSql; + final stmt = declaration.creatingStatement as CreateVirtualTableStatement; final moduleAndArgs = asDartLiteral( '${stmt.moduleName}(${stmt.argumentContent.join(', ')})'); _buffer diff --git a/moor_generator/test/services/schema/find_differences_test.dart b/moor_generator/test/services/schema/find_differences_test.dart new file mode 100644 index 00000000..a0330165 --- /dev/null +++ b/moor_generator/test/services/schema/find_differences_test.dart @@ -0,0 +1,81 @@ +import 'package:moor_generator/src/services/schema/find_differences.dart'; +import 'package:test/test.dart'; + +void main() { + group('compares individual', () { + group('tables', () { + test('with rowid mismatch', () { + final result = compare( + Input('a', 'CREATE TABLE a (id INTEGER) WITHOUT ROWID;'), + Input('a', 'CREATE TABLE a (id INTEGER);'), + ); + + expect(result, hasChanges); + expect( + result.describe(), + contains('Expected the table to have a WITHOUT ROWID clause'), + ); + }); + + test('with too few columns', () { + final result = compare( + Input('a', 'CREATE TABLE a (id INTEGER, b TEXT);'), + Input('a', 'CREATE TABLE a (id INTEGER);'), + ); + + expect(result, hasChanges); + expect( + result.describe(), + contains('Missing in schema'), + ); + }); + + test('with too many columns', () { + final result = compare( + Input('a', 'CREATE TABLE a (id INTEGER);'), + Input('a', 'CREATE TABLE a (id INTEGER, b TEXT);'), + ); + + expect(result, hasChanges); + expect( + result.describe(), + contains('Additional unexpected column'), + ); + }); + + test('that are equal', () { + final result = compare( + Input('a', 'CREATE TABLE a (b TEXT, id INTEGER PRIMARY KEY);'), + Input('a', 'CREATE TABLE a (id INTEGER PRIMARY KEY, b TEXT);'), + ); + + expect(result, hasNoChanges); + }); + }); + + test('of different type', () { + final result = compare( + Input('a', 'CREATE TABLE a (id INTEGER);'), + Input('a', 'CREATE INDEX a ON b (c, d);'), + ); + + expect(result, hasChanges); + expect( + result.describe(), + contains('Expected a table, but got a index.'), + ); + }); + }); +} + +CompareResult compare(Input a, Input b) { + return FindSchemaDifferences([a], [b], false).compare(); +} + +Matcher hasChanges = _matchChanges(false); +Matcher hasNoChanges = _matchChanges(true); + +Matcher _matchChanges(bool expectNoChanges) { + return isA() + .having((e) => e.noChanges, 'noChanges', expectNoChanges); +} diff --git a/moor_generator/test/services/schema/writer_test.dart b/moor_generator/test/services/schema/writer_test.dart index 281abeaf..984daacb 100644 --- a/moor_generator/test/services/schema/writer_test.dart +++ b/moor_generator/test/services/schema/writer_test.dart @@ -1,8 +1,12 @@ @Tags(['analyzer']) import 'dart:convert'; +import 'package:moor_generator/moor_generator.dart'; +import 'package:moor_generator/src/analyzer/options.dart'; import 'package:moor_generator/src/analyzer/runner/results.dart'; import 'package:moor_generator/src/services/schema/schema_files.dart'; +import 'package:moor_generator/src/writer/database_writer.dart'; +import 'package:moor_generator/src/writer/writer.dart'; import 'package:test/test.dart'; import '../../analyzer/utils.dart'; @@ -20,6 +24,8 @@ CREATE TABLE "groups" ( UNIQUE(name) ); +CREATE VIRTUAL TABLE email USING fts5(sender, title, body); + CREATE TABLE group_members ( "group" INT NOT NULL REFERENCES "groups"(id), user INT NOT NULL REFERENCES users(id), @@ -56,7 +62,7 @@ class SettingsConverter extends TypeConverter { @UseMoor(include: {'a.moor'}, tables: [Users]) class Database {} ''', - }); + }, options: const MoorOptions(modules: [SqlModule.fts5])); final file = await state.analyze('package:foo/main.dart'); expect(state.session.errorsInFileAndImports(file), isEmpty); @@ -67,6 +73,18 @@ class Database {} final schemaJson = SchemaWriter(db).createSchemaJson(); expect(schemaJson, json.decode(expected)); }); + + test('can generate code from schema json', () { + final reader = + SchemaReader.readJson(json.decode(expected) as Map); + final fakeDb = Database()..entities = [...reader.entities]; + + // Write the database. Not crashing is good enough for us here, we have + // separate tests for verification + final writer = Writer(const MoorOptions(), + generationOptions: const GenerationOptions(forSchema: 1)); + DatabaseWriter(fakeDb, writer.child()).write(); + }); } const expected = r''' @@ -78,9 +96,7 @@ const expected = r''' "entities":[ { "id":0, - "references":[ - - ], + "references":[], "type":"table", "data":{ "name":"groups", @@ -113,17 +129,12 @@ const expected = r''' "is_virtual":false, "constraints":[ "UNIQUE(name)" - ], - "explicit_pk":[ - "id" ] } }, { "id":1, - "references":[ - - ], + "references":[], "type":"table", "data":{ "name":"users", @@ -254,6 +265,46 @@ const expected = r''' "name":"groups_name", "sql":"CREATE INDEX groups_name ON \"groups\"(name);" } + }, + { + "id": 5, + "references": [], + "type": "table", + "data": { + "name": "email", + "was_declared_in_moor": true, + "columns": [ + { + "name": "sender", + "moor_type": "ColumnType.text", + "nullable": false, + "customConstraints": "", + "default_dart": null, + "default_client_dart": null, + "dsl_features": [] + }, + { + "name": "title", + "moor_type": "ColumnType.text", + "nullable": false, + "customConstraints": "", + "default_dart": null, + "default_client_dart": null, + "dsl_features": [] + }, + { + "name": "body", + "moor_type": "ColumnType.text", + "nullable": false, + "customConstraints": "", + "default_dart": null, + "default_client_dart": null, + "dsl_features": [] + } + ], + "is_virtual": true, + "create_virtual_stmt": "CREATE VIRTUAL TABLE email USING fts5(sender, title, body);" + } } ] }