From 119c6e3196ada58cdf9df018ab44c744554caaa3 Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Sat, 13 Jun 2026 10:16:23 -0700 Subject: [PATCH] Spotify (no-auth): report connected + 'Spotify (no-auth)' test result instead of a Deezer fallback Status checks asked is_spotify_authenticated() (official OAuth only) instead of is_spotify_metadata_available(), so a Spotify-Free primary read as disconnected. get_primary_source_status had spotify_free awareness but it was dead code: get_client_for_source('spotify') returns None unless officially authed, so the free-availability probe never had a client. Fetch the client directly for that check; add the missing free branch to the dashboard test message. Seam + regression tests. --- core/connection_test.py | 11 ++- core/metadata/registry.py | 12 ++- tests/test_spotify_free_status.py | 126 ++++++++++++++++++++++++++++++ 3 files changed, 146 insertions(+), 3 deletions(-) create mode 100644 tests/test_spotify_free_status.py diff --git a/core/connection_test.py b/core/connection_test.py index 745caa2a..f65a6fe2 100644 --- a/core/connection_test.py +++ b/core/connection_test.py @@ -82,7 +82,16 @@ def run_service_test(service, test_config): if temp_client.is_spotify_authenticated(): return True, "Spotify connection successful!" else: - # Using fallback metadata source + # Spotify-Free (no-auth) metadata path: officially unauthenticated, + # but the no-creds source is selected and available. Report it as the + # working source rather than the generic Deezer/Discogs/iTunes fallback. + try: + spotify_free_available = temp_client.is_spotify_metadata_available() + except Exception: + spotify_free_available = False + if spotify_free_available: + return True, "Spotify (no-auth) connection successful!" + # Using a different fallback metadata source fb_src = _get_metadata_fallback_source() fallback_name = 'Deezer' if fb_src == 'deezer' else 'Discogs' if fb_src == 'discogs' else 'iTunes' if spotify_configured: diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 4e6f343d..0d6035e7 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -442,10 +442,18 @@ def get_primary_source_status( connected = bool(client and client.is_spotify_authenticated()) # No-auth composite (fallback_source='spotify' + metadata.spotify_free): # works without authentication, so treat the free path's availability - # as "connected" too. + # as "connected" too. get_client_for_source() returns None when not + # officially authed, so fetch the client directly to probe the free + # path — otherwise this check can never fire for a no-auth user. if not connected and _get_config_value("metadata.spotify_free", False): + free_client = client + if free_client is None: + try: + free_client = get_spotify_client(client_factory=spotify_client_factory) + except Exception: + free_client = None try: - connected = bool(client and client.is_spotify_metadata_available()) + connected = bool(free_client and free_client.is_spotify_metadata_available()) except Exception: connected = False elif source == "hydrabase": diff --git a/tests/test_spotify_free_status.py b/tests/test_spotify_free_status.py new file mode 100644 index 00000000..d32b894f --- /dev/null +++ b/tests/test_spotify_free_status.py @@ -0,0 +1,126 @@ +"""Spotify-Free (no-auth) must read as a WORKING primary metadata source. + +A user who picks 'Spotify Free' (fallback_source='spotify' + metadata.spotify_free) +is officially unauthenticated, so is_spotify_authenticated() is False. The sidebar/ +dashboard status dot keys on get_primary_source_status()['connected'], and the +dashboard test button on run_service_test('spotify', ...). Both used to report +disconnected / "Deezer connection successful! (Spotify configured but not +authenticated)" even though Spotify metadata was actually flowing. + +Root cause (pinned by test_*_unauthed_free_seen_via_direct_client): get_client_for_source +('spotify') returns None unless officially authed, so the free-availability check in +get_primary_source_status could never fire — the client it probed was always None. +""" + +from __future__ import annotations + +import core.metadata.registry as registry +import core.connection_test as connection_test + + +class _FreeClient: + """No-auth Spotify: not officially authed, but free metadata IS available.""" + + def is_spotify_authenticated(self): + return False + + def is_spotify_metadata_available(self): + return True + + +class _NoMetaClient: + def is_spotify_authenticated(self): + return False + + def is_spotify_metadata_available(self): + return False + + +def _patch_registry(monkeypatch, *, free_selected, client): + cfg = { + "metadata.fallback_source": "spotify", + "metadata.spotify_free": free_selected, + } + monkeypatch.setattr(registry, "_get_config_value", lambda k, d=None: cfg.get(k, d)) + # get_client_for_source('spotify') returns None when unauthed; the direct fetch + # is what the fix relies on, so route both through the fake. + monkeypatch.setattr(registry, "get_spotify_client", lambda client_factory=None: client) + + +def test_unauthed_free_seen_via_direct_client(monkeypatch): + """REGRESSION: free selected + available but not officially authed → connected. + Before the fix this was False because the probed client was None.""" + _patch_registry(monkeypatch, free_selected=True, client=_FreeClient()) + status = registry.get_primary_source_status() + assert status["connected"] is True + assert status["source"] == "spotify_free" + + +def test_free_not_selected_unauthed_is_disconnected(monkeypatch): + """Free NOT chosen → an unauthenticated Spotify primary is genuinely down.""" + _patch_registry(monkeypatch, free_selected=False, client=_FreeClient()) + status = registry.get_primary_source_status() + assert status["connected"] is False + assert status["source"] == "spotify" + + +def test_free_selected_but_unavailable_is_disconnected(monkeypatch): + """Free chosen but the package/path can't serve → not connected (no false green).""" + _patch_registry(monkeypatch, free_selected=True, client=_NoMetaClient()) + status = registry.get_primary_source_status() + assert status["connected"] is False + + +# --- dashboard test button (run_service_test) --------------------------------- + + +class _FakeConfigManager: + def __init__(self, store): + self._store = store + + def get(self, key, default=None): + return self._store.get(key, default) + + def set(self, key, value): + self._store[key] = value + + +def _run_spotify_test(monkeypatch, *, metadata_available, fallback="deezer"): + fake_client = _FreeClient() if metadata_available else _NoMetaClient() + + class _Client: + def __init__(self): + self._d = fake_client + + def is_authenticated(self): + return True # free user passes the top-level auth gate + + def is_spotify_authenticated(self): + return self._d.is_spotify_authenticated() + + def is_spotify_metadata_available(self): + return self._d.is_spotify_metadata_available() + + monkeypatch.setattr(connection_test, "SpotifyClient", _Client) + monkeypatch.setattr( + connection_test, + "config_manager", + _FakeConfigManager({"spotify": {"client_id": "x", "client_secret": "y"}}), + ) + monkeypatch.setattr(connection_test, "_get_metadata_fallback_source", lambda: fallback) + monkeypatch.setattr(connection_test, "docker_resolve_url", lambda v: v, raising=False) + return connection_test.run_service_test("spotify", {}) + + +def test_test_button_reports_spotify_free(monkeypatch): + ok, msg = _run_spotify_test(monkeypatch, metadata_available=True) + assert ok is True + assert "Spotify (no-auth)" in msg + + +def test_test_button_falls_back_when_free_unavailable(monkeypatch): + """No free path → keep the honest Deezer-fallback message.""" + ok, msg = _run_spotify_test(monkeypatch, metadata_available=False, fallback="deezer") + assert ok is True + assert "Deezer connection successful" in msg + assert "Spotify (no-auth)" not in msg