From ac7840c4e5a38997f0234c80ec6c1b4a112d52bc Mon Sep 17 00:00:00 2001 From: zarzet Date: Wed, 22 Jul 2026 15:57:39 +0700 Subject: [PATCH] fix(download): persist and reconcile history reliably --- .../zarz/spotiflac/NativeDownloadFinalizer.kt | 58 +++- lib/providers/download_history_provider.dart | 265 ++++++++++++++---- lib/providers/download_queue_provider.dart | 2 +- ...download_queue_provider_native_worker.dart | 90 +++++- 4 files changed, 354 insertions(+), 61 deletions(-) diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt index 920eb2da..d853a328 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeDownloadFinalizer.kt @@ -33,7 +33,7 @@ object NativeDownloadFinalizer { const val NATIVE_WORKER_CONTRACT_VERSION = 1 // Native finalizer owns background-safe history writes while Flutter may be suspended. // Keep this schema contract in sync with Dart HistoryDatabase before bumping either side. - private const val HISTORY_SCHEMA_VERSION = 9 + private const val HISTORY_SCHEMA_VERSION = 10 private val activeFFmpegSessionIds = mutableSetOf() private val nativeFFmpegSessionIds = mutableSetOf() private val activeFFmpegSessionLock = Any() @@ -85,6 +85,8 @@ object NativeDownloadFinalizer { "spotify_id_norm", "isrc_norm", "match_key", + "album_key", + "search_text", ) private val androidStoragePathAliases = listOf( "/storage/emulated/0", @@ -2139,7 +2141,12 @@ object NativeDownloadFinalizer { genre TEXT, composer TEXT, label TEXT, - copyright TEXT + copyright TEXT, + spotify_id_norm TEXT, + isrc_norm TEXT, + match_key TEXT, + album_key TEXT, + search_text TEXT ) """.trimIndent() ) @@ -2156,6 +2163,8 @@ object NativeDownloadFinalizer { ensureHistoryColumn(db, "spotify_id_norm", "ALTER TABLE history ADD COLUMN spotify_id_norm TEXT") ensureHistoryColumn(db, "isrc_norm", "ALTER TABLE history ADD COLUMN isrc_norm TEXT") ensureHistoryColumn(db, "match_key", "ALTER TABLE history ADD COLUMN match_key TEXT") + ensureHistoryColumn(db, "album_key", "ALTER TABLE history ADD COLUMN album_key TEXT") + ensureHistoryColumn(db, "search_text", "ALTER TABLE history ADD COLUMN search_text TEXT") ensureHistoryPathKeyTable(db) if (needsBackfill) { backfillNormalizedHistoryColumns(db) @@ -2170,6 +2179,7 @@ object NativeDownloadFinalizer { db.execSQL("CREATE INDEX IF NOT EXISTS idx_history_spotify_id_norm ON history(spotify_id_norm)") db.execSQL("CREATE INDEX IF NOT EXISTS idx_history_isrc_norm ON history(isrc_norm)") db.execSQL("CREATE INDEX IF NOT EXISTS idx_history_match_key ON history(match_key)") + db.execSQL("CREATE INDEX IF NOT EXISTS idx_history_album_key ON history(album_key)") if (db.version < HISTORY_SCHEMA_VERSION) db.version = HISTORY_SCHEMA_VERSION if (deduplicateTrack) deleteDuplicateHistoryRows(db, values) db.insertWithOnConflict("history", null, values, SQLiteDatabase.CONFLICT_REPLACE) @@ -2296,8 +2306,8 @@ object NativeDownloadFinalizer { private fun backfillNormalizedHistoryColumns(db: SQLiteDatabase) { db.query( "history", - arrayOf("id", "spotify_id", "isrc", "track_name", "artist_name"), - "spotify_id_norm IS NULL OR isrc_norm IS NULL OR match_key IS NULL", + arrayOf("id", "spotify_id", "isrc", "track_name", "artist_name", "album_name", "album_artist"), + "spotify_id_norm IS NULL OR isrc_norm IS NULL OR match_key IS NULL OR album_key IS NULL OR search_text IS NULL", null, null, null, @@ -2308,6 +2318,8 @@ object NativeDownloadFinalizer { val isrcIndex = cursor.getColumnIndex("isrc") val trackIndex = cursor.getColumnIndex("track_name") val artistIndex = cursor.getColumnIndex("artist_name") + val albumIndex = cursor.getColumnIndex("album_name") + val albumArtistIndex = cursor.getColumnIndex("album_artist") while (cursor.moveToNext()) { if (idIndex < 0) continue val values = ContentValues() @@ -2315,9 +2327,18 @@ object NativeDownloadFinalizer { val isrc = cursor.getNullableString(isrcIndex) val trackName = cursor.getNullableString(trackIndex) val artistName = cursor.getNullableString(artistIndex) + val albumName = cursor.getNullableString(albumIndex) + val albumArtist = cursor.getNullableString(albumArtistIndex) values.put("spotify_id_norm", normalizeSpotifyId(spotifyId)) values.put("isrc_norm", normalizeIsrc(isrc)) values.put("match_key", matchKeyFor(trackName, artistName)) + putAlbumSearchHistoryColumns( + values, + trackName = trackName, + artistName = artistName, + albumName = albumName, + albumArtist = albumArtist, + ) db.update("history", values, "id = ?", arrayOf(cursor.getString(idIndex))) } } @@ -2354,6 +2375,35 @@ object NativeDownloadFinalizer { "match_key", matchKeyFor(values.getAsString("track_name"), values.getAsString("artist_name")), ) + putAlbumSearchHistoryColumns( + values, + trackName = values.getAsString("track_name"), + artistName = values.getAsString("artist_name"), + albumName = values.getAsString("album_name"), + albumArtist = values.getAsString("album_artist"), + ) + } + + private fun putAlbumSearchHistoryColumns( + values: ContentValues, + trackName: String?, + artistName: String?, + albumName: String?, + albumArtist: String?, + ) { + val track = normalizeLookupText(trackName) + val artist = normalizeLookupText(artistName) + val album = normalizeLookupText(albumName) + val resolvedAlbumArtist = normalizeLookupText( + albumArtist?.takeIf { it.trim().isNotEmpty() } ?: artistName, + ) + values.put("album_key", "$album|$resolvedAlbumArtist") + values.put( + "search_text", + listOf(track, artist, album, resolvedAlbumArtist) + .filter { it.isNotEmpty() } + .joinToString(" "), + ) } private fun normalizeLookupText(value: String?): String = diff --git a/lib/providers/download_history_provider.dart b/lib/providers/download_history_provider.dart index e0a266e3..f89f97f2 100644 --- a/lib/providers/download_history_provider.dart +++ b/lib/providers/download_history_provider.dart @@ -316,6 +316,7 @@ class DownloadHistoryNotifier extends Notifier { bool _isSafRepairInProgress = false; bool _isAudioMetadataBackfillInProgress = false; bool _startupMaintenanceScheduled = false; + Future _historyWriteChain = Future.value(); @override DownloadHistoryState build() { @@ -443,6 +444,28 @@ class DownloadHistoryNotifier extends Notifier { return ''; } + List _conversionRenameCandidates( + String fileName, { + bool includeAlternateExtensions = false, + }) { + if (fileName.trim().isEmpty) return const []; + final dotIndex = fileName.lastIndexOf('.'); + if (dotIndex < 0) return [fileName]; + final baseName = fileName.substring(0, dotIndex); + final extension = fileName.substring(dotIndex); + final plainBase = baseName.endsWith('_converted') + ? baseName.substring(0, baseName.length - '_converted'.length) + : baseName; + return { + fileName, + if (plainBase != baseName) '$plainBase$extension', + if (plainBase == baseName) '${baseName}_converted$extension', + if (includeAlternateExtensions) + for (final audioExtension in _audioExtensions) + '$plainBase$audioExtension', + }.toList(growable: false); + } + Future _repairMissingSafEntries( List items, { required int maxItems, @@ -457,7 +480,6 @@ class DownloadHistoryNotifier extends Notifier { for (var i = 0; i < items.length; i++) { final item = items[i]; if (item.storageMode != 'saf') continue; - if (item.safRepaired) continue; if (item.downloadTreeUri == null || item.downloadTreeUri!.isEmpty) { continue; } @@ -531,13 +553,23 @@ class DownloadHistoryNotifier extends Notifier { } try { - final resolved = await PlatformBridge.resolveSafFile( - treeUri: item.downloadTreeUri!, - relativeDir: item.safRelativeDir ?? '', - fileName: fallbackName, - ); - final newUri = (resolved['uri'] as String? ?? '').trim(); - if (newUri.isEmpty) continue; + 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( @@ -546,7 +578,7 @@ class DownloadHistoryNotifier extends Notifier { (newRelativeDir != null && newRelativeDir.isNotEmpty) ? newRelativeDir : item.safRelativeDir, - safFileName: fallbackName, + safFileName: resolvedFileName, safRepaired: true, ); @@ -935,7 +967,7 @@ class DownloadHistoryNotifier extends Notifier { state = state.copyWith(loadedIndexVersion: state.loadedIndexVersion + 1); } - Future _putInMemoryHistory( + Future<({DownloadHistoryItem item, String? existingId})> _resolveHistoryItem( DownloadHistoryItem item, ) async { DownloadHistoryItem? existing; @@ -987,29 +1019,34 @@ class DownloadHistoryNotifier extends Notifier { normalizeOptionalString(item.copyright) ?? normalizeOptionalString(existing.copyright), ); + return (item: mergedItem, existingId: existing?.id); + } - if (existing != null) { + void _putResolvedHistoryInMemory( + DownloadHistoryItem item, + String? existingId, + ) { + if (existingId != null) { final updatedItems = state.items - .where((i) => i.id != existing!.id) + .where((candidate) => candidate.id != existingId) .toList(); - updatedItems.insert(0, mergedItem); + updatedItems.insert(0, item); final updatedLookupItems = state.lookupItems - .where((i) => i.id != existing!.id) + .where((candidate) => candidate.id != existingId) .toList(growable: false); state = state.copyWith( items: updatedItems, - lookupItems: [mergedItem, ...updatedLookupItems], + lookupItems: [item, ...updatedLookupItems], ); - _historyLog.d('Updated existing history entry: ${mergedItem.trackName}'); + _historyLog.d('Updated existing history entry: ${item.trackName}'); } else { state = state.copyWith( - items: [mergedItem, ...state.items], + items: [item, ...state.items], totalCount: state.totalCount + 1, - lookupItems: [mergedItem, ...state.lookupItems], + lookupItems: [item, ...state.lookupItems], ); - _historyLog.d('Added new history entry: ${mergedItem.trackName}'); + _historyLog.d('Added new history entry: ${item.trackName}'); } - return mergedItem; } List _lookupItemsWithUpdates( @@ -1028,40 +1065,65 @@ class DownloadHistoryNotifier extends Notifier { return byId.values.toList(growable: false); } - void addToHistory( + Future addToHistory( DownloadHistoryItem item, { bool preserveTrackVariant = false, - }) => _persistHistoryItem( - item, - 'save to database', - preserveTrackVariant: preserveTrackVariant, + }) => _enqueueHistoryWrite( + () => _persistHistoryItem( + item, + 'save to database', + preserveTrackVariant: preserveTrackVariant, + ), ); - void adoptNativeHistoryItem( + Future adoptNativeHistoryItem( DownloadHistoryItem item, { bool preserveTrackVariant = false, - }) => _persistHistoryItem( - item, - 'adopt native history item', - preserveTrackVariant: preserveTrackVariant, + }) => _enqueueHistoryWrite( + () => _persistHistoryItem( + item, + 'adopt native history item', + preserveTrackVariant: preserveTrackVariant, + ), ); - void _persistHistoryItem( + Future _enqueueHistoryWrite(Future Function() operation) { + final pending = _historyWriteChain.then((_) => operation()); + _historyWriteChain = pending.then( + (_) {}, + onError: (Object _, StackTrace _) {}, + ); + return pending; + } + + Future _persistHistoryItem( DownloadHistoryItem item, String action, { required bool preserveTrackVariant, - }) { - unawaited( - () async { - final persistedItem = preserveTrackVariant - ? _putInMemoryTrackVariant(item) - : await _putInMemoryHistory(item); - await _db.upsert(persistedItem.toJson()); - _bumpHistoryRevision(); - }().catchError((Object e, StackTrace stack) { - _historyLog.e('Failed to $action: $e', e, stack); - }), - ); + }) async { + try { + if (preserveTrackVariant) { + await _db.upsert(item.toJson()); + _putInMemoryTrackVariant(item); + } else { + final resolved = await _resolveHistoryItem(item); + await _db.upsert(resolved.item.toJson()); + _putResolvedHistoryInMemory(resolved.item, resolved.existingId); + } + int? persistedCount; + try { + persistedCount = await _db.getCount(); + } catch (error) { + _historyLog.w('History saved but count refresh failed: $error'); + } + state = state.copyWith( + totalCount: persistedCount ?? state.totalCount, + loadedIndexVersion: state.loadedIndexVersion + 1, + ); + } catch (e, stack) { + _historyLog.e('Failed to $action: $e', e, stack); + rethrow; + } } DownloadHistoryItem _putInMemoryTrackVariant(DownloadHistoryItem item) { @@ -1298,18 +1360,31 @@ class DownloadHistoryNotifier extends Notifier { '.opus', '.ogg', '.wav', + '.aiff', '.aac', + '.mp4', ]; - Future _findConvertedSibling(String originalPath) async { + Future _findConvertedSibling( + String originalPath, { + bool includeAlternateExtensions = true, + }) async { final dotIndex = originalPath.lastIndexOf('.'); if (dotIndex < 0) return null; - final basePath = originalPath.substring(0, dotIndex); - final originalExt = originalPath.substring(dotIndex).toLowerCase(); + final directoryPrefix = originalPath.substring( + 0, + originalPath.lastIndexOf(Platform.pathSeparator) + 1, + ); + final fileName = originalPath.substring( + originalPath.lastIndexOf(Platform.pathSeparator) + 1, + ); - for (final ext in _audioExtensions) { - if (ext == originalExt) continue; - final candidatePath = '$basePath$ext'; + for (final candidateName in _conversionRenameCandidates( + fileName, + includeAlternateExtensions: includeAlternateExtensions, + )) { + final candidatePath = '$directoryPrefix$candidateName'; + if (candidatePath == originalPath) continue; try { if (await fileExists(candidatePath)) return candidatePath; } catch (_) {} @@ -1317,6 +1392,67 @@ class DownloadHistoryNotifier extends Notifier { return null; } + Future verifyOrRepairHistoryItem(DownloadHistoryItem item) async { + if (await fileExists(item.filePath)) return true; + + DownloadHistoryItem? repaired; + if (item.storageMode == 'saf' && + item.downloadTreeUri != null && + item.downloadTreeUri!.isNotEmpty) { + var fileName = (item.safFileName ?? '').trim(); + if (fileName.isEmpty && isContentUri(item.filePath)) { + fileName = _fileNameFromUri(item.filePath); + } + for (final candidate in _conversionRenameCandidates(fileName)) { + try { + final resolved = await PlatformBridge.resolveSafFile( + treeUri: item.downloadTreeUri!, + relativeDir: item.safRelativeDir ?? '', + fileName: candidate, + ); + final uri = (resolved['uri'] as String? ?? '').trim(); + if (uri.isEmpty || !await fileExists(uri)) continue; + final relativeDir = (resolved['relative_dir'] as String? ?? '') + .trim(); + repaired = item.copyWith( + filePath: uri, + safFileName: candidate, + safRelativeDir: relativeDir.isEmpty + ? item.safRelativeDir + : relativeDir, + safRepaired: true, + ); + break; + } catch (error) { + _historyLog.w('Failed to resolve renamed SAF file: $error'); + } + } + } else if (!isContentUri(item.filePath)) { + final sibling = await _findConvertedSibling( + item.filePath, + includeAlternateExtensions: false, + ); + if (sibling != null) repaired = item.copyWith(filePath: sibling); + } + + if (repaired == null) return false; + await _db.upsert(repaired.toJson()); + final updatedItems = state.items + .map((entry) => entry.id == repaired!.id ? repaired : entry) + .toList(growable: false); + final updatedLookupItems = state.lookupItems + .map((entry) => entry.id == repaired!.id ? repaired : entry) + .toList(growable: false); + state = state.copyWith( + items: updatedItems, + lookupItems: updatedLookupItems, + ); + _historyLog.i( + 'Reconciled renamed conversion: ${item.filePath} -> ${repaired.filePath}', + ); + return true; + } + Future< ({ List orphanedIds, @@ -1615,7 +1751,12 @@ final downloadHistoryExistsProvider = FutureProvider.autoDispose ref.watch( downloadHistoryProvider.select((state) => state.loadedIndexVersion), ); - return HistoryDatabase.instance.existsTrack(request); + final notifier = ref.read(downloadHistoryProvider.notifier); + final row = await HistoryDatabase.instance.findExistingTrack(request); + if (row == null) return false; + return notifier.verifyOrRepairHistoryItem( + DownloadHistoryItem.fromJson(row), + ); }); final downloadHistoryBatchExistsProvider = FutureProvider.autoDispose @@ -1623,7 +1764,29 @@ final downloadHistoryBatchExistsProvider = FutureProvider.autoDispose ref.watch( downloadHistoryProvider.select((state) => state.loadedIndexVersion), ); - return HistoryDatabase.instance.existingTrackKeys(request.tracks); + final notifier = ref.read(downloadHistoryProvider.notifier); + final rows = await HistoryDatabase.instance.findExistingTracks( + request.tracks, + ); + final found = {}; + const chunkSize = 16; + for (var start = 0; start < rows.length; start += chunkSize) { + final end = min(start + chunkSize, rows.length); + final checks = await Future.wait( + List.generate(end - start, (offset) async { + final index = start + offset; + final row = rows[index]; + if (row == null) return null; + final exists = await notifier.verifyOrRepairHistoryItem( + DownloadHistoryItem.fromJson(row), + ); + if (!exists) return null; + return request.tracks[index].lookupKey; + }), + ); + found.addAll(checks.whereType()); + } + return found; }); class DownloadedAlbumTracksRequest { diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index cd4642ae..4321e236 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -3856,7 +3856,7 @@ class DownloadQueueNotifier extends Notifier { final historyBitrate = isLossyOutput ? finalBitrateKbps : null; if (settings.saveDownloadHistory) { - ref + await ref .read(downloadHistoryProvider.notifier) .addToHistory( _historyItemFromResult( diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index 9f52a433..f2b3bd8d 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -31,6 +31,73 @@ class _NativeWorkerStartupTimeout implements Exception { } extension _DownloadQueueNativeWorker on DownloadQueueNotifier { + Future _persistNativeFinalizedHistoryFallback( + _NativeWorkerRequestContext context, + Map result, + String filePath, + ) async { + final format = + normalizeAudioFormatValue( + result['audio_codec']?.toString() ?? result['format']?.toString(), + ) ?? + normalizeAudioFormatValue(audioFormatForPath(filePath)); + final isLossy = isLossyAudioFormat(format); + final bitDepth = isLossy + ? null + : readPositiveInt(result['actual_bit_depth'] ?? result['bit_depth']); + final sampleRate = isLossy + ? null + : readPositiveInt( + result['actual_sample_rate'] ?? result['sample_rate'], + ); + final bitrate = isLossy + ? readPositiveBitrateKbps(result['actual_bitrate'] ?? result['bitrate']) + : null; + final storedQuality = + result['quality']?.toString().trim().isNotEmpty == true + ? result['quality'].toString() + : context.quality; + final quality = + resolveDisplayQuality( + filePath: filePath, + fileName: result['file_name']?.toString(), + detectedFormat: format, + bitDepth: bitDepth, + sampleRate: sampleRate, + bitrateKbps: bitrate, + storedQuality: storedQuality, + ) ?? + storedQuality; + final useSaf = context.storageMode == 'saf'; + final resultFileName = result['file_name']?.toString().trim(); + + await ref + .read(downloadHistoryProvider.notifier) + .addToHistory( + _historyItemFromResult( + item: context.item, + trackToDownload: context.item.track, + result: result, + filePath: filePath, + quality: quality, + useSaf: useSaf, + downloadTreeUri: context.downloadTreeUri, + safRelativeDir: context.safRelativeDir, + safFileName: resultFileName?.isNotEmpty == true + ? resultFileName + : context.safFileName, + bitDepth: bitDepth, + sampleRate: sampleRate, + bitrate: bitrate, + format: format, + genre: normalizeOptionalString(result['genre']?.toString()), + label: normalizeOptionalString(result['label']?.toString()), + copyright: normalizeOptionalString(result['copyright']?.toString()), + ), + preserveTrackVariant: context.item.preserveQualityVariant, + ); + } + bool _canUseAndroidNativeWorker(AppSettings settings) { if (!Platform.isAndroid || !settings.nativeDownloadWorkerEnabled) { return false; @@ -869,7 +936,7 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { final historyItem = result['history_item']; if (historyItem is Map) { try { - ref + await ref .read(downloadHistoryProvider.notifier) .adoptNativeHistoryItem( DownloadHistoryItem.fromJson( @@ -879,12 +946,25 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { ); } catch (e) { _log.w('Failed to adopt native history item: $e'); - await ref - .read(downloadHistoryProvider.notifier) - .reloadFromStorage(); + await _persistNativeFinalizedHistoryFallback( + context, + result, + filePath, + ); } } else if (result['history_written'] == true) { await ref.read(downloadHistoryProvider.notifier).reloadFromStorage(); + } else if (result['already_exists'] == true) { + await ref.read(downloadHistoryProvider.notifier).reloadFromStorage(); + } else { + _log.w( + 'Native finalizer completed without history; persisting Dart fallback', + ); + await _persistNativeFinalizedHistoryFallback( + context, + result, + filePath, + ); } } _completedInSession++; @@ -1107,7 +1187,7 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { lowerFilePath.endsWith('.ogg'); if (settings.saveDownloadHistory) { - ref + await ref .read(downloadHistoryProvider.notifier) .addToHistory( _historyItemFromResult(