mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-19 14:07:26 +02:00
fix(library): remove deleted files from both library indexes
This commit is contained in:
1 parent
772bd97f7d
commit
a6c1409ef5
14 files changed
+331
-31
No files matched your search
@@ -421,6 +421,15 @@ class DownloadHistoryNotifier extends Notifier<DownloadHistoryState> {
|
||||
});
|
||||
}
|
||||
|
||||
Future<void> removePhysicalFiles(Iterable<String> 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<void> removeManyFromHistory(Iterable<String> ids) async {
|
||||
final removedIds = ids.toSet();
|
||||
if (removedIds.isEmpty) return;
|
||||
@@ -444,6 +453,7 @@ class DownloadHistoryNotifier extends Notifier<DownloadHistoryState> {
|
||||
);
|
||||
} catch (error) {
|
||||
_historyLog.e('Failed to delete from database: $error');
|
||||
rethrow;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -1662,6 +1662,10 @@ class LocalLibraryNotifier extends Notifier<LocalLibraryState> {
|
||||
}
|
||||
}
|
||||
|
||||
Future<void> removePhysicalFiles(Iterable<String> filePaths) async {
|
||||
await removeItems(await _db.getPhysicalFileIds(filePaths));
|
||||
}
|
||||
|
||||
Future<void> removeItem(String id) async {
|
||||
await removeItems([id]);
|
||||
}
|
||||
|
||||
@@ -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<DownloadedAlbumScreen>
|
||||
}
|
||||
|
||||
Future<void> _deleteSelected(List<DownloadHistoryItem> 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<DownloadedAlbumScreen>
|
||||
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,
|
||||
);
|
||||
}
|
||||
|
||||
@@ -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<LocalAlbumScreen>
|
||||
}
|
||||
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,
|
||||
);
|
||||
|
||||
|
||||
@@ -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]);
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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';
|
||||
|
||||
@@ -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<void> removeDeletedLibraryFileEntries(
|
||||
WidgetRef ref,
|
||||
Iterable<String> filePaths,
|
||||
) async {
|
||||
final paths = filePaths.toSet();
|
||||
if (paths.isEmpty) return;
|
||||
await ref.read(downloadHistoryProvider.notifier).removePhysicalFiles(paths);
|
||||
await ref.read(localLibraryProvider.notifier).removePhysicalFiles(paths);
|
||||
}
|
||||
@@ -896,6 +896,9 @@ class HistoryDatabase {
|
||||
return _dbRowToJson(rows.first);
|
||||
}
|
||||
|
||||
Future<List<String>> getPhysicalFileIds(Iterable<String> filePaths) async =>
|
||||
sqlite.findPhysicalFileRowIds(await database, 'history', filePaths);
|
||||
|
||||
Future<Map<String, dynamic>?> getBySpotifyId(String spotifyId) async {
|
||||
final db = await database;
|
||||
final rows = await db.query(
|
||||
|
||||
@@ -989,6 +989,9 @@ class LibraryDatabase {
|
||||
];
|
||||
}
|
||||
|
||||
Future<List<String>> getPhysicalFileIds(Iterable<String> filePaths) async =>
|
||||
sqlite.findPhysicalFileRowIds(await database, 'library', filePaths);
|
||||
|
||||
Future<void> deleteByPath(String filePath) async {
|
||||
final db = await database;
|
||||
final rows = await db.query(
|
||||
|
||||
@@ -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<void> 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 = <String>{};
|
||||
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))),
|
||||
|
||||
@@ -310,6 +310,42 @@ Future<void> 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<List<String>> findPhysicalFileRowIds(
|
||||
DatabaseExecutor db,
|
||||
String table,
|
||||
Iterable<String> filePaths,
|
||||
) async {
|
||||
final keys = {
|
||||
for (final path in filePaths) ...buildPhysicalPathMatchKeys(path),
|
||||
};
|
||||
final values = keys.toList();
|
||||
final ids = <String>{};
|
||||
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<void> backfillPathKeys(
|
||||
Database db,
|
||||
String sourceTable,
|
||||
|
||||
@@ -354,6 +354,10 @@ Future<Map<String, bool?>> fileExistenceByPath(List<String> paths) async {
|
||||
/// `true`, otherwise the app would hide a file that still exists on storage.
|
||||
Future<bool> 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
|
||||
|
||||
@@ -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<String, DownloadHistoryItem> rows;
|
||||
final snapshotRead = Completer<void>();
|
||||
final count = Completer<int>();
|
||||
bool failDeletion = false;
|
||||
|
||||
@override
|
||||
Future<bool> migrateFromSharedPreferences() async => false;
|
||||
|
||||
@override
|
||||
Future<int> getCount() => count.future;
|
||||
|
||||
@override
|
||||
Future<List<Map<String, dynamic>>> getAll({int? limit, int? offset}) async {
|
||||
final snapshot = rows.values.map((item) => item.toJson()).toList();
|
||||
snapshotRead.complete();
|
||||
return snapshot;
|
||||
}
|
||||
|
||||
@override
|
||||
Future<List<String>> getPhysicalFileIds(Iterable<String> paths) async => [
|
||||
for (final item in rows.values)
|
||||
if (paths.contains(item.filePath)) item.id,
|
||||
];
|
||||
|
||||
@override
|
||||
Future<int> deleteByIds(List<String> 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<DownloadHistoryItem> _initialItems;
|
||||
|
||||
@override
|
||||
DownloadHistoryState build() => DownloadHistoryState(
|
||||
items: _initialItems,
|
||||
totalCount: _initialItems.length,
|
||||
);
|
||||
}
|
||||
|
||||
class _HistoryRows implements DatabaseExecutor {
|
||||
final statements = <Map<String, Object?>>[];
|
||||
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<void>.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 {
|
||||
|
||||
@@ -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<String, String> paths;
|
||||
int queries = 0;
|
||||
|
||||
@override
|
||||
Future<List<Map<String, Object?>>> rawQuery(
|
||||
String sql, [
|
||||
List<Object?>? 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<String, Object?>.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');
|
||||
});
|
||||
}
|
||||
Reference in new issue
Block a user