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.
This commit is contained in:
parent
41f73f0c38
commit
119c6e3196
3 changed files with 146 additions and 3 deletions
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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:
|
||||
connected = bool(client and client.is_spotify_metadata_available())
|
||||
free_client = get_spotify_client(client_factory=spotify_client_factory)
|
||||
except Exception:
|
||||
free_client = None
|
||||
try:
|
||||
connected = bool(free_client and free_client.is_spotify_metadata_available())
|
||||
except Exception:
|
||||
connected = False
|
||||
elif source == "hydrabase":
|
||||
|
|
|
|||
126
tests/test_spotify_free_status.py
Normal file
126
tests/test_spotify_free_status.py
Normal file
|
|
@ -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
|
||||
Loading…
Reference in a new issue