mirror of https://github.com/Nezreka/SoulSync.git
Soulseek matched-download contexts populate `original_search_result` with `artist` (singular string) and no `artists` list — the full multi-artist array lives on `track_info` (the matched Spotify track object). `extract_source_metadata` only read `original_search.artists`, so the Soulseek path always fell through to the single-artist branch and TPE1 ended up with the primary artist only. Deezer-direct downloads were unaffected because their context populates `original_search.artists` as a proper list. Lifted artist resolution into a pure helper `core/metadata/artist_resolution.py:resolve_track_artists` that walks `original_search.artists` → `track_info.artists` → `artist_dict.name` fallback chain. Normalizes mixed list-item shapes (Spotify-style dicts, bare strings, anything else stringified) and drops empty entries. 13 new tests pin the resolution order, fallback chain, mixed-shape normalization, whitespace stripping, and empty/none handling. The existing `_artists_list` no-fall-through test in `test_multi_artist_tag_settings.py` was updated to reflect the new contract (always populated; multi-value write still gated on `len > 1`) plus a new regression test for the Soulseek shape. Composes with the existing Deezer per-track upgrade (still fires when single-artist + track_id available) and feat_in_title / artist_separator settings (still drive the joined ARTIST string downstream).pull/583/head
Broque Thomas
1 day ago
parent
b80567672e
commit
0769fcd5cc
@ -0,0 +1,74 @@
|
||||
"""Pure artist-list resolution for tag-write paths.
|
||||
|
||||
Single source of truth for "what is the canonical multi-value artists
|
||||
list for this track?" Different download paths populate `context` with
|
||||
different keys — Deezer-direct downloads stamp `original_search.artists`
|
||||
as a proper list, but Soulseek matched downloads only carry `artist`
|
||||
(singular string) in `original_search_result` while the full list lives
|
||||
on `track_info` (the full Spotify track object).
|
||||
|
||||
Resolution order:
|
||||
1. `context.original_search_result.artists` (preferred — already-
|
||||
curated by the source path that constructed the context)
|
||||
2. `context.track_info.artists` (Spotify/Deezer/Tidal full track
|
||||
object — always carries the artists array when matched)
|
||||
3. `[artist_dict.name]` as a single-element fallback when neither
|
||||
carries a list (primary-artist-only)
|
||||
|
||||
Each list item may be a dict with a `name` key (Spotify shape), a bare
|
||||
string, or any other object — the helper normalizes all three to
|
||||
strings and drops empty entries.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Any, Dict, List, Optional
|
||||
|
||||
|
||||
def _normalize_artists_iterable(items: Any) -> List[str]:
|
||||
if not isinstance(items, list):
|
||||
return []
|
||||
result: List[str] = []
|
||||
for item in items:
|
||||
if isinstance(item, dict):
|
||||
name = item.get("name")
|
||||
if isinstance(name, str) and name.strip():
|
||||
result.append(name.strip())
|
||||
elif isinstance(item, str):
|
||||
stripped = item.strip()
|
||||
if stripped:
|
||||
result.append(stripped)
|
||||
elif item is not None:
|
||||
text = str(item).strip()
|
||||
if text:
|
||||
result.append(text)
|
||||
return result
|
||||
|
||||
|
||||
def resolve_track_artists(
|
||||
original_search: Optional[Dict[str, Any]],
|
||||
track_info: Optional[Dict[str, Any]],
|
||||
artist_dict: Optional[Dict[str, Any]],
|
||||
) -> List[str]:
|
||||
"""Return the canonical multi-value artists list for tag-write.
|
||||
|
||||
Falls through preferred → track_info → primary-artist fallback. Each
|
||||
candidate is normalized to a list of stripped non-empty strings.
|
||||
Empty list returned only when every candidate is empty/invalid.
|
||||
"""
|
||||
if isinstance(original_search, dict):
|
||||
primary = _normalize_artists_iterable(original_search.get("artists"))
|
||||
if primary:
|
||||
return primary
|
||||
|
||||
if isinstance(track_info, dict):
|
||||
secondary = _normalize_artists_iterable(track_info.get("artists"))
|
||||
if secondary:
|
||||
return secondary
|
||||
|
||||
if isinstance(artist_dict, dict):
|
||||
name = artist_dict.get("name")
|
||||
if isinstance(name, str) and name.strip():
|
||||
return [name.strip()]
|
||||
|
||||
return []
|
||||
@ -0,0 +1,79 @@
|
||||
from core.metadata.artist_resolution import resolve_track_artists
|
||||
|
||||
|
||||
def test_prefers_original_search_artists_when_populated():
|
||||
original = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
|
||||
track = {"artists": [{"name": "Should Not Be Used"}]}
|
||||
artist = {"name": "Primary"}
|
||||
|
||||
assert resolve_track_artists(original, track, artist) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_falls_back_to_track_info_artists_when_original_lacks_list():
|
||||
# Soulseek context shape: original_search_result has 'artist' (string)
|
||||
# and no 'artists' (list). Full Spotify track object lives on track_info.
|
||||
original = {"artist": "Kendrick Lamar"}
|
||||
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
|
||||
artist = {"name": "Kendrick Lamar"}
|
||||
|
||||
assert resolve_track_artists(original, track, artist) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_falls_back_to_artist_dict_name_when_no_lists_available():
|
||||
original = {"artist": "Solo Artist"}
|
||||
track = {"name": "Track Title"}
|
||||
artist = {"name": "Solo Artist"}
|
||||
|
||||
assert resolve_track_artists(original, track, artist) == ["Solo Artist"]
|
||||
|
||||
|
||||
def test_returns_empty_list_when_everything_missing():
|
||||
assert resolve_track_artists(None, None, None) == []
|
||||
assert resolve_track_artists({}, {}, {}) == []
|
||||
|
||||
|
||||
def test_handles_bare_string_artist_items():
|
||||
original = {"artists": ["Kendrick Lamar", "Rihanna"]}
|
||||
assert resolve_track_artists(original, None, None) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_mixed_dict_and_string_items_normalized():
|
||||
original = {"artists": [{"name": "Kendrick Lamar"}, "Rihanna"]}
|
||||
assert resolve_track_artists(original, None, None) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_strips_whitespace_and_drops_empty_entries():
|
||||
original = {"artists": [{"name": " Kendrick "}, {"name": ""}, " ", "Rihanna"]}
|
||||
assert resolve_track_artists(original, None, None) == ["Kendrick", "Rihanna"]
|
||||
|
||||
|
||||
def test_dict_item_without_name_key_skipped():
|
||||
original = {"artists": [{"id": "abc"}, {"name": "Rihanna"}]}
|
||||
assert resolve_track_artists(original, None, None) == ["Rihanna"]
|
||||
|
||||
|
||||
def test_non_list_artists_value_falls_through():
|
||||
original = {"artists": "Kendrick Lamar"} # string, not list
|
||||
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
|
||||
assert resolve_track_artists(original, track, None) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_empty_original_artists_list_falls_through_to_track_info():
|
||||
original = {"artists": []}
|
||||
track = {"artists": [{"name": "Kendrick Lamar"}, {"name": "Rihanna"}]}
|
||||
assert resolve_track_artists(original, track, None) == ["Kendrick Lamar", "Rihanna"]
|
||||
|
||||
|
||||
def test_artist_dict_name_blank_returns_empty():
|
||||
assert resolve_track_artists({}, {}, {"name": " "}) == []
|
||||
assert resolve_track_artists({}, {}, {"name": ""}) == []
|
||||
|
||||
|
||||
def test_non_string_artist_items_coerced_to_string():
|
||||
original = {"artists": [123, {"name": "Real Artist"}]}
|
||||
assert resolve_track_artists(original, None, None) == ["123", "Real Artist"]
|
||||
|
||||
|
||||
def test_none_artist_items_dropped():
|
||||
original = {"artists": [None, {"name": "Real Artist"}, None]}
|
||||
assert resolve_track_artists(original, None, None) == ["Real Artist"]
|
||||
Loading…
Reference in new issue