diff --git a/core/metadata_service.py b/core/metadata_service.py index 15387201..e2636d0f 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -1783,8 +1783,14 @@ def get_artist_image_url( artist_id: str, source_override: Optional[str] = None, plugin: Optional[str] = None, + artist_name: Optional[str] = None, ) -> Optional[str]: - """Resolve an artist image URL using the configured source priority.""" + """Resolve an artist image URL using the configured source priority. + + `artist_name` is used when the source-of-record doesn't store artist + images (MusicBrainz) — the resolver then searches fallback sources + (iTunes/Deezer) by name for a matching artist and returns their image. + """ if not artist_id: return None @@ -1801,6 +1807,14 @@ def get_artist_image_url( return _get_artist_image_from_source('itunes', artist_id) return None + # MusicBrainz doesn't store artist images directly — use the artist + # name (passed by the frontend) to look up the image on a fallback + # source that does. Without a name we can't resolve. + if source_override == 'musicbrainz': + if not artist_name: + return None + return _lookup_artist_image_by_name(artist_name) + if source_override: return _get_artist_image_from_source(source_override, artist_id) @@ -1812,6 +1826,41 @@ def get_artist_image_url( return None +def _lookup_artist_image_by_name(name: str) -> Optional[str]: + """Look up an artist image by NAME (not MBID) across fallback sources. + Used when the primary source doesn't store artist images (MusicBrainz). + + Tries configured sources in priority order, searches each for the + artist name, and returns the first matching result's image URL. + """ + name = (name or '').strip() + if not name: + return None + + # Skip sources that don't do artist-name search or don't have images. + _SKIP_SOURCES = {'musicbrainz', 'soulseek', 'youtube_videos', 'hydrabase'} + for source in get_source_priority(get_primary_source()): + if source in _SKIP_SOURCES: + continue + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_artists'): + continue + try: + results = client.search_artists(name, limit=1) or [] + if results: + top = results[0] + img = getattr(top, 'image_url', None) or ( + top.get('image_url') if isinstance(top, dict) else None + ) + if img: + return img + except Exception as exc: + logger.debug("Artist image lookup by name failed on %s for %r: %s", + source, name, exc) + continue + return None + + def get_deezer_client(): """Get cached Deezer client. diff --git a/core/musicbrainz_search.py b/core/musicbrainz_search.py index f7b8370a..c396662b 100644 --- a/core/musicbrainz_search.py +++ b/core/musicbrainz_search.py @@ -92,6 +92,28 @@ def _extract_artist_credit(artist_credit) -> List[str]: return [n for n in names if n] +def _extract_title_hint(query: str, artist_name: str) -> Optional[str]: + """If `query` starts with `artist_name` followed by more words, return + the trailing portion. Used to pick out the album/track title the user + typed after the artist name (e.g. "The Beatles Abbey Road" → "Abbey + Road"). Returns None when the query is just the artist name. + + Case-insensitive prefix match on whitespace-normalized versions of + both strings, so "the beatles abbey road" → "abbey road" and + "The Beatles" → None. + """ + if not query or not artist_name: + return None + q_norm = ' '.join(query.split()).lower() + a_norm = ' '.join(artist_name.split()).lower() + if q_norm == a_norm: + return None + # Require a word boundary between the artist name and the trailing bit. + if q_norm.startswith(a_norm + ' '): + return query[len(artist_name):].strip() or None + return None + + def _map_release_type(primary_type: str, secondary_types: List[str] = None) -> str: """Map MusicBrainz release group type to standard album_type.""" pt = (primary_type or '').lower() @@ -303,6 +325,13 @@ class MusicBrainzSearchClient: if top: mbid = top.get('id', '') tname = top.get('name', '') or query + # If the query has words beyond the artist name (e.g. "The + # Beatles Abbey Road"), extract the leftover as a title hint. + # We'll use it below to narrow browse results to the specific + # album the user typed rather than dumping the full back + # catalogue. kettui flagged the regression — bare-name browse + # was burying a specific-album query inside a discography list. + title_hint = _extract_title_hint(query, tname) rgs = self._client.browse_artist_release_groups( mbid, # 'compilation' is a SECONDARY type, not a primary type @@ -332,6 +361,25 @@ class MusicBrainzSearchClient: # fall back to the unfiltered list — better than no results. rgs = studio or rgs + # Narrow to the title-hint if the user gave one ("The Beatles + # Abbey Road" → filter to RGs whose title contains "abbey + # road"). If no RG matches, fall back to text-search so the + # user finds the specific album instead of either seeing the + # full discography or getting zero results. (kettui flagged + # this regression — artist-first alone was burying specific- + # album queries inside the unfiltered discography list.) + if title_hint: + hint_lower = title_hint.lower() + matched = [rg for rg in rgs if hint_lower in (rg.get('title') or '').lower()] + if matched: + rgs = matched + else: + fallback = self._search_albums_text(title_hint, tname, limit) + if fallback: + return fallback + # Text-search also missed — fall through and show the + # full (unfiltered) discography rather than nothing. + # Sort by primary-type priority first (album > ep > single > # compilation), then chronologically ASC — the standard way # discographies are listed ("their debut was X, then Y, then Z"). @@ -440,7 +488,10 @@ class MusicBrainzSearchClient: release_date = '' image_url = None album_type = 'single' - total_tracks = 1 + # Initialized to 0 and summed from the release's media track-counts. + # Previously initialized to 1, which made every track-with-release + # report one more than the album actually has (kettui caught this). + total_tracks = 0 releases = r.get('releases', []) or [] if releases: @@ -460,6 +511,12 @@ class MusicBrainzSearchClient: rg_mbid = rg.get('id', '') or '' image_url = self._cached_art(album_id, rg_mbid) if album_id else None + # Tracks with no release info are standalone recordings — give them + # total_tracks=1 (the track itself). Keeps the old shape for that + # edge case but fixes the off-by-one for every normal case. + if not releases: + total_tracks = 1 + return Track( id=mbid, name=title, diff --git a/tests/test_musicbrainz_search.py b/tests/test_musicbrainz_search.py index 939e83fe..f9933a11 100644 --- a/tests/test_musicbrainz_search.py +++ b/tests/test_musicbrainz_search.py @@ -15,6 +15,7 @@ import pytest from core.musicbrainz_search import ( MusicBrainzSearchClient, _cover_art_url, + _extract_title_hint, ) @@ -520,6 +521,164 @@ def test_get_album_falls_back_to_release_lookup_on_rg_miss(): assert album['id'] == 'rel-abc' # Falls back to release MBID since rg lookup missed. +# --------------------------------------------------------------------------- +# Title-hint extraction — for "Artist Album Title" bare queries +# --------------------------------------------------------------------------- + +def test_extract_title_hint_basic(): + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_case_insensitive(): + assert _extract_title_hint('the beatles abbey road', 'The Beatles') == 'abbey road' + + +def test_extract_title_hint_preserves_original_casing(): + # Query slicing should return the original casing of the title portion. + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_whitespace_tolerant(): + assert _extract_title_hint('The Beatles Abbey Road', 'The Beatles') == 'Abbey Road' + + +def test_extract_title_hint_bare_artist_returns_none(): + assert _extract_title_hint('The Beatles', 'The Beatles') is None + + +def test_extract_title_hint_artist_not_prefix_returns_none(): + # Query where the artist name isn't the prefix — nothing to extract. + assert _extract_title_hint('Abbey Road', 'The Beatles') is None + + +def test_extract_title_hint_word_boundary_required(): + # "Metallicasomething" shouldn't split as artist=Metallica + hint=something + assert _extract_title_hint('Metallicasomething', 'Metallica') is None + + +def test_search_albums_filters_browse_results_by_title_hint(): + """Regression: 'The Beatles Abbey Road' used to return the whole + discography; should now narrow to Abbey Road specifically.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-abbey', 'title': 'Abbey Road', 'primary-type': 'Album', + 'first-release-date': '1969-09-26', 'secondary-types': []}, + {'id': 'rg-white', 'title': 'The Beatles', 'primary-type': 'Album', + 'first-release-date': '1968-11-22', 'secondary-types': []}, + {'id': 'rg-revolver', 'title': 'Revolver', 'primary-type': 'Album', + 'first-release-date': '1966-08-05', 'secondary-types': []}, + ] + + albums = client.search_albums('The Beatles Abbey Road', limit=10) + + # Filtered to only the album whose title matches the hint. + assert [a.name for a in albums] == ['Abbey Road'] + + +def test_search_albums_falls_back_to_text_when_hint_matches_nothing(): + """If the title hint doesn't match any browse result, fall back to + text-search rather than returning the full discography or nothing.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + # Browse returns albums that don't match the hint. + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-1', 'title': 'Some Other Album', 'primary-type': 'Album', + 'first-release-date': '1965-01-01', 'secondary-types': []}, + ] + # Text-search fallback (_search_albums_text → search_release) returns the album. + client._client.search_release.return_value = [ + {'id': 'rel-abbey', 'title': 'Abbey Road', 'score': 100, + 'release-group': {'id': 'rg-abbey', 'primary-type': 'Album'}, + 'artist-credit': [{'name': 'The Beatles'}]}, + ] + + albums = client.search_albums('The Beatles Totally Fake Album Name', limit=10) + + # Browse had no hit for the title hint, then fallback kicks in when + # the filter results are also empty (after studio-pref filter etc.). + # NOTE: in this test the hint filter returns empty, so we fall through + # to search_release. + client._client.search_release.assert_called_once() + assert any(a.name == 'Abbey Road' for a in albums) + + +def test_search_albums_bare_artist_no_hint_no_filter(): + """Bare artist name (no title hint) returns full discography — the + filter only kicks in when the user types extra words.""" + client = MusicBrainzSearchClient() + client._client = MagicMock() + client._client.search_artist.return_value = [_mk_artist('The Beatles', 'mb-1', score=100)] + client._client.browse_artist_release_groups.return_value = [ + {'id': 'rg-abbey', 'title': 'Abbey Road', 'primary-type': 'Album', + 'first-release-date': '1969-09-26', 'secondary-types': []}, + {'id': 'rg-revolver', 'title': 'Revolver', 'primary-type': 'Album', + 'first-release-date': '1966-08-05', 'secondary-types': []}, + ] + + albums = client.search_albums('the beatles', limit=10) + + # No filter — full discography. + titles = {a.name for a in albums} + assert 'Abbey Road' in titles + assert 'Revolver' in titles + + +def test_recording_to_track_total_tracks_matches_media_count(): + """Regression: total_tracks was initialized at 1 and summed with media + track-counts, producing an off-by-one. An 11-track album reported 12.""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Song', + 'length': 300000, + 'artist-credit': [{'name': 'Band'}], + 'releases': [{ + 'id': 'rel-1', + 'title': 'Album', + 'date': '2020-01-01', + 'release-group': {'id': 'rg-1', 'primary-type': 'Album', 'secondary-types': []}, + 'media': [{'track-count': 11}], + }], + } + track = client._recording_to_track(recording, 'Band') + assert track is not None + assert track.total_tracks == 11 + + +def test_recording_to_track_multi_disc_sums_media(): + """Two-disc album with 14 tracks total should report 14, not 15 (off by one) + or 3 (missing the sum).""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Song', + 'artist-credit': [{'name': 'Band'}], + 'releases': [{ + 'id': 'rel-1', 'title': 'Album', + 'release-group': {'id': 'rg-1', 'primary-type': 'Album'}, + 'media': [{'track-count': 7}, {'track-count': 7}], + }], + } + track = client._recording_to_track(recording, 'Band') + assert track.total_tracks == 14 + + +def test_recording_to_track_no_release_defaults_total_tracks_to_one(): + """A recording with no release info is a standalone track — report 1.""" + client = MusicBrainzSearchClient() + recording = { + 'id': 'rec-1', + 'title': 'Standalone', + 'artist-credit': [{'name': 'Band'}], + 'releases': [], + } + track = client._recording_to_track(recording, 'Band') + assert track.total_tracks == 1 + + def test_pick_representative_release_prefers_official_with_media(): """The release picker should skip stub releases (no media) and pick Official over Promotion status.""" diff --git a/web_server.py b/web_server.py index 1e9c6196..2f00b8d6 100644 --- a/web_server.py +++ b/web_server.py @@ -11735,10 +11735,15 @@ def get_artist_image(artist_id): source_override = request.args.get('source', '').strip().lower() or None plugin = request.args.get('plugin', '').strip().lower() or None + # `name` is optional but required for sources that don't store + # artist images directly (MusicBrainz) — the resolver falls back + # to searching iTunes/Deezer by name. + artist_name = request.args.get('name', '').strip() or None image_url = _get_artist_image_url( artist_id, source_override=source_override, plugin=plugin, + artist_name=artist_name, ) return jsonify({"success": True, "image_url": image_url}) except Exception as e: diff --git a/webui/static/downloads.js b/webui/static/downloads.js index 0f89ed7d..30a9d2bd 100644 --- a/webui/static/downloads.js +++ b/webui/static/downloads.js @@ -5339,7 +5339,7 @@ function _gsRenderFromState(state) { if (artists.length) { h += `