Fix Spotify source ID fallback
- Prefer real Spotify IDs when importing Spotify contexts - Skip numeric fallback IDs so Deezer values do not leak into spotify_* columns - Add regressions for import context and SoulSync library writes - Keep the route test asserting the Spotify album link
This commit is contained in:
parent
2693640c62
commit
2b3022f6b0
3 changed files with 106 additions and 3 deletions
|
|
@ -32,6 +32,20 @@ def _first_id_value(*values: Any) -> str:
|
|||
return ""
|
||||
|
||||
|
||||
def _first_source_aware_id(source: str, *values: Any) -> str:
|
||||
source_name = (source or "").strip().lower()
|
||||
for value in values:
|
||||
if value in (None, ""):
|
||||
continue
|
||||
text = str(value).strip()
|
||||
if not text:
|
||||
continue
|
||||
if source_name.startswith("spotify") and text.isdigit():
|
||||
continue
|
||||
return text
|
||||
return ""
|
||||
|
||||
|
||||
def extract_artist_name(artist: Any) -> str:
|
||||
if isinstance(artist, dict):
|
||||
return str(artist.get("name", "") or "")
|
||||
|
|
@ -209,6 +223,7 @@ def get_import_has_full_metadata(context: Optional[Dict[str, Any]]) -> bool:
|
|||
|
||||
|
||||
def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]:
|
||||
source = get_import_source(context)
|
||||
track_info = get_import_track_info(context)
|
||||
original_search = get_import_original_search(context)
|
||||
search_result = get_import_search_result(context)
|
||||
|
|
@ -216,7 +231,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]:
|
|||
album = get_import_context_album(context)
|
||||
|
||||
return {
|
||||
"track_id": _first_id_value(
|
||||
"track_id": _first_source_aware_id(
|
||||
source,
|
||||
_first_value(track_info, "id", "track_id", "trackId", "source_track_id", default=""),
|
||||
_first_value(track_info, "spotify_track_id", "itunes_track_id", "deezer_id", "deezer_track_id", "discogs_id", "soul_id", default=""),
|
||||
_first_value(original_search, "id", "track_id", "source_track_id", default=""),
|
||||
|
|
@ -224,7 +240,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]:
|
|||
_first_value(search_result, "id", "track_id", "source_track_id", default=""),
|
||||
_first_value(search_result, "spotify_track_id", "itunes_track_id", "deezer_id", "deezer_track_id", "discogs_id", "soul_id", default=""),
|
||||
),
|
||||
"artist_id": _first_id_value(
|
||||
"artist_id": _first_source_aware_id(
|
||||
source,
|
||||
_first_value(artist, "id", "artist_id", "source_artist_id", default=""),
|
||||
_first_value(artist, "spotify_artist_id", "itunes_artist_id", "deezer_id", "deezer_artist_id", "discogs_id", "soul_id", default=""),
|
||||
_first_value(original_search, "artist_id", "source_artist_id", default=""),
|
||||
|
|
@ -232,7 +249,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]:
|
|||
_first_value(search_result, "artist_id", "source_artist_id", default=""),
|
||||
_first_value(search_result, "spotify_artist_id", "itunes_artist_id", "deezer_id", "deezer_artist_id", "discogs_id", "soul_id", default=""),
|
||||
),
|
||||
"album_id": _first_id_value(
|
||||
"album_id": _first_source_aware_id(
|
||||
source,
|
||||
_first_value(album, "id", "album_id", "collectionId", "source_album_id", default=""),
|
||||
_first_value(album, "spotify_album_id", "itunes_album_id", "deezer_id", "deezer_album_id", "discogs_id", "soul_id", "album_soul_id", "hydrabase_album_id", default=""),
|
||||
_first_value(original_search, "album_id", "source_album_id", default=""),
|
||||
|
|
|
|||
|
|
@ -206,6 +206,35 @@ def test_get_import_source_ids_prefers_nested_source_specific_ids():
|
|||
}
|
||||
|
||||
|
||||
def test_get_import_source_ids_prefers_spotify_ids_over_numeric_fallbacks():
|
||||
context = normalize_import_context(
|
||||
{
|
||||
"source": "spotify",
|
||||
"artist": {
|
||||
"id": "396753",
|
||||
"spotify_artist_id": "sp-artist-1",
|
||||
"deezer_id": "396753",
|
||||
},
|
||||
"album": {
|
||||
"id": "284076172",
|
||||
"spotify_album_id": "sp-album-1",
|
||||
"deezer_id": "284076172",
|
||||
},
|
||||
"track_info": {
|
||||
"id": "1607091752",
|
||||
"spotify_track_id": "sp-track-1",
|
||||
"deezer_id": "1607091752",
|
||||
},
|
||||
}
|
||||
)
|
||||
|
||||
assert get_import_source_ids(context) == {
|
||||
"track_id": "sp-track-1",
|
||||
"artist_id": "sp-artist-1",
|
||||
"album_id": "sp-album-1",
|
||||
}
|
||||
|
||||
|
||||
def test_build_import_album_info_uses_normalized_album_context():
|
||||
context = normalize_import_context(
|
||||
{
|
||||
|
|
|
|||
|
|
@ -142,3 +142,59 @@ def test_record_soulsync_library_entry_writes_artist_album_and_track(tmp_path, m
|
|||
assert track_row["track_artist"] == "Guest Artist"
|
||||
assert track_row["album_id"] == album_row["id"]
|
||||
assert track_row["file_path"] == str(final_path)
|
||||
|
||||
|
||||
def test_record_soulsync_library_entry_ignores_numeric_spotify_ids(tmp_path, monkeypatch):
|
||||
conn = _make_soulsync_db()
|
||||
fake_db = _FakeDB(conn)
|
||||
final_path = tmp_path / "track.flac"
|
||||
final_path.write_bytes(b"audio")
|
||||
|
||||
monkeypatch.setattr(side_effects, "get_database", lambda: fake_db)
|
||||
monkeypatch.setattr(
|
||||
side_effects,
|
||||
"_get_config_manager",
|
||||
lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"),
|
||||
)
|
||||
|
||||
import core.genre_filter as genre_filter
|
||||
|
||||
monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres)
|
||||
|
||||
context = {
|
||||
"source": "spotify",
|
||||
"artist": {"id": "396753", "name": "Artist One"},
|
||||
"album": {
|
||||
"id": "284076172",
|
||||
"name": "Album One",
|
||||
"release_date": "2024-02-03",
|
||||
"total_tracks": 12,
|
||||
},
|
||||
"track_info": {
|
||||
"id": "1607091752",
|
||||
"name": "Song One",
|
||||
"track_number": 7,
|
||||
"duration_ms": 210000,
|
||||
"artists": [{"name": "Guest Artist"}],
|
||||
"_source": "spotify",
|
||||
},
|
||||
"original_search_result": {
|
||||
"title": "Song One",
|
||||
"artists": [{"name": "Guest Artist"}],
|
||||
"_source": "spotify",
|
||||
},
|
||||
"_final_processed_path": str(final_path),
|
||||
}
|
||||
|
||||
artist_context = {"name": "Artist One", "genres": ["rock"]}
|
||||
album_info = {"is_album": True, "album_name": "Album One", "track_number": 7}
|
||||
|
||||
side_effects.record_soulsync_library_entry(context, artist_context, album_info)
|
||||
|
||||
artist_row = conn.execute("SELECT * FROM artists").fetchone()
|
||||
album_row = conn.execute("SELECT * FROM albums").fetchone()
|
||||
track_row = conn.execute("SELECT * FROM tracks").fetchone()
|
||||
|
||||
assert artist_row["spotify_artist_id"] is None
|
||||
assert album_row["spotify_album_id"] is None
|
||||
assert track_row["spotify_track_id"] is None
|
||||
|
|
|
|||
Loading…
Reference in a new issue