From 5b675a811b4335e458f3baf8d0a1e87819677415 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Mon, 16 Mar 2020 20:36:03 +0100 Subject: [PATCH] Don't consider rowid aliases required (#445) --- moor/CHANGELOG.md | 2 ++ .../analyzer/sql_queries/lints/linter.dart | 5 +-- moor_generator/lib/src/model/column.dart | 9 ----- moor_generator/lib/src/model/table.dart | 36 +++++++++++++++++++ .../lib/src/writer/tables/table_writer.dart | 2 +- .../tables/update_companion_writer.dart | 2 +- moor_generator/pubspec.yaml | 4 +-- .../test/analyzer/dart/table_parser_test.dart | 15 ++++++++ .../test/analyzer/moor/moor_parser_test.dart | 32 +++++++++++++++++ 9 files changed, 92 insertions(+), 15 deletions(-) diff --git a/moor/CHANGELOG.md b/moor/CHANGELOG.md index dc8470a6..71663443 100644 --- a/moor/CHANGELOG.md +++ b/moor/CHANGELOG.md @@ -14,6 +14,8 @@ - __Breaking__: Remove the second type variable on `Expression` and subclasses. - __Breaking__: Remove `customSelectStream` from `QueryEngine`. The `customSelect` method now returns an `Selectable` (like `customSelectQuery`, which in turn has been deprecated). +- __Breaking__: Columns that are aliases to sqlite's `rowid` column are now longer considered required + for inserts - Experimentally support IndexedDB to store sqlite data on the web - Moor will no longer wait for query stream listeners to receive a done event when closing a database or transaction. diff --git a/moor_generator/lib/src/analyzer/sql_queries/lints/linter.dart b/moor_generator/lib/src/analyzer/sql_queries/lints/linter.dart index b011e776..dce406e7 100644 --- a/moor_generator/lib/src/analyzer/sql_queries/lints/linter.dart +++ b/moor_generator/lib/src/analyzer/sql_queries/lints/linter.dart @@ -93,8 +93,9 @@ class _LintingVisitor extends RecursiveVisitor { // second, check that no required columns are left out final specifiedTable = linter.mapper.tableToMoor(e.table.resolved as Table); - final required = - specifiedTable.columns.where((c) => c.requiredDuringInsert).toList(); + final required = specifiedTable.columns + .where(specifiedTable.isColumnRequiredForInsert) + .toList(); if (required.isNotEmpty && e.source is DefaultValues) { linter.lints.add(AnalysisError( diff --git a/moor_generator/lib/src/model/column.dart b/moor_generator/lib/src/model/column.dart index 94769c80..c29db8cc 100644 --- a/moor_generator/lib/src/model/column.dart +++ b/moor_generator/lib/src/model/column.dart @@ -170,15 +170,6 @@ class MoorColumn implements HasDeclaration { ColumnType.real: 'GeneratedRealColumn', }[type]; - /// Whether this column is required for insert statements, meaning that a - /// non-absent value must be provided for an insert statement to be valid. - bool get requiredDuringInsert { - final hasDefault = defaultArgument != null || clientDefaultCode != null; - final aliasForPk = type == ColumnType.integer && - features.any((f) => f is PrimaryKey || f is AutoIncrement); - return !nullable && !hasDefault && !aliasForPk; - } - /// The class inside the moor library that represents the same sql type as /// this column. String get sqlTypeName => sqlTypes[type]; diff --git a/moor_generator/lib/src/model/table.dart b/moor_generator/lib/src/model/table.dart index cc41ef02..c4928001 100644 --- a/moor_generator/lib/src/model/table.dart +++ b/moor_generator/lib/src/model/table.dart @@ -69,8 +69,20 @@ class MoorTable implements MoorSchemaEntity { /// The set of primary keys, if they have been explicitly defined by /// overriding `primaryKey` in the table class. `null` if the primary key has /// not been defined that way. + /// + /// For the full primary key, see [fullPrimaryKey]. final Set primaryKey; + /// The primary key for this table. + /// + /// Unlikely [primaryKey], this method is not limited to the `primaryKey` + /// override in Dart table declarations. + Set get fullPrimaryKey { + if (primaryKey != null) return primaryKey; + + return columns.where((c) => c.features.any((f) => f is PrimaryKey)).toSet(); + } + /// When non-null, the generated table class will override the `withoutRowId` /// getter on the table class with this value. final bool overrideWithoutRowId; @@ -138,6 +150,30 @@ class MoorTable implements MoorSchemaEntity { } } + /// Determines whether [column] would be required for inserts performed via + /// companions. + bool isColumnRequiredForInsert(MoorColumn column) { + assert(columns.contains(column)); + + if (column.defaultArgument != null || + column.clientDefaultCode != null || + column.nullable) { + // default value would be applied, so it's not required for inserts + return false; + } + + // A column isn't required if it's an alias for the rowid, as explained + // at https://www.sqlite.org/lang_createtable.html#rowid + final isWithoutRowId = overrideWithoutRowId ?? false; + final fullPk = fullPrimaryKey; + final isAliasForRowId = !isWithoutRowId && + column.type == ColumnType.integer && + fullPk.length == 1 && + fullPk.single == column; + + return !isAliasForRowId; + } + @override String get displayName { if (isFromSql) { diff --git a/moor_generator/lib/src/writer/tables/table_writer.dart b/moor_generator/lib/src/writer/tables/table_writer.dart index 476c5c2b..d82339ba 100644 --- a/moor_generator/lib/src/writer/tables/table_writer.dart +++ b/moor_generator/lib/src/writer/tables/table_writer.dart @@ -235,7 +235,7 @@ class TableWriter { '$getterName.isAcceptableValue(d.$getterName.value, $metaName));') ..write('}'); - if (column.requiredDuringInsert) { + if (table.isColumnRequiredForInsert(column)) { _buffer ..write(' else if (isInserting) {\n') ..write('context.missing($metaName);\n') diff --git a/moor_generator/lib/src/writer/tables/update_companion_writer.dart b/moor_generator/lib/src/writer/tables/update_companion_writer.dart index d23b140d..a4f0dfd4 100644 --- a/moor_generator/lib/src/writer/tables/update_companion_writer.dart +++ b/moor_generator/lib/src/writer/tables/update_companion_writer.dart @@ -59,7 +59,7 @@ class UpdateCompanionWriter { for (final column in table.columns) { final param = column.dartGetterName; - if (column.requiredDuringInsert) { + if (table.isColumnRequiredForInsert(column)) { requiredColumns.add(column); _buffer.write('@required ${column.dartTypeName} $param,'); diff --git a/moor_generator/pubspec.yaml b/moor_generator/pubspec.yaml index 7480b5ff..54fd316e 100644 --- a/moor_generator/pubspec.yaml +++ b/moor_generator/pubspec.yaml @@ -1,6 +1,6 @@ name: moor_generator description: Dev-dependency to generate table and dataclasses together with the moor package. -version: 2.4.0 +version: 3.0.0-dev repository: https://github.com/simolus3/moor homepage: https://moor.simonbinder.eu/ issue_tracker: https://github.com/simolus3/moor/issues @@ -22,7 +22,7 @@ dependencies: cli_util: ^0.1.0 # Moor-specific analysis - moor: ^2.3.0 + moor: ^3.0.0-dev sqlparser: ^0.7.0 # Dart analysis diff --git a/moor_generator/test/analyzer/dart/table_parser_test.dart b/moor_generator/test/analyzer/dart/table_parser_test.dart index 9e01bbb7..09d5922d 100644 --- a/moor_generator/test/analyzer/dart/table_parser_test.dart +++ b/moor_generator/test/analyzer/dart/table_parser_test.dart @@ -65,6 +65,14 @@ void main() { TextColumn get archivedBy => text()(); DateTimeColumn get archivedOn => dateTime()(); } + + class WithAliasForRowId extends Table { + IntColumn get id => integer()(); + TextColumn get name => text()(); + + @override + Set get primaryKey => {id}; + } ''' }); }); @@ -175,6 +183,13 @@ void main() { expect(table.columns.any((column) => column.hasAI), isFalse); }); + test('recognizes aliases for rowid', () async { + final table = await parse('WithAliasForRowId'); + final idColumn = table.columns.singleWhere((c) => c.name.name == 'id'); + + expect(table.isColumnRequiredForInsert(idColumn), isFalse); + }); + group('inheritance', () { test('from abstract classes or mixins', () async { final table = await parse('Foos'); diff --git a/moor_generator/test/analyzer/moor/moor_parser_test.dart b/moor_generator/test/analyzer/moor/moor_parser_test.dart index a89b685a..b1fb2db1 100644 --- a/moor_generator/test/analyzer/moor/moor_parser_test.dart +++ b/moor_generator/test/analyzer/moor/moor_parser_test.dart @@ -1,9 +1,11 @@ import 'package:build/build.dart'; +import 'package:moor_generator/src/analyzer/runner/results.dart'; import 'package:moor_generator/src/analyzer/runner/steps.dart'; import 'package:moor_generator/src/analyzer/session.dart'; import 'package:test/test.dart'; import '../../utils/test_backend.dart'; +import '../utils.dart'; void main() { const content = ''' @@ -46,4 +48,34 @@ usersWithLongName: SELECT * FROM users WHERE LENGTH(name) > 25 backend.finish(); }); + + test('recognizes aliases to rowid', () async { + final state = TestState.withContent({ + 'foo|lib/a.moor': ''' + CREATE TABLE users ( + id INTEGER PRIMARY KEY, + name TEXT NOT NULL + ); + + CREATE TABLE users2 ( + id INTEGER, + name TEXT NOT NULL, + PRIMARY KEY (id) + ); + ''' + }); + + final result = await state.analyze('package:foo/a.moor'); + final file = result.currentResult as ParsedMoorFile; + + final users1 = file.declaredTables.singleWhere((t) => t.sqlName == 'users'); + final users2 = + file.declaredTables.singleWhere((t) => t.sqlName == 'users2'); + + expect(users1.isColumnRequiredForInsert(users1.columns[0]), isFalse); + expect(users1.isColumnRequiredForInsert(users1.columns[1]), isTrue); + + expect(users2.isColumnRequiredForInsert(users2.columns[0]), isFalse); + expect(users2.isColumnRequiredForInsert(users2.columns[1]), isTrue); + }); }