mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-11 22:24:30 +00:00
Compare commits
8
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
ea6a79767b | ||
|
|
b41361eb1b | ||
|
|
6c98aba433 | ||
|
|
b3215694e7 | ||
|
|
ebe59d3f97 | ||
|
|
d6f2df14f7 | ||
|
|
94a58b7a42 | ||
|
|
58120745bc |
@@ -7,6 +7,81 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
- **The packaged desktop app could not start (`ModuleNotFoundError: No module named
|
||||
'appstate'`).** feedback-desktop's `scripts/bundle-slopsmith.sh` copies a *hardcoded
|
||||
list* of core files into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`,
|
||||
`static/`, `plugins/__init__.py`. The root-level `appstate.py` and `routers/` added in
|
||||
R3 shipped correctly in Docker and passed every test, and were then silently dropped
|
||||
from the packaged app, which died at startup. Both now live under **`lib/`** — the one
|
||||
core directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
|
||||
bundler (`cp -r lib`) all copy wholesale, and that all three put on `sys.path` (on
|
||||
Windows via the embeddable-Python `._pth`, where `PYTHONPATH` is ignored). This needs no
|
||||
change in feedback-desktop and no new release to take effect. Placing them there is also
|
||||
correct under Principle V: with the injection seam, `appstate.py` constructs nothing and
|
||||
does no import-time IO, and a route module only builds an `APIRouter`. The
|
||||
`Dockerfile` / `.dockerignore` / `docker-compose.yml` entries added for the root layout
|
||||
are reverted. New `tests/test_packaging.py` walks `server.py`'s module-level imports and
|
||||
fails if any first-party module resolves outside a directory the packagers copy, so the
|
||||
next root-level module can't ship broken.
|
||||
|
||||
### Added
|
||||
- **`routers/` — extracting `server.py`'s route layer, cheapest-first (R3).** Each PR moves a cohesive route group into a `fastapi.APIRouter` under `lib/routers/`, mounted with `app.include_router(...)` at its original site (FastAPI matches in registration order; the full route table stays byte-identical). Bodies are verbatim — only the decorator receiver (`@app` → `@router`) and singleton reads (`meta_db` → `appstate.meta_db`, resolved at call time) change. So far: `audio_effects` (5), `artist_aliases` (5), `loops` (3 — saved A/B practice regions). The next cut is picked by a dependency-closure scan that ranks groups by how many `monkeypatch.setattr(server, …)` targets they'd drag along.
|
||||
- **`routers/` — the first extracted route module (R3).** The five audio-effects mapping
|
||||
endpoints move out of `server.py` into `lib/routers/audio_effects.py` as a
|
||||
`fastapi.APIRouter`, mounted with `app.include_router(...)` **at the point in the file
|
||||
where they used to be defined** — FastAPI matches routes in registration order, so the
|
||||
mount site preserves it. Verified: the full 143-route table (paths, methods, *and*
|
||||
order) is byte-for-byte identical to `main`. Bodies are verbatim; the only edits are
|
||||
the decorator receiver (`@app.get` → `@router.get`) and the singleton read
|
||||
(`audio_effect_mappings` → `appstate.audio_effect_mappings`, a module attribute
|
||||
resolved at call time). This proves the seam from #833 under a real consumer, including
|
||||
the second slot. The `_demo_mode_guard` middleware still blocks all four moved write
|
||||
routes with 403, and `Query(...)` validation still 422s — both checked against a running
|
||||
server. `server.py`: **9,445 → 9,386 lines**.
|
||||
- **`appstate.py` — the router seam (R3).** Route modules moving out of `server.py`
|
||||
need `meta_db` and friends but must not `import server`, or the import graph goes
|
||||
circular the moment `server` imports them back. So `server.py` keeps *constructing*
|
||||
its singletons and now **injects** them once — `appstate.configure(meta_db=…,
|
||||
audio_effect_mappings=…)` — and a router reads them back as module attributes at call
|
||||
time (`import appstate; appstate.meta_db.…`). This is the Python analogue of the
|
||||
frontend refactor's injected `configureX({…})` seams and of the plugin
|
||||
`setup(app, context)` contract: dependencies flow one way, `server → routers →
|
||||
appstate`. Two properties are load-bearing and pinned by `tests/test_appstate.py`:
|
||||
(1) `import appstate` constructs nothing and touches no disk, so the ~49 test fixtures
|
||||
that `sys.modules.pop("server")` + re-import (to rebuild `meta_db` under a patched
|
||||
`CONFIG_DIR`) keep working untouched — a singleton *owned* by `appstate` would survive
|
||||
that pop and go stale; (2) reads must be late-bound (`appstate.meta_db`, never
|
||||
`from appstate import meta_db`), since a `from` import freezes the binding and defeats
|
||||
both a later `configure()` and `monkeypatch.setattr` — the same read-only-binding trap
|
||||
as ES `import`. `configure()` rejects an unknown slot rather than silently creating a
|
||||
global nothing reads, and the suite asserts `server` actually calls it (a seam whose
|
||||
wiring can no-op undetected is worse than no seam). Lives at `lib/appstate.py`.
|
||||
|
||||
### Changed
|
||||
- **`AudioEffectsMappingDB` moved out of `server.py` into `lib/audio_effects_db.py`
|
||||
(R3, move-only).** The core-owned song/tone → provider routing index follows
|
||||
`MetadataDB` out of the host file, byte-identical apart from the same constructor
|
||||
seam (`__init__` takes `config_dir`; `audio_effect_mappings = AudioEffectsMappingDB(CONFIG_DIR)`),
|
||||
so the module does no IO at import. The singleton stays in `server.py`; no route,
|
||||
no test and no `monkeypatch.setattr(server, …)` target moves. `server.py`:
|
||||
**9,705 → 9,433 lines**.
|
||||
- **`MetadataDB` moved out of `server.py` into `lib/metadata_db.py` (R3, move-only).**
|
||||
The library metadata cache — the `MetadataDB` class (4,018 lines) plus the query
|
||||
helpers it owns (keyset paging cursors, the tuning grouping key, smart-arrangement
|
||||
naming, tag normalisation, the startup DB-restore swap) — now lives in its own flat
|
||||
`lib/` module. `server.py` drops from **14,037 → 9,705 lines** and keeps the
|
||||
`meta_db` singleton, so `server.meta_db` and `server.app` resolve exactly as before
|
||||
and every route is untouched. The only non-verbatim change is the seam that lets the
|
||||
class leave `server.py`: `MetadataDB.__init__` now takes `config_dir` explicitly
|
||||
(`meta_db = MetadataDB(CONFIG_DIR)`) instead of reading the module-level `CONFIG_DIR`,
|
||||
which also means `lib/metadata_db.py` performs no IO at import (Principle V). Logging
|
||||
still goes through the `feedBack.server` logger, so existing log filters and `caplog`
|
||||
assertions resolve to the same logger object. `tests/test_settings_export_library_db.py`
|
||||
now imports `_apply_pending_db_restore` from `metadata_db` (the test moved with its
|
||||
subject); no other test changed. Every moved block is byte-identical to its
|
||||
`server.py` original.
|
||||
|
||||
### Added
|
||||
- **Plugins can ship an ES-module `src/` tree (module-migration rails, R0).** The host gains three things so a plugin can move off a single global-scope `screen.js` IIFE onto native ES modules with **no build step**: (1) a new sandboxed `GET /api/plugins/{id}/src/{path}` route that serves a plugin's `src/` source subtree, containment-checked by the same `safe_join` guard as `assets/` (traversal/absolute/NUL → 404); (2) the live-edit cache contract — `Cache-Control: no-cache` + a weak mtime/size `ETag` + `If-None-Match`→`304` — applied to `src/`, `screen.js`, and `assets/` (previously `screen.js` sent no cache headers and `assets/` emitted an ETag but never revalidated), so an edited module reloads on refresh while unchanged ones `304`; and (3) `scriptType`/`minHost` passthrough from `plugin.json` to `/api/plugins`, with the loader injecting a plugin that declares `"scriptType":"module"` as `<script type="module">` (its screen.js becomes `import './src/main.js'`). A `<script type=module>` fires its load event only after its whole static-import graph evaluates, preserving the loader's completion-by-`onload` + `_loadingPluginId` contract. Classic plugins are unaffected; `minHost` is passthrough-only for now (enforcement deferred). Tests: `tests/test_plugin_src_route.py` (serve/media-type/traversal/304/no-stale-304/screen.js+assets conditional), `tests/js/plugin_loader_script_type.test.js` (guarded module injection).
|
||||
- **Module-migration governance & rails (R0).** Constitution amended to **v1.2.0**: Principle II now names native ES modules as a first-class, *build-free* extension mechanism (the `scriptType:"module"` load path, both plugins and — over time — core's `static/js/`), keeping the no-bundler/no-transpiler/source-served rule intact; Operating Constraints gains a "Module load contract" clause (a `<script type=module>` load event awaits the whole static-import graph, so completion-by-`onload` is preserved; per-visit re-init comes from the `screen:changed` event, not screen.js re-execution). Mirrored into `CLAUDE.md`. New `docs/plugin-modules.md` (the migration playbook — layering, import-time purity, `import.meta.url` assets, the ETag live-edit loop) and `docs/size-exemptions.md` (the signed 1,500-line size-norm register; Byron signs core/bundled rows, Christian the authored virtuoso row). Adds a **maintainer/CI-only** ESLint gate (`eslint.config.js` + a `lint` CI job): `max-lines` warns at 1,500 as a non-blocking ratchet (ceilings for exempt files mirror the register), and `import-x/no-unresolved` + `import-x/no-cycle` hard-error on ES-module graphs — dormant until module code lands, never on the serve/Docker path.
|
||||
|
||||
@@ -54,8 +54,12 @@ without a *signed* exemption" is unenforceable.
|
||||
|
||||
## Planned, NOT exempt (owned by split plans — listed so nothing falls between states)
|
||||
|
||||
core `static/app.js` (11,821) · `static/highway.js` (4,154, whole file) · `server.py`
|
||||
(13,948) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`
|
||||
core `static/app.js` (11,852) · `static/highway.js` (4,168, whole file) · `server.py`
|
||||
(9,302 — was 14,037; ratcheted by the R3 `MetadataDB` + `AudioEffectsMappingDB`
|
||||
extractions and the first three `routers/` modules) ·
|
||||
`lib/metadata_db.py` (4,373 — new in R3; the `MetadataDB` class alone is 4,018 lines
|
||||
and is a monolith in its own right, to be split per-table once the router train
|
||||
lands) · `static/v3/songs.js` (4,134) · `static/capabilities/audio-session.js`
|
||||
(2,974) · `plugins/highway_3d/screen.js` (15,656) · `plugins/keys_highway_3d/screen.js`
|
||||
(3,780) · `plugins/drum_highway_3d/screen.js` (3,597) — and every monolith with a PR
|
||||
train in the refactor plan. Test files (e.g. `tests/test_plugins.py`) are out of scope
|
||||
|
||||
@@ -0,0 +1,81 @@
|
||||
"""Shared application state — the seam that lets route modules reach core
|
||||
singletons without importing ``server``.
|
||||
|
||||
``server.py`` is the host: it owns the FastAPI ``app``, constructs the DB
|
||||
singletons, and runs the lifecycle. As routes move out into ``routers/`` (R3),
|
||||
those modules need ``meta_db`` and friends — but they must not ``import
|
||||
server``, or the import graph goes circular the moment ``server`` imports them
|
||||
back.
|
||||
|
||||
So ``server`` **injects** its singletons here once, at the point it builds them::
|
||||
|
||||
# server.py
|
||||
meta_db = MetadataDB(CONFIG_DIR)
|
||||
appstate.configure(meta_db=meta_db, ...)
|
||||
|
||||
and a router reads them back as **module attributes, at call time**::
|
||||
|
||||
# routers/artists.py
|
||||
import appstate
|
||||
|
||||
@router.get("/api/artist/{name}/page")
|
||||
def artist_page(name):
|
||||
return appstate.meta_db.artist_page(name)
|
||||
|
||||
This is the Python analogue of the injected `configureX({...})` seams the
|
||||
frontend refactor uses (stems' ``configureStreaming``, studio's
|
||||
``configureAudioGraph``, the editor's ``src/host.js``), and of the plugin
|
||||
``setup(app, context)`` contract in Principle III: dependencies flow one way,
|
||||
``server -> routers -> appstate``, and nothing imports back up.
|
||||
|
||||
Two properties this shape buys, both load-bearing:
|
||||
|
||||
* **``import appstate`` performs no IO and constructs nothing.** ``server``
|
||||
still owns construction, so the ~49 test fixtures that do
|
||||
``sys.modules.pop("server")`` + re-import (to rebuild ``meta_db`` under a
|
||||
patched ``CONFIG_DIR``) keep working untouched — a singleton *owned* here
|
||||
would survive that pop and go stale.
|
||||
* **Reads are late-bound.** Routers must use ``appstate.meta_db``, never
|
||||
``from appstate import meta_db`` — a ``from`` import freezes the binding at
|
||||
its current value, so a later ``configure()`` (or a
|
||||
``monkeypatch.setattr(appstate, "meta_db", fake)``) would not reach the
|
||||
router. This is the same read-only-binding trap as ES ``import``.
|
||||
|
||||
Defaults are ``None`` on purpose: they are inert but *type-honest*, so a router
|
||||
that runs before ``configure()`` fails loudly on ``NoneType`` instead of
|
||||
quietly operating on a stand-in.
|
||||
|
||||
Slots are added here only when a router actually needs one — this is a seam,
|
||||
not a grab-bag for everything in ``server.py``.
|
||||
|
||||
**Why this lives in ``lib/`` and not the repo root.** Because it constructs
|
||||
nothing and does no import-time IO, it satisfies Principle V's rule for ``lib/``
|
||||
modules — and ``lib/`` is the only core directory every packaging path already
|
||||
copies: the Dockerfile (``COPY lib/``), ``docker-compose.yml``, and
|
||||
feedback-desktop's ``bundle-slopsmith.sh`` (``cp -r lib``). All three also put
|
||||
both the bundle root and ``lib/`` on ``sys.path``. A root-level module ships in
|
||||
Docker but is silently dropped from the packaged desktop app, whose bundler
|
||||
copies a hardcoded file list — that regression is what moved this file here.
|
||||
"""
|
||||
|
||||
# The singletons routers may read. Every name here must also be a `_SLOTS` key.
|
||||
meta_db = None
|
||||
audio_effect_mappings = None
|
||||
|
||||
# Declared up front so `configure()` can reject a typo'd or stale keyword
|
||||
# instead of silently creating a new global that nothing ever reads. A seam
|
||||
# whose wiring can no-op undetected is worse than no seam.
|
||||
_SLOTS = frozenset({"meta_db", "audio_effect_mappings"})
|
||||
|
||||
|
||||
def configure(**kwargs) -> None:
|
||||
"""Publish `server`'s singletons into this module. Called once per
|
||||
`server` import (and again on re-import), so it must be idempotent."""
|
||||
unknown = set(kwargs) - _SLOTS
|
||||
if unknown:
|
||||
raise TypeError(
|
||||
f"appstate.configure() got unknown slot(s): {sorted(unknown)}. "
|
||||
f"Known slots: {sorted(_SLOTS)}. Add the name to _SLOTS if a router "
|
||||
f"genuinely needs it."
|
||||
)
|
||||
globals().update(kwargs)
|
||||
@@ -0,0 +1,287 @@
|
||||
"""Core-owned song/tone -> audio-effect-provider mapping index.
|
||||
|
||||
Extracted verbatim from ``server.py`` (R3). ``server.py`` still owns the
|
||||
``audio_effect_mappings`` singleton; this module only supplies the class, so
|
||||
nothing here touches config paths at import time — the caller passes
|
||||
``config_dir`` in.
|
||||
"""
|
||||
|
||||
import json
|
||||
import sqlite3
|
||||
import threading
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
class AudioEffectsMappingDB:
|
||||
"""Core-owned public song/tone -> provider mapping index.
|
||||
|
||||
Providers own the preset/chain rows addressed by provider_ref. Core owns
|
||||
the cross-provider routing index and the active mapping per song/tone.
|
||||
"""
|
||||
|
||||
def __init__(self, config_dir: Path):
|
||||
config_dir.mkdir(parents=True, exist_ok=True)
|
||||
self.db_path = str(config_dir / "audio_effects.db")
|
||||
self.conn = sqlite3.connect(self.db_path, check_same_thread=False)
|
||||
self.conn.execute("PRAGMA journal_mode=WAL")
|
||||
self.conn.execute("PRAGMA foreign_keys=ON")
|
||||
self.conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS audio_effect_mappings (
|
||||
id INTEGER PRIMARY KEY AUTOINCREMENT,
|
||||
song_key TEXT NOT NULL,
|
||||
filename TEXT NOT NULL DEFAULT '',
|
||||
tone_key TEXT NOT NULL,
|
||||
provider_id TEXT NOT NULL,
|
||||
provider_ref TEXT NOT NULL,
|
||||
label TEXT NOT NULL DEFAULT '',
|
||||
source TEXT NOT NULL DEFAULT 'manual',
|
||||
metadata_json TEXT NOT NULL DEFAULT '{}',
|
||||
created_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
UNIQUE(song_key, tone_key, provider_id)
|
||||
)
|
||||
""")
|
||||
self.conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS audio_effect_active_mappings (
|
||||
song_key TEXT NOT NULL,
|
||||
tone_key TEXT NOT NULL,
|
||||
mapping_id INTEGER NOT NULL,
|
||||
updated_at TEXT NOT NULL DEFAULT (datetime('now')),
|
||||
PRIMARY KEY (song_key, tone_key),
|
||||
FOREIGN KEY (mapping_id) REFERENCES audio_effect_mappings(id) ON DELETE CASCADE
|
||||
)
|
||||
""")
|
||||
self.conn.execute(
|
||||
"CREATE INDEX IF NOT EXISTS idx_audio_effect_mappings_provider "
|
||||
"ON audio_effect_mappings(provider_id)"
|
||||
)
|
||||
self.conn.execute(
|
||||
"CREATE INDEX IF NOT EXISTS idx_audio_effect_mappings_filename "
|
||||
"ON audio_effect_mappings(filename)"
|
||||
)
|
||||
self.conn.commit()
|
||||
self._lock = threading.Lock()
|
||||
|
||||
@staticmethod
|
||||
def _text(value, *, field: str, limit: int, allow_empty: bool = False) -> str:
|
||||
if value is None:
|
||||
text = ""
|
||||
elif not isinstance(value, str):
|
||||
raise ValueError(f"{field} must be a string")
|
||||
else:
|
||||
text = value.strip()
|
||||
if not text and not allow_empty:
|
||||
raise ValueError(f"{field} is required")
|
||||
if len(text) > limit:
|
||||
raise ValueError(f"{field} is too long")
|
||||
return text
|
||||
|
||||
@staticmethod
|
||||
def _mapping_id(value) -> int | None:
|
||||
# Bind only values SQLite can store as an INTEGER; an out-of-range id is a
|
||||
# clean miss (404), not a 500 at bind time.
|
||||
if isinstance(value, int) and not isinstance(value, bool) and -(2 ** 63) <= value < 2 ** 63:
|
||||
return value
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _field(data: dict, *keys):
|
||||
# Select the first present snake/camel alias by key, not by truthiness, so a
|
||||
# falsey non-string value (false/0) still reaches _text() and is rejected
|
||||
# instead of being silently swallowed by an `or` chain.
|
||||
for key in keys:
|
||||
if key in data:
|
||||
return data[key]
|
||||
return None
|
||||
|
||||
@staticmethod
|
||||
def _metadata(value) -> str:
|
||||
if value is None:
|
||||
return "{}"
|
||||
if not isinstance(value, dict):
|
||||
raise ValueError("metadata must be an object")
|
||||
encoded = json.dumps(value, ensure_ascii=True, sort_keys=True)
|
||||
if len(encoded) > 8192:
|
||||
raise ValueError("metadata is too large")
|
||||
return encoded
|
||||
|
||||
@staticmethod
|
||||
def _row(row) -> dict | None:
|
||||
if row is None:
|
||||
return None
|
||||
metadata = {}
|
||||
try:
|
||||
metadata = json.loads(row[8]) if row[8] else {}
|
||||
except Exception:
|
||||
metadata = {}
|
||||
return {
|
||||
"id": int(row[0]),
|
||||
"song_key": row[1],
|
||||
"filename": row[2] or "",
|
||||
"tone_key": row[3],
|
||||
"provider_id": row[4],
|
||||
"provider_ref": row[5],
|
||||
"label": row[6] or "",
|
||||
"source": row[7] or "manual",
|
||||
"metadata": metadata if isinstance(metadata, dict) else {},
|
||||
"created_at": row[9] or "",
|
||||
"updated_at": row[10] or "",
|
||||
"active": bool(row[11]),
|
||||
}
|
||||
|
||||
def _select_sql(self) -> str:
|
||||
return """
|
||||
SELECT m.id, m.song_key, m.filename, m.tone_key, m.provider_id,
|
||||
m.provider_ref, m.label, m.source, m.metadata_json,
|
||||
m.created_at, m.updated_at,
|
||||
CASE WHEN a.mapping_id IS NULL THEN 0 ELSE 1 END AS active
|
||||
FROM audio_effect_mappings m
|
||||
LEFT JOIN audio_effect_active_mappings a
|
||||
ON a.song_key = m.song_key AND a.tone_key = m.tone_key AND a.mapping_id = m.id
|
||||
"""
|
||||
|
||||
def list(self, *, song_key: str = "", filename: str = "", tone_key: str = "", provider_id: str = "") -> list[dict]:
|
||||
clauses: list[str] = []
|
||||
params: list[str] = []
|
||||
song_key = self._text(song_key, field="song_key", limit=240, allow_empty=True)
|
||||
filename = self._text(filename, field="filename", limit=500, allow_empty=True)
|
||||
tone_key = self._text(tone_key, field="tone_key", limit=160, allow_empty=True)
|
||||
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
|
||||
if song_key and filename:
|
||||
clauses.append("(m.song_key = ? OR m.filename = ?)")
|
||||
params.extend([song_key, filename])
|
||||
elif song_key:
|
||||
clauses.append("m.song_key = ?")
|
||||
params.append(song_key)
|
||||
elif filename:
|
||||
clauses.append("(m.song_key = ? OR m.filename = ?)")
|
||||
params.extend([filename, filename])
|
||||
if tone_key:
|
||||
clauses.append("m.tone_key = ?")
|
||||
params.append(tone_key)
|
||||
if provider_id:
|
||||
clauses.append("m.provider_id = ?")
|
||||
params.append(provider_id)
|
||||
sql = self._select_sql()
|
||||
if clauses:
|
||||
sql += " WHERE " + " AND ".join(clauses)
|
||||
sql += " ORDER BY m.song_key COLLATE NOCASE, m.tone_key COLLATE NOCASE, m.provider_id COLLATE NOCASE"
|
||||
with self._lock:
|
||||
rows = self.conn.execute(sql, params).fetchall()
|
||||
return [self._row(row) for row in rows]
|
||||
|
||||
def get(self, mapping_id: int) -> dict | None:
|
||||
mapping_id = self._mapping_id(mapping_id)
|
||||
if mapping_id is None:
|
||||
return None
|
||||
with self._lock:
|
||||
row = self.conn.execute(self._select_sql() + " WHERE m.id = ?", (mapping_id,)).fetchone()
|
||||
return self._row(row)
|
||||
|
||||
def upsert(self, data: dict) -> dict:
|
||||
if not isinstance(data, dict):
|
||||
raise ValueError("mapping body must be an object")
|
||||
filename = self._text(data.get("filename", ""), field="filename", limit=500, allow_empty=True)
|
||||
song_key_raw = self._field(data, "song_key", "songKey")
|
||||
if song_key_raw is None or song_key_raw == "":
|
||||
song_key_raw = filename
|
||||
song_key = self._text(song_key_raw, field="song_key", limit=240)
|
||||
tone_key = self._text(self._field(data, "tone_key", "toneKey"), field="tone_key", limit=160, allow_empty=True)
|
||||
provider_id = self._text(self._field(data, "provider_id", "providerId"), field="provider_id", limit=96)
|
||||
provider_ref = self._text(self._field(data, "provider_ref", "providerRef"), field="provider_ref", limit=240)
|
||||
label = self._text(data.get("label", ""), field="label", limit=160, allow_empty=True)
|
||||
source = self._text(data.get("source", "manual"), field="source", limit=40, allow_empty=True) or "manual"
|
||||
metadata_json = self._metadata(data.get("metadata", {}))
|
||||
with self._lock:
|
||||
self.conn.execute(
|
||||
"""
|
||||
INSERT INTO audio_effect_mappings
|
||||
(song_key, filename, tone_key, provider_id, provider_ref, label, source, metadata_json, updated_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, datetime('now'))
|
||||
ON CONFLICT(song_key, tone_key, provider_id) DO UPDATE SET
|
||||
-- Only overwrite filename when a non-empty one was supplied; an
|
||||
-- omitted/empty filename must preserve the stored value (it's an
|
||||
-- alternate lookup key for list(..., filename=...)).
|
||||
filename=CASE WHEN excluded.filename <> '' THEN excluded.filename ELSE audio_effect_mappings.filename END,
|
||||
provider_ref=excluded.provider_ref,
|
||||
label=excluded.label,
|
||||
source=excluded.source,
|
||||
metadata_json=excluded.metadata_json,
|
||||
updated_at=datetime('now')
|
||||
""",
|
||||
(song_key, filename, tone_key, provider_id, provider_ref, label, source, metadata_json),
|
||||
)
|
||||
row = self.conn.execute(
|
||||
"SELECT id FROM audio_effect_mappings WHERE song_key = ? AND tone_key = ? AND provider_id = ?",
|
||||
(song_key, tone_key, provider_id),
|
||||
).fetchone()
|
||||
if row is None:
|
||||
raise ValueError("failed to create audio-effects mapping")
|
||||
mapping_id = int(row[0])
|
||||
if data.get("active") is True:
|
||||
self.conn.execute(
|
||||
"""
|
||||
INSERT INTO audio_effect_active_mappings (song_key, tone_key, mapping_id, updated_at)
|
||||
VALUES (?, ?, ?, datetime('now'))
|
||||
ON CONFLICT(song_key, tone_key) DO UPDATE SET
|
||||
mapping_id=excluded.mapping_id,
|
||||
updated_at=datetime('now')
|
||||
""",
|
||||
(song_key, tone_key, mapping_id),
|
||||
)
|
||||
self.conn.commit()
|
||||
return self.get(mapping_id)
|
||||
|
||||
def delete(self, mapping_id: int, *, provider_id: str = "") -> bool:
|
||||
mapping_id = self._mapping_id(mapping_id)
|
||||
if mapping_id is None:
|
||||
return False
|
||||
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
|
||||
with self._lock:
|
||||
if provider_id:
|
||||
cur = self.conn.execute(
|
||||
"DELETE FROM audio_effect_mappings WHERE id = ? AND provider_id = ?",
|
||||
(mapping_id, provider_id),
|
||||
)
|
||||
else:
|
||||
cur = self.conn.execute("DELETE FROM audio_effect_mappings WHERE id = ?", (mapping_id,))
|
||||
self.conn.commit()
|
||||
return cur.rowcount > 0
|
||||
|
||||
def activate(self, mapping_id: int, *, provider_id: str = "") -> dict | None:
|
||||
mapping_id = self._mapping_id(mapping_id)
|
||||
if mapping_id is None:
|
||||
return None
|
||||
provider_id = self._text(provider_id, field="provider_id", limit=96, allow_empty=True)
|
||||
with self._lock:
|
||||
row = self.conn.execute(
|
||||
self._select_sql() + " WHERE m.id = ?",
|
||||
(mapping_id,),
|
||||
).fetchone()
|
||||
mapping = self._row(row)
|
||||
if not mapping or (provider_id and mapping["provider_id"] != provider_id):
|
||||
return None
|
||||
self.conn.execute(
|
||||
"""
|
||||
INSERT INTO audio_effect_active_mappings (song_key, tone_key, mapping_id, updated_at)
|
||||
VALUES (?, ?, ?, datetime('now'))
|
||||
ON CONFLICT(song_key, tone_key) DO UPDATE SET
|
||||
mapping_id=excluded.mapping_id,
|
||||
updated_at=datetime('now')
|
||||
""",
|
||||
(mapping["song_key"], mapping["tone_key"], mapping_id),
|
||||
)
|
||||
self.conn.commit()
|
||||
selected = self.conn.execute(self._select_sql() + " WHERE m.id = ?", (mapping_id,)).fetchone()
|
||||
return self._row(selected)
|
||||
|
||||
def clear_active(self, *, song_key: str, tone_key: str) -> bool:
|
||||
song_key = self._text(song_key, field="song_key", limit=240)
|
||||
tone_key = self._text(tone_key, field="tone_key", limit=160, allow_empty=True)
|
||||
with self._lock:
|
||||
cur = self.conn.execute(
|
||||
"DELETE FROM audio_effect_active_mappings WHERE song_key = ? AND tone_key = ?",
|
||||
(song_key, tone_key),
|
||||
)
|
||||
self.conn.commit()
|
||||
return cur.rowcount > 0
|
||||
+4373
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,31 @@
|
||||
"""FastAPI route modules extracted from ``server.py`` (R3).
|
||||
|
||||
Each module here exposes a module-level ``router`` (a ``fastapi.APIRouter``)
|
||||
that ``server.py`` mounts with ``app.include_router(...)`` at the point in the
|
||||
file where those routes used to be defined — FastAPI matches routes in
|
||||
registration order, so keeping the mount site preserves it.
|
||||
|
||||
**Routers must never ``import server``.** They reach core singletons through
|
||||
the injected seam instead::
|
||||
|
||||
import appstate
|
||||
|
||||
@router.get("/api/thing")
|
||||
def get_thing():
|
||||
return appstate.meta_db.thing()
|
||||
|
||||
and always as a **module attribute, at call time** — never
|
||||
``from appstate import meta_db``, which freezes the binding and defeats both a
|
||||
later ``appstate.configure()`` and ``monkeypatch.setattr``. See ``appstate.py``.
|
||||
|
||||
Dependencies flow one way: ``server -> routers -> appstate``.
|
||||
|
||||
**Why this lives under ``lib/``.** ``lib/`` is the only core directory every
|
||||
packaging path already copies wholesale — the Dockerfile (``COPY lib/``),
|
||||
``docker-compose.yml``, and feedback-desktop's ``bundle-slopsmith.sh``
|
||||
(``cp -r lib``) — and all three put it on ``sys.path``. A root-level package
|
||||
ships in Docker but is silently dropped from the packaged desktop app, whose
|
||||
bundler copies a hardcoded file list. Route modules import nothing at module
|
||||
scope beyond FastAPI and ``appstate``, so they do no import-time IO and satisfy
|
||||
Principle V's rule for ``lib/``.
|
||||
"""
|
||||
@@ -0,0 +1,68 @@
|
||||
"""Artist aliases / Tidy-up (P4) — canonicalize messy artist tags at DISPLAY
|
||||
("ACDC" -> "AC/DC") without touching feedpak files or the scanner-derived
|
||||
songs.artist. All DB-only.
|
||||
|
||||
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
|
||||
(``@app`` -> ``@router``) and the singleton read (``meta_db`` ->
|
||||
``appstate.meta_db``) changed. The read stays a module attribute so a re-imported
|
||||
``server`` re-publishes a fresh DB into the seam — see ``appstate.py``.
|
||||
"""
|
||||
|
||||
from fastapi import APIRouter
|
||||
from fastapi.responses import JSONResponse
|
||||
|
||||
import appstate
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
@router.get("/api/artist-aliases")
|
||||
def list_artist_aliases():
|
||||
"""Existing raw→canonical overrides (the Tidy-up 'current merges' list)."""
|
||||
return {"aliases": appstate.meta_db.list_artist_aliases()}
|
||||
|
||||
|
||||
@router.get("/api/artists/raw")
|
||||
def list_raw_artists(limit: int = 2000):
|
||||
"""Distinct RAW artist names + song counts + current canonical — the Tidy-up
|
||||
picker (you merge raw variants into one canonical)."""
|
||||
return {"artists": appstate.meta_db.raw_artists(limit)}
|
||||
|
||||
|
||||
@router.post("/api/artist-aliases")
|
||||
def set_artist_alias(data: dict):
|
||||
"""Upsert one override: {raw_name, canonical_name, mb_artist_id?}. A self-alias
|
||||
(raw == canonical) clears the row instead (un-merge)."""
|
||||
raw = (data.get("raw_name") or "").strip()
|
||||
canon = (data.get("canonical_name") or "").strip()
|
||||
if not raw or not canon:
|
||||
return JSONResponse({"error": "raw_name and canonical_name are required"}, 400)
|
||||
result = appstate.meta_db.set_artist_alias(raw, canon, (data.get("mb_artist_id") or None))
|
||||
if not result.get("ok"):
|
||||
# Would form a cycle (raw → … → raw) — refuse rather than corrupt the chain.
|
||||
return JSONResponse(
|
||||
{"error": "alias would create a cycle", "raw_name": raw, "canonical_name": canon},
|
||||
409)
|
||||
return {"ok": True, "raw_name": raw, "canonical_name": result.get("canonical_name", canon)}
|
||||
|
||||
|
||||
@router.post("/api/artist-aliases/merge")
|
||||
def merge_artist_aliases(data: dict):
|
||||
"""Merge several raw artist variants into one canonical:
|
||||
{raw_names: [...], canonical_name}. The canonical's own self-alias is skipped.
|
||||
Returns {merged: N}."""
|
||||
canon = (data.get("canonical_name") or "").strip()
|
||||
raws = data.get("raw_names")
|
||||
if not canon:
|
||||
return JSONResponse({"error": "canonical_name is required"}, 400)
|
||||
if not isinstance(raws, list) or not raws:
|
||||
return JSONResponse({"error": "raw_names must be a non-empty array"}, 400)
|
||||
n = appstate.meta_db.merge_artists(raws, canon)
|
||||
return {"merged": n, "canonical_name": canon}
|
||||
|
||||
|
||||
@router.delete("/api/artist-aliases/{raw_name:path}")
|
||||
def delete_artist_alias(raw_name: str):
|
||||
"""Remove one override so that raw artist stands on its own again."""
|
||||
appstate.meta_db.remove_artist_alias(raw_name)
|
||||
return {"ok": True}
|
||||
@@ -0,0 +1,80 @@
|
||||
"""Audio-effects mapping API — the core-owned song/tone -> provider routing index.
|
||||
|
||||
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
|
||||
(``@app`` -> ``@router``) and the singleton read (``audio_effect_mappings`` ->
|
||||
``appstate.audio_effect_mappings``) changed. The read must stay a module
|
||||
attribute so a re-imported ``server`` re-publishes a fresh DB into the seam and
|
||||
`monkeypatch.setattr` reaches this module — see ``appstate.py``.
|
||||
"""
|
||||
|
||||
from fastapi import APIRouter, Body, Query
|
||||
from fastapi.responses import JSONResponse
|
||||
|
||||
import appstate
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
def _audio_effects_error(exc: Exception):
|
||||
return JSONResponse({"error": str(exc)}, status_code=400)
|
||||
|
||||
|
||||
@router.get("/api/audio-effects/mappings")
|
||||
def list_audio_effect_mappings(
|
||||
song_key: str = Query(""),
|
||||
filename: str = Query(""),
|
||||
tone_key: str = Query(""),
|
||||
provider_id: str = Query(""),
|
||||
):
|
||||
try:
|
||||
return {
|
||||
"mappings": appstate.audio_effect_mappings.list(
|
||||
song_key=song_key,
|
||||
filename=filename,
|
||||
tone_key=tone_key,
|
||||
provider_id=provider_id,
|
||||
)
|
||||
}
|
||||
except ValueError as exc:
|
||||
return _audio_effects_error(exc)
|
||||
|
||||
|
||||
@router.post("/api/audio-effects/mappings")
|
||||
def upsert_audio_effect_mapping(data: dict = Body(...)):
|
||||
try:
|
||||
mapping = appstate.audio_effect_mappings.upsert(data)
|
||||
except ValueError as exc:
|
||||
return _audio_effects_error(exc)
|
||||
return {"ok": True, "mapping": mapping}
|
||||
|
||||
|
||||
@router.delete("/api/audio-effects/mappings/{mapping_id}")
|
||||
def delete_audio_effect_mapping(mapping_id: int, provider_id: str = Query("")):
|
||||
try:
|
||||
deleted = appstate.audio_effect_mappings.delete(mapping_id, provider_id=provider_id)
|
||||
except ValueError as exc:
|
||||
return _audio_effects_error(exc)
|
||||
if not deleted:
|
||||
return JSONResponse({"error": "mapping not found"}, status_code=404)
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
@router.post("/api/audio-effects/mappings/{mapping_id}/activate")
|
||||
def activate_audio_effect_mapping(mapping_id: int, data: dict = Body(default_factory=dict)):
|
||||
try:
|
||||
provider_id = data.get("provider_id") if "provider_id" in data else data.get("providerId")
|
||||
mapping = appstate.audio_effect_mappings.activate(mapping_id, provider_id="" if provider_id is None else provider_id)
|
||||
except ValueError as exc:
|
||||
return _audio_effects_error(exc)
|
||||
if not mapping:
|
||||
return JSONResponse({"error": "mapping not found"}, status_code=404)
|
||||
return {"ok": True, "mapping": mapping}
|
||||
|
||||
|
||||
@router.delete("/api/audio-effects/active-mapping")
|
||||
def clear_audio_effect_active_mapping(song_key: str = Query(...), tone_key: str = Query("")):
|
||||
try:
|
||||
cleared = appstate.audio_effect_mappings.clear_active(song_key=song_key, tone_key=tone_key)
|
||||
except ValueError as exc:
|
||||
return _audio_effects_error(exc)
|
||||
return {"ok": True, "cleared": cleared}
|
||||
@@ -0,0 +1,60 @@
|
||||
"""Practice loops — saved A/B regions per song.
|
||||
|
||||
Extracted verbatim from ``server.py`` (R3); only the decorator receiver
|
||||
(``@app`` -> ``@router``) and the singleton reads (``meta_db`` ->
|
||||
``appstate.meta_db``) changed. See ``appstate.py`` for why the reads stay
|
||||
module attributes.
|
||||
"""
|
||||
|
||||
from fastapi import APIRouter
|
||||
|
||||
import appstate
|
||||
|
||||
router = APIRouter()
|
||||
|
||||
|
||||
@router.get("/api/loops")
|
||||
def list_loops(filename: str):
|
||||
# Hold the DB lock for the read: the shared single connection
|
||||
# (check_same_thread=False) is serialized through meta_db._lock by every
|
||||
# writer, so an unlocked SELECT here can overlap a POST/DELETE commit.
|
||||
db = appstate.meta_db
|
||||
with db._lock:
|
||||
rows = db.conn.execute(
|
||||
"SELECT id, name, start_time, end_time FROM loops WHERE filename = ? ORDER BY start_time",
|
||||
(filename,)
|
||||
).fetchall()
|
||||
return [{"id": r[0], "name": r[1], "start": r[2], "end": r[3]} for r in rows]
|
||||
|
||||
|
||||
@router.post("/api/loops")
|
||||
def save_loop(data: dict):
|
||||
filename = data.get("filename", "")
|
||||
name = data.get("name", "").strip()
|
||||
start = data.get("start")
|
||||
end = data.get("end")
|
||||
if not filename or start is None or end is None:
|
||||
return {"error": "Missing fields"}
|
||||
db = appstate.meta_db
|
||||
with db._lock:
|
||||
# COUNT + INSERT under one lock so two unnamed POSTs can't read the same
|
||||
# count and both mint "Loop N" (the count is only used to name the row).
|
||||
if not name:
|
||||
count = db.conn.execute(
|
||||
"SELECT COUNT(*) FROM loops WHERE filename = ?", (filename,)
|
||||
).fetchone()[0]
|
||||
name = f"Loop {count + 1}"
|
||||
db.conn.execute(
|
||||
"INSERT INTO loops (filename, name, start_time, end_time) VALUES (?, ?, ?, ?)",
|
||||
(filename, name, float(start), float(end))
|
||||
)
|
||||
db.conn.commit()
|
||||
return {"ok": True, "name": name}
|
||||
|
||||
|
||||
@router.delete("/api/loops/{loop_id}")
|
||||
def delete_loop(loop_id: int):
|
||||
with appstate.meta_db._lock:
|
||||
appstate.meta_db.conn.execute("DELETE FROM loops WHERE id = ?", (loop_id,))
|
||||
appstate.meta_db.conn.commit()
|
||||
return {"ok": True}
|
||||
+1
-1
@@ -3,7 +3,7 @@
|
||||
This module is deliberately kept apart from ``server.py`` so that
|
||||
``ProcessPoolExecutor`` workers can import and unpickle ``_scan_one``
|
||||
without dragging in ``server.py``'s import-time side effects
|
||||
(``configure_logging()``, ``meta_db = MetadataDB()`` opening/migrating
|
||||
(``configure_logging()``, ``meta_db = MetadataDB(CONFIG_DIR)`` opening/migrating
|
||||
SQLite, and ``register_plugin_api(app)`` registering routes).
|
||||
|
||||
The background scan spawns its pool with the ``spawn`` start method (see
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
"""The router seam (`appstate.py`).
|
||||
|
||||
The load-bearing assertion here is `test_server_wires_the_seam`: that `server`
|
||||
actually calls `appstate.configure(...)`. Every other test in this file would
|
||||
pass just fine against a seam nothing ever wires up — the same class of silent
|
||||
no-op that bit the frontend refactor twice when a scripted `setHostHooks` edit
|
||||
stopped matching its anchor. Unit tests cannot see wiring unless you make them
|
||||
look at it.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
|
||||
import pytest
|
||||
|
||||
import appstate
|
||||
|
||||
|
||||
def _close_server_dbs(mod):
|
||||
conn = getattr(getattr(mod, "meta_db", None), "conn", None)
|
||||
if conn is not None:
|
||||
getattr(mod, "_join_background_db_threads", lambda: None)()
|
||||
conn.close()
|
||||
ae_conn = getattr(getattr(mod, "audio_effect_mappings", None), "conn", None)
|
||||
if ae_conn is not None:
|
||||
ae_conn.close()
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def isolated_server(tmp_path, monkeypatch):
|
||||
"""A freshly imported `server` bound to a throwaway CONFIG_DIR.
|
||||
|
||||
Importing `server` constructs `MetadataDB` + `AudioEffectsMappingDB` at
|
||||
module level, so it MUST be re-imported under a patched CONFIG_DIR — an
|
||||
unguarded `import server` would create/mutate the developer's real
|
||||
`~/.local/share/feedback` databases. Same idiom as the other ~49
|
||||
server-importing suites.
|
||||
|
||||
Teardown restores the appstate slots as well as closing the connections:
|
||||
leaving `appstate.meta_db` published but pointing at a closed sqlite handle
|
||||
would hand a later test (or router) a live-looking, dead singleton.
|
||||
"""
|
||||
previous = (appstate.meta_db, appstate.audio_effect_mappings)
|
||||
monkeypatch.setenv("CONFIG_DIR", str(tmp_path))
|
||||
sys.modules.pop("server", None)
|
||||
mod = importlib.import_module("server")
|
||||
yield mod
|
||||
_close_server_dbs(mod)
|
||||
# Leave no half-torn-down `server` behind: the next fixture re-imports it.
|
||||
sys.modules.pop("server", None)
|
||||
appstate.configure(meta_db=previous[0], audio_effect_mappings=previous[1])
|
||||
|
||||
|
||||
def test_import_is_side_effect_free():
|
||||
"""`import appstate` must construct nothing and touch no disk.
|
||||
|
||||
This is why the ~49 fixtures that `sys.modules.pop("server")` and re-import
|
||||
(to rebuild `meta_db` under a patched CONFIG_DIR) keep working untouched:
|
||||
server owns construction, appstate only mirrors it. A singleton *owned*
|
||||
here would survive that pop and go stale.
|
||||
"""
|
||||
sys.modules.pop("appstate", None)
|
||||
fresh = importlib.import_module("appstate")
|
||||
try:
|
||||
assert fresh.meta_db is None
|
||||
assert fresh.audio_effect_mappings is None
|
||||
finally:
|
||||
sys.modules["appstate"] = appstate
|
||||
|
||||
|
||||
def test_configure_publishes_known_slots():
|
||||
sentinel = object()
|
||||
original = appstate.meta_db
|
||||
try:
|
||||
appstate.configure(meta_db=sentinel)
|
||||
assert appstate.meta_db is sentinel
|
||||
finally:
|
||||
appstate.configure(meta_db=original)
|
||||
|
||||
|
||||
def test_configure_is_idempotent():
|
||||
"""server re-imports call configure() again; the last write must win."""
|
||||
original = appstate.meta_db
|
||||
try:
|
||||
appstate.configure(meta_db="first")
|
||||
appstate.configure(meta_db="second")
|
||||
assert appstate.meta_db == "second"
|
||||
finally:
|
||||
appstate.configure(meta_db=original)
|
||||
|
||||
|
||||
def test_configure_rejects_an_unknown_slot():
|
||||
"""A typo'd or stale keyword must raise, not silently create a global that
|
||||
nothing reads. A seam whose wiring can no-op undetected is worse than none."""
|
||||
with pytest.raises(TypeError, match="unknown slot"):
|
||||
appstate.configure(met_db="typo")
|
||||
assert not hasattr(appstate, "met_db")
|
||||
|
||||
|
||||
def test_late_bound_read_sees_a_later_configure():
|
||||
"""Routers must read `appstate.meta_db`, never `from appstate import meta_db`.
|
||||
This pins the property that makes that rule work."""
|
||||
def router_style_read():
|
||||
return appstate.meta_db # module attribute, resolved at call time
|
||||
|
||||
original = appstate.meta_db
|
||||
try:
|
||||
appstate.configure(meta_db="before")
|
||||
assert router_style_read() == "before"
|
||||
appstate.configure(meta_db="after")
|
||||
assert router_style_read() == "after"
|
||||
finally:
|
||||
appstate.configure(meta_db=original)
|
||||
|
||||
|
||||
def test_server_wires_the_seam(isolated_server):
|
||||
"""The one that catches a dropped `appstate.configure(...)` call.
|
||||
|
||||
Identity, not truthiness, so a stray re-assignment or a half-applied edit
|
||||
fails here rather than in some router months later.
|
||||
"""
|
||||
assert appstate.meta_db is isolated_server.meta_db
|
||||
assert appstate.audio_effect_mappings is isolated_server.audio_effect_mappings
|
||||
assert appstate.meta_db is not None
|
||||
|
||||
|
||||
def test_reimporting_server_republishes_the_fresh_singletons(
|
||||
isolated_server, tmp_path, monkeypatch
|
||||
):
|
||||
"""The 49-fixture contract, exercised end to end.
|
||||
|
||||
Those fixtures `sys.modules.pop("server")` + re-import to rebuild `meta_db`
|
||||
under a new CONFIG_DIR, and know nothing about appstate. So the seam must
|
||||
re-publish on that second import. This is the test that would fail if
|
||||
`appstate` ever *owned* the singletons: a module-level `meta_db` there
|
||||
survives the pop and the assertions below would still see the FIRST DB.
|
||||
"""
|
||||
first_db = isolated_server.meta_db
|
||||
assert appstate.meta_db is first_db
|
||||
assert str(tmp_path) in first_db.db_path
|
||||
|
||||
second_config = tmp_path / "second"
|
||||
monkeypatch.setenv("CONFIG_DIR", str(second_config))
|
||||
sys.modules.pop("server", None)
|
||||
second_server = importlib.import_module("server")
|
||||
try:
|
||||
assert second_server.meta_db is not first_db # genuinely rebuilt
|
||||
assert str(second_config) in second_server.meta_db.db_path
|
||||
assert appstate.meta_db is second_server.meta_db # ...and re-published
|
||||
assert appstate.audio_effect_mappings is second_server.audio_effect_mappings
|
||||
finally:
|
||||
_close_server_dbs(second_server)
|
||||
sys.modules.pop("server", None)
|
||||
@@ -0,0 +1,84 @@
|
||||
"""Concurrent unnamed loop saves must get unique names.
|
||||
|
||||
`save_loop` auto-names an unnamed loop `Loop {count+1}`. If the `COUNT(*)` runs
|
||||
outside the DB lock (as it did before the fix), two simultaneous unnamed POSTs
|
||||
read the same count and both mint the same name. A `threading.Barrier` releases
|
||||
all workers into `save_loop` at once to force that interleave.
|
||||
|
||||
Single-user app, so this race is unlikely in practice — but the fix is one lock
|
||||
scope, and the test pins it.
|
||||
"""
|
||||
|
||||
import threading
|
||||
|
||||
import pytest
|
||||
|
||||
import appstate
|
||||
from metadata_db import MetadataDB
|
||||
from routers import loops
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def meta_db(tmp_path):
|
||||
prev = appstate.meta_db
|
||||
db = MetadataDB(tmp_path)
|
||||
appstate.configure(meta_db=db)
|
||||
yield db
|
||||
db.conn.close()
|
||||
appstate.configure(meta_db=prev)
|
||||
|
||||
|
||||
def test_concurrent_unnamed_saves_get_unique_names(meta_db):
|
||||
workers = 16
|
||||
barrier = threading.Barrier(workers)
|
||||
names, errors = [], []
|
||||
lock = threading.Lock()
|
||||
|
||||
def save():
|
||||
try:
|
||||
barrier.wait() # release all at once
|
||||
r = loops.save_loop({"filename": "song.feedpak", "start": 0.0, "end": 1.0})
|
||||
with lock:
|
||||
names.append(r["name"])
|
||||
except Exception as e: # noqa: BLE001 — surface any thread error
|
||||
with lock:
|
||||
errors.append(e)
|
||||
|
||||
threads = [threading.Thread(target=save) for _ in range(workers)]
|
||||
for t in threads:
|
||||
t.start()
|
||||
for t in threads:
|
||||
t.join()
|
||||
|
||||
assert not errors, errors
|
||||
assert len(names) == workers
|
||||
# The load-bearing assertion: no two auto-named loops collide.
|
||||
assert len(set(names)) == workers, f"duplicate loop names: {sorted(names)}"
|
||||
# And the DB agrees — every insert landed.
|
||||
stored = meta_db.conn.execute(
|
||||
"SELECT COUNT(*) FROM loops WHERE filename = ?", ("song.feedpak",)
|
||||
).fetchone()[0]
|
||||
assert stored == workers
|
||||
|
||||
|
||||
def test_list_and_save_do_not_error_under_interleave(meta_db):
|
||||
"""A read overlapping writes must not raise (shared connection, one lock)."""
|
||||
stop = threading.Event()
|
||||
errors = []
|
||||
|
||||
def reader():
|
||||
while not stop.is_set():
|
||||
try:
|
||||
loops.list_loops("song.feedpak")
|
||||
except Exception as e: # noqa: BLE001
|
||||
errors.append(e)
|
||||
|
||||
r = threading.Thread(target=reader)
|
||||
r.start()
|
||||
try:
|
||||
for i in range(40):
|
||||
loops.save_loop({"filename": "song.feedpak", "name": f"n{i}", "start": 0.0, "end": 1.0})
|
||||
finally:
|
||||
stop.set()
|
||||
r.join()
|
||||
assert not errors, errors
|
||||
@@ -0,0 +1,112 @@
|
||||
"""Guard: every first-party module `server.py` imports must be one the packagers copy.
|
||||
|
||||
feedback-desktop's `scripts/bundle-slopsmith.sh` copies a **hardcoded list** from
|
||||
core into the app bundle — `server.py`, `VERSION`, `lib/`, `data/`, `static/`,
|
||||
`plugins/__init__.py`. A new root-level module (say `appstate.py`) ships fine in
|
||||
Docker, imports fine under pytest, and is then *silently dropped* from the
|
||||
packaged desktop app, which dies at startup with:
|
||||
|
||||
File ".../Resources/slopsmith/server.py", line 71, in <module>
|
||||
import appstate
|
||||
ModuleNotFoundError: No module named 'appstate'
|
||||
|
||||
That shipped once. This test is why it can't ship twice: it walks `server.py`'s
|
||||
module-level imports, keeps the ones that resolve inside this repo, and asserts
|
||||
each lives under a directory every packaging path already copies wholesale.
|
||||
|
||||
If you add a first-party module for `server.py`, put it in `lib/` — the one core
|
||||
directory the Dockerfile (`COPY lib/`), `docker-compose.yml`, and the desktop
|
||||
bundler (`cp -r lib`) all copy, and that all three put on `sys.path`. If you
|
||||
genuinely need it at the repo root, you must also teach `bundle-slopsmith.sh`,
|
||||
the `Dockerfile`, `.dockerignore`, and `docker-compose.yml` about it — and then
|
||||
update `BUNDLED_ROOTS` below.
|
||||
"""
|
||||
|
||||
import ast
|
||||
import importlib.util
|
||||
import pathlib
|
||||
|
||||
import pytest
|
||||
|
||||
REPO_ROOT = pathlib.Path(__file__).resolve().parent.parent
|
||||
|
||||
# Directories every packaging path copies wholesale, plus the files copied by name.
|
||||
BUNDLED_ROOTS = ("lib", "plugins", "data", "static")
|
||||
BUNDLED_FILES = ("server.py", "main.py")
|
||||
|
||||
|
||||
def _server_toplevel_imports():
|
||||
"""Module names imported at `server.py`'s top level (not inside a function)."""
|
||||
tree = ast.parse((REPO_ROOT / "server.py").read_text())
|
||||
names = set()
|
||||
for node in tree.body: # top level only — lazy imports are fine
|
||||
if isinstance(node, ast.Import):
|
||||
names.update(a.name.split(".")[0] for a in node.names)
|
||||
elif isinstance(node, ast.ImportFrom) and node.level == 0 and node.module:
|
||||
names.add(node.module.split(".")[0])
|
||||
return sorted(names)
|
||||
|
||||
|
||||
# `spec.origin` is not always a path: built-in and frozen stdlib modules use
|
||||
# these sentinels. `Path("frozen").resolve()` would land inside the repo and
|
||||
# report `os` as first-party — so filter them before touching the filesystem.
|
||||
_NON_PATH_ORIGINS = {"built-in", "frozen", "namespace"}
|
||||
|
||||
|
||||
def _first_party_origin(name):
|
||||
"""Path of `name` if it resolves inside this repo, else None (stdlib/site-package)."""
|
||||
try:
|
||||
spec = importlib.util.find_spec(name)
|
||||
except (ImportError, ValueError):
|
||||
return None
|
||||
if spec is None:
|
||||
return None
|
||||
|
||||
if spec.origin and spec.origin not in _NON_PATH_ORIGINS:
|
||||
origin = pathlib.Path(spec.origin)
|
||||
else:
|
||||
# Namespace/frozen: fall back to the first search location, if any.
|
||||
locations = list(getattr(spec, "submodule_search_locations", None) or [])
|
||||
if not locations:
|
||||
return None
|
||||
origin = pathlib.Path(locations[0])
|
||||
|
||||
if not origin.is_absolute():
|
||||
return None # a sentinel, not a real path
|
||||
origin = origin.resolve()
|
||||
try:
|
||||
origin.relative_to(REPO_ROOT)
|
||||
except ValueError:
|
||||
return None # outside the repo → a dependency
|
||||
return origin
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", _server_toplevel_imports())
|
||||
def test_server_import_is_bundled(name):
|
||||
origin = _first_party_origin(name)
|
||||
if origin is None:
|
||||
return # stdlib or an installed dependency
|
||||
|
||||
rel = origin.relative_to(REPO_ROOT)
|
||||
if rel.as_posix() in BUNDLED_FILES or rel.parts[0] in BUNDLED_ROOTS:
|
||||
return
|
||||
|
||||
raise AssertionError(
|
||||
f"server.py imports `{name}` from {rel}, which no packager copies.\n"
|
||||
f"The desktop bundler (scripts/bundle-slopsmith.sh) copies only "
|
||||
f"{BUNDLED_FILES} and {BUNDLED_ROOTS}/, so the packaged app would die "
|
||||
f"at startup with ModuleNotFoundError: No module named '{name}'.\n"
|
||||
f"Move it under lib/, or teach bundle-slopsmith.sh + Dockerfile + "
|
||||
f".dockerignore + docker-compose.yml about it and update BUNDLED_ROOTS."
|
||||
)
|
||||
|
||||
|
||||
def test_the_seam_and_routers_live_under_lib():
|
||||
"""Pin the two that already caused a shipped break."""
|
||||
for name in ("appstate", "routers"):
|
||||
origin = _first_party_origin(name)
|
||||
assert origin is not None, f"{name} does not resolve inside the repo"
|
||||
assert origin.relative_to(REPO_ROOT).parts[0] == "lib", (
|
||||
f"{name} resolved to {origin.relative_to(REPO_ROOT)}; it must live "
|
||||
f"under lib/ or the packaged desktop app will not ship it"
|
||||
)
|
||||
@@ -50,7 +50,7 @@ def client(tmp_path, monkeypatch):
|
||||
# Point CONFIG_DIR at a per-test temp path BEFORE server's
|
||||
# import-time side effects run. server.py reads CONFIG_DIR from the
|
||||
# environment at module load (line 35) and immediately constructs
|
||||
# `meta_db = MetadataDB()` at module level, which calls
|
||||
# `meta_db = MetadataDB(CONFIG_DIR)` at module level, which calls
|
||||
# CONFIG_DIR.mkdir(...) and opens a sqlite file — a plain
|
||||
# post-import monkeypatch on server.CONFIG_DIR wouldn't catch those
|
||||
# side effects, and the real user config dir would get written to.
|
||||
|
||||
@@ -22,6 +22,11 @@ from pathlib import Path
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
# The startup DB swap lives with the DB layer it guards, not with server.py.
|
||||
# metadata_db reads no environment at import, so a plain module import is safe
|
||||
# alongside the env-patched `server_mod` fixture below.
|
||||
from metadata_db import _apply_pending_db_restore
|
||||
|
||||
|
||||
@pytest.fixture()
|
||||
def server_mod(tmp_path, monkeypatch):
|
||||
@@ -219,7 +224,7 @@ def test_apply_pending_db_restore_swaps_and_clears_sidecars(server_mod, tmp_path
|
||||
(tmp_path / "web_library.db-shm").write_bytes(b"OLD-SHM")
|
||||
(tmp_path / "web_library.db.restore").write_bytes(new_db)
|
||||
|
||||
server_mod._apply_pending_db_restore(tmp_path)
|
||||
_apply_pending_db_restore(tmp_path)
|
||||
|
||||
assert main.read_bytes() == new_db # swapped in
|
||||
assert not (tmp_path / "web_library.db.restore").exists()
|
||||
@@ -234,7 +239,7 @@ def test_apply_pending_db_restore_discards_corrupt_keeps_live(server_mod, tmp_pa
|
||||
main.write_bytes(b"LIVE-GOOD-DB")
|
||||
(tmp_path / "web_library.db.restore").write_bytes(b"SQLite format 3\x00" + b"\xff" * 64)
|
||||
|
||||
server_mod._apply_pending_db_restore(tmp_path)
|
||||
_apply_pending_db_restore(tmp_path)
|
||||
|
||||
assert main.read_bytes() == b"LIVE-GOOD-DB" # live DB preserved
|
||||
assert not (tmp_path / "web_library.db.restore").exists() # bad restore dropped
|
||||
@@ -242,7 +247,7 @@ def test_apply_pending_db_restore_discards_corrupt_keeps_live(server_mod, tmp_pa
|
||||
|
||||
def test_apply_pending_db_restore_noop_without_staging(server_mod, tmp_path):
|
||||
(tmp_path / "web_library.db").write_bytes(b"LIVE")
|
||||
server_mod._apply_pending_db_restore(tmp_path) # nothing staged
|
||||
_apply_pending_db_restore(tmp_path) # nothing staged
|
||||
assert (tmp_path / "web_library.db").read_bytes() == b"LIVE"
|
||||
|
||||
|
||||
@@ -266,7 +271,7 @@ def test_full_db_backup_restore_round_trip(client, server_mod, tmp_path):
|
||||
# Simulate a restart: close the live conn, apply the staged restore,
|
||||
# reopen — the song is back.
|
||||
server_mod.meta_db.conn.close()
|
||||
server_mod._apply_pending_db_restore(tmp_path)
|
||||
_apply_pending_db_restore(tmp_path)
|
||||
conn = sqlite3.connect(str(tmp_path / "web_library.db"))
|
||||
try:
|
||||
rows = conn.execute(
|
||||
|
||||
Reference in New Issue
Block a user