mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-09-11 09:54:29 +00:00
Compare commits
2
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
47085991de | ||
|
|
4db3db4622 |
+19
-1
@@ -49,6 +49,15 @@ def _apply_to_sloppak_manifest(manifest: dict, fields: dict) -> bool:
|
||||
if "year" in fields:
|
||||
manifest["year"] = _coerce_year(fields["year"])
|
||||
dirty = True
|
||||
# `genres` is the feedpak list field (spec 1.12.0). Accepts a list/tuple
|
||||
# (stringified item-wise) or a single name (wrapped); None leaves the
|
||||
# existing value alone, mirroring the string fields above. Sent by the
|
||||
# overwrite lane (R4b) — the manual Edit Metadata path never includes it.
|
||||
if fields.get("genres") is not None:
|
||||
raw = fields["genres"]
|
||||
manifest["genres"] = ([str(g) for g in raw]
|
||||
if isinstance(raw, (list, tuple)) else [str(raw)])
|
||||
dirty = True
|
||||
# Opportunistically declare the format version (spec §4) when we're already
|
||||
# rewriting because a metadata field was supplied. Gated on `dirty` (i.e. a
|
||||
# field was given) so this never forces a *standalone* rewrite with no fields
|
||||
@@ -102,7 +111,16 @@ def write_sloppak_metadata(path: Path, fields: dict) -> bool:
|
||||
mf = path / "manifest.yaml"
|
||||
if not mf.exists() and (path / "manifest.yml").exists():
|
||||
mf = path / "manifest.yml"
|
||||
mf.write_text(dumped, encoding="utf-8")
|
||||
# One-time backup + temp + atomic replace, exactly like the zip
|
||||
# rewriter and gap_fill_sloppak's dir branch: the FIRST backup is the
|
||||
# pristine author original and is never clobbered by a later write —
|
||||
# it's what "Revert file to original" (R4b) restores.
|
||||
backup = mf.with_name(mf.name + ".bak")
|
||||
if mf.exists() and not backup.exists():
|
||||
shutil.copy2(mf, backup)
|
||||
tmp = mf.with_name(mf.name + ".tmp")
|
||||
tmp.write_text(dumped, encoding="utf-8")
|
||||
tmp.replace(mf)
|
||||
return True
|
||||
return _rewrite_zip_manifest(path, dumped)
|
||||
|
||||
|
||||
@@ -246,6 +246,8 @@ _DEMO_BLOCKED: list[tuple[str, re.Pattern]] = [
|
||||
("GET", re.compile(r"^/api/chart/.+/fileinfo$")),
|
||||
# Gap-fill (R4a) rewrites pack files on disk — never for demo visitors.
|
||||
("POST", re.compile(r"^/api/song/.+/gap-fill$")),
|
||||
# Overwrite (R4b): revert-original rewrites the pack file from its backup.
|
||||
("POST", re.compile(r"^/api/song/.+/revert-original$")),
|
||||
# Art layer (R3): all three mutate server state / touch the network on a
|
||||
# visitor's behalf — the base64 upload writes files, the URL fetch makes the
|
||||
# server request arbitrary images, and the override delete removes files.
|
||||
@@ -910,6 +912,25 @@ class MetadataDB:
|
||||
self.conn.execute(ddl)
|
||||
except sqlite3.OperationalError:
|
||||
pass
|
||||
# Write-back provenance (R4b): one row per key actually written into a
|
||||
# pack file — gap-fill additions (old_value NULL) and overwrite
|
||||
# replacements alike — with the match source/score that authorized the
|
||||
# value. Local receipts until the spec's provenance FEP gives an
|
||||
# in-file shape. Capped at _WRITE_LOG_MAX rows (oldest pruned on
|
||||
# insert) so the ledger can't grow unbounded. Additive + idempotent.
|
||||
self.conn.execute("""
|
||||
CREATE TABLE IF NOT EXISTS write_log (
|
||||
id INTEGER PRIMARY KEY,
|
||||
filename TEXT NOT NULL,
|
||||
key TEXT NOT NULL,
|
||||
old_value TEXT,
|
||||
new_value TEXT,
|
||||
source TEXT,
|
||||
score REAL,
|
||||
ts REAL
|
||||
)
|
||||
""")
|
||||
self.conn.execute("CREATE INDEX IF NOT EXISTS idx_write_log_filename ON write_log(filename)")
|
||||
# Progression (spec 010): instrument paths, challenges, quests, the
|
||||
# Decibels wallet, and the cosmetics shop. Targets/titles live in the
|
||||
# bundled content (data/progression/); these tables hold only player
|
||||
@@ -2765,6 +2786,53 @@ class MetadataDB:
|
||||
out[k] = []
|
||||
return out
|
||||
|
||||
# ── Write-back provenance (R4b) ──────────────────────────────────────────
|
||||
|
||||
_WRITE_LOG_MAX = 5000
|
||||
|
||||
@staticmethod
|
||||
def _write_log_text(value):
|
||||
"""Normalize a logged value to TEXT: None stays NULL (a gap-filled key
|
||||
had no old value), lists keep JSON shape, everything else str()s."""
|
||||
if value is None:
|
||||
return None
|
||||
if isinstance(value, (list, tuple)):
|
||||
return json.dumps(list(value), ensure_ascii=False)
|
||||
return str(value)
|
||||
|
||||
def add_write_log(self, filename: str, entries, source=None, score=None):
|
||||
"""Record file write-backs: one row per (key, old, new) entry, all
|
||||
stamped with the source/score of the match that authorized the values.
|
||||
Prunes the oldest rows beyond _WRITE_LOG_MAX after the insert so the
|
||||
receipts table stays bounded."""
|
||||
now = time.time()
|
||||
rows = [(filename, k, self._write_log_text(old), self._write_log_text(new),
|
||||
source, score, now) for k, old, new in entries]
|
||||
if not rows:
|
||||
return
|
||||
with self._lock:
|
||||
self.conn.executemany(
|
||||
"INSERT INTO write_log (filename, key, old_value, new_value, source, score, ts) "
|
||||
"VALUES (?, ?, ?, ?, ?, ?, ?)", rows)
|
||||
# Keep the newest _WRITE_LOG_MAX rows. The subquery yields the id
|
||||
# just below the cut (NULL while under the cap → no-op delete).
|
||||
self.conn.execute(
|
||||
"DELETE FROM write_log WHERE id <= ("
|
||||
"SELECT id FROM write_log ORDER BY id DESC LIMIT 1 OFFSET ?)",
|
||||
(self._WRITE_LOG_MAX,))
|
||||
self.conn.commit()
|
||||
|
||||
def write_log_rows(self, filename: str, limit: int = 200) -> list[dict]:
|
||||
"""One song's write-back receipts, newest first (id order — ts has
|
||||
second granularity, so same-request rows would tie on it)."""
|
||||
with self._lock:
|
||||
rows = self.conn.execute(
|
||||
"SELECT id, key, old_value, new_value, source, score, ts "
|
||||
"FROM write_log WHERE filename = ? ORDER BY id DESC LIMIT ?",
|
||||
(filename, max(1, int(limit)))).fetchall()
|
||||
return [dict(zip(("id", "key", "old_value", "new_value", "source", "score", "ts"), r))
|
||||
for r in rows]
|
||||
|
||||
def enrichment_state_counts(self) -> dict:
|
||||
"""{match_state: count} over rows whose song still exists (dead rows are
|
||||
filtered at read time, matching the never-purged-on-rescan contract)."""
|
||||
@@ -9073,6 +9141,13 @@ def _default_settings():
|
||||
# surface first (they gain the most), artist = A–Z, recent = newest
|
||||
# files first.
|
||||
"enrich_review_order": "missing_first",
|
||||
# Overwrite (R4b). Whether the gap-fill endpoint may also REPLACE
|
||||
# author-set pack fields with a user-confirmed (pinned) match's values
|
||||
# — per-field confirmation in the drawer, a .bak of the original is
|
||||
# always kept, receipts land in write_log. Default OFF: without
|
||||
# opting in, writes to your files stay adds-absent-keys-only (the
|
||||
# §7 contract).
|
||||
"allow_pack_overwrite": False,
|
||||
}
|
||||
|
||||
|
||||
@@ -9243,9 +9318,12 @@ def save_settings(data: dict):
|
||||
if not math.isfinite(t) or not (0.5 <= t <= 1.01):
|
||||
return {"error": "enrich_auto_threshold must be a number between 0.5 and 1.01"}
|
||||
updates["enrich_auto_threshold"] = t
|
||||
# allow_pack_overwrite (R4b) shares the plain-boolean shape; unlike the
|
||||
# enrich toggles its default is OFF (see _default_settings).
|
||||
for _bool_key in ("enrich_src_musicbrainz", "enrich_src_caa",
|
||||
"enrich_apply_names", "enrich_apply_year",
|
||||
"enrich_apply_genres", "enrich_apply_art"):
|
||||
"enrich_apply_genres", "enrich_apply_art",
|
||||
"allow_pack_overwrite"):
|
||||
if _bool_key in data:
|
||||
raw = data[_bool_key]
|
||||
if raw is not None:
|
||||
@@ -10675,6 +10753,79 @@ def _gap_fill_proposals(cache_key: str, resolved) -> tuple[dict, str]:
|
||||
return out, ("" if out else "nothing-missing")
|
||||
|
||||
|
||||
# ── Overwrite (R4b): replace author-set fields with the confirmed match ───────
|
||||
# The §7-AMENDMENT shape (draft, pending the spec chair's ack — ships as a
|
||||
# DRAFT PR): still user-initiated + single-song, per-field with default-off
|
||||
# confirmation, values only from a match the user EXPLICITLY confirmed
|
||||
# (`manual` — an automatic match may gap-fill absent keys but is never
|
||||
# authority to destroy author bytes), gated by the default-off
|
||||
# allow_pack_overwrite setting, receipts in write_log, .bak = the pristine
|
||||
# author original + a visible Revert. Identity keys (mbid/isrc) are
|
||||
# deliberately NOT overwritable: they change only via explicit manual
|
||||
# re-match. Chart/practice fields never appear here at all.
|
||||
_OVERWRITE_KEYS = ("title", "artist", "album", "year", "genres")
|
||||
|
||||
|
||||
def _overwrite_proposals(cache_key: str, resolved) -> list[dict]:
|
||||
"""What overwrite could REPLACE for this song: allowlisted keys where the
|
||||
manifest carries an author-set value AND the user-confirmed match supplies
|
||||
a DIFFERENT one — [{key, current, proposed}] in _OVERWRITE_KEYS order.
|
||||
|
||||
Empty unless match_state == 'manual' (the librarian rule above).
|
||||
Present-but-empty values ('' / year 0 / []) are not author values — those
|
||||
stay the metadata editor's job, exactly as append-only gap-fill skips
|
||||
them."""
|
||||
if resolved is None or not resolved.exists() or not sloppak_mod.is_sloppak(resolved):
|
||||
return []
|
||||
row = meta_db.get_enrichment(cache_key)
|
||||
if not row or row.get("match_state") != "manual":
|
||||
return []
|
||||
try:
|
||||
manifest = sloppak_mod.load_manifest(resolved) or {}
|
||||
except Exception:
|
||||
return []
|
||||
out = []
|
||||
for key, canon in (("title", "canon_title"), ("artist", "canon_artist"),
|
||||
("album", "canon_album")):
|
||||
proposed = (row.get(canon) or "").strip()
|
||||
cur = manifest.get(key)
|
||||
cur_s = str(cur).strip() if cur is not None else ""
|
||||
if proposed and cur_s and cur_s != proposed:
|
||||
out.append({"key": key, "current": cur_s, "proposed": proposed})
|
||||
year = (row.get("canon_year") or "").strip()
|
||||
cur = manifest.get("year")
|
||||
cur_s = str(cur).strip() if cur is not None else ""
|
||||
if (year.isdigit() and int(year) and cur_s and cur_s != "0"
|
||||
and cur_s != str(int(year))):
|
||||
out.append({"key": "year", "current": cur_s, "proposed": int(year)})
|
||||
genres = [str(g) for g in (row.get("genres") or []) if isinstance(g, str) and g.strip()]
|
||||
cur = manifest.get("genres")
|
||||
if isinstance(cur, (list, tuple)):
|
||||
cur_list = [str(g).strip() for g in cur if str(g).strip()]
|
||||
else:
|
||||
cur_list = [str(cur).strip()] if cur is not None and str(cur).strip() else []
|
||||
if genres and cur_list and cur_list != genres:
|
||||
out.append({"key": "genres", "current": cur_list, "proposed": genres})
|
||||
return out
|
||||
|
||||
|
||||
def _song_backup_path(resolved) -> Path | None:
|
||||
"""The pack's pristine-original backup, if one exists: dir-form packs back
|
||||
up the manifest (`manifest.yaml.bak`, written once by the first metadata
|
||||
write), zip-form packs back up the whole file (`<name>.bak`). None when
|
||||
the pack has never been written to (nothing to revert)."""
|
||||
if resolved is None or not resolved.exists():
|
||||
return None
|
||||
if resolved.is_dir():
|
||||
for name in ("manifest.yaml.bak", "manifest.yml.bak"):
|
||||
b = resolved / name
|
||||
if b.exists():
|
||||
return b
|
||||
return None
|
||||
b = resolved.with_name(resolved.name + ".bak")
|
||||
return b if b.exists() else None
|
||||
|
||||
|
||||
@app.get("/api/song/{filename:path}/gap-fill")
|
||||
def get_song_gap_fill(filename: str):
|
||||
"""Preview what "Write missing info to file" would add — the Details
|
||||
@@ -10691,11 +10842,18 @@ def get_song_gap_fill(filename: str):
|
||||
pass
|
||||
proposals, reason = _gap_fill_proposals(cache_key, resolved)
|
||||
row = meta_db.get_enrichment(cache_key) or {}
|
||||
cfg = _load_config(CONFIG_DIR / "config.json") or _default_settings()
|
||||
return {
|
||||
"eligible": bool(proposals),
|
||||
"reason": reason,
|
||||
"match_state": row.get("match_state"),
|
||||
"missing": [{"key": k, "value": v} for k, v in proposals.items()],
|
||||
# Overwrite (R4b): what a user-confirmed match could REPLACE, and
|
||||
# whether the Settings gate currently allows it. has_backup drives
|
||||
# the drawer's "Revert file to original…" affordance.
|
||||
"differs": _overwrite_proposals(cache_key, resolved),
|
||||
"overwrite_allowed": cfg.get("allow_pack_overwrite") is True,
|
||||
"has_backup": _song_backup_path(resolved) is not None,
|
||||
}
|
||||
|
||||
|
||||
@@ -10703,14 +10861,36 @@ def get_song_gap_fill(filename: str):
|
||||
def post_song_gap_fill(filename: str, data: dict):
|
||||
"""Write the user-confirmed subset of the preview into the pack file.
|
||||
Proposals are recomputed under the io lock, so a key that gained an
|
||||
author value between preview and confirm is skipped, never replaced."""
|
||||
keys = (data or {}).get("keys")
|
||||
if not isinstance(keys, list) or not keys:
|
||||
return JSONResponse({"error": "keys must be a non-empty list"}, 400)
|
||||
author value between preview and confirm is skipped, never replaced.
|
||||
|
||||
`keys` (gap-fill: add absent keys) and `overwrite_keys` (R4b: replace an
|
||||
author-set value with the confirmed match's) may arrive together —
|
||||
additions append first (author bytes preserved), replacements then
|
||||
re-serialize. Overwrites are triple-gated: the default-off
|
||||
allow_pack_overwrite setting, `manual` match state (recomputed under the
|
||||
lock via _overwrite_proposals), and the _OVERWRITE_KEYS allowlist. Every
|
||||
key actually written lands in the write_log receipts table."""
|
||||
keys = (data or {}).get("keys") or []
|
||||
ow_keys = (data or {}).get("overwrite_keys") or []
|
||||
if not isinstance(keys, list) or not isinstance(ow_keys, list) or not (keys or ow_keys):
|
||||
return JSONResponse(
|
||||
{"error": "keys and/or overwrite_keys must be a non-empty list"}, 400)
|
||||
bad = [k for k in keys if k not in _GAP_FILL_KEYS]
|
||||
if bad:
|
||||
return JSONResponse(
|
||||
{"error": "unknown key(s): " + ", ".join(sorted(set(map(str, bad))))}, 400)
|
||||
bad = [k for k in ow_keys if k not in _OVERWRITE_KEYS]
|
||||
if bad:
|
||||
return JSONResponse(
|
||||
{"error": "unknown overwrite key(s): " + ", ".join(sorted(set(map(str, bad))))}, 400)
|
||||
if ow_keys:
|
||||
# The Settings gate refuses the WHOLE request (nothing partial): the
|
||||
# user explicitly asked to overwrite, so failing loudly beats silently
|
||||
# writing only the gap-fill half.
|
||||
cfg = _load_config(CONFIG_DIR / "config.json") or _default_settings()
|
||||
if cfg.get("allow_pack_overwrite") is not True:
|
||||
return JSONResponse(
|
||||
{"error": "overwriting pack fields is disabled — enable it in Settings"}, 409)
|
||||
|
||||
dlc = _get_dlc_dir()
|
||||
cache_key, resolved = filename, None
|
||||
@@ -10726,20 +10906,36 @@ def post_song_gap_fill(filename: str, data: dict):
|
||||
with _song_io_lock:
|
||||
proposals, reason = _gap_fill_proposals(cache_key, resolved)
|
||||
additions = {k: proposals[k] for k in _GAP_FILL_KEYS if k in keys and k in proposals}
|
||||
skipped = sorted(set(keys) - set(additions))
|
||||
if not additions:
|
||||
# Overwrites recomputed under the lock too: a row that lost its manual
|
||||
# pin, or a manifest edit that erased the difference, falls out here
|
||||
# and is reported skipped — never written.
|
||||
diff_map = ({d["key"]: d for d in _overwrite_proposals(cache_key, resolved)}
|
||||
if ow_keys else {})
|
||||
replace = {k: diff_map[k] for k in _OVERWRITE_KEYS if k in ow_keys and k in diff_map}
|
||||
skipped = sorted((set(keys) - set(additions)) | (set(ow_keys) - set(replace)))
|
||||
if not additions and not replace:
|
||||
return JSONResponse({"error": "nothing to write", "reason": reason,
|
||||
"skipped": skipped}, 409)
|
||||
try:
|
||||
import songmeta
|
||||
songmeta.gap_fill_sloppak(resolved, additions)
|
||||
# Order matters: append the absent keys FIRST (gap-fill preserves
|
||||
# the author's existing bytes), then re-serialize for the
|
||||
# replacements — write_sloppak_metadata re-reads the manifest, so
|
||||
# it keeps the just-appended keys. Both writers share the
|
||||
# .bak-once contract, so whichever runs first snapshots the
|
||||
# pristine original.
|
||||
if additions:
|
||||
songmeta.gap_fill_sloppak(resolved, additions)
|
||||
if replace:
|
||||
songmeta.write_sloppak_metadata(
|
||||
resolved, {k: d["proposed"] for k, d in replace.items()})
|
||||
except Exception:
|
||||
log.warning("gap-fill write failed for %s", cache_key, exc_info=True)
|
||||
return JSONResponse({"error": "write failed"}, 500)
|
||||
|
||||
# Keep the cache row consistent with what the scanner would now derive
|
||||
# (same contract as the metadata editor above): sync the columns the
|
||||
# scan reads from the keys we appended, then re-stat so the row stays
|
||||
# scan reads from the keys we wrote, then re-stat so the row stays
|
||||
# cache-fresh.
|
||||
fields = {}
|
||||
if "album" in additions:
|
||||
@@ -10748,6 +10944,13 @@ def post_song_gap_fill(filename: str, data: dict):
|
||||
fields["year"] = str(additions["year"])
|
||||
if "genres" in additions:
|
||||
fields["genre"] = additions["genres"][0]
|
||||
for k, d in replace.items():
|
||||
if k == "genres":
|
||||
fields["genre"] = d["proposed"][0]
|
||||
elif k == "year":
|
||||
fields["year"] = str(d["proposed"])
|
||||
else: # title / artist / album
|
||||
fields[k] = d["proposed"]
|
||||
with meta_db._lock:
|
||||
updates = [f"{field} = ?" for field in fields]
|
||||
params = list(fields.values())
|
||||
@@ -10763,9 +10966,103 @@ def post_song_gap_fill(filename: str, data: dict):
|
||||
f"UPDATE songs SET {', '.join(updates)} WHERE filename = ?", params)
|
||||
meta_db.conn.commit()
|
||||
|
||||
# Receipts (R4b): one row per key actually written — gap-fills carry
|
||||
# no old value, overwrites carry old + new. Source/score identify the
|
||||
# match that authorized the values.
|
||||
row = meta_db.get_enrichment(cache_key) or {}
|
||||
meta_db.add_write_log(
|
||||
cache_key,
|
||||
[(k, None, v) for k, v in additions.items()]
|
||||
+ [(k, d["current"], d["proposed"]) for k, d in replace.items()],
|
||||
source=row.get("match_source"), score=row.get("match_score"))
|
||||
|
||||
_invalidate_song_caches(cache_key)
|
||||
_kick_scan()
|
||||
return {"ok": True, "written": additions, "skipped": skipped}
|
||||
out = {"ok": True, "written": additions, "skipped": skipped}
|
||||
if ow_keys:
|
||||
# Only present when overwriting was requested, so pre-R4b consumers
|
||||
# (and their exact-shape tests) see the unchanged response.
|
||||
out["overwritten"] = {k: {"old": d["current"], "new": d["proposed"]}
|
||||
for k, d in replace.items()}
|
||||
return out
|
||||
|
||||
|
||||
@app.get("/api/song/{filename:path}/write-log")
|
||||
def get_song_write_log(filename: str):
|
||||
"""This song's write-back receipts (R4b provenance), newest first —
|
||||
read-only. The local ledger of every key gap-fill/overwrite wrote into
|
||||
the pack file, until the spec's provenance FEP gives an in-file shape."""
|
||||
dlc = _get_dlc_dir()
|
||||
cache_key = filename
|
||||
if dlc:
|
||||
resolved = _resolve_dlc_path(dlc, filename)
|
||||
if resolved is None:
|
||||
return JSONResponse({"error": "forbidden"}, 403)
|
||||
try:
|
||||
cache_key = resolved.relative_to(dlc.resolve()).as_posix()
|
||||
except ValueError:
|
||||
pass
|
||||
return {"rows": meta_db.write_log_rows(cache_key)}
|
||||
|
||||
|
||||
@app.post("/api/song/{filename:path}/revert-original")
|
||||
def post_song_revert_original(filename: str):
|
||||
"""Restore the pack file from its backup — the pristine author original
|
||||
that the FIRST metadata write snapshotted (dir form: manifest.yaml.bak →
|
||||
manifest.yaml; zip form: the whole-file .bak → the file). The backup
|
||||
itself is PRESERVED after the revert (the user may re-apply later).
|
||||
Refuses with 404 when no backup exists. Demo-blocked (middleware)."""
|
||||
dlc = _get_dlc_dir()
|
||||
cache_key, resolved = filename, None
|
||||
if dlc:
|
||||
resolved = _resolve_dlc_path(dlc, filename)
|
||||
if resolved is None:
|
||||
return JSONResponse({"error": "forbidden"}, 403)
|
||||
try:
|
||||
cache_key = resolved.relative_to(dlc.resolve()).as_posix()
|
||||
except ValueError:
|
||||
pass
|
||||
|
||||
# Confine revert to actual song packages, mirroring the write path's guard
|
||||
# (_gap_fill_proposals / _overwrite_proposals both refuse non-sloppak
|
||||
# targets). Without this, a non-package path under DLC_DIR that happens to
|
||||
# have a sibling `.bak` (or a plain directory carrying a manifest.yaml.bak)
|
||||
# would get its backup copied over it and the DB re-synced — mutating a file
|
||||
# this feature was never meant to touch. Same 404 the art/source endpoints
|
||||
# return for a non-sloppak target.
|
||||
if resolved is None or not resolved.exists() or not sloppak_mod.is_sloppak(resolved):
|
||||
return JSONResponse({"error": "not found"}, 404)
|
||||
|
||||
with _song_io_lock:
|
||||
bak = _song_backup_path(resolved)
|
||||
if bak is None:
|
||||
return JSONResponse({"error": "no backup to revert to"}, 404)
|
||||
try:
|
||||
# Copy (never move) + temp + atomic replace: the .bak survives.
|
||||
if resolved.is_dir():
|
||||
target = resolved / bak.name[: -len(".bak")]
|
||||
else:
|
||||
target = resolved
|
||||
tmp = target.with_name(target.name + ".tmp")
|
||||
shutil.copy2(bak, tmp)
|
||||
tmp.replace(target)
|
||||
except Exception:
|
||||
log.warning("revert-original failed for %s", cache_key, exc_info=True)
|
||||
return JSONResponse({"error": "revert failed"}, 500)
|
||||
|
||||
# The file's identity may have changed wholesale — re-extract and
|
||||
# re-stat so the DB row matches what the scanner would now derive
|
||||
# (the full-row mirror of the metadata editor's column sync).
|
||||
try:
|
||||
meta = _extract_meta_for_file(resolved, _get_dlc_dir)
|
||||
mtime, size = _stat_for_cache(resolved)
|
||||
meta_db.put(cache_key, mtime, size, meta)
|
||||
except Exception:
|
||||
log.warning("revert-original DB resync failed for %s", cache_key, exc_info=True)
|
||||
|
||||
_invalidate_song_caches(cache_key)
|
||||
_kick_scan()
|
||||
return {"ok": True}
|
||||
|
||||
|
||||
def _save_art_override(filename: str, img_data: bytes) -> dict:
|
||||
|
||||
@@ -785,6 +785,16 @@
|
||||
<span id="enrich-status" class="text-xs text-gray-500"></span>
|
||||
</div>
|
||||
</div>
|
||||
<!-- Writing to your files (R4a gap-fill + R4b overwrite — wired by static/v3/match-review.js) -->
|
||||
<div class="fb-srow fb-srow-stack">
|
||||
<div class="fb-srow-main">
|
||||
<div class="fb-srow-title">Writing to your files</div>
|
||||
<div class="fb-srow-desc">"Write missing info to file" in a song's details only ever adds fields the pack is missing. Overwriting additionally lets a match you confirmed replace existing fields — always per-field, never automatic.</div>
|
||||
</div>
|
||||
<div class="fb-srow-wide">
|
||||
<label class="flex items-center gap-2 text-xs text-gray-400"><input type="checkbox" id="allow-pack-overwrite" class="rounded border-gray-600 bg-dark-700 text-accent"> Allow overwriting existing pack fields — per-field confirmation, a backup of the original is always kept</label>
|
||||
</div>
|
||||
</div>
|
||||
<!-- Backup -->
|
||||
<div class="fb-srow fb-srow-stack">
|
||||
<div class="fb-srow-main">
|
||||
|
||||
@@ -406,7 +406,7 @@
|
||||
const btn = document.getElementById('enrich-match-now');
|
||||
// Boolean toggles, element id → settings key. enrich-enabled is the
|
||||
// master background switch; the rest are the R1 scraper options
|
||||
// (per-source + per-field auto-apply).
|
||||
// (per-source + per-field auto-apply) plus the R4b overwrite gate.
|
||||
const toggles = [
|
||||
['enrich-enabled', 'enrich_enabled'],
|
||||
['enrich-src-musicbrainz', 'enrich_src_musicbrainz'],
|
||||
@@ -415,6 +415,9 @@
|
||||
['enrich-apply-year', 'enrich_apply_year'],
|
||||
['enrich-apply-genres', 'enrich_apply_genres'],
|
||||
['enrich-apply-art', 'enrich_apply_art'],
|
||||
// R4b pack-overwrite gate — the one DEFAULT-OFF key in this list
|
||||
// (see the load logic below).
|
||||
['allow-pack-overwrite', 'allow_pack_overwrite'],
|
||||
].map(([id, key]) => [document.getElementById(id), key]).filter(([el]) => el);
|
||||
if (!toggles.length && !sel && !btn) return;
|
||||
(async () => {
|
||||
@@ -422,7 +425,11 @@
|
||||
const r = await fetch('/api/settings');
|
||||
if (r.ok) {
|
||||
const cfg = await r.json();
|
||||
for (const [el, key] of toggles) el.checked = cfg[key] !== false;
|
||||
// The enrich keys default ON (absent → checked); the
|
||||
// overwrite gate defaults OFF (only an explicit true ticks it).
|
||||
for (const [el, key] of toggles) {
|
||||
el.checked = key === 'allow_pack_overwrite' ? cfg[key] === true : cfg[key] !== false;
|
||||
}
|
||||
if (sel) {
|
||||
const t = Number(cfg.enrich_auto_threshold);
|
||||
const want = Number.isFinite(t) ? t : 0.9;
|
||||
|
||||
+108
-19
@@ -2462,6 +2462,7 @@
|
||||
notes: meta.notes || '', tags: (meta.tags || []).slice(),
|
||||
fav: !!song.favorite, artDataUrl: null,
|
||||
gap: null, gapSel: null, // gap-fill (R4a): preview state + selected keys
|
||||
owSel: null, // overwrite (R4b): selected differs keys — default NONE ticked
|
||||
};
|
||||
|
||||
const overlay = document.createElement('div');
|
||||
@@ -2488,39 +2489,76 @@
|
||||
if (first) { try { first.focus({ preventScroll: true }); const n = first.value.length; first.setSelectionRange(n, n); } catch (_) { /* */ } }
|
||||
}
|
||||
|
||||
// Gap-fill (R4a) block inside the drawer's Identity section: preview →
|
||||
// per-key confirm → written. Adds ABSENT keys only; the server re-checks
|
||||
// under its io lock, so this UI can never replace an author-set value.
|
||||
const GAP_KEY_LABELS = { album: 'Album', year: 'Year', genres: 'Genres', mbid: 'MusicBrainz ID', isrc: 'ISRC' };
|
||||
// Gap-fill (R4a) + overwrite (R4b) block inside the drawer's Identity
|
||||
// section: preview → per-key confirm → written. Gap-fill adds ABSENT keys
|
||||
// only (defaults all ticked); overwrite (`differs`) REPLACES author-set
|
||||
// values — gated by the Settings allow_pack_overwrite toggle + a manual
|
||||
// (user-pinned) match, and its rows default UNTICKED (the per-field
|
||||
// checkbox IS the are-you-sure). The server re-checks everything under
|
||||
// its io lock, so this UI can never write a value the server wouldn't.
|
||||
const GAP_KEY_LABELS = { title: 'Title', artist: 'Artist', album: 'Album', year: 'Year', genres: 'Genres', mbid: 'MusicBrainz ID', isrc: 'ISRC' };
|
||||
function gapFillHtml(st) {
|
||||
const g = st.gap;
|
||||
if (!g) return '<button data-gapfill-check class="text-xs text-fb-textDim hover:text-fb-text">Write missing info to file…</button>';
|
||||
if (g.loading) return '<div class="text-xs text-fb-textDim">Checking the file…</div>';
|
||||
if (g.written) {
|
||||
const names = Object.keys(g.written).map((k) => GAP_KEY_LABELS[k] || k).join(', ');
|
||||
return '<div class="text-xs text-fb-text">✓ Added to file: ' + esc(names) + '</div>';
|
||||
// Small revert affordance whenever a pristine-original backup exists.
|
||||
const revertLink = g.has_backup
|
||||
? '<div><button data-gapfill-revert class="text-[0.6875rem] text-fb-textDim hover:text-fb-text underline decoration-dotted">Revert file to original…</button></div>'
|
||||
: '';
|
||||
if (g.written || g.overwritten) {
|
||||
const names = (obj) => Object.keys(obj || {}).map((k) => GAP_KEY_LABELS[k] || k).join(', ');
|
||||
const added = names(g.written), replaced = names(g.overwritten);
|
||||
return '<div class="space-y-1">' +
|
||||
(added ? '<div class="text-xs text-fb-text">✓ Added to file: ' + esc(added) + '</div>' : '') +
|
||||
(replaced ? '<div class="text-xs text-fb-text">✓ Replaced in file: ' + esc(replaced) + '</div>' : '') +
|
||||
revertLink + '</div>';
|
||||
}
|
||||
if (!g.eligible) {
|
||||
const missing = g.missing || [], differs = g.differs || [];
|
||||
const lockedLine = (differs.length && !g.overwrite_allowed)
|
||||
? '<div class="text-[0.6875rem] text-fb-textDim">' + differs.length +
|
||||
(differs.length === 1 ? ' field differs' : ' fields differ') +
|
||||
' from the matched data — enable overwriting in Settings to change them.</div>'
|
||||
: '';
|
||||
const canOverwrite = differs.length && g.overwrite_allowed;
|
||||
if (!missing.length && !canOverwrite) {
|
||||
const why = {
|
||||
'not-sloppak': 'Only feedpak songs can be written to.',
|
||||
'no-match': 'No confirmed match yet — nothing verified to write.',
|
||||
'review': 'This song’s match is waiting for review — confirm it first.',
|
||||
'nothing-missing': 'Nothing missing — the file already has all of this.',
|
||||
}[g.reason] || 'Could not check the file. Try again.';
|
||||
return '<div class="text-xs text-fb-textDim">' + esc(why) + '</div>';
|
||||
return '<div class="space-y-1"><div class="text-xs text-fb-textDim">' + esc(why) + '</div>' + lockedLine + revertLink + '</div>';
|
||||
}
|
||||
const rows = (g.missing || []).map((m) => {
|
||||
const rows = missing.map((m) => {
|
||||
const val = Array.isArray(m.value) ? m.value.join(', ') : String(m.value);
|
||||
return '<label class="flex items-center gap-2 text-sm text-fb-text">' +
|
||||
'<input type="checkbox" data-gapfill-key="' + esc(m.key) + '"' + (st.gapSel && st.gapSel.has(m.key) ? ' checked' : '') + '>' +
|
||||
'<span class="text-fb-textDim shrink-0">' + esc(GAP_KEY_LABELS[m.key] || m.key) + '</span>' +
|
||||
'<span class="truncate" title="' + esc(val) + '">' + esc(val) + '</span></label>';
|
||||
}).join('');
|
||||
const owVal = (v) => Array.isArray(v) ? v.join(', ') : String(v);
|
||||
const owRows = canOverwrite ? differs.map((d) => {
|
||||
const cur = owVal(d.current), nxt = owVal(d.proposed);
|
||||
const pair = '“' + cur + '” → “' + nxt + '”';
|
||||
return '<label class="flex items-center gap-2 text-sm text-fb-text">' +
|
||||
'<input type="checkbox" data-ow-key="' + esc(d.key) + '"' + (st.owSel && st.owSel.has(d.key) ? ' checked' : '') + '>' +
|
||||
'<span class="text-fb-textDim shrink-0">' + esc(GAP_KEY_LABELS[d.key] || d.key) + '</span>' +
|
||||
'<span class="truncate" title="' + esc(pair) + '">' + esc(pair) + '</span></label>';
|
||||
}).join('') : '';
|
||||
return '<div class="space-y-2">' +
|
||||
'<div class="text-xs font-semibold uppercase tracking-wider text-fb-textDim">Write to file</div>' + rows +
|
||||
'<div class="text-[0.6875rem] text-fb-textDim">Only adds what’s missing — nothing already in the file is changed. A backup (.bak) is kept beside the file.</div>' +
|
||||
(missing.length
|
||||
? '<div class="text-xs font-semibold uppercase tracking-wider text-fb-textDim">Write to file</div>' + rows +
|
||||
'<div class="text-[0.6875rem] text-fb-textDim">Only adds what’s missing — nothing already in the file is changed. A backup (.bak) is kept beside the file.</div>'
|
||||
: '') +
|
||||
(owRows
|
||||
? '<div class="pt-2 border-t border-fb-border/40 space-y-2">' +
|
||||
'<div class="text-xs font-semibold uppercase tracking-wider text-fb-textDim">Overwrite existing fields</div>' + owRows +
|
||||
'<div class="text-[0.6875rem] text-yellow-500/90">Replaces what the pack author wrote. The original file is kept as a backup.</div></div>'
|
||||
: '') +
|
||||
lockedLine +
|
||||
'<div class="flex gap-2"><button data-gapfill-write class="bg-fb-primary hover:bg-fb-primaryHi text-white px-3 py-1.5 rounded-lg text-xs font-semibold">Write to file</button>' +
|
||||
'<button data-gapfill-cancel class="px-3 py-1.5 bg-fb-card/60 hover:bg-fb-card border border-fb-border/50 rounded-lg text-xs text-fb-text">Cancel</button></div></div>';
|
||||
'<button data-gapfill-cancel class="px-3 py-1.5 bg-fb-card/60 hover:bg-fb-card border border-fb-border/50 rounded-lg text-xs text-fb-text">Cancel</button></div>' +
|
||||
revertLink + '</div>';
|
||||
}
|
||||
|
||||
function detailsHtml(song, st, vocab) {
|
||||
@@ -2624,15 +2662,17 @@
|
||||
$('[data-det-save]')?.addEventListener('click', () => saveDetails(song, st));
|
||||
$('[data-det-remove]')?.addEventListener('click', () => removeFromLibrary(song));
|
||||
|
||||
// Gap-fill (R4a): user-initiated write of CONFIRMED missing info into
|
||||
// the pack file. The server recomputes proposals under its io lock, so
|
||||
// a key that gained an author value since the preview is skipped.
|
||||
// Gap-fill (R4a) + overwrite (R4b): user-initiated write of CONFIRMED
|
||||
// info into the pack file. The server recomputes proposals under its
|
||||
// io lock, so a key that gained an author value since the preview is
|
||||
// skipped, and an overwrite that lost eligibility is never written.
|
||||
$('[data-gapfill-check]')?.addEventListener('click', async () => {
|
||||
st.gap = { loading: true }; render();
|
||||
let d = null;
|
||||
try { const r = await fetch('/api/song/' + enc(song.filename) + '/gap-fill'); if (r.ok) d = await r.json(); } catch (_) { /* offline */ }
|
||||
st.gap = d || { eligible: false, reason: 'error' };
|
||||
st.gapSel = new Set(((d && d.missing) || []).map((m) => m.key));
|
||||
st.owSel = new Set(); // overwrite rows start UNTICKED — always
|
||||
render();
|
||||
});
|
||||
drawer.querySelectorAll('[data-gapfill-key]').forEach((cb) => cb.addEventListener('change', () => {
|
||||
@@ -2640,13 +2680,21 @@
|
||||
if (!st.gapSel) st.gapSel = new Set();
|
||||
if (cb.checked) st.gapSel.add(k); else st.gapSel.delete(k);
|
||||
}));
|
||||
drawer.querySelectorAll('[data-ow-key]').forEach((cb) => cb.addEventListener('change', () => {
|
||||
const k = cb.getAttribute('data-ow-key');
|
||||
if (!st.owSel) st.owSel = new Set();
|
||||
if (cb.checked) st.owSel.add(k); else st.owSel.delete(k);
|
||||
}));
|
||||
$('[data-gapfill-cancel]')?.addEventListener('click', () => { st.gap = null; render(); });
|
||||
$('[data-gapfill-write]')?.addEventListener('click', async () => {
|
||||
const keys = st.gapSel ? Array.from(st.gapSel) : [];
|
||||
if (!keys.length) return;
|
||||
const owKeys = st.owSel ? Array.from(st.owSel) : [];
|
||||
if (!keys.length && !owKeys.length) return;
|
||||
const body = { keys };
|
||||
if (owKeys.length) body.overwrite_keys = owKeys;
|
||||
let d = null, ok = false;
|
||||
try {
|
||||
const r = await fetch('/api/song/' + enc(song.filename) + '/gap-fill', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ keys }) });
|
||||
const r = await fetch('/api/song/' + enc(song.filename) + '/gap-fill', { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body) });
|
||||
ok = r.ok; d = await r.json();
|
||||
} catch (_) { /* offline */ }
|
||||
if (!ok || !d || !d.written) {
|
||||
@@ -2658,7 +2706,48 @@
|
||||
// the grid quietly.
|
||||
if (d.written.album != null) { st.al = String(d.written.album); song.album = st.al; }
|
||||
if (d.written.year != null) { st.y = String(d.written.year); song.year = d.written.year; }
|
||||
st.gap = { written: d.written }; render();
|
||||
const ow = d.overwritten || {};
|
||||
if (ow.title) { st.t = String(ow.title.new); song.title = st.t; }
|
||||
if (ow.artist) { st.a = String(ow.artist.new); song.artist = st.a; }
|
||||
if (ow.album) { st.al = String(ow.album.new); song.album = st.al; }
|
||||
if (ow.year) { st.y = String(ow.year.new); song.year = ow.year.new; }
|
||||
// A write always leaves a backup behind → keep the revert link.
|
||||
st.gap = { written: d.written, overwritten: (Object.keys(ow).length ? ow : null), has_backup: true };
|
||||
render();
|
||||
try { reload(); } catch (_) { /* not on the songs grid */ }
|
||||
});
|
||||
// Revert (R4b): restore the pack file from its pristine-original
|
||||
// backup. Confirmed like removeFromLibrary; the .bak is preserved.
|
||||
$('[data-gapfill-revert]')?.addEventListener('click', async () => {
|
||||
const title = song.title || song.filename;
|
||||
let sure;
|
||||
if (window._confirmDialog) {
|
||||
sure = await window._confirmDialog({
|
||||
title: 'Revert file to original?',
|
||||
body: '<p class="text-sm text-gray-300">Restore <span class="font-semibold text-white">' + esc(title) + '</span>'s file to what the pack author originally wrote? Everything written to the file since (added and overwritten fields) is undone.</p>' +
|
||||
'<p class="text-xs text-gray-500 mt-2">The backup is kept, so you can write the matched data again later.</p>',
|
||||
confirmText: 'Revert', cancelText: 'Cancel', danger: true,
|
||||
});
|
||||
} else { sure = window.confirm('Revert "' + title + '" to the pack author\'s original file?'); }
|
||||
if (!sure) return;
|
||||
let ok = false;
|
||||
try { const r = await fetch('/api/song/' + enc(song.filename) + '/revert-original', { method: 'POST' }); ok = r.ok; } catch (_) { /* offline */ }
|
||||
if (!ok) {
|
||||
if (window.fbNotify) { try { window.fbNotify.show({ title: 'Revert failed', message: 'Could not restore the original file. Please try again.', icon: '⚠️', accent: '#EF4444' }); } catch (e) { /* */ } }
|
||||
return;
|
||||
}
|
||||
// Refresh the drawer from the restored file, then the grid.
|
||||
try {
|
||||
const r = await fetch('/api/song/' + enc(song.filename));
|
||||
if (r.ok) {
|
||||
const m = await r.json();
|
||||
song.title = m.title || ''; song.artist = m.artist || '';
|
||||
song.album = m.album || ''; song.year = m.year;
|
||||
st.t = song.title; st.a = song.artist; st.al = song.album;
|
||||
st.y = (m.year != null && m.year !== '') ? String(m.year) : '';
|
||||
}
|
||||
} catch (_) { /* keep the stale fields; reload() below still runs */ }
|
||||
st.gap = null; st.gapSel = null; st.owSel = null; render();
|
||||
try { reload(); } catch (_) { /* not on the songs grid */ }
|
||||
});
|
||||
}
|
||||
|
||||
@@ -8,6 +8,7 @@ No network anywhere: matches are seeded straight into the enrichment cache
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
import time
|
||||
import zipfile
|
||||
|
||||
import pytest
|
||||
@@ -27,6 +28,16 @@ def server(tmp_path, monkeypatch, isolate_logging):
|
||||
try:
|
||||
yield srv
|
||||
finally:
|
||||
# A write endpoint kicks a background scan (which then kicks the
|
||||
# enrichment worker); both daemon threads share meta_db's sqlite
|
||||
# connection. Drain them before closing it — closing mid-scan
|
||||
# crashes the thread with an access violation on Windows. Network
|
||||
# is off (FEEDBACK_SKIP_STARTUP_TASKS), so both drain in ms.
|
||||
deadline = time.time() + 10
|
||||
while time.time() < deadline and (
|
||||
srv._scan_status.get("running")
|
||||
or srv._enrich_status.get("running")):
|
||||
time.sleep(0.05)
|
||||
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
|
||||
if conn is not None:
|
||||
conn.close()
|
||||
|
||||
@@ -0,0 +1,361 @@
|
||||
"""Tests for the R4b pack-field overwrite — the §7-AMENDMENT shape made
|
||||
executable: per-field confirmation with values only from a match the user
|
||||
EXPLICITLY confirmed (`manual` — an automatic match may gap-fill but never
|
||||
replace), a default-OFF Settings gate (allow_pack_overwrite), identity keys
|
||||
(mbid/isrc) never overwritable, receipts in write_log (old/new/source/score,
|
||||
pruned at 5000 rows), and .bak = the pristine author original with a working
|
||||
Revert that preserves the backup.
|
||||
|
||||
Reuses the gap-fill fixtures/helpers (tests/test_gap_fill.py); no network
|
||||
anywhere — matches are seeded straight into the enrichment cache.
|
||||
"""
|
||||
|
||||
import importlib
|
||||
import sys
|
||||
import zipfile
|
||||
|
||||
import yaml
|
||||
from fastapi.testclient import TestClient
|
||||
|
||||
from tests.test_gap_fill import ( # noqa: F401 (server/client fixtures)
|
||||
BASE_MANIFEST,
|
||||
client,
|
||||
make_dir_sloppak,
|
||||
make_zip_sloppak,
|
||||
seed_match,
|
||||
server,
|
||||
)
|
||||
|
||||
# Author-set values that all DIFFER from the seeded match (artist/album/year/
|
||||
# genres), plus a title equal to the match (equal values are never offered).
|
||||
DIFF_MANIFEST = ("# my hand-made pack\n"
|
||||
"title: Thunderstruck\n"
|
||||
"artist: ACDC # typo the match fixes\n"
|
||||
"album: Razors Edge\n"
|
||||
"year: 1991\n"
|
||||
"genres:\n"
|
||||
"- Rock\n"
|
||||
"duration: 292\n"
|
||||
"arrangements: []\n"
|
||||
"stems: []\n")
|
||||
|
||||
|
||||
def enable_overwrite(client):
|
||||
r = client.post("/api/settings", json={"allow_pack_overwrite": True})
|
||||
assert r.status_code == 200
|
||||
|
||||
|
||||
# ── differs (preview) ─────────────────────────────────────────────────────────
|
||||
|
||||
def test_differs_only_for_manual_rows(server, client):
|
||||
"""The librarian rule: an automatic match may gap-fill absent keys but is
|
||||
not authority to replace author bytes — differs is empty until the user
|
||||
pins the match."""
|
||||
make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="matched")
|
||||
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
||||
assert d["differs"] == []
|
||||
# The same row, user-pinned → the differences surface.
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
||||
got = {x["key"]: x for x in d["differs"]}
|
||||
assert set(got) == {"artist", "album", "year", "genres"}
|
||||
assert got["artist"]["current"] == "ACDC" and got["artist"]["proposed"] == "AC/DC"
|
||||
assert got["album"]["current"] == "Razors Edge"
|
||||
assert got["album"]["proposed"] == "The Razors Edge"
|
||||
assert got["year"]["current"] == "1991" and got["year"]["proposed"] == 1990
|
||||
assert got["genres"]["current"] == ["Rock"]
|
||||
assert got["genres"]["proposed"] == ["hard rock", "rock"]
|
||||
|
||||
|
||||
def test_differs_excludes_identity_keys_and_equal_values(server, client):
|
||||
"""mbid/isrc present in the file AND different from the match are still
|
||||
never offered (identity changes only via explicit re-match); a value equal
|
||||
to the match isn't a difference; an ABSENT key is a gap (missing), not a
|
||||
differ."""
|
||||
make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST +
|
||||
"mbid: 00000000-0000-4000-8000-000000000000\n"
|
||||
"isrc: USZZZ0000001\n")
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
||||
assert d["differs"] == [] # title/artist equal; mbid/isrc barred
|
||||
assert {m["key"] for m in d["missing"]} == {"album", "year", "genres"}
|
||||
|
||||
|
||||
def test_preview_reports_gate_state_and_backup(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
d = client.get("/api/song/a.sloppak/gap-fill").json()
|
||||
assert d["overwrite_allowed"] is False # default OFF
|
||||
assert d["has_backup"] is False # nothing written yet
|
||||
enable_overwrite(client)
|
||||
assert client.get("/api/song/a.sloppak/gap-fill").json()["overwrite_allowed"] is True
|
||||
|
||||
|
||||
# ── refusals ──────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_overwrite_refused_when_setting_off(server, client):
|
||||
"""The gate refuses the WHOLE request before anything is written."""
|
||||
d = make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
before = (d / "manifest.yaml").read_text(encoding="utf-8")
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": ["artist"]})
|
||||
assert r.status_code == 409
|
||||
assert (d / "manifest.yaml").read_text(encoding="utf-8") == before
|
||||
assert not (d / "manifest.yaml.bak").exists()
|
||||
|
||||
|
||||
def test_overwrite_refused_when_not_manual(server, client):
|
||||
d = make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="matched")
|
||||
enable_overwrite(client)
|
||||
before = (d / "manifest.yaml").read_text(encoding="utf-8")
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": ["artist"]})
|
||||
assert r.status_code == 409
|
||||
assert r.json()["skipped"] == ["artist"]
|
||||
assert (d / "manifest.yaml").read_text(encoding="utf-8") == before
|
||||
|
||||
|
||||
def test_overwrite_validates_keys(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
# Identity keys and unknown keys are turned away wholesale (400).
|
||||
for bad in (["mbid"], ["isrc"], ["nope"]):
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": bad})
|
||||
assert r.status_code == 400
|
||||
# Both lists empty is still a 400.
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"keys": [], "overwrite_keys": []}).status_code == 400
|
||||
|
||||
|
||||
def test_overwrite_equal_value_is_skipped(server, client):
|
||||
"""A requested key whose value already equals the match is not in differs
|
||||
→ skipped, and with nothing else to write the request 409s untouched."""
|
||||
d = make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": ["title"]})
|
||||
assert r.status_code == 409
|
||||
assert r.json()["skipped"] == ["title"]
|
||||
assert (d / "manifest.yaml").read_text(encoding="utf-8") == DIFF_MANIFEST
|
||||
|
||||
|
||||
# ── happy path ────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_overwrite_dir_form_replaces_and_keeps_pristine_bak(server, client):
|
||||
d = make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
r = client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["artist", "genres"]})
|
||||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert body["overwritten"] == {
|
||||
"artist": {"old": "ACDC", "new": "AC/DC"},
|
||||
"genres": {"old": ["Rock"], "new": ["hard rock", "rock"]},
|
||||
}
|
||||
assert body["written"] == {}
|
||||
manifest = yaml.safe_load((d / "manifest.yaml").read_text(encoding="utf-8"))
|
||||
assert manifest["artist"] == "AC/DC"
|
||||
assert manifest["genres"] == ["hard rock", "rock"]
|
||||
assert manifest["album"] == "Razors Edge" # unrequested keys untouched
|
||||
assert manifest["year"] == 1991
|
||||
# The backup is the author's pristine original…
|
||||
assert (d / "manifest.yaml.bak").read_text(encoding="utf-8") == DIFF_MANIFEST
|
||||
# …and a SECOND write never clobbers it.
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": ["album"]})
|
||||
assert r.status_code == 200
|
||||
assert (d / "manifest.yaml.bak").read_text(encoding="utf-8") == DIFF_MANIFEST
|
||||
manifest = yaml.safe_load((d / "manifest.yaml").read_text(encoding="utf-8"))
|
||||
assert manifest["album"] == "The Razors Edge"
|
||||
assert manifest["artist"] == "AC/DC" # the first write survives
|
||||
# DB sync: the songs row reflects the replaced values.
|
||||
row = client.get("/api/song/a.sloppak").json()
|
||||
assert row["artist"] == "AC/DC"
|
||||
assert row["album"] == "The Razors Edge"
|
||||
|
||||
|
||||
def test_gap_fill_and_overwrite_in_one_request(server, client):
|
||||
"""`keys` keeps working unchanged alongside `overwrite_keys`: absent keys
|
||||
append (author bytes preserved into the one pristine backup), the differing
|
||||
key is replaced."""
|
||||
d = make_dir_sloppak(server, "a.sloppak", BASE_MANIFEST + "year: 1991\n")
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
r = client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"keys": ["album", "mbid"], "overwrite_keys": ["year"]})
|
||||
assert r.status_code == 200
|
||||
body = r.json()
|
||||
assert body["written"] == {"album": "The Razors Edge",
|
||||
"mbid": "12345678-abcd-4ef0-9876-0123456789ab"}
|
||||
assert body["overwritten"] == {"year": {"old": "1991", "new": 1990}}
|
||||
manifest = yaml.safe_load((d / "manifest.yaml").read_text(encoding="utf-8"))
|
||||
assert manifest["album"] == "The Razors Edge"
|
||||
assert manifest["mbid"] == "12345678-abcd-4ef0-9876-0123456789ab"
|
||||
assert manifest["year"] == 1990
|
||||
# One request, one pristine backup: the pre-request original bytes.
|
||||
assert ((d / "manifest.yaml.bak").read_text(encoding="utf-8")
|
||||
== BASE_MANIFEST + "year: 1991\n")
|
||||
|
||||
|
||||
def test_zip_form_overwrite(server, client):
|
||||
p = make_zip_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"overwrite_keys": ["artist"]})
|
||||
assert r.status_code == 200
|
||||
with zipfile.ZipFile(p) as z:
|
||||
manifest = yaml.safe_load(z.read("manifest.yaml"))
|
||||
assert manifest["artist"] == "AC/DC"
|
||||
assert z.read("stems/full.ogg") == b"OggS-fake" # pack intact
|
||||
bak = p.with_name(p.name + ".bak")
|
||||
with zipfile.ZipFile(bak) as z:
|
||||
assert z.read("manifest.yaml").decode("utf-8") == DIFF_MANIFEST
|
||||
|
||||
|
||||
# ── write_log (provenance receipts) ───────────────────────────────────────────
|
||||
|
||||
def test_write_log_records_old_and_new(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
r = client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["artist", "year"]})
|
||||
assert r.status_code == 200
|
||||
rows = client.get("/api/song/a.sloppak/write-log").json()["rows"]
|
||||
by_key = {x["key"]: x for x in rows}
|
||||
assert by_key["artist"]["old_value"] == "ACDC"
|
||||
assert by_key["artist"]["new_value"] == "AC/DC"
|
||||
assert by_key["year"]["old_value"] == "1991"
|
||||
assert by_key["year"]["new_value"] == "1990"
|
||||
for x in rows:
|
||||
assert x["source"] == "text" and x["score"] == 1.0 and x["ts"]
|
||||
|
||||
|
||||
def test_write_log_records_gap_fills_too(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak")
|
||||
seed_match(server, "a.sloppak")
|
||||
r = client.post("/api/song/a.sloppak/gap-fill", json={"keys": ["album", "genres"]})
|
||||
assert r.status_code == 200
|
||||
rows = client.get("/api/song/a.sloppak/write-log").json()["rows"]
|
||||
by_key = {x["key"]: x for x in rows}
|
||||
assert by_key["album"]["old_value"] is None # was a gap — no old value
|
||||
assert by_key["album"]["new_value"] == "The Razors Edge"
|
||||
assert by_key["genres"]["new_value"] == '["hard rock", "rock"]'
|
||||
|
||||
|
||||
def test_write_log_endpoint_shape_and_order(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["artist"]}).status_code == 200
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["album"]}).status_code == 200
|
||||
rows = client.get("/api/song/a.sloppak/write-log").json()["rows"]
|
||||
assert [x["key"] for x in rows] == ["album", "artist"] # newest first
|
||||
assert set(rows[0]) == {"id", "key", "old_value", "new_value",
|
||||
"source", "score", "ts"}
|
||||
# Another song's rows don't bleed in.
|
||||
make_dir_sloppak(server, "b.sloppak")
|
||||
assert client.get("/api/song/b.sloppak/write-log").json()["rows"] == []
|
||||
|
||||
|
||||
def test_write_log_prunes_beyond_cap(server):
|
||||
"""The receipts table stays bounded at 5000 rows — oldest pruned first."""
|
||||
db = server.meta_db
|
||||
db.add_write_log("bulk.sloppak",
|
||||
[("album", None, str(i)) for i in range(5100)],
|
||||
source="text", score=1.0)
|
||||
n = db.conn.execute("SELECT COUNT(*) FROM write_log").fetchone()[0]
|
||||
assert n == 5000
|
||||
oldest = db.conn.execute(
|
||||
"SELECT new_value FROM write_log ORDER BY id ASC LIMIT 1").fetchone()[0]
|
||||
assert oldest == "100" # rows 0..99 fell off the bottom
|
||||
|
||||
|
||||
# ── revert ────────────────────────────────────────────────────────────────────
|
||||
|
||||
def test_revert_dir_form_restores_original_and_resyncs_db(server, client):
|
||||
d = make_dir_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["artist"]}).status_code == 200
|
||||
assert client.get("/api/song/a.sloppak").json()["artist"] == "AC/DC"
|
||||
r = client.post("/api/song/a.sloppak/revert-original")
|
||||
assert r.status_code == 200
|
||||
# Author bytes restored verbatim; the backup is PRESERVED (re-apply later).
|
||||
assert (d / "manifest.yaml").read_text(encoding="utf-8") == DIFF_MANIFEST
|
||||
assert (d / "manifest.yaml.bak").exists()
|
||||
row = client.get("/api/song/a.sloppak").json()
|
||||
assert row["artist"] == "ACDC"
|
||||
assert str(row["year"]) == "1991"
|
||||
# And the preview still offers the revert + the differences again.
|
||||
d2 = client.get("/api/song/a.sloppak/gap-fill").json()
|
||||
assert d2["has_backup"] is True
|
||||
assert {x["key"] for x in d2["differs"]} >= {"artist"}
|
||||
|
||||
|
||||
def test_revert_zip_form(server, client):
|
||||
p = make_zip_sloppak(server, "a.sloppak", DIFF_MANIFEST)
|
||||
seed_match(server, "a.sloppak", state="manual")
|
||||
enable_overwrite(client)
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"overwrite_keys": ["artist"]}).status_code == 200
|
||||
assert client.post("/api/song/a.sloppak/revert-original").status_code == 200
|
||||
with zipfile.ZipFile(p) as z:
|
||||
assert z.read("manifest.yaml").decode("utf-8") == DIFF_MANIFEST
|
||||
assert z.read("stems/full.ogg") == b"OggS-fake" # pack intact
|
||||
assert p.with_name(p.name + ".bak").exists() # backup preserved
|
||||
assert client.get("/api/song/a.sloppak").json()["artist"] == "ACDC"
|
||||
|
||||
|
||||
def test_revert_without_backup_is_404(server, client):
|
||||
make_dir_sloppak(server, "a.sloppak")
|
||||
assert client.post("/api/song/a.sloppak/revert-original").status_code == 404
|
||||
|
||||
|
||||
def test_revert_refuses_non_package_target_with_sibling_bak(server, client):
|
||||
"""A non-package file under DLC_DIR with a sibling `.bak` must NOT be
|
||||
reverted — revert mirrors the write path's is_sloppak guard, so it never
|
||||
restores a stray backup over a file the feature was not meant to touch."""
|
||||
target = server.DLC_DIR / "notes.txt"
|
||||
target.write_bytes(b"user notes, not a pack")
|
||||
(server.DLC_DIR / "notes.txt.bak").write_bytes(b"stray backup")
|
||||
r = client.post("/api/song/notes.txt/revert-original")
|
||||
assert r.status_code == 404
|
||||
# The target is left byte-for-byte untouched.
|
||||
assert target.read_bytes() == b"user notes, not a pack"
|
||||
|
||||
|
||||
def test_preview_reports_backup_after_gap_fill(server, client):
|
||||
"""Plain gap-fill (R4a) also leaves the one-time backup — the preview
|
||||
surfaces it so the drawer can offer Revert."""
|
||||
make_dir_sloppak(server, "a.sloppak")
|
||||
seed_match(server, "a.sloppak")
|
||||
assert client.post("/api/song/a.sloppak/gap-fill",
|
||||
json={"keys": ["album"]}).status_code == 200
|
||||
assert client.get("/api/song/a.sloppak/gap-fill").json()["has_backup"] is True
|
||||
|
||||
|
||||
def test_demo_mode_blocks_revert(tmp_path, monkeypatch, isolate_logging):
|
||||
"""The middleware turns revert away before any handler runs — demo
|
||||
visitors can never rewrite pack files."""
|
||||
monkeypatch.setenv("CONFIG_DIR", str(tmp_path / "config"))
|
||||
dlc = tmp_path / "dlc"
|
||||
dlc.mkdir()
|
||||
monkeypatch.setenv("DLC_DIR", str(dlc))
|
||||
monkeypatch.setenv("FEEDBACK_SKIP_STARTUP_TASKS", "1")
|
||||
monkeypatch.setenv("FEEDBACK_DEMO_MODE", "1")
|
||||
sys.modules.pop("server", None)
|
||||
srv = importlib.import_module("server")
|
||||
try:
|
||||
r = TestClient(srv.app).post("/api/song/a.sloppak/revert-original")
|
||||
assert r.status_code == 403
|
||||
finally:
|
||||
conn = getattr(getattr(srv, "meta_db", None), "conn", None)
|
||||
if conn is not None:
|
||||
conn.close()
|
||||
sys.modules.pop("server", None)
|
||||
Reference in New Issue
Block a user