From 9bcc24df556bb002040a1682a8659c289f5f25e4 Mon Sep 17 00:00:00 2001 From: Timm Preetz Date: Tue, 5 Nov 2024 14:27:25 +0100 Subject: [PATCH] Add try/catch with `ROLLBACK` in case a transaction fails Explicit index deletion before insertion --- CHANGELOG.md | 5 ++ example/pubspec.lock | 2 +- lib/src/index_entity_store.dart | 84 +++++++++++++++++++---------- pubspec.yaml | 2 +- test/indexed_entity_store_test.dart | 4 +- 5 files changed, 64 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d387be..28fca98 100644 --- 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>` interface diff --git a/example/pubspec.lock b/example/pubspec.lock index db4c2fc..82db360 100644 --- 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 index 2fa65b7..5758fc3 100644 --- a/lib/src/index_entity_store.dart +++ b/lib/src/index_entity_store.dart @@ -261,16 +261,22 @@ class IndexedEntityStore { /// 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 { /// /// 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 entities) { - _database.execute('BEGIN'); - assert(_database.autocommit == false); - final keys = {}; - 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 { '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 index bae849d..e8a5c65 100644 --- 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 index 402b211..14457fc 100644 --- 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, [