diff --git a/core/imports/routes.py b/core/imports/routes.py index c5f49943..39674bba 100644 --- a/core/imports/routes.py +++ b/core/imports/routes.py @@ -16,6 +16,7 @@ from core.imports.staging import ( AUDIO_EXTENSIONS, get_import_suggestions_cache, get_primary_source as _get_primary_source, + get_primary_source_label as _get_primary_source_label, get_staging_path as _get_staging_path, read_staging_file_metadata as _read_staging_file_metadata, refresh_import_suggestions_cache as _refresh_import_suggestions_cache, @@ -48,6 +49,7 @@ class ImportRouteRuntime: read_staging_file_metadata: Callable[[str, str], Dict[str, Any]] = _read_staging_file_metadata read_tags: Callable[[str], Any] = _default_read_tags get_primary_source: Callable[[], str] = _get_primary_source + get_primary_source_label: Callable[[], str] = _get_primary_source_label search_import_albums: Callable[..., list] = _search_import_albums search_import_tracks: Callable[..., list] = _search_import_tracks build_album_import_match_payload: Callable[..., Dict[str, Any]] = build_album_import_match_payload @@ -222,7 +224,7 @@ def staging_suggestions() -> tuple[Dict[str, Any], int]: "success": True, "suggestions": cache["suggestions"], "ready": cache["built"], - "primary_source": _get_primary_source(), + "primary_source": _get_primary_source_label(), }, 200 @@ -239,7 +241,10 @@ def search_albums(runtime: ImportRouteRuntime, query: str, limit: int = 12) -> t runtime.hydrabase_worker.enqueue(query, "albums") albums = runtime.search_import_albums(query, limit=limit) - return {"success": True, "albums": albums, "primary_source": primary_source}, 200 + # The label names the user's CONFIGURED source (Spotify Free reads as + # 'spotify', not the deezer fallback the functional source downgrades to). + return {"success": True, "albums": albums, + "primary_source": runtime.get_primary_source_label()}, 200 except Exception as exc: runtime.logger.error("Error searching albums for import: %s", exc) return {"success": False, "error": str(exc)}, 500 @@ -385,7 +390,8 @@ def search_tracks(runtime: ImportRouteRuntime, query: str, limit: int = 10) -> t runtime.hydrabase_worker.enqueue(query, "tracks") tracks = runtime.search_import_tracks(query, limit=limit) - return {"success": True, "tracks": tracks, "primary_source": primary_source}, 200 + return {"success": True, "tracks": tracks, + "primary_source": runtime.get_primary_source_label()}, 200 except Exception as exc: runtime.logger.error("Error searching tracks for import: %s", exc) return {"success": False, "error": str(exc)}, 500 diff --git a/core/imports/staging.py b/core/imports/staging.py index 76a42beb..ec124058 100644 --- a/core/imports/staging.py +++ b/core/imports/staging.py @@ -56,6 +56,12 @@ def get_primary_source() -> str: return _get_primary_source() +def get_primary_source_label() -> str: + from core.metadata_service import get_primary_source_label as _get_primary_source_label + + return _get_primary_source_label() + + def get_source_priority(preferred_source: str): from core.metadata_service import get_source_priority as _get_source_priority diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 0d6035e7..a6c6ea39 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -368,6 +368,24 @@ def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = return source +def get_primary_source_label() -> str: + """Configured primary source for UI that *names* "your primary source" (the + import-search fallback banner, etc.). + + Identical to ``get_primary_source()`` except it does NOT downgrade a no-auth + Spotify Free user to the working fallback: Spotify Free (fallback_source= + 'spotify' + metadata.spotify_free) is reported as 'spotify', because that IS + their configured source — even though free-text album search itself has no + free-path implementation and legitimately falls back to another provider. + ``get_primary_source()`` keeps the downgrade so client routing always yields a + usable client; labels want the user's actual intent, not the fallback.""" + _default = METADATA_SOURCE_PRIORITY[0] + source = _get_config_value("metadata.fallback_source", _default) or _default + if source == "spotify" and _get_config_value("metadata.spotify_free", False): + return "spotify" + return get_primary_source() + + def get_spotify_disconnect_source(configured_source: Optional[str] = None) -> str: """Return the active metadata source after Spotify is disconnected.""" _default = METADATA_SOURCE_PRIORITY[0] diff --git a/core/metadata_service.py b/core/metadata_service.py index a0bdadc1..7c390746 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -51,6 +51,7 @@ from core.metadata.registry import ( get_itunes_client, get_primary_client, get_primary_source, + get_primary_source_label, get_spotify_client_for_profile, get_registered_runtime_client, get_source_priority, @@ -117,6 +118,7 @@ __all__ = [ "get_musicmap_similar_artists", "get_primary_client", "get_primary_source", + "get_primary_source_label", "get_spotify_client_for_profile", "get_registered_runtime_client", "get_spotify_client", diff --git a/tests/imports/test_import_routes.py b/tests/imports/test_import_routes.py index 2e1b080d..34409e9c 100644 --- a/tests/imports/test_import_routes.py +++ b/tests/imports/test_import_routes.py @@ -207,7 +207,7 @@ def test_staging_suggestions_returns_cache_payload(monkeypatch): "get_import_suggestions_cache", lambda: {"suggestions": [{"album": "Album"}], "built": True}, ) - monkeypatch.setattr(import_routes, "_get_primary_source", lambda: "deezer") + monkeypatch.setattr(import_routes, "_get_primary_source_label", lambda: "deezer") payload, status = staging_suggestions() @@ -296,6 +296,7 @@ def test_search_albums_enqueues_hydrabase_and_caps_limit(): calls = [] runtime = ImportRouteRuntime( get_primary_source=lambda: "hydrabase", + get_primary_source_label=lambda: "hydrabase", hydrabase_worker=worker, dev_mode_enabled=True, search_import_albums=lambda query, limit: calls.append((query, limit)) or [{"id": "album-1"}], @@ -327,10 +328,15 @@ def test_search_albums_exposes_primary_source_when_chain_falls_back(): # serves results from a different source, the response must carry both # `primary_source` (what the user configured) and per-album `source` # (what actually served the result) so the UI can warn the user. + # + # The configured source for the BANNER is the label, NOT the functional + # source (issue #922): a Spotify Free user's functional source downgrades to + # the deezer fallback, but the banner must still name what they configured. runtime = ImportRouteRuntime( - get_primary_source=lambda: "musicbrainz", + get_primary_source=lambda: "deezer", # functional (downgraded fallback) + get_primary_source_label=lambda: "spotify", # configured intent (Spotify Free) search_import_albums=lambda query, limit: [ - {"id": "deezer-1", "name": "Album", "source": "deezer"}, + {"id": "discogs-1", "name": "Album", "source": "discogs"}, ], logger=_FakeLogger(), ) @@ -339,8 +345,8 @@ def test_search_albums_exposes_primary_source_when_chain_falls_back(): assert status == 200 assert payload["success"] is True - assert payload["primary_source"] == "musicbrainz" - assert payload["albums"][0]["source"] == "deezer" + assert payload["primary_source"] == "spotify" # label, not the deezer fallback + assert payload["albums"][0]["source"] == "discogs" def test_search_tracks_enqueues_hydrabase_and_caps_limit(): @@ -348,6 +354,7 @@ def test_search_tracks_enqueues_hydrabase_and_caps_limit(): calls = [] runtime = ImportRouteRuntime( get_primary_source=lambda: "hydrabase", + get_primary_source_label=lambda: "hydrabase", hydrabase_worker=worker, dev_mode_enabled=True, search_import_tracks=lambda query, limit: calls.append((query, limit)) or [{"id": "track-1"}], diff --git a/tests/test_import_primary_source_label.py b/tests/test_import_primary_source_label.py new file mode 100644 index 00000000..083b311a --- /dev/null +++ b/tests/test_import_primary_source_label.py @@ -0,0 +1,94 @@ +"""The import-search 'primary source' label must name the user's CONFIGURED source. + +Bug #922: a Spotify Free (no-auth) user saw "Showing Discogs results - not from your +primary source (Deezer)" on the manual album-import search. Root cause: get_primary_source() +deliberately downgrades an unauthenticated Spotify to the working fallback (deezer) so +client routing always yields a usable client — and the import payload reused that +FUNCTIONAL value for the LABEL. The free source has no album-name search (SpotifyFree +.search_albums() returns []), so falling back for results is correct; only the label was +wrong. get_primary_source_label() preserves the configured intent (Spotify Free reads as +'spotify') without touching client routing, and the import route returns the label. +""" + +from __future__ import annotations + +import core.metadata.registry as registry +from core.imports.routes import ImportRouteRuntime, search_albums + + +class _AuthedSpotify: + def is_spotify_authenticated(self): + return True + + +class _UnauthedSpotify: + """No-auth Spotify (free tier): officially unauthenticated.""" + + def is_spotify_authenticated(self): + return False + + +def _patch_cfg(monkeypatch, cfg, *, client=None): + monkeypatch.setattr(registry, "_get_config_value", lambda k, d=None: cfg.get(k, d)) + monkeypatch.setattr(registry, "get_spotify_client", lambda client_factory=None: client) + + +# --- get_primary_source_label seam ------------------------------------------- + +def test_label_spotify_free_reads_as_spotify(monkeypatch): + """THE FIX: no-auth Spotify Free is labelled 'spotify', not the deezer fallback.""" + _patch_cfg( + monkeypatch, + {"metadata.fallback_source": "spotify", "metadata.spotify_free": True}, + client=_UnauthedSpotify(), + ) + assert registry.get_primary_source_label() == "spotify" + + +def test_label_spotify_authed_reads_as_spotify(monkeypatch): + _patch_cfg( + monkeypatch, + {"metadata.fallback_source": "spotify", "metadata.spotify_free": False}, + client=_AuthedSpotify(), + ) + assert registry.get_primary_source_label() == "spotify" + + +def test_label_spotify_unauthed_no_free_downgrades(monkeypatch): + """Spotify configured but neither authed nor free → genuinely on the fallback, + so the label honestly reports the working default (not a misleading 'spotify').""" + _patch_cfg( + monkeypatch, + {"metadata.fallback_source": "spotify", "metadata.spotify_free": False}, + client=_UnauthedSpotify(), + ) + label = registry.get_primary_source_label() + assert label == registry.METADATA_SOURCE_PRIORITY[0] # deezer default + assert label != "spotify" + + +def test_label_non_spotify_source_unchanged(monkeypatch): + _patch_cfg( + monkeypatch, + {"metadata.fallback_source": "deezer", "metadata.spotify_free": False}, + ) + assert registry.get_primary_source_label() == "deezer" + + +# --- import route regression: label decoupled from functional source ---------- + +def test_search_albums_payload_uses_label_not_functional_source(): + """REGRESSION (#922): the payload's primary_source is the LABEL ('spotify'), + even though the functional source the search chain used downgraded to 'deezer'.""" + runtime = ImportRouteRuntime( + get_primary_source=lambda: "deezer", # functional (downgraded) + get_primary_source_label=lambda: "spotify", # configured intent + search_import_albums=lambda q, limit=12: [{"name": "X", "source": "discogs"}], + hydrabase_worker=None, + dev_mode_enabled=False, + ) + payload, status = search_albums(runtime, "some album") + assert status == 200 + assert payload["primary_source"] == "spotify" + # The functional source is still free to differ (the chain genuinely used a fallback). + assert runtime.get_primary_source() == "deezer"