diff --git a/core/debug_info.py b/core/debug_info.py index 4ad83f1d..2af9d31a 100644 --- a/core/debug_info.py +++ b/core/debug_info.py @@ -300,23 +300,24 @@ def get_debug_info(): # API rate monitor — current calls/min, 24h totals, peaks, rate limit events try: from core.api_call_tracker import api_call_tracker + from core.metadata.status import get_spotify_status rates = api_call_tracker.get_all_rates() info['api_rates'] = rates # Rich 24h debug summary with peaks, totals, per-endpoint breakdown, events info['api_debug_summary'] = api_call_tracker.get_debug_summary() # Spotify rate limit details - if spotify_client: - rl_info = spotify_client.get_rate_limit_info() - if rl_info: - info['spotify_rate_limit'] = { - 'active': True, - 'remaining_seconds': rl_info.get('remaining_seconds', 0), - 'retry_after': rl_info.get('retry_after', 0), - 'endpoint': rl_info.get('endpoint', ''), - 'expires_at': rl_info.get('expires_at', ''), - } - else: - info['spotify_rate_limit'] = {'active': False} + spotify_status = get_spotify_status(spotify_client=spotify_client) + rl_info = spotify_status.get('rate_limit') + if spotify_status.get('rate_limited') and rl_info: + info['spotify_rate_limit'] = { + 'active': True, + 'remaining_seconds': rl_info.get('remaining_seconds', 0), + 'retry_after': rl_info.get('retry_after', 0), + 'endpoint': rl_info.get('endpoint', ''), + 'expires_at': rl_info.get('expires_at', ''), + } + else: + info['spotify_rate_limit'] = {'active': False} except Exception: info['api_rates'] = {} info['api_debug_summary'] = {} diff --git a/core/imports/context.py b/core/imports/context.py index a3e4a4c3..c302891e 100644 --- a/core/imports/context.py +++ b/core/imports/context.py @@ -32,6 +32,20 @@ def _first_id_value(*values: Any) -> str: return "" +def _first_source_aware_id(source: str, *values: Any) -> str: + source_name = (source or "").strip().lower() + for value in values: + if value in (None, ""): + continue + text = str(value).strip() + if not text: + continue + if source_name.startswith("spotify") and text.isdigit(): + continue + return text + return "" + + def extract_artist_name(artist: Any) -> str: if isinstance(artist, dict): return str(artist.get("name", "") or "") @@ -209,6 +223,7 @@ def get_import_has_full_metadata(context: Optional[Dict[str, Any]]) -> bool: def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]: + source = get_import_source(context) track_info = get_import_track_info(context) original_search = get_import_original_search(context) search_result = get_import_search_result(context) @@ -216,7 +231,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]: album = get_import_context_album(context) return { - "track_id": _first_id_value( + "track_id": _first_source_aware_id( + source, _first_value(track_info, "id", "track_id", "trackId", "source_track_id", default=""), _first_value(track_info, "spotify_track_id", "itunes_track_id", "deezer_id", "deezer_track_id", "discogs_id", "soul_id", default=""), _first_value(original_search, "id", "track_id", "source_track_id", default=""), @@ -224,7 +240,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]: _first_value(search_result, "id", "track_id", "source_track_id", default=""), _first_value(search_result, "spotify_track_id", "itunes_track_id", "deezer_id", "deezer_track_id", "discogs_id", "soul_id", default=""), ), - "artist_id": _first_id_value( + "artist_id": _first_source_aware_id( + source, _first_value(artist, "id", "artist_id", "source_artist_id", default=""), _first_value(artist, "spotify_artist_id", "itunes_artist_id", "deezer_id", "deezer_artist_id", "discogs_id", "soul_id", default=""), _first_value(original_search, "artist_id", "source_artist_id", default=""), @@ -232,7 +249,8 @@ def get_import_source_ids(context: Optional[Dict[str, Any]]) -> Dict[str, str]: _first_value(search_result, "artist_id", "source_artist_id", default=""), _first_value(search_result, "spotify_artist_id", "itunes_artist_id", "deezer_id", "deezer_artist_id", "discogs_id", "soul_id", default=""), ), - "album_id": _first_id_value( + "album_id": _first_source_aware_id( + source, _first_value(album, "id", "album_id", "collectionId", "source_album_id", default=""), _first_value(album, "spotify_album_id", "itunes_album_id", "deezer_id", "deezer_album_id", "discogs_id", "soul_id", "album_soul_id", "hydrabase_album_id", default=""), _first_value(original_search, "album_id", "source_album_id", default=""), diff --git a/core/metadata/__init__.py b/core/metadata/__init__.py index ea434e48..e7142adf 100644 --- a/core/metadata/__init__.py +++ b/core/metadata/__init__.py @@ -8,6 +8,7 @@ from core.metadata.album_tracks import ( resolve_album_reference, ) from core.metadata.artist_image import get_artist_image_url +from core.metadata.artwork import is_internal_image_host, normalize_image_url from core.metadata.cache import MetadataCache, get_metadata_cache from core.metadata.completion import ( check_album_completion, @@ -40,6 +41,13 @@ from core.metadata.registry import ( register_profile_spotify_credentials_provider, register_runtime_clients, ) +from core.metadata.status import ( + METADATA_SOURCE_STATUS_TTL, + get_metadata_source_status, + get_spotify_status, + get_status_snapshot, + invalidate_metadata_status_caches, +) from core.metadata.service import MetadataProvider, MetadataService, get_metadata_service from core.metadata.similar_artists import ( get_musicmap_similar_artists, @@ -48,6 +56,7 @@ from core.metadata.similar_artists import ( __all__ = [ "METADATA_SOURCE_PRIORITY", + "METADATA_SOURCE_STATUS_TTL", "MetadataCache", "MetadataLookupOptions", "MetadataProvider", @@ -71,6 +80,7 @@ __all__ = [ "get_hydrabase_client", "get_itunes_client", "get_metadata_cache", + "get_metadata_source_status", "get_metadata_service", "get_musicmap_similar_artists", "get_primary_client", @@ -78,11 +88,16 @@ __all__ = [ "get_spotify_client_for_profile", "get_registered_runtime_client", "get_spotify_client", + "get_spotify_status", "get_source_priority", + "get_status_snapshot", "iter_artist_discography_completion_events", "iter_musicmap_similar_artist_events", "is_hydrabase_enabled", + "is_internal_image_host", "register_profile_spotify_credentials_provider", "register_runtime_clients", + "normalize_image_url", "resolve_album_reference", + "invalidate_metadata_status_caches", ] diff --git a/core/metadata/artwork.py b/core/metadata/artwork.py index ca268e52..795a3fdf 100644 --- a/core/metadata/artwork.py +++ b/core/metadata/artwork.py @@ -5,6 +5,8 @@ from __future__ import annotations import os import re import urllib.request +from ipaddress import ip_address +from urllib.parse import quote, urlparse from core.imports.context import get_import_context_album from core.metadata.common import ( @@ -17,12 +19,193 @@ from utils.logging_config import get_logger as _create_logger __all__ = [ "embed_album_art_metadata", "download_cover_art", + "is_internal_image_host", + "is_image_proxy_url", + "normalize_image_url", ] logger = _create_logger("metadata.artwork") +def normalize_image_url(thumb_url: str | None) -> str | None: + """Convert media-server image URLs into browser-safe URLs.""" + if not thumb_url: + return None + + try: + if is_image_proxy_url(thumb_url): + # Already normalized for browser use; avoid wrapping it in another proxy layer. + return thumb_url + + # Check if it's a localhost URL or relative path that needs fixing + needs_fixing = ( + thumb_url.startswith('http://localhost:') or + thumb_url.startswith('https://localhost:') or + thumb_url.startswith('http://127.0.0.1:') or + thumb_url.startswith('https://127.0.0.1:') or + thumb_url.startswith('http://host.docker.internal:') or + thumb_url.startswith('https://host.docker.internal:') or + (thumb_url.startswith('http://') and is_internal_image_host(thumb_url)) or + thumb_url.startswith('/library/') or # Plex relative paths + thumb_url.startswith('/Items/') or # Jellyfin relative paths + thumb_url.startswith('/api/') or # Old Navidrome API paths + thumb_url.startswith('/rest/') # Navidrome Subsonic API paths + ) + + if needs_fixing: + cfg = get_config_manager() + active_server = cfg.get_active_media_server() + logger.debug("Fixing URL: %s, Active server: %s", thumb_url, active_server) + + if active_server == 'plex': + plex_config = cfg.get_plex_config() + plex_base_url = plex_config.get('base_url', '') + plex_token = plex_config.get('token', '') + logger.info("Plex config - base_url: %s, token: %s...", plex_base_url, plex_token[:10]) + + if plex_base_url and plex_token: + # Extract the path from URL + if thumb_url.startswith('/library/'): + # Already a path + path = thumb_url + else: + # Full localhost URL, extract path + parsed = urlparse(thumb_url) + path = parsed.path + + # Construct proper Plex URL with token + fixed_url = f"{plex_base_url.rstrip('/')}{path}?X-Plex-Token={plex_token}" + logger.info("Fixed URL: %s", fixed_url) + return _browser_safe_image_url(fixed_url) + + elif active_server == 'jellyfin': + jellyfin_config = cfg.get_jellyfin_config() + jellyfin_base_url = jellyfin_config.get('base_url', '') + jellyfin_token = jellyfin_config.get('api_key', '') + logger.info( + "Jellyfin config - base_url: %s, token: %s...", + jellyfin_base_url, + jellyfin_token[:10] if jellyfin_token else 'None', + ) + + if jellyfin_base_url: + # Extract the path from URL + if thumb_url.startswith('/Items/') or thumb_url.startswith('/api/'): + # Already a path + path = thumb_url + else: + # Full localhost URL, extract path + parsed = urlparse(thumb_url) + path = parsed.path + + # Construct proper Jellyfin URL with token + if jellyfin_token: + separator = '&' if '?' in path else '?' + fixed_url = f"{jellyfin_base_url.rstrip('/')}{path}{separator}X-Emby-Token={jellyfin_token}" + else: + fixed_url = f"{jellyfin_base_url.rstrip('/')}{path}" + logger.info("Fixed URL: %s", fixed_url) + return _browser_safe_image_url(fixed_url) + + elif active_server == 'navidrome': + navidrome_config = cfg.get_navidrome_config() + navidrome_base_url = navidrome_config.get('base_url', '') + navidrome_username = navidrome_config.get('username', '') + navidrome_password = navidrome_config.get('password', '') + logger.info("Navidrome config - base_url: %s, username: %s", navidrome_base_url, navidrome_username) + + if navidrome_base_url and navidrome_username and navidrome_password: + # Extract the path from URL + if thumb_url.startswith('/rest/'): + # Already a Subsonic API path + path = thumb_url + else: + # Full localhost URL, extract path + parsed = urlparse(thumb_url) + path = parsed.path + + # Generate Subsonic API authentication + import hashlib + import secrets + salt = secrets.token_hex(6) + token = hashlib.md5((navidrome_password + salt).encode()).hexdigest() + + # Add authentication parameters to the URL + separator = '&' if '?' in path else '?' + auth_params = f"u={navidrome_username}&t={token}&s={salt}&v=1.16.1&c=SoulSync&f=json" + + # Construct proper Navidrome Subsonic URL + fixed_url = f"{navidrome_base_url.rstrip('/')}{path}{separator}{auth_params}" + logger.info("Fixed URL: %s", fixed_url) + return _browser_safe_image_url(fixed_url) + + logger.warning("No configuration found for %s or unsupported server type", active_server) + + # Return a browser-safe URL even if no server-specific rebuild was possible. + return _browser_safe_image_url(thumb_url) + + except Exception as exc: + logger.error("Error fixing image URL '%s': %s", thumb_url, exc) + return _browser_safe_image_url(thumb_url) + + +def is_image_proxy_url(url: str) -> bool: + """Return True for SoulSync image-proxy URLs, absolute or relative.""" + if not url: + return False + + try: + parsed = urlparse(url) + return parsed.path == '/api/image-proxy' + except Exception: + return False + + +def is_internal_image_host(url: str) -> bool: + """Return True when an image URL points at a host the browser likely cannot reach directly.""" + try: + parsed = urlparse(url) + host = (parsed.hostname or '').strip('[]').lower() + if not host: + return False + + if host in {'localhost', '127.0.0.1', '::1', 'host.docker.internal'}: + return True + + # Single-label hosts are usually Docker service names or local LAN aliases. + if '.' not in host: + return True + + try: + ip = ip_address(host) + return ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved + except ValueError: + return False + except Exception: + return False + + +def _browser_safe_image_url(url: str) -> str: + """Return a browser-safe image URL, proxying internal hosts through SoulSync.""" + if not url: + return url + + if is_image_proxy_url(url): + return url + + if url.startswith('/api/image-proxy?url='): + return url + + if url.startswith('http://') or url.startswith('https://'): + if is_internal_image_host(url): + return f"/api/image-proxy?url={quote(url, safe='')}" + return url + + # Relative media-server paths should already have been expanded before this point. + return url + + def embed_album_art_metadata(audio_file, metadata: dict): cfg = get_config_manager() symbols = get_mutagen_symbols() 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/core/metadata/status.py b/core/metadata/status.py new file mode 100644 index 00000000..9e3fdffb --- /dev/null +++ b/core/metadata/status.py @@ -0,0 +1,179 @@ +"""Cached metadata-provider and Spotify status snapshots.""" + +from __future__ import annotations + +import threading +import time +from typing import Any, Dict, Optional + +from utils.logging_config import get_logger + +from core.metadata.registry import get_primary_source_status + +logger = get_logger("metadata.status") + +METADATA_SOURCE_STATUS_TTL = 120 + +_UNSET = object() + +_status_lock = threading.RLock() +_metadata_source_status_cache: Dict[str, Any] = { + "source": "deezer", + "connected": False, + "response_time": 0, +} +_metadata_source_status_timestamp = 0.0 +_spotify_status_cache: Dict[str, Any] = { + "connected": False, + "authenticated": False, + "rate_limited": False, + "rate_limit": None, + "post_ban_cooldown": None, +} +_spotify_status_timestamp = 0.0 +_spotify_status_initialized = False +_spotify_rate_limit_expires_at = 0.0 +_spotify_post_ban_cooldown_expires_at = 0.0 + + +def invalidate_metadata_status_caches() -> None: + """Mark the cached metadata-source snapshot stale.""" + global _metadata_source_status_timestamp + with _status_lock: + _metadata_source_status_timestamp = 0.0 + + +def publish_spotify_status( + *, + connected: Any = _UNSET, + authenticated: Any = _UNSET, + rate_limited: Any = _UNSET, + rate_limit: Any = _UNSET, + post_ban_cooldown: Any = _UNSET, +) -> Dict[str, Any]: + """Update the cached Spotify status snapshot from an event.""" + global _spotify_status_timestamp, _spotify_status_initialized + global _spotify_rate_limit_expires_at, _spotify_post_ban_cooldown_expires_at + + with _status_lock: + if connected is not _UNSET: + _spotify_status_cache["connected"] = connected + if authenticated is not _UNSET: + _spotify_status_cache["authenticated"] = authenticated + if rate_limited is not _UNSET: + _spotify_status_cache["rate_limited"] = rate_limited + if rate_limit is not _UNSET: + _spotify_status_cache["rate_limit"] = rate_limit + if rate_limit and isinstance(rate_limit, dict): + _spotify_rate_limit_expires_at = float(rate_limit.get("expires_at") or 0.0) + else: + _spotify_rate_limit_expires_at = 0.0 + if post_ban_cooldown is not _UNSET: + _spotify_status_cache["post_ban_cooldown"] = post_ban_cooldown + if post_ban_cooldown is not None: + _spotify_post_ban_cooldown_expires_at = time.time() + max(0, float(post_ban_cooldown)) + else: + _spotify_post_ban_cooldown_expires_at = 0.0 + _spotify_status_timestamp = time.time() + _spotify_status_initialized = True + _normalize_spotify_status_locked(_spotify_status_timestamp) + return dict(_spotify_status_cache) + + +def refresh_spotify_status_from_client(spotify_client: Optional[Any]) -> Dict[str, Any]: + """Probe Spotify once to seed the cache when no event has populated it yet.""" + if spotify_client is None: + with _status_lock: + return dict(_spotify_status_cache) + + try: + 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 + except Exception as exc: + logger.debug("Spotify status probe failed: %s", exc) + authenticated = False + is_rate_limited = False + rate_limit_info = None + cooldown_remaining = 0 + + return publish_spotify_status( + 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, + ) + + +def get_metadata_source_status() -> Dict[str, Any]: + """Return a cached snapshot for the active primary metadata source.""" + global _metadata_source_status_timestamp + + current_time = time.time() + with _status_lock: + if _metadata_source_status_timestamp and current_time - _metadata_source_status_timestamp <= METADATA_SOURCE_STATUS_TTL: + return dict(_metadata_source_status_cache) + + try: + status_data = get_primary_source_status() + except Exception as exc: + logger.debug("Metadata source status refresh failed: %s", exc) + status_data = None + + if status_data: + with _status_lock: + _metadata_source_status_cache.update(status_data) + _metadata_source_status_timestamp = current_time + + with _status_lock: + return dict(_metadata_source_status_cache) + + +def get_spotify_status(spotify_client: Optional[Any] = None) -> Dict[str, Any]: + """Return a cached Spotify-specific status snapshot.""" + with _status_lock: + if _spotify_status_initialized: + _normalize_spotify_status_locked(time.time()) + return dict(_spotify_status_cache) + + return refresh_spotify_status_from_client(spotify_client) + + +def _normalize_spotify_status_locked(current_time: float) -> None: + """Update derived Spotify status fields and clear expired ban state.""" + global _spotify_rate_limit_expires_at, _spotify_post_ban_cooldown_expires_at + + rate_limit = _spotify_status_cache.get("rate_limit") + if _spotify_status_cache.get("rate_limited") and rate_limit and isinstance(rate_limit, dict): + expires_at = float(rate_limit.get("expires_at") or 0.0) + if expires_at > 0: + remaining = int(max(0, expires_at - current_time)) + if remaining > 0: + _spotify_status_cache["rate_limit"] = {**rate_limit, "remaining_seconds": remaining} + _spotify_rate_limit_expires_at = expires_at + else: + _spotify_status_cache["rate_limited"] = False + _spotify_status_cache["rate_limit"] = None + _spotify_rate_limit_expires_at = 0.0 + elif _spotify_rate_limit_expires_at and current_time >= _spotify_rate_limit_expires_at: + _spotify_status_cache["rate_limited"] = False + _spotify_status_cache["rate_limit"] = None + _spotify_rate_limit_expires_at = 0.0 + + if _spotify_post_ban_cooldown_expires_at > 0: + remaining = int(max(0, _spotify_post_ban_cooldown_expires_at - current_time)) + if remaining > 0: + _spotify_status_cache["post_ban_cooldown"] = remaining + else: + _spotify_status_cache["post_ban_cooldown"] = None + _spotify_post_ban_cooldown_expires_at = 0.0 + + +def get_status_snapshot(spotify_client: Optional[Any] = None) -> Dict[str, Any]: + """Return the combined metadata-provider status snapshot.""" + return { + "metadata_source": get_metadata_source_status(), + "spotify": get_spotify_status(spotify_client=spotify_client), + } diff --git a/core/spotify_client.py b/core/spotify_client.py index 50979bd5..124aa7b5 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -138,6 +138,18 @@ def _set_global_rate_limit(retry_after_seconds, endpoint_name, has_real_header=F ) except Exception: pass + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=True, + rate_limited=True, + rate_limit=_get_rate_limit_info(), + post_ban_cooldown=_get_post_ban_cooldown_remaining() or None, + ) + except Exception: + pass def _is_globally_rate_limited(): @@ -210,6 +222,16 @@ def _clear_rate_limit(): _rate_limit_hit_count = 0 _rate_limit_first_hit = 0 logger.info("Global rate limit ban cleared (including post-ban cooldown)") + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception: + pass def _detect_and_set_rate_limit(exception, endpoint_name="unknown"): @@ -603,13 +625,38 @@ class SpotifyClient: """Check if Spotify client is specifically authenticated (not just iTunes fallback). Results are cached for 60 seconds to avoid excessive API calls. During rate limit bans and post-ban cooldown, returns False without making API calls.""" + rate_limited_state = False if self.sp is None: + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=False, + rate_limited=_is_globally_rate_limited(), + rate_limit=_get_rate_limit_info(), + post_ban_cooldown=_get_post_ban_cooldown_remaining() or None, + ) + except Exception: + pass return False # If globally rate limited, report as NOT authenticated so callers # skip Spotify and fall through to iTunes fallback naturally. # This prevents any API calls that could extend the ban. if _is_globally_rate_limited(): + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=True, + rate_limited=True, + rate_limit=_get_rate_limit_info(), + post_ban_cooldown=_get_post_ban_cooldown_remaining() or None, + ) + except Exception: + pass return False # Post-ban cooldown: after a ban expires, don't probe Spotify immediately. @@ -618,11 +665,35 @@ class SpotifyClient: if _is_in_post_ban_cooldown(): remaining = _get_post_ban_cooldown_remaining() logger.debug(f"Post-ban cooldown active ({remaining}s left), skipping auth probe") + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=True, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=remaining or None, + ) + except Exception: + pass return False # Check cache first (lock only for brief read) with self._auth_cache_lock: if self._auth_cached_result is not None and (time.time() - self._auth_cache_time) < self._AUTH_CACHE_TTL: + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=self._auth_cached_result, + authenticated=self._auth_cached_result, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception: + pass return self._auth_cached_result # Cache miss — make API call outside the lock. @@ -637,6 +708,18 @@ class SpotifyClient: with self._auth_cache_lock: self._auth_cached_result = False self._auth_cache_time = time.time() + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=False, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception: + pass return False except Exception: pass @@ -668,6 +751,7 @@ class SpotifyClient: ban_duration = max(delay, _BASE_UNKNOWN_BAN) _set_global_rate_limit(ban_duration, 'is_spotify_authenticated', has_real_header=has_real_header) logger.warning(f"Auth probe rate limited — activating {ban_duration}s global ban") + rate_limited_state = True result = True else: logger.debug(f"Spotify authentication check failed: {e}") @@ -677,6 +761,19 @@ class SpotifyClient: self._auth_cached_result = result self._auth_cache_time = time.time() + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=result, + authenticated=result, + rate_limited=rate_limited_state, + rate_limit=_get_rate_limit_info() if rate_limited_state else None, + post_ban_cooldown=None, + ) + except Exception: + pass + return result def disconnect(self): @@ -686,6 +783,18 @@ class SpotifyClient: self.user_id = None self._invalidate_auth_cache() _clear_rate_limit() + try: + from core.metadata.status import publish_spotify_status + + publish_spotify_status( + connected=False, + authenticated=False, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception: + pass cache_path = 'config/.spotify_cache' try: diff --git a/tests/conftest.py b/tests/conftest.py index 03b5ec5f..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 = { - 'spotify': {'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'}, } @@ -255,6 +256,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']), diff --git a/tests/imports/test_import_context.py b/tests/imports/test_import_context.py index 65ab1e12..e71584b7 100644 --- a/tests/imports/test_import_context.py +++ b/tests/imports/test_import_context.py @@ -206,6 +206,35 @@ def test_get_import_source_ids_prefers_nested_source_specific_ids(): } +def test_get_import_source_ids_prefers_spotify_ids_over_numeric_fallbacks(): + context = normalize_import_context( + { + "source": "spotify", + "artist": { + "id": "396753", + "spotify_artist_id": "sp-artist-1", + "deezer_id": "396753", + }, + "album": { + "id": "284076172", + "spotify_album_id": "sp-album-1", + "deezer_id": "284076172", + }, + "track_info": { + "id": "1607091752", + "spotify_track_id": "sp-track-1", + "deezer_id": "1607091752", + }, + } + ) + + assert get_import_source_ids(context) == { + "track_id": "sp-track-1", + "artist_id": "sp-artist-1", + "album_id": "sp-album-1", + } + + def test_build_import_album_info_uses_normalized_album_context(): context = normalize_import_context( { diff --git a/tests/imports/test_import_side_effects.py b/tests/imports/test_import_side_effects.py index 697d845b..2091f0b0 100644 --- a/tests/imports/test_import_side_effects.py +++ b/tests/imports/test_import_side_effects.py @@ -142,3 +142,59 @@ def test_record_soulsync_library_entry_writes_artist_album_and_track(tmp_path, m assert track_row["track_artist"] == "Guest Artist" assert track_row["album_id"] == album_row["id"] assert track_row["file_path"] == str(final_path) + + +def test_record_soulsync_library_entry_ignores_numeric_spotify_ids(tmp_path, monkeypatch): + conn = _make_soulsync_db() + fake_db = _FakeDB(conn) + final_path = tmp_path / "track.flac" + final_path.write_bytes(b"audio") + + monkeypatch.setattr(side_effects, "get_database", lambda: fake_db) + monkeypatch.setattr( + side_effects, + "_get_config_manager", + lambda: SimpleNamespace(get_active_media_server=lambda: "soulsync"), + ) + + import core.genre_filter as genre_filter + + monkeypatch.setattr(genre_filter, "filter_genres", lambda genres, _cfg: genres) + + context = { + "source": "spotify", + "artist": {"id": "396753", "name": "Artist One"}, + "album": { + "id": "284076172", + "name": "Album One", + "release_date": "2024-02-03", + "total_tracks": 12, + }, + "track_info": { + "id": "1607091752", + "name": "Song One", + "track_number": 7, + "duration_ms": 210000, + "artists": [{"name": "Guest Artist"}], + "_source": "spotify", + }, + "original_search_result": { + "title": "Song One", + "artists": [{"name": "Guest Artist"}], + "_source": "spotify", + }, + "_final_processed_path": str(final_path), + } + + artist_context = {"name": "Artist One", "genres": ["rock"]} + album_info = {"is_album": True, "album_name": "Album One", "track_number": 7} + + side_effects.record_soulsync_library_entry(context, artist_context, album_info) + + artist_row = conn.execute("SELECT * FROM artists").fetchone() + album_row = conn.execute("SELECT * FROM albums").fetchone() + track_row = conn.execute("SELECT * FROM tracks").fetchone() + + assert artist_row["spotify_artist_id"] is None + assert album_row["spotify_album_id"] is None + assert track_row["spotify_track_id"] is None diff --git a/tests/metadata/test_image_url_normalization.py b/tests/metadata/test_image_url_normalization.py new file mode 100644 index 00000000..2832ba4c --- /dev/null +++ b/tests/metadata/test_image_url_normalization.py @@ -0,0 +1,44 @@ +"""Regression tests for image URL normalization.""" + +from __future__ import annotations + +import pytest +from urllib.parse import quote + + +@pytest.mark.parametrize( + "thumb_url", + [ + "/api/image-proxy?url=https%3A%2F%2Fexample.com%2Fcover.jpg", + "http://host.docker.internal:4533/api/image-proxy?u=ketiska&t=abc&s=def&v=1.16.1&c=SoulSync&f=json", + ], +) +def test_normalize_image_url_leaves_existing_image_proxy_urls_alone(thumb_url): + """Existing proxy URLs should not be wrapped in a second proxy layer.""" + from core.metadata import normalize_image_url + + assert normalize_image_url(thumb_url) == thumb_url + + +def test_normalize_image_url_proxies_internal_http_urls(monkeypatch): + """Raw internal image URLs should still be routed through SoulSync's proxy.""" + from core.metadata import normalize_image_url + from core.metadata import artwork + + class _FakeConfig: + def get_active_media_server(self): + return "spotify" + + def get_plex_config(self): + return {} + + def get_jellyfin_config(self): + return {} + + def get_navidrome_config(self): + return {} + + monkeypatch.setattr(artwork, "get_config_manager", lambda: _FakeConfig()) + + url = "http://localhost:4533/cover.jpg" + assert normalize_image_url(url) == f"/api/image-proxy?url={quote(url, safe='')}" diff --git a/tests/test_websocket_infrastructure.py b/tests/test_websocket_infrastructure.py index 392f80aa..4e17eead 100644 --- a/tests/test_websocket_infrastructure.py +++ b/tests/test_websocket_infrastructure.py @@ -52,10 +52,12 @@ class TestServiceStatus: status_events = [e for e in received if e['name'] == 'status:update'] 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 'source' in data['metadata_source'] assert 'authenticated' in data['spotify'] def test_status_matches_http(self, test_app, shared_state): @@ -73,6 +75,7 @@ class TestServiceStatus: assert len(status_events) >= 1 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'] @@ -86,13 +89,13 @@ class TestServiceStatus: build = shared_state['build_status_payload'] # Mutate cache - status_cache['spotify']['source'] = 'itunes' + status_cache['metadata_source']['source'] = 'itunes' socketio.emit('status:update', build()) received = client.get_received() status_events = [e for e in received if e['name'] == 'status:update'] data = status_events[-1]['args'][0] - assert data['spotify']['source'] == 'itunes' + assert data['metadata_source']['source'] == 'itunes' # ========================================================================= diff --git a/web_server.py b/web_server.py index 2b4be31d..0642210c 100644 --- a/web_server.py +++ b/web_server.py @@ -5,7 +5,6 @@ import json import asyncio import requests import socket -import ipaddress import subprocess import platform import threading @@ -18,7 +17,7 @@ import collections import functools from contextlib import contextmanager from pathlib import Path -from urllib.parse import quote, urljoin, urlparse +from urllib.parse import urljoin, urlparse from concurrent.futures import ThreadPoolExecutor, as_completed from flask import Flask, render_template, request, jsonify, redirect, send_file, send_from_directory, Response, session, g, abort @@ -96,6 +95,8 @@ from core.database_update_worker import DatabaseUpdateWorker from core.web_scan_manager import WebScanManager from core.metadata.cache import get_metadata_cache from core.metadata import registry as metadata_registry +from core.metadata import is_internal_image_host +from core.metadata import normalize_image_url as fix_artist_image_url from core.metadata.registry import ( clear_cached_metadata_client, get_metadata_source_label, @@ -103,6 +104,12 @@ from core.metadata.registry import ( get_spotify_disconnect_source, register_runtime_clients, ) +from core.metadata.status import ( + get_status_snapshot as get_metadata_status_snapshot, + get_spotify_status, + publish_spotify_status, + invalidate_metadata_status_caches, +) from core.imports.context import ( get_import_clean_album, get_import_clean_title, @@ -811,12 +818,10 @@ _idle_since = {} _IDLE_GRACE_SECONDS = 5 _status_cache = { - 'spotify': {'connected': False, 'authenticated': False, 'response_time': 0, 'source': 'itunes'}, 'media_server': {'connected': False, 'response_time': 0, 'type': None}, 'soulseek': {'connected': False, 'response_time': 0}, } _status_cache_timestamps: dict[str, float] = { - 'spotify': 0, 'media_server': 0, 'soulseek': 0, } @@ -3446,43 +3451,7 @@ def get_status(): current_time = time.time() active_server = config_manager.get_active_media_server() - # Test Spotify - with caching to avoid excessive API calls - if current_time - _status_cache_timestamps['spotify'] > 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 - spotify_session_active = 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 - spotify_response_time = 0 - else: - spotify_start = time.time() - spotify_connected = spotify_client.is_spotify_authenticated() if spotify_client else False - spotify_response_time = (time.time() - spotify_start) * 1000 - # Use configured source; fall back to deezer only if Spotify isn't authenticated - if configured_source == 'spotify': - music_source = 'spotify' if spotify_connected else 'deezer' - else: - music_source = configured_source - - _status_cache['spotify'] = { - 'connected': spotify_session_active, - 'authenticated': spotify_session_active, - 'response_time': round(spotify_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_timestamps['spotify'] = current_time - # else: use cached value + metadata_status = get_metadata_status_snapshot(spotify_client=spotify_client) # Test media server - use EXISTING instances (they have internal caching) # Media server clients already cache connection checks internally @@ -3556,7 +3525,8 @@ def get_status(): active_dl_count += 1 status_data = { - 'spotify': _status_cache['spotify'], + 'metadata_source': metadata_status['metadata_source'], + 'spotify': metadata_status['spotify'], 'media_server': _status_cache['media_server'], 'soulseek': _status_cache['soulseek'], 'active_media_server': active_server, @@ -4185,8 +4155,16 @@ def handle_settings(): genius_worker._init_client() if tidal_enrichment_worker: tidal_enrichment_worker.client = tidal_client + if 'spotify' in new_settings: + publish_spotify_status( + connected=False, + authenticated=False, + rate_limited=False, + rate_limit=None, + post_ban_cooldown=None, + ) # Invalidate status cache so next poll reflects new settings (e.g. fallback source change) - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() logger.info("Service clients re-initialized with new settings.") return jsonify({"success": True, "message": "Settings saved successfully."}) except Exception as e: @@ -4803,11 +4781,7 @@ def test_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False - _status_cache['spotify']['connected'] = True - _status_cache['spotify']['authenticated'] = spotify_session_active - _status_cache['spotify']['source'] = _get_metadata_fallback_source() - _status_cache_timestamps['spotify'] = current_time + invalidate_metadata_status_caches() logger.info("Updated Spotify status cache after successful test") elif service in ['plex', 'jellyfin', 'navidrome', 'soulsync']: _status_cache['media_server']['connected'] = True @@ -4971,11 +4945,7 @@ def test_dashboard_connection_endpoint(): if success: current_time = time.time() if service == 'spotify': - spotify_session_active = spotify_client.is_spotify_authenticated() if spotify_client else False - _status_cache['spotify']['connected'] = True - _status_cache['spotify']['authenticated'] = spotify_session_active - _status_cache['spotify']['source'] = _get_metadata_fallback_source() - _status_cache_timestamps['spotify'] = current_time + invalidate_metadata_status_caches() logger.info("Updated Spotify status cache after successful dashboard test") elif service in ['plex', 'jellyfin', 'navidrome', 'soulsync']: _status_cache['media_server']['connected'] = True @@ -5854,7 +5824,7 @@ def spotify_callback(): return _spotify_auth_result_page("Your personal Spotify account is now connected. You can close this window.", authenticated=True) if profile_client: profile_client._invalidate_auth_cache() - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() add_activity_item("", "Spotify Auth Warning", f"Profile {profile_id_from_state} completed OAuth but Spotify did not confirm an authenticated session", "Now") return _spotify_auth_result_page( "Spotify authorization completed, but SoulSync could not confirm an authenticated Spotify session for this profile. You can close this window and try Authenticate again.", @@ -5890,7 +5860,7 @@ def spotify_callback(): _clear_rate_limit() spotify_client._invalidate_auth_cache() # Invalidate status cache so next poll picks up the new connection - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() # Refresh enrichment worker's client so it picks up new auth if spotify_enrichment_worker and hasattr(spotify_enrichment_worker, 'client'): spotify_enrichment_worker.client.reload_config() @@ -5900,7 +5870,7 @@ def spotify_callback(): else: logger.warning("Spotify OAuth token exchange succeeded but authentication validation failed") spotify_client._invalidate_auth_cache() - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() add_activity_item("", "Spotify Auth Warning", "OAuth completed, but Spotify did not confirm an authenticated session", "Now") return _spotify_auth_result_page( "Spotify authorization completed, but SoulSync could not confirm an authenticated Spotify session. You can close this window and try Authenticate again.", @@ -5929,16 +5899,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['spotify'] = { - 'connected': False, - 'authenticated': False, - 'response_time': 0, - 'source': active_source, - 'rate_limited': False, - 'rate_limit': None, - 'post_ban_cooldown': None - } - _status_cache_timestamps['spotify'] = time.time() + invalidate_metadata_status_caches() add_activity_item("", "Spotify Disconnected", f"Using {source_label} for metadata", "Now") return jsonify({ 'success': True, @@ -5956,15 +5917,20 @@ def spotify_disconnect(): def spotify_rate_limit_status(): """Get Spotify rate limit ban details""" try: - info = spotify_client.get_rate_limit_info() + info = get_spotify_status(spotify_client=spotify_client) + rate_limit_info = info.get('rate_limit') if info: - return jsonify({ - 'rate_limited': True, - 'remaining_seconds': info['remaining_seconds'], - 'retry_after': info['retry_after'], - 'endpoint': info['endpoint'], - 'expires_at': info['expires_at'] - }) + payload = { + 'rate_limited': bool(info.get('rate_limited')), + } + if rate_limit_info: + payload.update({ + 'remaining_seconds': rate_limit_info.get('remaining_seconds', 0), + 'retry_after': rate_limit_info.get('retry_after'), + 'endpoint': rate_limit_info.get('endpoint'), + 'expires_at': rate_limit_info.get('expires_at'), + }) + return jsonify(payload) return jsonify({'rate_limited': False}) except Exception as e: logger.error(f"Error getting Spotify rate limit status: {e}") @@ -8123,163 +8089,6 @@ def maintain_search_history(): except Exception as e: logger.error(f"Error maintaining search history: {e}") return jsonify({"success": False, "error": str(e)}), 500 - -def fix_artist_image_url(thumb_url): - """Convert media-server image URLs into browser-safe URLs.""" - if not thumb_url: - return None - - try: - # Check if it's a localhost URL or relative path that needs fixing - needs_fixing = ( - thumb_url.startswith('http://localhost:') or - thumb_url.startswith('https://localhost:') or - thumb_url.startswith('http://127.0.0.1:') or - thumb_url.startswith('https://127.0.0.1:') or - thumb_url.startswith('http://host.docker.internal:') or - thumb_url.startswith('https://host.docker.internal:') or - (thumb_url.startswith('http://') and _is_internal_image_host(thumb_url)) or - thumb_url.startswith('/library/') or # Plex relative paths - thumb_url.startswith('/Items/') or # Jellyfin relative paths - thumb_url.startswith('/api/') or # Old Navidrome API paths - thumb_url.startswith('/rest/') # Navidrome Subsonic API paths - ) - - if needs_fixing: - active_server = config_manager.get_active_media_server() - logger.debug(f"Fixing URL: {thumb_url}, Active server: {active_server}") - - if active_server == 'plex': - plex_config = config_manager.get_plex_config() - plex_base_url = plex_config.get('base_url', '') - plex_token = plex_config.get('token', '') - logger.info(f"Plex config - base_url: {plex_base_url}, token: {plex_token[:10]}...") - - if plex_base_url and plex_token: - # Extract the path from URL - if thumb_url.startswith('/library/'): - # Already a path - path = thumb_url - else: - # Full localhost URL, extract path - from urllib.parse import urlparse - parsed = urlparse(thumb_url) - path = parsed.path - - # Construct proper Plex URL with token - fixed_url = f"{plex_base_url.rstrip('/')}{path}?X-Plex-Token={plex_token}" - logger.info(f"Fixed URL: {fixed_url}") - return _browser_safe_image_url(fixed_url) - - elif active_server == 'jellyfin': - jellyfin_config = config_manager.get_jellyfin_config() - jellyfin_base_url = jellyfin_config.get('base_url', '') - jellyfin_token = jellyfin_config.get('api_key', '') - logger.info(f"Jellyfin config - base_url: {jellyfin_base_url}, token: {jellyfin_token[:10] if jellyfin_token else 'None'}...") - - if jellyfin_base_url: - # Extract the path from URL - if thumb_url.startswith('/Items/') or thumb_url.startswith('/api/'): - # Already a path - path = thumb_url - else: - # Full localhost URL, extract path - from urllib.parse import urlparse - parsed = urlparse(thumb_url) - path = parsed.path - - # Construct proper Jellyfin URL with token - if jellyfin_token: - separator = '&' if '?' in path else '?' - fixed_url = f"{jellyfin_base_url.rstrip('/')}{path}{separator}X-Emby-Token={jellyfin_token}" - else: - fixed_url = f"{jellyfin_base_url.rstrip('/')}{path}" - logger.info(f"Fixed URL: {fixed_url}") - return _browser_safe_image_url(fixed_url) - - elif active_server == 'navidrome': - navidrome_config = config_manager.get_navidrome_config() - navidrome_base_url = navidrome_config.get('base_url', '') - navidrome_username = navidrome_config.get('username', '') - navidrome_password = navidrome_config.get('password', '') - logger.info(f"Navidrome config - base_url: {navidrome_base_url}, username: {navidrome_username}") - - if navidrome_base_url and navidrome_username and navidrome_password: - # Extract the path from URL - if thumb_url.startswith('/rest/'): - # Already a Subsonic API path - path = thumb_url - else: - # Full localhost URL, extract path - from urllib.parse import urlparse - parsed = urlparse(thumb_url) - path = parsed.path - - # Generate Subsonic API authentication - import hashlib - import secrets - salt = secrets.token_hex(6) - token = hashlib.md5((navidrome_password + salt).encode()).hexdigest() - - # Add authentication parameters to the URL - separator = '&' if '?' in path else '?' - auth_params = f"u={navidrome_username}&t={token}&s={salt}&v=1.16.1&c=SoulSync&f=json" - - # Construct proper Navidrome Subsonic URL - fixed_url = f"{navidrome_base_url.rstrip('/')}{path}{separator}{auth_params}" - logger.info(f"Fixed URL: {fixed_url}") - return _browser_safe_image_url(fixed_url) - - logger.warning(f"No configuration found for {active_server} or unsupported server type") - - # Return a browser-safe URL even if no server-specific rebuild was possible. - return _browser_safe_image_url(thumb_url) - - except Exception as e: - logger.error(f"Error fixing image URL '{thumb_url}': {e}") - return _browser_safe_image_url(thumb_url) - - -def _is_internal_image_host(url: str) -> bool: - """Return True when an image URL points at a host the browser likely cannot reach directly.""" - try: - parsed = urlparse(url) - host = (parsed.hostname or '').strip('[]').lower() - if not host: - return False - - if host in {'localhost', '127.0.0.1', '::1', 'host.docker.internal'}: - return True - - # Single-label hosts are usually Docker service names or local LAN aliases. - if '.' not in host: - return True - - try: - ip = ipaddress.ip_address(host) - return ip.is_loopback or ip.is_private or ip.is_link_local or ip.is_reserved - except ValueError: - return False - except Exception: - return False - - -def _browser_safe_image_url(url: str) -> str: - """Return a browser-safe image URL, proxying internal hosts through SoulSync.""" - if not url: - return url - - if url.startswith('/api/image-proxy?url='): - return url - - if url.startswith('http://') or url.startswith('https://'): - if _is_internal_image_host(url): - return f"/api/image-proxy?url={quote(url, safe='')}" - return url - - # Relative media-server paths should already have been expanded before this point. - return url - @app.route('/api/library/history') def get_library_history(): """Get persistent library history (downloads and server imports).""" @@ -27933,7 +27742,7 @@ def image_proxy(): 'img.discogs.com', 'i.discogs.com', # Discogs 'localhost', '127.0.0.1', 'host.docker.internal', # Local/Docker media servers ] - if not any(host == h or host.endswith('.' + h) for h in allowed_hosts) and not _is_internal_image_host(url): + if not any(host == h or host.endswith('.' + h) for h in allowed_hosts) and not is_internal_image_host(url): return '', 403 try: resp = requests.get(url, timeout=10, stream=True, headers={ @@ -32073,7 +31882,7 @@ def start_oauth_callback_servers(): _clear_rate_limit() spotify_client._invalidate_auth_cache() # Invalidate status cache so next poll picks up the new connection - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() # Refresh enrichment worker's client so it picks up new auth if spotify_enrichment_worker and hasattr(spotify_enrichment_worker, 'client'): spotify_enrichment_worker.client.reload_config() @@ -32086,7 +31895,7 @@ def start_oauth_callback_servers(): else: _oauth_logger.warning("Spotify token exchange succeeded but authentication validation failed") spotify_client._invalidate_auth_cache() - _status_cache_timestamps['spotify'] = 0 + invalidate_metadata_status_caches() add_activity_item("", "Spotify Auth Warning", "OAuth completed, but Spotify did not confirm an authenticated session", "Now") self.send_response(200) self.send_header('Content-type', 'text/html') @@ -34050,17 +33859,7 @@ 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 - spotify_data = dict(_status_cache.get('spotify', {})) - 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 - spotify_data['rate_limited'] = is_rl - spotify_data['rate_limit'] = rate_limit_info - if cooldown_remaining > 0: - spotify_data['post_ban_cooldown'] = cooldown_remaining + metadata_status = get_metadata_status_snapshot(spotify_client=spotify_client) # Count active downloads for nav badge active_dl_count = 0 @@ -34073,7 +33872,8 @@ def _build_status_payload(): pass return { - 'spotify': spotify_data, + 'metadata_source': metadata_status['metadata_source'], + 'spotify': metadata_status['spotify'], 'media_server': _status_cache.get('media_server', {}), 'soulseek': soulseek_data, 'active_media_server': config_manager.get_active_media_server(), @@ -34437,12 +34237,12 @@ def _emit_rate_monitor_loop(): # Add Spotify rate limit state try: - if spotify_client: - rl_info = spotify_client.get_rate_limit_info() - if rl_info: - payload['spotify']['rate_limited'] = True - payload['spotify']['rl_remaining'] = rl_info.get('remaining_seconds', 0) - payload['spotify']['rl_endpoint'] = rl_info.get('endpoint', '') + spotify_status = get_spotify_status(spotify_client=spotify_client) + rl_info = spotify_status.get('rate_limit') + if spotify_status.get('rate_limited') and rl_info: + payload['spotify']['rate_limited'] = True + payload['spotify']['rl_remaining'] = rl_info.get('remaining_seconds', 0) + payload['spotify']['rl_endpoint'] = rl_info.get('endpoint', '') except Exception: pass diff --git a/webui/index.html b/webui/index.html index 1b9c2694..57ef243f 100644 --- a/webui/index.html +++ b/webui/index.html @@ -279,17 +279,17 @@

