refactor(server): extract the album-art routes into routers/art.py (R3) (#862)

The six song-art routes — GET /api/song/{f}/art, .../art/cover-search,
.../art/candidates, POST .../art/upload, .../art/url, DELETE /api/art/{f}/override
— plus their exclusive helpers (the ETag/304 response machinery, _save_art_override,
_url_host_is_internal, _fetch_art_url + the art size/redirect caps) move to
lib/routers/art.py. Bodies verbatim except @app->@router and the seam reads:
meta_db->appstate.meta_db, ART_CACHE_DIR->appstate.art_cache_dir, and the three
shared art helpers that stay in server.py (used by the song/delete routes too)
-> appstate.<callable> (_song_pack_art_exists, _art_override_paths — already
seam-injected for the enrichment worker — plus a new art_safe_name slot). The
CAA / release-search transport lives in lib/enrichment.py and is reached as
enrichment.X. LocalLibraryProvider.get_art now calls art_router.get_song_art.

server.py: 5,988 -> 5,540 (-448).

Verified: pyflakes clean (bar the pre-existing File/safe_join/tuning_name/ET);
route table IDENTICAL (143); full pytest 2399 passed (33 art serve/candidates/
override/url cases incl the SSRF-guard _url_host_is_internal + _fetch_art_url
size-cap tests, retargeted onto the art module); test_packaging 43; eslint 0.
Boot smoke: /art 404, /art/candidates 404, DELETE /override 200 from the router.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Byron Gamatos
2026-07-11 11:56:53 +02:00
committed by GitHub
co-authored by Claude Opus 4.8
parent 165475d115
commit 73127d5416
7 changed files with 542 additions and 475 deletions
+8 -7
View File
@@ -13,6 +13,7 @@ tests/test_art_layer.py.
import importlib
import enrichment
from routers import art
import io as _io
import sys
@@ -252,7 +253,7 @@ def test_caa_candidates_capped_at_12(server, client, caa_index):
caa_index.indexes["rel-big"] = {
"images": [_img(300 + i, front=(i == 0)) for i in range(20)]}
_match_row(server, "a.sloppak", release_id="rel-big")
assert len(_caa(_get(client))) == server._ART_PICKER_MAX_CAA == 12
assert len(_caa(_get(client))) == art._ART_PICKER_MAX_CAA == 12
def test_demo_mode_blocks_candidates(server, client, monkeypatch):
@@ -342,9 +343,9 @@ def test_fetch_art_url_follows_redirects_validating_each_hop(server, monkeypatch
monkeypatch.setattr(requests, "get", fake_get)
monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True)
monkeypatch.setattr(server, "_url_host_is_internal",
monkeypatch.setattr(art, "_url_host_is_internal",
lambda u: (checked.append(u), False)[1])
data = server._fetch_art_url("https://coverartarchive.example/release/x/front-500")
data = art._fetch_art_url("https://coverartarchive.example/release/x/front-500")
assert data == b"IMGDATA"
assert fetched == ["https://coverartarchive.example/release/x/front-500",
"https://archive.example/img.png"]
@@ -356,10 +357,10 @@ def test_fetch_art_url_blocks_redirect_to_internal(server, monkeypatch):
monkeypatch.setattr(requests, "get", lambda url, **kw: _FakeResp(
302, {"Location": "http://internal.example/x.png"}))
monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True)
monkeypatch.setattr(server, "_url_host_is_internal",
monkeypatch.setattr(art, "_url_host_is_internal",
lambda u: "internal" in u)
with pytest.raises(ValueError):
server._fetch_art_url("https://public.example/x.png")
art._fetch_art_url("https://public.example/x.png")
def test_fetch_art_url_redirect_budget(server, monkeypatch):
@@ -367,6 +368,6 @@ def test_fetch_art_url_redirect_budget(server, monkeypatch):
monkeypatch.setattr(requests, "get", lambda url, **kw: _FakeResp(
307, {"Location": "https://public.example/next.png"}))
monkeypatch.setattr(enrichment, "_enrich_network_enabled", lambda: True)
monkeypatch.setattr(server, "_url_host_is_internal", lambda u: False)
monkeypatch.setattr(art, "_url_host_is_internal", lambda u: False)
with pytest.raises(enrichment.EnrichTransportError):
server._fetch_art_url("https://public.example/x.png")
art._fetch_art_url("https://public.example/x.png")
+11 -10
View File
@@ -8,6 +8,7 @@ here opens a socket, and the offline default is itself asserted.
import importlib
import enrichment
from routers import art
import io as _io
import sys
@@ -122,7 +123,7 @@ def test_bad_upload_rejected(server, client):
def test_art_url_fetches_and_overrides(server, client, monkeypatch):
make_sloppak(server, "a.sloppak", with_cover=True)
monkeypatch.setattr(server, "_fetch_art_url", lambda url: png_bytes((9, 9, 9)))
monkeypatch.setattr(art, "_fetch_art_url", lambda url: png_bytes((9, 9, 9)))
body = client.post("/api/song/a.sloppak/art/url",
json={"url": "https://example.com/cover.png"}).json()
assert body == {"ok": True, "kind": "png"}
@@ -139,7 +140,7 @@ def test_art_url_validation(server, client, monkeypatch):
# Oversize → 400 (the seam raises ValueError at the cap).
def _huge(url):
raise ValueError("image larger than 10 MB")
monkeypatch.setattr(server, "_fetch_art_url", _huge)
monkeypatch.setattr(art, "_fetch_art_url", _huge)
assert client.post("/api/song/a.sloppak/art/url",
json={"url": "https://example.com/x.png"}).status_code == 400
@@ -303,7 +304,7 @@ def test_upload_rejects_unknown_song_and_oversize(server, client):
assert server._art_override_paths("ghost.sloppak") == []
# Oversize decoded payload → 400 (bounds the base64 upload path).
make_sloppak(server, "a.sloppak")
huge = b64(b"\x00" * (server._ART_URL_MAX_BYTES + 1))
huge = b64(b"\x00" * (art._ART_URL_MAX_BYTES + 1))
assert client.post("/api/song/a.sloppak/art/upload",
json={"image": huge}).status_code == 400
@@ -311,10 +312,10 @@ def test_upload_rejects_unknown_song_and_oversize(server, client):
def test_fetch_art_url_blocks_internal_hosts(server):
"""The SSRF guard refuses loopback / link-local / private targets before
any request is made (the real seam, not the faked one)."""
assert server._url_host_is_internal("http://127.0.0.1/x.png")
assert server._url_host_is_internal("http://localhost/x.png")
assert server._url_host_is_internal("http://169.254.169.254/latest/meta-data")
assert server._url_host_is_internal("http://10.0.0.5/x.png")
assert server._url_host_is_internal("http://[::1]/x.png")
assert server._url_host_is_internal("http://nonexistent.invalid/x.png") # unresolvable → closed
assert not server._url_host_is_internal("http://93.184.216.34/x.png") # public literal
assert art._url_host_is_internal("http://127.0.0.1/x.png")
assert art._url_host_is_internal("http://localhost/x.png")
assert art._url_host_is_internal("http://169.254.169.254/latest/meta-data")
assert art._url_host_is_internal("http://10.0.0.5/x.png")
assert art._url_host_is_internal("http://[::1]/x.png")
assert art._url_host_is_internal("http://nonexistent.invalid/x.png") # unresolvable → closed
assert not art._url_host_is_internal("http://93.184.216.34/x.png") # public literal