From e2bd0e1871115f70d7c035ec691e453b38574ba7 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Sat, 2 May 2026 11:05:01 +0300 Subject: [PATCH] Split metadata source and Spotify status - Keep the primary metadata provider snapshot generic and move Spotify auth/rate-limit details into a separate status object. - Update the websocket fixture and dashboard/settings consumers to read the two buckets independently. --- core/metadata/registry.py | 39 +++++++++++ tests/conftest.py | 4 +- tests/test_websocket_infrastructure.py | 5 +- web_server.py | 95 +++++++++----------------- webui/static/core.js | 14 ++-- webui/static/settings.js | 12 ++-- webui/static/shared-helpers.js | 58 ++++++++-------- 7 files changed, 121 insertions(+), 106 deletions(-) diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 71206cfe..4a6de606 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -9,6 +9,7 @@ from __future__ import annotations import threading import hashlib +import time from typing import Any, Callable, Dict, Optional from utils.logging_config import get_logger @@ -341,6 +342,44 @@ def get_primary_client( ) +def get_primary_source_status( + *, + spotify_client_factory: Optional[MetadataClientFactory] = None, + itunes_client_factory: Optional[MetadataClientFactory] = None, + deezer_client_factory: Optional[MetadataClientFactory] = None, + discogs_client_factory: Optional[MetadataClientFactory] = None, +) -> Dict[str, Any]: + """Return a generic status snapshot for the active primary metadata source.""" + source = _get_config_value("metadata.fallback_source", "deezer") or "deezer" + started = time.time() + connected = False + + try: + client = get_client_for_source( + source, + spotify_client_factory=spotify_client_factory, + itunes_client_factory=itunes_client_factory, + deezer_client_factory=deezer_client_factory, + discogs_client_factory=discogs_client_factory, + ) + if source == "spotify": + connected = bool(client and client.is_spotify_authenticated()) + elif source == "hydrabase": + connected = bool(client and (client.is_connected() if hasattr(client, "is_connected") else client.is_authenticated())) + elif client is not None and hasattr(client, "is_authenticated"): + connected = bool(client.is_authenticated()) + else: + connected = client is not None + except Exception: + connected = False + + return { + "source": source, + "connected": connected, + "response_time": round((time.time() - started) * 1000, 1), + } + + def get_client_for_source( source: str, *, diff --git a/tests/conftest.py b/tests/conftest.py index 4adc2f96..2ee99421 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -17,7 +17,8 @@ from flask_socketio import SocketIO, join_room, leave_room # --------------------------------------------------------------------------- _DEFAULT_STATUS_CACHE = { - 'metadata_source': {'connected': True, 'authenticated': True, 'response_time': 12.5, 'source': 'spotify'}, + 'metadata_source': {'connected': True, 'response_time': 12.5, 'source': 'spotify'}, + 'spotify': {'connected': True, 'authenticated': True, 'rate_limited': False, 'rate_limit': None, 'post_ban_cooldown': None}, 'media_server': {'connected': True, 'response_time': 8.1, 'type': 'plex'}, 'soulseek': {'connected': True, 'response_time': 5.3, 'source': 'soulseek'}, } @@ -256,6 +257,7 @@ wishlist_stats_state = copy.deepcopy(_DEFAULT_WISHLIST_STATS) def _build_status_payload(): return { 'metadata_source': dict(_status_cache['metadata_source']), + 'spotify': dict(_status_cache['spotify']), 'media_server': dict(_status_cache['media_server']), 'soulseek': dict(_status_cache['soulseek']), 'active_media_server': _status_cache['media_server'].get('type', 'plex'), diff --git a/tests/test_websocket_infrastructure.py b/tests/test_websocket_infrastructure.py index dc3d81bf..4e17eead 100644 --- a/tests/test_websocket_infrastructure.py +++ b/tests/test_websocket_infrastructure.py @@ -53,10 +53,12 @@ class TestServiceStatus: assert len(status_events) >= 1 data = status_events[0]['args'][0] assert 'metadata_source' in data + assert 'spotify' in data assert 'media_server' in data assert 'soulseek' in data assert 'active_media_server' in data - assert 'authenticated' in data['metadata_source'] + assert 'source' in data['metadata_source'] + assert 'authenticated' in data['spotify'] def test_status_matches_http(self, test_app, shared_state): """Socket event data matches HTTP endpoint response exactly.""" @@ -74,6 +76,7 @@ class TestServiceStatus: ws_data = status_events[0]['args'][0] assert ws_data['metadata_source'] == http_data['metadata_source'] + assert ws_data['spotify'] == http_data['spotify'] assert ws_data['media_server'] == http_data['media_server'] assert ws_data['soulseek'] == http_data['soulseek'] assert ws_data['active_media_server'] == http_data['active_media_server'] diff --git a/web_server.py b/web_server.py index 72cfd393..c78d9be6 100644 --- a/web_server.py +++ b/web_server.py @@ -811,7 +811,7 @@ _idle_since = {} _IDLE_GRACE_SECONDS = 5 _status_cache = { - 'metadata_source': {'connected': False, 'authenticated': False, 'response_time': 0, 'source': 'itunes'}, + 'metadata_source': {'connected': False, 'response_time': 0, 'source': 'itunes'}, 'media_server': {'connected': False, 'response_time': 0, 'type': None}, 'soulseek': {'connected': False, 'response_time': 0}, } @@ -3446,41 +3446,9 @@ def get_status(): current_time = time.time() active_server = config_manager.get_active_media_server() - # Test Spotify - with caching to avoid excessive API calls + # Test primary metadata provider and Spotify separately if current_time - _status_cache_timestamps['metadata_source'] > STATUS_CACHE_TTL: - # Guard against spotify_client being None (partial init) - is_rate_limited = spotify_client.is_rate_limited() if spotify_client else False - rate_limit_info = spotify_client.get_rate_limit_info() if (spotify_client and is_rate_limited) else None - cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 - metadata_source_authenticated = spotify_client.is_spotify_authenticated() if spotify_client else False - - # Read configured source once — no auth validation here, we do that explicitly below - configured_source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer' - - if is_rate_limited or cooldown_remaining > 0: - # During rate limit or post-ban cooldown, skip the auth probe entirely. - # Probing Spotify here would reset the rate limit timer. - music_source = 'deezer' if configured_source == 'spotify' else configured_source - metadata_source_response_time = 0 - else: - metadata_source_start = time.time() - metadata_source_connected = spotify_client.is_spotify_authenticated() if spotify_client else False - metadata_source_response_time = (time.time() - metadata_source_start) * 1000 - # Use configured source; fall back to deezer only if Spotify isn't authenticated - if configured_source == 'spotify': - music_source = 'spotify' if metadata_source_connected else 'deezer' - else: - music_source = configured_source - - _status_cache['metadata_source'] = { - 'connected': metadata_source_authenticated, - 'authenticated': metadata_source_authenticated, - 'response_time': round(metadata_source_response_time, 1), - 'source': music_source, - 'rate_limited': is_rate_limited, - 'rate_limit': rate_limit_info, - 'post_ban_cooldown': cooldown_remaining if cooldown_remaining > 0 else None - } + _status_cache['metadata_source'] = metadata_registry.get_primary_source_status() _status_cache_timestamps['metadata_source'] = current_time # else: use cached value @@ -3557,6 +3525,7 @@ def get_status(): status_data = { 'metadata_source': _status_cache['metadata_source'], + 'spotify': _build_spotify_status_payload(), 'media_server': _status_cache['media_server'], 'soulseek': _status_cache['soulseek'], 'active_media_server': active_server, @@ -4804,10 +4773,7 @@ def test_connection_endpoint(): current_time = time.time() if service == 'spotify': spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False - _status_cache['metadata_source']['connected'] = True - _status_cache['metadata_source']['authenticated'] = spotify_session_active - _status_cache['metadata_source']['source'] = _get_metadata_fallback_source() - _status_cache_timestamps['metadata_source'] = current_time + _status_cache_timestamps['metadata_source'] = 0 logger.info("Updated Spotify status cache after successful test") elif service in ['plex', 'jellyfin', 'navidrome', 'soulsync']: _status_cache['media_server']['connected'] = True @@ -4972,10 +4938,7 @@ def test_dashboard_connection_endpoint(): current_time = time.time() if service == 'spotify': spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False - _status_cache['metadata_source']['connected'] = True - _status_cache['metadata_source']['authenticated'] = spotify_session_active - _status_cache['metadata_source']['source'] = _get_metadata_fallback_source() - _status_cache_timestamps['metadata_source'] = current_time + _status_cache_timestamps['metadata_source'] = 0 logger.info("Updated Spotify status cache after successful dashboard test") elif service in ['plex', 'jellyfin', 'navidrome', 'soulsync']: _status_cache['media_server']['connected'] = True @@ -5929,16 +5892,7 @@ def spotify_disconnect(): source_label = get_metadata_source_label(active_source) if configured_source == 'spotify': config_manager.set('metadata.fallback_source', active_source) - _status_cache['metadata_source'] = { - 'connected': False, - 'authenticated': False, - 'response_time': 0, - 'source': active_source, - 'rate_limited': False, - 'rate_limit': None, - 'post_ban_cooldown': None - } - _status_cache_timestamps['metadata_source'] = time.time() + _status_cache_timestamps['metadata_source'] = 0 add_activity_item("", "Spotify Disconnected", f"Using {source_label} for metadata", "Now") return jsonify({ 'success': True, @@ -34050,17 +34004,8 @@ def _build_status_payload(): download_mode = config_manager.get('download_source.mode', 'hybrid') soulseek_data = dict(_status_cache.get('soulseek', {})) soulseek_data['source'] = download_mode - - # Always include fresh rate limit info (it changes over time as ban expires) - # Call is_rate_limited() first to ensure ban-end timestamp is recorded for cooldown metadata_source_data = dict(_status_cache.get('metadata_source', {})) - is_rl = spotify_client.is_rate_limited() if spotify_client else False - rate_limit_info = spotify_client.get_rate_limit_info() if is_rl else None - cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 - metadata_source_data['rate_limited'] = is_rl - metadata_source_data['rate_limit'] = rate_limit_info - if cooldown_remaining > 0: - metadata_source_data['post_ban_cooldown'] = cooldown_remaining + spotify_data = _build_spotify_status_payload() # Count active downloads for nav badge active_dl_count = 0 @@ -34074,6 +34019,7 @@ def _build_status_payload(): return { 'metadata_source': metadata_source_data, + 'spotify': spotify_data, 'media_server': _status_cache.get('media_server', {}), 'soulseek': soulseek_data, 'active_media_server': config_manager.get_active_media_server(), @@ -34081,6 +34027,29 @@ def _build_status_payload(): 'active_downloads': active_dl_count, } +def _build_spotify_status_payload(): + """Build a Spotify-specific status snapshot for auth and rate-limit state.""" + status_data = {} + + try: + # Always include fresh rate limit info because it can change independently of cache TTL. + is_rate_limited = spotify_client.is_rate_limited() if spotify_client else False + rate_limit_info = spotify_client.get_rate_limit_info() if (spotify_client and is_rate_limited) else None + cooldown_remaining = spotify_client.get_post_ban_cooldown_remaining() if spotify_client else 0 + authenticated = spotify_client.is_spotify_authenticated() if spotify_client else False + + status_data.update({ + 'connected': authenticated, + 'authenticated': authenticated, + 'rate_limited': is_rate_limited, + 'rate_limit': rate_limit_info, + 'post_ban_cooldown': cooldown_remaining if cooldown_remaining > 0 else None, + }) + except Exception: + pass + + return status_data + def _build_watchlist_count_payload(profile_id=1): """Build the same payload used by GET /api/watchlist/count.""" try: diff --git a/webui/static/core.js b/webui/static/core.js index cf0f4809..27b90003 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -474,21 +474,21 @@ function handleServiceStatusUpdate(data) { _lastStatusPayload = data; if (typeof syncSpotifySettingsAuthState === 'function') { - syncSpotifySettingsAuthState(data?.metadata_source || null); + syncSpotifySettingsAuthState(data?.spotify || null); } if (typeof syncPrimaryMetadataSourceAvailability === 'function') { - syncPrimaryMetadataSourceAvailability(data?.metadata_source || null); + syncPrimaryMetadataSourceAvailability(data?.spotify || null); } if (typeof sanitizeMetadataSourceSelection === 'function') { sanitizeMetadataSourceSelection({ quiet: true }); } // Same logic as fetchAndUpdateServiceStatus response handler - updateServiceStatus('spotify', data.metadata_source); + updateServiceStatus('spotify', data.metadata_source, data.spotify); updateServiceStatus('media-server', data.media_server); updateServiceStatus('soulseek', data.soulseek); - updateSidebarServiceStatus('spotify', data.metadata_source); + updateSidebarServiceStatus('spotify', data.metadata_source, data.spotify); updateSidebarServiceStatus('media-server', data.media_server); updateSidebarServiceStatus('soulseek', data.soulseek); @@ -514,10 +514,10 @@ function handleServiceStatusUpdate(data) { if (data.enrichment) renderEnrichmentCards(data.enrichment); // Spotify rate limit / cooldown / recovery - if (data.metadata_source?.rate_limited && data.metadata_source.rate_limit) { - handleSpotifyRateLimit(data.metadata_source.rate_limit); + if (data.spotify?.rate_limited && data.spotify.rate_limit) { + handleSpotifyRateLimit(data.spotify.rate_limit); _spotifyInCooldown = false; - } else if (data.metadata_source?.post_ban_cooldown > 0) { + } else if (data.spotify?.post_ban_cooldown > 0) { if (_spotifyRateLimitShown && !_spotifyInCooldown) { _spotifyRateLimitShown = false; _spotifyInCooldown = true; diff --git a/webui/static/settings.js b/webui/static/settings.js index 6307e575..79013d1a 100644 --- a/webui/static/settings.js +++ b/webui/static/settings.js @@ -60,7 +60,7 @@ function syncMetadataSourceSelection(source) { function _isMetadataSourceSelectable(source) { if (source === 'spotify') { - return _lastStatusPayload?.metadata_source?.authenticated === true; + return _lastStatusPayload?.spotify?.authenticated === true; } if (source === 'discogs') { const token = document.getElementById('discogs-token'); @@ -173,7 +173,7 @@ function initializeSettings() { if (discogsTokenInput) { discogsTokenInput.addEventListener('input', () => { if (typeof syncPrimaryMetadataSourceAvailability === 'function') { - syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.metadata_source || null); + syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.spotify || null); } sanitizeMetadataSourceSelection({ quiet: true }); }); @@ -207,9 +207,9 @@ function initializeSettings() { // Test button event listeners removed - they use onclick attributes in HTML to avoid double firing if (typeof syncPrimaryMetadataSourceAvailability === 'function') { - syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.metadata_source || null); + syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.spotify || null); } - syncSpotifySettingsAuthState(_lastStatusPayload?.metadata_source || null); + syncSpotifySettingsAuthState(_lastStatusPayload?.spotify || null); syncMetadataSourceSelection(_lastStatusPayload?.metadata_source?.source); sanitizeMetadataSourceSelection({ quiet: true }); if (metadataSourceSelect) { @@ -408,7 +408,7 @@ async function applyServiceStatusGradients() { else header.appendChild(spinner); } }); - syncSpotifySettingsAuthState(_lastStatusPayload?.metadata_source || null); + syncSpotifySettingsAuthState(_lastStatusPayload?.spotify || null); } catch (e) { console.warn('[Settings Status] Failed to apply gradients:', e); } @@ -2544,7 +2544,7 @@ async function saveSettings(quiet = false) { const discogsTokenInput = document.getElementById('discogs-token'); const discogsTokenPresent = !!discogsTokenInput?.value?.trim(); let metadataSource = metadataSourceSelect?.value || 'itunes'; - const spotifySessionActive = _lastStatusPayload?.metadata_source?.authenticated === true; + const spotifySessionActive = _lastStatusPayload?.spotify?.authenticated === true; if (metadataSource === 'spotify' && !spotifySessionActive) { metadataSource = 'deezer'; if (metadataSourceSelect) metadataSourceSelect.value = metadataSource; diff --git a/webui/static/shared-helpers.js b/webui/static/shared-helpers.js index 7b675fd3..0dc1b086 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3146,22 +3146,22 @@ async function fetchAndUpdateServiceStatus() { _lastStatusPayload = data; if (typeof syncSpotifySettingsAuthState === 'function') { - syncSpotifySettingsAuthState(data?.metadata_source || null); + syncSpotifySettingsAuthState(data?.spotify || null); } if (typeof syncPrimaryMetadataSourceAvailability === 'function') { - syncPrimaryMetadataSourceAvailability(data?.metadata_source || null); + syncPrimaryMetadataSourceAvailability(data?.spotify || null); } if (typeof sanitizeMetadataSourceSelection === 'function') { sanitizeMetadataSourceSelection({ quiet: true }); } // Update service status indicators and text (dashboard) - updateServiceStatus('spotify', data.metadata_source); + updateServiceStatus('spotify', data.metadata_source, data.spotify); updateServiceStatus('media-server', data.media_server); updateServiceStatus('soulseek', data.soulseek); // Update sidebar service status indicators - updateSidebarServiceStatus('spotify', data.metadata_source); + updateSidebarServiceStatus('spotify', data.metadata_source, data.spotify); updateSidebarServiceStatus('media-server', data.media_server); updateSidebarServiceStatus('soulseek', data.soulseek); @@ -3185,8 +3185,8 @@ async function fetchAndUpdateServiceStatus() { if (data.enrichment) renderEnrichmentCards(data.enrichment); // Check for Spotify rate limit - if (data.metadata_source && data.metadata_source.rate_limited && data.metadata_source.rate_limit) { - handleSpotifyRateLimit(data.metadata_source.rate_limit); + if (data.spotify && data.spotify.rate_limited && data.spotify.rate_limit) { + handleSpotifyRateLimit(data.spotify.rate_limit); } else if (_spotifyRateLimitShown) { handleSpotifyRateLimit(null); } @@ -3232,14 +3232,16 @@ function getMetadataSourceLabel(source) { return 'Unmapped'; } -function getSpotifyStatusPresentation(statusData) { - const sourceLabel = getMetadataSourceLabel(statusData?.source); - const rateLimited = !!(statusData?.rate_limited && statusData?.rate_limit); - const cooldown = !!(statusData?.post_ban_cooldown > 0); - const sessionActive = statusData?.authenticated === true || (statusData?.authenticated === undefined && statusData?.source === 'spotify'); +function getMetadataSourcePresentation(metadataStatus, spotifyStatus) { + const source = metadataStatus?.source; + const sourceLabel = getMetadataSourceLabel(source); + const connected = metadataStatus?.connected === true; + const sessionActive = spotifyStatus?.authenticated === true || (source === 'spotify' && connected); + const rateLimited = !!(source === 'spotify' && spotifyStatus?.rate_limited && spotifyStatus?.rate_limit); + const cooldown = !!(source === 'spotify' && spotifyStatus?.post_ban_cooldown > 0); if (rateLimited) { - const remaining = statusData.rate_limit?.remaining_seconds || 0; + const remaining = spotifyStatus.rate_limit?.remaining_seconds || 0; return { statusClass: 'rate-limited', statusText: `Spotify paused \u2014 ${formatRateLimitDuration(remaining)}`, @@ -3250,7 +3252,7 @@ function getSpotifyStatusPresentation(statusData) { } if (cooldown) { - const remaining = statusData.post_ban_cooldown; + const remaining = spotifyStatus.post_ban_cooldown; return { statusClass: 'rate-limited', statusText: `Spotify recovering \u2014 ${formatRateLimitDuration(remaining)}`, @@ -3260,32 +3262,32 @@ function getSpotifyStatusPresentation(statusData) { }; } - if (statusData?.source && statusData.source !== 'spotify') { + if (source) { return { - statusClass: 'connected', - statusText: sourceLabel, - dotClass: 'connected', - dotTitle: sourceLabel, + statusClass: connected ? 'connected' : 'disconnected', + statusText: connected ? (source === 'spotify' ? `Connected (${metadataStatus?.response_time}ms)` : sourceLabel) : 'Disconnected', + dotClass: connected ? 'connected' : 'disconnected', + dotTitle: connected ? sourceLabel : 'Disconnected', sessionActive }; } return { - statusClass: 'connected', - statusText: `Connected (${statusData?.response_time}ms)`, - dotClass: 'connected', - dotTitle: '', + statusClass: 'disconnected', + statusText: 'Disconnected', + dotClass: 'disconnected', + dotTitle: 'Disconnected', sessionActive }; } -function updateServiceStatus(service, statusData) { +function updateServiceStatus(service, statusData, spotifyStatus = null) { const indicator = document.getElementById(`${service}-status-indicator`); const statusText = document.getElementById(`${service}-status-text`); if (indicator && statusText) { if (service === 'spotify') { - const presentation = getSpotifyStatusPresentation(statusData || {}); + const presentation = getMetadataSourcePresentation(statusData || {}, spotifyStatus || {}); indicator.className = `service-card-indicator ${presentation.statusClass}`; statusText.textContent = presentation.statusText; statusText.className = `service-card-status-text ${presentation.statusClass}`; @@ -3312,7 +3314,7 @@ function updateServiceStatus(service, statusData) { } // Keep the Spotify action buttons aligned with the actual auth session. - const spotifySessionActive = getSpotifyStatusPresentation(statusData || {}).sessionActive; + const spotifySessionActive = spotifyStatus?.authenticated === true; const authBtn = document.querySelector('button[onclick="authenticateSpotify()"]'); const disconnectBtn = document.getElementById('spotify-disconnect-btn'); if (authBtn) { @@ -3322,7 +3324,7 @@ function updateServiceStatus(service, statusData) { disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } - syncPrimaryMetadataSourceAvailability(statusData); + syncPrimaryMetadataSourceAvailability(spotifyStatus); const responseTimeElement = document.getElementById('spotify-response-time'); if (responseTimeElement) { @@ -3348,7 +3350,7 @@ function updateServiceStatus(service, statusData) { } } -function updateSidebarServiceStatus(service, statusData) { +function updateSidebarServiceStatus(service, statusData, spotifyStatus = null) { const indicator = document.getElementById(`${service}-indicator`); if (indicator) { const dot = indicator.querySelector('.status-dot'); @@ -3356,7 +3358,7 @@ function updateSidebarServiceStatus(service, statusData) { if (dot) { if (service === 'spotify') { - const presentation = getSpotifyStatusPresentation(statusData || {}); + const presentation = getMetadataSourcePresentation(statusData || {}, spotifyStatus || {}); dot.className = `status-dot ${presentation.dotClass}`; dot.title = presentation.dotTitle; } else {