From 62ef5386905d8143a5e4cdca06e554b2bd28ff0e Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Sun, 6 Sep 2026 02:34:38 +0700 Subject: [PATCH] perf(library): page missing-file cleanup with safe SAF batch checks --- .../kotlin/com/zarz/spotiflac/MainActivity.kt | 7 + .../com/zarz/spotiflac/MainActivitySafScan.kt | 49 +++- lib/providers/local_library_provider.dart | 16 +- lib/services/library_cleanup.dart | 82 +++++++ lib/services/library_database.dart | 58 +---- lib/services/platform_bridge.dart | 17 ++ lib/utils/file_access.dart | 50 ++++ test/library_cleanup_test.dart | 231 ++++++++++++++++++ 8 files changed, 456 insertions(+), 54 deletions(-) create mode 100644 lib/services/library_cleanup.dart create mode 100644 test/library_cleanup_test.dart diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt index d1fd2c5a..50f5e721 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -1115,6 +1115,13 @@ class MainActivity: FlutterFragmentActivity() { } result.success(exists) } + "safExistsBatch" -> { + val urisJson = call.argument("uris_json") ?: "[]" + val response = withContext(Dispatchers.IO) { + safExistsBatch(urisJson) + } + result.success(response) + } "isSafTreeAccessible" -> { val uriStr = call.argument("tree_uri") ?: "" val accessible = withContext(Dispatchers.IO) { diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt index 9fabdb69..d3bbf9d8 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt @@ -1558,10 +1558,51 @@ internal fun MainActivity.scanSafTreeIncremental( return spill.result() } - /** - * Resolve SAF file last-modified values for a list of content URIs. - * Returns JSON object mapping uri -> lastModified (unix millis). - */ +// A failed DocumentsProvider query must never be mistaken for a missing file. +// Unlike DocumentFile.exists(), this keeps null cursors/exceptions inconclusive. +internal fun MainActivity.safExistsBatch(urisJson: String): String { + val result = JSONObject() + val uris = JSONArray(urisJson) + val accessibleTrees = mutableMapOf() + val permissions = contentResolver.persistedUriPermissions + val projection = arrayOf(DocumentsContract.Document.COLUMN_DOCUMENT_ID) + for (index in 0 until uris.length()) { + val path = uris.optString(index) + if (path.isBlank()) continue + var status = "unknown" + try { + val uri = Uri.parse(path) + val treeAccessible = if (DocumentsContract.isTreeUri(uri)) { + val treeId = DocumentsContract.getTreeDocumentId(uri) + val tree = DocumentsContract.buildTreeDocumentUri(uri.authority, treeId) + accessibleTrees.getOrPut(tree.toString()) { + val granted = permissions.any { it.uri == tree && it.isReadPermission } + if (!granted) false else { + val root = DocumentsContract.buildDocumentUriUsingTree(tree, treeId) + contentResolver.query(root, projection, null, null, null)?.use { + it.moveToFirst() + } == true + } + } + } else false + contentResolver.query(uri, projection, null, null, null)?.use { cursor -> + status = if (cursor.moveToFirst()) "found" + else if (treeAccessible && + !cursor.extras.getBoolean(DocumentsContract.EXTRA_LOADING, false)) + "missing" else "unknown" + } + } catch (_: Exception) { + // Revoked permission, offline storage and provider errors: retain row. + } + result.put(path, status) + } + return result.toString() +} + +/** + * Resolve SAF file last-modified values for a list of content URIs. + * Returns JSON object mapping uri -> lastModified (unix millis). + */ internal fun MainActivity.getSafFileModTimes(urisJson: String): String { val result = JSONObject() val uris = try { diff --git a/lib/providers/local_library_provider.dart b/lib/providers/local_library_provider.dart index 6b85672e..f0090200 100644 --- a/lib/providers/local_library_provider.dart +++ b/lib/providers/local_library_provider.dart @@ -8,6 +8,7 @@ import 'package:spotiflac_android/services/library_database.dart'; import 'package:spotiflac_android/services/notification_service.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/utils/logger.dart'; +import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/utils/local_library_scan_prefs.dart'; import 'package:spotiflac_android/utils/progress_stream_poller.dart'; @@ -1143,7 +1144,20 @@ class LocalLibraryNotifier extends Notifier { ); if (securityAccess == null) continue; } - removed += await _db.cleanupMissingFiles(sourceId: source.id); + removed += await _db.cleanupMissingFiles( + sourceId: source.id, + canDelete: () async { + if (isContentUri(source.path)) { + return PlatformBridge.isSafTreeAccessible(source.path); + } + // Recheck access after the file probes, before deleting a page. + // An empty but readable source is valid; an offline one throws. + await Directory( + securityAccess?.path ?? source.path, + ).list(followLinks: false).take(1).drain(); + return true; + }, + ); } finally { if (securityAccess != null) { await PlatformBridge.stopAccessingIosBookmark(securityAccess); diff --git a/lib/services/library_cleanup.dart b/lib/services/library_cleanup.dart new file mode 100644 index 00000000..bcda0982 --- /dev/null +++ b/lib/services/library_cleanup.dart @@ -0,0 +1,82 @@ +import 'package:sqflite/sqflite.dart'; +import 'package:spotiflac_android/utils/file_access.dart'; + +/// Pages by stable ID (not OFFSET, since rows are removed during traversal). +/// A high-water mark bounds the run if a concurrent scan adds new rows. +Future cleanupMissingLibraryRows( + Database db, { + String? sourceId, + Future Function()? canDelete, + Future> Function(List) checkPaths = + fileExistenceByPath, +}) async { + const pageSize = 256; + final sourceWhere = sourceId == null ? null : 'source_id = ?'; + final sourceArgs = sourceId == null ? null : [sourceId]; + final last = await db.query( + 'library', + columns: ['id'], + where: sourceWhere, + whereArgs: sourceArgs, + orderBy: 'id DESC', + limit: 1, + ); + if (last.isEmpty) return 0; + final upperId = last.single['id'] as String; + String? cursor; + var removed = 0; + while (true) { + final rows = await db.query( + 'library', + columns: ['id', 'file_path'], + where: [ + ?sourceWhere, + 'id <= ?', + if (cursor != null) 'id > ?', + ].join(' AND '), + whereArgs: [?sourceId, upperId, ?cursor], + orderBy: 'id ASC', + limit: pageSize, + ); + if (rows.isEmpty) break; + cursor = rows.last['id'] as String; + final paths = rows.map((row) => row['file_path'] as String).toList(); + Map checks; + try { + checks = await checkPaths(paths); + } catch (_) { + continue; // No deletion on a failed batch. + } + final missing = rows + .where((row) => checks[row['file_path']] == false) + .toList(); + if (missing.isEmpty) continue; + if (canDelete != null) { + try { + if (!await canDelete()) break; + } catch (_) { + break; + } + } + // Compare the original path as well: a scan may repair it while I/O runs. + final conditions = List.filled( + missing.length, + '(id = ? AND file_path = ?)', + ).join(' OR '); + final where = + '($conditions)${sourceWhere == null ? '' : ' AND $sourceWhere'}'; + final args = [ + for (final row in missing) ...[row['id'], row['file_path']], + ?sourceId, + ]; + removed += await db.transaction((txn) async { + await txn.rawDelete( + 'DELETE FROM library_path_keys WHERE item_id IN ' + '(SELECT id FROM library WHERE $where)', + args, + ); + return txn.rawDelete('DELETE FROM library WHERE $where', args); + }); + } + return removed; +} diff --git a/lib/services/library_database.dart b/lib/services/library_database.dart index 4a1366f0..2434a14e 100644 --- a/lib/services/library_database.dart +++ b/lib/services/library_database.dart @@ -7,6 +7,7 @@ import 'package:spotiflac_android/utils/logger.dart'; import 'package:spotiflac_android/utils/audio_format_utils.dart'; import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/services/history_database.dart'; +import 'package:spotiflac_android/services/library_cleanup.dart'; import 'package:spotiflac_android/services/sqlite_helpers.dart' as sqlite; part 'library_database_models.dart'; @@ -1944,58 +1945,17 @@ class LibraryDatabase { }); } - Future cleanupMissingFiles({String? sourceId}) async { + Future cleanupMissingFiles({ + String? sourceId, + Future Function()? canDelete, + }) async { final db = await database; - final rows = await db.query( - 'library', - columns: ['id', 'file_path'], - where: sourceId == null ? null : 'source_id = ?', - whereArgs: sourceId == null ? null : [sourceId], + final removed = await cleanupMissingLibraryRows( + db, + sourceId: sourceId, + canDelete: canDelete, ); - final missingIds = []; - const checkChunkSize = 16; - for (var i = 0; i < rows.length; i += checkChunkSize) { - final end = (i + checkChunkSize < rows.length) - ? i + checkChunkSize - : rows.length; - final chunk = rows.sublist(i, end); - final checks = await Future.wait>( - chunk.map((row) async { - final id = row['id'] as String; - final filePath = row['file_path'] as String; - return MapEntry(id, await fileExists(filePath)); - }), - ); - for (final check in checks) { - if (!check.value) { - missingIds.add(check.key); - } - } - } - - if (missingIds.isEmpty) { - return 0; - } - - var removed = 0; - const deleteChunkSize = 500; - for (var i = 0; i < missingIds.length; i += deleteChunkSize) { - final end = (i + deleteChunkSize < missingIds.length) - ? i + deleteChunkSize - : missingIds.length; - final idChunk = missingIds.sublist(i, end); - final placeholders = List.filled(idChunk.length, '?').join(','); - await db.rawDelete( - 'DELETE FROM library_path_keys WHERE item_id IN ($placeholders)', - idChunk, - ); - removed += await db.rawDelete( - 'DELETE FROM library WHERE id IN ($placeholders)', - idChunk, - ); - } - if (removed > 0) { _log.i('Cleaned up $removed missing files from library'); } diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 6b804ee4..77cff721 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -750,6 +750,23 @@ class PlatformBridge { return result as bool; } + /// Exact-path checks. Only explicit `missing` results authorize cleanup; + /// absent responses and permission/provider failures remain unknown. + static Future> safExistsBatch(List uris) async { + if (uris.isEmpty) return const {}; + final result = await _invokeMap('safExistsBatch', { + 'uris_json': jsonEncode(uris), + }); + return { + for (final uri in uris) + uri: switch (result[uri]) { + 'found' => true, + 'missing' => false, + _ => null, + }, + }; + } + /// Whether the persisted SAF grant for [treeUri] is still usable: the /// permission is present in the system's persisted list and the tree /// document still exists and is writable. Returns true on channel errors diff --git a/lib/utils/file_access.dart b/lib/utils/file_access.dart index 7251c8ca..07e658fd 100644 --- a/lib/utils/file_access.dart +++ b/lib/utils/file_access.dart @@ -296,6 +296,56 @@ Future fileExists(String? path) async { return File(realPath).exists(); } +/// Bounded, tri-state checks for destructive library cleanup. Unknown results +/// retain their database rows. CUE tracks are checked against the backing file. +Future> fileExistenceByPath(List paths) async { + final realPaths = {for (final path in paths) path: stripCueTrackSuffix(path)}; + final results = {}; + final unique = realPaths.values.toSet(); + final saf = unique.where(isContentUri).toList(growable: false); + const batchSize = 64; + for (var start = 0; start < saf.length; start += batchSize) { + final end = start + batchSize < saf.length ? start + batchSize : saf.length; + final batch = saf.sublist(start, end); + try { + results.addAll(await PlatformBridge.safExistsBatch(batch)); + } catch (_) { + // Missing native method on an older build is also inconclusive. + } + } + final local = unique.where((path) => !isContentUri(path)).toList(); + const concurrency = 16; + for (var start = 0; start < local.length; start += concurrency) { + final end = start + concurrency < local.length + ? start + concurrency + : local.length; + await Future.wait( + local.sublist(start, end).map((path) async { + if (path.isEmpty) return; + try { + final stat = await File(path).stat(); + if (stat.type != FileSystemEntityType.notFound) { + results[path] = true; + } else { + // Dart's stat also returns notFound for permission errors. Opening + // exposes the OS error without reading the audio file's contents. + final handle = await File(path).open(); + await handle.close(); + results[path] = true; + } + } on FileSystemException catch (error) { + final code = error.osError?.errorCode; + final absent = code == 2 || code == (Platform.isWindows ? 3 : 20); + results[path] = absent ? false : null; + } catch (_) { + results[path] = null; + } + }), + ); + } + return {for (final item in realPaths.entries) item.key: results[item.value]}; +} + /// Deletes [path] and reports whether the file is confirmed absent afterward. /// /// SAF providers are allowed to reject a delete request by returning `false`. diff --git a/test/library_cleanup_test.dart b/test/library_cleanup_test.dart new file mode 100644 index 00000000..f44f698e --- /dev/null +++ b/test/library_cleanup_test.dart @@ -0,0 +1,231 @@ +import 'dart:convert'; +import 'dart:io'; +import 'package:flutter/services.dart'; +import 'package:sqflite/sqflite.dart'; +import 'package:spotiflac_android/services/library_cleanup.dart'; +import 'package:spotiflac_android/utils/file_access.dart'; +import 'package:flutter_test/flutter_test.dart'; + +// Records the paging/compare-and-delete contract without requiring a device DB. +class _CleanupDatabase implements Database, Transaction { + final rows = >{}; + final keys = {}; + final limits = []; + int transactions = 0; + void add(String id, String path, {String source = 'source'}) { + rows[id] = {'id': id, 'file_path': path, 'source_id': source}; + keys.add(id); + } + + @override + Future>> query( + String table, { + bool? distinct, + List? columns, + String? where, + List? whereArgs, + String? groupBy, + String? having, + String? orderBy, + int? limit, + int? offset, + }) async { + expect(offset, isNull); + limits.add(limit); + var argsIndex = 0; + final source = where?.contains('source_id = ?') == true + ? whereArgs![argsIndex++] + : null; + final upper = where?.contains('id <= ?') == true + ? whereArgs![argsIndex++] as String + : null; + final after = where?.contains('id > ?') == true + ? whereArgs![argsIndex++] as String + : null; + final result = + rows.values.where((row) { + final id = row['id'] as String; + return (source == null || row['source_id'] == source) && + (upper == null || id.compareTo(upper) <= 0) && + (after == null || id.compareTo(after) > 0); + }).toList() + ..sort((a, b) => (a['id'] as String).compareTo(b['id'] as String)); + final sorted = orderBy == 'id DESC' ? result.reversed : result; + return sorted + .take(limit!) + .map((row) => {for (final key in columns!) key: row[key]}) + .toList(); + } + + @override + Future transaction( + Future Function(Transaction txn) action, { + bool? exclusive, + }) { + transactions++; + return action(this); + } + + @override + Future rawDelete(String sql, [List? arguments]) async { + expect(sql, contains('id = ? AND file_path = ?')); + final hasSource = sql.contains('source_id = ?'); + final count = arguments!.length - (hasSource ? 1 : 0); + final ids = []; + for (var i = 0; i < count; i += 2) { + final id = arguments[i] as String; + final row = rows[id]; + if (row != null && + row['file_path'] == arguments[i + 1] && + (!hasSource || row['source_id'] == arguments.last)) { + ids.add(id); + } + } + for (final id in ids) { + if (sql.startsWith('DELETE FROM library_path_keys')) { + keys.remove(id); + } else { + rows.remove(id); + } + } + return ids.length; + } + + @override + dynamic noSuchMethod(Invocation invocation) => super.noSuchMethod(invocation); +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('com.zarz.spotiflac/backend'); + final messenger = + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger; + tearDown(() => messenger.setMockMethodCallHandler(channel, null)); + + test( + 'cleanup pages without skipping after deletions and preserves unknown', + () async { + final db = _CleanupDatabase(); + for (var i = 0; i < 700; i++) { + db.add(i.toString().padLeft(4, '0'), 'file-$i'); + } + db.add('other', 'file-other', source: 'other'); + final checked = []; + final removed = await cleanupMissingLibraryRows( + db, + sourceId: 'source', + checkPaths: (paths) async { + expect(paths.length, lessThanOrEqualTo(256)); + checked.addAll(paths); + return { + for (final path in paths) + path: switch (int.parse(path.split('-').last) % 3) { + 0 => false, + 1 => true, + _ => null, + }, + }; + }, + ); + expect(checked.length, 700); + expect(checked.toSet().length, 700); + expect(removed, 234); + expect(db.rows.length, 467); + expect(db.keys, db.rows.keys.toSet()); + expect(db.rows.containsKey('other'), isTrue); + expect(db.limits.first, 1); + expect(db.limits.skip(1), everyElement(256)); + }, + ); + + test( + 'cleanup retains repaired paths, newly added rows and unavailable sources', + () async { + final db = _CleanupDatabase() + ..add('a', 'old') + ..add('b', 'missing'); + final removed = await cleanupMissingLibraryRows( + db, + checkPaths: (paths) async { + db.add('a', 'repaired'); + db.add('z', 'new-row'); + return {for (final path in paths) path: false}; + }, + ); + expect(removed, 1); + expect(db.keys, {'a', 'z'}); + expect( + await cleanupMissingLibraryRows( + db, + canDelete: () async => false, + checkPaths: (paths) async => {for (final path in paths) path: false}, + ), + 0, + ); + expect( + await cleanupMissingLibraryRows( + db, + checkPaths: (_) async => throw const FileSystemException('offline'), + ), + 0, + ); + expect(db.keys, {'a', 'z'}); + }, + ); + + test( + 'SAF probes use bounded batches and preserve unknown or failed results', + () async { + final batches = >[]; + messenger.setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'safExistsBatch'); + final paths = + (jsonDecode((call.arguments as Map)['uris_json'] as String) as List) + .cast(); + batches.add(paths); + if (batches.length == 2) { + throw PlatformException(code: 'permission-denied'); + } + return jsonEncode({ + for (final path in paths) + path: path.endsWith('/0') + ? 'missing' + : path.endsWith('/1') + ? 'unknown' + : 'found', + }); + }); + final paths = List.generate(140, (i) => 'content://test/$i'); + final results = await fileExistenceByPath([ + ...paths, + '${paths.first}#track01', + ]); + expect(batches.map((batch) => batch.length), [64, 64, 12]); + expect(results[paths[0]], isFalse); + expect(results['${paths.first}#track01'], isFalse); + expect(results[paths[1]], isNull); + expect(results[paths[64]], isNull); + expect(results[paths.last], isTrue); + }, + ); + + test( + 'local probes distinguish present and missing files and keep empty paths unknown', + () async { + final dir = await Directory.systemTemp.createTemp('cleanup_probes_'); + addTearDown(() => dir.delete(recursive: true)); + final file = await File('${dir.path}/album.cue').writeAsString('test'); + final absent = '${dir.path}/missing.flac'; + final result = await fileExistenceByPath([ + file.path, + '${file.path}#track01', + absent, + '', + ]); + expect(result[file.path], isTrue); + expect(result['${file.path}#track01'], isTrue); + expect(result[absent], isFalse); + expect(result[''], isNull); + }, + ); +}