4d1b9a5639
The enhanced-tab "Sync" button's stale-removal phase deleted any track whose file wasn't on disk, with NO guard — so if the music storage was momentarily unavailable (sleeping NAS, dropped mount, unmounted Docker volume, WSL hiccup), os.path.exists returned False for EVERY file and one click wiped the whole artist (tracks + their now-"empty" albums) from the DB. The deep-scan path already had a 50%-stale safety net (#828); this endpoint never got one. - New core/library/stale_guard.py: is_implausible_stale_removal(missing, total) — a tested rule (skip removal when missing > 50% of a >=5-track set), centralised so every stale-removal site can share it. - sync_artist_library: if the guard trips, SKIP removal (delete nothing), return removal_skipped + warn; the frontend shows "storage may be offline — skipped" instead of silently deleting. Empty-album cleanup now also only runs on the non-skipped path and uses `album_id IS NOT NULL` (fixes the NOT IN-with-NULL no-op). Frontend also refreshes the view on additions, not just removals. - @admin_only on the endpoint — it deletes tracks + albums but was ungated, while the sibling delete_album endpoint is gated. Deep scan was already safe (different mechanism: server-diff + its own 50% guard). Tests: guard unit rules; endpoint skips removal when all files missing (keeps the tracks), removes only the genuinely-gone few otherwise, and 403s for non-admins. 7 new tests pass.
45 lines
1.8 KiB
Python
45 lines
1.8 KiB
Python
"""Guard against mass-deleting library rows when storage is unreachable.
|
|
|
|
The library "sync" / cleanup paths mark a track stale when its file isn't on
|
|
disk and then delete the row. But ``os.path.exists`` returns False for EVERY
|
|
file when the music storage is momentarily unavailable — a sleeping NAS, a
|
|
dropped network mount, an unmounted Docker volume, a WSL mount hiccup. Without a
|
|
guard, one click then wipes the whole artist/library from the DB even though the
|
|
files are fine.
|
|
|
|
This mirrors the safety the deep-scan path already had (``database_update_worker``
|
|
skips removal when stale > 50% of a >100-track library — issue #828). Centralised
|
|
here so every stale-removal site can share one tested rule.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
# Don't second-guess tiny sets — a 2-track artist legitimately losing both files
|
|
# shouldn't be blocked. Above this, an implausibly large missing fraction almost
|
|
# always means "storage down", not "files actually deleted".
|
|
DEFAULT_MIN_TOTAL = 5
|
|
DEFAULT_MAX_MISSING_FRACTION = 0.5
|
|
|
|
|
|
def is_implausible_stale_removal(
|
|
missing_count: int,
|
|
total_count: int,
|
|
*,
|
|
min_total: int = DEFAULT_MIN_TOTAL,
|
|
max_fraction: float = DEFAULT_MAX_MISSING_FRACTION,
|
|
) -> bool:
|
|
"""True when ``missing_count`` is too large a share of ``total_count`` to be a
|
|
real deletion — i.e. the storage is probably unreachable and the caller should
|
|
SKIP removal (and warn) rather than delete.
|
|
|
|
Returns False for small sets (< ``min_total``) so normal cleanup of a few
|
|
genuinely-gone files still works.
|
|
"""
|
|
if total_count <= 0 or missing_count <= 0:
|
|
return False
|
|
if total_count < min_total:
|
|
return False
|
|
return missing_count > total_count * max_fraction
|
|
|
|
|
|
__all__ = ["is_implausible_stale_removal", "DEFAULT_MIN_TOTAL", "DEFAULT_MAX_MISSING_FRACTION"]
|