mirror of
https://github.com/zarzet/SpotiFLAC-Mobile.git
synced 2026-10-20 22:41:38 +02:00
fix(lyrics): preserve batch track identity and report results promptly
This commit is contained in:
1 parent
eb6218b1ab
commit
ae679214a9
5 files changed
+125
-9
No files matched your search
@@ -107,6 +107,7 @@ Map<String, dynamic> buildBatchReEnrichRequest({
|
||||
required List<String> updateFields,
|
||||
bool previewOnly = false,
|
||||
Map<String, dynamic>? resolvedMetadata,
|
||||
String? sourceTrackId,
|
||||
}) {
|
||||
final request = <String, dynamic>{
|
||||
'file_path': item.filePath,
|
||||
@@ -115,7 +116,7 @@ Map<String, dynamic> buildBatchReEnrichRequest({
|
||||
'embed_lyrics': settings.embedLyrics,
|
||||
'lyrics_mode': settings.lyricsMode,
|
||||
'artist_tag_mode': settings.artistTagMode,
|
||||
'spotify_id': '',
|
||||
'spotify_id': sourceTrackId ?? '',
|
||||
'track_name': item.trackName,
|
||||
'artist_name': item.artistName,
|
||||
'album_name': item.albumName,
|
||||
@@ -131,7 +132,12 @@ Map<String, dynamic> buildBatchReEnrichRequest({
|
||||
'label': item.label ?? '',
|
||||
'copyright': item.copyright ?? '',
|
||||
'duration_ms': (item.duration ?? 0) * 1000,
|
||||
'search_online': resolvedMetadata == null,
|
||||
// Lyrics-only previews do not need a second metadata-provider search.
|
||||
// Preserve the file's title/artist and fetch lyrics during apply.
|
||||
'search_online':
|
||||
resolvedMetadata == null &&
|
||||
!(updateFields.length == 1 &&
|
||||
updateFields.single == ReEnrichFields.lyrics),
|
||||
'replace_release_metadata': false,
|
||||
'update_fields': updateFields,
|
||||
if (previewOnly) 'preview_only': true,
|
||||
@@ -139,6 +145,10 @@ Map<String, dynamic> buildBatchReEnrichRequest({
|
||||
if (resolvedMetadata != null) {
|
||||
request.addAll(resolvedMetadata);
|
||||
}
|
||||
if ((request['spotify_id']?.toString().trim().isEmpty ?? true) &&
|
||||
sourceTrackId != null) {
|
||||
request['spotify_id'] = sourceTrackId;
|
||||
}
|
||||
return request;
|
||||
}
|
||||
|
||||
|
||||
@@ -19,11 +19,14 @@ import 'package:spotiflac_android/utils/ffmpeg_reenrich.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/utils/string_utils.dart';
|
||||
import 'package:spotiflac_android/widgets/batch_progress_dialog.dart';
|
||||
import 'package:spotiflac_android/widgets/re_enrich_field_dialog.dart';
|
||||
import 'package:spotiflac_android/widgets/re_enrich_review_sheet.dart';
|
||||
|
||||
final _reEnrichLog = AppLogger('BatchReEnrich');
|
||||
|
||||
Future<void> queueLocalTracksAsFlac(
|
||||
BuildContext context,
|
||||
WidgetRef ref,
|
||||
@@ -176,6 +179,11 @@ Future<void> reEnrichLibraryTracks(
|
||||
onSelectionHide: onSelectionHide,
|
||||
onSelectionRestore: onSelectionRestore,
|
||||
onComplete: onComplete,
|
||||
sourceTrackIds: {
|
||||
for (final track in downloads)
|
||||
if (track.spotifyId?.trim().isNotEmpty == true)
|
||||
track.id: track.spotifyId!,
|
||||
},
|
||||
refreshLibrary: () async {
|
||||
// Downloaded tracks read history, not the local-scan index. Read back
|
||||
// saved tags so failed or unselected fields cannot change history.
|
||||
@@ -245,6 +253,7 @@ Future<void> reEnrichLocalTracks(
|
||||
required VoidCallback onSelectionRestore,
|
||||
required VoidCallback onComplete,
|
||||
Future<void> Function()? refreshLibrary,
|
||||
Map<String, String> sourceTrackIds = const {},
|
||||
}) async {
|
||||
if (selected.isEmpty) return;
|
||||
// Capture a stable route context before a caller removes its overlay.
|
||||
@@ -263,6 +272,7 @@ Future<void> reEnrichLocalTracks(
|
||||
}
|
||||
|
||||
final runner = BatchReEnrichRunner(
|
||||
sourceTrackIds: sourceTrackIds,
|
||||
beginPhase: () async {
|
||||
final settings = ref.read(settingsProvider);
|
||||
await ref
|
||||
@@ -346,6 +356,12 @@ Future<void> reEnrichLocalTracks(
|
||||
if (!context.mounted || !isActive()) return;
|
||||
if (!cancelled) BatchProgressDialog.dismiss(context);
|
||||
|
||||
// Report file-write outcomes immediately. A scan of a large Library can
|
||||
// take much longer than the batch and must not hide its completed result.
|
||||
ScaffoldMessenger.of(context).clearSnackBars();
|
||||
final summary = lyricsSummary.message(context.l10n, successCount, total);
|
||||
final resultClosed = showReEnrichResultDialog(context, message: summary);
|
||||
|
||||
if (refreshLibrary != null) {
|
||||
await refreshLibrary();
|
||||
} else {
|
||||
@@ -354,16 +370,14 @@ Future<void> reEnrichLocalTracks(
|
||||
|
||||
if (!context.mounted || !isActive()) return;
|
||||
onComplete();
|
||||
|
||||
ScaffoldMessenger.of(context).clearSnackBars();
|
||||
final summary = lyricsSummary.message(context.l10n, successCount, total);
|
||||
await showReEnrichResultDialog(context, message: summary);
|
||||
await resultClosed;
|
||||
}
|
||||
|
||||
/// Runs a batch against one settings snapshot per phase. Dependencies are
|
||||
/// injectable so cancellation, dispatch, and partial failure can be verified
|
||||
/// without writing media files or opening dialogs.
|
||||
class BatchReEnrichRunner {
|
||||
final Map<String, String> sourceTrackIds;
|
||||
final Future<AppSettings> Function() beginPhase;
|
||||
final Future<Map<String, dynamic>> Function(Map<String, dynamic>) reEnrich;
|
||||
final Future<void> Function({
|
||||
@@ -380,6 +394,7 @@ class BatchReEnrichRunner {
|
||||
|
||||
BatchReEnrichRunner({
|
||||
required this.beginPhase,
|
||||
this.sourceTrackIds = const {},
|
||||
this.reEnrich = PlatformBridge.reEnrichFile,
|
||||
this.writeSidecar = writeReEnrichSidecarLrc,
|
||||
this.applyFfmpeg = applyFfmpegReEnrichResult,
|
||||
@@ -412,10 +427,16 @@ class BatchReEnrichRunner {
|
||||
settings: settings,
|
||||
updateFields: fields,
|
||||
previewOnly: true,
|
||||
sourceTrackId: sourceTrackIds[item.id],
|
||||
),
|
||||
);
|
||||
final rawMetadata = result['enriched_metadata'];
|
||||
if (result['method'] != 'preview' || rawMetadata is! Map) continue;
|
||||
if (result['method'] != 'preview' || rawMetadata is! Map) {
|
||||
_reEnrichLog.w(
|
||||
'Metadata lookup failed for ${item.id}: ${result['error'] ?? result['method']}',
|
||||
);
|
||||
continue;
|
||||
}
|
||||
final metadata = rawMetadata.map(
|
||||
(key, value) => MapEntry(key.toString(), value),
|
||||
);
|
||||
@@ -429,7 +450,9 @@ class BatchReEnrichRunner {
|
||||
changes: changes,
|
||||
),
|
||||
);
|
||||
} catch (_) {
|
||||
} catch (error, stack) {
|
||||
_reEnrichLog.w('Metadata lookup failed for ${item.id}: $error');
|
||||
_reEnrichLog.d('$stack');
|
||||
// A failed lookup must not prevent review of other tracks.
|
||||
}
|
||||
}
|
||||
@@ -457,6 +480,7 @@ class BatchReEnrichRunner {
|
||||
settings: settings,
|
||||
updateFields: preview.updateFields,
|
||||
resolvedMetadata: preview.enrichedMetadata,
|
||||
sourceTrackId: sourceTrackIds[preview.item.id],
|
||||
),
|
||||
);
|
||||
switch (result['method']) {
|
||||
@@ -475,9 +499,17 @@ class BatchReEnrichRunner {
|
||||
)) {
|
||||
successes++;
|
||||
onResult?.call(result);
|
||||
} else {
|
||||
_reEnrichLog.w('Metadata write failed for ${preview.item.id}');
|
||||
}
|
||||
default:
|
||||
_reEnrichLog.w(
|
||||
'Re-enrich failed for ${preview.item.id}: ${result['error'] ?? result['method']}',
|
||||
);
|
||||
}
|
||||
} catch (_) {
|
||||
} catch (error, stack) {
|
||||
_reEnrichLog.w('Re-enrich failed for ${preview.item.id}: $error');
|
||||
_reEnrichLog.d('$stack');
|
||||
// Keep successful files and continue with the rest of the selection.
|
||||
}
|
||||
}
|
||||
|
||||
@@ -112,6 +112,7 @@ void main() {
|
||||
item: _item(),
|
||||
settings: const AppSettings(embeddedCoverMaxDimension: 1000),
|
||||
updateFields: const ['isrc'],
|
||||
sourceTrackId: 'original-id',
|
||||
resolvedMetadata: const {
|
||||
'isrc': 'USRC17607839',
|
||||
'spotify_id': 'resolved-id',
|
||||
@@ -125,6 +126,19 @@ void main() {
|
||||
expect(request['cover_max_dimension'], 1000);
|
||||
});
|
||||
|
||||
test('empty preview identity cannot discard the original track ID', () {
|
||||
final request = buildBatchReEnrichRequest(
|
||||
item: _item(),
|
||||
settings: const AppSettings(),
|
||||
updateFields: const [ReEnrichFields.lyrics],
|
||||
sourceTrackId: 'original-id',
|
||||
resolvedMetadata: const {'spotify_id': ''},
|
||||
);
|
||||
|
||||
expect(request['spotify_id'], 'original-id');
|
||||
expect(request['search_online'], isFalse);
|
||||
});
|
||||
|
||||
test('review only includes values that would actually change', () {
|
||||
final changes = buildReEnrichMetadataChanges(
|
||||
_item(isrc: 'OLD'),
|
||||
|
||||
@@ -149,6 +149,7 @@ void main() {
|
||||
UnifiedLibraryItem.fromDownloadHistory(
|
||||
DownloadHistoryItem(
|
||||
id: 'download',
|
||||
spotifyId: 'provider-a:track-1',
|
||||
trackName: 'Downloaded',
|
||||
artistName: 'Artist',
|
||||
albumName: 'Album',
|
||||
@@ -186,6 +187,7 @@ void main() {
|
||||
await tester.tap(find.text('Review changes'));
|
||||
await tester.pumpAndSettle();
|
||||
expect(requests.map((request) => request['file_path']), paths);
|
||||
expect(requests.first['spotify_id'], 'provider-a:track-1');
|
||||
await tester.tap(find.text('Apply changes'));
|
||||
for (var attempt = 0; attempt < 50 && !completed; attempt++) {
|
||||
await tester.pump(const Duration(milliseconds: 100));
|
||||
@@ -198,6 +200,7 @@ void main() {
|
||||
...paths,
|
||||
...paths,
|
||||
]);
|
||||
expect(requests[paths.length]['spotify_id'], 'provider-a:track-1');
|
||||
expect(history._updates, {'download': 'USABC2600001'});
|
||||
expect(library._refreshes, includeLocal ? 1 : 0);
|
||||
expect(completed, isTrue);
|
||||
|
||||
@@ -48,6 +48,58 @@ class _BatchLibrary extends LocalLibraryNotifier {
|
||||
}
|
||||
|
||||
void main() {
|
||||
test(
|
||||
'lyrics-only batch retains provider identity without metadata lookups',
|
||||
() async {
|
||||
final requests = <Map<String, dynamic>>[];
|
||||
final runner = BatchReEnrichRunner(
|
||||
beginPhase: () async => const AppSettings(embedLyrics: true),
|
||||
sourceTrackIds: {'1': 'provider-a:track-1'},
|
||||
reEnrich: (request) async {
|
||||
requests.add(request);
|
||||
return request['preview_only'] == true
|
||||
? {
|
||||
'method': 'preview',
|
||||
'enriched_metadata': {
|
||||
'spotify_id': request['spotify_id'],
|
||||
'duration_ms': request['duration_ms'],
|
||||
},
|
||||
}
|
||||
: {'method': 'native', 'lyrics_status': 'updated'};
|
||||
},
|
||||
writeSidecar:
|
||||
({required audioFilePath, required reEnrichResult}) async {},
|
||||
);
|
||||
final previews = await runner.preview(
|
||||
[item('1')],
|
||||
const ReEnrichFieldSelection(
|
||||
mode: ReEnrichBatchMode.selectedFields,
|
||||
fields: [ReEnrichFields.lyrics],
|
||||
),
|
||||
shouldStop: running,
|
||||
onProgress: progress,
|
||||
);
|
||||
expect(previews.single.updateFields, ['lyrics']);
|
||||
expect(
|
||||
await runner.apply(previews, shouldStop: running, onProgress: progress),
|
||||
1,
|
||||
);
|
||||
expect(requests, hasLength(2));
|
||||
expect(
|
||||
requests.map((request) => request['spotify_id']),
|
||||
everyElement('provider-a:track-1'),
|
||||
);
|
||||
expect(
|
||||
requests.map((request) => request['search_online']),
|
||||
everyElement(false),
|
||||
);
|
||||
expect(
|
||||
requests.map((request) => request['track_name']),
|
||||
everyElement('Song 1'),
|
||||
);
|
||||
},
|
||||
);
|
||||
|
||||
test(
|
||||
'lookup/apply each capture settings once, dispatch native/ffmpeg, and retain partial success',
|
||||
() async {
|
||||
@@ -305,6 +357,11 @@ void main() {
|
||||
await tester.tap(find.text('Apply changes'));
|
||||
await tester.pumpAndSettle();
|
||||
expect(library._refreshStarted, isTrue);
|
||||
// The result is visible even while an entire-library scan is blocked.
|
||||
expect(
|
||||
find.text('Metadata re-enriched successfully (2/2)'),
|
||||
findsOneWidget,
|
||||
);
|
||||
expect(settings.phases.length, 2);
|
||||
active = !leaveDuringRefresh;
|
||||
library.refreshed.complete();
|
||||
|
||||
Reference in new issue
Block a user