From a4bfda494d360abe0b8cff16cd5bc502f776a883 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 31 Jul 2019 20:47:58 +0200 Subject: [PATCH] Add integration tests for transactions Also fixes some bugs on the way --- analysis_options.yaml | 1 - .../flutter_db/lib/main.dart | 5 +- .../tests/lib/database/database.dart | 4 ++ .../tests/lib/suite/suite.dart | 2 + .../tests/lib/suite/transactions.dart | 54 +++++++++++++++++++ moor/CHANGELOG.md | 8 +++ moor/lib/src/runtime/database.dart | 1 + .../runtime/executor/helpers/delegates.dart | 6 +++ .../src/runtime/executor/helpers/engines.dart | 23 +++++--- moor/lib/src/web/web_db.dart | 26 +++++++-- moor_flutter/lib/moor_flutter.dart | 36 +++++++++++-- 11 files changed, 149 insertions(+), 17 deletions(-) create mode 100644 extras/integration_tests/tests/lib/suite/transactions.dart diff --git a/analysis_options.yaml b/analysis_options.yaml index bb75690b..9922aad2 100644 --- a/analysis_options.yaml +++ b/analysis_options.yaml @@ -28,7 +28,6 @@ linter: - await_only_futures - camel_case_types - cancel_subscriptions - - cascade_invocations - comment_references - constant_identifier_names - curly_braces_in_flow_control_structures diff --git a/extras/integration_tests/flutter_db/lib/main.dart b/extras/integration_tests/flutter_db/lib/main.dart index a7c75bff..f9bf024f 100644 --- a/extras/integration_tests/flutter_db/lib/main.dart +++ b/extras/integration_tests/flutter_db/lib/main.dart @@ -8,7 +8,10 @@ import 'package:path/path.dart'; class SqfliteExecutor extends TestExecutor { @override QueryExecutor createExecutor() { - return FlutterQueryExecutor.inDatabaseFolder(path: 'app.db'); + return FlutterQueryExecutor.inDatabaseFolder( + path: 'app.db', + singleInstance: false, + ); } @override diff --git a/extras/integration_tests/tests/lib/database/database.dart b/extras/integration_tests/tests/lib/database/database.dart index 9ed9f4a9..a0eb2411 100644 --- a/extras/integration_tests/tests/lib/database/database.dart +++ b/extras/integration_tests/tests/lib/database/database.dart @@ -121,6 +121,10 @@ class Database extends _$Database { return (select(users)..where((u) => u.id.equals(id))).watchSingle(); } + Future getUserById(int id) { + return (select(users)..where((u) => u.id.equals(id))).getSingle(); + } + Future writeUser(Insertable user) { return into(users).insert(user); } diff --git a/extras/integration_tests/tests/lib/suite/suite.dart b/extras/integration_tests/tests/lib/suite/suite.dart index e46966a7..5204ab65 100644 --- a/extras/integration_tests/tests/lib/suite/suite.dart +++ b/extras/integration_tests/tests/lib/suite/suite.dart @@ -1,5 +1,6 @@ import 'package:moor/moor.dart'; import 'package:test/test.dart'; +import 'package:tests/suite/transactions.dart'; import 'custom_objects.dart'; import 'migrations.dart'; @@ -18,4 +19,5 @@ void runAllTests(TestExecutor executor) { migrationTests(executor); customObjectTests(executor); + transactionTests(executor); } diff --git a/extras/integration_tests/tests/lib/suite/transactions.dart b/extras/integration_tests/tests/lib/suite/transactions.dart new file mode 100644 index 00000000..fa10bfb8 --- /dev/null +++ b/extras/integration_tests/tests/lib/suite/transactions.dart @@ -0,0 +1,54 @@ +import 'package:test/test.dart'; +import 'package:tests/data/sample_data.dart'; +import 'package:tests/database/database.dart'; + +import 'suite.dart'; + +void transactionTests(TestExecutor executor) { + test('transactions write data', () async { + final db = Database(executor.createExecutor()); + + await db.transaction((_) async { + final florianId = await db.writeUser(People.florian); + print(florianId); + + final dash = await db.getUserById(People.dashId); + final florian = await db.getUserById(florianId); + + await db.makeFriends(dash, florian, goodFriends: true); + }); + + final countResult = await db.userCount(); + expect(countResult.single.cOUNTid, 4); + + final friendsResult = await db.amountOfGoodFriends(People.dashId); + expect(friendsResult.single.count, 1); + + await db.close(); + }); + + test('transaction is rolled back then an exception occurs', () async { + final db = Database(executor.createExecutor()); + + try { + await db.transaction((_) async { + final florianId = await db.writeUser(People.florian); + + final dash = await db.getUserById(People.dashId); + final florian = await db.getUserById(florianId); + + await db.makeFriends(dash, florian, goodFriends: true); + throw Exception('nope i made a mistake please rollback thank you'); + }); + fail('the transaction should have thrown!'); + } on Exception catch (_) {} + + final countResult = await db.userCount(); + expect(countResult.single.cOUNTid, 3); // only the default folks + + final friendsResult = await db.amountOfGoodFriends(People.dashId); + expect(friendsResult.single.count, 0); // no friendship was inserted + + await db.close(); + }); +} diff --git a/moor/CHANGELOG.md b/moor/CHANGELOG.md index 581d82f8..277bb79a 100644 --- a/moor/CHANGELOG.md +++ b/moor/CHANGELOG.md @@ -1,10 +1,18 @@ ## unreleased - Support custom columns via type converters. See the [docs](https://moor.simonbinder.eu/type_converters) for details on how to use this feature. +- Transactions now roll back when not completed successfully, they also rethrow the exception +to make debugging easier. - New `backends` api, making it easier to write database drivers that work with moor. Apart from `moor_flutter`, new experimental backends can be checked out from git: 1. `encrypted_moor`: An encrypted moor database: https://github.com/simolus3/moor/tree/develop/extras/encryption 2. `moor_mysql`: Work in progress mysql backend for moor. https://github.com/simolus3/moor/tree/develop/extras/mysql +- The compiled sql feature is no longer experimental and will stay stable until a major version bump +- New, experimental support for `.moor` files! Instead of declaring your tables in Dart, you can + choose to declare them with sql by writing the `CREATE TABLE` statement in a `.moor` file. + You can then use these tables in the database and with daos by using the `include` parameter + on `@UseMoor` and `@UseDao`. Again, please notice that this is an experimental api and there + might be some hiccups. Please report any issues you run into. ## 1.6.0 - Experimental web support! See [the documentation](https://moor.simonbinder.eu/web) for details. - Make transactions easier to use: Thanks to some Dart async magic, you no longer need to run diff --git a/moor/lib/src/runtime/database.dart b/moor/lib/src/runtime/database.dart index bfb60851..61e472a8 100644 --- a/moor/lib/src/runtime/database.dart +++ b/moor/lib/src/runtime/database.dart @@ -244,6 +244,7 @@ mixin QueryEngine on DatabaseConnectionUser { success = true; } catch (e) { await transactionExecutor.rollback(); + // pass the exception on to the one who called transaction() rethrow; } finally { diff --git a/moor/lib/src/runtime/executor/helpers/delegates.dart b/moor/lib/src/runtime/executor/helpers/delegates.dart index 48d76963..c8cc988c 100644 --- a/moor/lib/src/runtime/executor/helpers/delegates.dart +++ b/moor/lib/src/runtime/executor/helpers/delegates.dart @@ -13,6 +13,12 @@ import 'package:moor/src/runtime/executor/helpers/results.dart'; /// - [String] /// - [Uint8List] abstract class DatabaseDelegate implements QueryDelegate { + /// Whether the database managed by this delegate is in a transaction at the + /// moment. This field is only set when the [transactionDelegate] is a + /// [NoTransactionDelegate], because in that case transactions are run on + /// this delegate. + bool isInTransaction = false; + /// Returns an appropriate class to resolve the current schema version in /// this database. /// diff --git a/moor/lib/src/runtime/executor/helpers/engines.dart b/moor/lib/src/runtime/executor/helpers/engines.dart index 30c821e8..c6a2c70b 100644 --- a/moor/lib/src/runtime/executor/helpers/engines.dart +++ b/moor/lib/src/runtime/executor/helpers/engines.dart @@ -103,6 +103,7 @@ class _TransactionExecutor extends TransactionExecutor String _sendOnRollback; Future get completed => _sendCalled.future; + bool _sendFakeErrorOnRollback = false; _TransactionExecutor(this._db); @@ -125,13 +126,15 @@ class _TransactionExecutor extends TransactionExecutor if (transactionManager is NoTransactionDelegate) { assert( _db.isSequential, - 'When using the default NoTransactionDelegate, the database must be' + 'When using the default NoTransactionDelegate, the database must be ' 'sequential.'); // run all the commands on the main database, which we block while the // transaction is running. unawaited(_db._synchronized(() async { impl = _db.delegate; - await impl.runCustom(transactionManager.start, const []); + await runCustom(transactionManager.start, const []); + _db.delegate.isInTransaction = true; + _sendOnCommit = transactionManager.commit; _sendOnRollback = transactionManager.rollback; @@ -143,6 +146,9 @@ class _TransactionExecutor extends TransactionExecutor } else if (transactionManager is SupportedTransactionDelegate) { transactionManager.startTransaction((transaction) async { impl = transaction; + // specs say that the db implementation will perform a rollback when + // this future completes with an error. + _sendFakeErrorOnRollback = true; transactionStarted.complete(); // this callback must be running as long as the transaction, so we do @@ -161,7 +167,7 @@ class _TransactionExecutor extends TransactionExecutor @override Future send() async { if (_sendOnCommit != null) { - await impl.runCustom(_sendOnCommit, const []); + await runCustom(_sendOnCommit, const []); } _sendCalled.complete(); @@ -170,11 +176,16 @@ class _TransactionExecutor extends TransactionExecutor @override Future rollback() async { if (_sendOnRollback != null) { - await impl.runCustom(_sendOnRollback, const []); + await runCustom(_sendOnRollback, const []); + _db.delegate.isInTransaction = false; } - _sendCalled.completeError( - Exception('artificial exception to rollback the transaction')); + if (_sendFakeErrorOnRollback) { + _sendCalled.completeError( + Exception('artificial exception to rollback the transaction')); + } else { + _sendCalled.complete(); + } } } diff --git a/moor/lib/src/web/web_db.dart b/moor/lib/src/web/web_db.dart index 6b0b6631..4c7a6d8a 100644 --- a/moor/lib/src/web/web_db.dart +++ b/moor/lib/src/web/web_db.dart @@ -4,7 +4,8 @@ part of 'package:moor/moor_web.dart'; /// include the latest version of `sql.js` in your html. class WebDatabase extends DelegatedDatabase { WebDatabase(String name, {bool logStatements = false}) - : super(_WebDelegate(name), logStatements: logStatements); + : super(_WebDelegate(name), + logStatements: logStatements, isSequential: true); } class _WebDelegate extends DatabaseDelegate { @@ -13,8 +14,23 @@ class _WebDelegate extends DatabaseDelegate { String get _persistenceKey => 'moor_db_str_$name'; + bool _inTransaction = false; + _WebDelegate(this.name); + @override + set isInTransaction(bool value) { + _inTransaction = value; + + if (!_inTransaction) { + // transaction completed, save the database! + _storeDb(); + } + } + + @override + bool get isInTransaction => _inTransaction; + @override final TransactionDelegate transactionDelegate = const NoTransactionDelegate(); @@ -113,9 +129,11 @@ class _WebDelegate extends DatabaseDelegate { } void _storeDb() { - final data = _db.export(); - final binStr = bin2str.encode(data); - window.localStorage[_persistenceKey] = binStr; + if (!isInTransaction) { + final data = _db.export(); + final binStr = bin2str.encode(data); + window.localStorage[_persistenceKey] = binStr; + } } } diff --git a/moor_flutter/lib/moor_flutter.dart b/moor_flutter/lib/moor_flutter.dart index 2093fb1a..f02c49f0 100644 --- a/moor_flutter/lib/moor_flutter.dart +++ b/moor_flutter/lib/moor_flutter.dart @@ -22,7 +22,11 @@ class _SqfliteDelegate extends DatabaseDelegate with _SqfliteExecutor { final bool inDbFolder; final String path; - _SqfliteDelegate(this.inDbFolder, this.path); + bool singleInstance; + + _SqfliteDelegate(this.inDbFolder, this.path, {this.singleInstance}) { + singleInstance ??= true; + } @override DbVersionDelegate get versionDelegate { @@ -57,8 +61,14 @@ class _SqfliteDelegate extends DatabaseDelegate with _SqfliteExecutor { onUpgrade: (db, from, to) { _loadedSchemaVersion = from; }, + singleInstance: singleInstance, ); } + + @override + Future close() { + return db.close(); + } } class _SqfliteTransactionDelegate extends SupportedTransactionDelegate { @@ -71,6 +81,10 @@ class _SqfliteTransactionDelegate extends SupportedTransactionDelegate { delegate.db.transaction((transaction) async { final executor = _SqfliteTransactionExecutor(transaction); await run(executor); + }).catchError((_) { + // Ignore the errr! We send a fake exception to indicate a rollback. + // sqflite will rollback, but the exception will bubble up. Here we stop + // the exception. }); } } @@ -122,10 +136,22 @@ mixin _SqfliteExecutor on QueryDelegate { /// A query executor that uses sqflite internally. class FlutterQueryExecutor extends DelegatedDatabase { - FlutterQueryExecutor({@required String path, bool logStatements}) - : super(_SqfliteDelegate(false, path), logStatements: logStatements); + /// A query executor that will store the database in the file declared by + /// [path]. If [logStatements] is true, statements sent to the database will + /// be [print]ed, which can be handy for debugging. The [singleInstance] + /// parameter sets the corresponding parameter on [s.openDatabase]. + FlutterQueryExecutor( + {@required String path, bool logStatements, bool singleInstance}) + : super(_SqfliteDelegate(false, path, singleInstance: singleInstance), + logStatements: logStatements); + /// A query executor that will store the database in the file declared by + /// [path], which will be resolved relative to [s.getDatabasesPath()]. + /// If [logStatements] is true, statements sent to the database will + /// be [print]ed, which can be handy for debugging. The [singleInstance] + /// parameter sets the corresponding parameter on [s.openDatabase]. FlutterQueryExecutor.inDatabaseFolder( - {@required String path, bool logStatements}) - : super(_SqfliteDelegate(true, path), logStatements: logStatements); + {@required String path, bool logStatements, bool singleInstance}) + : super(_SqfliteDelegate(true, path, singleInstance: singleInstance), + logStatements: logStatements); }