Multi-artist tags: Search → Download Now finally knows its metadata source (Netti93)
Third round of the multi-artist report. The earlier fixes (Deezer contributors upgrade, _artists_list, feat_in_title/artist_separator) were all in place and correct — but gated on source == 'deezer', and on the real Search → Download Now path NOTHING carried the source: core/search/sources.py serialized tracks with no source field, search.js's enrichedTrack didn't add one, so get_import_source() resolved '' and the whole Deezer-specific block silently skipped. Files were tagged with only the primary artist until a Retag (which rebuilds context with the source set — exactly why retagging always fixed it). The earlier tests passed because they set context['source'] directly — the one field the real flow never had (same mock-drift as the #823 append tests). Reproduced with Netti93's exact track (deezer 3966840171) through the real extract_source_metadata: before — source '', artists ['August Burns Red']; after — source 'deezer', contributors fetched, artists ['August Burns Red', 'Polaris'], title 'Sonic Salvation (feat. Polaris)' per feat_in_title. Fix, three layers: - core/search/sources.py: serialized tracks/albums/artists carry "source" (the canonical name the orchestrator already passes; '' when unnamed). - core/imports/context.py get_import_source: also reads '_source' from the nested dicts (track_info/original_search/album/artist) — additionally fixes the discography/wishlist flows, which always passed '_source' that nothing read. - search.js: enrichedTrack + the album-download path carry source through to the download task. Tests: real-payload staging-shaped contexts (source in track_info, '_source' shape, and the pre-fix sourceless shape staying safe — mocked Deezer client), serializer source-field tests, resolver fallback tests; exact-shape serializer tests updated for the new key. 1977 import/metadata/search tests pass (the only 2 failures are the known soundcloud ones).
This commit is contained in:
parent
e32e2e5e14
commit
bcd69c8baa
6 changed files with 193 additions and 6 deletions
|
|
@ -129,30 +129,36 @@ def get_import_search_result(context: Optional[Dict[str, Any]]) -> Dict[str, Any
|
|||
|
||||
|
||||
def get_import_source(context: Optional[Dict[str, Any]]) -> str:
|
||||
# Several track payloads carry the metadata source under "_source" rather
|
||||
# than "source" (the discography/wishlist dicts, frontend search results).
|
||||
# Only the context-level "_source" was honored (normalize_import_context);
|
||||
# the nested dicts were checked for "source" alone, so a Deezer-sourced
|
||||
# Download Now resolved to '' and source-specific metadata logic (the
|
||||
# Deezer contributors upgrade for multi-artist tags) never ran (Netti93).
|
||||
if not isinstance(context, dict):
|
||||
return ""
|
||||
|
||||
source = context.get("source")
|
||||
source = context.get("source") or context.get("_source")
|
||||
if source:
|
||||
return str(source)
|
||||
|
||||
track_info = get_import_track_info(context)
|
||||
source = _first_value(track_info, "source", default="")
|
||||
source = _first_value(track_info, "source", "_source", default="")
|
||||
if source:
|
||||
return str(source)
|
||||
|
||||
original_search = get_import_original_search(context)
|
||||
source = _first_value(original_search, "source", default="")
|
||||
source = _first_value(original_search, "source", "_source", default="")
|
||||
if source:
|
||||
return str(source)
|
||||
|
||||
album = get_import_context_album(context)
|
||||
source = _first_value(album, "source", default="")
|
||||
source = _first_value(album, "source", "_source", default="")
|
||||
if source:
|
||||
return str(source)
|
||||
|
||||
artist = get_import_context_artist(context)
|
||||
source = _first_value(artist, "source", default="")
|
||||
source = _first_value(artist, "source", "_source", default="")
|
||||
return str(source) if source else ""
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -35,6 +35,7 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None
|
|||
artists.append({
|
||||
"id": artist.id,
|
||||
"name": artist.name,
|
||||
"source": source_name or "",
|
||||
"image_url": artist.image_url,
|
||||
"external_urls": artist.external_urls or {},
|
||||
})
|
||||
|
|
@ -52,6 +53,7 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None
|
|||
"id": album.id,
|
||||
"name": album.name,
|
||||
"artist": artist_name,
|
||||
"source": source_name or "",
|
||||
"image_url": album.image_url,
|
||||
"release_date": album.release_date,
|
||||
"total_tracks": album.total_tracks,
|
||||
|
|
@ -78,6 +80,21 @@ def search_kind(client, query: str, kind: str, source_name: Optional[str] = None
|
|||
"id": track.id,
|
||||
"name": track.name,
|
||||
"artist": artist_name,
|
||||
# The REAL artist list, not the joined display string above.
|
||||
# Spotify/Tidal/iTunes searches return collabs as a list;
|
||||
# collapsing them to one "A, B" string made the import
|
||||
# pipeline tag downloads with a single combined artist
|
||||
# (resolve_track_artists saw one value). The frontend keeps
|
||||
# using "artist" for display.
|
||||
"artists": list(track.artists or []),
|
||||
# Which metadata source this result came from. Travels with
|
||||
# the payload through Download Now -> download task ->
|
||||
# import context, where extract_source_metadata needs it to
|
||||
# run source-specific logic (the Deezer contributors
|
||||
# upgrade for multi-artist tags — Netti93's report: without
|
||||
# it get_import_source() resolved '' and collab tracks
|
||||
# were tagged with only the primary artist until a Retag).
|
||||
"source": source_name or "",
|
||||
"album": track.album,
|
||||
"duration_ms": track.duration_ms,
|
||||
"image_url": track.image_url,
|
||||
|
|
|
|||
|
|
@ -338,3 +338,20 @@ def test_detect_album_info_web_forces_album_when_track_and_artist_differ():
|
|||
assert album_info["album_name"] == "Album One"
|
||||
assert album_info["track_number"] == 4
|
||||
assert album_info["disc_number"] == 2
|
||||
|
||||
|
||||
def test_get_import_source_reads_underscore_source_from_nested_dicts():
|
||||
"""Netti93 multi-artist fix: many track payloads carry '_source' (the
|
||||
discography/wishlist dicts) or 'source' only inside track_info (search
|
||||
results). get_import_source must resolve all of them — previously only
|
||||
the context-level keys worked, so direct downloads resolved '' and
|
||||
source-specific metadata logic never ran."""
|
||||
from core.imports.context import get_import_source
|
||||
|
||||
assert get_import_source({"track_info": {"source": "deezer"}}) == "deezer"
|
||||
assert get_import_source({"track_info": {"_source": "deezer"}}) == "deezer"
|
||||
assert get_import_source({"original_search_result": {"_source": "itunes"}}) == "itunes"
|
||||
assert get_import_source({"_source": "tidal"}) == "tidal"
|
||||
# context-level 'source' still wins over nested
|
||||
assert get_import_source({"source": "spotify", "track_info": {"_source": "deezer"}}) == "spotify"
|
||||
assert get_import_source({}) == ""
|
||||
|
|
|
|||
|
|
@ -561,3 +561,93 @@ class TestDeezerDirectDownloadFlow:
|
|||
assert meta["_artists_list"] == ["FAYAN", "Dalton"]
|
||||
assert meta["artist"] == "FAYAN;Dalton"
|
||||
assert meta["title"] == "VERLIEBT IN MICH"
|
||||
|
||||
|
||||
class TestSourceResolutionFromRealDownloadPayload:
|
||||
"""Netti93 round 3: the prior tests set context['source'] — a field the
|
||||
REAL Search → Download Now flow never had. The serialized search results
|
||||
carried no source at all, so get_import_source() resolved '' and the
|
||||
Deezer contributors upgrade never ran on direct downloads (single-artist
|
||||
tags until a Retag). These pin the actual payload shapes end to end."""
|
||||
|
||||
def _staging_context(self, track_info):
|
||||
# Shape built by core/downloads/staging.py:457 for a stream download.
|
||||
return {
|
||||
"track_info": track_info,
|
||||
"spotify_artist": {"name": "August Burns Red", "id": None},
|
||||
"spotify_album": {"name": "Death Below"},
|
||||
"original_search_result": {
|
||||
"username": "tidal", "filename": "/x.flac",
|
||||
"title": "Sonic Salvation", "artist": "August Burns Red",
|
||||
"spotify_clean_title": "Sonic Salvation",
|
||||
"spotify_clean_album": "Death Below",
|
||||
"spotify_clean_artist": "August Burns Red",
|
||||
"track_number": 1, "disc_number": 1,
|
||||
},
|
||||
"is_album_download": False,
|
||||
"staging_source": True,
|
||||
}
|
||||
|
||||
def _fake_deezer(self):
|
||||
return SimpleNamespace(get_track_details=MagicMock(return_value={
|
||||
"id": "3966840171", "name": "Sonic Salvation",
|
||||
"artists": ["August Burns Red", "Polaris"],
|
||||
}))
|
||||
|
||||
def test_source_in_track_info_triggers_upgrade(self):
|
||||
"""The fixed flow: serialized search result carries source='deezer'
|
||||
inside track_info (no context-level source) → upgrade fires."""
|
||||
from core.metadata import source as src_module
|
||||
|
||||
track_info = {
|
||||
"id": "3966840171", "name": "Sonic Salvation",
|
||||
"source": "deezer",
|
||||
"artists": ["August Burns Red"],
|
||||
"album": {"name": "Death Below", "id": None},
|
||||
}
|
||||
fake_deezer = self._fake_deezer()
|
||||
with patch.object(src_module, "get_config_manager",
|
||||
return_value=_make_cfg({"metadata_enhancement.tags.write_multi_artist": True})), \
|
||||
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
|
||||
meta = src_module.extract_source_metadata(
|
||||
self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {})
|
||||
|
||||
assert meta["_artists_list"] == ["August Burns Red", "Polaris"]
|
||||
fake_deezer.get_track_details.assert_called_once_with("3966840171")
|
||||
|
||||
def test_underscore_source_shape_also_triggers_upgrade(self):
|
||||
"""Discography/wishlist payloads carry '_source' instead of 'source' —
|
||||
get_import_source must honor that shape too."""
|
||||
from core.metadata import source as src_module
|
||||
|
||||
track_info = {
|
||||
"id": "3966840171", "name": "Sonic Salvation",
|
||||
"_source": "deezer",
|
||||
"artists": ["August Burns Red"],
|
||||
}
|
||||
fake_deezer = self._fake_deezer()
|
||||
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
|
||||
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
|
||||
meta = src_module.extract_source_metadata(
|
||||
self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {})
|
||||
|
||||
assert meta["_artists_list"] == ["August Burns Red", "Polaris"]
|
||||
fake_deezer.get_track_details.assert_called_once_with("3966840171")
|
||||
|
||||
def test_sourceless_payload_still_no_upgrade(self):
|
||||
"""A payload with no source anywhere (pre-fix shape) must not crash —
|
||||
and must not call the Deezer API blindly."""
|
||||
from core.metadata import source as src_module
|
||||
|
||||
track_info = {
|
||||
"id": "3966840171", "name": "Sonic Salvation",
|
||||
"artists": ["August Burns Red"],
|
||||
}
|
||||
fake_deezer = self._fake_deezer()
|
||||
with patch.object(src_module, "get_config_manager", return_value=_make_cfg()), \
|
||||
patch("core.metadata.get_deezer_client", return_value=fake_deezer):
|
||||
meta = src_module.extract_source_metadata(
|
||||
self._staging_context(track_info), {"name": "August Burns Red", "id": ""}, {})
|
||||
|
||||
assert meta["_artists_list"] == ["August Burns Red"]
|
||||
fake_deezer.get_track_details.assert_not_called()
|
||||
|
|
|
|||
|
|
@ -84,6 +84,7 @@ def test_search_kind_artists_returns_normalized_dicts():
|
|||
assert result == [{
|
||||
'id': 'id1',
|
||||
'name': 'Pink Floyd',
|
||||
'source': 'spotify',
|
||||
'image_url': 'thumb.jpg',
|
||||
'external_urls': {'spotify': 'url'},
|
||||
}]
|
||||
|
|
@ -137,6 +138,8 @@ def test_search_kind_tracks_returns_full_shape():
|
|||
'id': 't1',
|
||||
'name': 'Money',
|
||||
'artist': 'Pink Floyd',
|
||||
'artists': ['Pink Floyd'],
|
||||
'source': '',
|
||||
'album': 'DSOTM',
|
||||
'duration_ms': 383000,
|
||||
'image_url': 'm.jpg',
|
||||
|
|
@ -201,3 +204,45 @@ def test_search_source_all_fail_returns_empty_lists():
|
|||
client = _Client(fail={'artists', 'albums', 'tracks'})
|
||||
result = sources.search_source('q', client, 'spotify')
|
||||
assert result == {'artists': [], 'albums': [], 'tracks': [], 'available': True}
|
||||
|
||||
|
||||
# ── source field on serialized results (Netti93 multi-artist fix) ───────────
|
||||
# Search results must carry which metadata source they came from: the payload
|
||||
# travels Download Now → download task → import context, where the Deezer
|
||||
# contributors upgrade (multi-artist tags) is gated on source == 'deezer'.
|
||||
# Without it get_import_source() resolved '' and collab tracks were tagged
|
||||
# with only the primary artist until a Retag.
|
||||
|
||||
def _full_client():
|
||||
return _Client(
|
||||
artists=[_Artist('id1', 'A')],
|
||||
albums=[_Album('a1', 'Album', artists=['A'])],
|
||||
tracks=[_Track('t1', 'T', artists=['A'], album='Album')],
|
||||
)
|
||||
|
||||
|
||||
def test_serialized_tracks_carry_source():
|
||||
out = sources.search_kind(_full_client(), "q", "tracks", source_name="deezer")
|
||||
assert out and all(t["source"] == "deezer" for t in out)
|
||||
|
||||
|
||||
def test_serialized_albums_and_artists_carry_source():
|
||||
albums = sources.search_kind(_full_client(), "q", "albums", source_name="deezer")
|
||||
artists = sources.search_kind(_full_client(), "q", "artists", source_name="deezer")
|
||||
assert albums and all(a["source"] == "deezer" for a in albums)
|
||||
assert artists and all(a["source"] == "deezer" for a in artists)
|
||||
|
||||
|
||||
def test_serialized_source_empty_when_unnamed():
|
||||
# hydrabase path calls without a source_name — emit '' not the class name.
|
||||
out = sources.search_kind(_full_client(), "q", "tracks")
|
||||
assert out and all(t["source"] == "" for t in out)
|
||||
|
||||
|
||||
def test_serialized_tracks_carry_real_artists_list():
|
||||
"""Collabs must survive as a LIST — the joined "A, B" display string made
|
||||
downloads tag a single combined artist (Marcus's report)."""
|
||||
client = _Client(tracks=[_Track('t1', 'Collab', artists=['Artist A', 'Artist B'], album='X')])
|
||||
out = sources.search_kind(client, 'q', 'tracks', source_name='spotify')
|
||||
assert out[0]['artists'] == ['Artist A', 'Artist B']
|
||||
assert out[0]['artist'] == 'Artist A, Artist B' # display string unchanged
|
||||
|
|
|
|||
|
|
@ -750,6 +750,9 @@ function initializeSearchModeToggle() {
|
|||
// Enrich each track with full album object (needed for wishlist functionality)
|
||||
const enrichedTracks = albumData.tracks.map(track => ({
|
||||
...track,
|
||||
// Carry the metadata source for source-specific import logic
|
||||
// (Deezer contributors upgrade for multi-artist tags).
|
||||
source: track.source || album.source || null,
|
||||
album: {
|
||||
name: albumData.name,
|
||||
id: albumData.id,
|
||||
|
|
@ -902,7 +905,16 @@ function initializeSearchModeToggle() {
|
|||
const enrichedTrack = {
|
||||
id: track.id,
|
||||
name: track.name,
|
||||
artists: [track.artist], // Convert string to array for modal compatibility
|
||||
// Carry the metadata source so the import pipeline can run
|
||||
// source-specific logic (Deezer contributors upgrade for
|
||||
// multi-artist tags) on the downloaded file.
|
||||
source: track.source || null,
|
||||
// Prefer the real artist list (Spotify/Tidal collabs) over the
|
||||
// joined "A, B" display string, so downloads get proper
|
||||
// multi-artist tags instead of one combined artist.
|
||||
artists: (Array.isArray(track.artists) && track.artists.length)
|
||||
? track.artists
|
||||
: [track.artist],
|
||||
album: {
|
||||
name: track.album,
|
||||
id: null,
|
||||
|
|
|
|||
Loading…
Reference in a new issue