From b0e7dae7c6d130d5338e39272db0f5b71446c94c Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Sun, 26 Apr 2026 21:16:41 -0700 Subject: [PATCH 1/2] Cache static assets 1y + cache discover GETs 5min MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses #365 (reported by JohnBaumb), parts 1 & 2 of the proposal. Service worker, client-side IDB/sessionStorage, and PWA manifest deferred to follow-up PRs. 1. Static asset cache (CSS/JS/icons/fonts). `SEND_FILE_MAX_AGE_DEFAULT` flipped from 0 to 31536000 (1 year) in production. Safe because every static URL is bust-tagged with `?v=static_v` (computed once per process start), so each server restart effectively invalidates every cached asset for every user. Within a single deploy, repeat page loads hit zero round-trips on static files — was a 304 round-trip per asset before. Dev override (`SOULSYNC_WEB_DEV_NO_CACHE=1`) keeps it at 0 so iterating on JS/CSS doesn't need a server restart between edits. Collateral fixes from the bump: - Music streaming endpoint (L16140): `response.headers.add('Cache-Control', 'no-cache')` → bracket-assign. Under the old max-age=0, send_file set `no-cache` and `.add()` duplicated harmlessly. Under the new max-age=31536000, `.add()` would APPEND a second Cache-Control value → two conflicting headers, browser-undefined behavior. Bracket-assign replaces. - Backup download endpoint (L25181): explicit `Cache-Control: no-store` on the response so DB backups don't inherit the new long max-age — sensitive content, must never cache. 2. Discover GET browser cache (5 min). New `@app.after_request` hook scoped to `/api/discover/` and `/api/discovery/` paths, GET method, 2xx responses only. Sets `Cache-Control: public, max-age=300`. Skipped when the endpoint already set its own Cache-Control. Toggling between Discover sections within 5 min serves from browser cache, no backend hit. Try/except wraps the hook body and logs a warning if anything throws — never let a header-tagging bug turn a successful response into a 500. (Logging instead of `pass` since silent except-pass is exactly the anti-pattern issue #369 is about.) Audited every other Cache-Control set site in web_server.py — only the two `send_file` callers needed adjustment. Range-branch streaming uses `Response()` directly, unaffected by the config change. 603 tests pass. --- web_server.py | 60 ++++++++++++++++++++++++++++++++++++------ webui/static/helper.js | 1 + 2 files changed, 53 insertions(+), 8 deletions(-) diff --git a/web_server.py b/web_server.py index 24d55d90..6d500ffe 100644 --- a/web_server.py +++ b/web_server.py @@ -171,12 +171,17 @@ app = Flask( ) app.config['TEMPLATES_AUTO_RELOAD'] = DEV_STATIC_NO_CACHE app.jinja_env.auto_reload = DEV_STATIC_NO_CACHE -# Force static assets (library.js / style.css / etc.) to revalidate -# with ETag on every load instead of Flask's default 12-hour browser -# cache. Updates ship live without users having to clear cache. -# Modern browsers still serve 304 Not Modified when the file hasn't -# changed, so the cost per asset per reload is just a header round-trip. -app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 0 +# Static assets (library.js / style.css / etc.) get aggressive browser +# caching (1 year). Safe because every static URL is bust-tagged with +# `?v=static_v` (computed once per process start — see below) so each +# server restart effectively invalidates every cached asset for every +# user. Within a single deploy, repeat page loads hit zero round-trips +# on static files — was a 304 round-trip per asset under the old +# max-age=0 setting. +# +# In dev, DEV_STATIC_NO_CACHE flips this back to 0 so iterating on JS +# / CSS doesn't require a server restart between edits. +app.config['SEND_FILE_MAX_AGE_DEFAULT'] = 0 if DEV_STATIC_NO_CACHE else 31536000 # Cache-bust query string for static assets — appended to every @@ -306,6 +311,39 @@ def _log_slow_request(response): return response + +@app.after_request +def _add_discover_cache_headers(response): + """Browser-cache discover GETs for 5 minutes. + + The discover surface (hero, similar artists, recent releases, release + radar, deep cuts, etc.) returns semi-stable data that's expensive to + compute and not user-action-driven within a session. A short browser + cache eliminates redundant fetches when the user toggles between + Discover sections or navigates back. + + Scope: only `/api/discover/` and `/api/discovery/` paths, only GET, + only successful 2xx responses. Any endpoint that explicitly sets + its own Cache-Control wins (we don't override). + """ + try: + if request.method != 'GET': + return response + if not (request.path.startswith('/api/discover/') + or request.path.startswith('/api/discovery/')): + return response + if not (200 <= response.status_code < 300): + return response + if response.headers.get('Cache-Control'): + return response + response.headers['Cache-Control'] = 'public, max-age=300' + except Exception as exc: + # Don't let a header-tagging bug turn a successful response into + # a 500 — log and ship the response without the cache header. + logger.warning(f"[discover-cache-headers] failed for {request.path}: {exc}") + return response + + def get_current_profile_id() -> int: """Get the current profile ID from Flask g context or default to 1""" try: @@ -16099,7 +16137,9 @@ def stream_audio(): response = send_file(file_path, as_attachment=False, mimetype=mimetype) response.headers.add('Accept-Ranges', 'bytes') response.headers.add('Content-Length', str(file_size)) - response.headers.add('Cache-Control', 'no-cache') + # Override the default static-cache max-age — streaming media + # bypasses caching (range requests, mid-track seeks). + response.headers['Cache-Control'] = 'no-cache' return response except Exception as e: @@ -25137,7 +25177,11 @@ def download_backup_endpoint(filename): backup_path = os.path.join(os.path.dirname(db_path), filename) if not os.path.exists(backup_path): return jsonify({"success": False, "error": "Backup not found"}), 404 - return send_file(backup_path, as_attachment=True, download_name=filename) + # Override the default static-cache max-age — this is a sensitive + # DB backup, browsers should never cache it. + response = send_file(backup_path, as_attachment=True, download_name=filename) + response.headers['Cache-Control'] = 'no-store' + return response except Exception as e: return jsonify({"success": False, "error": str(e)}), 500 diff --git a/webui/static/helper.js b/webui/static/helper.js index be8e1a62..e9560b3c 100644 --- a/webui/static/helper.js +++ b/webui/static/helper.js @@ -3447,6 +3447,7 @@ const WHATS_NEW = { { title: 'Lock Down Socket.IO CORS', desc: 'socket.io was accepting websocket connections from any origin (cors=*). now defaults to same-origin only. if your websocket fails after updating, the server logs a clear warning with the rejected origin — add it to settings → security → allowed websocket origins.', page: 'settings' }, { title: 'Faster Docker Startup — yt-dlp Pinned', desc: 'docker startup used to run `pip install -U yt-dlp` on every container start. removed that — yt-dlp is now pinned in requirements.txt so startup is fast and reproducible. tradeoff: youtube fixes ship via soulsync releases now instead of next container restart.' }, { title: 'Settings Endpoints: Admin-Only', desc: 'the /api/settings endpoints (read, write, log-level, config-status, verify) had no auth gate — any logged-in profile could read or change service tokens, oauth secrets, api keys. now admin-only. single-admin setups (no multi-profile config) work transparently as before.', page: 'settings' }, + { title: 'Browser Caching for Static Assets + Discover Pages', desc: 'static assets (js/css/icons) now get a 1-year browser cache instead of revalidating on every page load. safe because the existing ?v=static_v cache-bust query changes every server restart, so deploys still ship live. discover pages (hero, similar artists, recent releases, deep cuts, etc.) now cache 5 minutes browser-side so toggling between sections doesn\'t re-fetch everything. faster repeat loads, fewer round-trips.', page: 'discover' }, ], '2.4.0': [ // --- April 26, 2026 — Search & Artists unification + reorganize queue --- From 5d9e5e57811d3c046a279141b810501fb5af0dcc Mon Sep 17 00:00:00 2001 From: Broque Thomas <26755000+Nezreka@users.noreply.github.com> Date: Sun, 26 Apr 2026 21:27:46 -0700 Subject: [PATCH 2/2] Discover cache: switch from public to private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review nit on b0e7dae. Discover data is user-specific (hero artists from your watchlist, similar artists from your taste, recently-played derivations, etc.) — `Cache-Control: public` would let intermediate proxies (corporate caching proxy, Cloudflare with cache rules, Nginx with proxy_cache) store one user's response and serve it to another. Privacy leak. Switched to `private, max-age=300`. Browser-only cache, proxies skip. Static assets stay `public` (shared content — everyone gets the same library.js). Streaming and backup endpoints already correct (`no-cache` and `no-store` respectively). 603 tests pass. --- web_server.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/web_server.py b/web_server.py index 6d500ffe..8ce039e8 100644 --- a/web_server.py +++ b/web_server.py @@ -325,6 +325,12 @@ def _add_discover_cache_headers(response): Scope: only `/api/discover/` and `/api/discovery/` paths, only GET, only successful 2xx responses. Any endpoint that explicitly sets its own Cache-Control wins (we don't override). + + Uses `private` not `public` because discover data is user-specific + (hero artists from your watchlist, similar artists from your taste, + etc.). `private` keeps it browser-only — intermediate proxies + (corporate caching proxies, Cloudflare with cache rules, Nginx + proxy_cache) won't store one user's response and serve it to another. """ try: if request.method != 'GET': @@ -336,7 +342,7 @@ def _add_discover_cache_headers(response): return response if response.headers.get('Cache-Control'): return response - response.headers['Cache-Control'] = 'public, max-age=300' + response.headers['Cache-Control'] = 'private, max-age=300' except Exception as exc: # Don't let a header-tagging bug turn a successful response into # a 500 — log and ship the response without the cache header.