Better errors on QueryExecutor misuse, fix beforeOpen

extras/integration_tests/tests/lib/database/database.dart

@@ -101,8 +101,11 @@ class Database extends _$Database {
       },
       beforeOpen: (details) async {
         if (details.wasCreated) {
-          await into(users)
-              .insertAll([People.dash, People.duke, People.gopher]);
+          // make sure that transactions can be used in the beforeOpen callback.
+          await transaction(() {
+            return into(users)
+                .insertAll([People.dash, People.duke, People.gopher]);
+          });
         }
       },
     );

moor/lib/src/runtime/database.dart

@@ -135,7 +135,7 @@ mixin QueryEngine on DatabaseConnectionUser {
   /// Whether this connection user is "top level", e.g. there is no parent
   /// connection user. We consider a [GeneratedDatabase] and a
   /// [DatabaseAccessor] to be top-level, while a [Transaction] or a
-  /// [BeforeOpenEngine] aren't.
+  /// [BeforeOpenRunner] aren't.
   ///
   /// If any query method is called on a [topLevel] database user, we check if
   /// it could instead be delegated to a child executor. For instance, consider
@@ -478,7 +478,10 @@ abstract class GeneratedDatabase extends DatabaseConnectionUser
     final migration = _resolvedMigration;
 
     if (migration.beforeOpen != null) {
-      return migration.beforeOpen(details);
+      return _runEngineZoned(
+        BeforeOpenRunner(this, executor),
+        () => migration.beforeOpen(details),
+      );
     }
     return Future.value();
   }
@@ -487,10 +490,4 @@ abstract class GeneratedDatabase extends DatabaseConnectionUser
   Future<void> close() async {
     await executor.close();
   }
-
-  /// Creates another instance of this [GeneratedDatabase] that uses the
-  /// [connection] instead of the current connection.
-  GeneratedDatabase cloneWith(DatabaseConnection connection) {
-    throw UnimplementedError();
-  }
 }

moor/lib/src/runtime/executor/helpers/engines.dart

@@ -13,6 +13,10 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
   bool get logStatements => false;
   final Lock _lock = Lock();
 
