From fb13b875610b56e3ee8db441cac612fd761a9acf Mon Sep 17 00:00:00 2001 From: zarzet <42882290+zarzet@users.noreply.github.com> Date: Mon, 28 Sep 2026 11:45:10 +0700 Subject: [PATCH] fix(metadata): resolve album artist consistently across separate downloads --- lib/providers/download_queue_provider.dart | 1 + .../download_queue_provider_embedding.dart | 35 ++++++ ...download_queue_provider_native_worker.dart | 4 + .../download_queue_provider_single_item.dart | 6 + lib/services/download_album_metadata.dart | 42 +++++++ test/download_album_metadata_test.dart | 118 ++++++++++++++++++ 6 files changed, 206 insertions(+) create mode 100644 lib/services/download_album_metadata.dart create mode 100644 test/download_album_metadata_test.dart diff --git a/lib/providers/download_queue_provider.dart b/lib/providers/download_queue_provider.dart index 8c89eb0a..a2210548 100644 --- a/lib/providers/download_queue_provider.dart +++ b/lib/providers/download_queue_provider.dart @@ -23,6 +23,7 @@ import 'package:spotiflac_android/providers/download_queue_state.dart'; import 'package:spotiflac_android/services/app_state_database.dart'; import 'package:spotiflac_android/services/platform_bridge.dart'; import 'package:spotiflac_android/services/download_request_payload.dart'; +import 'package:spotiflac_android/services/download_album_metadata.dart'; import 'package:spotiflac_android/services/download_motion_artwork_source.dart'; import 'package:spotiflac_android/services/ffmpeg_service.dart'; import 'package:spotiflac_android/services/hires_check_service.dart'; diff --git a/lib/providers/download_queue_provider_embedding.dart b/lib/providers/download_queue_provider_embedding.dart index ffbdcca2..639313c3 100644 --- a/lib/providers/download_queue_provider_embedding.dart +++ b/lib/providers/download_queue_provider_embedding.dart @@ -22,6 +22,41 @@ class _DeezerExtendedMetadataFields { } extension _DownloadQueueEmbedding on DownloadQueueNotifier { + Future _resolveDownloadAlbumCredit( + Track track, + AppSettings settings, + ) async { + if (!settings.embedMetadata || + normalizeOptionalString(track.albumId) == null) { + return track; + } + final source = + normalizeOptionalString(track.source)?.toLowerCase() ?? + (track.id.contains(':') ? track.id.split(':').first.toLowerCase() : ''); + if (source.isEmpty) return track; + final providers = ref + .read(extensionProvider) + .extensions + .where( + (extension) => extension.enabled && extension.hasMetadataProvider, + ); + final provider = + providers + .where((extension) => extension.id.toLowerCase() == source) + .firstOrNull ?? + providers + .where( + (extension) => + extension.replacesBuiltInProviders.contains(source), + ) + .firstOrNull; + // Never send an album ID to an unrelated provider selected as a fallback. + // This resolves the source release itself, even when the provider disables + // supplemental cross-provider enrichment (for example genre/label lookup). + if (provider == null) return track; + return resolveDownloadAlbumArtist(track, provider.id); + } + String? _resolveAlbumArtistForMetadata(Track track, AppSettings settings) { var albumArtist = normalizeOptionalString(track.albumArtist); if (settings.filterContributingArtistsInAlbumArtist) { diff --git a/lib/providers/download_queue_provider_native_worker.dart b/lib/providers/download_queue_provider_native_worker.dart index 13b9ad9b..d64a064a 100644 --- a/lib/providers/download_queue_provider_native_worker.dart +++ b/lib/providers/download_queue_provider_native_worker.dart @@ -877,6 +877,10 @@ extension _DownloadQueueNativeWorker on DownloadQueueNotifier { return null; } + item = item.copyWith( + track: await _resolveDownloadAlbumCredit(item.track, settings), + ); + final isSafMode = _isSafMode(settings); final rawOutputDir = isSafMode ? _buildRelativeOutputDir( diff --git a/lib/providers/download_queue_provider_single_item.dart b/lib/providers/download_queue_provider_single_item.dart index a59d27c3..248c0ee4 100644 --- a/lib/providers/download_queue_provider_single_item.dart +++ b/lib/providers/download_queue_provider_single_item.dart @@ -187,6 +187,12 @@ class _DownloadRun { if (!await _enrichDeezerTrackIfNeeded()) return; + trackToDownload = await n._resolveDownloadAlbumCredit( + trackToDownload, + settings, + ); + if (await _shouldAbort('during album metadata lookup')) return; + resolvedAlbumArtist = n._resolveAlbumArtistForMetadata( trackToDownload, settings, diff --git a/lib/services/download_album_metadata.dart b/lib/services/download_album_metadata.dart new file mode 100644 index 00000000..d0497b01 --- /dev/null +++ b/lib/services/download_album_metadata.dart @@ -0,0 +1,42 @@ +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/services/platform_bridge.dart'; +import 'package:spotiflac_android/utils/logger.dart'; +import 'package:spotiflac_android/utils/string_utils.dart'; + +final _log = AppLogger('DownloadAlbumMetadata'); + +/// Resolve the release credit independently of which tracks were selected. +/// The bridge caches album responses and coalesces concurrent album lookups. +Future resolveDownloadAlbumArtist( + Track track, + String providerId, { + Future> Function(String, String, String)? loadMetadata, +}) async { + final albumId = normalizeOptionalString(track.albumId); + if (albumId == null || providerId.isEmpty) return track; + try { + final response = await (loadMetadata ?? PlatformBridge.getProviderMetadata)( + providerId, + 'album', + albumId, + ).timeout(const Duration(seconds: 8)); + final info = response['album_info'] ?? response['album'] ?? response; + if (info is! Map) return track; + final returnedId = normalizeOptionalString(info['id']?.toString()); + if (returnedId != null && returnedId != albumId) return track; + // Only album-level credits are authoritative. Track artists may include + // guests, and compilations/joint albums must retain their complete credit. + String? artist; + for (final key in ['album_artist', 'artists', 'artist']) { + final value = info[key]; + if (value is String) artist = normalizeOptionalString(value); + if (artist != null) break; + } + return artist == null || artist == track.albumArtist + ? track + : track.copyWith(albumArtist: artist); + } catch (error) { + _log.w('Album credit lookup failed; keeping track metadata: $error'); + return track; + } +} diff --git a/test/download_album_metadata_test.dart b/test/download_album_metadata_test.dart new file mode 100644 index 00000000..b6151b92 --- /dev/null +++ b/test/download_album_metadata_test.dart @@ -0,0 +1,118 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:spotiflac_android/models/track.dart'; +import 'package:spotiflac_android/providers/download_queue_provider.dart'; +import 'package:spotiflac_android/services/download_album_metadata.dart'; + +void main() { + const solo = Track( + id: 'solo', + name: 'Solo', + artistName: 'Artist', + albumName: 'Release', + albumArtist: 'Artist', + albumId: 'release-1', + source: 'example-provider', + duration: 180, + ); + final collaboration = solo.copyWith( + id: 'duet', + name: 'Duet', + artistName: 'Artist, Guest', + albumArtist: 'Artist, Guest', + ); + + test('separate downloads use the same album credit as a batch', () async { + Future> metadata( + String provider, + String kind, + String id, + ) async { + expect((provider, kind, id), ('example-provider', 'album', 'release-1')); + return { + 'album_info': {'id': id, 'artists': 'Artist'}, + }; + } + + final batch = normalizeBatchAlbumArtists([solo, collaboration]); + final later = await resolveDownloadAlbumArtist( + collaboration, + 'example-provider', + loadMetadata: metadata, + ); + expect(later.albumArtist, batch.first.albumArtist); + expect(later.artistName, 'Artist, Guest'); + expect(later.toJson(), { + ...collaboration.toJson(), + 'albumArtist': 'Artist', + }); + final embedded = buildTrackForMetadataEmbedding(later, { + 'album_artist': 'Artist, Guest', + }, later.albumArtist); + expect(embedded.albumArtist, 'Artist'); + }); + + for (final credit in ['Artist A & Artist B', 'Various Artists']) { + test('retains the complete album credit: $credit', () async { + final track = await resolveDownloadAlbumArtist( + collaboration, + 'example-provider', + loadMetadata: (_, _, _) async => {'artists': credit}, + ); + expect(track.albumArtist, credit); + expect(track.artistName, collaboration.artistName); + }); + } + + for (final response in >[ + { + 'album_info': {'id': 'different-release', 'artists': 'Other artist'}, + }, + { + 'album_info': {'artists': ' '}, + 'track_list': [ + {'artists': 'Guest'}, + ], + }, + { + 'album_info': { + 'artists': ['Artist'], + }, + }, + ]) { + test( + 'keeps source tags for missing or mismatched album data: $response', + () async { + final track = await resolveDownloadAlbumArtist( + collaboration, + 'example-provider', + loadMetadata: (_, _, _) async => response, + ); + expect(track, same(collaboration)); + }, + ); + } + + test('a failed lookup does not prevent downloading', () async { + final track = await resolveDownloadAlbumArtist( + collaboration, + 'example-provider', + loadMetadata: (_, _, _) async => throw StateError('Provider unavailable'), + ); + expect(track, same(collaboration)); + }); + + test('does not guess a release by its title', () async { + final missingId = solo.copyWith(albumId: ''); + var lookups = 0; + final track = await resolveDownloadAlbumArtist( + missingId, + 'example-provider', + loadMetadata: (_, _, _) async { + lookups++; + return {'artists': 'Different artist'}; + }, + ); + expect(track, same(missingId)); + expect(lookups, 0); + }); +}