From 7f7d2ab1b0a03383eb6acd30ca376026720ce347 Mon Sep 17 00:00:00 2001 From: Simon Binder Date: Sat, 3 Feb 2024 14:31:58 +0100 Subject: [PATCH] Run statements in correct context --- drift/CHANGELOG.md | 8 ++- .../lib/src/runtime/api/connection_user.dart | 21 +++--- .../query_builder/statements/delete.dart | 4 +- .../query_builder/statements/insert.dart | 6 +- .../statements/select/custom_select.dart | 2 +- .../statements/select/select.dart | 2 +- .../statements/select/select_with_join.dart | 2 +- .../query_builder/statements/update.dart | 4 +- drift/test/database/transactions_test.dart | 67 +++++++++++++++++++ 9 files changed, 96 insertions(+), 20 deletions(-) diff --git a/drift/CHANGELOG.md b/drift/CHANGELOG.md index fc9573df..91122d10 100644 --- a/drift/CHANGELOG.md +++ b/drift/CHANGELOG.md @@ -1,8 +1,14 @@ ## 2.16.0-dev -- When a migration throws, the database will now block subsequent operations +- When a migration throws, the database will now block subsequent operations instead of potentially allowing them to operate on a database in an inconsistent state. +- Statements built through the Dart query builder will now run in the context + active while they are running, instead of the context active at the time they + were created. For instance, creating an `UpdateStatement` with + `database.update` outside of a transaction and then calling + `UpdateStatement.write` inside of a transaction will now perform the update + inside of the transaction, instead of causing a deadlock. ## 2.15.0 diff --git a/drift/lib/src/runtime/api/connection_user.dart b/drift/lib/src/runtime/api/connection_user.dart index 66d194b2..9e9162cb 100644 --- a/drift/lib/src/runtime/api/connection_user.dart +++ b/drift/lib/src/runtime/api/connection_user.dart @@ -57,7 +57,7 @@ abstract class DatabaseConnectionUser { /// Creates and auto-updating stream from the given select statement. This /// method should not be used directly. Stream>> createStream(QueryStreamFetcher stmt) => - streamQueries.registerStream(stmt); + resolvedEngine.streamQueries.registerStream(stmt); /// Creates a copy of the table with an alias so that it can be used in the /// same query more than once. @@ -165,7 +165,7 @@ abstract class DatabaseConnectionUser { /// Starts an [InsertStatement] for a given table. You can use that statement /// to write data into the [table] by using [InsertStatement.insert]. InsertStatement into(TableInfo table) { - return InsertStatement(resolvedEngine, table); + return InsertStatement(this, table); } /// Starts an [UpdateStatement] for the given table. You can use that @@ -173,7 +173,7 @@ abstract class DatabaseConnectionUser { /// clause on that table and then use [UpdateStatement.write]. UpdateStatement update( TableInfo table) => - UpdateStatement(resolvedEngine, table); + UpdateStatement(this, table); /// Starts a query on the given table. /// @@ -202,8 +202,7 @@ abstract class DatabaseConnectionUser { SimpleSelectStatement select( ResultSetImplementation table, {bool distinct = false}) { - return SimpleSelectStatement(resolvedEngine, table, - distinct: distinct); + return SimpleSelectStatement(this, table, distinct: distinct); } /// Starts a complex statement on [table] that doesn't necessarily use all of @@ -239,8 +238,7 @@ abstract class DatabaseConnectionUser { JoinedSelectStatement selectOnly( ResultSetImplementation table, {bool distinct = false}) { - return JoinedSelectStatement( - resolvedEngine, table, [], distinct, false, false); + return JoinedSelectStatement(this, table, [], distinct, false, false); } /// Starts a [DeleteStatement] that can be used to delete rows from a table. @@ -248,7 +246,7 @@ abstract class DatabaseConnectionUser { /// See the [documentation](https://drift.simonbinder.eu/docs/getting-started/writing_queries/#updates-and-deletes) /// for more details and example on how delete statements work. DeleteStatement delete(TableInfo table) { - return DeleteStatement(resolvedEngine, table); + return DeleteStatement(this, table); } /// Executes a custom delete or update statement and returns the amount of @@ -360,7 +358,7 @@ abstract class DatabaseConnectionUser { Selectable customSelect(String query, {List variables = const [], Set readsFrom = const {}}) { - return CustomSelectStatement(query, variables, readsFrom, resolvedEngine); + return CustomSelectStatement(query, variables, readsFrom, this); } /// Creates a custom select statement from the given sql [query]. To run the @@ -617,4 +615,9 @@ extension RunWithEngine on DatabaseConnectionUser { DatabaseConnectionUser user, Future Function() calculation) { return _runConnectionZoned(user, calculation); } + + Future withCurrentExecutor(Future Function(QueryExecutor e) run) { + final engine = resolvedEngine; + return engine.doWhenOpened(run); + } } diff --git a/drift/lib/src/runtime/query_builder/statements/delete.dart b/drift/lib/src/runtime/query_builder/statements/delete.dart index 978c8430..6533a4a8 100644 --- a/drift/lib/src/runtime/query_builder/statements/delete.dart +++ b/drift/lib/src/runtime/query_builder/statements/delete.dart @@ -49,7 +49,7 @@ class DeleteStatement extends Query Future go() async { final ctx = constructQuery(); - return ctx.executor!.doWhenOpened((e) async { + return database.withCurrentExecutor((e) async { final rows = await e.runDelete(ctx.sql, ctx.boundVariables); if (rows > 0) { @@ -69,7 +69,7 @@ class DeleteStatement extends Query Future> _goReturning() async { final ctx = constructQuery(); - return ctx.executor!.doWhenOpened((e) async { + return database.withCurrentExecutor((e) async { final rows = await e.runSelect(ctx.sql, ctx.boundVariables); if (rows.isNotEmpty) { diff --git a/drift/lib/src/runtime/query_builder/statements/insert.dart b/drift/lib/src/runtime/query_builder/statements/insert.dart index dc3f8efc..7d22a953 100644 --- a/drift/lib/src/runtime/query_builder/statements/insert.dart +++ b/drift/lib/src/runtime/query_builder/statements/insert.dart @@ -70,7 +70,7 @@ class InsertStatement { final ctx = createContext(entity, mode ?? InsertMode.insert, onConflict: onConflict); - return await database.doWhenOpened((e) async { + return await database.withCurrentExecutor((e) async { final id = await e.runInsert(ctx.sql, ctx.boundVariables); database .notifyUpdates({TableUpdate.onTable(table, kind: UpdateKind.insert)}); @@ -131,7 +131,7 @@ class InsertStatement { ..write(' FROM $sourceCte'); _writeOnConflict(ctx, mode, null, onConflict); - return await database.doWhenOpened((e) async { + return await database.withCurrentExecutor((e) async { await e.runInsert(ctx.sql, ctx.boundVariables); database .notifyUpdates({TableUpdate.onTable(table, kind: UpdateKind.insert)}); @@ -170,7 +170,7 @@ class InsertStatement { final ctx = createContext(entity, mode ?? InsertMode.insert, onConflict: onConflict, returning: true); - return database.doWhenOpened((e) async { + return database.withCurrentExecutor((e) async { final result = await e.runSelect(ctx.sql, ctx.boundVariables); if (result.isNotEmpty) { database.notifyUpdates( diff --git a/drift/lib/src/runtime/query_builder/statements/select/custom_select.dart b/drift/lib/src/runtime/query_builder/statements/select/custom_select.dart index a992b315..3cb200ae 100644 --- a/drift/lib/src/runtime/query_builder/statements/select/custom_select.dart +++ b/drift/lib/src/runtime/query_builder/statements/select/custom_select.dart @@ -48,7 +48,7 @@ class CustomSelectStatement with Selectable { } Future>> _executeRaw(List mappedArgs) { - return _db.doWhenOpened((e) => e.runSelect(query, mappedArgs)); + return _db.withCurrentExecutor((e) => e.runSelect(query, mappedArgs)); } List _mapDbResponse(List> rows) { diff --git a/drift/lib/src/runtime/query_builder/statements/select/select.dart b/drift/lib/src/runtime/query_builder/statements/select/select.dart index 3b834b93..7d725325 100644 --- a/drift/lib/src/runtime/query_builder/statements/select/select.dart +++ b/drift/lib/src/runtime/query_builder/statements/select/select.dart @@ -77,7 +77,7 @@ class SimpleSelectStatement extends Query } Future>> _getRaw(GenerationContext ctx) { - return database.doWhenOpened((e) { + return database.withCurrentExecutor((e) { return e.runSelect(ctx.sql, ctx.boundVariables); }); } diff --git a/drift/lib/src/runtime/query_builder/statements/select/select_with_join.dart b/drift/lib/src/runtime/query_builder/statements/select/select_with_join.dart index ac7a6edc..556566fe 100644 --- a/drift/lib/src/runtime/query_builder/statements/select/select_with_join.dart +++ b/drift/lib/src/runtime/query_builder/statements/select/select_with_join.dart @@ -261,7 +261,7 @@ class JoinedSelectStatement } Future>> _getRaw(GenerationContext ctx) { - return ctx.executor!.doWhenOpened((e) async { + return database.withCurrentExecutor((e) async { try { return await e.runSelect(ctx.sql, ctx.boundVariables); } catch (e, s) { diff --git a/drift/lib/src/runtime/query_builder/statements/update.dart b/drift/lib/src/runtime/query_builder/statements/update.dart index d87b0d9d..5b62ce84 100644 --- a/drift/lib/src/runtime/query_builder/statements/update.dart +++ b/drift/lib/src/runtime/query_builder/statements/update.dart @@ -32,7 +32,7 @@ class UpdateStatement extends Query Future _performQuery() async { final ctx = constructQuery(); - final rows = await ctx.executor!.doWhenOpened((e) async { + final rows = await database.withCurrentExecutor((e) async { return await e.runUpdate(ctx.sql, ctx.boundVariables); }); @@ -84,7 +84,7 @@ class UpdateStatement extends Query await write(entity, dontExecute: true); final ctx = constructQuery(); - final rows = await ctx.executor!.doWhenOpened((e) { + final rows = await database.withCurrentExecutor((e) { return e.runSelect(ctx.sql, ctx.boundVariables); }); diff --git a/drift/test/database/transactions_test.dart b/drift/test/database/transactions_test.dart index 0b670b15..4689efaa 100644 --- a/drift/test/database/transactions_test.dart +++ b/drift/test/database/transactions_test.dart @@ -255,4 +255,71 @@ void main() { .having((e) => e.exception, 'exception', rollbackException)), ); }); + + group('statements run in correct zone', () { + // Statements used to run in the executor of the zone that created them, but + // they should actually run in the zone that calls the async method starting + // the database operation (https://github.com/simolus3/drift/issues/2873). + + test('select', () async { + when(executor.runSelect(any, any)) + .thenAnswer((_) => Future.error('should run select in transaction')); + + final simpleQuery = db.users.select(); + final joinedQuery = db.selectOnly(db.users)..addColumns([db.users.id]); + final customQuery = db.customSelect('SELECT 1'); + + await db.transaction(() async { + expect(await simpleQuery.get(), isEmpty); + expect(await joinedQuery.get(), isEmpty); + expect(await customQuery.get(), isEmpty); + }); + }); + + test('update', () async { + when(executor.runUpdate(any, any)) + .thenAnswer((_) => Future.error('should run update in transaction')); + + final stmt = db.update(db.users); + await db.transaction(() async { + await stmt.write(UsersCompanion(isAwesome: Value(true))); + }); + + verify(executor.transactions + .runUpdate('UPDATE "users" SET "is_awesome" = ?;', [1])); + }); + + test('delete', () async { + when(executor.runDelete(any, any)) + .thenAnswer((_) => Future.error('should run delete in transaction')); + + final stmt = db.delete(db.users); + await db.transaction(() async { + await stmt.go(); + }); + + verify(executor.transactions.runDelete('DELETE FROM "users";', [])); + }); + + test('insert', () async { + when(executor.runSelect(any, any)) + .thenAnswer((_) => Future.error('should run select in transaction')); + when(executor.runInsert(any, any)) + .thenAnswer((_) => Future.error('should run delete in transaction')); + + final stmt = db.into(db.categories); + + await db.transaction(() async { + await stmt.insert(CategoriesCompanion.insert(description: 'test')); + await stmt.insertReturningOrNull( + CategoriesCompanion.insert(description: 'test2')); + }); + + verify(executor.transactions + .runInsert('INSERT INTO "categories" ("desc") VALUES (?)', ['test'])); + verify(executor.transactions.runSelect( + 'INSERT INTO "categories" ("desc") VALUES (?) RETURNING *', + ['test2'])); + }); + }); }