diff --git a/lib/sloppak.py b/lib/sloppak.py index 2b9bf83..a0db4f8 100644 --- a/lib/sloppak.py +++ b/lib/sloppak.py @@ -29,6 +29,12 @@ log = logging.getLogger("slopsmith.lib.sloppak") # (additive/MINOR compatibility); writers stamp this. FEEDPAK_VERSION = "1.2.0" +# Package suffixes. The format is byte-identical regardless of suffix; `.feedpak` +# is the current write extension, `.sloppak` the legacy one we still read. +FEEDPAK_EXT = ".feedpak" +SLOPPAK_EXT = ".sloppak" +SONG_EXTS = (FEEDPAK_EXT, SLOPPAK_EXT) # accepted on read/discovery + import yaml from safepath import safe_join @@ -48,8 +54,12 @@ import notation as notation_mod # ── Format detection ────────────────────────────────────────────────────────── def is_sloppak(path: Path) -> bool: - """True if path looks like a sloppak (zip file or directory).""" - return path.name.lower().endswith(".sloppak") + """True if path looks like a song package (zip file or directory). + + Accepts both the current `.feedpak` suffix and the legacy `.sloppak` one — + same on-disk format, either form. + """ + return path.name.lower().endswith(SONG_EXTS) # ── Source resolution (zip unpack cache + directory passthrough) ────────────── diff --git a/server.py b/server.py index 4bd1f3a..914cb55 100644 --- a/server.py +++ b/server.py @@ -3230,8 +3230,10 @@ def _background_scan(): # path for playback. def _is_excluded_from_library(p: Path) -> bool: return "tutorials-builtin" in p.parts or "minigames-builtin" in p.parts - # Sloppaks: match both file (zip) and directory form by suffix. - sloppaks = [f for f in sorted(dlc.rglob("*.sloppak")) + # Sloppaks: match both file (zip) and directory form, across both the + # `.feedpak` and legacy `.sloppak` suffixes. + _cands = sorted(p for ext in sloppak_mod.SONG_EXTS for p in dlc.rglob(f"*{ext}")) + sloppaks = [f for f in _cands if sloppak_mod.is_sloppak(f) and not _is_excluded_from_library(f)] @@ -3249,7 +3251,7 @@ def _background_scan(): if _is_excluded_from_library(wem): continue d = wem.parent - if d in sloppak_dirs or d.name.lower().endswith(".sloppak"): + if d in sloppak_dirs or d.name.lower().endswith(sloppak_mod.SONG_EXTS): continue if d not in seen_loose and loosefolder_mod.is_loose_song(d): loose_songs.append(d) @@ -3933,7 +3935,7 @@ def trigger_full_rescan(): # ── Song upload ─────────────────────────────────────────────────────────────── -_ALLOWED_SONG_EXTS = {".sloppak"} +_ALLOWED_SONG_EXTS = set(sloppak_mod.SONG_EXTS) _MAX_UPLOAD_BYTES = 1024 * 1024 * 1024 # 1 GB — covers sloppaks bundled with stems # Per-request batch cap. Lets a user drop a whole album of sloppaks at once # without giving a hostile client a 1000-file DoS surface via Starlette's @@ -4164,7 +4166,7 @@ async def _save_uploaded_song(upload: UploadFile, dlc: Path, overwrite: bool) -> suffix = Path(base).suffix.lower() if suffix not in _ALLOWED_SONG_EXTS: return {"status": "error", "filename": base, - "error": "Only .sloppak files are accepted"} + "error": "Only .feedpak files are accepted"} dest = dlc / base if dest.exists(): @@ -4222,10 +4224,10 @@ async def _save_uploaded_song(upload: UploadFile, dlc: Path, overwrite: bool) -> if bytes_read == 0: error_result = {"status": "error", "filename": base, "error": "Empty upload — file is 0 bytes"} - elif suffix == ".sloppak": + elif suffix in _ALLOWED_SONG_EXTS: if head[:2] != b"PK": error_result = {"status": "error", "filename": base, - "error": "Not a valid sloppak file (expected zip archive)"} + "error": "Not a valid feedpak file (expected zip archive)"} else: # ZIP magic alone admits any renamed zip — verify the sloppak # loader can actually parse a manifest.yaml inside. Without @@ -5447,8 +5449,9 @@ def save_settings(data: dict): else: if Path(dlc_path).is_dir(): updates["dlc_dir"] = dlc_path - count = sum(1 for f in Path(dlc_path).iterdir() if f.suffix == ".sloppak") - messages.append(f"DLC folder: {count} sloppak files found") + count = sum(1 for f in Path(dlc_path).iterdir() + if f.suffix.lower() in sloppak_mod.SONG_EXTS) + messages.append(f"DLC folder: {count} song files found") else: return {"error": f"DLC directory not found: {dlc_path}"} diff --git a/static/app.js b/static/app.js index aaee812..eead72d 100644 --- a/static/app.js +++ b/static/app.js @@ -1918,7 +1918,7 @@ function formatBadge(fmt, stemCount) { return `STEMS`; } if (fmt === 'sloppak') { - return `SLOPPAK`; + return `FEEDPAK`; } if (fmt === 'loose') { return `FOLDER`; @@ -1931,7 +1931,7 @@ function formatBadgeInline(fmt, stemCount) { return `STEMS`; } if (fmt === 'sloppak') { - return `SLOPPAK`; + return `FEEDPAK`; } if (fmt === 'loose') { return `FOLDER`; @@ -4154,10 +4154,10 @@ async function uploadSongs(fileList) { const files = []; for (const f of all) { const lower = f.name.toLowerCase(); - if (lower.endsWith('.sloppak')) { + if (lower.endsWith('.feedpak') || lower.endsWith('.sloppak')) { files.push(f); } else { - failures.push(`${f.name}: only .sloppak accepted`); + failures.push(`${f.name}: only .feedpak accepted`); } } if (files.length === 0) { diff --git a/static/index.html b/static/index.html index fc17214..8cafe98 100644 --- a/static/index.html +++ b/static/index.html @@ -120,7 +120,7 @@ diff --git a/static/v3/dashboard.js b/static/v3/dashboard.js index 1fdc1a5..8231d11 100644 --- a/static/v3/dashboard.js +++ b/static/v3/dashboard.js @@ -58,15 +58,15 @@ let f = ((song && song.format) || '').toLowerCase(); if (!f) { const fn = ((song && song.filename) || '').toLowerCase(); - f = fn.endsWith('.sloppak') ? 'sloppak' : ''; + f = (fn.endsWith('.feedpak') || fn.endsWith('.sloppak')) ? 'sloppak' : ''; } - return f === 'sloppak' ? 'SLOPPAK' : f === 'loose' ? 'FOLDER' : ''; + return f === 'sloppak' ? 'FEEDPAK' : f === 'loose' ? 'FOLDER' : ''; } // Corner badge for art-thumbnail cards (sloppak accented, others muted). function fmtBadge(song) { const l = fmtName(song); if (!l) return ''; - const c = l === 'SLOPPAK' ? 'bg-fb-primary text-white' : 'bg-black/70 text-fb-textDim'; + const c = l === 'FEEDPAK' ? 'bg-fb-primary text-white' : 'bg-black/70 text-fb-textDim'; return '' + l + ''; } // Inline pill for the hero (Pick/Continue) card, where the art is text-overlaid @@ -74,7 +74,7 @@ function fmtTag(song) { const l = fmtName(song); if (!l) return ''; - const c = l === 'SLOPPAK' ? 'bg-fb-primary/20 text-fb-primary' : 'bg-fb-card/80 text-fb-textDim'; + const c = l === 'FEEDPAK' ? 'bg-fb-primary/20 text-fb-primary' : 'bg-fb-card/80 text-fb-textDim'; return '' + l + ''; } diff --git a/static/v3/index.html b/static/v3/index.html index 50b9d1a..6ccb70d 100644 --- a/static/v3/index.html +++ b/static/v3/index.html @@ -223,7 +223,7 @@ diff --git a/static/v3/songs.js b/static/v3/songs.js index 2db1d1e..fa2319f 100644 --- a/static/v3/songs.js +++ b/static/v3/songs.js @@ -22,7 +22,7 @@ ['recent', 'Recently Added'], ['year-desc', 'Year (newest)'], ['year', 'Year (oldest)'], ['tuning', 'Tuning'], ]; - const FORMATS = [['', 'All formats'], ['sloppak', 'Sloppak'], ['loose', 'Folder']]; + const FORMATS = [['', 'All formats'], ['sloppak', 'Feedpak'], ['loose', 'Folder']]; const ARRANGEMENTS = ['Lead', 'Rhythm', 'Bass', 'Combo', 'Vocals']; const STEMS = ['guitar', 'bass', 'drums', 'vocals', 'other']; const PAGE_SIZE = 24; @@ -314,15 +314,15 @@ let f = (song.format || '').toLowerCase(); if (!f) { const fn = (song.filename || '').toLowerCase(); - f = fn.endsWith('.sloppak') ? 'sloppak' : ''; + f = (fn.endsWith('.feedpak') || fn.endsWith('.sloppak')) ? 'sloppak' : ''; } - return f === 'sloppak' ? 'SLOPPAK' : f === 'loose' ? 'FOLDER' : ''; + return f === 'sloppak' ? 'FEEDPAK' : f === 'loose' ? 'FOLDER' : ''; } // Corner badge for art-based cards (sloppak accented, others muted). function fmtBadge(song) { const l = fmtLabel(song); if (!l) return ''; - const c = l === 'SLOPPAK' ? 'bg-fb-primary text-white' : 'bg-black/70 text-fb-textDim'; + const c = l === 'FEEDPAK' ? 'bg-fb-primary text-white' : 'bg-black/70 text-fb-textDim'; return '' + l + ''; } @@ -674,7 +674,7 @@ '
' + '' + '' + esc(s.title) + '' + - (fl ? '' + fl + '' : '') + + (fl ? '' + fl + '' : '') + (state.accuracy[k] != null ? '' + Math.round(state.accuracy[k] * 100) + '%' : '') + '' + '
'); }).join('') + '').join('') + '').join(''); diff --git a/tests/test_feedpak_extension.py b/tests/test_feedpak_extension.py new file mode 100644 index 0000000..6e7d445 --- /dev/null +++ b/tests/test_feedpak_extension.py @@ -0,0 +1,246 @@ +"""Coverage for the `.sloppak → .feedpak` dual-suffix support (PR #553). + +The package format is byte-identical regardless of suffix: `.feedpak` is the +current write extension, `.sloppak` the legacy one we still read. These tests +pin the four behaviours the PR widened so a future refactor can't silently drop +back-compat for `.sloppak` libraries or stop accepting the new `.feedpak`: + +1. ``sloppak.is_sloppak`` / ``SONG_EXTS`` — suffix detection (unit). +2. ``_background_scan`` discovery glob — finds both suffixes (DLC scan). +3. ``POST /api/songs/upload`` gate — accepts both, rejects others (endpoint). +4. ``save_settings`` DLC count — counts both suffixes (settings handler). +""" + +from __future__ import annotations + +import importlib +import io +import sys +import zipfile +from pathlib import Path + +import pytest +import yaml + +import sloppak as sloppak_mod +from sloppak import FEEDPAK_EXT, SLOPPAK_EXT, SONG_EXTS + + +# ── helpers ────────────────────────────────────────────────────────────────── + +def _feedpak_zip_bytes(manifest: dict | None = None) -> bytes: + """A minimal valid package zip: a `manifest.yaml` that parses to a dict. + + The upload gate verifies more than ZIP magic — it runs + ``sloppak.load_manifest`` to reject any renamed zip without a parseable + top-level manifest mapping, so the bytes must contain a real one. + """ + buf = io.BytesIO() + with zipfile.ZipFile(buf, "w") as zf: + zf.writestr("manifest.yaml", + yaml.safe_dump(manifest or {"title": "T", "artist": "A"})) + return buf.getvalue() + + +# ── 1. is_sloppak / SONG_EXTS (unit) ───────────────────────────────────────── + +def test_song_exts_are_both_suffixes_lowercase(): + assert FEEDPAK_EXT == ".feedpak" + assert SLOPPAK_EXT == ".sloppak" + # `.feedpak` first — it is the write/discovery-preferred extension; both + # entries must be lowercase because is_sloppak relies on `.lower()` + a + # tuple-suffix match. + assert SONG_EXTS == (FEEDPAK_EXT, SLOPPAK_EXT) + assert all(e == e.lower() for e in SONG_EXTS) + + +@pytest.mark.parametrize("name", [ + "song.feedpak", # current write suffix + "song.sloppak", # legacy suffix, still read + "My Song_p.feedpak", # editor create-mode names land as *_p.feedpak + "dir-form.feedpak", # directory-form bundle (suffix-only check) + "dir-form.sloppak", + "SONG.FEEDPAK", # case-insensitive + "Song.SlopPak", +]) +def test_is_sloppak_accepts_both_suffixes(name): + assert sloppak_mod.is_sloppak(Path("/dlc") / name) is True + + +@pytest.mark.parametrize("name", [ + "song.zip", # a renamed zip is not a package by suffix + "song.txt", + "song.feedpak.bak", + "feedpak", # bare word, no dot-suffix + "noext", +]) +def test_is_sloppak_rejects_other_suffixes(name): + assert sloppak_mod.is_sloppak(Path("/dlc") / name) is False + + +# ── 2. _background_scan discovery glob (DLC scan) ──────────────────────────── + +@pytest.fixture() +def scan_server(tmp_path, monkeypatch, isolate_logging): + """Fresh server import with the background scan forced in-process. + + Mirrors tests/test_settings_api.py::scan_module — the production scan uses + a ``spawn`` ProcessPoolExecutor whose workers an in-process mock can't + reach, so swap in a ThreadPoolExecutor and mock metadata extraction. + """ + import concurrent.futures + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + monkeypatch.delenv("DLC_DIR", raising=False) + sys.modules.pop("server", None) + mod = importlib.import_module("server") + monkeypatch.setattr( + mod, "_make_scan_executor", + lambda: concurrent.futures.ThreadPoolExecutor(max_workers=4), + ) + yield mod + conn = getattr(getattr(mod, "meta_db", None), "conn", None) + if conn is not None: + conn.close() + + +def test_background_scan_discovers_both_suffixes(tmp_path, scan_server): + """The discovery glob unions over SONG_EXTS, so a `.feedpak` and a + legacy `.sloppak` in the DLC folder are both handed to extraction.""" + import unittest.mock as mock + + dlc = tmp_path / "dlc" + dlc.mkdir() + # Empty stubs are enough — is_sloppak keys on suffix and extraction is mocked. + (dlc / "new.feedpak").write_bytes(b"") + (dlc / "legacy.sloppak").write_bytes(b"") + (dlc / "ignore.zip").write_bytes(b"") # not a package — must be skipped + (tmp_path / "config.json").write_text('{"dlc_dir": "%s"}' % dlc) + + seen: list[str] = [] + + def mock_extract(f, dlc_dir): + seen.append(f.name) + return {"title": f.name, "artist": "", "album": ""} + + with mock.patch("scan_worker._extract_meta_for_file", new=mock_extract): + scan_server._background_scan() + + assert "new.feedpak" in seen + assert "legacy.sloppak" in seen + assert "ignore.zip" not in seen + + +# ── 3. POST /api/songs/upload gate (endpoint) ──────────────────────────────── + +@pytest.fixture() +def upload_client(tmp_path, monkeypatch): + """A TestClient with a temp DLC_DIR and startup side-effects stubbed. + + No lifespan is run (the client is not used as a context manager), so the + background scan / plugin load never fire; the upload handler only needs + ``_get_dlc_dir`` to resolve, which it does from the DLC_DIR env var. + """ + from fastapi.testclient import TestClient + + dlc = tmp_path / "dlc" + dlc.mkdir() + config = tmp_path / "cfg" + config.mkdir() + monkeypatch.setenv("DLC_DIR", str(dlc)) + monkeypatch.setenv("CONFIG_DIR", str(config)) + monkeypatch.setenv("SLOPSMITH_SYNC_STARTUP", "1") + sys.modules.pop("server", None) + server = importlib.import_module("server") + server.sloppak_mod._source_cache.clear() + monkeypatch.setattr(server, "load_plugins", lambda *a, **kw: None) + monkeypatch.setattr(server, "startup_scan", lambda: None) + tc = TestClient(server.app, client=("127.0.0.1", 50000)) + try: + yield tc, dlc + finally: + tc.close() + conn = getattr(getattr(server, "meta_db", None), "conn", None) + if conn is not None: + conn.close() + + +def _upload(tc, name, data): + return tc.post("/api/songs/upload", + files={"file": (name, data, "application/octet-stream")}) + + +def test_upload_accepts_feedpak(upload_client): + tc, dlc = upload_client + r = _upload(tc, "new_p.feedpak", _feedpak_zip_bytes()) + assert r.status_code == 200, r.text + result = r.json()["results"][0] + assert result["status"] == "ok", result + assert (dlc / "new_p.feedpak").exists() + + +def test_upload_still_accepts_legacy_sloppak(upload_client): + """Back-compat: a `.sloppak` upload keeps working.""" + tc, dlc = upload_client + r = _upload(tc, "legacy.sloppak", _feedpak_zip_bytes()) + assert r.status_code == 200, r.text + assert r.json()["results"][0]["status"] == "ok" + assert (dlc / "legacy.sloppak").exists() + + +def test_upload_rejects_wrong_suffix(upload_client): + """A valid zip under a non-package suffix is rejected at the ext gate.""" + tc, dlc = upload_client + r = _upload(tc, "song.zip", _feedpak_zip_bytes()) + assert r.status_code == 200, r.text + result = r.json()["results"][0] + assert result["status"] == "error" + assert "feedpak" in result["error"].lower() + assert not (dlc / "song.zip").exists() + + +def test_upload_rejects_feedpak_that_is_not_a_zip(upload_client): + """A `.feedpak` whose bytes aren't a ZIP archive fails the magic check + with the suffix-appropriate message.""" + tc, dlc = upload_client + r = _upload(tc, "bogus.feedpak", b"this is not a zip archive") + assert r.status_code == 200, r.text + result = r.json()["results"][0] + assert result["status"] == "error" + assert "feedpak" in result["error"].lower() + assert not (dlc / "bogus.feedpak").exists() + + +# ── 4. save_settings DLC count (settings handler) ──────────────────────────── + +@pytest.fixture() +def settings_server(tmp_path, monkeypatch): + """Fresh server import with an isolated CONFIG_DIR, startup tasks skipped. + + save_settings is driven directly — it takes the DLC path in its payload, so + no DLC_DIR env or HTTP layer is needed. + """ + monkeypatch.setenv("CONFIG_DIR", str(tmp_path)) + monkeypatch.setenv("SLOPSMITH_SKIP_STARTUP_TASKS", "1") + sys.modules.pop("server", None) + server = importlib.import_module("server") + try: + yield server + finally: + conn = getattr(getattr(server, "meta_db", None), "conn", None) + if conn is not None: + conn.close() + + +def test_settings_dlc_count_includes_both_suffixes(tmp_path, settings_server): + dlc = tmp_path / "dlc" + dlc.mkdir() + (dlc / "a.feedpak").write_bytes(b"") + (dlc / "b.sloppak").write_bytes(b"") + (dlc / "c.FEEDPAK").write_bytes(b"") # case-insensitive (suffix.lower()) + (dlc / "notes.txt").write_bytes(b"") # ignored + + result = settings_server.save_settings({"dlc_dir": str(dlc)}) + + assert "error" not in result, result + # save_settings joins its notices into a single ``message`` string. + assert "3 song files" in result.get("message", ""), result