From 01326deb7b98bb94ab11d4c7bfb0d480d9cc7c2e Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Wed, 29 Jan 2020 22:03:29 +0100 Subject: [PATCH] Fix crash when using a no-op transaction (#361) --- .../tests/lib/suite/transactions.dart | 5 +++++ moor/CHANGELOG.md | 1 + moor/lib/src/runtime/api/query_engine.dart | 2 +- moor/lib/src/runtime/executor/executor.dart | 8 ++++++++ moor/lib/src/runtime/executor/helpers/engines.dart | 6 ++++++ moor/lib/src/runtime/isolate/client.dart | 14 ++++++++++---- moor/test/isolate_test.dart | 9 ++++++++- 7 files changed, 39 insertions(+), 6 deletions(-) diff --git a/extras/integration_tests/tests/lib/suite/transactions.dart b/extras/integration_tests/tests/lib/suite/transactions.dart index 1538b46f..6d5b7c37 100644 --- a/extras/integration_tests/tests/lib/suite/transactions.dart +++ b/extras/integration_tests/tests/lib/suite/transactions.dart @@ -52,4 +52,9 @@ void transactionTests(TestExecutor executor) { await db.close(); }); + + test('can use no-op transactions', () async { + final db = Database(executor.createExecutor()); + await db.transaction(() => Future.value(null)); + }); } diff --git a/moor/CHANGELOG.md b/moor/CHANGELOG.md index 8e7e9793..23596933 100644 --- a/moor/CHANGELOG.md +++ b/moor/CHANGELOG.md @@ -5,6 +5,7 @@ has been updated to explain how to use them. - Support table-valued functions (like `json_each` and `json_tree`) in moor files [#260](https://github.com/simolus3/moor/issues/260). +- Fix a crash when opening a transaction without using it ([#361](https://github.com/simolus3/moor/issues/361)) ## 2.3.0 diff --git a/moor/lib/src/runtime/api/query_engine.dart b/moor/lib/src/runtime/api/query_engine.dart index 3b62a272..ff47d79b 100644 --- a/moor/lib/src/runtime/api/query_engine.dart +++ b/moor/lib/src/runtime/api/query_engine.dart @@ -26,7 +26,7 @@ mixin QueryEngine on DatabaseConnectionUser { /// Here, the `update` method would be called on the [GeneratedDatabase] /// although it is very likely that the user meant to call it on the /// [Transaction] t. We can detect this by calling the function passed to - /// `transaction` in a forked [Zone] storing the transaction in + /// `transaction` in a forked [Zone]. @protected bool get topLevel => false; diff --git a/moor/lib/src/runtime/executor/executor.dart b/moor/lib/src/runtime/executor/executor.dart index 41408b40..93f8c319 100644 --- a/moor/lib/src/runtime/executor/executor.dart +++ b/moor/lib/src/runtime/executor/executor.dart @@ -104,9 +104,17 @@ class BatchedStatement { abstract class TransactionExecutor extends QueryExecutor { /// Completes the transaction. No further queries may be sent to to this /// [QueryExecutor] after this method was called. + /// + /// This may be called before [ensureOpen] was awaited, implementations must + /// support this. That state implies that no query was sent, so it should be + /// a no-op. Future send(); /// Cancels this transaction. No further queries may be sent ot this /// [QueryExecutor] after this method was called. + /// + /// This may be called before [ensureOpen] was awaited, implementations must + /// support this. That state implies that no query was sent, so it should be + /// a no-op. Future rollback(); } diff --git a/moor/lib/src/runtime/executor/helpers/engines.dart b/moor/lib/src/runtime/executor/helpers/engines.dart index d4309158..a1b36502 100644 --- a/moor/lib/src/runtime/executor/helpers/engines.dart +++ b/moor/lib/src/runtime/executor/helpers/engines.dart @@ -176,6 +176,9 @@ class _TransactionExecutor extends TransactionExecutor @override Future send() async { + // don't do anything if the transaction completes before it was opened + if (_openingCompleter == null) return; + if (_sendOnCommit != null) { await runCustom(_sendOnCommit, const []); _db.delegate.isInTransaction = false; @@ -186,6 +189,9 @@ class _TransactionExecutor extends TransactionExecutor @override Future rollback() async { + // don't do anything if the transaction completes before it was opened + if (_openingCompleter == null) return; + if (_sendOnRollback != null) { await runCustom(_sendOnRollback, const []); _db.delegate.isInTransaction = false; diff --git a/moor/lib/src/runtime/isolate/client.dart b/moor/lib/src/runtime/isolate/client.dart index fee995d5..8b8302a1 100644 --- a/moor/lib/src/runtime/isolate/client.dart +++ b/moor/lib/src/runtime/isolate/client.dart @@ -158,13 +158,19 @@ class _TransactionIsolateExecutor extends _BaseExecutor } @override - Future rollback() { - return _sendAction(_TransactionControl.rollback); + Future rollback() async { + // don't do anything if the transaction isn't open yet + if (_pendingOpen == null) return; + + return await _sendAction(_TransactionControl.rollback); } @override - Future send() { - return _sendAction(_TransactionControl.commit); + Future send() async { + // don't do anything if the transaction isn't open yet + if (_pendingOpen == null) return; + + return await _sendAction(_TransactionControl.commit); } } diff --git a/moor/test/isolate_test.dart b/moor/test/isolate_test.dart index c51c6381..270c9ef1 100644 --- a/moor/test/isolate_test.dart +++ b/moor/test/isolate_test.dart @@ -114,7 +114,14 @@ void _runTests( expect(result, isNotEmpty); }); - test('transactions have an isolate view on data', () async { + test('supports no-op transactions', () async { + final database = TodoDb.connect(isolateConnection); + await database.transaction(() { + return Future.value(null); + }); + }); + + test('transactions have an isolated view on data', () async { // regression test for https://github.com/simolus3/moor/issues/324 final db = TodoDb.connect(isolateConnection);