+  /// Used to provide better error messages when calling operations without
+  /// calling [ensureOpen] before.
+  bool _ensureOpenCalled = false;
+
   Future<T> _synchronized<T>(FutureOr<T> Function() action) async {
     if (isSequential) {
       return await _lock.synchronized(action);
@@ -31,6 +35,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
   @override
   Future<List<Map<String, dynamic>>> runSelect(
       String statement, List args) async {
+    assert(_ensureOpenCalled);
     final result = await _synchronized(() {
       _log(statement, args);
       return impl.runSelect(statement, args);
@@ -40,6 +45,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
 
   @override
   Future<int> runUpdate(String statement, List args) {
+    assert(_ensureOpenCalled);
     return _synchronized(() {
       _log(statement, args);
       return impl.runUpdate(statement, args);
@@ -48,6 +54,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
 
   @override
   Future<int> runDelete(String statement, List args) {
+    assert(_ensureOpenCalled);
     return _synchronized(() {
       _log(statement, args);
       return impl.runUpdate(statement, args);
@@ -56,6 +63,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
 
   @override
   Future<int> runInsert(String statement, List args) {
+    assert(_ensureOpenCalled);
     return _synchronized(() {
       _log(statement, args);
       return impl.runInsert(statement, args);
@@ -64,6 +72,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
 
   @override
   Future<void> runCustom(String statement, [List<dynamic> args]) {
+    assert(_ensureOpenCalled);
     return _synchronized(() {
       final resolvedArgs = args ?? const [];
       _log(statement, resolvedArgs);
@@ -73,6 +82,7 @@ mixin _ExecutorWithQueryDelegate on QueryExecutor {
 
   @override
   Future<void> runBatched(List<BatchedStatement> statements) {
+    assert(_ensureOpenCalled);
     return _synchronized(() {
       if (logStatements) {
         print('Moor: Executing $statements in a batch');
@@ -113,6 +123,7 @@ class _TransactionExecutor extends TransactionExecutor
 
   @override
   Future<bool> ensureOpen() async {
+    _ensureOpenCalled = true;
     if (_openingCompleter != null) {
       return await _openingCompleter.future;
     }
@@ -189,7 +200,7 @@ class _TransactionExecutor extends TransactionExecutor
   }
 }
 
-/// A database engine (implements [QueryExecutor]) that delegated the relevant
+/// A database engine (implements [QueryExecutor]) that delegates the relevant
 /// work to a [DatabaseDelegate].
 class DelegatedDatabase extends QueryExecutor with _ExecutorWithQueryDelegate {
   /// The [DatabaseDelegate] to send queries to.
@@ -217,12 +228,15 @@ class DelegatedDatabase extends QueryExecutor with _ExecutorWithQueryDelegate {
 
   @override
   Future<bool> ensureOpen() {
+    _ensureOpenCalled = true;
     return _openingLock.synchronized(() async {
       final alreadyOpen = await delegate.isOpen;
       if (alreadyOpen) {
         return true;
       }
 
+      assert(databaseInfo != null,
+          'A databaseInfo needs to be set to use a QueryExeuctor');
       await delegate.open(databaseInfo);
       await _runMigrations();
       return true;
@@ -274,6 +288,30 @@ class DelegatedDatabase extends QueryExecutor with _ExecutorWithQueryDelegate {
   }
 
   Future<void> _runBeforeOpen(OpeningDetails d) {
-    return databaseInfo.beforeOpenCallback(this, d);
+    return databaseInfo.beforeOpenCallback(_BeforeOpeningExecutor(this), d);
   }
 }
+
+/// Inside a `beforeOpen` callback, all moor apis must be available. At the same
+/// time, the `beforeOpen` callback must complete before any query sent outside
+/// of a `beforeOpen` callback can run. We do this by introducing a special
+/// executor that delegates all work to the original executor, but without
+/// blocking on `ensureOpen`
+class _BeforeOpeningExecutor extends QueryExecutor
+    with _ExecutorWithQueryDelegate {
+  final DelegatedDatabase _base;
+
+  _BeforeOpeningExecutor(this._base);
+
+  @override
+  TransactionExecutor beginTransaction() => _base.beginTransaction();
+
+  @override
+  Future<bool> ensureOpen() {
+    _ensureOpenCalled = true;
+    return Future.value(true);
+  }
+
+  @override
+  QueryDelegate get impl => _base.impl;
+}

moor/lib/src/runtime/executor/transactions.dart

@@ -2,6 +2,8 @@ import 'package:moor/moor.dart';
 import 'package:moor/src/runtime/executor/stream_queries.dart';
 
 /// Runs multiple statements transactionally.
+///
+/// Moor users should use [QueryEngine.transaction] to use this api.
 class Transaction extends DatabaseConnectionUser with QueryEngine {
   /// Constructs a transaction executor from the [other] user and the underlying
   /// [executor].
@@ -46,3 +48,14 @@ class _TransactionStreamStore extends StreamQueryStore {
     return parent.handleTableUpdates(affectedTables);
   }
 }
+
+/// Special query engine to run the [MigrationStrategy.beforeOpen] callback.
+///
+/// To use this api, moor users should use the [MigrationStrategy.beforeOpen]
+/// parameter inside the [GeneratedDatabase.migration] getter.
+class BeforeOpenRunner extends DatabaseConnectionUser with QueryEngine {
+  /// Creates a [BeforeOpenRunner] from the [database] and the special
+  /// [executor] running the queries.
+  BeforeOpenRunner(DatabaseConnectionUser database, QueryExecutor executor)
+      : super.delegate(database, executor: executor);
+}