Add try/catch with ROLLBACK in case a transaction fails

Explicit index deletion before insertion
LunaONE · Nov 5, 2024 · 9bcc24d · 9bcc24d
1 parent 889f01f
commit 9bcc24d
Show file tree

Hide file tree

Showing 5 changed files with 64 additions and 33 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 1.4.5
+
+* try/catch with `ROLLBACK` in case a transaction fails, so as to not leave the database in a locked state
+* Re-add explicit deletion of index, as `REPLACE INTO` was observed to not remove the entity's index on all platforms (and there is no clear documentation under what circumstances it would do or not do so)
+
 ## 1.4.4
 
 * Add another example, showing how to build repositories with a `Future<ValueListenable<T>>` interface

diff --git a/example/pubspec.lock b/example/pubspec.lock
@@ -105,7 +105,7 @@ packages:
       path: ".."
       relative: true
     source: path
-    version: "1.4.4"
+    version: "1.4.5"
   leak_tracker:
     dependency: transitive
     description:

diff --git a/lib/src/index_entity_store.dart b/lib/src/index_entity_store.dart
@@ -261,16 +261,22 @@ class IndexedEntityStore<T, K> {
   /// In case an entity with the same primary already exists in the database, it will be updated.
   // TODO(tp): We might want to rename this to `upsert` going forward to make it clear that this will overwrite and not error when the entry already exits (alternatively maybe `persist`, `write`, or `set`).
   void insert(T e) {
-    _database.execute('BEGIN');
-    assert(_database.autocommit == false);
+    try {
+      _database.execute('BEGIN');
+      assert(_database.autocommit == false);
 
-    _entityInsertStatement.execute(
-      [_entityKey, _connector.getPrimaryKey(e), _connector.serialize(e)],
-    );
+      _entityInsertStatement.execute(
+        [_entityKey, _connector.getPrimaryKey(e), _connector.serialize(e)],
+      );
 
-    _updateIndexInternal(e);
+      _updateIndexInternal(e);
 
-    _database.execute('COMMIT');
+      _database.execute('COMMIT');
+    } catch (e) {
+      _database.execute('ROLLBACK');
+
+      rethrow;
+    }
 
     _handleUpdate({_connector.getPrimaryKey(e)});
   }
@@ -279,31 +285,45 @@ class IndexedEntityStore<T, K> {
   ///
   /// Notification for changes will only fire after all changes have been written (meaning queries will get a single update after all writes are finished)
   void insertMany(Iterable<T> entities) {
-    _database.execute('BEGIN');
-    assert(_database.autocommit == false);
-
     final keys = <K>{};
-    for (final e in entities) {
-      _entityInsertStatement.execute(
-        [_entityKey, _connector.getPrimaryKey(e), _connector.serialize(e)],
-      );
 
-      _updateIndexInternal(e);
+    try {
+      _database.execute('BEGIN');
+      assert(_database.autocommit == false);
 
-      keys.add(_connector.getPrimaryKey(e));
-    }
+      for (final e in entities) {
+        _entityInsertStatement.execute(
+          [_entityKey, _connector.getPrimaryKey(e), _connector.serialize(e)],
+        );
+
+        _updateIndexInternal(e);
 
-    _database.execute('COMMIT');
+        keys.add(_connector.getPrimaryKey(e));
+      }
+
+      _database.execute('COMMIT');
+    } catch (e) {
+      _database.execute('ROLLBACK');
+
+      rethrow;
+    }
 
     _handleUpdate(keys);
   }
 
+  late final _deleteIndexStatement = _database.prepare(
+    'DELETE FROM `index` WHERE `type` = ? AND `entity` = ?',
+    persistent: true,
+  );
+
   late final _insertIndexStatement = _database.prepare(
     'INSERT INTO `index` (`type`, `entity`, `field`, `value`) VALUES (?, ?, ?, ?)',
     persistent: true,
   );
 
   void _updateIndexInternal(T e) {
+    _deleteIndexStatement.execute([_entityKey, _connector.getPrimaryKey(e)]);
+
     for (final indexColumn in _indexColumns._indexColumns.values) {
       _insertIndexStatement.execute(
         [
@@ -381,22 +401,28 @@ class IndexedEntityStore<T, K> {
         'Need to update index as fields where changed or added',
       );
 
-      _database.execute('BEGIN');
+      try {
+        _database.execute('BEGIN');
 
-      _database.execute(
-        'DELETE FROM `index` WHERE `type` = ?',
-        [_entityKey],
-      );
+        _database.execute(
+          'DELETE FROM `index` WHERE `type` = ?',
+          [_entityKey],
+        );
 
-      final entities = getAllOnce();
+        final entities = getAllOnce();
 
-      for (final e in entities) {
-        _updateIndexInternal(e);
-      }
+        for (final e in entities) {
+          _updateIndexInternal(e);
+        }
 
-      _database.execute('COMMIT');
+        _database.execute('COMMIT');
 
-      debugPrint('Updated indices for ${entities.length} entities');
+        debugPrint('Updated indices for ${entities.length} entities');
+      } catch (e) {
+        _database.execute('ROLLBACK');
+
+        rethrow;
+      }
     }
   }
 

diff --git a/pubspec.yaml b/pubspec.yaml
@@ -1,6 +1,6 @@
 name: indexed_entity_store
 description: A fast, simple, and synchronous entity store for Flutter applications.
-version: 1.4.4
+version: 1.4.5
 repository: https://github.com/LunaONE/indexed_entity_store
 
 environment:

diff --git a/test/indexed_entity_store_test.dart b/test/indexed_entity_store_test.dart
@@ -339,7 +339,7 @@ void main() {
       {
         valueStore.insert(_ValueWrapper(1, 'one'));
 
-        // both subscriptions got updated
+        // subscriptions did not emit a new value
         expect(
           valuesWithId1,
           [
@@ -364,7 +364,7 @@ void main() {
       {
         valueStore.insert(_ValueWrapper(3, 'three'));
 
-        // both subscriptions got updated
+        // subscriptions did not emit a new value
         expect(
           valuesWithId1,
           [