diff --git a/lib/services/library_database.dart b/lib/services/library_database.dart index 556d50ab..6f2fa96a 100644 --- a/lib/services/library_database.dart +++ b/lib/services/library_database.dart @@ -909,6 +909,7 @@ class LibraryDatabase { try { final batch = db.batch(); + final stagedIds = {}; for (final json in items) { final id = json['id'] as String?; if (id == null || id.trim().isEmpty) { @@ -924,6 +925,8 @@ class LibraryDatabase { _incrementalStagePathKeysTable, id, json['filePath'] as String?, + // The stage starts empty: only a repeated id has keys to replace. + replaceExisting: !stagedIds.add(id), ); } await batch.commit(noResult: true); @@ -1056,6 +1059,7 @@ class LibraryDatabase { pending = 0; } + final stagedIds = {}; await for (final json in items) { final id = json['id'] as String?; if (id == null || id.trim().isEmpty) { @@ -1071,6 +1075,8 @@ class LibraryDatabase { _scanStagePathKeysTable, id, json['filePath'] as String?, + // The stage starts empty: only a repeated id has keys to replace. + replaceExisting: !stagedIds.add(id), ); streamed++; pending++; @@ -2166,31 +2172,26 @@ class LibraryDatabase { final db = await database; var totalDeleted = 0; const chunkSize = 500; - for (var i = 0; i < filePaths.length; i += chunkSize) { - final end = (i + chunkSize < filePaths.length) - ? i + chunkSize - : filePaths.length; - final chunk = filePaths.sublist(i, end); - final placeholders = List.filled(chunk.length, '?').join(','); - final rows = await db.rawQuery( - 'SELECT id FROM library WHERE file_path IN ($placeholders)', - chunk, - ); - final ids = rows - .map((row) => row['id'] as String) - .toList(growable: false); - if (ids.isNotEmpty) { - final idPlaceholders = List.filled(ids.length, '?').join(','); - await db.rawDelete( - 'DELETE FROM library_path_keys WHERE item_id IN ($idPlaceholders)', - ids, + // One commit for the whole removal; a rescan that dropped a folder can + // otherwise pay several WAL commits per chunk. + await db.transaction((txn) async { + for (var i = 0; i < filePaths.length; i += chunkSize) { + final end = (i + chunkSize < filePaths.length) + ? i + chunkSize + : filePaths.length; + final chunk = filePaths.sublist(i, end); + final placeholders = List.filled(chunk.length, '?').join(','); + await txn.rawDelete( + 'DELETE FROM library_path_keys WHERE item_id IN ' + '(SELECT id FROM library WHERE file_path IN ($placeholders))', + chunk, + ); + totalDeleted += await txn.rawDelete( + 'DELETE FROM library WHERE file_path IN ($placeholders)', + chunk, ); } - totalDeleted += await db.rawDelete( - 'DELETE FROM library WHERE file_path IN ($placeholders)', - chunk, - ); - } + }); if (totalDeleted > 0) { _log.i('Deleted $totalDeleted items from library'); } diff --git a/lib/services/sqlite_helpers.dart b/lib/services/sqlite_helpers.dart index edbba964..92ab0477 100644 --- a/lib/services/sqlite_helpers.dart +++ b/lib/services/sqlite_helpers.dart @@ -328,13 +328,18 @@ Future backfillPathKeys( await batch.commit(noResult: true); } +/// [replaceExisting] may be false when the caller knows [table] holds no keys +/// for [id] yet, such as the first row for an id in a fresh staging table. void putPathKeysInBatch( Batch batch, String table, String id, - String? filePath, -) { - batch.delete(table, where: 'item_id = ?', whereArgs: [id]); + String? filePath, { + bool replaceExisting = true, +}) { + if (replaceExisting) { + batch.delete(table, where: 'item_id = ?', whereArgs: [id]); + } for (final key in buildPathMatchKeys(filePath)) { batch.insert(table, { 'item_id': id, diff --git a/test/sqlite_path_keys_batch_test.dart b/test/sqlite_path_keys_batch_test.dart new file mode 100644 index 00000000..f65149e6 --- /dev/null +++ b/test/sqlite_path_keys_batch_test.dart @@ -0,0 +1,56 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/sqlite_helpers.dart'; +import 'package:spotiflac_android/utils/path_match_keys.dart'; +import 'package:sqflite/sqflite.dart'; + +class _RecordingBatch implements Batch { + final operations = []; + + @override + void delete(String table, {String? where, List? whereArgs}) { + operations.add('delete $table ${whereArgs?.single}'); + } + + @override + void insert( + String table, + Map values, { + String? nullColumnHack, + ConflictAlgorithm? conflictAlgorithm, + }) { + expect(conflictAlgorithm, ConflictAlgorithm.ignore); + operations.add('insert $table ${values['item_id']} ${values['path_key']}'); + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +void main() { + const path = '/storage/emulated/0/Music/Album/01 Song.flac'; + + test('path keys replace existing rows by default', () { + final batch = _RecordingBatch(); + putPathKeysInBatch(batch, 'stage_keys', 'lib_1', path); + expect(batch.operations.first, 'delete stage_keys lib_1'); + expect(batch.operations.skip(1), [ + for (final key in buildPathMatchKeys(path)) + 'insert stage_keys lib_1 $key', + ]); + }); + + test('first row in a fresh stage writes the same keys without a delete', () { + final batch = _RecordingBatch(); + putPathKeysInBatch( + batch, + 'stage_keys', + 'lib_1', + path, + replaceExisting: false, + ); + expect(batch.operations, [ + for (final key in buildPathMatchKeys(path)) + 'insert stage_keys lib_1 $key', + ]); + }); +}