Merge pull request #468 from Nezreka/fix/qobuz-connection-persistence
Sync Qobuz auth to enrichment worker after login
This commit is contained in:
BoulderBadgeDad
GitHub
commit
8bbac3ac8b
4 changed files with 348 additions and 0 deletions
|
|
@ -516,6 +516,45 @@ class QobuzClient:
|
|||
config_manager.set('qobuz.session', {})
|
||||
logger.info("Qobuz session cleared")
|
||||
|
||||
def reload_credentials(self) -> None:
|
||||
"""Pull session state from config without making a network probe.
|
||||
|
||||
SoulSync runs two ``QobuzClient`` instances side by side — one wired
|
||||
through ``soulseek_client.qobuz`` for the auth-flow endpoints, and a
|
||||
second owned by the enrichment worker for thread safety. When the user
|
||||
logs in via ``/api/qobuz/auth/login`` or ``/api/qobuz/auth/token`` only
|
||||
the auth-flow instance's in-memory state is updated; the worker's
|
||||
instance still believes itself unauthenticated, which is what made the
|
||||
dashboard "yellow" indicator and the connection-test step report
|
||||
``Qobuz not authenticated`` even after a successful Connect.
|
||||
|
||||
Call this on the worker's client immediately after a successful login
|
||||
(and on logout, to clear) to keep the two instances in lockstep.
|
||||
Unlike ``_restore_session`` this does not validate the token over the
|
||||
network — the caller has just authenticated, so the token is known
|
||||
good.
|
||||
"""
|
||||
saved = config_manager.get('qobuz.session', {}) or {}
|
||||
new_app_id = saved.get('app_id', '') or None
|
||||
new_app_secret = saved.get('app_secret', '') or None
|
||||
new_token = saved.get('user_auth_token', '') or None
|
||||
|
||||
self.app_id = new_app_id
|
||||
self.app_secret = new_app_secret
|
||||
self.user_auth_token = new_token
|
||||
|
||||
if new_app_id:
|
||||
self.session.headers['X-App-Id'] = new_app_id
|
||||
else:
|
||||
self.session.headers.pop('X-App-Id', None)
|
||||
|
||||
if new_token:
|
||||
self.session.headers['X-User-Auth-Token'] = new_token
|
||||
else:
|
||||
self.session.headers.pop('X-User-Auth-Token', None)
|
||||
self.user_info = None
|
||||
self._auth_error = None
|
||||
|
||||
def is_authenticated(self) -> bool:
|
||||
"""Check if we have a valid Qobuz session."""
|
||||
return bool(self.user_auth_token and self.app_id and self.app_secret)
|
||||
|
|
|
|||
289
tests/test_qobuz_credential_sync.py
Normal file
289
tests/test_qobuz_credential_sync.py
Normal file
|
|
@ -0,0 +1,289 @@
|
|||
"""Regression tests for QobuzClient.reload_credentials.
|
||||
|
||||
Discord-reported (Foxxify): logging in via the Qobuz Connect button on
|
||||
Settings showed "Connected: <username> (Active)" but underneath an error
|
||||
"Qobuz not authenticated...", and the dashboard indicator stayed
|
||||
yellow even after a successful login.
|
||||
|
||||
Root cause: SoulSync runs two QobuzClient instances side by side — one
|
||||
through ``soulseek_client.qobuz`` for the auth-flow endpoints, and a
|
||||
second owned by the enrichment worker thread for thread safety. Login
|
||||
only updated the first instance's in-memory state. The dashboard's
|
||||
"configured" check (and the connection-test step) read the worker
|
||||
instance, which still believed itself unauthenticated until the next
|
||||
process restart.
|
||||
|
||||
The fix adds ``QobuzClient.reload_credentials()`` — a public,
|
||||
network-free method that re-reads the saved session from config and
|
||||
updates the instance's in-memory state + session headers. Called from
|
||||
the auth login / token / logout endpoints to keep the worker instance
|
||||
in lockstep with the auth instance.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
from typing import Any, Dict
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Stubs for heavyweight dependencies that QobuzClient pulls in at import time
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def qobuz_client_module():
|
||||
"""Import core.qobuz_client with config_manager stubbed to a mutable
|
||||
in-memory dict so we can drive `qobuz.session` from the test.
|
||||
|
||||
Snapshots and restores sys.modules entries on teardown — without
|
||||
this, every downstream test that imports config.settings would
|
||||
receive our stub and the real config_manager.get would no longer
|
||||
reach the live config (which breaks tests like
|
||||
test_tidal_auth_instructions that monkeypatch config_manager.get
|
||||
directly).
|
||||
"""
|
||||
config_state: Dict[str, Any] = {}
|
||||
|
||||
class _StubConfigManager:
|
||||
def get(self, key, default=None):
|
||||
cur: Any = config_state
|
||||
for part in key.split('.'):
|
||||
if isinstance(cur, dict) and part in cur:
|
||||
cur = cur[part]
|
||||
else:
|
||||
return default
|
||||
return cur
|
||||
|
||||
def set(self, key, value):
|
||||
cur: Any = config_state
|
||||
parts = key.split('.')
|
||||
for part in parts[:-1]:
|
||||
cur = cur.setdefault(part, {})
|
||||
cur[parts[-1]] = value
|
||||
|
||||
# Snapshot what we are about to mutate so teardown can put it back.
|
||||
original_modules = {
|
||||
name: sys.modules.get(name)
|
||||
for name in ('config', 'config.settings', 'core.qobuz_client')
|
||||
}
|
||||
|
||||
if 'config' not in sys.modules:
|
||||
sys.modules['config'] = types.ModuleType('config')
|
||||
settings_mod = types.ModuleType('config.settings')
|
||||
settings_mod.config_manager = _StubConfigManager()
|
||||
sys.modules['config.settings'] = settings_mod
|
||||
|
||||
sys.modules.pop('core.qobuz_client', None)
|
||||
try:
|
||||
import core.qobuz_client as qobuz_client_module
|
||||
yield qobuz_client_module, config_state
|
||||
finally:
|
||||
# Restore each entry — set back to original, or pop if it didn't
|
||||
# exist beforehand. This protects every downstream test that
|
||||
# imports any of these modules.
|
||||
for name, original in original_modules.items():
|
||||
if original is None:
|
||||
sys.modules.pop(name, None)
|
||||
else:
|
||||
sys.modules[name] = original
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def fresh_client(qobuz_client_module):
|
||||
"""A QobuzClient instance with no saved session — the constructor's
|
||||
``_restore_session()`` call is a no-op when config is empty."""
|
||||
module, _config = qobuz_client_module
|
||||
return module.QobuzClient()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# reload_credentials — populates an empty client from saved config
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReloadCredentialsPopulatesFromConfig:
|
||||
def test_picks_up_token_app_id_and_app_secret(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
# Initial state — empty (constructor saw empty config).
|
||||
assert fresh_client.user_auth_token is None
|
||||
assert fresh_client.app_id is None
|
||||
assert fresh_client.app_secret is None
|
||||
|
||||
fresh_client.reload_credentials()
|
||||
|
||||
assert fresh_client.user_auth_token == 'TOKEN-1'
|
||||
assert fresh_client.app_id == 'APP-1'
|
||||
assert fresh_client.app_secret == 'SECRET-1'
|
||||
|
||||
def test_session_headers_get_set(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.session.headers.get('X-App-Id') == 'APP-1'
|
||||
assert fresh_client.session.headers.get('X-User-Auth-Token') == 'TOKEN-1'
|
||||
|
||||
def test_authenticated_after_reload(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.is_authenticated() is True
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# reload_credentials — clears state when config is wiped (logout path)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReloadCredentialsClearsOnEmptyConfig:
|
||||
def test_clears_token_app_id_and_app_secret(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
# Pre-populate
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.user_auth_token == 'TOKEN-1'
|
||||
|
||||
# Simulate logout — config wiped
|
||||
config['qobuz']['session'] = {}
|
||||
fresh_client.reload_credentials()
|
||||
|
||||
assert fresh_client.user_auth_token is None
|
||||
assert fresh_client.app_id is None
|
||||
assert fresh_client.app_secret is None
|
||||
|
||||
def test_session_headers_get_cleared(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
assert 'X-User-Auth-Token' in fresh_client.session.headers
|
||||
|
||||
config['qobuz']['session'] = {}
|
||||
fresh_client.reload_credentials()
|
||||
assert 'X-User-Auth-Token' not in fresh_client.session.headers
|
||||
assert 'X-App-Id' not in fresh_client.session.headers
|
||||
|
||||
def test_user_info_reset_when_token_cleared(self, qobuz_client_module, fresh_client):
|
||||
"""When the token gets cleared, stale user_info should not survive
|
||||
— otherwise downstream code could think a user is still attached
|
||||
to an unauthenticated instance."""
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
fresh_client.user_info = {'display_name': 'someone'}
|
||||
|
||||
config['qobuz']['session'] = {}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.user_info is None
|
||||
|
||||
def test_not_authenticated_after_clear(self, qobuz_client_module, fresh_client):
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {
|
||||
'session': {
|
||||
'app_id': 'APP-1',
|
||||
'app_secret': 'SECRET-1',
|
||||
'user_auth_token': 'TOKEN-1',
|
||||
}
|
||||
}
|
||||
fresh_client.reload_credentials()
|
||||
config['qobuz']['session'] = {}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.is_authenticated() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# reload_credentials — defensive against missing config keys
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestReloadCredentialsDefensive:
|
||||
def test_no_qobuz_key_at_all_clears_state(self, qobuz_client_module, fresh_client):
|
||||
_module, _config = qobuz_client_module
|
||||
# Set then tear down so there's nothing in config
|
||||
fresh_client.app_id = 'X'
|
||||
fresh_client.user_auth_token = 'Y'
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.app_id is None
|
||||
assert fresh_client.user_auth_token is None
|
||||
|
||||
def test_partial_session_doesnt_crash(self, qobuz_client_module, fresh_client):
|
||||
"""If only token is in config but app_id/secret missing, no crash —
|
||||
client just isn't authenticated."""
|
||||
_module, config = qobuz_client_module
|
||||
config['qobuz'] = {'session': {'user_auth_token': 'TOKEN-1'}}
|
||||
fresh_client.reload_credentials()
|
||||
assert fresh_client.user_auth_token == 'TOKEN-1'
|
||||
assert fresh_client.app_id is None
|
||||
assert fresh_client.is_authenticated() is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Sync scenario — the actual reported bug
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestTwoInstanceSync:
|
||||
"""Reproduce the Foxxify scenario: instance A logs in (writes to
|
||||
config), instance B reads stale state until reload_credentials is
|
||||
called."""
|
||||
|
||||
def test_second_instance_unaware_until_reload(self, qobuz_client_module):
|
||||
module, config = qobuz_client_module
|
||||
instance_a = module.QobuzClient()
|
||||
instance_b = module.QobuzClient()
|
||||
|
||||
# Instance A "logs in" — directly mutate the way login() would
|
||||
instance_a.app_id = 'APP-A'
|
||||
instance_a.app_secret = 'SECRET-A'
|
||||
instance_a.user_auth_token = 'TOKEN-A'
|
||||
instance_a._save_session()
|
||||
|
||||
# Instance B is still in the dark.
|
||||
assert instance_b.user_auth_token is None
|
||||
assert instance_b.is_authenticated() is False
|
||||
|
||||
# Sync.
|
||||
instance_b.reload_credentials()
|
||||
|
||||
assert instance_b.user_auth_token == 'TOKEN-A'
|
||||
assert instance_b.is_authenticated() is True
|
||||
assert instance_b.session.headers.get('X-User-Auth-Token') == 'TOKEN-A'
|
||||
|
|
@ -19971,6 +19971,22 @@ def tidal_download_auth_status():
|
|||
# QOBUZ AUTH ENDPOINTS
|
||||
# ===================================================================
|
||||
|
||||
def _sync_qobuz_credentials_to_worker():
|
||||
"""Push the just-saved Qobuz session into the enrichment worker's
|
||||
QobuzClient. Two separate client instances run side by side (one for
|
||||
the auth endpoints, one for the worker thread); without this sync the
|
||||
worker's instance never sees the new token until the next process
|
||||
restart, which is what made the dashboard indicator stay yellow and
|
||||
the connection test return ``Qobuz not authenticated`` after a
|
||||
successful Connect."""
|
||||
try:
|
||||
worker = qobuz_enrichment_worker if 'qobuz_enrichment_worker' in globals() else None
|
||||
if worker and getattr(worker, 'client', None):
|
||||
worker.client.reload_credentials()
|
||||
except Exception as e:
|
||||
logger.debug(f"Could not sync Qobuz credentials to enrichment worker: {e}")
|
||||
|
||||
|
||||
@app.route('/api/qobuz/auth/login', methods=['POST'])
|
||||
def qobuz_auth_login():
|
||||
"""Login to Qobuz with email/password."""
|
||||
|
|
@ -19986,6 +20002,7 @@ def qobuz_auth_login():
|
|||
result = qobuz.login(email, password)
|
||||
|
||||
if result['status'] == 'success':
|
||||
_sync_qobuz_credentials_to_worker()
|
||||
return jsonify({"success": True, **result})
|
||||
else:
|
||||
return jsonify({"success": False, "error": result.get('message', 'Login failed')}), 400
|
||||
|
|
@ -20008,6 +20025,7 @@ def qobuz_auth_token():
|
|||
result = qobuz.login_with_token(token)
|
||||
|
||||
if result['status'] == 'success':
|
||||
_sync_qobuz_credentials_to_worker()
|
||||
return jsonify({"success": True, **result})
|
||||
else:
|
||||
return jsonify({"success": False, "error": result.get('message', 'Token login failed')}), 400
|
||||
|
|
@ -20038,6 +20056,7 @@ def qobuz_auth_logout():
|
|||
"""Logout from Qobuz."""
|
||||
try:
|
||||
soulseek_client.qobuz.logout()
|
||||
_sync_qobuz_credentials_to_worker()
|
||||
return jsonify({"success": True})
|
||||
except Exception as e:
|
||||
return jsonify({"success": False, "error": str(e)}), 500
|
||||
|
|
|
|||
|
|
@ -3444,6 +3444,7 @@ const WHATS_NEW = {
|
|||
'2.4.2': [
|
||||
// --- post-2.4.1 dev work — entries hidden by _getLatestWhatsNewVersion until the build version bumps ---
|
||||
{ date: 'Unreleased — 2.4.2 dev cycle' },
|
||||
{ title: 'Fix Qobuz Connection Not Sticking After Login', desc: 'logging in via the qobuz connect button on settings showed "connected: <username> (active)" but underneath an error said "qobuz not authenticated...", and the dashboard indicator stayed yellow. cause: two separate qobuz client instances run side by side (one for the auth flow, one for the enrichment worker) and login only updated the first one. now the worker\'s client gets synced from config the moment login / token / logout completes, so the dashboard indicator goes green and connection-test stops yelling.', page: 'settings' },
|
||||
{ title: 'Sidebar Library Button Shows Artist Breadcrumb', desc: 'when you open an artist detail page (from library, search, or the global search popover), the sidebar Library button now lights up and rewrites its label to "Library / Artist Name" — long names truncate with an ellipsis and the full name shows on hover. revertes to plain "Library" when you leave. purely visual, no functionality change.', page: 'library' },
|
||||
{ title: 'Enrichment Bubble Routes Consolidated', desc: 'internal — every dashboard enrichment bubble (musicbrainz, spotify, itunes, deezer, discogs, audiodb, lastfm, genius, tidal, qobuz) used to hit its own per-service status / pause / resume route in web_server.py. unified them under a single registry-driven endpoint set: /api/enrichment/<service>/<action>. spotify\'s rate-limit guard, lastfm/genius yield-override behavior, and tidal/qobuz extra status fields are encoded as data on the registry. 27 new tests cover the registry behavior.' },
|
||||
{ title: 'Drop Old Per-Service Enrichment Routes', desc: 'internal — followup to the registry consolidation. now that the dashboard has cut over to /api/enrichment/<service>/<action>, deleted the 30 hand-rolled per-service routes from web_server.py (musicbrainz/audiodb/discogs/deezer/spotify/itunes/lastfm/genius/tidal/qobuz status+pause+resume). ~510 lines gone from the monolith, no behavior change.' },
|
||||
|
|
|
|||
Loading…
Reference in a new issue