From f0988070bf90a929199f63f83ec027b50e07f8e2 Mon Sep 17 00:00:00 2001 From: zarzet Date: Wed, 22 Jul 2026 15:56:07 +0700 Subject: [PATCH] fix(convert): simplify output names and harden writes --- lib/screens/track_metadata_convert.dart | 44 ++-- lib/services/batch_track_actions.dart | 177 ++++++++------ lib/services/conversion_library_service.dart | 71 ++++++ lib/services/ffmpeg_service.dart | 232 ++++++++++++++----- lib/utils/audio_conversion_utils.dart | 4 +- lib/widgets/batch_convert_sheet.dart | 7 +- test/audio_conversion_utils_test.dart | 6 +- test/batch_track_actions_test.dart | 94 ++++++++ test/conversion_library_service_test.dart | 70 ++++++ 9 files changed, 553 insertions(+), 152 deletions(-) create mode 100644 test/batch_track_actions_test.dart create mode 100644 test/conversion_library_service_test.dart diff --git a/lib/screens/track_metadata_convert.dart b/lib/screens/track_metadata_convert.dart index e5f24b66..fc2b14e9 100644 --- a/lib/screens/track_metadata_convert.dart +++ b/lib/screens/track_metadata_convert.dart @@ -821,6 +821,9 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { if (_isConverting) return; _setState(() => _isConverting = true); final losslessLabels = context.l10n.losslessConversionLabels; + String? coverPath; + String? safTempPath; + String? convertedSafTempPath; try { ScaffoldMessenger.of(context).showSnackBar( @@ -851,7 +854,6 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { durationMs: (duration ?? 0) * 1000, ); - String? coverPath; try { final tempDir = await getTemporaryDirectory(); final coverOutput = @@ -867,10 +869,11 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { String workingPath = cleanFilePath; final isSaf = _isSafFile; - String? safTempPath; if (isSaf) { - safTempPath = await PlatformBridge.copyContentUriToTemp(cleanFilePath); + safTempPath = await ConversionLibraryService.copySafSourceToTemp( + cleanFilePath, + ); if (safTempPath == null) { if (mounted) { _setState(() => _isConverting = false); @@ -916,6 +919,7 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { } return; } + if (isSaf) convertedSafTempPath = newPath; final isLosslessOutput = isLosslessConversionTarget(targetFormat); int? convertedBitDepth; @@ -1007,21 +1011,15 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { return; } - final convTarget = convertTargetExtAndMime(targetFormat); - final mimeType = convTarget.mime; - final newFileName = convertedOutputFileName( - originalFileName: oldFileName, - targetFormat: targetFormat, - keepOriginal: keepOriginal, - ); - - final safUri = await PlatformBridge.createSafFileFromPath( + final published = await ConversionLibraryService.publishSafConversion( treeUri: treeUri, relativeDir: relativeDir, - fileName: newFileName, - mimeType: mimeType, - srcPath: newPath, + originalFileName: oldFileName, + targetFormat: targetFormat, + sourcePath: newPath, + keepOriginal: keepOriginal, ); + final safUri = published?.uri; if (safUri == null || safUri.isEmpty) { try { @@ -1062,7 +1060,7 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { bitDepth: convertedBitDepth, sampleRate: convertedSampleRate, keepOriginal: keepOriginal, - newSafFileName: newFileName, + newSafFileName: published!.fileName, ); await ref.read(downloadHistoryProvider.notifier).reloadFromStorage(); } else { @@ -1129,6 +1127,20 @@ extension _TrackMetadataConvertAndCueSplit on _TrackMetadataScreenState { SnackBar(content: Text(context.l10n.trackSaveFailed(e.toString()))), ); } + } finally { + for (final path in [ + coverPath, + safTempPath, + convertedSafTempPath, + ]) { + if (path == null || path.isEmpty) continue; + try { + final file = File(path); + if (await file.exists()) await file.delete(); + } catch (error) { + _log.w('Failed to clean conversion temp file $path: $error'); + } + } } } } diff --git a/lib/services/batch_track_actions.dart b/lib/services/batch_track_actions.dart index da4a1b64..831b162e 100644 --- a/lib/services/batch_track_actions.dart +++ b/lib/services/batch_track_actions.dart @@ -17,9 +17,22 @@ import 'package:spotiflac_android/utils/audio_conversion_utils.dart'; import 'package:spotiflac_android/utils/file_access.dart'; import 'package:spotiflac_android/utils/int_utils.dart'; import 'package:spotiflac_android/utils/lyrics_metadata_helper.dart'; +import 'package:spotiflac_android/utils/logger.dart'; import 'package:spotiflac_android/widgets/batch_convert_sheet.dart'; import 'package:spotiflac_android/widgets/batch_progress_dialog.dart'; +final _batchActionsLog = AppLogger('BatchActions'); + +class _BatchConversionFailure implements Exception { + final String stage; + final String message; + + const _BatchConversionFailure(this.stage, this.message); + + @override + String toString() => '$stage: $message'; +} + /// Shows the batch format-conversion sheet for [selectedItems] and runs the /// conversion when confirmed. Handles both history-backed and local-backed /// items, including SAF-hosted files. @@ -64,17 +77,23 @@ Future showBatchConvertSheet( var didStartConversion = false; + // The queue launches this action from a temporary OverlayEntry. Its + // onSheetOpen callback removes that entry, which deactivates [context]. Keep + // the root Navigator's context before doing so; it remains mounted for the + // sheet, the confirmation dialog, progress updates, and the final snackbar. + final modalContext = Navigator.of(context, rootNavigator: true).context; + // Resolve localized strings up front; the builder must not look up // Localizations via a possibly deactivated context. - final sheetTitle = context.l10n.selectionBatchConvertConfirmTitle; - final sheetConfirmLabel = context.l10n.selectionConvertCount( + final sheetTitle = modalContext.l10n.selectionBatchConvertConfirmTitle; + final sheetConfirmLabel = modalContext.l10n.selectionConvertCount( selectedItems.length, ); onSheetOpen?.call(); await showModalBottomSheet( - context: context, + context: modalContext, useRootNavigator: true, isScrollControlled: true, shape: const RoundedRectangleBorder( @@ -91,7 +110,7 @@ Future showBatchConvertSheet( didStartConversion = true; Navigator.pop(sheetContext); _performBatchConversion( - context, + modalContext, ref, selectedItems: selectedItems, targetFormat: format, @@ -195,6 +214,7 @@ Future _performBatchConversion( if (confirmed != true || !context.mounted) return; int successCount = 0; + final failures = []; final total = convertibleItems.length; final settings = ref.read(settingsProvider); final shouldEmbedLyrics = @@ -218,6 +238,10 @@ Future _performBatchConversion( BatchProgressDialog.update(current: i + 1, detail: item.trackName); + String failureStage = 'metadata'; + String? coverPath; + String? safTempPath; + String? convertedSafTempPath; try { final metadata = { 'TITLE': item.trackName, @@ -242,7 +266,6 @@ Future _performBatchConversion( 1000, ); - String? coverPath; try { final tempDir = await getTemporaryDirectory(); final coverOutput = @@ -258,14 +281,22 @@ Future _performBatchConversion( String workingPath = item.filePath; final isSaf = isContentUri(item.filePath); - String? safTempPath; if (isSaf) { - safTempPath = await PlatformBridge.copyContentUriToTemp(item.filePath); - if (safTempPath == null) continue; + failureStage = 'read SAF source'; + safTempPath = await ConversionLibraryService.copySafSourceToTemp( + item.filePath, + ); + if (safTempPath == null || safTempPath.isEmpty) { + throw const _BatchConversionFailure( + 'read SAF source', + 'temporary copy was not created', + ); + } workingPath = safTempPath; } + failureStage = 'FFmpeg conversion'; final newPath = await FFmpegService.convertAudioFormat( inputPath: workingPath, targetFormat: targetFormat.toLowerCase(), @@ -279,20 +310,13 @@ Future _performBatchConversion( losslessProcessing: losslessProcessing, ); - if (coverPath != null) { - try { - await File(coverPath).delete(); - } catch (_) {} - } - if (newPath == null) { - if (safTempPath != null) { - try { - await File(safTempPath).delete(); - } catch (_) {} - } - continue; + throw const _BatchConversionFailure( + 'FFmpeg conversion', + 'converter returned no output', + ); } + if (isSaf) convertedSafTempPath = newPath; final sourceBitDepth = item.historyItem?.bitDepth ?? item.localItem?.bitDepth; @@ -328,37 +352,27 @@ Future _performBatchConversion( ); if (isSaf && item.historyItem != null) { + failureStage = 'publish SAF output'; final hi = item.historyItem!; final treeUri = hi.downloadTreeUri; final relativeDir = hi.safRelativeDir ?? ''; if (treeUri != null && treeUri.isNotEmpty) { final oldFileName = hi.safFileName ?? ''; - final convTarget = convertTargetExtAndMime(targetFormat); - final mimeType = convTarget.mime; - final newFileName = convertedOutputFileName( - originalFileName: oldFileName, - targetFormat: targetFormat, - keepOriginal: keepOriginal, - ); - - final safUri = await PlatformBridge.createSafFileFromPath( + final published = await ConversionLibraryService.publishSafConversion( treeUri: treeUri, relativeDir: relativeDir, - fileName: newFileName, - mimeType: mimeType, - srcPath: newPath, + originalFileName: oldFileName, + targetFormat: targetFormat, + sourcePath: newPath, + keepOriginal: keepOriginal, ); + final safUri = published?.uri; if (safUri == null || safUri.isEmpty) { - try { - await File(newPath).delete(); - } catch (_) {} - if (safTempPath != null) { - try { - await File(safTempPath).delete(); - } catch (_) {} - } - continue; + throw const _BatchConversionFailure( + 'publish SAF output', + 'destination file was not created', + ); } if (!keepOriginal && !isSameContentUri(item.filePath, safUri)) { @@ -376,7 +390,12 @@ Future _performBatchConversion( bitDepth: convertedBitDepth, sampleRate: convertedSampleRate, keepOriginal: keepOriginal, - newSafFileName: newFileName, + newSafFileName: published!.fileName, + ); + } else { + throw const _BatchConversionFailure( + 'publish SAF output', + 'download folder permission is unavailable', ); } try { @@ -388,6 +407,7 @@ Future _performBatchConversion( } catch (_) {} } } else if (isSaf && item.localItem != null) { + failureStage = 'publish SAF output'; final uri = Uri.parse(item.filePath); final pathSegments = uri.pathSegments; @@ -426,32 +446,21 @@ Future _performBatchConversion( } if (treeUri != null && oldFileName.isNotEmpty) { - final convTarget = convertTargetExtAndMime(targetFormat); - final mimeType = convTarget.mime; - final newFileName = convertedOutputFileName( - originalFileName: oldFileName, - targetFormat: targetFormat, - keepOriginal: keepOriginal, - ); - - final safUri = await PlatformBridge.createSafFileFromPath( + final published = await ConversionLibraryService.publishSafConversion( treeUri: treeUri, relativeDir: relativeDir, - fileName: newFileName, - mimeType: mimeType, - srcPath: newPath, + originalFileName: oldFileName, + targetFormat: targetFormat, + sourcePath: newPath, + keepOriginal: keepOriginal, ); + final safUri = published?.uri; if (safUri == null || safUri.isEmpty) { - try { - await File(newPath).delete(); - } catch (_) {} - if (safTempPath != null) { - try { - await File(safTempPath).delete(); - } catch (_) {} - } - continue; + throw const _BatchConversionFailure( + 'publish SAF output', + 'destination file was not created', + ); } if (!keepOriginal && !isSameContentUri(item.filePath, safUri)) { @@ -468,6 +477,11 @@ Future _performBatchConversion( sampleRate: convertedSampleRate, keepOriginal: keepOriginal, ); + } else { + throw const _BatchConversionFailure( + 'publish SAF output', + 'source document location could not be resolved', + ); } try { @@ -502,11 +516,35 @@ Future _performBatchConversion( } successCount++; - } catch (_) {} + } catch (error, stackTrace) { + final detail = error is _BatchConversionFailure + ? error.toString() + : '$failureStage: $error'; + failures.add('${item.trackName} — $detail'); + _batchActionsLog.e( + 'Batch conversion failed for ${item.trackName} at $failureStage', + error, + stackTrace, + ); + } finally { + for (final path in [ + coverPath, + safTempPath, + convertedSafTempPath, + ]) { + if (path == null || path.isEmpty) continue; + try { + final file = File(path); + if (await file.exists()) await file.delete(); + } catch (error) { + _batchActionsLog.w('Failed to clean temporary file $path: $error'); + } + } + } } - ref.read(downloadHistoryProvider.notifier).reloadFromStorage(); - ref.read(localLibraryProvider.notifier).reloadFromStorage(); + await ref.read(downloadHistoryProvider.notifier).reloadFromStorage(); + await ref.read(localLibraryProvider.notifier).reloadFromStorage(); onExitSelectionMode(); @@ -518,11 +556,8 @@ Future _performBatchConversion( ScaffoldMessenger.of(context).showSnackBar( SnackBar( content: Text( - context.l10n.selectionBatchConvertSuccess( - successCount, - total, - targetFormat, - ), + '${context.l10n.selectionBatchConvertSuccess(successCount, total, targetFormat)}' + '${failures.isEmpty ? '' : ' • ${context.l10n.trackConvertFailed}: ${failures.length} (${failures.first})'}', ), ), ); diff --git a/lib/services/conversion_library_service.dart b/lib/services/conversion_library_service.dart index 901ad118..5442f9b8 100644 --- a/lib/services/conversion_library_service.dart +++ b/lib/services/conversion_library_service.dart @@ -1,10 +1,81 @@ import 'package:spotiflac_android/providers/download_history_provider.dart'; import 'package:spotiflac_android/services/history_database.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/utils/audio_conversion_utils.dart'; class ConversionLibraryService { const ConversionLibraryService._(); + static Future _retryTransientSaf( + Future Function() operation, + ) async { + Object? lastError; + for (var attempt = 0; attempt < 2; attempt++) { + try { + final result = await operation(); + if (result != null) return result; + } catch (error) { + lastError = error; + } + if (attempt == 0) { + await Future.delayed(const Duration(milliseconds: 250)); + } + } + if (lastError != null) throw lastError; + return null; + } + + static Future copySafSourceToTemp(String uri) { + return _retryTransientSaf(() => PlatformBridge.copyContentUriToTemp(uri)); + } + + static Future<({String uri, String fileName})?> publishSafConversion({ + required String treeUri, + required String relativeDir, + required String originalFileName, + required String targetFormat, + required String sourcePath, + required bool keepOriginal, + }) async { + final target = convertTargetExtAndMime(targetFormat); + final requestedName = convertedOutputFileName( + originalFileName: originalFileName, + targetFormat: targetFormat, + ); + + final replacesSameNamedDocument = + !keepOriginal && + requestedName.toLowerCase() == originalFileName.toLowerCase(); + if (!replacesSameNamedDocument) { + final result = await _retryTransientSaf( + () => PlatformBridge.createUniqueSafFileFromPath( + treeUri: treeUri, + relativeDir: relativeDir, + fileName: requestedName, + mimeType: target.mime, + srcPath: sourcePath, + ), + ); + if (result == null) return null; + final uri = (result['uri'] as String? ?? '').trim(); + final fileName = (result['file_name'] as String? ?? '').trim(); + if (uri.isEmpty || fileName.isEmpty) return null; + return (uri: uri, fileName: fileName); + } + + final uri = await _retryTransientSaf( + () => PlatformBridge.createSafFileFromPath( + treeUri: treeUri, + relativeDir: relativeDir, + fileName: requestedName, + mimeType: target.mime, + srcPath: sourcePath, + ), + ); + if (uri == null || uri.trim().isEmpty) return null; + return (uri: uri, fileName: requestedName); + } + static Future persistHistoryConversion({ required DownloadHistoryItem source, required String newFilePath, diff --git a/lib/services/ffmpeg_service.dart b/lib/services/ffmpeg_service.dart index 83029f4a..e6763c5f 100644 --- a/lib/services/ffmpeg_service.dart +++ b/lib/services/ffmpeg_service.dart @@ -122,6 +122,18 @@ class _ResolvedLosslessConversionQuality { }); } +class _ConversionOutputPlan { + final String workingPath; + final String finalPath; + + const _ConversionOutputPlan({ + required this.workingPath, + required this.finalPath, + }); + + bool get requiresPromotion => workingPath != finalPath; +} + class FFmpegService { static const int _commandLogPreviewLength = 300; static const Duration _liveTunnelStartupTimeout = Duration(seconds: 8); @@ -156,6 +168,120 @@ class FFmpegService { return outputPath; } + static bool _sameLocalPath(String first, String second) { + final firstPath = File(first).absolute.path; + final secondPath = File(second).absolute.path; + return Platform.isWindows + ? firstPath.toLowerCase() == secondPath.toLowerCase() + : firstPath == secondPath; + } + + static Future _uniqueConversionPath(String requestedPath) async { + if (!await File(requestedPath).exists()) return requestedPath; + final file = File(requestedPath); + final fileName = file.uri.pathSegments.last; + final dotIndex = fileName.lastIndexOf('.'); + final baseName = dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName; + final extension = dotIndex > 0 ? fileName.substring(dotIndex) : ''; + for (var index = 2; ; index++) { + final candidate = + '${file.parent.path}${Platform.pathSeparator}$baseName ($index)$extension'; + if (!await File(candidate).exists()) return candidate; + } + } + + static Future<_ConversionOutputPlan> _conversionOutputPlan( + String inputPath, + String extension, { + required bool deleteOriginal, + }) async { + final normalizedExt = extension.startsWith('.') ? extension : '.$extension'; + final inputFile = File(inputPath); + final fileName = inputFile.uri.pathSegments.last; + final dotIndex = fileName.lastIndexOf('.'); + final baseName = dotIndex > 0 ? fileName.substring(0, dotIndex) : fileName; + final requestedPath = + '${inputFile.parent.path}${Platform.pathSeparator}$baseName$normalizedExt'; + + if (_sameLocalPath(requestedPath, inputPath) && deleteOriginal) { + final token = DateTime.now().microsecondsSinceEpoch; + return _ConversionOutputPlan( + workingPath: + '${inputFile.parent.path}${Platform.pathSeparator}.$baseName.spotiflac-$token$normalizedExt', + finalPath: inputPath, + ); + } + + final finalPath = await _uniqueConversionPath(requestedPath); + return _ConversionOutputPlan(workingPath: finalPath, finalPath: finalPath); + } + + static Future _cleanupConversionOutput( + _ConversionOutputPlan plan, + ) async { + try { + final output = File(plan.workingPath); + if (await output.exists()) await output.delete(); + } catch (e) { + _log.w('Failed to clean conversion output: $e'); + } + } + + static Future _finalizeConversionOutput({ + required _ConversionOutputPlan plan, + required String inputPath, + required bool deleteOriginal, + }) async { + if (!await File(plan.workingPath).exists()) { + _log.e('Converted output is missing: ${plan.workingPath}'); + return null; + } + + if (plan.requiresPromotion) { + final source = File(inputPath); + final backupPath = + '$inputPath.spotiflac-backup-${DateTime.now().microsecondsSinceEpoch}'; + final backup = File(backupPath); + var sourceMovedToBackup = false; + try { + if (await source.exists()) { + await source.rename(backupPath); + sourceMovedToBackup = true; + } + await File(plan.workingPath).rename(plan.finalPath); + } catch (e) { + _log.e('Failed to replace original after conversion: $e'); + try { + if (sourceMovedToBackup && + !await source.exists() && + await backup.exists()) { + await backup.rename(inputPath); + } + } catch (restoreError) { + _log.e('Failed to restore original conversion backup: $restoreError'); + } + await _cleanupConversionOutput(plan); + return null; + } + try { + if (await backup.exists()) await backup.delete(); + } catch (e) { + _log.w('Converted file is ready but backup cleanup failed: $e'); + } + return plan.finalPath; + } + + if (deleteOriginal && !_sameLocalPath(inputPath, plan.finalPath)) { + try { + final source = File(inputPath); + if (await source.exists()) await source.delete(); + } catch (e) { + _log.w('Failed to delete original after conversion: $e'); + } + } + return plan.finalPath; + } + static String _previewCommandForLog(String command) { final redacted = command .replaceAll( @@ -519,7 +645,12 @@ class FFmpegService { 'aac' || 'm4a' => '.m4a', _ => '.mp3', }; - final outputPath = _buildOutputPath(inputPath, extension); + final outputPlan = await _conversionOutputPlan( + inputPath, + extension, + deleteOriginal: deleteOriginal, + ); + final outputPath = outputPlan.workingPath; String command; if (normalizedFormat == 'opus') { @@ -536,15 +667,15 @@ class FFmpegService { final result = await _execute(command); if (result.success) { - if (deleteOriginal) { - try { - await File(inputPath).delete(); - } catch (_) {} - } - return outputPath; + return _finalizeConversionOutput( + plan: outputPlan, + inputPath: inputPath, + deleteOriginal: deleteOriginal, + ); } _log.e('M4A to $normalizedFormat conversion failed: ${result.output}'); + await _cleanupConversionOutput(outputPlan); return null; } @@ -2088,8 +2219,7 @@ class FFmpegService { final promoted = await _promoteTempOutput( tempOutput, opusPath, - onMissing: () => - _log.e('Temp Opus output file not found: $tempOutput'), + onMissing: () => _log.e('Temp Opus output file not found: $tempOutput'), onError: (e) => _log.e('Failed to replace Opus file after metadata embed: $e'), ); @@ -2406,7 +2536,12 @@ class FFmpegService { 'aac' => '.m4a', _ => '.mp3', }; - final outputPath = _buildOutputPath(inputPath, extension); + final outputPlan = await _conversionOutputPlan( + inputPath, + extension, + deleteOriginal: deleteOriginal, + ); + final outputPath = outputPlan.workingPath; String command; if (format == 'opus') { @@ -2427,6 +2562,7 @@ class FFmpegService { if (!result.success) { _log.e('Audio conversion failed: ${result.output}'); + await _cleanupConversionOutput(outputPlan); return null; } @@ -2461,30 +2597,16 @@ class FFmpegService { _log.e( 'Metadata/Cover preservation failed, rolling back converted file', ); - try { - final out = File(outputPath); - if (await out.exists()) { - await out.delete(); - } - } catch (e) { - _log.w('Failed to cleanup failed converted file: $e'); - } + await _cleanupConversionOutput(outputPlan); return null; } } - if (deleteOriginal) { - try { - await File(inputPath).delete(); - _log.i( - 'Deleted original: ${inputPath.split(Platform.pathSeparator).last}', - ); - } catch (e) { - _log.w('Failed to delete original: $e'); - } - } - - return outputPath; + return _finalizeConversionOutput( + plan: outputPlan, + inputPath: inputPath, + deleteOriginal: deleteOriginal, + ); } /// Convert to ALAC (.m4a) or FLAC per [codec]. @@ -2502,7 +2624,12 @@ class FFmpegService { bool deleteOriginal = true, }) async { final isAlac = codec == 'alac'; - final outputPath = _buildOutputPath(inputPath, isAlac ? '.m4a' : '.flac'); + final outputPlan = await _conversionOutputPlan( + inputPath, + isAlac ? '.m4a' : '.flac', + deleteOriginal: deleteOriginal, + ); + final outputPath = outputPlan.workingPath; final arguments = ['-v', 'error', '-hide_banner', '-i', inputPath]; final hasCover = @@ -2566,21 +2693,15 @@ class FFmpegService { if (!result.success) { _log.e('$label conversion failed: ${result.output}'); + await _cleanupConversionOutput(outputPlan); return null; } - if (deleteOriginal) { - try { - await File(inputPath).delete(); - _log.i( - 'Deleted original: ${inputPath.split(Platform.pathSeparator).last}', - ); - } catch (e) { - _log.w('Failed to delete original: $e'); - } - } - - return outputPath; + return _finalizeConversionOutput( + plan: outputPlan, + inputPath: inputPath, + deleteOriginal: deleteOriginal, + ); } /// Convert to uncompressed PCM (WAV or AIFF), preserving bit depth when known. @@ -2599,7 +2720,12 @@ class FFmpegService { bool deleteOriginal = true, }) async { final isAiff = container == 'aiff'; - final outputPath = _buildOutputPath(inputPath, isAiff ? '.aiff' : '.wav'); + final outputPlan = await _conversionOutputPlan( + inputPath, + isAiff ? '.aiff' : '.wav', + deleteOriginal: deleteOriginal, + ); + final outputPath = outputPlan.workingPath; var depth = targetBitDepth ?? sourceBitDepth; if (depth == null || depth <= 0) { depth = await probeBitDepth(inputPath); @@ -2639,6 +2765,7 @@ class FFmpegService { final result = await _executeWithArguments(arguments); if (!result.success) { _log.e('${container.toUpperCase()} conversion failed: ${result.output}'); + await _cleanupConversionOutput(outputPlan); return null; } @@ -2654,18 +2781,11 @@ class FFmpegService { } } - if (deleteOriginal) { - try { - await File(inputPath).delete(); - _log.i( - 'Deleted original: ${inputPath.split(Platform.pathSeparator).last}', - ); - } catch (e) { - _log.w('Failed to delete original: $e'); - } - } - - return outputPath; + return _finalizeConversionOutput( + plan: outputPlan, + inputPath: inputPath, + deleteOriginal: deleteOriginal, + ); } /// Writes tags + cover into a WAV/AIFF file via the Go native ID3-chunk diff --git a/lib/utils/audio_conversion_utils.dart b/lib/utils/audio_conversion_utils.dart index d7de0dd8..5f3d934c 100644 --- a/lib/utils/audio_conversion_utils.dart +++ b/lib/utils/audio_conversion_utils.dart @@ -335,14 +335,12 @@ String normalizedConvertedAudioFormat(String targetFormat) { String convertedOutputFileName({ required String originalFileName, required String targetFormat, - required bool keepOriginal, }) { final dotIndex = originalFileName.lastIndexOf('.'); final baseName = dotIndex > 0 ? originalFileName.substring(0, dotIndex) : originalFileName; - final suffix = keepOriginal ? '_converted' : ''; - return '$baseName$suffix${convertTargetExtAndMime(targetFormat).ext}'; + return '$baseName${convertTargetExtAndMime(targetFormat).ext}'; } String convertedLibraryItemId(String sourceId, String filePath) { diff --git a/lib/widgets/batch_convert_sheet.dart b/lib/widgets/batch_convert_sheet.dart index 308fb6c5..2d4fa878 100644 --- a/lib/widgets/batch_convert_sheet.dart +++ b/lib/widgets/batch_convert_sheet.dart @@ -433,13 +433,16 @@ class _BatchConvertSheetState extends State { return Container( width: double.infinity, margin: const EdgeInsets.only(bottom: 12), - padding: const EdgeInsets.all(16), decoration: BoxDecoration( color: settingsGroupColor(context), borderRadius: BorderRadius.circular(20), border: Border.all(color: cs.outlineVariant.withValues(alpha: 0.5)), ), - child: child, + clipBehavior: Clip.antiAlias, + child: Material( + color: Colors.transparent, + child: Padding(padding: const EdgeInsets.all(16), child: child), + ), ); } diff --git a/test/audio_conversion_utils_test.dart b/test/audio_conversion_utils_test.dart index 59120ea7..a53480c0 100644 --- a/test/audio_conversion_utils_test.dart +++ b/test/audio_conversion_utils_test.dart @@ -47,20 +47,18 @@ void main() { }); group('kept conversion output identity', () { - test('uses a distinct filename when the original is retained', () { + test('keeps the original base name and only changes the extension', () { expect( convertedOutputFileName( originalFileName: 'Track.flac', targetFormat: 'FLAC', - keepOriginal: true, ), - 'Track_converted.flac', + 'Track.flac', ); expect( convertedOutputFileName( originalFileName: 'Track.flac', targetFormat: 'MP3', - keepOriginal: false, ), 'Track.mp3', ); diff --git a/test/batch_track_actions_test.dart b/test/batch_track_actions_test.dart new file mode 100644 index 00000000..564bd7ab --- /dev/null +++ b/test/batch_track_actions_test.dart @@ -0,0 +1,94 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_localizations/flutter_localizations.dart'; +import 'package:flutter_riverpod/flutter_riverpod.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/l10n/app_localizations.dart'; +import 'package:spotiflac_android/models/unified_library_item.dart'; +import 'package:spotiflac_android/services/batch_track_actions.dart'; + +void main() { + testWidgets( + 'batch conversion keeps a valid context after its launcher is removed', + (tester) async { + await tester.pumpWidget( + const ProviderScope( + child: MaterialApp( + localizationsDelegates: [ + AppLocalizations.delegate, + GlobalMaterialLocalizations.delegate, + GlobalWidgetsLocalizations.delegate, + GlobalCupertinoLocalizations.delegate, + ], + supportedLocales: AppLocalizations.supportedLocales, + home: _BatchConvertHarness(), + ), + ), + ); + + await tester.tap(find.byKey(const Key('open-batch-convert'))); + await tester.pumpAndSettle(); + + final convertButton = find.widgetWithText( + FilledButton, + 'Convert 1 track', + ); + await tester.scrollUntilVisible( + convertButton, + 300, + scrollable: find.byType(Scrollable).last, + ); + await tester.tap(convertButton); + await tester.pumpAndSettle(); + + expect(find.byType(AlertDialog), findsOneWidget); + expect( + find.textContaining('Original files will be deleted'), + findsOneWidget, + ); + expect(tester.takeException(), isNull); + }, + ); +} + +class _BatchConvertHarness extends ConsumerStatefulWidget { + const _BatchConvertHarness(); + + @override + ConsumerState<_BatchConvertHarness> createState() => + _BatchConvertHarnessState(); +} + +class _BatchConvertHarnessState extends ConsumerState<_BatchConvertHarness> { + bool _showLauncher = true; + + @override + Widget build(BuildContext context) { + return Scaffold( + body: _showLauncher + ? Builder( + builder: (launcherContext) => FilledButton( + key: const Key('open-batch-convert'), + onPressed: () => showBatchConvertSheet( + launcherContext, + ref, + [ + UnifiedLibraryItem( + id: 'track-1', + trackName: 'Track', + artistName: 'Artist', + albumName: 'Album', + filePath: '/music/track.flac', + addedAt: DateTime(2026), + source: LibraryItemSource.local, + ), + ], + onExitSelectionMode: () {}, + onSheetOpen: () => setState(() => _showLauncher = false), + ), + child: const Text('Open'), + ), + ) + : const SizedBox.shrink(), + ); + } +} diff --git a/test/conversion_library_service_test.dart b/test/conversion_library_service_test.dart new file mode 100644 index 00000000..1c96fdc9 --- /dev/null +++ b/test/conversion_library_service_test.dart @@ -0,0 +1,70 @@ +import 'dart:convert'; + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/services/conversion_library_service.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('com.zarz.spotiflac/backend'); + + tearDown(() async { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, null); + }); + + test( + 'SAF conversion removes _converted and retries a transient write', + () async { + var attempts = 0; + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'safCreateUniqueFromPath'); + expect((call.arguments as Map)['file_name'], 'Song.mp3'); + attempts++; + if (attempts == 1) { + throw PlatformException(code: 'temporary_io'); + } + return jsonEncode({ + 'uri': 'content://music/Song%20(2).mp3', + 'file_name': 'Song (2).mp3', + }); + }); + + final published = await ConversionLibraryService.publishSafConversion( + treeUri: 'content://music/tree/root', + relativeDir: 'Album', + originalFileName: 'Song.flac', + targetFormat: 'mp3', + sourcePath: '/tmp/Song.mp3', + keepOriginal: true, + ); + + expect(attempts, 2); + expect(published?.fileName, 'Song (2).mp3'); + }, + ); + + test( + 'same-format replacement reuses the original SAF document name', + () async { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + expect(call.method, 'safCreateFromPath'); + expect((call.arguments as Map)['file_name'], 'Song.flac'); + return 'content://music/Song.flac'; + }); + + final published = await ConversionLibraryService.publishSafConversion( + treeUri: 'content://music/tree/root', + relativeDir: 'Album', + originalFileName: 'Song.flac', + targetFormat: 'flac', + sourcePath: '/tmp/Song.flac', + keepOriginal: false, + ); + + expect(published?.fileName, 'Song.flac'); + }, + ); +}