diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml index ce9eec30..54e62f40 100644 --- a/.github/workflows/docker-publish.yml +++ b/.github/workflows/docker-publish.yml @@ -9,9 +9,9 @@ on: workflow_dispatch: inputs: version_tag: - description: 'Version tag (e.g. 2.8.1)' + description: 'Version tag (e.g. 2.8.2)' required: true - default: '2.8.1' + default: '2.8.2' jobs: build-and-push: diff --git a/RELEASE_2.8.1_discord.md b/RELEASE_2.8.1_discord.md index 76feb510..5c80d8af 100644 --- a/RELEASE_2.8.1_discord.md +++ b/RELEASE_2.8.1_discord.md @@ -1,15 +1,15 @@ **SoulSync 2.8.1** is out π a feature + reliability release. -π§ **Export playlists to Spotify & Deezer** β the mirrored-playlist export now has **Sync to Spotify** and **Sync to Deezer** right next to the ListenBrainz / JSPF options. it builds a playlist in your account from the track IDs soulsync already has β the discovery cache first, then your library β so an already-discovered playlist exports **instantly with zero API calls**. re-exporting updates the same playlist instead of duplicating it, and an optional *"match missing tracks"* toggle confidently searches for the stragglers (a wrong-artist or karaoke version is left out, never guessed in). spotify needs a one-time reconnect for write access. (#945) +π§ **Export playlists to Spotify & Deezer** β the mirrored-playlist export now has **Sync to Spotify** and **Sync to Deezer** next to the ListenBrainz / JSPF options. it builds a playlist in your account from the IDs soulsync already has (the discovery cache first, then your library), so an already-discovered playlist exports **instantly with zero API calls**. re-exporting updates the same playlist instead of duplicating it, and an optional *"match missing tracks"* toggle confidently searches for the stragglers β a wrong-artist or karaoke version is left out, never guessed. the first Spotify export asks permission once. (#945) -π·οΈ **Library Reorganize β Rename only** β a lighter reorganize action that just **renames your files** to your current naming scheme: no re-tagging, no quality/AcoustID re-check, no copy-to-staging. much faster on a NAS, and only touches files whose path actually changes (which also fixes the "2 of 14 previewed but everything got modified" album-splitting). pick it from the new Action dropdown. (#875 β thanks @tsoulard / @Tacobell444) +π·οΈ **Library Reorganize β Rename only** β a lighter action that just **renames your files** to your naming scheme: no re-tag, no quality/AcoustID re-check, no copy-to-staging. much faster on a NAS, and only touches files whose path actually changes. pick it from the new Action dropdown. (#875 β thanks @tsoulard / @Tacobell444) -πΏ **Broader lossless handling** β lossy-copy now works for **all lossless formats**, not just FLAC (#941); and **DSD** (`.dsf`/`.dff`) is recognized as lossless instead of being false-flagged as "truncated" (#939). +πΏ **Broader lossless handling** β lossy-copy now covers **all lossless formats**, not just FLAC (#941); and **DSD** (`.dsf`/`.dff`) is recognized as lossless instead of false-flagged "truncated" (#939). -π **Download + search fixes** β an unbalanced bracket in a filename no longer false-fails as "file not found"; a file we couldn't quarantine is left for retry instead of deleted; "file not found" errors are actionable now; pasted Qobuz/Tidal links inject the exact track into manual search (#932); and the Wing It pool "Fix Match" search works again. +π **Download + search fixes** β an unbalanced bracket no longer false-fails as "file not found"; a file we couldn't quarantine is left for retry instead of deleted; "file not found" errors are actionable now; pasted Qobuz/Tidal links inject the exact track into manual search (#932); the Wing It pool "Fix Match" works again. -β‘ **Reduce visual effects, refined** β it no longer freezes functional motion (spinners, progress) and only kills the expensive GPU stuff (blur, shadows, glow). worker orbs default OFF on Firefox for new users and run at ~30fps under reduce-effects. plus jellyfin scans page the bulk fetch so the watchdog can't false-stall a big library. +β‘ **Reduce visual effects, refined** β it no longer freezes functional motion (spinners, progress), only the expensive GPU stuff (blur, shadows, glow). worker orbs default OFF on Firefox and run at ~30fps under reduce-effects. plus a jellyfin scan watchdog fix for big libraries. -π§ **Under the hood** β settings page cleanup (#943, thanks @nick2000713), spotify oauth hardening (#942, thanks HellRa1SeR), and npm audit security fixes for vite / undici / @babel (#944, thanks HellRa1SeR). +π§ **Under the hood** β settings cleanup (#943, @nick2000713), spotify oauth hardening (#942) + npm security fixes (#944, HellRa1SeR). enjoy! πΆ diff --git a/RELEASE_2.8.2_discord.md b/RELEASE_2.8.2_discord.md new file mode 100644 index 00000000..1d33ecf0 --- /dev/null +++ b/RELEASE_2.8.2_discord.md @@ -0,0 +1,9 @@ +**SoulSync 2.8.2** is out π a stability + performance release. + +π§ **Spotify, reliably** β the Docker boot hang is fixed: with Spotify as your primary source, an unreachable Spotify API could block startup so the container bound `:8008` but never served the UI. auth probes are now deferred during boot + capped with a timeout. the "re-auth didn't stick" bug is fixed too (the OAuth callback and the app were reading different token caches), and **Sync to Spotify** now works β it asks for playlist-write permission once, on-demand, leaving your normal login untouched. (#949 β thanks HellRa1SeR) + +β‘ **The "slow after update" fix** β the post-update lag wasn't SoulSync, it was browser password managers (Bitwarden/1Password/etc.) rebuilding their autofill overlay on *every* DOM change. non-credential fields are now marked so they skip them β **~110Γ less main-thread blocking** in the reporter's benchmark. plus a new **Max Performance** mode (Settings β Appearance) that kills every effect for no-GPU / Docker setups. (#948 β thanks @nick2000713) + +π₯ **Large-library imports no longer time out** β dropping a whole library into staging used to make the import page scan every file synchronously and never load. the scan runs in the background now with a live "Scanning N of Mβ¦" progress, and fills in when done. (#947 β thanks @ramonskie) + +enjoy! πΆ diff --git a/core/boot_phase.py b/core/boot_phase.py new file mode 100644 index 00000000..9d6853df --- /dev/null +++ b/core/boot_phase.py @@ -0,0 +1,27 @@ +"""Boot-phase guard for non-blocking container startup. + +While the gunicorn worker is importing ``web_server`` (module-level client and +worker initialization), external provider API probes must not block startup. +Network validation is deferred until ``mark_boot_complete()`` runs at the end +of that import pass. +""" + +from __future__ import annotations + +import threading + +_boot_lock = threading.Lock() +_boot_active = True + + +def is_boot_phase() -> bool: + """Return True while module import must avoid blocking provider API calls.""" + with _boot_lock: + return _boot_active + + +def mark_boot_complete() -> None: + """End the boot phase β provider clients may perform network probes again.""" + global _boot_active + with _boot_lock: + _boot_active = False diff --git a/core/deezer_download_client.py b/core/deezer_download_client.py index d08a9ef1..99fe4753 100644 --- a/core/deezer_download_client.py +++ b/core/deezer_download_client.py @@ -121,6 +121,7 @@ class DeezerDownloadClient(DownloadSourcePlugin): self._license_token = None self._user_data = None self._authenticated = False + self._pending_arl: Optional[str] = None # Quality preference self._quality = quality_tier_for_source('deezer', default='flac') @@ -128,7 +129,12 @@ class DeezerDownloadClient(DownloadSourcePlugin): # Try to authenticate on init if ARL is configured arl = config_manager.get('deezer_download.arl', '') if arl: - self._authenticate(arl) + from core.boot_phase import is_boot_phase + if is_boot_phase(): + self._pending_arl = arl + logger.debug("Deezer ARL present β authentication deferred until after boot") + else: + self._authenticate(arl) logger.info(f"Deezer download client initialized (download path: {self.download_path})") @@ -227,6 +233,11 @@ class DeezerDownloadClient(DownloadSourcePlugin): return self._authenticated def is_authenticated(self) -> bool: + if self._pending_arl and not self._authenticated: + from core.boot_phase import is_boot_phase + if not is_boot_phase(): + self._authenticate(self._pending_arl) + self._pending_arl = None return self._authenticated async def check_connection(self) -> bool: diff --git a/core/imports/routes.py b/core/imports/routes.py index 09043aed..77448c62 100644 --- a/core/imports/routes.py +++ b/core/imports/routes.py @@ -8,7 +8,7 @@ import time import uuid from concurrent.futures import as_completed from dataclasses import dataclass -from typing import Any, Callable, Dict +from typing import Any, Callable, Dict, Optional from core.imports.album import build_album_import_context, build_album_import_match_payload, resolve_album_artist_context from core.imports.context import get_import_context_artist, get_import_track_info, normalize_import_context @@ -84,12 +84,124 @@ class ImportRouteRuntime: _STAGING_SCAN_LOCK = threading.Lock() _STAGING_SCAN_TTL = 6.0 # seconds β covers the page-open burst; re-scans after _staging_scan_cache: Dict[str, Any] = {"path": None, "ts": 0.0, "records": None} +# Bumped by invalidate_staging_scan_cache() so a background scan that finishes after an +# import doesn't re-commit stale (pre-import) records (see the generation guard above). +_staging_scan_generation: Dict[str, int] = {"value": 0} + +# Background-scan plumbing: a large staging folder (whole-library migration, #947) makes +# the synchronous scan exceed gunicorn's 120s request timeout. The runner moves the SAME +# scan off the request thread; the endpoints report progress instead of blocking. +_staging_scan_status: Dict[str, Any] = { + "status": "idle", "scanned": 0, "total": 0, "path": None, "error": None, +} +_staging_scan_status_lock = threading.Lock() -def _scan_staging_records(runtime: ImportRouteRuntime, staging_path: str) -> list[Dict[str, Any]]: +def _staging_cache_hit(staging_path: str) -> Optional[list]: + """The cached records for ``staging_path`` if still fresh, else None (no scan triggered).""" + c = _staging_scan_cache + if (c["records"] is not None and c["path"] == staging_path + and (time.time() - c["ts"]) < _STAGING_SCAN_TTL): + return c["records"] + return None + + +def ensure_background_staging_scan(runtime: ImportRouteRuntime, staging_path: str) -> None: + """Start a background scan for ``staging_path`` unless the cache is warm or a scan for + this path is already running. Idempotent β safe to call on every request.""" + if _staging_cache_hit(staging_path) is not None: + return + with _staging_scan_status_lock: + if (_staging_scan_status["status"] == "scanning" + and _staging_scan_status["path"] == staging_path): + return + _staging_scan_status.update({"status": "scanning", "scanned": 0, "total": 0, + "path": staging_path, "error": None}) + + def _run() -> None: + try: + _scan_staging_records(runtime, staging_path, progress=_staging_scan_status) + with _staging_scan_status_lock: + if _staging_scan_status["path"] == staging_path: + _staging_scan_status["status"] = "done" + except Exception as exc: # noqa: BLE001 β surface any scan error to the poller + with _staging_scan_status_lock: + _staging_scan_status.update({"status": "error", "error": str(exc)}) + + threading.Thread(target=_run, name="staging-scan", daemon=True).start() + + +def get_staging_records_or_status(runtime: ImportRouteRuntime, staging_path: str, + *, grace_seconds: float = 3.0) -> tuple[str, Any]: + """Non-blocking staging access for the page endpoints. Returns ``("ready", records)`` + when the cache is warm or the scan completes within ``grace_seconds`` (so small/normal + folders still answer in a single request), otherwise ``("scanning", status_dict)`` after + making sure a background scan is running.""" + records = _staging_cache_hit(staging_path) + if records is not None: + return ("ready", records) + ensure_background_staging_scan(runtime, staging_path) + deadline = time.time() + max(0.0, grace_seconds) + while True: + records = _staging_cache_hit(staging_path) + if records is not None: + return ("ready", records) + with _staging_scan_status_lock: + status = dict(_staging_scan_status) + if status.get("status") == "error": + return ("error", status) + if time.time() >= deadline: + return ("scanning", status) + time.sleep(0.05) + + +def _records_or_scanning_payload(runtime: ImportRouteRuntime, staging_path: str): + """Shared helper for the page endpoints: returns ``(records, None)`` when the scan is + ready, or ``(None, payload)`` when a background scan is still running β the caller + returns that payload so the page polls + shows progress instead of blocking/timing out. + + A scan error is re-raised so the endpoint's own try/except logs + returns it exactly as + when the scan ran inline (preserves the existing error contract).""" + state, val = get_staging_records_or_status(runtime, staging_path) + if state == "error": + raise RuntimeError(val.get("error") or "staging scan failed") + if state == "scanning": + return None, {"success": True, "scanning": True, + "progress": {"scanned": val.get("scanned", 0), + "total": val.get("total", 0)}} + return val, None + + +def staging_scan_status(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: + """Lightweight, instant scan-progress poll for the page (no grace-wait, no file I/O) β + ``ready`` true once the cache is warm and the files/groups/hints calls will answer fast.""" + try: + staging_path = runtime.get_staging_path() + except Exception as exc: + return {"success": False, "error": str(exc)}, 500 + with _staging_scan_status_lock: + st = dict(_staging_scan_status) + return { + "success": True, + "ready": _staging_cache_hit(staging_path) is not None, + "status": st.get("status", "idle"), + "scanned": st.get("scanned", 0), + "total": st.get("total", 0), + "error": st.get("error"), + }, 200 + + +def _scan_staging_records(runtime: ImportRouteRuntime, staging_path: str, + *, progress: Optional[Dict[str, Any]] = None) -> list[Dict[str, Any]]: """Walk staging + read each audio file's tags ONCE, returning per-file records that staging files/groups/hints all derive from. Briefly cached + locked so the - page-open trio shares a single scan rather than each re-walking and re-reading.""" + page-open trio shares a single scan rather than each re-walking and re-reading. + + ``progress`` (optional, default None = unchanged behaviour) is a dict the scan + updates live with ``total`` (audio-file count, from a fast first pass) and ``scanned`` + (tag-reads done so far) so a background runner can report progress. A generation guard + keeps a scan that finishes AFTER an import (which bumped ``_staging_scan_generation``) + from committing stale records to the cache.""" now = time.time() cached = _staging_scan_cache if (cached["records"] is not None and cached["path"] == staging_path @@ -103,33 +215,50 @@ def _scan_staging_records(runtime: ImportRouteRuntime, staging_path: str) -> lis and (now - cached["ts"]) < _STAGING_SCAN_TTL): return cached["records"] - records: list[Dict[str, Any]] = [] + start_generation = _staging_scan_generation["value"] + + # Pass 1 (fast): collect the audio-file list β no tag I/O β so we know the total. + audio_files: list[tuple[str, str, Optional[str]]] = [] if os.path.isdir(staging_path): for root, _dirs, filenames in os.walk(staging_path): rel_dir = os.path.relpath(root, staging_path) top_folder = rel_dir.split(os.sep)[0] if rel_dir != "." else None for fname in filenames: - ext = os.path.splitext(fname)[1].lower() - if ext not in AUDIO_EXTENSIONS: - continue - full_path = os.path.join(root, fname) - rel_path = os.path.relpath(full_path, staging_path) - meta = runtime.read_staging_file_metadata(full_path, rel_path) - records.append({ - "filename": fname, "rel_path": rel_path, "full_path": full_path, - "extension": ext, "title": meta["title"], "album": meta["album"], - "artist": meta["artist"], "albumartist": meta["albumartist"], - "track_number": meta["track_number"], "disc_number": meta["disc_number"], - "top_folder": top_folder, - }) + if os.path.splitext(fname)[1].lower() in AUDIO_EXTENSIONS: + audio_files.append((root, fname, top_folder)) + if progress is not None: + progress["total"] = len(audio_files) + progress["scanned"] = 0 - _staging_scan_cache.update({"path": staging_path, "ts": time.time(), "records": records}) + # Pass 2 (slow): read each file's tags, updating progress as we go. + records: list[Dict[str, Any]] = [] + for root, fname, top_folder in audio_files: + full_path = os.path.join(root, fname) + rel_path = os.path.relpath(full_path, staging_path) + meta = runtime.read_staging_file_metadata(full_path, rel_path) + records.append({ + "filename": fname, "rel_path": rel_path, "full_path": full_path, + "extension": os.path.splitext(fname)[1].lower(), + "title": meta["title"], "album": meta["album"], + "artist": meta["artist"], "albumartist": meta["albumartist"], + "track_number": meta["track_number"], "disc_number": meta["disc_number"], + "top_folder": top_folder, + }) + if progress is not None: + progress["scanned"] += 1 + + # Generation guard: if an import invalidated the cache mid-scan, these records are + # stale β return them to this caller but do NOT commit them as the shared cache. + if _staging_scan_generation["value"] == start_generation: + _staging_scan_cache.update({"path": staging_path, "ts": time.time(), "records": records}) return records def invalidate_staging_scan_cache() -> None: """Drop the cached staging scan (call after an import moves/removes files so the - next files/groups/hints request reflects the new state immediately).""" + next files/groups/hints request reflects the new state immediately). Also bumps the + scan generation so an in-flight background scan won't re-commit pre-import records.""" + _staging_scan_generation["value"] += 1 _staging_scan_cache.update({"path": None, "ts": 0.0, "records": None}) @@ -139,6 +268,10 @@ def staging_files(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: staging_path = runtime.get_staging_path() os.makedirs(staging_path, exist_ok=True) + records, scanning = _records_or_scanning_payload(runtime, staging_path) + if scanning is not None: + return scanning, 200 + files = [ { "filename": r["filename"], @@ -151,7 +284,7 @@ def staging_files(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: "disc_number": r["disc_number"], "extension": r["extension"], } - for r in _scan_staging_records(runtime, staging_path) + for r in records ] files.sort(key=lambda f: f["filename"].lower()) @@ -168,8 +301,12 @@ def staging_groups(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: if not os.path.isdir(staging_path): return {"success": True, "groups": []}, 200 + records, scanning = _records_or_scanning_payload(runtime, staging_path) + if scanning is not None: + return scanning, 200 + album_groups = {} - for r in _scan_staging_records(runtime, staging_path): + for r in records: album = r["album"] artist = r["albumartist"] or r["artist"] if not album or not artist: @@ -215,9 +352,13 @@ def staging_hints(runtime: ImportRouteRuntime) -> tuple[Dict[str, Any], int]: if not os.path.isdir(staging_path): return {"success": True, "hints": []}, 200 + records, scanning = _records_or_scanning_payload(runtime, staging_path) + if scanning is not None: + return scanning, 200 + tag_albums = {} folder_hints = {} - for r in _scan_staging_records(runtime, staging_path): + for r in records: if r["top_folder"]: folder_hints[r["top_folder"]] = folder_hints.get(r["top_folder"], 0) + 1 diff --git a/core/metadata/registry.py b/core/metadata/registry.py index 6d90c40f..c9fad42a 100644 --- a/core/metadata/registry.py +++ b/core/metadata/registry.py @@ -357,10 +357,26 @@ def get_hydrabase_client(allow_fallback: bool = True, require_enabled: bool = Tr return None +def get_configured_primary_source() -> str: + """Return metadata.fallback_source as stored in config, without runtime downgrade. + + Unlike get_primary_source(), this never probes Spotify auth. Use at boot and + anywhere blocking network I/O must be avoided (gunicorn worker import, Docker + cold start when Spotify is unreachable). + """ + _default = METADATA_SOURCE_PRIORITY[0] + return _get_config_value("metadata.fallback_source", _default) or _default + + def get_primary_source(spotify_client_factory: Optional[MetadataClientFactory] = None) -> str: """Return configured primary metadata source.""" + from core.boot_phase import is_boot_phase + + if is_boot_phase(): + return get_configured_primary_source() + _default = METADATA_SOURCE_PRIORITY[0] - source = _get_config_value("metadata.fallback_source", _default) or _default + source = get_configured_primary_source() if source == "spotify": try: @@ -447,7 +463,19 @@ def get_primary_source_status( musicbrainz_client_factory: Optional[MetadataClientFactory] = None, ) -> Dict[str, Any]: """Return a generic status snapshot for the active primary metadata source.""" + from core.boot_phase import is_boot_phase + source = _get_config_value("metadata.fallback_source", "deezer") or "deezer" + if is_boot_phase(): + display_source = source + if source == "spotify" and _get_config_value("metadata.spotify_free", False): + display_source = "spotify_free" + return { + "source": display_source, + "connected": False, + "response_time": 0, + } + started = time.time() connected = False @@ -515,9 +543,13 @@ def get_client_for_source( musicbrainz_client_factory: Optional[MetadataClientFactory] = None, ): """Return exact client for a source, or None if unavailable.""" + from core.boot_phase import is_boot_phase + if source == "spotify": try: client = get_spotify_client(client_factory=spotify_client_factory) + if is_boot_phase(): + return client if client and getattr(client, "sp", None) else None if client and client.is_spotify_authenticated(): return client except Exception as e: diff --git a/core/metadata_service.py b/core/metadata_service.py index 7c390746..fd412a99 100644 --- a/core/metadata_service.py +++ b/core/metadata_service.py @@ -49,6 +49,7 @@ from core.metadata.registry import ( get_discogs_client, get_hydrabase_client, get_itunes_client, + get_configured_primary_source, get_primary_client, get_primary_source, get_primary_source_label, @@ -117,6 +118,7 @@ __all__ = [ "get_metadata_service", "get_musicmap_similar_artists", "get_primary_client", + "get_configured_primary_source", "get_primary_source", "get_primary_source_label", "get_spotify_client_for_profile", diff --git a/core/qobuz_client.py b/core/qobuz_client.py index f82e6da4..d7a9b169 100644 --- a/core/qobuz_client.py +++ b/core/qobuz_client.py @@ -164,6 +164,8 @@ class QobuzClient(DownloadSourcePlugin): def _restore_session(self): """Try to restore saved session from config.""" + from core.boot_phase import is_boot_phase + saved = config_manager.get('qobuz.session', {}) app_id = saved.get('app_id', '') app_secret = saved.get('app_secret', '') @@ -178,6 +180,10 @@ class QobuzClient(DownloadSourcePlugin): 'X-User-Auth-Token': self.user_auth_token, }) + if is_boot_phase(): + logger.info("Loaded Qobuz session from config (verification deferred until after boot)") + return + # Verify the token is still valid try: resp = self.session.get( diff --git a/core/spotify_client.py b/core/spotify_client.py index 66514c2d..7f59265e 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -17,12 +17,27 @@ logger = get_logger("spotify_client") # Single source of truth for the Spotify OAuth scope. Used by EVERY SpotifyOAuth # construction (the client, the per-profile registry, and all web_server callbacks) so # the authorize URL and token exchange can never request different scopes β a mismatch -# silently re-prompts or denies. The `playlist-modify-*` pair powers exporting a mirrored -# playlist back to Spotify (#945); existing users re-auth once to grant it. +# silently re-prompts or denies. +# +# IMPORTANT β do NOT add scopes here lightly. Spotipy's validate_token treats a cached +# token as invalid the moment the requested scope is no longer a subset of the token's +# granted scope, so GROWING this string invalidates EVERY existing user's token and forces +# a re-auth on upgrade. `playlist-modify-*` (for exporting a playlist back to Spotify, #945) +# was pulled back out for exactly that reason β it broke all Spotify users on upgrade. The +# Spotify export must request write access on-demand (incremental auth) instead. SPOTIFY_OAUTH_SCOPE = ( "user-library-read user-read-private playlist-read-private " - "playlist-read-collaborative user-read-email user-follow-read " - "playlist-modify-public playlist-modify-private" + "playlist-read-collaborative user-read-email user-follow-read" +) + +# The export scope = the normal login scope PLUS playlist write. Requested ONLY by the +# on-demand export-auth route (/auth/spotify/export) when a user chooses to export a playlist +# to Spotify β NEVER by the normal login. That's the whole safety property: the global scope +# above is unchanged, so no existing token is invalidated. The token Spotify returns from the +# export flow is a SUPERSET of the read scope, so it still passes the normal auth check +# (read β read+write) β one account, one token, just with write added for the opt-in user. +SPOTIFY_EXPORT_SCOPE = ( + SPOTIFY_OAUTH_SCOPE + " playlist-modify-public playlist-modify-private" ) @@ -791,6 +806,16 @@ class SpotifyClient: self._auth_cached_result = None self._auth_cache_time = 0 + def _has_cached_oauth_token(self) -> bool: + """Return True when a persisted OAuth token exists (no network I/O).""" + if self.sp is None: + return False + try: + cache_handler = getattr(self.sp.auth_manager, 'cache_handler', None) + return bool(cache_handler and cache_handler.get_cached_token() is not None) + except Exception: + return False + def is_spotify_authenticated(self) -> bool: """Check if Spotify client is specifically authenticated (not just iTunes fallback). Results are cached for 60 seconds to avoid excessive API calls. @@ -866,6 +891,26 @@ class SpotifyClient: logger.debug("publish_spotify_status cache hit: %s", e) return self._auth_cached_result + from core.boot_phase import is_boot_phase + if is_boot_phase(): + result = self._has_cached_oauth_token() + with self._auth_cache_lock: + 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=False, + rate_limit=None, + post_ban_cooldown=None, + ) + except Exception as e: + logger.debug("publish_spotify_status boot-phase: %s", e) + return result + # Cache miss β make API call outside the lock. # Safety: if there's no cached token, return False immediately. # Without this guard, spotipy's auth_manager will try to start an interactive @@ -897,7 +942,8 @@ class SpotifyClient: # Use a dedicated probe client (retries=0) so a 429 here propagates # immediately and we can detect long Retry-After bans. try: - probe = spotipy.Spotify(auth_manager=self.sp.auth_manager, retries=0) + probe = spotipy.Spotify( + auth_manager=self.sp.auth_manager, retries=0, requests_timeout=15) probe.current_user() result = True except Exception as e: @@ -1148,6 +1194,19 @@ class SpotifyClient: return playlists return [] + def has_write_scope(self) -> bool: + """True when the cached Spotify token carries playlist-modify (the export write scope). + The export endpoint uses this to decide whether to run, or to first send the user + through the on-demand export-auth flow. Fail-safe: any error β False (not authorized).""" + if self.sp is None: + return False + try: + cache_handler = getattr(self.sp.auth_manager, "cache_handler", None) + token = cache_handler.get_cached_token() if cache_handler else None + return "playlist-modify" in ((token or {}).get("scope") or "") + except Exception: + return False + def create_or_update_playlist(self, name, track_ids, *, existing_id=None, public=False, description=""): """Create a Spotify playlist owned by the authed user (or replace an existing diff --git a/core/tidal_client.py b/core/tidal_client.py index 4b542d3d..41dc7605 100644 --- a/core/tidal_client.py +++ b/core/tidal_client.py @@ -520,8 +520,18 @@ class TidalClient: return True - def is_authenticated(self): + def is_authenticated(self) -> bool: """Check if client is authenticated, refreshing expired tokens if possible""" + from core.boot_phase import is_boot_phase + + if is_boot_phase(): + if self.access_token and time.time() < self.token_expires_at: + return True + return bool(self.access_token and self.refresh_token) + + # Token still valid β no refresh needed. (Restored: #949 moved this short-circuit + # into the boot-phase branch only, so every post-boot call fell through to the + # refresh below β a constant silent-refresh loop on a perfectly valid token.) if self.access_token and time.time() < self.token_expires_at: return True diff --git a/core/tidal_download_client.py b/core/tidal_download_client.py index 89757d42..5d577419 100644 --- a/core/tidal_download_client.py +++ b/core/tidal_download_client.py @@ -134,6 +134,7 @@ class TidalDownloadClient(DownloadSourcePlugin): self._device_auth_future = None self._device_auth_link = None + self._boot_session_tokens: Optional[dict] = None # Engine reference is populated by set_engine() at registration # time. Until then dispatch returns None β orchestrator wires @@ -163,6 +164,14 @@ class TidalDownloadClient(DownloadSourcePlugin): expiry_time = saved.get('expiry_time', 0) if token_type and access_token: + from core.boot_phase import is_boot_phase + if is_boot_phase(): + self._boot_session_tokens = saved + logger.info( + "Loaded Tidal download session from config (verification deferred until after boot)" + ) + return + try: expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None @@ -181,6 +190,39 @@ class TidalDownloadClient(DownloadSourcePlugin): except Exception as e: logger.warning(f"Could not restore Tidal session: {e}") + def _complete_deferred_session(self) -> bool: + """Finish restoring a session that was deferred during boot.""" + pending = getattr(self, '_boot_session_tokens', None) + if not pending or tidalapi is None: + self._boot_session_tokens = None + return False + + if not self.session: + self.session = tidalapi.Session() + + token_type = pending.get('token_type', '') + access_token = pending.get('access_token', '') + refresh_token = pending.get('refresh_token', '') + expiry_time = pending.get('expiry_time', 0) + self._boot_session_tokens = None + + try: + expiry_dt = datetime.fromtimestamp(expiry_time, tz=timezone.utc) if expiry_time else None + restored = self.session.load_oauth_session( + token_type=token_type, + access_token=access_token, + refresh_token=refresh_token, + expiry_time=expiry_dt, + ) + if restored and self.session.check_login(): + logger.info("Restored Tidal download session from saved tokens") + self._save_session() + return True + logger.warning("Saved Tidal session tokens are invalid/expired") + except Exception as e: + logger.warning(f"Could not restore Tidal session: {e}") + return False + def _save_session(self): if not self.session: return @@ -192,6 +234,14 @@ class TidalDownloadClient(DownloadSourcePlugin): }) def is_authenticated(self) -> bool: + from core.boot_phase import is_boot_phase + if is_boot_phase(): + pending = getattr(self, '_boot_session_tokens', None) + return bool(pending and pending.get('access_token')) + + if getattr(self, '_boot_session_tokens', None): + return self._complete_deferred_session() + if not self.session: return False try: diff --git a/pr_description.md b/pr_description.md index ed1bdd35..2d1d9d7e 100644 --- a/pr_description.md +++ b/pr_description.md @@ -1,43 +1,21 @@ -# soulsync 2.8.1 β `dev` β `main` +# soulsync 2.8.2 β `dev` β `main` -a feature + reliability release. the headline is **export a mirrored playlist back to Spotify or Deezer** β same one-click flow as the listenbrainz export, now pointed at the streaming services. plus a **rename-only mode** for Library Reorganize, broader lossless handling, a pile of download fixes, and the reduce-visual-effects pass refined so it stops freezing functional motion. +a stability + performance release. the headline is **Spotify reliability** (the Docker boot hang and the "logged out / re-auth won't stick" issues are fixed), a big **performance** win that explains the "slow after update" reports, and **large-library imports** that no longer time out the import page. --- ## what's new -### π§ Export playlists to Spotify & Deezer (#945) -the mirrored-playlist export modal now has **Sync to Spotify** and **Sync to Deezer** next to the listenbrainz / jspf options. it builds a playlist in your account from the tracks soulsync already has the service IDs for: +### π§ Spotify reliability +- **Docker boot hang fixed (#949 β thanks HellRa1SeR)** β with Spotify set as your primary metadata source, an unreachable Spotify API could block the gunicorn worker during startup, so the container bound port 8008 but never actually served the Web UI. provider auth probes are now deferred during boot (and capped with a timeout), so startup can't hang on a slow Spotify. same guard added for Qobuz / Deezer / Tidal. +- **"re-auth didn't stick" fixed** β the OAuth callback wrote your token to one cache while the app read another, so re-authenticating could silently fail validation (and trip an `Address already in use` on the callback port). unified on one token store β re-auth takes effect now. +- **Sync to Spotify works** β exporting a mirrored playlist to Spotify now asks for playlist-write permission **once**, on-demand, the first time you use it. your normal Spotify login is untouched, so upgrading never forces a re-auth. -- resolves each track from what's already on hand first β the **discovery cache**, then your library's stored IDs β so for an already-discovered playlist it's instant and uses **zero API calls** -- re-exporting **updates the same playlist in place** instead of spawning duplicates -- an optional **"match missing tracks"** toggle does a confident live search for the stragglers β and only adds a match it's sure about (a wrong-artist or karaoke version is left out, never guessed) -- service buttons grey out + point you to Settings when that service isn't connected -- spotify needs a one-time reconnect to grant playlist-write access +### β‘ Performance β the "slow after update" fix +- **Password-manager autofill storm fixed (#948 β thanks @nick2000713)** β the real cause of the post-update lag wasn't SoulSync rendering, it was browser password managers (Bitwarden / 1Password / etc.) rebuilding their autofill overlay on **every** DOM change β and SoulSync mutates the DOM constantly (live status, progress bars, countdowns). non-credential fields are now marked so managers skip them (your login fields are left alone). the reporter measured **~110Γ less main-thread blocking** and ~20 β ~96 FPS. +- **Max Performance mode (new)** β Settings β Appearance. one switch kills the worker orbs, particles, all blur/shadows, and every animation/transition, and greys out the individual effect toggles so it's clearly in charge. for software-rendered / no-GPU setups (Docker, remote desktop) where even simple animations cost real CPU. -### π·οΈ Library Reorganize β Rename only (#875) -a lighter reorganize action: it just **renames your files** to your current naming scheme β no re-tagging, no quality/AcoustID re-check, no copy-to-staging. much faster on a NAS, won't fail on post-processing reasons, and only touches files whose path actually changes (which also fixes the "2 of 14 previewed but everything got modified" album-splitting). pick it from the new **Action** dropdown in the reorganize modal. - -### πΏ Lossless handling -- lossy-copy now works for **all lossless formats**, not just FLAC (#941) -- **DSD** (`.dsf` / `.dff`) is recognized as lossless and no longer false-flagged as "truncated" (#939) - -### π Download + search fixes -- a download with an unbalanced bracket in its name no longer false-fails as "file not found" -- a file we couldn't quarantine is left in place for retry instead of deleted -- the Identify search for single imports defaults to "artist - title" (with the dash) -- "file not found" failures now say what actually happened instead of an opaque error -- pasted Qobuz/Tidal links **inject the exact track** into manual search instead of hoping text-search surfaces it (#932) -- the Wing It pool "Fix Match" search works again (it was returning "no results" for everything) - -### β‘ Visual effects + scan reliability -- **Reduce visual effects** no longer freezes functional motion (spinners, progress) β it only kills the expensive GPU stuff (blur, shadows, glow) -- worker orbs default **OFF on Firefox** for new users, and run at ~30fps under reduce-effects -- jellyfin library scans page the bulk fetch so the no-progress watchdog can't false-stall a big library - -### π§ Under the hood -- settings page cleanup (#943 β thanks @nick2000713) -- spotify oauth credential normalization + redirect-uri handling (#942 β thanks HellRa1SeR) -- security: npm audit fixes for vite / undici / @babel (#944 β thanks HellRa1SeR) +### π₯ Large-library imports no longer time out (#947 β thanks @ramonskie) +- dropping a whole library into your staging folder used to make the import page scan every file synchronously and blow past the request timeout β so the page never loaded, and every reload re-timed-out. the scan now runs in the **background** with a live **"Scanning N of Mβ¦"** progress, and the page fills in automatically when it's done. (auto-import remains the hands-off path for the actual matching.) enjoy πΆ diff --git a/tests/imports/test_staging_scan_async.py b/tests/imports/test_staging_scan_async.py new file mode 100644 index 00000000..9a89900e --- /dev/null +++ b/tests/imports/test_staging_scan_async.py @@ -0,0 +1,163 @@ +"""Async/background staging scan (#947): a whole-library migration makes the synchronous +scan exceed gunicorn's 120s timeout. The runner moves the SAME scan off the request thread +with progress + a generation guard. Metadata reads are injected so no real audio is needed.""" + +import os +import time +import types + +import pytest + +import core.imports.routes as routes + + +def _meta(_full, _rel): + return {"title": "t", "album": "Alb", "artist": "Art", "albumartist": "Art", + "track_number": None, "disc_number": None} + + +def _runtime(read=_meta): + return types.SimpleNamespace(read_staging_file_metadata=read) + + +def _staging(tmp_path, n=3, subdir="Artist/Album"): + d = tmp_path / "staging" + for part in subdir.split("/"): + d = d / part + d.mkdir(parents=True, exist_ok=True) + for i in range(n): + (d / f"{i:02d}.flac").write_text("x") + return str(tmp_path / "staging") + + +@pytest.fixture(autouse=True) +def _reset(): + routes.invalidate_staging_scan_cache() + routes._staging_scan_status.update({"status": "idle", "scanned": 0, "total": 0, + "path": None, "error": None}) + yield + routes.invalidate_staging_scan_cache() + + +def _await_done(timeout=5.0): + end = time.time() + timeout + while time.time() < end and routes._staging_scan_status["status"] == "scanning": + time.sleep(0.03) + + +def test_scan_reports_progress(tmp_path): + sp = _staging(tmp_path, 3) + prog = {} + recs = routes._scan_staging_records(_runtime(), sp, progress=prog) + assert len(recs) == 3 + assert prog["total"] == 3 and prog["scanned"] == 3 + + +def test_default_scan_behaviour_unchanged(tmp_path): + sp = _staging(tmp_path, 2) + assert len(routes._scan_staging_records(_runtime(), sp)) == 2 # no progress arg = as before + + +def test_accessor_ready_for_small_folder(tmp_path): + sp = _staging(tmp_path, 2) + state, val = routes.get_staging_records_or_status(_runtime(), sp, grace_seconds=3.0) + assert state == "ready" and len(val) == 2 + + +def test_accessor_scanning_when_scan_exceeds_grace(tmp_path): + sp = _staging(tmp_path, 2) + + def slow(_f, _r): + time.sleep(0.4) + return _meta(_f, _r) + + state, val = routes.get_staging_records_or_status(_runtime(slow), sp, grace_seconds=0.1) + assert state == "scanning" + assert val["status"] == "scanning" and val["total"] in (0, 2) + _await_done() + + +def test_ensure_scan_is_idempotent(tmp_path): + sp = _staging(tmp_path, 2) + calls = {"n": 0} + + def counting(_f, _r): + calls["n"] += 1 + time.sleep(0.15) + return _meta(_f, _r) + + rt = _runtime(counting) + routes.ensure_background_staging_scan(rt, sp) + routes.ensure_background_staging_scan(rt, sp) # must NOT start a second scan + _await_done() + assert calls["n"] == 2 # 2 files read once, not 4 + + +def test_generation_guard_discards_stale_records(tmp_path): + sp = _staging(tmp_path, 2) + + def read_then_import(_f, _r): + routes.invalidate_staging_scan_cache() # simulate an import landing mid-scan + return _meta(_f, _r) + + recs = routes._scan_staging_records(_runtime(read_then_import), sp) + assert len(recs) == 2 # caller still gets its records + assert routes._staging_scan_cache["records"] is None # but stale set NOT committed to cache + + +def _full_runtime(staging_path, read=_meta): + return types.SimpleNamespace( + get_staging_path=lambda: staging_path, + read_staging_file_metadata=read, + logger=types.SimpleNamespace(error=lambda *a, **k: None), + ) + + +def test_helper_passes_records_through_when_ready(monkeypatch): + monkeypatch.setattr(routes, 'get_staging_records_or_status', + lambda rt, sp: ("ready", [{"x": 1}])) + records, scanning = routes._records_or_scanning_payload(_runtime(), "/x") + assert scanning is None and records == [{"x": 1}] + + +def test_helper_builds_scanning_payload(monkeypatch): + monkeypatch.setattr(routes, 'get_staging_records_or_status', + lambda rt, sp: ("scanning", {"scanned": 5, "total": 20, "status": "scanning"})) + records, scanning = routes._records_or_scanning_payload(_runtime(), "/x") + assert records is None + assert scanning == {"success": True, "scanning": True, + "progress": {"scanned": 5, "total": 20}} + + +def test_staging_files_endpoint_ready(tmp_path): + payload, status = routes.staging_files(_full_runtime(_staging(tmp_path, 2))) + assert status == 200 and payload["success"] and len(payload["files"]) == 2 + + +def test_staging_files_endpoint_returns_scanning(tmp_path, monkeypatch): + monkeypatch.setattr(routes, 'get_staging_records_or_status', + lambda r, p: ("scanning", {"scanned": 3, "total": 10})) + payload, status = routes.staging_files(_full_runtime(_staging(tmp_path, 2))) + assert status == 200 and payload.get("scanning") is True + assert payload["progress"] == {"scanned": 3, "total": 10} + + +def test_staging_groups_endpoint_returns_scanning(tmp_path, monkeypatch): + monkeypatch.setattr(routes, 'get_staging_records_or_status', + lambda r, p: ("scanning", {"scanned": 1, "total": 9})) + payload, status = routes.staging_groups(_full_runtime(_staging(tmp_path, 2))) + assert payload.get("scanning") is True + + +def test_scan_status_ready_after_warm(tmp_path): + sp = _staging(tmp_path, 2) + routes._scan_staging_records(_runtime(), sp) # warm the cache + payload, status = routes.staging_scan_status(_full_runtime(sp)) + assert status == 200 and payload["success"] and payload["ready"] is True + + +def test_scan_status_not_ready_when_cold(tmp_path): + sp = _staging(tmp_path, 2) # cold (autouse reset) + payload, _ = routes.staging_scan_status(_full_runtime(sp)) + assert payload["ready"] is False + assert "scanned" in payload and "total" in payload diff --git a/tests/metadata/test_boot_phase_providers.py b/tests/metadata/test_boot_phase_providers.py new file mode 100644 index 00000000..f67b468d --- /dev/null +++ b/tests/metadata/test_boot_phase_providers.py @@ -0,0 +1,77 @@ +"""Boot-phase guards must defer blocking provider network probes.""" + +from unittest.mock import MagicMock, patch + +from core.boot_phase import is_boot_phase, mark_boot_complete +from core.metadata import registry + + +def setup_function(): + mark_boot_complete() + + +def teardown_function(): + mark_boot_complete() + + +def test_get_primary_source_skips_spotify_probe_during_boot(monkeypatch): + import core.boot_phase as boot_phase + + boot_phase._boot_active = True + monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify") + + with patch.object(registry, "get_spotify_client") as get_client: + assert registry.get_primary_source() == "spotify" + get_client.assert_not_called() + + +def test_get_primary_source_status_skips_client_probe_during_boot(monkeypatch): + import core.boot_phase as boot_phase + + boot_phase._boot_active = True + monkeypatch.setattr( + registry, "_get_config_value", + lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default, + ) + + with patch.object(registry, "get_client_for_source") as get_client: + status = registry.get_primary_source_status() + get_client.assert_not_called() + assert status["source"] == "spotify" + assert status["connected"] is False + + +def test_spotify_auth_uses_token_presence_only_during_boot(monkeypatch): + import core.boot_phase as boot_phase + from core.spotify_client import SpotifyClient + + boot_phase._boot_active = True + client = SpotifyClient.__new__(SpotifyClient) + client.sp = MagicMock() + client._auth_cache_lock = __import__('threading').Lock() + client._auth_cached_result = None + client._auth_cache_time = 0 + client._AUTH_CACHE_TTL = 900 + + monkeypatch.setattr(client, "_has_cached_oauth_token", lambda: True) + + with patch("spotipy.Spotify") as spotify_cls: + assert client.is_spotify_authenticated() is True + spotify_cls.assert_not_called() + + +def test_deezer_download_defers_arl_auth_during_boot(monkeypatch): + import core.boot_phase as boot_phase + from core.deezer_download_client import DeezerDownloadClient + + boot_phase._boot_active = True + monkeypatch.setattr( + "config.settings.config_manager.get", + lambda key, default=None: "fake-arl" if key == "deezer_download.arl" else default, + ) + + with patch.object(DeezerDownloadClient, "_authenticate") as authenticate: + client = DeezerDownloadClient(download_path="/tmp/deezer-test") + authenticate.assert_not_called() + assert client._pending_arl == "fake-arl" + assert client.is_authenticated() is False diff --git a/tests/metadata/test_configured_primary_source.py b/tests/metadata/test_configured_primary_source.py new file mode 100644 index 00000000..2154673c --- /dev/null +++ b/tests/metadata/test_configured_primary_source.py @@ -0,0 +1,32 @@ +"""Tests for boot-safe configured primary source lookup.""" + +from unittest.mock import MagicMock, patch + +from core.boot_phase import mark_boot_complete +from core.metadata import registry + + +def setup_function(): + mark_boot_complete() + + +def test_get_configured_primary_source_reads_config_without_auth_probe(monkeypatch): + monkeypatch.setattr( + registry, + "_get_config_value", + lambda key, default=None: "spotify" if key == "metadata.fallback_source" else default, + ) + + with patch.object(registry, "get_spotify_client") as get_client: + assert registry.get_configured_primary_source() == "spotify" + get_client.assert_not_called() + + +def test_get_primary_source_still_downgrades_unauthenticated_spotify(monkeypatch): + monkeypatch.setattr(registry, "get_configured_primary_source", lambda: "spotify") + + spotify = MagicMock() + spotify.is_spotify_authenticated.return_value = False + monkeypatch.setattr(registry, "get_spotify_client", lambda **_: spotify) + + assert registry.get_primary_source() == registry.METADATA_SOURCE_PRIORITY[0] diff --git a/tests/test_service_playlist_export.py b/tests/test_service_playlist_export.py index e9bc7b8d..056a80fd 100644 --- a/tests/test_service_playlist_export.py +++ b/tests/test_service_playlist_export.py @@ -128,3 +128,27 @@ def test_spotify_backfill_search_disables_cross_service_fallback(monkeypatch): assert fn is not None fn('Kendrick Lamar', 'Not Like Us') # drives the search assert seen['allow_fallback'] is False + + +def test_spotify_export_endpoint_demands_auth_when_no_write_scope(monkeypatch): + """The export endpoint must return needs_auth (not start a doomed job) when the Spotify + token lacks write scope β and it must short-circuit BEFORE touching the DB.""" + import types + monkeypatch.setattr(ws, 'spotify_client', + types.SimpleNamespace(has_write_scope=lambda: False)) + resp = ws.app.test_client().post('/api/playlists/5/export/service/spotify') + data = resp.get_json() + assert data['needs_auth'] is True + assert data['auth_url'] == '/auth/spotify/export' + assert data['success'] is False + + +def test_spotify_export_endpoint_proceeds_when_write_scope_present(monkeypatch): + """With write scope, the spotify path must NOT short-circuit on needs_auth (it goes on to + start a job β here it just must not be a needs_auth response).""" + import types + monkeypatch.setattr(ws, 'spotify_client', + types.SimpleNamespace(has_write_scope=lambda: True)) + resp = ws.app.test_client().post('/api/playlists/999999/export/service/spotify') + data = resp.get_json() + assert not data.get('needs_auth') diff --git a/tests/test_spotify_client.py b/tests/test_spotify_client.py index 4dc18dfb..2cbb3e01 100644 --- a/tests/test_spotify_client.py +++ b/tests/test_spotify_client.py @@ -164,3 +164,77 @@ def test_insufficient_scope_says_reconnect(): raise Exception('403 Forbidden: insufficient client scope') res = _spotify_with(_ScopeErr()).create_or_update_playlist('X', ['a']) assert not res['success'] and 'Reconnect Spotify' in res['error'] + + +# ββ Spotify auth regression hotfix: scope must not force re-auth; callbacks must write +# the DB store the client reads (else a re-auth never takes effect) ββ + +import os as _os + + +def test_oauth_scope_has_no_write_scope_that_forces_reauth(): + """Spotipy invalidates a cached token the moment the requested scope stops being a subset + of the token's granted scope β so GROWING the global scope forces every user to re-auth on + upgrade (it broke all Spotify users). The write scope (playlist-modify) must NOT live in the + global scope; request it on-demand instead.""" + from core.spotify_client import SPOTIFY_OAUTH_SCOPE + assert 'playlist-modify' not in SPOTIFY_OAUTH_SCOPE + # the read scopes existing tokens already carry must stay + for s in ('user-library-read', 'user-read-private', 'playlist-read-private', + 'playlist-read-collaborative', 'user-read-email', 'user-follow-read'): + assert s in SPOTIFY_OAUTH_SCOPE + + +def test_global_oauth_callbacks_use_db_token_cache_not_file(): + """The OAuth callbacks wrote the new token to the legacy file cache while the client reads + DatabaseTokenCache, so a re-auth never reached the client ("validation failed" despite a good + exchange). The global callbacks must write the same DB-backed store the client uses.""" + root = _os.path.dirname(_os.path.dirname(_os.path.abspath(__file__))) + src = open(_os.path.join(root, 'web_server.py'), encoding='utf-8').read() + assert "cache_path='config/.spotify_cache'" not in src # no global file-cache writes + assert src.count('cache_handler=DatabaseTokenCache(config_manager)') >= 2 + + +# ββ on-demand Spotify export write-auth (#945 follow-up) ββ + +def test_export_scope_is_global_plus_write_and_global_stays_readonly(): + """The export scope adds playlist-modify ON TOP of the unchanged global scope. Critically + the GLOBAL scope must NOT gain write (that's what force-invalidated everyone's token).""" + from core.spotify_client import SPOTIFY_OAUTH_SCOPE, SPOTIFY_EXPORT_SCOPE + assert 'playlist-modify' not in SPOTIFY_OAUTH_SCOPE # global stays read-only + assert 'playlist-modify-public' in SPOTIFY_EXPORT_SCOPE + assert 'playlist-modify-private' in SPOTIFY_EXPORT_SCOPE + # export scope is a strict superset of the global read scope + assert set(SPOTIFY_OAUTH_SCOPE.split()).issubset(set(SPOTIFY_EXPORT_SCOPE.split())) + + +class _CacheHandler: + def __init__(self, token): + self._token = token + + def get_cached_token(self): + return self._token + + +def _client_with_token(token): + import types + c = _SpotifyClient.__new__(_SpotifyClient) + c.sp = types.SimpleNamespace(auth_manager=types.SimpleNamespace(cache_handler=_CacheHandler(token))) + return c + + +def test_has_write_scope_true_when_token_carries_playlist_modify(): + tok = {'scope': 'user-library-read playlist-modify-public playlist-read-private'} + assert _client_with_token(tok).has_write_scope() is True + + +def test_has_write_scope_false_for_readonly_token_or_missing(): + assert _client_with_token({'scope': 'user-library-read playlist-read-private'}).has_write_scope() is False + assert _client_with_token(None).has_write_scope() is False # no cached token + assert _client_with_token({}).has_write_scope() is False # token w/o scope field + + +def test_has_write_scope_false_when_no_client(): + c = _SpotifyClient.__new__(_SpotifyClient) + c.sp = None + assert c.has_write_scope() is False diff --git a/tests/test_tidal_token_refresh_loop.py b/tests/test_tidal_token_refresh_loop.py new file mode 100644 index 00000000..f62da188 --- /dev/null +++ b/tests/test_tidal_token_refresh_loop.py @@ -0,0 +1,46 @@ +"""Regression: post-boot, is_authenticated() must NOT refresh a still-valid Tidal token. + +#949 moved the "token still valid -> return True" short-circuit into the boot-phase branch +only, so every post-boot call fell through to the silent refresh β a constant-refresh loop +(wolf's logs: "access token expired -> refresh -> success" every few seconds).""" + +import time + +import core.boot_phase as boot_phase +from core.tidal_client import TidalClient + + +def _client(expires_at): + c = TidalClient.__new__(TidalClient) + c.access_token = "tok" + c.refresh_token = "refresh" + c.token_expires_at = expires_at + c._refresh_calls = 0 + + def _fake_refresh(): + c._refresh_calls += 1 + return True + + c._refresh_access_token = _fake_refresh + return c + + +def test_valid_token_does_not_refresh_post_boot(monkeypatch): + monkeypatch.setattr(boot_phase, "_boot_active", False) # post-boot + c = _client(time.time() + 3600) # valid for an hour + assert c.is_authenticated() is True + assert c._refresh_calls == 0 # MUST NOT refresh a valid token + + +def test_expired_token_still_refreshes_post_boot(monkeypatch): + monkeypatch.setattr(boot_phase, "_boot_active", False) + c = _client(time.time() - 10) # expired + assert c.is_authenticated() is True + assert c._refresh_calls == 1 # expired -> one refresh + + +def test_valid_token_returns_true_during_boot(monkeypatch): + monkeypatch.setattr(boot_phase, "_boot_active", True) # boot phase + c = _client(time.time() + 3600) + assert c.is_authenticated() is True + assert c._refresh_calls == 0 # boot never probes/refreshes diff --git a/web_server.py b/web_server.py index 83e88a87..96033fbf 100644 --- a/web_server.py +++ b/web_server.py @@ -40,7 +40,7 @@ logger = setup_logging(_log_level, _log_path) # App version β single source of truth for backup metadata, system-info, update check, etc. # Semver: MAJOR.MINOR.PATCH. Bump at each devβmain release. -_SOULSYNC_BASE_VERSION = "2.8.1" +_SOULSYNC_BASE_VERSION = "2.8.2" def _build_version_string(): """Append short commit hash to version when available (e.g. 2.35+abc1234).""" @@ -177,6 +177,7 @@ from core.imports.routes import singles_process as _import_singles_process from core.imports.routes import staging_files as _import_staging_files from core.imports.routes import staging_groups as _import_staging_groups from core.imports.routes import staging_hints as _import_staging_hints +from core.imports.routes import staging_scan_status as _import_staging_scan_status from core.imports.routes import staging_suggestions as _import_staging_suggestions from core.imports.paths import build_final_path_for_track as _build_final_path_for_track from core.imports.pipeline import build_import_pipeline_runtime as _build_import_pipeline_runtime @@ -387,6 +388,7 @@ def _initial_appearance_context(): _request_is_firefox(), ) reduce_effects = config_manager.get('ui_appearance.reduce_effects', False) is True + max_performance = config_manager.get('ui_appearance.max_performance', False) is True r, g, b = _hex_to_rgb(accent) hue, saturation, lightness = _rgb_to_hsl(r, g, b) light = _hsl_to_rgb(hue, saturation, min(lightness + 0.16, 0.95)) @@ -399,6 +401,7 @@ def _initial_appearance_context(): 'initial_particles_enabled': particles_enabled, 'initial_worker_orbs_enabled': worker_orbs_enabled, 'initial_reduce_effects': reduce_effects, + 'initial_max_performance': max_performance, } @@ -4940,6 +4943,38 @@ def auth_spotify(): logger.error(f"Error starting Spotify auth: {e}") return f"
{str(e)}
", 500 + +@app.route('/auth/spotify/export') +def auth_spotify_export(): + """On-demand authorization for Spotify playlist EXPORT (#945). + + Requests the export scope (the normal read scope + playlist-modify) so the user can write + a playlist to their Spotify β WITHOUT changing the global login scope, so no existing + token is invalidated. Spotify returns a superset token; the normal /callback exchanges and + stores it unchanged (read β read+write keeps the standard auth check happy). show_dialog + forces the consent screen so the new write permission is actually granted.""" + try: + from spotipy.oauth2 import SpotifyOAuth + from core.spotify_client import normalize_spotify_oauth_config, SPOTIFY_EXPORT_SCOPE + from core.spotify_token_cache import DatabaseTokenCache + cfg = normalize_spotify_oauth_config(config_manager.get_spotify_config()) + if not cfg.get('client_id') or not cfg.get('client_secret'): + return "Add your Spotify app credentials in Settings first.
", 400 + auth_manager = SpotifyOAuth( + client_id=cfg['client_id'], + client_secret=cfg['client_secret'], + redirect_uri=cfg.get('redirect_uri', 'http://127.0.0.1:8888/callback'), + scope=SPOTIFY_EXPORT_SCOPE, + cache_handler=DatabaseTokenCache(config_manager), + show_dialog=True, + ) + add_activity_item("", "Spotify Export Auth", "Requesting permission to create playlists", "Now") + return redirect(auth_manager.get_authorize_url()) + except Exception as e: + logger.error(f"Error starting Spotify export auth: {e}") + return f"{str(e)}
", 500 + + @app.route('/auth/tidal') def auth_tidal(): """ @@ -5229,12 +5264,16 @@ def spotify_callback(): configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") logger.info(f"Using redirect_uri for token exchange: {configured_uri}") + # Write the freshly-exchanged token to the SAME store the client reads + # (DatabaseTokenCache), not the legacy file β otherwise a re-auth never reaches the + # client and "validation failed" even though the exchange succeeded. + from core.spotify_token_cache import DatabaseTokenCache auth_manager = SpotifyOAuth( client_id=config['client_id'], client_secret=config['client_secret'], redirect_uri=configured_uri, scope=SPOTIFY_OAUTH_SCOPE, - cache_path='config/.spotify_cache' + cache_handler=DatabaseTokenCache(config_manager) ) token_info = auth_manager.get_access_token(auth_code) @@ -27916,6 +27955,15 @@ def start_playlist_export_service(playlist_id, service): service = (service or '').lower() if service not in ('spotify', 'deezer'): return jsonify({"success": False, "error": f"Unsupported export target: {service}"}), 400 + # Spotify export needs the write scope, which the normal login doesn't request. If the + # current token doesn't carry it, tell the UI to send the user through the one-time + # on-demand export-auth (instead of starting a job that would 403). #945. + if service == 'spotify' and not (spotify_client and spotify_client.has_write_scope()): + return jsonify({ + "success": False, "needs_auth": True, + "auth_url": "/auth/spotify/export", + "error": "Spotify needs permission to create playlists", + }), 200 body = request.get_json(silent=True) or {} backfill = bool(body.get('backfill')) db = get_database() @@ -36111,13 +36159,17 @@ def start_oauth_callback_servers(): configured_uri = config.get('redirect_uri', "http://127.0.0.1:8888/callback") _oauth_logger.info(f"Using redirect_uri for token exchange: {configured_uri}") - # Create auth manager and exchange code for token + # Create auth manager and exchange code for token. Use the SAME store + # the client reads (DatabaseTokenCache), not the legacy file β a re-auth + # written to the file never reaches the DB-backed client, so it would + # report "validation failed" despite a successful exchange. + from core.spotify_token_cache import DatabaseTokenCache auth_manager = SpotifyOAuth( client_id=config['client_id'], client_secret=config['client_secret'], redirect_uri=configured_uri, scope=SPOTIFY_OAUTH_SCOPE, - cache_path='config/.spotify_cache' + cache_handler=DatabaseTokenCache(config_manager) ) # Extract the authorization code and exchange it for tokens @@ -36483,11 +36535,13 @@ except Exception as e: # they explicitly want background Spotify enrichment. spotify_enrichment_worker = None try: - from core.metadata_service import get_primary_source as _get_primary_source + from core.metadata_service import get_configured_primary_source from database.music_database import MusicDatabase spotify_enrichment_db = MusicDatabase() spotify_enrichment_worker = SpotifyWorker(database=spotify_enrichment_db) - _primary = _get_primary_source() + # Use configured source only β get_primary_source() probes Spotify auth and can + # block gunicorn worker boot indefinitely when the API is unreachable. + _primary = get_configured_primary_source() _user_paused = config_manager.get('spotify_enrichment_paused', False) if _user_paused or _primary != 'spotify': spotify_enrichment_worker.paused = True # Set BEFORE start() to prevent race condition @@ -37649,6 +37703,12 @@ def import_staging_groups(): return jsonify(payload), status +@app.route('/api/import/staging/scan-status', methods=['GET']) +def import_staging_scan_status(): + payload, status = _import_staging_scan_status(_build_import_route_runtime()) + return jsonify(payload), status + + @app.route('/api/import/staging/hints', methods=['GET']) def import_staging_hints(): payload, status = _import_staging_hints(_build_import_route_runtime()) @@ -38967,6 +39027,11 @@ def start_runtime_services(): _runtime_started = True +# Module import is complete β provider clients may now perform network probes. +from core.boot_phase import mark_boot_complete +mark_boot_complete() + + # Direct execution: python web_server.py (dev/Windows fallback) # Production should use: gunicorn -c gunicorn.conf.py wsgi:application if _DIRECT_RUN: diff --git a/webui/index.html b/webui/index.html index 5e5a1580..5565a794 100644 --- a/webui/index.html +++ b/webui/index.html @@ -24,11 +24,12 @@ window._particlesEnabled = {{ initial_particles_enabled|tojson }}; window._workerOrbsEnabled = {{ initial_worker_orbs_enabled|tojson }}; window._reduceEffectsActive = {{ initial_reduce_effects|tojson }}; + window._maxPerfActive = {{ initial_max_performance|tojson }}; {{ vite_assets('head')|safe }} - + diff --git a/webui/src/routes/import/-import.types.ts b/webui/src/routes/import/-import.types.ts index 8828a3c5..73821b08 100644 --- a/webui/src/routes/import/-import.types.ts +++ b/webui/src/routes/import/-import.types.ts @@ -23,11 +23,21 @@ export interface ImportStagingFile { manual_match?: ImportTrackResult; } +/** While a large staging folder is still being scanned in the background (#947), the + * staging endpoints return `scanning: true` + progress instead of files/groups; the query + * polls until the scan completes and real data arrives. */ +export interface ImportScanProgress { + scanned: number; + total: number; +} + export interface ImportStagingFilesPayload { success: boolean; files?: ImportStagingFile[]; staging_path?: string; error?: string; + scanning?: boolean; + progress?: ImportScanProgress; } export interface ImportStagingGroup { @@ -47,6 +57,8 @@ export interface ImportStagingGroupsPayload { success: boolean; groups?: ImportStagingGroup[]; error?: string; + scanning?: boolean; + progress?: ImportScanProgress; } export interface ImportAlbumResult { diff --git a/webui/src/routes/import/-route.test.tsx b/webui/src/routes/import/-route.test.tsx index b8572b95..afd184e4 100644 --- a/webui/src/routes/import/-route.test.tsx +++ b/webui/src/routes/import/-route.test.tsx @@ -250,6 +250,19 @@ describe('import route', () => { expect(await screen.findByText('Import folder: error')).toBeInTheDocument(); }); + it('shows scan progress while a large staging folder is still scanning (#947)', async () => { + server.use( + http.get('/api/import/staging/files', () => + HttpResponse.json({ success: true, scanning: true, progress: { scanned: 5, total: 20 } }), + ), + ); + + renderImportRoute(); + + expect(await screen.findByTestId('import-page')).toBeInTheDocument(); + expect(await screen.findByText(/Scanning 5 of 20 files/)).toBeInTheDocument(); + }); + it('stores the active tab in nested route paths', async () => { const { history } = renderImportRoute(); diff --git a/webui/src/routes/import/-ui/import-page.tsx b/webui/src/routes/import/-ui/import-page.tsx index 840c8d5e..b33eadfe 100644 --- a/webui/src/routes/import/-ui/import-page.tsx +++ b/webui/src/routes/import/-ui/import-page.tsx @@ -21,17 +21,24 @@ import { fallbackImage, RefreshIcon, useImportStaging } from './import-shared'; export function ImportPage() { useReactPageShell('import'); - const { refreshStaging, stagingFiles, stagingPath, stagingQuery } = useImportStaging(); + const { refreshStaging, scanning, scanProgress, stagingFiles, stagingPath, stagingQuery } = + useImportStaging(); const isRefreshing = stagingQuery.isRefetching; const lastRefreshedAt = stagingQuery.dataUpdatedAt > 0 ? formatShortTime(stagingQuery.dataUpdatedAt) : null; + // While a large staging folder is still scanning (#947), show progress instead of a count. + const fileCountText = scanning + ? scanProgress && scanProgress.total > 0 + ? `Scanning ${scanProgress.scanned} of ${scanProgress.total} filesβ¦` + : 'Scanning staging folderβ¦' + : getStagingStatsText(stagingFiles); return (