Service Status

-
+
- Spotify + Metadata Source
-
+
- Plex + Media Server
-
+
- Soulseek + Download Source
@@ -616,22 +616,22 @@

Service Status

-
+
- Spotify + Metadata Source ● + id="metadata-source-status-indicator">●
-

Disconnected

-

Response: --

+

Disconnected

+

Response: --

+ onclick="testDashboardConnection(getActiveMetadataSource())">Test Connection
-
+
- Server + Media Server ●
@@ -642,9 +642,9 @@ Connection
-
+
- Soulseek + Download Source ●
diff --git a/webui/static/api-monitor.js b/webui/static/api-monitor.js index c900d824..01e27f0a 100644 --- a/webui/static/api-monitor.js +++ b/webui/static/api-monitor.js @@ -1837,6 +1837,19 @@ function startWatchlistCountdownTimer(initialSeconds) { } let remainingSeconds = initialSeconds; + const timerElement = document.getElementById('watchlist-next-auto-timer'); + const updateTimerText = (seconds) => { + if (!timerElement) return; + const countdownText = seconds > 0 ? formatCountdownTime(seconds) : ''; + timerElement.textContent = `Next Auto${countdownText ? ': ' + countdownText : ''}`; + }; + + // If there is no scheduled next run, don't keep polling the endpoint every second. + if (remainingSeconds <= 0) { + updateTimerText(0); + watchlistCountdownInterval = null; + return; + } watchlistCountdownInterval = setInterval(async () => { remainingSeconds--; @@ -1846,23 +1859,23 @@ function startWatchlistCountdownTimer(initialSeconds) { try { const response = await fetch('/api/watchlist/count'); const data = await response.json(); - remainingSeconds = data.next_run_in_seconds || 0; + const nextRunSeconds = data.next_run_in_seconds || 0; - const timerElement = document.getElementById('watchlist-next-auto-timer'); - if (timerElement) { - const countdownText = formatCountdownTime(remainingSeconds); - timerElement.textContent = `Next Auto${countdownText ? ': ' + countdownText : ''}`; + if (nextRunSeconds > 0) { + remainingSeconds = nextRunSeconds; + updateTimerText(remainingSeconds); + } else { + // No scheduled auto-run; stop polling until the page is refreshed + clearInterval(watchlistCountdownInterval); + watchlistCountdownInterval = null; + updateTimerText(0); } } catch (error) { console.debug('Error updating watchlist countdown:', error); } } else { // Update the display - const timerElement = document.getElementById('watchlist-next-auto-timer'); - if (timerElement) { - const countdownText = formatCountdownTime(remainingSeconds); - timerElement.textContent = `Next Auto${countdownText ? ': ' + countdownText : ''}`; - } + updateTimerText(remainingSeconds); } }, 1000); // Update every second } @@ -3791,4 +3804,3 @@ function cleanupSyncPageLogs() { // --- Global Cleanup on Page Unload --- // Note: Automatic wishlist processing now runs server-side and continues even when browser is closed // =============================== - diff --git a/webui/static/core.js b/webui/static/core.js index 3a23cb53..3852fda7 100644 --- a/webui/static/core.js +++ b/webui/static/core.js @@ -471,7 +471,7 @@ function initializeWebSocket() { function handleServiceStatusUpdate(data) { // Cache for library status card - _lastServiceStatus = data; + _lastStatusPayload = data; if (typeof syncSpotifySettingsAuthState === 'function') { syncSpotifySettingsAuthState(data?.spotify || null); @@ -484,11 +484,11 @@ function handleServiceStatusUpdate(data) { } // Same logic as fetchAndUpdateServiceStatus response handler - updateServiceStatus('spotify', data.spotify); + updateServiceStatus('metadata-source', data.metadata_source, data.spotify); updateServiceStatus('media-server', data.media_server); updateServiceStatus('soulseek', data.soulseek); - updateSidebarServiceStatus('spotify', data.spotify); + updateSidebarServiceStatus('metadata-source', data.metadata_source, data.spotify); updateSidebarServiceStatus('media-server', data.media_server); updateSidebarServiceStatus('soulseek', data.soulseek); @@ -881,8 +881,12 @@ const API = { } }; -// Track last service status for library card (used by websocket handler in core + artists) -let _lastServiceStatus = null; +// Track the last `/status` payload (shared service snapshot used across the UI) +let _lastStatusPayload = null; let _isSoulsyncStandalone = false; // Global flag: true when no media server (sync buttons hidden) +function getActiveMetadataSource() { + return _lastStatusPayload?.metadata_source?.source || 'spotify'; +} + // =============================== diff --git a/webui/static/helper.js b/webui/static/helper.js index 3afabac5..76088a39 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -173,7 +173,7 @@ const HELPER_CONTENT = { title: 'Support & Community', description: 'Links to the SoulSync community Discord, GitHub issues for bug reports, and documentation resources.', }, - '#spotify-indicator': { + '#metadata-source-indicator': { title: 'Metadata Source', description: 'Connection status of your primary metadata source. This service provides artist, album, and track information for searches, enrichment, and discovery.', tips: [ @@ -236,7 +236,7 @@ const HELPER_CONTENT = { // ─── DASHBOARD: SERVICE CARDS ─────────────────────────────────── - '#spotify-service-card': { + '#metadata-source-service-card': { title: 'Metadata Source Status', description: 'Detailed connection info for your active metadata source. Shows connection state, response latency, and allows manual connection testing.', tips: [ @@ -2375,7 +2375,7 @@ const HELPER_TOURS = { { page: 'dashboard', selector: '#wishlist-button', title: 'Wishlist', description: 'Tracks queued for download. Failed downloads, watchlist discoveries, and manual additions all land here for retry.' }, // Service cards - { page: 'dashboard', selector: '#spotify-service-card', title: 'Metadata Source', description: 'Shows your metadata source connection (Spotify, iTunes, or Deezer). This determines where album, artist, and track info comes from. Click "Test Connection" to verify.' }, + { page: 'dashboard', selector: '#metadata-source-service-card', title: 'Metadata Source', description: 'Shows your metadata source connection (Spotify, iTunes, or Deezer). This determines where album, artist, and track info comes from. Click "Test Connection" to verify.' }, { page: 'dashboard', selector: '#media-server-service-card', title: 'Media Server', description: 'Your media server (Plex, Jellyfin, or Navidrome). This is where your music library lives. SoulSync reads your collection and sends downloads here.' }, { page: 'dashboard', selector: '#soulseek-service-card', title: 'Download Source', description: 'Your primary download source status. In hybrid mode, shows the first source in your priority chain.' }, @@ -2974,13 +2974,13 @@ async function _checkSetupStatus() { const completion = _getSetupCompletion(); const results = { ...completion }; - // ── /status — checks services (spotify, media_server, soulseek) ───── + // ── /status — checks metadata_source, media_server, soulseek ──────── try { const resp = await fetch('/status'); if (resp.ok) { const data = await resp.json(); // Metadata source is available when status reports a source. - if (data.spotify?.source) { + if (data.metadata_source?.source) { results['metadata-source'] = results['metadata-source'] || Date.now(); _markSetupComplete('metadata-source'); } @@ -4336,7 +4336,7 @@ function _updateHelperBadge() { const TROUBLESHOOT_RULES = [ { - selector: '#spotify-service-card .status-dot.disconnected, #spotify-service-card .status-dot.error', + selector: '#metadata-source-service-card .service-card-indicator.disconnected, #metadata-source-service-card .service-card-indicator.error', title: 'Metadata Source Disconnected', steps: [ 'Go to Settings → Connections and verify your API credentials', @@ -4347,7 +4347,7 @@ const TROUBLESHOOT_RULES = [ action: { label: 'Open Settings', fn: () => navigateToPage('settings') } }, { - selector: '#media-server-service-card .status-dot.disconnected, #media-server-service-card .status-dot.error', + selector: '#media-server-service-card .service-card-indicator.disconnected, #media-server-service-card .service-card-indicator.error', title: 'Media Server Disconnected', steps: [ 'Check that your media server (Plex/Jellyfin/Navidrome) is running', @@ -4358,7 +4358,7 @@ const TROUBLESHOOT_RULES = [ action: { label: 'Open Settings', fn: () => navigateToPage('settings') } }, { - selector: '#soulseek-service-card .status-dot.disconnected, #soulseek-service-card .status-dot.error', + selector: '#soulseek-service-card .service-card-indicator.disconnected, #soulseek-service-card .service-card-indicator.error', title: 'Download Source Disconnected', steps: [ 'Verify your Soulseek/download client is running and reachable', diff --git a/webui/static/settings.js b/webui/static/settings.js index 8b9aa005..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 _lastServiceStatus?.spotify?.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(_lastServiceStatus?.spotify || null); + syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.spotify || null); } sanitizeMetadataSourceSelection({ quiet: true }); }); @@ -207,10 +207,10 @@ function initializeSettings() { // Test button event listeners removed - they use onclick attributes in HTML to avoid double firing if (typeof syncPrimaryMetadataSourceAvailability === 'function') { - syncPrimaryMetadataSourceAvailability(_lastServiceStatus?.spotify || null); + syncPrimaryMetadataSourceAvailability(_lastStatusPayload?.spotify || null); } - syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || null); - syncMetadataSourceSelection(_lastServiceStatus?.spotify?.source); + syncSpotifySettingsAuthState(_lastStatusPayload?.spotify || null); + syncMetadataSourceSelection(_lastStatusPayload?.metadata_source?.source); sanitizeMetadataSourceSelection({ quiet: true }); if (metadataSourceSelect) { metadataSourceSelect.dataset.lastValidSource = metadataSourceSelect.value; @@ -408,7 +408,7 @@ async function applyServiceStatusGradients() { else header.appendChild(spinner); } }); - syncSpotifySettingsAuthState(_lastServiceStatus?.spotify || 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 = _lastServiceStatus?.spotify?.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 6fed9d88..c9635f12 100644 --- a/webui/static/shared-helpers.js +++ b/webui/static/shared-helpers.js @@ -3143,7 +3143,7 @@ async function fetchAndUpdateServiceStatus() { const data = await response.json(); // Cache for library status card - _lastServiceStatus = data; + _lastStatusPayload = data; if (typeof syncSpotifySettingsAuthState === 'function') { syncSpotifySettingsAuthState(data?.spotify || null); @@ -3156,12 +3156,12 @@ async function fetchAndUpdateServiceStatus() { } // Update service status indicators and text (dashboard) - updateServiceStatus('spotify', data.spotify); + updateServiceStatus('metadata-source', data.metadata_source, data.spotify); updateServiceStatus('media-server', data.media_server); updateServiceStatus('soulseek', data.soulseek); // Update sidebar service status indicators - updateSidebarServiceStatus('spotify', data.spotify); + updateSidebarServiceStatus('metadata-source', data.metadata_source, data.spotify); updateSidebarServiceStatus('media-server', data.media_server); updateSidebarServiceStatus('soulseek', data.soulseek); @@ -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,37 @@ 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 serviceCard = document.getElementById(`${service}-service-card`); const indicator = document.getElementById(`${service}-status-indicator`); const statusText = document.getElementById(`${service}-status-text`); + if (serviceCard) { + serviceCard.dataset.statusReady = 'true'; + } + if (indicator && statusText) { - if (service === 'spotify') { - const presentation = getSpotifyStatusPresentation(statusData || {}); + if (service === 'metadata-source') { + const presentation = getMetadataSourcePresentation(statusData || {}, spotifyStatus || {}); indicator.className = `service-card-indicator ${presentation.statusClass}`; statusText.textContent = presentation.statusText; statusText.className = `service-card-status-text ${presentation.statusClass}`; @@ -3303,8 +3310,8 @@ function updateServiceStatus(service, statusData) { } // Update music source title based on active source - if (service === 'spotify' && statusData.source) { - const musicSourceTitleElement = document.getElementById('music-source-title'); + if (service === 'metadata-source' && statusData.source) { + const musicSourceTitleElement = document.getElementById('metadata-source-title'); if (musicSourceTitleElement) { const sourceName = getMetadataSourceLabel(statusData.source); musicSourceTitleElement.textContent = sourceName; @@ -3312,7 +3319,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 +3329,21 @@ function updateServiceStatus(service, statusData) { disconnectBtn.style.display = spotifySessionActive ? '' : 'none'; } - syncPrimaryMetadataSourceAvailability(statusData); + syncPrimaryMetadataSourceAvailability(spotifyStatus); + + const responseTimeElement = document.getElementById('metadata-source-response-time'); + if (responseTimeElement) { + const responseTime = statusData.response_time; + responseTimeElement.textContent = responseTime !== undefined && responseTime !== null + ? `Response: ${responseTime}ms` + : 'Response: --'; + } + + const testButton = document.querySelector('#metadata-source-service-card .service-card-button'); + if (testButton) { + const source = statusData.source || 'spotify'; + testButton.setAttribute('onclick', `testDashboardConnection('${source}')`); + } } // Update download source title on dashboard card @@ -3334,15 +3355,16 @@ function updateServiceStatus(service, statusData) { } } -function updateSidebarServiceStatus(service, statusData) { +function updateSidebarServiceStatus(service, statusData, spotifyStatus = null) { const indicator = document.getElementById(`${service}-indicator`); if (indicator) { + indicator.dataset.statusReady = 'true'; const dot = indicator.querySelector('.status-dot'); const nameElement = indicator.querySelector('.status-name'); if (dot) { - if (service === 'spotify') { - const presentation = getSpotifyStatusPresentation(statusData || {}); + if (service === 'metadata-source') { + const presentation = getMetadataSourcePresentation(statusData || {}, spotifyStatus || {}); dot.className = `status-dot ${presentation.dotClass}`; dot.title = presentation.dotTitle; } else { @@ -3361,8 +3383,8 @@ function updateSidebarServiceStatus(service, statusData) { } // Update music source name in sidebar based on active source - if (service === 'spotify' && statusData.source) { - const musicSourceNameElement = document.getElementById('music-source-name'); + if (service === 'metadata-source' && statusData.source) { + const musicSourceNameElement = document.getElementById('metadata-source-name'); if (musicSourceNameElement) { const sourceName = getMetadataSourceLabel(statusData.source); musicSourceNameElement.textContent = sourceName; diff --git a/webui/static/style.css b/webui/static/style.css index cbd549e2..1f42a95a 100644 --- a/webui/static/style.css +++ b/webui/static/style.css @@ -2820,6 +2820,19 @@ body.helper-mode-active #dashboard-activity-feed:hover { font-weight: 400; } +.status-indicator[data-status-ready="false"] .status-dot, +.status-indicator[data-status-ready="false"] .status-name, +.service-card[data-status-ready="false"] .service-card-title, +.service-card[data-status-ready="false"] .service-card-indicator, +.service-card[data-status-ready="false"] .service-card-status-text, +.service-card[data-status-ready="false"] .service-card-response-time { + visibility: hidden; +} + +#metadata-source-service-card[data-status-ready="false"] .service-card-button { + visibility: hidden; +} + /* ===================================== MAIN CONTENT AREA STYLING ===================================== */ diff --git a/webui/static/wishlist-tools.js b/webui/static/wishlist-tools.js index 3d40bb63..15766b5d 100644 --- a/webui/static/wishlist-tools.js +++ b/webui/static/wishlist-tools.js @@ -6558,8 +6558,8 @@ function updateLibraryStatusCard(dbStats) { const isScanning = window._libraryStatusScanning || false; // Determine state - const serverConnected = _lastServiceStatus && _lastServiceStatus.media_server && _lastServiceStatus.media_server.connected; - const serverType = _lastServiceStatus && _lastServiceStatus.active_media_server; + const serverConnected = _lastStatusPayload && _lastStatusPayload.media_server && _lastStatusPayload.media_server.connected; + const serverType = _lastStatusPayload && _lastStatusPayload.active_media_server; const hasData = tracks > 0; const hasServer = !!serverType && serverType !== 'none'; @@ -6668,7 +6668,7 @@ function updateLibraryStatusCard(dbStats) { } } -// _lastServiceStatus and _isSoulsyncStandalone are declared in core.js +// _lastStatusPayload and _isSoulsyncStandalone are declared in core.js const _origFetchServiceStatus = typeof fetchAndUpdateServiceStatus === 'function' ? fetchAndUpdateServiceStatus : null; function _capitalize(s) { return s ? s.charAt(0).toUpperCase() + s.slice(1) : ''; }