diff --git a/lib/providers/download_history_provider.dart b/lib/providers/download_history_provider.dart index 5c711ec7..2f2a1a8c 100644 --- a/lib/providers/download_history_provider.dart +++ b/lib/providers/download_history_provider.dart @@ -421,6 +421,15 @@ class DownloadHistoryNotifier extends Notifier { }); } + Future removePhysicalFiles(Iterable filePaths) { + final paths = filePaths.toList(); + // Serialize with reloads so a snapshot read before deletion cannot restore + // the removed entries after both indexes have been cleaned. + return _enqueueHistoryWrite(() async { + await removeManyFromHistory(await _db.getPhysicalFileIds(paths)); + }); + } + Future removeManyFromHistory(Iterable ids) async { final removedIds = ids.toSet(); if (removedIds.isEmpty) return; @@ -444,6 +453,7 @@ class DownloadHistoryNotifier extends Notifier { ); } catch (error) { _historyLog.e('Failed to delete from database: $error'); + rethrow; } } diff --git a/lib/providers/local_library_provider.dart b/lib/providers/local_library_provider.dart index cea47c27..9072b9a3 100644 --- a/lib/providers/local_library_provider.dart +++ b/lib/providers/local_library_provider.dart @@ -1662,6 +1662,10 @@ class LocalLibraryNotifier extends Notifier { } } + Future removePhysicalFiles(Iterable filePaths) async { + await removeItems(await _db.getPhysicalFileIds(filePaths)); + } + Future removeItem(String id) async { await removeItems([id]); } diff --git a/lib/screens/downloaded_album_screen.dart b/lib/screens/downloaded_album_screen.dart index e837a696..6fe94cd4 100644 --- a/lib/screens/downloaded_album_screen.dart +++ b/lib/screens/downloaded_album_screen.dart @@ -7,6 +7,7 @@ import 'package:spotiflac_android/theme/cover_palette.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:cached_network_image/cached_network_image.dart'; import 'package:spotiflac_android/services/cover_cache_manager.dart'; +import 'package:spotiflac_android/services/deleted_library_files.dart'; import 'package:spotiflac_android/services/batch_track_actions.dart'; import 'package:spotiflac_android/services/local_track_batch_actions.dart'; import 'package:spotiflac_android/models/unified_library_item.dart'; @@ -159,7 +160,6 @@ class _DownloadedAlbumScreenState extends ConsumerState } Future _deleteSelected(List currentTracks) async { - final historyNotifier = ref.read(downloadHistoryProvider.notifier); final tracksById = {for (final track in currentTracks) track.id: track}; await confirmAndDeleteTracks( @@ -168,11 +168,12 @@ class _DownloadedAlbumScreenState extends ConsumerState deleteItem: (id) async { final item = tracksById[id]; if (item == null) return false; - final deleted = await deleteFile(item.filePath); - if (!deleted) return false; - historyNotifier.removeFromHistory(id); - return true; + return deleteFile(item.filePath); }, + persistDeletedItems: (ids) => removeDeletedLibraryFileEntries( + ref, + ids.map((id) => tracksById[id]!.filePath), + ), onExitSelectionMode: exitSelectionMode, ); } diff --git a/lib/screens/local_album_screen.dart b/lib/screens/local_album_screen.dart index 60a65224..412d154f 100644 --- a/lib/screens/local_album_screen.dart +++ b/lib/screens/local_album_screen.dart @@ -1,5 +1,6 @@ import 'dart:io'; import 'dart:math'; +import 'package:spotiflac_android/services/deleted_library_files.dart'; import 'package:flutter/material.dart'; import 'package:spotiflac_android/theme/app_tokens.dart'; import 'package:spotiflac_android/theme/mornye_theme.dart'; @@ -169,7 +170,20 @@ class _LocalAlbumScreenState extends ConsumerState } return true; }, - persistDeletedItems: libraryNotifier.removeItems, + persistDeletedItems: (ids) async { + final items = ids.map((id) => tracksById[id]!); + await removeDeletedLibraryFileEntries( + ref, + items + .where((item) => !isCueVirtualPath(item.filePath)) + .map((item) => item.filePath), + ); + await libraryNotifier.removeItems( + items + .where((item) => isCueVirtualPath(item.filePath)) + .map((item) => item.id), + ); + }, onExitSelectionMode: exitSelectionMode, ); diff --git a/lib/screens/track_metadata_actions.dart b/lib/screens/track_metadata_actions.dart index 3f57d512..d7b78539 100644 --- a/lib/screens/track_metadata_actions.dart +++ b/lib/screens/track_metadata_actions.dart @@ -137,25 +137,14 @@ extension _TrackMetadataFileActions on _TrackMetadataScreenState { isDestructive: true, onPressed: () async { var fileDeleted = true; - if (_isLocalItem) { - if (_isCueVirtualTrack && _localLibraryItem != null) { - await ref - .read(localLibraryProvider.notifier) - .removeItem(_localLibraryItem!.id); - } else { - fileDeleted = await deleteFile(cleanFilePath); - if (fileDeleted && _localLibraryItem != null) { - await ref - .read(localLibraryProvider.notifier) - .removeItem(_localLibraryItem!.id); - } - } + if (_isCueVirtualTrack && _localLibraryItem != null) { + await ref + .read(localLibraryProvider.notifier) + .removeItem(_localLibraryItem!.id); } else { fileDeleted = await deleteFile(cleanFilePath); if (fileDeleted) { - ref - .read(downloadHistoryProvider.notifier) - .removeFromHistory(_downloadItem!.id); + await removeDeletedLibraryFileEntries(ref, [cleanFilePath]); } } diff --git a/lib/screens/track_metadata_screen.dart b/lib/screens/track_metadata_screen.dart index 55fd1c2d..2e2f50d3 100644 --- a/lib/screens/track_metadata_screen.dart +++ b/lib/screens/track_metadata_screen.dart @@ -10,6 +10,7 @@ import 'package:spotiflac_android/widgets/re_enrich_review_sheet.dart'; import 'package:flutter/services.dart'; import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:spotiflac_android/services/conversion_library_service.dart'; +import 'package:spotiflac_android/services/deleted_library_files.dart'; import 'package:spotiflac_android/services/library_database.dart'; import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/utils/re_enrich_result.dart'; diff --git a/lib/services/deleted_library_files.dart b/lib/services/deleted_library_files.dart new file mode 100644 index 00000000..993d1be3 --- /dev/null +++ b/lib/services/deleted_library_files.dart @@ -0,0 +1,15 @@ +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:spotiflac_android/providers/download_history_provider.dart'; +import 'package:spotiflac_android/providers/local_library_provider.dart'; + +/// Call only after deletion succeeds. A download can also have a scanned row; +/// removing just one index makes its other row visible as a ghost entry. +Future removeDeletedLibraryFileEntries( + WidgetRef ref, + Iterable filePaths, +) async { + final paths = filePaths.toSet(); + if (paths.isEmpty) return; + await ref.read(downloadHistoryProvider.notifier).removePhysicalFiles(paths); + await ref.read(localLibraryProvider.notifier).removePhysicalFiles(paths); +} diff --git a/lib/services/history_database.dart b/lib/services/history_database.dart index 5318f208..ecf1c314 100644 --- a/lib/services/history_database.dart +++ b/lib/services/history_database.dart @@ -896,6 +896,9 @@ class HistoryDatabase { return _dbRowToJson(rows.first); } + Future> getPhysicalFileIds(Iterable filePaths) async => + sqlite.findPhysicalFileRowIds(await database, 'history', filePaths); + Future?> getBySpotifyId(String spotifyId) async { final db = await database; final rows = await db.query( diff --git a/lib/services/library_database.dart b/lib/services/library_database.dart index 34eb1770..f2f4a4b3 100644 --- a/lib/services/library_database.dart +++ b/lib/services/library_database.dart @@ -989,6 +989,9 @@ class LibraryDatabase { ]; } + Future> getPhysicalFileIds(Iterable filePaths) async => + sqlite.findPhysicalFileRowIds(await database, 'library', filePaths); + Future deleteByPath(String filePath) async { final db = await database; final rows = await db.query( diff --git a/lib/services/local_track_batch_actions.dart b/lib/services/local_track_batch_actions.dart index f6eca632..560a12a0 100644 --- a/lib/services/local_track_batch_actions.dart +++ b/lib/services/local_track_batch_actions.dart @@ -12,6 +12,7 @@ import 'package:spotiflac_android/providers/local_library_provider.dart'; import 'package:spotiflac_android/providers/settings_provider.dart'; import 'package:spotiflac_android/services/batch_metadata_re_enrich.dart'; import 'package:spotiflac_android/services/downloaded_embedded_cover_resolver.dart'; +import 'package:spotiflac_android/services/deleted_library_files.dart'; import 'package:spotiflac_android/services/library_database.dart'; import 'package:spotiflac_android/services/local_track_redownload_service.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; @@ -580,25 +581,19 @@ Future deleteLibraryTracks( ); if (confirmed != true || !isActive() || !context.mounted) return; - final historyNotifier = ref.read(downloadHistoryProvider.notifier); final messenger = ScaffoldMessenger.of(context); final l10n = context.l10n; + final deletedPaths = {}; var deletedCount = 0; for (final item in selected) { final cleanPath = DownloadedEmbeddedCoverResolver.cleanFilePath( item.filePath, ); if (!await deleteFile(cleanPath)) continue; - if (item.source == LibraryItemSource.downloaded) { - historyNotifier.removeFromHistory(item.historyItem!.id); - } else { - await LibraryDatabase.instance.deleteByPath(item.filePath); - } + deletedPaths.add(cleanPath); deletedCount++; } - if (selected.any((item) => item.source == LibraryItemSource.local)) { - ref.read(localLibraryProvider.notifier).reloadFromStorage(); - } + await removeDeletedLibraryFileEntries(ref, deletedPaths); onComplete(); messenger.showSnackBar( SnackBar(content: Text(l10n.snackbarDeletedTracks(deletedCount))), diff --git a/lib/services/sqlite_helpers.dart b/lib/services/sqlite_helpers.dart index 92ab0477..e8744476 100644 --- a/lib/services/sqlite_helpers.dart +++ b/lib/services/sqlite_helpers.dart @@ -310,6 +310,42 @@ Future createPathKeyTable(DatabaseExecutor db, String table) async { ); } +/// Finds every index entry for these physical files, including SAF aliases. +/// Keep extensions: deleting a FLAC must not remove a surviving Opus variant. +Future> findPhysicalFileRowIds( + DatabaseExecutor db, + String table, + Iterable filePaths, +) async { + final keys = { + for (final path in filePaths) ...buildPhysicalPathMatchKeys(path), + }; + final values = keys.toList(); + final ids = {}; + const chunkSize = 450; + for (var start = 0; start < values.length; start += chunkSize) { + final chunk = values.sublist( + start, + (start + chunkSize).clamp(0, values.length), + ); + final placeholders = List.filled(chunk.length, '?').join(','); + final rows = await db.rawQuery(''' + SELECT DISTINCT item.id, item.file_path + FROM $table item + JOIN ${table}_path_keys keys ON keys.item_id = item.id + WHERE keys.path_key IN ($placeholders) + ''', chunk); + for (final row in rows) { + if (buildPhysicalPathMatchKeys( + row['file_path'] as String?, + ).any(keys.contains)) { + ids.add(row['id'] as String); + } + } + } + return ids.toList(); +} + Future backfillPathKeys( Database db, String sourceTable, diff --git a/lib/utils/file_access.dart b/lib/utils/file_access.dart index e2929048..e14cff3d 100644 --- a/lib/utils/file_access.dart +++ b/lib/utils/file_access.dart @@ -354,6 +354,10 @@ Future> fileExistenceByPath(List paths) async { /// `true`, otherwise the app would hide a file that still exists on storage. Future deleteFile(String? path) async { if (path == null || path.isEmpty) return false; + if (path.startsWith('EXISTS:')) { + path = path.substring(7).trim(); + if (path.isEmpty) return false; + } if (path.startsWith('network://')) return false; // CUE virtual paths should NOT be deleted through this function — // deleting album.cue would remove ALL tracks. Callers should handle diff --git a/test/history_maintenance_test.dart b/test/history_maintenance_test.dart index 968e6b03..0a3d3fcb 100644 --- a/test/history_maintenance_test.dart +++ b/test/history_maintenance_test.dart @@ -1,11 +1,68 @@ +import 'dart:async'; import 'dart:convert'; import 'dart:io'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:shared_preferences/shared_preferences.dart'; import 'package:sqflite/sqflite.dart'; import 'package:spotiflac_android/providers/download_history_provider.dart'; +import 'package:spotiflac_android/services/history_database.dart'; import 'package:spotiflac_android/services/history_maintenance.dart'; +class _ReloadDatabase implements HistoryDatabase { + _ReloadDatabase(this.rows); + + final Map rows; + final snapshotRead = Completer(); + final count = Completer(); + bool failDeletion = false; + + @override + Future migrateFromSharedPreferences() async => false; + + @override + Future getCount() => count.future; + + @override + Future>> getAll({int? limit, int? offset}) async { + final snapshot = rows.values.map((item) => item.toJson()).toList(); + snapshotRead.complete(); + return snapshot; + } + + @override + Future> getPhysicalFileIds(Iterable paths) async => [ + for (final item in rows.values) + if (paths.contains(item.filePath)) item.id, + ]; + + @override + Future deleteByIds(List ids) async { + if (failDeletion) throw StateError('database write failed'); + final previousCount = rows.length; + rows.removeWhere((id, _) => ids.contains(id)); + return previousCount - rows.length; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +class _HistoryNotifier extends DownloadHistoryNotifier { + _HistoryNotifier(_ReloadDatabase database) + : _initialItems = database.rows.values.toList(), + super(database: database); + + final List _initialItems; + + @override + DownloadHistoryState build() => DownloadHistoryState( + items: _initialItems, + totalCount: _initialItems.length, + ); +} + class _HistoryRows implements DatabaseExecutor { final statements = >[]; final _keys = _PathKeys(); @@ -81,6 +138,58 @@ DownloadHistoryItem _track(String id) => DownloadHistoryItem( ); void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + SharedPreferences.setMockInitialValues({}); + + test('an older reload cannot restore a deleted physical file', () async { + final item = _track('removed'); + final db = _ReloadDatabase({item.id: item}); + final container = ProviderContainer( + overrides: [ + downloadHistoryProvider.overrideWith(() => _HistoryNotifier(db)), + ], + ); + addTearDown(container.dispose); + final notifier = container.read(downloadHistoryProvider.notifier); + final reload = notifier.reloadFromStorage(); + await db.snapshotRead.future; + final removal = notifier.removePhysicalFiles([item.filePath]); + await Future.delayed(Duration.zero); + expect(db.rows.keys, [ + item.id, + ], reason: 'deletion waits for the older reload'); + db.count.complete(1); + await Future.wait([reload, removal]); + final state = container.read(downloadHistoryProvider); + expect(db.rows, isEmpty); + expect(state.items, isEmpty); + expect(state.lookupItems, isEmpty); + expect(state.totalCount, 0); + }); + + test( + 'failed index deletion is reported and keeps the current state', + () async { + final item = _track('retained'); + final db = _ReloadDatabase({item.id: item})..failDeletion = true; + final container = ProviderContainer( + overrides: [ + downloadHistoryProvider.overrideWith(() => _HistoryNotifier(db)), + ], + ); + addTearDown(container.dispose); + await expectLater( + container.read(downloadHistoryProvider.notifier).removePhysicalFiles([ + item.filePath, + ]), + throwsStateError, + ); + expect(db.rows.keys, [item.id]); + expect(container.read(downloadHistoryProvider).items, [item]); + expect(container.read(downloadHistoryProvider).totalCount, 1); + }, + ); + test( 'late maintenance cannot recreate deleted rows or their path keys', () async { diff --git a/test/physical_file_index_lookup_test.dart b/test/physical_file_index_lookup_test.dart new file mode 100644 index 00000000..8bb810e6 --- /dev/null +++ b/test/physical_file_index_lookup_test.dart @@ -0,0 +1,116 @@ +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/sqlite_helpers.dart'; +import 'package:spotiflac_android/utils/file_access.dart'; +import 'package:spotiflac_android/utils/path_match_keys.dart'; +import 'package:sqflite/sqflite.dart'; + +class _SqliteRows implements DatabaseExecutor { + _SqliteRows(this.table, this.paths); + + final String table; + final Map paths; + int queries = 0; + + @override + Future>> rawQuery( + String sql, [ + List? arguments, + ]) async { + queries++; + expect(arguments!.length, lessThanOrEqualTo(450)); + final result = await Process.run('python3', [ + '-c', + r''' +import json, sqlite3, sys +table, rows, sql, args = json.loads(sys.argv[1]) +db = sqlite3.connect(':memory:') +db.row_factory = sqlite3.Row +db.executescript(f""" +CREATE TABLE {table}(id TEXT PRIMARY KEY, file_path TEXT); +CREATE TABLE {table}_path_keys(item_id TEXT, path_key TEXT, PRIMARY KEY(item_id, path_key)); +""") +for row in rows: + db.execute(f'INSERT INTO {table} VALUES (?,?)', (row['id'], row['path'])) + db.executemany(f'INSERT INTO {table}_path_keys VALUES (?,?)', [(row['id'], key) for key in row['keys']]) +print(json.dumps([dict(row) for row in db.execute(sql, args)])) +''', + jsonEncode([ + table, + [ + for (final entry in paths.entries) + { + 'id': entry.key, + 'path': entry.value, + 'keys': buildPathMatchKeys(entry.value).toList(), + }, + ], + sql, + arguments, + ]), + ]); + expect(result.exitCode, 0, reason: result.stderr.toString()); + return (jsonDecode(result.stdout as String) as List) + .map((row) => Map.from(row as Map)) + .toList(); + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +void main() { + test( + 'legacy history markers delete the actual file before index cleanup', + () async { + final directory = await Directory.systemTemp.createTemp( + 'library-delete-', + ); + addTearDown(() => directory.delete(recursive: true)); + final file = await File( + '${directory.path}/song.flac', + ).writeAsString('audio'); + expect(await deleteFile('EXISTS:${file.path}'), isTrue); + expect(await file.exists(), isFalse); + expect(await deleteFile('EXISTS:'), isFalse); + }, + ); + + const root = 'content://example.nas/tree/music'; + const album = 'content://example.nas/tree/music%2Falbum'; + const document = '/document/music%2Falbum%2Fsong.flac'; + for (final table in ['history', 'library']) { + test( + '$table finds every alias without removing other audio files', + () async { + final db = _SqliteRows(table, { + 'download': '$root$document', + 'scan': '$album$document', + 'variant': '$album/document/music%2Falbum%2Fsong.opus', + 'another-folder': '$root/document/music%2Fother%2Fsong.flac', + 'another-server': 'content://other.nas/tree/music$document', + }); + final ids = await findPhysicalFileRowIds(db, table, [ + 'EXISTS:$root$document', + ]); + expect(ids.toSet(), {'download', 'scan'}); + expect(db.queries, 1); + }, + ); + } + + test('large removals keep all IDs and stay within SQLite limits', () async { + final paths = { + for (var i = 0; i < 600; i++) 'id-$i': '/music/song-$i.flac', + }; + final db = _SqliteRows('library', paths); + final ids = await findPhysicalFileRowIds(db, 'library', paths.values); + expect(ids.toSet(), paths.keys.toSet()); + expect(db.queries, greaterThan(1)); + final queryCount = db.queries; + await findPhysicalFileRowIds(db, 'library', []); + expect(db.queries, queryCount, reason: 'empty paths do not query storage'); + }); +}