#912: Empty Folder Cleaner reads its opt-in from the right config key

The #891 'also remove image/sidecar-only folders' toggle never worked. Job settings are
persisted as a nested dict under repair.jobs.<id>.settings (RepairWorker.set_job_settings),
but the scan read flat keys — repair.jobs.empty_folder_cleaner.remove_residual_files — which
never matched, so it always fell back to the False default and skipped every image/.lrc-only
folder. (remove_junk_files had the same mismatch but its default is True, which is why only the
truly-empty 'deleted' folder kept showing up.) Now reads from .settings like get_job_config /
lossy_converter do.

The pure dir_is_removable logic was already correct + tested; the bug was purely the config
read in scan(), which had no test. Added two scan-level regression tests driving the real
JobContext + a config that stores the toggle the way the UI does.
This commit is contained in:
BoulderBadgeDad 2026-06-23 11:26:00 -07:00
parent 65f73fae92
commit e301877e64
2 changed files with 65 additions and 8 deletions

View file

@ -91,13 +91,19 @@ class EmptyFolderCleanerJob(RepairJob):
ignore_disposable = False ignore_disposable = False
try: try:
if context.config_manager: if context.config_manager:
ignore_junk = bool(context.config_manager.get( # #912: job settings are persisted as a nested dict under
'repair.jobs.empty_folder_cleaner.remove_junk_files', True)) # `repair.jobs.<id>.settings` (see RepairWorker.set_job_settings / get_job_config).
# #891: also clear folders left holding only images / .lrc / sidecars # The old flat-key reads ('repair.jobs.empty_folder_cleaner.remove_residual_files')
# (what a reorganize leaves behind). Opt-in — default off. # never matched what the UI saves, so the #891 opt-in toggle silently did nothing —
ignore_disposable = bool(context.config_manager.get( # the scan always fell back to the False default and skipped every image/.lrc folder.
'repair.jobs.empty_folder_cleaner.remove_residual_files', False)) job_settings = context.config_manager.get(
except Exception: # noqa: S110 — setting read is best-effort; defaults to True 'repair.jobs.empty_folder_cleaner.settings', {}) or {}
if isinstance(job_settings, dict):
ignore_junk = bool(job_settings.get('remove_junk_files', True))
# #891: also clear folders left holding only images / .lrc / sidecars
# (what a reorganize leaves behind). Opt-in — default off.
ignore_disposable = bool(job_settings.get('remove_residual_files', False))
except Exception: # noqa: S110 — setting read is best-effort; defaults to junk-only
pass pass
flagged = set() # dir paths we'd remove → a parent sees them as "gone" flagged = set() # dir paths we'd remove → a parent sees them as "gone"

View file

@ -6,7 +6,10 @@ from __future__ import annotations
import os import os
from core.repair_jobs.empty_folder_cleaner import dir_is_removable, remove_empty_folder, is_junk from core.repair_jobs.base import JobContext
from core.repair_jobs.empty_folder_cleaner import (
EmptyFolderCleanerJob, dir_is_removable, is_junk, remove_empty_folder,
)
# ── pure decision ─────────────────────────────────────────────────────────── # ── pure decision ───────────────────────────────────────────────────────────
@ -120,3 +123,51 @@ def test_apply_residual_opt_still_refuses_real_content(tmp_path):
remove_disposable=True, root=str(root), **_fx()) remove_disposable=True, root=str(root), **_fx())
assert res['removed'] is False and d.exists() assert res['removed'] is False and d.exists()
assert (d / 'booklet.pdf').exists() and (d / 'cover.jpg').exists() # nothing deleted assert (d / 'booklet.pdf').exists() and (d / 'cover.jpg').exists() # nothing deleted
# ── #912: scan() must read the opt-in from where the UI SAVES it ─────────────
class _Cfg:
"""Mimics ConfigManager: job settings live as a nested dict under
`repair.jobs.<id>.settings` (RepairWorker.set_job_settings writes there)."""
def __init__(self, settings):
self._settings = settings
def get(self, key, default=None):
if key == 'repair.jobs.empty_folder_cleaner.settings':
return self._settings
return default
def _run_scan(tmp_path, settings):
root = tmp_path / 'lib'; root.mkdir()
res_dir = root / 'Artist' / 'Old Album'; res_dir.mkdir(parents=True)
(res_dir / 'cover.jpg').write_text('img') # image + lyric only — the #912 case
(res_dir / 'lyrics.lrc').write_text('la')
keep = root / 'Artist2' / 'Real Album'; keep.mkdir(parents=True)
(keep / 'song.flac').write_text('audio') # has audio — must never be flagged
flagged = []
ctx = JobContext(
db=None, transfer_folder=str(root), config_manager=_Cfg(settings),
create_finding=lambda **kw: (flagged.append(kw.get('file_path')), True)[1],
)
EmptyFolderCleanerJob().scan(ctx)
return str(res_dir), str(keep), set(flagged)
def test_scan_flags_residual_folder_when_opt_in_saved_under_settings(tmp_path):
# The toggle is stored at repair.jobs.<id>.settings.remove_residual_files. The scan must
# read it from THERE — the old flat-key read missed it, so the option did nothing (#912).
res_dir, keep, flagged = _run_scan(
tmp_path, {'remove_junk_files': True, 'remove_residual_files': True})
assert res_dir in flagged # the cover.jpg + .lrc folder is now found
assert keep not in flagged # the audio folder is never touched
def test_scan_keeps_residual_folder_when_opt_off(tmp_path):
# Opt-off preserves the conservative default: an image/.lrc folder is left alone.
res_dir, keep, flagged = _run_scan(
tmp_path, {'remove_junk_files': True, 'remove_residual_files': False})
assert res_dir not in flagged
assert keep not in flagged