From 6dbca64a80e9bc95b447b74f375a4b1d1e4fcfe2 Mon Sep 17 00:00:00 2001 From: zarzet Date: Tue, 11 Aug 2026 16:50:35 +0700 Subject: [PATCH] perf(history): batch SAF orphan inspection #511 --- .../kotlin/com/zarz/spotiflac/MainActivity.kt | 7 + .../com/zarz/spotiflac/MainActivitySafScan.kt | 241 +++++++++++++++++- lib/providers/download_history_provider.dart | 152 ++++++----- ...download_history_provider_maintenance.dart | 103 ++++---- lib/services/platform_bridge.dart | 17 ++ test/platform_bridge_metadata_test.dart | 37 +++ 6 files changed, 434 insertions(+), 123 deletions(-) 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 8877ad1b..199eec09 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivity.kt @@ -973,6 +973,13 @@ class MainActivity: FlutterFragmentActivity() { } result.success(response) } + "inspectSafFiles" -> { + val requestsJson = call.argument("requests_json") ?: "[]" + val response = withContext(Dispatchers.IO) { + inspectSafFiles(requestsJson) + } + result.success(response) + } "safCopyToTemp" -> { val uriStr = call.argument("uri") ?: "" val tempPath = 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 7e3d33e5..cb8def30 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/MainActivitySafScan.kt @@ -169,6 +169,246 @@ internal fun MainActivity.resolveSafFile(treeUriStr: String, relativeDir: String return obj.toString() } +private data class SafFileInspectionRequest( + val key: String, + val treeUri: String, + val relativeDir: String, + val currentUri: String, + val fileNames: List, +) + +/** + * Inspects many SAF history entries while walking each document tree at most + * once. The old per-file resolver could repeat a 20k-document breadth-first + * search for every missing history row and every conversion filename variant. + */ +internal fun MainActivity.inspectSafFiles(requestsJson: String): String { + val output = JSONObject() + val resultsByKey = linkedMapOf() + val requests = mutableListOf() + + fun result( + key: String, + status: String, + uri: String = "", + fileName: String = "", + relativeDir: String = "", + ) = JSONObject().apply { + put("key", key) + put("status", status) + put("uri", uri) + put("file_name", fileName) + put("relative_dir", relativeDir) + } + + try { + val rawRequests = JSONArray(requestsJson) + for (index in 0 until rawRequests.length()) { + val raw = rawRequests.optJSONObject(index) ?: continue + val key = raw.optString("key").trim() + if (key.isBlank()) continue + val names = mutableListOf() + val rawNames = raw.optJSONArray("file_names") + if (rawNames != null) { + for (nameIndex in 0 until rawNames.length()) { + val sanitized = SafDownloadHandler.sanitizeFilename( + rawNames.optString(nameIndex).trim(), + ) + if (sanitized.isNotBlank() && sanitized !in names) { + names.add(sanitized) + } + } + } + requests.add( + SafFileInspectionRequest( + key = key, + treeUri = raw.optString("tree_uri").trim(), + relativeDir = SafDownloadHandler.sanitizeRelativeDir( + raw.optString("relative_dir"), + ), + currentUri = raw.optString("current_uri").trim(), + fileNames = names, + ), + ) + } + + val pendingByTree = linkedMapOf>() + for (request in requests) { + if (request.currentUri.startsWith("content://")) { + try { + val current = DocumentFile.fromSingleUri(this, Uri.parse(request.currentUri)) + if (current != null && current.exists() && current.isFile) { + resultsByKey[request.key] = result( + key = request.key, + status = "found", + uri = request.currentUri, + fileName = current.name.orEmpty(), + relativeDir = request.relativeDir, + ) + continue + } + } catch (_: Exception) { + // Fall through to the persisted tree lookup. + } + } + if (request.treeUri.isBlank() || request.fileNames.isEmpty()) { + resultsByKey[request.key] = result(request.key, "unknown") + continue + } + pendingByTree.getOrPut(request.treeUri) { mutableListOf() }.add(request) + } + + for ((treeUriString, treeRequests) in pendingByTree) { + val treeUri = try { + Uri.parse(treeUriString) + } catch (_: Exception) { + null + } + val hasPermission = treeUri != null && contentResolver.persistedUriPermissions.any { + it.uri == treeUri && it.isReadPermission && it.isWritePermission + } + val root = if (hasPermission) { + try { + DocumentFile.fromTreeUri(this, treeUri) + } catch (_: Exception) { + null + } + } else { + null + } + if (root == null || !root.exists() || !root.canWrite()) { + for (request in treeRequests) { + resultsByKey[request.key] = result(request.key, "unknown") + } + continue + } + + val unresolved = mutableListOf() + val directoryCache = mutableMapOf>() + val resolvedDirectoryCache = mutableMapOf() + for (request in treeRequests) { + val directDir = if (resolvedDirectoryCache.containsKey(request.relativeDir)) { + resolvedDirectoryCache[request.relativeDir] + } else { + val resolved = try { + SafDownloadHandler.findDocumentDir(this, treeUri!!, request.relativeDir) + } catch (_: Exception) { + null + } + resolvedDirectoryCache[request.relativeDir] = resolved + resolved + } + val lookup = if (directDir == null) { + emptyMap() + } else { + getSafChildFileLookup(directDir, directoryCache) + } + val directName = request.fileNames.firstOrNull { + lookup.containsKey(it.lowercase(Locale.ROOT)) + } + val direct = directName?.let { lookup[it.lowercase(Locale.ROOT)] } + if (direct != null && direct.isFile) { + resultsByKey[request.key] = result( + key = request.key, + status = "found", + uri = direct.uri.toString(), + fileName = direct.name ?: directName, + relativeDir = request.relativeDir, + ) + } else { + unresolved.add(request) + } + } + if (unresolved.isEmpty()) continue + + val wantedNames = unresolved + .flatMap { it.fileNames } + .map { it.lowercase(Locale.ROOT) } + .toSet() + val requestKeysByName = mutableMapOf>() + for (request in unresolved) { + for (fileName in request.fileNames) { + requestKeysByName + .getOrPut(fileName.lowercase(Locale.ROOT)) { mutableSetOf() } + .add(request.key) + } + } + val matches = mutableMapOf>() + val matchedRequestKeys = mutableSetOf() + val queue: ArrayDeque> = ArrayDeque() + queue.add(root to "") + var visited = 0 + val maxVisited = 50000 + var scanComplete = true + + while (queue.isNotEmpty() && matchedRequestKeys.size < unresolved.size) { + if (visited >= maxVisited) { + scanComplete = false + break + } + val (directory, path) = queue.removeFirst() + val children = try { + directory.listFiles() + } catch (_: Exception) { + scanComplete = false + break + } + for (child in children) { + visited++ + if (visited >= maxVisited) { + scanComplete = false + break + } + if (child.isDirectory) { + val childName = child.name ?: continue + val childPath = if (path.isBlank()) childName else "$path/$childName" + queue.add(child to childPath) + } else if (child.isFile) { + val childName = child.name ?: continue + val normalized = childName.lowercase(Locale.ROOT) + if (normalized in wantedNames && normalized !in matches) { + matches[normalized] = child to path + matchedRequestKeys.addAll( + requestKeysByName[normalized].orEmpty(), + ) + } + } + } + } + + for (request in unresolved) { + val matchedName = request.fileNames.firstOrNull { + matches.containsKey(it.lowercase(Locale.ROOT)) + } + val match = matchedName?.let { matches[it.lowercase(Locale.ROOT)] } + resultsByKey[request.key] = if (match != null) { + result( + key = request.key, + status = "found", + uri = match.first.uri.toString(), + fileName = match.first.name ?: matchedName, + relativeDir = match.second, + ) + } else { + result(request.key, if (scanComplete) "missing" else "unknown") + } + } + } + } catch (error: Exception) { + android.util.Log.w("SpotiFLAC", "Batch SAF inspection failed: ${error.message}") + for (request in requests) { + resultsByKey.putIfAbsent(request.key, result(request.key, "unknown")) + } + } + + val results = JSONArray() + for (request in requests) { + results.put(resultsByKey[request.key] ?: result(request.key, "unknown")) + } + output.put("results", results) + return output.toString() +} + /** * Extract the audio filename referenced by a CUE sheet file. @@ -1001,4 +1241,3 @@ internal fun MainActivity.getSafFileModTimes(urisJson: String): String { return result.toString() } - diff --git a/lib/providers/download_history_provider.dart b/lib/providers/download_history_provider.dart index 9ab82121..f20966da 100644 --- a/lib/providers/download_history_provider.dart +++ b/lib/providers/download_history_provider.dart @@ -59,7 +59,6 @@ String? resolvePersistedHistoryQuality({ class DownloadHistoryNotifier extends Notifier { static const int _initialHistoryLoadLimit = 100; - static const int _safRepairBatchSize = 20; static const int _safRepairMaxPerLaunch = 60; static const int _orphanCleanupMaxPerLaunch = 80; static const int _audioMetadataBackfillMaxPerLaunch = 24; @@ -697,80 +696,36 @@ class DownloadHistoryNotifier extends Notifier { final replacementFileNames = {}; final replacementRelativeDirs = {}; final pathById = {}; + final regularEntries = >[]; + final safEntries = >[]; + + for (final entry in entries) { + final id = entry['id'] as String; + final filePath = (entry['file_path'] as String? ?? '').trim(); + if (filePath.isEmpty) continue; + pathById[id] = filePath; + if (entry['storage_mode'] == 'saf') { + safEntries.add(entry); + } else { + regularEntries.add(entry); + } + } + const checkChunkSize = 16; - for (var i = 0; i < entries.length; i += checkChunkSize) { - final end = (i + checkChunkSize < entries.length) + for (var i = 0; i < regularEntries.length; i += checkChunkSize) { + final end = (i + checkChunkSize < regularEntries.length) ? i + checkChunkSize - : entries.length; - final chunk = entries.sublist(i, end); + : regularEntries.length; + final chunk = regularEntries.sublist(i, end); final checks = await Future.wait?>( chunk.map((entry) async { final id = entry['id'] as String; - final filePath = entry['file_path'] as String?; - if (filePath == null || filePath.isEmpty) return null; - pathById[id] = filePath; + final filePath = entry['file_path'] as String; try { if (await fileExists(filePath)) return MapEntry(id, true); - if (entry['storage_mode'] == 'saf') { - final treeUri = (entry['download_tree_uri'] as String? ?? '') - .trim(); - var fileName = (entry['saf_file_name'] as String? ?? '').trim(); - if (fileName.isEmpty && isContentUri(filePath)) { - fileName = _fileNameFromUri(filePath); - } - if (treeUri.isEmpty || fileName.isEmpty) { - return MapEntry(id, null); - } - - bool treeAccessible; - try { - treeAccessible = await PlatformBridge.validateSafTreeAccess( - treeUri, - ); - } catch (error) { - _historyLog.w( - 'Unable to verify SAF tree while checking $id: $error', - ); - return MapEntry(id, null); - } - if (!treeAccessible) { - return MapEntry(id, null); - } - - for (final candidate in _conversionRenameCandidates( - fileName, - includeAlternateExtensions: true, - )) { - try { - final resolved = await PlatformBridge.resolveSafFile( - treeUri: treeUri, - relativeDir: entry['saf_relative_dir'] as String? ?? '', - fileName: candidate, - ); - final uri = (resolved['uri'] as String? ?? '').trim(); - if (uri.isEmpty) continue; - replacementPaths[id] = uri; - replacementFileNames[id] = candidate; - final relativeDir = - (resolved['relative_dir'] as String? ?? '').trim(); - if (relativeDir.isNotEmpty) { - replacementRelativeDirs[id] = relativeDir; - } - pathById[id] = uri; - return MapEntry(id, true); - } catch (error) { - _historyLog.w( - 'Unable to resolve SAF file while checking $id: $error', - ); - return MapEntry(id, null); - } - } - return MapEntry(id, false); - } - final sibling = await _findConvertedSibling(filePath); if (sibling != null) { _historyLog.i( @@ -798,6 +753,73 @@ class DownloadHistoryNotifier extends Notifier { } } + if (safEntries.isNotEmpty && Platform.isAndroid) { + final requests = >[]; + for (final entry in safEntries) { + final id = entry['id'] as String; + final filePath = entry['file_path'] as String; + var fileName = (entry['saf_file_name'] as String? ?? '').trim(); + if (fileName.isEmpty && isContentUri(filePath)) { + fileName = _fileNameFromUri(filePath); + } + requests.add({ + 'key': id, + 'tree_uri': entry['download_tree_uri'] as String? ?? '', + 'relative_dir': entry['saf_relative_dir'] as String? ?? '', + 'current_uri': isContentUri(filePath) ? filePath : '', + 'file_names': _conversionRenameCandidates( + fileName, + includeAlternateExtensions: true, + ), + }); + } + + try { + final results = await PlatformBridge.inspectSafFiles(requests); + final resultsById = >{ + for (final result in results) + if ((result['key'] as String? ?? '').isNotEmpty) + result['key'] as String: result, + }; + for (final entry in safEntries) { + final id = entry['id'] as String; + final result = resultsById[id]; + final status = result?['status'] as String? ?? 'unknown'; + if (status == 'missing') { + orphanedIds.add(id); + _historyLog.d( + 'Found orphaned SAF entry: $id (${pathById[id] ?? ''})', + ); + continue; + } + if (status != 'found' || result == null) continue; + + final uri = (result['uri'] as String? ?? '').trim(); + if (uri.isEmpty) continue; + final fileName = (result['file_name'] as String? ?? '').trim(); + final relativeDir = (result['relative_dir'] as String? ?? '').trim(); + final oldPath = pathById[id] ?? ''; + final oldFileName = (entry['saf_file_name'] as String? ?? '').trim(); + final oldRelativeDir = (entry['saf_relative_dir'] as String? ?? '') + .trim(); + if (uri != oldPath || + (fileName.isNotEmpty && fileName != oldFileName) || + (relativeDir.isNotEmpty && relativeDir != oldRelativeDir)) { + replacementPaths[id] = uri; + if (fileName.isNotEmpty) replacementFileNames[id] = fileName; + if (relativeDir.isNotEmpty) { + replacementRelativeDirs[id] = relativeDir; + } + pathById[id] = uri; + } + } + } catch (error) { + // A bridge or provider error is inconclusive. Never delete SAF history + // when the batch inspection could not establish that files are absent. + _historyLog.w('Unable to inspect SAF history entries: $error'); + } + } + return ( orphanedIds: orphanedIds, replacementPaths: replacementPaths, diff --git a/lib/providers/download_history_provider_maintenance.dart b/lib/providers/download_history_provider_maintenance.dart index e17157a2..e0010678 100644 --- a/lib/providers/download_history_provider_maintenance.dart +++ b/lib/providers/download_history_provider_maintenance.dart @@ -166,27 +166,11 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier { var verifiedCount = 0; try { - for (var c = 0; c < selectedIndexes.length; c++) { - final i = selectedIndexes[c]; + final requests = >[]; + for (final i in selectedIndexes) { final item = items[i]; final rawPath = item.filePath.trim(); final isDirectSafUri = rawPath.isNotEmpty && isContentUri(rawPath); - - if (isDirectSafUri) { - final exists = await fileExists(rawPath); - if (exists) { - final verified = item.copyWith( - safRepaired: true, - safFileName: item.safFileName ?? _fileNameFromUri(rawPath), - ); - updatedItems[i] = verified; - changed = true; - verifiedCount++; - persistedUpdates.add(verified.toJson()); - continue; - } - } - var fallbackName = (item.safFileName ?? '').trim(); if (fallbackName.isEmpty && isDirectSafUri) { fallbackName = _fileNameFromUri(rawPath); @@ -195,48 +179,53 @@ extension _HistoryStartupMaintenance on DownloadHistoryNotifier { _historyLog.w('Missing SAF filename for history item: ${item.id}'); continue; } + requests.add({ + 'key': item.id, + 'tree_uri': item.downloadTreeUri, + 'relative_dir': item.safRelativeDir ?? '', + 'current_uri': isDirectSafUri ? rawPath : '', + 'file_names': _conversionRenameCandidates(fallbackName), + }); + } - try { - Map? resolved; - String? resolvedFileName; - for (final candidate in _conversionRenameCandidates(fallbackName)) { - final candidateResult = await PlatformBridge.resolveSafFile( - treeUri: item.downloadTreeUri!, - relativeDir: item.safRelativeDir ?? '', - fileName: candidate, - ); - final candidateUri = (candidateResult['uri'] as String? ?? '') - .trim(); - if (candidateUri.isEmpty) continue; - resolved = candidateResult; - resolvedFileName = candidate; - break; - } - if (resolved == null || resolvedFileName == null) continue; - final newUri = (resolved['uri'] as String).trim(); - - final newRelativeDir = resolved['relative_dir'] as String?; - final updated = item.copyWith( - filePath: newUri, - safRelativeDir: - (newRelativeDir != null && newRelativeDir.isNotEmpty) - ? newRelativeDir - : item.safRelativeDir, - safFileName: resolvedFileName, - safRepaired: true, - ); - - updatedItems[i] = updated; - changed = true; + final results = await PlatformBridge.inspectSafFiles(requests); + final resultsById = >{ + for (final result in results) + if ((result['key'] as String? ?? '').isNotEmpty) + result['key'] as String: result, + }; + final indexById = { + for (final index in selectedIndexes) items[index].id: index, + }; + for (final result in resultsById.values) { + if (result['status'] != 'found') continue; + final id = result['key'] as String; + final index = indexById[id]; + if (index == null) continue; + final item = items[index]; + final newUri = (result['uri'] as String? ?? '').trim(); + if (newUri.isEmpty) continue; + final resultFileName = (result['file_name'] as String? ?? '').trim(); + final resultRelativeDir = (result['relative_dir'] as String? ?? '') + .trim(); + final updated = item.copyWith( + filePath: newUri, + safRelativeDir: resultRelativeDir.isNotEmpty + ? resultRelativeDir + : item.safRelativeDir, + safFileName: resultFileName.isNotEmpty + ? resultFileName + : item.safFileName, + safRepaired: true, + ); + updatedItems[index] = updated; + changed = true; + if (newUri == item.filePath) { + verifiedCount++; + } else { repairedCount++; - persistedUpdates.add(updated.toJson()); - } catch (e) { - _historyLog.w('Failed to repair SAF URI: $e'); - } - - if ((c + 1) % DownloadHistoryNotifier._safRepairBatchSize == 0) { - await Future.delayed(const Duration(milliseconds: 16)); } + persistedUpdates.add(updated.toJson()); } if (changed) { diff --git a/lib/services/platform_bridge.dart b/lib/services/platform_bridge.dart index 196ee0ef..38c7450b 100644 --- a/lib/services/platform_bridge.dart +++ b/lib/services/platform_bridge.dart @@ -646,6 +646,23 @@ class PlatformBridge { }); } + /// Checks and repairs many SAF file references in a single native pass. + /// Android groups requests by tree and scans each tree no more than once. + static Future>> inspectSafFiles( + List> requests, + ) async { + if (requests.isEmpty) return const []; + final response = await _invokeMap('inspectSafFiles', { + 'requests_json': jsonEncode(requests), + }); + final results = response['results']; + if (results is! List) return const []; + return results + .whereType>() + .map((result) => result.map((key, value) => MapEntry('$key', value))) + .toList(growable: false); + } + static Future copyContentUriToTemp(String uri) async { final result = await _channel.invokeMethod('safCopyToTemp', {'uri': uri}); return result as String?; diff --git a/test/platform_bridge_metadata_test.dart b/test/platform_bridge_metadata_test.dart index 9861143b..79dc8efb 100644 --- a/test/platform_bridge_metadata_test.dart +++ b/test/platform_bridge_metadata_test.dart @@ -130,4 +130,41 @@ void main() { expect(arguments?['request_id'], isA()); expect(results.single['provider_id'], 'custom-metadata'); }); + + test('SAF inspection sends one batch and decodes repair results', () async { + MethodCall? capturedCall; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(backendChannel, (call) async { + capturedCall = call; + return jsonEncode({ + 'results': [ + { + 'key': 'history-1', + 'status': 'found', + 'uri': 'content://tree/repaired.flac', + 'file_name': 'repaired.flac', + 'relative_dir': 'Artist/Album', + }, + ], + }); + }); + + final results = await PlatformBridge.inspectSafFiles([ + { + 'key': 'history-1', + 'tree_uri': 'content://tree/root', + 'current_uri': 'content://tree/stale.flac', + 'relative_dir': 'Artist/Album', + 'file_names': ['song.flac', 'song_converted.flac'], + }, + ]); + + expect(capturedCall?.method, 'inspectSafFiles'); + final arguments = capturedCall?.arguments as Map; + final requests = jsonDecode(arguments['requests_json'] as String) as List; + expect(requests, hasLength(1)); + expect((requests.single as Map)['file_names'], hasLength(2)); + expect(results.single['status'], 'found'); + expect(results.single['relative_dir'], 'Artist/Album'); + }); }