From 41825e058ec0be7122396e179d6bd0b28d2a87d3 Mon Sep 17 00:00:00 2001
From: Simon Binder <oss@simonbinder.eu>
Date: Sun, 19 Dec 2021 19:45:20 +0100
Subject: [PATCH] Rollback if commit fails (#1589)

---
 drift/CHANGELOG.md                            |  4 ++
 .../lib/src/runtime/api/connection_user.dart  |  9 +++-
 drift/lib/src/utils/synchronized.dart         |  8 +--
 .../integration_tests/regress_1589_test.dart  | 53 +++++++++++++++++++
 drift_dev/CHANGELOG.md                        |  4 ++
 5 files changed, 70 insertions(+), 8 deletions(-)
 create mode 100644 drift/test/integration_tests/regress_1589_test.dart

diff --git a/drift/CHANGELOG.md b/drift/CHANGELOG.md
index 4606e029..946da586 100644
--- a/drift/CHANGELOG.md
+++ b/drift/CHANGELOG.md
@@ -1,3 +1,7 @@
+## unreleased
+
+- Rollback transactions when a commit fails.
+
 ## 1.1.0
 
 - Add the `references` method to `BuildColumn` to reference a column declared
diff --git a/drift/lib/src/runtime/api/connection_user.dart b/drift/lib/src/runtime/api/connection_user.dart
index 3b1b1528..d0f81b12 100644
--- a/drift/lib/src/runtime/api/connection_user.dart
+++ b/drift/lib/src/runtime/api/connection_user.dart
@@ -434,8 +434,13 @@ abstract class DatabaseConnectionUser {
           rethrow;
         } finally {
           if (success) {
-            // complete() will also take care of committing the transaction
-            await transaction.complete();
+            try {
+              await transaction.complete();
+            } catch (e) {
+              // Couldn't commit -> roll back then.
+              await transactionExecutor.rollback();
+              rethrow;
+            }
           }
           await transaction.disposeChildStreams();
         }
diff --git a/drift/lib/src/utils/synchronized.dart b/drift/lib/src/utils/synchronized.dart
index e8c83dfb..fda1a0dc 100644
--- a/drift/lib/src/utils/synchronized.dart
+++ b/drift/lib/src/utils/synchronized.dart
@@ -13,12 +13,8 @@ class Lock {
     final blockCompleted = Completer<void>();
     _last = blockCompleted.future;
 
-    Future<T> callBlockAndComplete() async {
-      try {
-        return await block();
-      } finally {
-        blockCompleted.complete();
-      }
+    Future<T> callBlockAndComplete() {
+      return Future.sync(block).whenComplete(blockCompleted.complete);
     }
 
     if (previous != null) {
diff --git a/drift/test/integration_tests/regress_1589_test.dart b/drift/test/integration_tests/regress_1589_test.dart
new file mode 100644
index 00000000..e6162189
--- /dev/null
+++ b/drift/test/integration_tests/regress_1589_test.dart
@@ -0,0 +1,53 @@
+import 'package:drift/drift.dart';
+import 'package:drift/native.dart';
+import 'package:test/test.dart';
+
+void main() {
+  test('a failing commit does not block the whole database', () async {
+    final db = _Database(NativeDatabase.memory());
+    addTearDown(db.close);
+
+    await db.customStatement('''
+CREATE TABLE IF NOT EXISTS todo_items (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  title TEXT NOT NULL, content TEXT NULL,
+  category_id INTEGER NOT NULL
+    REFERENCES todo_categories (id) DEFERRABLE INITIALLY DEFERRED,
+  generated_text TEXT NULL
+    GENERATED ALWAYS AS (title || ' (' || content || ')') VIRTUAL
+);
+''');
+    await db.customStatement('''
+CREATE TABLE IF NOT EXISTS todo_categories (
+  id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT,
+  name TEXT NOT NULL
+);
+''');
+    await db.customStatement('PRAGMA foreign_keys = ON;');
+
+    await expectLater(
+      db.transaction(() async {
+        // Thanks to the deferrable clause, this statement will only cause a
+        // failing COMMIT.
+        await db.customStatement(
+            'INSERT INTO todo_items (title, category_id) VALUES (?, ?);',
+            ['a', 100]);
+      }),
+      throwsA(isA<SqliteException>()),
+    );
+
+    expect(
+        db.customSelect('SELECT * FROM todo_items').get(), completion(isEmpty));
+  });
+}
+
+class _Database extends GeneratedDatabase {
+  _Database(QueryExecutor executor)
+      : super(SqlTypeSystem.defaultInstance, executor);
+
+  @override
+  Iterable<TableInfo<Table, dynamic>> get allTables => const Iterable.empty();
+
+  @override
+  int get schemaVersion => 1;
+}
diff --git a/drift_dev/CHANGELOG.md b/drift_dev/CHANGELOG.md
index 49696227..2e117096 100644
--- a/drift_dev/CHANGELOG.md
+++ b/drift_dev/CHANGELOG.md
@@ -1,3 +1,7 @@
+## unreleased
+
+- Improve error handling around custom row classes.
+
 ## 1.1.0
 
 - Consider `drift`-named files when generating schema migrations ([#1486](https://github.com/simolus3/moor/issues/1486))