feat: save as .feedpak; discover and load both .feedpak and .sloppak (#553)

* feat: save songs as .feedpak; discover and load both .feedpak and .sloppak The open song format was renamed sloppak -> feedpak (public spec lives in the feedback-feedpak-spec repo), but the server still wrote and recognized only `.sloppak`. The two are byte-identical on disk. Read both suffixes everywhere songs are discovered, uploaded, and loaded; writing the new `.feedpak` suffix is handled in the editor plugin repo. Keep the internal `format` tag `sloppak` so existing feature gates (stems, drums, keys) are untouched, matching the "internal rename not landed yet" stance. - lib/sloppak.py: add FEEDPAK_EXT / SLOPPAK_EXT / SONG_EXTS; is_sloppak() now matches either suffix (covers all 7 callers). - server.py: union scan glob over SONG_EXTS; widen loose-folder exclusion, settings DLC count, upload gate (_ALLOWED_SONG_EXTS) and zip-magic check; refresh user-facing messages to .feedpak. - static: library format filter relabeled Sloppak -> Feedpak (value stays sloppak, matches both); badge text SLOPPAK -> FEEDPAK in v2 + v3; filename-suffix detection and upload drag-drop filter accept both. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Signed-off-by: topkoa <topkoa@gmail.com> * test: cover .feedpak/.sloppak dual-suffix support Add tests/test_feedpak_extension.py pinning the four paths PR #553 widened so a refactor can't drop .sloppak back-compat or stop accepting .feedpak: - is_sloppak / SONG_EXTS suffix detection (file + dir form, case-insensitive) - _background_scan discovery glob unions over both suffixes - POST /api/songs/upload accepts both, rejects wrong suffix + non-zip - save_settings DLC count includes both suffixes 19 tests, all passing; reuses the existing scan_module / TestClient / isolate_logging fixtures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Signed-off-by: topkoa <topkoa@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com>
2026-08-10 18:59:56 +00:00 · 2026-06-22 20:47:56 +02:00
parent a07edd9971
commit 32127bc70b
8 changed files with 285 additions and 26 deletions
@@ -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