Add integration tests for transactions

Also fixes some bugs on the way

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

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

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<User> getUserById(int id) {
+    return (select(users)..where((u) => u.id.equals(id))).getSingle();
+  }
+
   Future<int> writeUser(Insertable<User> user) {
     return into(users).insert(user);
   }

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);
 }

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();
+  });
+}

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

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 {

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.
   ///

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<void> 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<void> 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();
+    }
   }
 }
 

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;
+    }
   }
 }
 

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<void> 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);
 }