From 2b3022f6b0f3c95ebb2f4b17e0f216999619c228 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Sat, 2 May 2026 19:44:03 +0300 Subject: [PATCH] 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 --- core/imports/context.py | 24 ++++++++-- tests/imports/test_import_context.py | 29 ++++++++++++ tests/imports/test_import_side_effects.py | 56 +++++++++++++++++++++++ 3 files changed, 106 insertions(+), 3 deletions(-) diff --git a/core/imports/context.py b/core/imports/context.py index a3e4a4c3..c302891e 100644 --- a/core/imports/context.py +++ b/core/imports/context.py @@ -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=""), diff --git a/tests/imports/test_import_context.py b/tests/imports/test_import_context.py index 65ab1e12..e71584b7 100644 --- a/tests/imports/test_import_context.py +++ b/tests/imports/test_import_context.py @@ -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( { diff --git a/tests/imports/test_import_side_effects.py b/tests/imports/test_import_side_effects.py index 697d845b..2091f0b0 100644 --- a/tests/imports/test_import_side_effects.py +++ b/tests/imports/test_import_side_effects.py @@ -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