Dead File Cleaner: don't flag a whole library when paths just aren't reachable (#828)
macstainless: a Plex-on-macOS user running SoulSync in Docker had all 5,250 tracks flagged "dead" even though the files exist and play in Plex. Root cause: the DB stores paths as Plex reported them (/Volumes/Core/Music/...), which don't exist inside SoulSync's container. resolve_library_file_path() returns None for "couldn't find it at any known base dir" — and for a mis-mounted library that's EVERY track, not a deletion. The job treated None as "file deleted" and created a finding per track. Fix mirrors the existing transfer-folder abort: collect unresolvable tracks, and if at least max_unresolved_fraction (default 0.5) of the library is unresolvable once it's past min_tracks_for_guard (default 25), treat it as a path-mapping/ mount problem — abort with an actionable message (Docker mount / Settings → Library → Music Paths) and create ZERO findings. A small fraction unresolvable is still reported as genuine dead files, and tiny libraries (< min) report as before. Both thresholds are configurable per the job's settings. Tests: mass-unresolvable aborts with no findings; a lone dead file among real ones is still reported; a small all-dead library still reports; thresholds configurable. 54 repair tests pass.
This commit is contained in:
parent
90174de4b2
commit
26368a80ab
2 changed files with 232 additions and 35 deletions
|
|
@ -36,7 +36,17 @@ class DeadFileCleanerJob(RepairJob):
|
||||||
icon = 'repair-icon-deadfile'
|
icon = 'repair-icon-deadfile'
|
||||||
default_enabled = True
|
default_enabled = True
|
||||||
default_interval_hours = 24
|
default_interval_hours = 24
|
||||||
default_settings = {}
|
default_settings = {
|
||||||
|
# Mass-false-positive guard: if at least this fraction of tracks resolve
|
||||||
|
# to no file on disk, treat it as a path-mapping/mount problem (SoulSync
|
||||||
|
# can't SEE the library — e.g. Docker, or library.music_paths unset for
|
||||||
|
# this environment) rather than thousands of individually-deleted files,
|
||||||
|
# and abort without creating findings. Mirrors the transfer-folder abort.
|
||||||
|
'max_unresolved_fraction': 0.5,
|
||||||
|
# ...but only once the library is at least this big — a small library can
|
||||||
|
# legitimately have a high dead fraction.
|
||||||
|
'min_tracks_for_guard': 25,
|
||||||
|
}
|
||||||
auto_fix = False
|
auto_fix = False
|
||||||
|
|
||||||
def scan(self, context: JobContext) -> JobResult:
|
def scan(self, context: JobContext) -> JobResult:
|
||||||
|
|
@ -87,9 +97,30 @@ class DeadFileCleanerJob(RepairJob):
|
||||||
if context.config_manager:
|
if context.config_manager:
|
||||||
download_folder = context.config_manager.get('soulseek.download_path', '')
|
download_folder = context.config_manager.get('soulseek.download_path', '')
|
||||||
|
|
||||||
|
# Mass-false-positive guard thresholds (see default_settings).
|
||||||
|
max_unresolved_fraction = 0.5
|
||||||
|
min_tracks_for_guard = 25
|
||||||
|
if context.config_manager:
|
||||||
|
try:
|
||||||
|
max_unresolved_fraction = float(context.config_manager.get(
|
||||||
|
self.get_config_key('max_unresolved_fraction'), 0.5))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
max_unresolved_fraction = 0.5
|
||||||
|
try:
|
||||||
|
min_tracks_for_guard = int(context.config_manager.get(
|
||||||
|
self.get_config_key('min_tracks_for_guard'), 25))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
min_tracks_for_guard = 25
|
||||||
|
|
||||||
if context.report_progress:
|
if context.report_progress:
|
||||||
context.report_progress(phase=f'Checking {total} tracks...', total=total)
|
context.report_progress(phase=f'Checking {total} tracks...', total=total)
|
||||||
|
|
||||||
|
# Collect unresolvable tracks first; decide whether they're genuine dead
|
||||||
|
# files or a systemic path problem AFTER the full pass (below). A "None"
|
||||||
|
# from the resolver means "couldn't find it at any known base dir" — which
|
||||||
|
# for a mis-mounted library is EVERY track, not a real deletion.
|
||||||
|
dead_rows = []
|
||||||
|
|
||||||
for i, row in enumerate(tracks):
|
for i, row in enumerate(tracks):
|
||||||
if context.check_stop():
|
if context.check_stop():
|
||||||
return result
|
return result
|
||||||
|
|
@ -112,44 +143,75 @@ class DeadFileCleanerJob(RepairJob):
|
||||||
config_manager=context.config_manager)
|
config_manager=context.config_manager)
|
||||||
|
|
||||||
if resolved is None:
|
if resolved is None:
|
||||||
# File is truly missing — create finding
|
dead_rows.append(row)
|
||||||
if context.report_progress:
|
|
||||||
context.report_progress(
|
|
||||||
log_line=f'Missing: {title or "Unknown"} — {os.path.basename(file_path)}',
|
|
||||||
log_type='error'
|
|
||||||
)
|
|
||||||
if context.create_finding:
|
|
||||||
try:
|
|
||||||
inserted = context.create_finding(
|
|
||||||
job_id=self.job_id,
|
|
||||||
finding_type='dead_file',
|
|
||||||
severity='warning',
|
|
||||||
entity_type='track',
|
|
||||||
entity_id=str(track_id),
|
|
||||||
file_path=file_path,
|
|
||||||
title=f'Missing file: {title or "Unknown"}',
|
|
||||||
description=f'Track "{title}" by {artist_name or "Unknown"} points to a file that no longer exists',
|
|
||||||
details={
|
|
||||||
'track_id': track_id,
|
|
||||||
'title': title,
|
|
||||||
'artist': artist_name,
|
|
||||||
'album': album_title,
|
|
||||||
'original_path': file_path,
|
|
||||||
'album_thumb_url': album_thumb or None,
|
|
||||||
'artist_thumb_url': artist_thumb or None,
|
|
||||||
}
|
|
||||||
)
|
|
||||||
if inserted:
|
|
||||||
result.findings_created += 1
|
|
||||||
else:
|
|
||||||
result.findings_skipped_dedup += 1
|
|
||||||
except Exception as e:
|
|
||||||
logger.debug("Error creating dead file finding for track %s: %s", track_id, e)
|
|
||||||
result.errors += 1
|
|
||||||
|
|
||||||
if context.update_progress and (i + 1) % 100 == 0:
|
if context.update_progress and (i + 1) % 100 == 0:
|
||||||
context.update_progress(i + 1, total)
|
context.update_progress(i + 1, total)
|
||||||
|
|
||||||
|
# Mass-false-positive guard: a large fraction of unresolvable paths almost
|
||||||
|
# always means SoulSync can't SEE the library (Docker mount, or
|
||||||
|
# Settings → Library → Music Paths not set for this environment), NOT that
|
||||||
|
# thousands of files were individually deleted. Refuse to flag and say so
|
||||||
|
# — same principle as the transfer-folder abort above. (#828: a Plex-on-
|
||||||
|
# macOS user in Docker had all 5,250 tracks flagged because their stored
|
||||||
|
# /Volumes/... paths don't exist inside the container.)
|
||||||
|
if (dead_rows
|
||||||
|
and result.scanned >= min_tracks_for_guard
|
||||||
|
and len(dead_rows) >= result.scanned * max_unresolved_fraction):
|
||||||
|
logger.error(
|
||||||
|
"Dead file scan: %d/%d tracks unresolvable (>= %.0f%%) — aborting without "
|
||||||
|
"creating findings; this is a path-mapping/mount problem, not deleted files.",
|
||||||
|
len(dead_rows), result.scanned, max_unresolved_fraction * 100)
|
||||||
|
result.errors += 1
|
||||||
|
if context.report_progress:
|
||||||
|
context.report_progress(
|
||||||
|
phase='Aborted — too many unreachable paths',
|
||||||
|
log_line=(f"{len(dead_rows)} of {result.scanned} tracks point to paths SoulSync "
|
||||||
|
f"can't reach — almost always a path-mapping issue (Docker mount, or "
|
||||||
|
f"Settings → Library → Music Paths), not deleted files. No findings created."),
|
||||||
|
log_type='error'
|
||||||
|
)
|
||||||
|
if context.update_progress:
|
||||||
|
context.update_progress(total, total)
|
||||||
|
return result
|
||||||
|
|
||||||
|
# A small fraction unresolvable — treat as genuine dead files and report.
|
||||||
|
for row in dead_rows:
|
||||||
|
track_id, title, artist_name, album_title, file_path, album_thumb, artist_thumb = row
|
||||||
|
if context.report_progress:
|
||||||
|
context.report_progress(
|
||||||
|
log_line=f'Missing: {title or "Unknown"} — {os.path.basename(file_path)}',
|
||||||
|
log_type='error'
|
||||||
|
)
|
||||||
|
if context.create_finding:
|
||||||
|
try:
|
||||||
|
inserted = context.create_finding(
|
||||||
|
job_id=self.job_id,
|
||||||
|
finding_type='dead_file',
|
||||||
|
severity='warning',
|
||||||
|
entity_type='track',
|
||||||
|
entity_id=str(track_id),
|
||||||
|
file_path=file_path,
|
||||||
|
title=f'Missing file: {title or "Unknown"}',
|
||||||
|
description=f'Track "{title}" by {artist_name or "Unknown"} points to a file that no longer exists',
|
||||||
|
details={
|
||||||
|
'track_id': track_id,
|
||||||
|
'title': title,
|
||||||
|
'artist': artist_name,
|
||||||
|
'album': album_title,
|
||||||
|
'original_path': file_path,
|
||||||
|
'album_thumb_url': album_thumb or None,
|
||||||
|
'artist_thumb_url': artist_thumb or None,
|
||||||
|
}
|
||||||
|
)
|
||||||
|
if inserted:
|
||||||
|
result.findings_created += 1
|
||||||
|
else:
|
||||||
|
result.findings_skipped_dedup += 1
|
||||||
|
except Exception as e:
|
||||||
|
logger.debug("Error creating dead file finding for track %s: %s", track_id, e)
|
||||||
|
result.errors += 1
|
||||||
|
|
||||||
if context.update_progress:
|
if context.update_progress:
|
||||||
context.update_progress(total, total)
|
context.update_progress(total, total)
|
||||||
|
|
||||||
|
|
|
||||||
135
tests/test_dead_file_cleaner_guard.py
Normal file
135
tests/test_dead_file_cleaner_guard.py
Normal file
|
|
@ -0,0 +1,135 @@
|
||||||
|
"""Dead File Cleaner — mass-false-positive guard (#828).
|
||||||
|
|
||||||
|
macstainless: a Plex-on-macOS user running SoulSync in Docker had all 5,250
|
||||||
|
tracks flagged "dead" because their stored /Volumes/... paths don't exist inside
|
||||||
|
the container. The resolver returning None means "couldn't find it at any known
|
||||||
|
base dir" — for a mis-mounted library that's EVERY track, not a real deletion.
|
||||||
|
The job now refuses to flag when a large fraction is unresolvable (a path-mapping
|
||||||
|
problem) and reports it as such, mirroring the existing transfer-folder abort.
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
from core.repair_jobs.base import JobContext
|
||||||
|
from core.repair_jobs.dead_file_cleaner import DeadFileCleanerJob
|
||||||
|
|
||||||
|
|
||||||
|
class _Cur:
|
||||||
|
def __init__(self, rows):
|
||||||
|
self._rows = rows
|
||||||
|
|
||||||
|
def execute(self, *a, **k):
|
||||||
|
pass
|
||||||
|
|
||||||
|
def fetchall(self):
|
||||||
|
return self._rows
|
||||||
|
|
||||||
|
def fetchone(self):
|
||||||
|
return [len(self._rows)]
|
||||||
|
|
||||||
|
def close(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class _Conn:
|
||||||
|
def __init__(self, rows):
|
||||||
|
self._rows = rows
|
||||||
|
|
||||||
|
def cursor(self):
|
||||||
|
return _Cur(self._rows)
|
||||||
|
|
||||||
|
def close(self):
|
||||||
|
pass
|
||||||
|
|
||||||
|
|
||||||
|
class _Db:
|
||||||
|
def __init__(self, rows):
|
||||||
|
self._rows = rows
|
||||||
|
|
||||||
|
def _get_connection(self):
|
||||||
|
return _Conn(self._rows)
|
||||||
|
|
||||||
|
|
||||||
|
class _Cfg:
|
||||||
|
def __init__(self, overrides=None):
|
||||||
|
self._o = overrides or {}
|
||||||
|
|
||||||
|
def get(self, key, default=None):
|
||||||
|
return self._o.get(key, default)
|
||||||
|
|
||||||
|
|
||||||
|
def _row(i, path):
|
||||||
|
# (track_id, title, artist, album, file_path, album_thumb, artist_thumb)
|
||||||
|
return (i, f"Track {i}", "Yellowcard", "Ocean Avenue", path, None, None)
|
||||||
|
|
||||||
|
|
||||||
|
def _run(rows, transfer_folder, cfg_overrides=None):
|
||||||
|
findings = []
|
||||||
|
cfg = _Cfg({'soulseek.download_path': '', **(cfg_overrides or {})})
|
||||||
|
ctx = JobContext(
|
||||||
|
db=_Db(rows),
|
||||||
|
transfer_folder=str(transfer_folder),
|
||||||
|
config_manager=cfg,
|
||||||
|
create_finding=lambda **kw: (findings.append(kw) or True),
|
||||||
|
)
|
||||||
|
res = DeadFileCleanerJob().scan(ctx)
|
||||||
|
return res, findings
|
||||||
|
|
||||||
|
|
||||||
|
def test_mass_unresolvable_aborts_without_findings(tmp_path):
|
||||||
|
# 30 tracks all pointing to a /Volumes path that doesn't exist in this env
|
||||||
|
# -> systemic path problem -> abort, zero findings, one error.
|
||||||
|
rows = [_row(i, f"/Volumes/Core/Music/Plex/Yellowcard/{i}.mp3") for i in range(30)]
|
||||||
|
res, findings = _run(rows, tmp_path)
|
||||||
|
assert findings == []
|
||||||
|
assert res.findings_created == 0
|
||||||
|
assert res.errors >= 1
|
||||||
|
assert res.scanned == 30
|
||||||
|
|
||||||
|
|
||||||
|
def test_few_unresolvable_creates_findings(tmp_path):
|
||||||
|
# 4 real (resolvable) files + 1 genuinely missing -> fraction 0.2 < 0.5 ->
|
||||||
|
# the one dead file IS reported.
|
||||||
|
rows = []
|
||||||
|
for i in range(4):
|
||||||
|
f = tmp_path / f"real_{i}.mp3"
|
||||||
|
f.write_text("x")
|
||||||
|
rows.append(_row(i, str(f)))
|
||||||
|
rows.append(_row(99, "/no/such/path/dead.mp3"))
|
||||||
|
res, findings = _run(rows, tmp_path,
|
||||||
|
{'repair.jobs.dead_file_cleaner.min_tracks_for_guard': 4})
|
||||||
|
assert res.findings_created == 1
|
||||||
|
assert len(findings) == 1
|
||||||
|
assert findings[0]['entity_id'] == '99'
|
||||||
|
assert res.errors == 0
|
||||||
|
|
||||||
|
|
||||||
|
def test_small_library_all_dead_still_reports(tmp_path):
|
||||||
|
# 3 dead tracks, below the default min_tracks_for_guard (25) -> guard doesn't
|
||||||
|
# apply -> all 3 reported (a tiny library can legitimately be all-dead).
|
||||||
|
rows = [_row(i, f"/no/such/{i}.mp3") for i in range(3)]
|
||||||
|
res, findings = _run(rows, tmp_path)
|
||||||
|
assert res.findings_created == 3
|
||||||
|
|
||||||
|
|
||||||
|
def test_guard_thresholds_configurable(tmp_path):
|
||||||
|
# Lower min to 4; all 4 dead -> fraction 1.0 >= 0.5 -> abort.
|
||||||
|
rows = [_row(i, f"/no/such/{i}.mp3") for i in range(4)]
|
||||||
|
res, findings = _run(rows, tmp_path,
|
||||||
|
{'repair.jobs.dead_file_cleaner.min_tracks_for_guard': 4})
|
||||||
|
assert res.findings_created == 0
|
||||||
|
assert res.errors >= 1
|
||||||
|
|
||||||
|
|
||||||
|
def test_healthy_library_no_abort_no_findings(tmp_path):
|
||||||
|
# 30 fully-resolvable tracks -> 0 dead -> neither aborts nor flags anything.
|
||||||
|
rows = []
|
||||||
|
for i in range(30):
|
||||||
|
f = tmp_path / f"ok_{i}.mp3"
|
||||||
|
f.write_text("x")
|
||||||
|
rows.append(_row(i, str(f)))
|
||||||
|
res, findings = _run(rows, tmp_path)
|
||||||
|
assert res.findings_created == 0
|
||||||
|
assert res.errors == 0
|
||||||
|
assert res.scanned == 30
|
||||||
|
assert findings == []
|
||||||
Loading…
Reference in a new issue