diff --git a/core/library/residual_files.py b/core/library/residual_files.py new file mode 100644 index 00000000..5a15831d --- /dev/null +++ b/core/library/residual_files.py @@ -0,0 +1,53 @@ +"""What counts as a *residual* file — a leftover with no value once the audio it +accompanied is gone: OS junk, cover/scan images, and lyric/metadata sidecars. + +Single source of truth shared by: + * the **Reorganize** cleanup, which strips these from a source dir after every + track has moved out (so the empty-dir pruner can take the folder), and + * the **Empty Folder Cleaner** job, which can optionally treat a folder holding + ONLY residual files as removable (#891). + +Defining "disposable" in one place keeps the two features agreeing on what a "dead +folder" is. Pure predicates — no filesystem access — so they're unit-tested in +isolation. The whitelist is deliberately conservative: anything NOT recognized here +(a booklet ``.pdf``, a video, a ``.txt`` note) is treated as real content and kept. +""" + +from __future__ import annotations + +import os + +# OS / tooling junk. +JUNK_FILES = {'.ds_store', 'thumbs.db', 'desktop.ini', '.directory', 'album.nfo~'} +# Cover art + booklet scans. +IMAGE_EXTS = {'.jpg', '.jpeg', '.png', '.gif', '.bmp', '.webp', '.tiff', '.tif'} +# Lyric / metadata / playlist sidecars that are worthless without their audio. +SIDECAR_EXTS = {'.lrc', '.nfo', '.cue', '.m3u', '.m3u8'} + + +def _ext(name: str) -> str: + return os.path.splitext(name or '')[1].lower() + + +def is_junk(name: str) -> bool: + return (name or '').lower() in JUNK_FILES + + +def is_image(name: str) -> bool: + return _ext(name) in IMAGE_EXTS + + +def is_sidecar(name: str) -> bool: + return _ext(name) in SIDECAR_EXTS + + +def is_disposable(name: str) -> bool: + """True if this file is junk, a cover/scan image, or a lyric/metadata sidecar — + i.e. safe to delete from a folder that has no audio left.""" + return is_junk(name) or is_image(name) or is_sidecar(name) + + +__all__ = [ + 'JUNK_FILES', 'IMAGE_EXTS', 'SIDECAR_EXTS', + 'is_junk', 'is_image', 'is_sidecar', 'is_disposable', +] diff --git a/core/library_reorganize.py b/core/library_reorganize.py index e2991682..a5c9e4cd 100644 --- a/core/library_reorganize.py +++ b/core/library_reorganize.py @@ -1847,14 +1847,8 @@ def _prune_empty_album_dirs(artist_dir: str) -> None: # Sidecars that live alongside ONE audio file (same filename stem). _TRACK_SIDECAR_EXTS = ('.lrc', '.nfo', '.txt', '.cue', '.json') -# Sidecars that live at the ALBUM level (one per directory). -_ALBUM_SIDECARS = ( - 'cover.jpg', 'cover.jpeg', 'cover.png', - 'folder.jpg', 'folder.png', - 'front.jpg', 'front.png', - 'album.jpg', 'album.png', - 'artwork.jpg', 'artwork.png', -) +# Album-level leftovers (cover images, .lrc, etc.) are classified by the shared +# `core.library.residual_files.is_disposable` predicate — see `_delete_album_sidecars`. # Audio extensions used to decide whether a source directory still has # tracks the user might care about (i.e. a per-track failure left audio @@ -1954,16 +1948,30 @@ def _delete_track_sidecars(audio_path: str) -> None: def _delete_album_sidecars(src_dir: str) -> None: - """Delete album-level sidecars (cover.jpg, folder.jpg, etc.) from - `src_dir`. Used during end-of-run cleanup when no audio files remain - in the directory. Best-effort — individual failures are debug-logged.""" - for name in _ALBUM_SIDECARS: - sidecar = os.path.join(src_dir, name) - if os.path.isfile(sidecar): + """Delete album-level *residual* files from ``src_dir`` — any cover/scan image, + lyric/metadata sidecar (.lrc/.nfo/.cue/.m3u), or OS junk. Called during + end-of-run cleanup ONLY when no audio remains in the directory, so everything + here is leftover from the album that just moved (#891 — previously this only + removed a fixed list of cover names, so ``back.jpg`` / ``disc.jpg`` / ``.webp`` + survived and kept the folder un-prunable). + + Uses the shared ``is_disposable`` predicate so it agrees with the Empty Folder + Cleaner on what's a dead leftover; anything unrecognized (a booklet ``.pdf``, a + video) is deliberately LEFT. Best-effort — individual failures are debug-logged.""" + from core.library.residual_files import is_disposable + try: + entries = os.listdir(src_dir) + except OSError: + return + for name in entries: + if not is_disposable(name): + continue + full = os.path.join(src_dir, name) + if os.path.isfile(full): try: - os.remove(sidecar) + os.remove(full) except OSError as e: - logger.debug(f"[Reorganize] Couldn't remove album sidecar {sidecar}: {e}") + logger.debug(f"[Reorganize] Couldn't remove residual file {full}: {e}") def _has_remaining_audio(directory: str) -> bool: diff --git a/core/repair_jobs/empty_folder_cleaner.py b/core/repair_jobs/empty_folder_cleaner.py index 0f8f16e3..fe1f5d02 100644 --- a/core/repair_jobs/empty_folder_cleaner.py +++ b/core/repair_jobs/empty_folder_cleaner.py @@ -22,37 +22,36 @@ from __future__ import annotations import os from typing import Iterable, List +from core.library.residual_files import JUNK_FILES, is_disposable, is_junk # noqa: F401 — JUNK_FILES/is_junk re-exported from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger logger = get_logger("repair_jobs.empty_folder_cleaner") -# Files that don't count as real content — safe to delete along with the folder. -JUNK_FILES = {'.ds_store', 'thumbs.db', 'desktop.ini', '.directory', 'album.nfo~'} - - -def is_junk(name: str) -> bool: - return (name or '').lower() in JUNK_FILES - def dir_is_removable(files: Iterable[str], surviving_subdirs: Iterable[str], - *, ignore_junk: bool = True) -> bool: + *, ignore_junk: bool = True, ignore_disposable: bool = False) -> bool: """Pure: is a directory safe to remove? Removable iff it has **no surviving subdirectories** and **no real files** — where "no real files" means literally empty, or (when ``ignore_junk``) only - OS-junk files. ``surviving_subdirs`` is the list of child dirs that are NOT - themselves being removed (i.e. still hold content). + OS-junk files, or (when ``ignore_disposable`` — #891) only *residual* files: + junk + cover/scan images + lyric/metadata sidecars. ``ignore_disposable`` is the + broader opt-in that clears the cover.jpg-only folders a reorganize leaves behind. + ``surviving_subdirs`` is the list of child dirs that are NOT themselves being + removed (i.e. still hold content). """ if list(surviving_subdirs): return False files = list(files) if not files: return True - if not ignore_junk: - return False - return all(is_junk(f) for f in files) + if ignore_disposable: + return all(is_disposable(f) for f in files) + if ignore_junk: + return all(is_junk(f) for f in files) + return False @register_job @@ -65,15 +64,18 @@ class EmptyFolderCleanerJob(RepairJob): 'relocations, and deletions — empty artist/album folders, or folders that ' 'hold only OS junk like .DS_Store / Thumbs.db.\n\n' 'A finding is created for each. Applying one deletes the folder (after ' - 're-checking it is still empty). Folders that contain any real file — a ' - 'cover image, an audio track, anything — are never touched, the library ' - 'root is never removed, and it cascades: a folder left empty once its ' - 'empty children are removed is cleaned too.' + 're-checking it is still empty). Folders that contain any real file — an ' + 'audio track, a booklet, anything not recognized as a leftover — are never ' + 'touched, the library root is never removed, and it cascades: a folder left ' + 'empty once its empty children are removed is cleaned too.\n\n' + 'Enable "Also remove image/sidecar-only folders" to clear the cover.jpg / ' + '.lrc leftovers a Library Reorganize leaves behind — folders whose only ' + 'remaining files are cover/scan images or lyric/metadata sidecars.' ) icon = 'repair-icon-folder' default_enabled = False default_interval_hours = 168 # weekly — empties accrue slowly - default_settings = {'remove_junk_files': True} + default_settings = {'remove_junk_files': True, 'remove_residual_files': False} auto_fix = False def scan(self, context: JobContext) -> JobResult: @@ -86,10 +88,15 @@ class EmptyFolderCleanerJob(RepairJob): root = os.path.realpath(root) ignore_junk = True + ignore_disposable = False try: if context.config_manager: ignore_junk = bool(context.config_manager.get( 'repair.jobs.empty_folder_cleaner.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(context.config_manager.get( + 'repair.jobs.empty_folder_cleaner.remove_residual_files', False)) except Exception: # noqa: S110 — setting read is best-effort; defaults to True pass @@ -108,17 +115,29 @@ class EmptyFolderCleanerJob(RepairJob): surviving = [d for d in dirnames if os.path.join(dirpath, d) not in flagged] - if not dir_is_removable(filenames, surviving, ignore_junk=ignore_junk): + if not dir_is_removable(filenames, surviving, + ignore_junk=ignore_junk, ignore_disposable=ignore_disposable): result.skipped += 1 continue flagged.add(dirpath) junk = [f for f in filenames if is_junk(f)] + # Files that will be swept along with the folder (junk always; images/ + # sidecars only when the residual option is on). + purgeable = [f for f in filenames + if is_junk(f) or (ignore_disposable and is_disposable(f))] + residual = [f for f in purgeable if not is_junk(f)] rel = os.path.relpath(dirpath, root) if context.report_progress: context.report_progress(log_line=f'Empty folder: {rel}', log_type='info') if context.create_finding: try: + if residual: + extra = f' (only {len(residual)} leftover image/sidecar file(s))' + elif junk: + extra = f' (only {len(junk)} junk file(s))' + else: + extra = '' inserted = context.create_finding( job_id=self.job_id, finding_type='empty_folder', @@ -127,13 +146,13 @@ class EmptyFolderCleanerJob(RepairJob): entity_id=dirpath, file_path=dirpath, title=f'Empty folder: {os.path.basename(dirpath) or rel}', - description=(f'"{rel}" holds no music' - + (f' (only {len(junk)} junk file(s))' if junk else '') - + ' — safe to remove.'), + description=(f'"{rel}" holds no music' + extra + ' — safe to remove.'), details={ 'folder_path': dirpath, 'junk_files': junk, + 'purgeable_files': purgeable, 'remove_junk': ignore_junk, + 'remove_disposable': ignore_disposable, }) if inserted: result.findings_created += 1 @@ -158,12 +177,17 @@ class EmptyFolderCleanerJob(RepairJob): def remove_empty_folder(folder_path: str, *, junk_files: List[str], remove_junk: bool, - root: str, listdir, isdir, islink, remove_file, rmdir) -> dict: + root: str, listdir, isdir, islink, remove_file, rmdir, + remove_disposable: bool = False) -> dict: """Pure-ish orchestration for the apply handler — RE-CHECKS the folder is still - removable, then deletes any junk + the folder. Effects injected for testing. + removable, then deletes any purgeable leftovers + the folder. Effects injected + for testing. - Returns ``{'removed': bool, 'error': str|None}``. Refuses to touch the root, a - symlink, a non-dir, or a folder that gained real content since the scan. + With ``remove_disposable`` (#891) the re-check also treats cover images and + lyric/metadata sidecars as removable, and sweeps them before rmdir. Returns + ``{'removed': bool, 'error': str|None}``. Refuses to touch the root, a symlink, a + non-dir, or a folder that gained REAL content (audio, a booklet, anything not + recognized as residual) since the scan. """ if not folder_path or not isdir(folder_path): return {'removed': False, 'error': 'Folder no longer exists'} @@ -172,19 +196,21 @@ def remove_empty_folder(folder_path: str, *, junk_files: List[str], remove_junk: if root and os.path.realpath(folder_path) == os.path.realpath(root): return {'removed': False, 'error': 'Refusing to remove the library root'} - # Re-check at apply time: only junk/empty now? (Anything else = leave it.) + def _purgeable(e: str) -> bool: + return (remove_junk and is_junk(e)) or (remove_disposable and is_disposable(e)) + + # Re-check at apply time: only purgeable leftovers now? (Anything else = leave it.) entries = list(listdir(folder_path)) - real_entries = [e for e in entries if not (remove_junk and is_junk(e))] + real_entries = [e for e in entries if not _purgeable(e)] if real_entries: return {'removed': False, 'error': 'Folder is no longer empty — left untouched'} - if remove_junk: - for j in entries: - if is_junk(j): - try: - remove_file(os.path.join(folder_path, j)) - except Exception: # noqa: S110 — junk best-effort; rmdir below fails loudly if blocked - pass + for e in entries: + if _purgeable(e): + try: + remove_file(os.path.join(folder_path, e)) + except Exception: # noqa: S110 — leftover best-effort; rmdir below fails loudly if blocked + pass try: rmdir(folder_path) except Exception as e: diff --git a/core/repair_worker.py b/core/repair_worker.py index 1394d8b4..9a552a5f 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1567,6 +1567,7 @@ class RepairWorker: resolved, junk_files=details.get('junk_files') or [], remove_junk=bool(details.get('remove_junk', True)), + remove_disposable=bool(details.get('remove_disposable', False)), root=self.transfer_folder, listdir=os.listdir, isdir=os.path.isdir, islink=os.path.islink, remove_file=os.remove, rmdir=os.rmdir, diff --git a/tests/library/test_residual_files.py b/tests/library/test_residual_files.py new file mode 100644 index 00000000..3c030d4e --- /dev/null +++ b/tests/library/test_residual_files.py @@ -0,0 +1,51 @@ +"""#891: the shared 'residual file' classifier — junk + cover/scan images + +lyric/metadata sidecars — used by both the Reorganize cleanup and the Empty +Folder Cleaner, plus the reorganize sweep that uses it. +""" + +from __future__ import annotations + +from pathlib import Path + +from core.library.residual_files import ( + is_disposable, + is_image, + is_junk, + is_sidecar, +) + + +def test_images_classified(): + for n in ('cover.jpg', 'Cover.JPEG', 'folder.png', 'back.webp', 'scan.tiff', 'art.gif'): + assert is_image(n) and is_disposable(n) + + +def test_sidecars_classified(): + for n in ('lyrics.lrc', 'album.nfo', 'disc.cue', 'playlist.m3u', 'x.m3u8'): + assert is_sidecar(n) and is_disposable(n) + + +def test_junk_classified(): + assert is_junk('.DS_Store') and is_disposable('Thumbs.db') + + +def test_real_content_not_disposable(): + # Audio + anything unrecognized (booklet, video, a note) is real content. + for n in ('song.flac', 'track.mp3', 'booklet.pdf', 'movie.mkv', 'readme.txt', 'data.json'): + assert not is_disposable(n), n + + +# ── the reorganize sweep that uses the predicate ────────────────────────────── +def test_delete_album_sidecars_sweeps_all_residual_keeps_real(tmp_path: Path): + from core.library_reorganize import _delete_album_sidecars + + d = tmp_path / 'Old Album' + d.mkdir() + for n in ('cover.jpg', 'back.jpg', 'disc.png', 'lyrics.lrc', 'album.nfo', '.DS_Store'): + (d / n).write_text('x') + (d / 'booklet.pdf').write_text('keep') # unrecognized → must survive + + _delete_album_sidecars(str(d)) + + survivors = {p.name for p in d.iterdir()} + assert survivors == {'booklet.pdf'} # every residual swept, booklet kept diff --git a/tests/test_empty_folder_cleaner.py b/tests/test_empty_folder_cleaner.py index 54f65ca0..776a8e5a 100644 --- a/tests/test_empty_folder_cleaner.py +++ b/tests/test_empty_folder_cleaner.py @@ -36,6 +36,23 @@ def test_is_junk(): assert is_junk('.DS_Store') and is_junk('thumbs.db') and not is_junk('cover.jpg') +# ── #891: residual (image / sidecar only) folders ─────────────────────────── +def test_image_only_dir_kept_by_default_removed_with_residual_opt(): + # Default (junk only): a cover.jpg keeps the folder (the conservative behavior). + assert dir_is_removable(['cover.jpg'], []) is False + # Opt-in: image/sidecar-only folders become removable. + assert dir_is_removable(['cover.jpg'], [], ignore_disposable=True) is True + assert dir_is_removable(['back.jpg', 'lyrics.lrc', '.DS_Store'], [], ignore_disposable=True) is True + assert dir_is_removable(['folder.png', 'album.nfo'], [], ignore_disposable=True) is True + + +def test_residual_opt_still_keeps_real_content(): + # Audio, or anything not recognized as a leftover (a booklet pdf), still blocks. + assert dir_is_removable(['cover.jpg', 'song.flac'], [], ignore_disposable=True) is False + assert dir_is_removable(['cover.jpg', 'booklet.pdf'], [], ignore_disposable=True) is False + assert dir_is_removable([], ['Album'], ignore_disposable=True) is False # surviving subdir + + # ── apply re-check (real FS) ──────────────────────────────────────────────── def _fx(): return dict(listdir=os.listdir, isdir=os.path.isdir, islink=os.path.islink, @@ -72,3 +89,34 @@ def test_apply_refuses_library_root(tmp_path): res = remove_empty_folder(str(root), junk_files=[], remove_junk=True, root=str(root), **_fx()) assert res['removed'] is False and 'root' in res['error'].lower() assert root.exists() + + +def test_apply_sweeps_residual_then_folder_when_enabled(tmp_path): + root = tmp_path / 'lib'; root.mkdir() + d = root / 'Artist' / 'Old Album'; d.mkdir(parents=True) + (d / 'cover.jpg').write_text('img') + (d / 'back.jpg').write_text('img') + (d / 'lyrics.lrc').write_text('la') + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, + remove_disposable=True, root=str(root), **_fx()) + assert res['removed'] is True and not d.exists() + + +def test_apply_without_residual_opt_leaves_image_folder(tmp_path): + # The default apply (no residual opt) must NOT delete a cover.jpg folder. + root = tmp_path / 'lib'; root.mkdir() + d = root / 'HasCover'; d.mkdir() + (d / 'cover.jpg').write_text('img') + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, root=str(root), **_fx()) + assert res['removed'] is False and d.exists() + + +def test_apply_residual_opt_still_refuses_real_content(tmp_path): + root = tmp_path / 'lib'; root.mkdir() + d = root / 'Mixed'; d.mkdir() + (d / 'cover.jpg').write_text('img') + (d / 'booklet.pdf').write_text('pdf') # unrecognized → real content + res = remove_empty_folder(str(d), junk_files=[], remove_junk=True, + remove_disposable=True, root=str(root), **_fx()) + assert res['removed'] is False and d.exists() + assert (d / 'booklet.pdf').exists() and (d / 'cover.jpg').exists() # nothing deleted