diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt index d37391bd..1d754925 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizationPolicy.kt @@ -157,6 +157,24 @@ internal object NativeFinalizationPolicy { return "$stem - $qualityLabel$extension" } + /** + * Returns the user-facing name that a deferred SAF download was assigned + * before its audio was materialized in the app cache. Container and + * decryption passes may replace [currentFileName] with a temporary + * `native_saf_work_*` name, which must never become the published name. + */ + fun logicalOutputFileName( + deferredSafPublish: Boolean, + resultSafFileName: String?, + requestSafFileName: String?, + currentFileName: String, + ): String { + if (!deferredSafPublish) return currentFileName + return normalizeOptional(resultSafFileName) + ?: normalizeOptional(requestSafFileName) + ?: currentFileName + } + fun resolvePreferredDecryptionExtension( inputPath: String, requested: String, diff --git a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt index 89f40686..466b91aa 100644 --- a/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt +++ b/android/app/src/main/kotlin/com/zarz/spotiflac/NativeFinalizerMedia.kt @@ -20,6 +20,7 @@ import com.zarz.spotiflac.NativeFinalizationPolicy.displayAudioQuality import com.zarz.spotiflac.NativeFinalizationPolicy.formatIndexTag import com.zarz.spotiflac.NativeFinalizationPolicy.isLosslessAudioCodec import com.zarz.spotiflac.NativeFinalizationPolicy.isLossyAudioCodec +import com.zarz.spotiflac.NativeFinalizationPolicy.logicalOutputFileName import com.zarz.spotiflac.NativeFinalizationPolicy.normalizeAudioCodec import com.zarz.spotiflac.NativeFinalizationPolicy.resolvePreferredDecryptionExtension import gobackend.Gobackend @@ -61,12 +62,18 @@ internal fun NativeDownloadFinalizer.finalizeQualityVariantFilename( return } + val logicalFileName = logicalOutputFileName( + deferredSafPublish = isDeferredSafPublish(input), + resultSafFileName = input.result.optString("saf_final_file_name", ""), + requestSafFileName = input.request.optString("saf_file_name", ""), + currentFileName = state.fileName, + ) val preferredName = applyQualityVariantFilenameLabel( - fileName = state.fileName, + fileName = logicalFileName, stagingLabel = stagingLabel, qualityLabel = qualityLabel, ) - if (preferredName == state.fileName) return + if (preferredName == logicalFileName && preferredName == state.fileName) return input.result.put("quality_variant_file_name", preferredName) if (isDeferredSafPublish(input)) { state.fileName = preferredName diff --git a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt index 32609825..c8be74cc 100644 --- a/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt +++ b/android/app/src/test/kotlin/com/zarz/spotiflac/NativeFinalizationPolicyTest.kt @@ -150,6 +150,38 @@ class NativeFinalizationPolicyTest { ) } + @Test + fun deferredSafNamingNeverPublishesTheNativeCacheName() { + val logicalName = NativeFinalizationPolicy.logicalOutputFileName( + deferredSafPublish = true, + resultSafFileName = "Sunidhi Chauhan - Aisa Jadoo - qv_ab12cd34.flac", + requestSafFileName = "fallback.flac", + currentFileName = "native_saf_work_603020549715656640.m4a", + ) + + assertEquals( + "Sunidhi Chauhan - Aisa Jadoo - 16bit-44.1kHz.flac", + NativeFinalizationPolicy.applyQualityVariantFilenameLabel( + fileName = logicalName, + stagingLabel = "qv_ab12cd34", + qualityLabel = "16bit-44.1kHz", + ), + ) + } + + @Test + fun nonSafNamingStillFollowsTheCurrentConvertedFile() { + assertEquals( + "converted.flac", + NativeFinalizationPolicy.logicalOutputFileName( + deferredSafPublish = false, + resultSafFileName = "ignored.flac", + requestSafFileName = "ignored-too.flac", + currentFileName = "converted.flac", + ), + ) + } + @Test fun decryptionExtensionAndIndexTagsHaveStableFallbacks() { assertEquals( diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index cdfec7ea..59522695 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -74,6 +74,15 @@ Future persistBeforePublishingDownloadCompletion({ publish(); } +/// Keeps native-worker finalization visually distinct from completion. +/// Download bytes may reach 100% before SAF publishing, metadata persistence, +/// and queue reconciliation have finished. +double nativeWorkerFinalizingProgress(double progress) { + final normalized = progress.clamp(0.0, 1.0).toDouble(); + if (normalized <= 0) return 0.95; + return min(normalized, 0.99); +} + final _invalidFolderChars = RegExp(r'[<>:"/\\|?*]'); final _trimDotsAndSpacesRegex = RegExp(r'^[. ]+|[. ]+$'); final _trimUnderscoresAndSpacesRegex = RegExp(r'^[_ ]+|[_ ]+$'); @@ -1488,7 +1497,6 @@ class DownloadQueueNotifier extends Notifier { _verificationRetryGuard.retainItems(remainingIds); _rateLimitRetriedItemIds.removeWhere((id) => !remainingIds.contains(id)); } - } final downloadQueueProvider = diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index 789502c9..619854ea 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -803,7 +803,7 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { updateItemStatus( itemId, DownloadStatus.finalizing, - progress: progress <= 0 ? 0.95 : progress, + progress: nativeWorkerFinalizingProgress(progress), ); continue; } @@ -811,12 +811,34 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { if (status == 'completed') { final result = itemSnapshot['result']; if (result is Map) { - reconciledIds.add(itemId); - await _completeAndroidNativeWorkerItem( - context, - Map.from(result), - settings, - ); + try { + await _completeAndroidNativeWorkerItem( + context, + Map.from(result), + settings, + ); + reconciledIds.add(itemId); + } catch (e, stack) { + // A native output can be complete while Library persistence or + // Dart-side adoption fails. Do not leave the queue permanently at + // 100%: surface the reconciliation failure and keep processing + // the rest of the worker batch. + _log.e( + 'Failed to reconcile completed native worker item $itemId: $e', + e, + stack, + ); + if (_findItemById(itemId) != null) { + updateItemStatus( + itemId, + DownloadStatus.failed, + error: 'Downloaded file could not be added to the Library: $e', + errorType: DownloadErrorType.unknown, + ); + _failedInSession++; + } + reconciledIds.add(itemId); + } } continue; } diff --git a/lib/screens/album_screen.dart b/lib/screens/album_screen.dart index 30a48a25..12bec6f0 100644 --- a/lib/screens/album_screen.dart +++ b/lib/screens/album_screen.dart @@ -537,7 +537,12 @@ class _AlbumScreenState extends ConsumerState child: TrackListTile( track: track, isInHistory: isInHistory, - onDownload: () => _downloadTrack(context, track), + onDownload: ({bool forceQualityPicker = false}) => + _downloadTrack( + context, + track, + forceQualityPicker: forceQualityPicker, + ), clickableArtist: true, leading: SizedBox( width: 32, @@ -559,12 +564,17 @@ class _AlbumScreenState extends ConsumerState ); } - void _downloadTrack(BuildContext context, Track track) { + void _downloadTrack( + BuildContext context, + Track track, { + bool forceQualityPicker = false, + }) { downloadSingleTrack( context, ref, track, recommendedService: _recommendedDownloadService(), + forceQualityPicker: forceQualityPicker, ); } diff --git a/lib/screens/artist_screen_widgets.dart b/lib/screens/artist_screen_widgets.dart index 3236b139..fa3b3442 100644 --- a/lib/screens/artist_screen_widgets.dart +++ b/lib/screens/artist_screen_widgets.dart @@ -423,7 +423,12 @@ extension _ArtistScreenSections on _ArtistScreenState { final isQueued = queueItem != null; return InkWell( - onTap: () => _handlePopularTrackTap(track, isQueued: isQueued), + onTap: () => _handlePopularTrackTap( + track, + isQueued: isQueued, + isInHistory: isInHistory, + isInLocalLibrary: isInLocalLibrary, + ), onLongPress: () => TrackCollectionQuickActions.showTrackOptionsSheet( context, ref, @@ -531,9 +536,20 @@ extension _ArtistScreenSections on _ArtistScreenState { ); } - void _handlePopularTrackTap(Track track, {required bool isQueued}) async { + void _handlePopularTrackTap( + Track track, { + required bool isQueued, + required bool isInHistory, + required bool isInLocalLibrary, + }) async { if (isQueued) return; + final settings = ref.read(settingsProvider); + if (settings.allowQualityVariants && (isInHistory || isInLocalLibrary)) { + _downloadTrack(track); + return; + } + final playedLocal = await playLocalIfAvailable(context, ref, track); if (playedLocal) { return; diff --git a/lib/screens/playlist_screen.dart b/lib/screens/playlist_screen.dart index 9438fd38..b3df2d5d 100644 --- a/lib/screens/playlist_screen.dart +++ b/lib/screens/playlist_screen.dart @@ -323,8 +323,13 @@ class _PlaylistScreenState extends ConsumerState child: TrackListTile( track: track, isInHistory: isInHistory, - onDownload: () => - _downloadTrack(context, track, playlistPosition: index + 1), + onDownload: ({bool forceQualityPicker = false}) => + _downloadTrack( + context, + track, + playlistPosition: index + 1, + forceQualityPicker: forceQualityPicker, + ), leading: track.coverUrl != null ? CachedCoverImage( imageUrl: track.coverUrl!, @@ -358,6 +363,7 @@ class _PlaylistScreenState extends ConsumerState BuildContext context, Track track, { int? playlistPosition, + bool forceQualityPicker = false, }) { downloadSingleTrack( context, @@ -366,6 +372,7 @@ class _PlaylistScreenState extends ConsumerState recommendedService: _recommendedDownloadService(), playlistName: _playlistName, playlistPosition: playlistPosition, + forceQualityPicker: forceQualityPicker, ); } diff --git a/lib/widgets/track_list_tile.dart b/lib/widgets/track_list_tile.dart index d9ff8378..4551587e 100644 --- a/lib/widgets/track_list_tile.dart +++ b/lib/widgets/track_list_tile.dart @@ -11,14 +11,15 @@ import 'package:spotiflac_android/widgets/in_library_badge.dart'; import 'package:spotiflac_android/widgets/preview_button.dart'; import 'package:spotiflac_android/widgets/track_collection_quick_actions.dart'; -/// Track row shared by the album and playlist screens. Tap plays the local -/// copy when one exists and otherwise triggers [onDownload]; long-press opens -/// the track options sheet. Callers supply [leading] (track number, cover -/// art, ...) and choose whether the artist name links to the artist screen. +/// Track row shared by the album and playlist screens. Tap offers another +/// download when quality variants are enabled, otherwise it plays the local +/// copy when one exists and falls back to [onDownload]. Long-press opens the +/// track options sheet. Callers supply [leading] (track number, cover art, +/// ...) and choose whether the artist name links to the artist screen. class TrackListTile extends ConsumerWidget { final Track track; final bool isInHistory; - final VoidCallback onDownload; + final void Function({bool forceQualityPicker}) onDownload; final Widget leading; final bool clickableArtist; @@ -118,7 +119,12 @@ class TrackListTile extends ConsumerWidget { TrackCollectionQuickActions(track: track), ], ), - onTap: () => _handleTap(context, ref, isQueued: isQueued), + onTap: () => _handleTap( + context, + ref, + isQueued: isQueued, + isInLocalLibrary: isInLocalLibrary, + ), onLongPress: () => TrackCollectionQuickActions.showTrackOptionsSheet( context, ref, @@ -133,9 +139,16 @@ class TrackListTile extends ConsumerWidget { BuildContext context, WidgetRef ref, { required bool isQueued, + required bool isInLocalLibrary, }) async { if (isQueued) return; + final settings = ref.read(settingsProvider); + if (settings.allowQualityVariants && (isInHistory || isInLocalLibrary)) { + onDownload(forceQualityPicker: true); + return; + } + final playedLocal = await playLocalIfAvailable(context, ref, track); if (playedLocal) { return; diff --git a/test/models_and_utils_test.dart b/test/models_and_utils_test.dart index 901fcac2..e6059204 100644 --- a/test/models_and_utils_test.dart +++ b/test/models_and_utils_test.dart @@ -19,6 +19,14 @@ import 'package:spotiflac_android/utils/path_match_keys.dart'; import 'package:spotiflac_android/utils/string_utils.dart'; void main() { + group('native worker progress', () { + test('does not publish 100 percent while finalization is pending', () { + expect(nativeWorkerFinalizingProgress(0), 0.95); + expect(nativeWorkerFinalizingProgress(0.7), 0.7); + expect(nativeWorkerFinalizingProgress(1), 0.99); + }); + }); + group('native worker contracts', () { final finalizerSource = File( 'android/app/src/main/kotlin/com/zarz/spotiflac/'