diff --git a/app/main.py b/app/main.py index b8ada01..8f57051 100644 --- a/app/main.py +++ b/app/main.py @@ -479,6 +479,8 @@ def _list_folder_files(real): def _build_file_to_tfid(all_files, album_ids, corrupt_details, lidarr_url, lidarr_key): """Map folder files to Lidarr trackfile IDs, adding new matches to *album_ids*; None if Lidarr can't be read.""" + if _drop_wrong_lidarr_matches(corrupt_details, all_files, lidarr_url, lidarr_key) is None: + return None file_to_tfid = { fp: corrupt_details[fp]["trackfileId"] for fp in all_files @@ -530,17 +532,33 @@ def _match_artist_trackfiles(files, lidarr_url, lidarr_key): tfs = _lidarr_get_trackfiles(lidarr_url, lidarr_key, artist["id"]) if tfs is None: return None + tfs = [dict(tf, artistPath=artist["path"]) for tf in tfs] for fp in anchored: - parts = fp.split(os.sep) - anchor = parts.index(name) - # A looser match could pick a same-named track from another album and delete it. - match = _best_suffix_match(fp, tfs, min_score=len(parts) - anchor) - if match and not _another_copy_exists(fp, match["path"], anchor): + match = _pick_lidarr_copy(fp, tfs) + if match: found.setdefault(fp, []).append(match) # Artists sharing a folder name in two root folders can both match: keep neither. return {fp: ms[0] for fp, ms in found.items() if len(ms) == 1} +def _pick_lidarr_copy(fp, candidates): + """The candidate (with "path" and "artistPath") that is the file at *fp*; None if none is, or two tie.""" + return _best_suffix_match(fp, [c for c in candidates if _is_lidarr_copy(fp, c["path"], c["artistPath"])]) + + +def _is_lidarr_copy(fp, lidarr_path, artist_path): + """True if Lidarr's *lidarr_path* is the file at *fp*: the same path from the artist folder down, and no other copy.""" + artist = artist_path.replace("\\", "/").rstrip("/") + lidarr = lidarr_path.replace("\\", "/") + if not lidarr.startswith(artist + "/"): + return False + # A shorter match could pick a same-named track from another album or edition, and Lidarr would delete that one. + depth = lidarr[len(artist):].count("/") + 1 + if _suffix_match_path(fp, lidarr_path) < depth: + return False + return not _another_copy_exists(fp, lidarr_path, len(fp.split(os.sep)) - depth) + + def _another_copy_exists(fp, lidarr_path, anchor): """True if *lidarr_path* is a different file on this disk, or fp's path from the artist folder also exists higher up.""" try: @@ -2468,7 +2486,7 @@ def _load_lidarr_api_key(): def _build_lidarr_index(base_url, api_key): """Pre-fetch all Lidarr trackfiles into a basename-keyed index. - Returns ``{basename: [{id, albumId, artistId, path}, ...]}``. + Returns ``{basename: [{id, albumId, artistId, artistPath, path}, ...]}``. Queries per-artist — Lidarr's trackfile endpoint requires a filter (artistId/albumId) and rejects unfiltered requests with HTTP 400. Returns ``None`` if the artist list or any artist's track files cannot be fetched. @@ -2488,6 +2506,7 @@ def _build_lidarr_index(base_url, api_key): "id": tf["id"], "albumId": tf["albumId"], "artistId": artist["id"], + "artistPath": artist["path"], "path": tf["path"], }) logger.info(" Lidarr: indexed %d trackfiles from %d artists", @@ -2498,6 +2517,9 @@ def _build_lidarr_index(base_url, api_key): return None +_LIDARR_MATCH_KEYS = ("trackfileId", "albumId", "artistId", "lidarrPath") + + def _apply_lidarr_match(detail, match): """Copy ID fields from a Lidarr trackfile match into a detail dict.""" detail["trackfileId"] = match["id"] @@ -2509,7 +2531,7 @@ def _apply_lidarr_match(detail, match): def _resolve_single_lidarr_id(file_path, corrupt_details, lidarr_index): """Resolve Lidarr IDs for one corrupt file using the pre-built index.""" candidates = lidarr_index.get(os.path.basename(file_path)) or [] - match = _best_suffix_match(file_path, candidates, min_score=1) + match = _pick_lidarr_copy(file_path, candidates) if match and file_path in corrupt_details: _apply_lidarr_match(corrupt_details[file_path], match) @@ -2560,33 +2582,55 @@ def _resolve_lidarr_ids(corrupt_details, base_url, api_key, paths=None): logger.warning("Lidarr: could not fetch trackfiles for ID resolution") return None - # Two passes: strict (>=2 components) then loose (>=1 unambiguous). - # Strict avoids wrong matches when many files share a filename. matched = 0 - for min_score in (2, 1): - for path in list(unresolved): - candidates = index.get(os.path.basename(path)) or [] - match = _best_suffix_match(path, candidates, min_score=min_score) - if match: - _apply_lidarr_match(corrupt_details.setdefault(path, {}), match) - unresolved.remove(path) - matched += 1 + for path in unresolved: + match = _pick_lidarr_copy(path, index.get(os.path.basename(path)) or []) + if match: + _apply_lidarr_match(corrupt_details.setdefault(path, {}), match) + matched += 1 if matched: logger.info(" Lidarr: resolved %d corrupt files to trackfile IDs", matched) - if unresolved: + if len(unresolved) > matched: logger.info(" Lidarr: %d corrupt files could not be matched", - len(unresolved)) + len(unresolved) - matched) return matched +def _drop_wrong_lidarr_matches(corrupt_details, paths, base_url, api_key): + """Clear saved Lidarr matches in *paths* that aren't the file itself; returns how many, or None if Lidarr can't be read.""" + saved = [p for p in paths if "trackfileId" in corrupt_details.get(p, {})] + if not saved: + return 0 + artists = _lidarr_get_artists(base_url, api_key) + if artists is None: + logger.warning("Lidarr: could not fetch artists to re-check saved matches") + return None + artist_paths = {a["id"]: a["path"] for a in artists} + dropped = 0 + for path in saved: + detail = corrupt_details[path] + artist_path = artist_paths.get(detail.get("artistId")) + if artist_path and detail.get("lidarrPath") and _is_lidarr_copy(path, detail["lidarrPath"], artist_path): + continue + for key in _LIDARR_MATCH_KEYS: + detail.pop(key, None) + dropped += 1 + if dropped: + logger.info(" Lidarr: %d saved matches are not the flagged file; looking them up again", dropped) + return dropped + + def _lidarr_ids_before_delete(corrupt_details, paths, base_url, api_key, log_dir): - """Retry the Lidarr ID lookup for *paths*; False if Lidarr couldn't be read, so the caller must not delete.""" + """Re-check saved Lidarr matches for *paths* and look up the rest; False if Lidarr couldn't be read, so don't delete.""" + dropped = _drop_wrong_lidarr_matches(corrupt_details, paths, base_url, api_key) + if dropped is None: + return False matched = _resolve_lidarr_ids(corrupt_details, base_url, api_key, paths) if matched is None: return False - if matched: + if matched or dropped: write_json_atomic(os.path.join(log_dir, "corrupt_details.json"), corrupt_details) return True diff --git a/docs/lidarr.md b/docs/lidarr.md index a236f01..54cb000 100644 --- a/docs/lidarr.md +++ b/docs/lidarr.md @@ -12,7 +12,7 @@ lidarr_blocklist = true ## How it works -1. **Scan** — corrupt files are detected and matched to Lidarr trackfile IDs via path suffix matching. IDs are stored in `corrupt_details.json`. Before most deletes, BeatsCheck reads the album's current track files from Lidarr and matches them again, falling back to the stored ID. +1. **Scan** — corrupt files are detected and matched to Lidarr trackfile IDs (see [Path handling](#path-handling)). IDs are stored in `corrupt_details.json`. Each delete checks the stored matches of the files it deletes again, and looks up any that no longer pass. Before most deletes, BeatsCheck also reads the album's current track files from Lidarr and matches each file again by its name and folder. It keeps the stored ID when that read fails or returns no track files, and for a file that no current track file matches. 2. **Blocklist** (`LIDARR_BLOCKLIST=true`) — before deletion, the most recent grab for each affected album is marked as failed. Lidarr auto-creates a blocklist entry so the same corrupt release is not re-downloaded. **If the history lookup or marking the grab as failed fails, that album is skipped.** An album with no grab history (for example, one imported by hand) is deleted without a blocklist entry. 3. **Delete** — track files are removed via Lidarr's bulk delete API. Albums stay monitored and show as "missing". If the bulk call fails, an album with more than one track file has each one retried on its own. **If nothing is deleted, the album is skipped** (no fallback to filesystem delete). If only some retries succeed, the log counts all of the album's files as deleted, but files still on disk stay in `corrupt.txt`. 4. **Re-download** — Lidarr searches for each monitored album after its track files are deleted, and grabs a replacement if it finds one. @@ -25,7 +25,9 @@ With `LIDARR_SEARCH=true` and auto-delete, unmonitored album IDs are written to ## Path handling -BeatsCheck runs in a container where music is mounted at `/data`, while Lidarr sees the same files at the same or different mount path. When both containers mount the same host path to `/data`, the paths match exactly. At scan time, corrupt files are matched to Lidarr trackfiles by comparing path components from the right (suffix matching). Different mount paths work as long as the path ending points to one Lidarr track file. If two or more track files tie for the best match, the file stays unmatched. +BeatsCheck runs in a container where music is mounted at `/data`, while Lidarr sees the same files at the same or different mount path. A corrupt file matches a Lidarr track file only when its whole path from the artist folder down is the same as Lidarr's, so the mount paths above the artist folders can differ. A file isn't matched when Lidarr's track file is a different file on this disk, or when the same path under the artist folder also exists higher in the library (a second copy of the album). A file that shares only its name, or its album folder name, with a Lidarr track file (another edition, or another artist's album with the same title) isn't matched either. If two or more track files tie for the best match, the file stays unmatched. + +Files with no match are deleted directly, and Lidarr isn't asked to replace them. So the music folder BeatsCheck scans must hold the artist folders, as Lidarr's root folder does. If it points inside an artist folder, no file matches. ## Error handling @@ -33,8 +35,8 @@ When a Lidarr API call fails: - Marking the grab as failed fails → that album is skipped, error shown - The grab history lookup fails → that album is skipped, error shown - Bulk delete fails → each track file is retried on its own (albums with more than one); if none succeed, the album is skipped and listed for manual deletion -- If Lidarr is unreachable when a scan ends → the files it flagged get no Lidarr ID yet. Each later scan that checks at least one file tries again, and so does every delete. If that lookup fails at delete time, nothing is deleted: auto-delete tries again next run, and the WebUI and interactive delete show an error. A file Lidarr still doesn't match after a good lookup is deleted directly -- A folder delete reads each album's track files to match the rest of the folder. If Lidarr doesn't track the flagged file, it reads the track files of the artist whose folder is in the path instead, and matches each file only by its whole path under that folder. A file isn't matched when Lidarr's track file is a different file on this disk, or when the same path under the artist folder also exists higher in the library (a second copy of the album). If either read fails → nothing in that folder is deleted, error shown +- If Lidarr is unreachable when a scan ends → the files it flagged get no Lidarr ID yet. Each later scan that checks at least one file tries again, and so does every delete. The same lookup re-checks stored matches against Lidarr's artist list. If that lookup fails at delete time, nothing is deleted: auto-delete tries again next run, and the WebUI and interactive delete show an error. A file Lidarr still doesn't match after a good lookup is deleted directly +- A folder delete reads each album's track files to match the rest of the folder. If Lidarr doesn't track the flagged file, it reads the track files of the artist whose folder is in the path instead, and matches each file the same way a scan does (see [Path handling](#path-handling)). If either read fails → nothing in that folder is deleted, error shown - Files not tracked by Lidarr (cover art, .nfo, etc.) are deleted directly, not through Lidarr ## Security diff --git a/tests/test_lidarr_copy_match.py b/tests/test_lidarr_copy_match.py new file mode 100644 index 0000000..fe50753 --- /dev/null +++ b/tests/test_lidarr_copy_match.py @@ -0,0 +1,178 @@ +"""A flagged file matches a Lidarr track file only when it is that file, so a delete can't remove Lidarr's good copy.""" + +import json +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..", "app")) + +import main # noqa: E402 + +URL = "http://lidarr.invalid" +KEY = "not-a-key" +OTHER_MOUNT = "/lidarr-root.invalid" + + +def _lidarr(monkeypatch, root, tracks=("Album/01.flac",), up=True): + """Stub Lidarr with one artist at /Artist holding *tracks*, IDs from 5; returns (requests, bulk-deleted IDs).""" + calls, deleted = [], [] + + def fake(url, api_key, method="GET", data=None, timeout=30): + path = url.split("/api/v1/", 1)[1] + calls.append(f"{method} {path.split('?', 1)[0]}") + if not up: + return None + if path == "artist": + return [{"id": 1, "path": f"{root}/Artist"}] + if path.startswith("trackfile?"): + return [{"id": 5 + i, "path": f"{root}/Artist/{t}", "albumId": 7} for i, t in enumerate(tracks)] + if path == "trackfile/bulk": + deleted.extend(data["trackFileIds"]) + return {} + if path.startswith("album/"): + return {"title": "Album", "artist": {"artistName": "Artist"}} + if path == "command": + return [] + return {} + monkeypatch.setattr(main, "_lidarr_request", fake) + monkeypatch.setattr(main, "_log_delete_and_search", lambda *a, **k: None) + return calls, deleted + + +def _file(tmp_path, rel): + path = tmp_path / "data" / rel + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(b"x") + return str(path) + + +def _root(tmp_path, mount): + return str(tmp_path / "data") if mount == "same" else OTHER_MOUNT + + +def _look_up(when, fp, tmp_path): + """Match *fp* the way a scan or the lookup before a delete does; returns its details.""" + details = {fp: {"reason": "damaged"}} + if when == "scan": + main._resolve_single_lidarr_id(fp, details, main._build_lidarr_index(URL, KEY)) + else: + main._lidarr_ids_before_delete(details, [fp], URL, KEY, str(tmp_path)) + return details[fp] + + +@pytest.mark.parametrize("when", ["scan", "delete"]) +@pytest.mark.parametrize("flagged, tracked, mount", [ + ("Incoming/Artist/Album/01.flac", "Album/01.flac", "same"), + ("Incoming/Artist/Album/01.flac", "Album/01.flac", "other"), + ("Artist/Album (Deluxe)/01.flac", "Album/01.flac", "other"), + ("Artist/Album (Deluxe)/CD1/07.flac", "Album/CD1/07.flac", "other"), + ("Other/Album/01.flac", "Album/01.flac", "other"), +]) +def test_a_file_that_is_not_lidarrs_copy_gets_no_lidarr_id(tmp_path, monkeypatch, when, flagged, tracked, mount): + _file(tmp_path, f"Artist/{tracked}") + fp = _file(tmp_path, flagged) + _lidarr(monkeypatch, _root(tmp_path, mount), tracks=(tracked,)) + detail = _look_up(when, fp, tmp_path) + assert "trackfileId" not in detail + + +@pytest.mark.parametrize("when", ["scan", "delete"]) +@pytest.mark.parametrize("mount", ["same", "other"]) +def test_lidarrs_own_file_still_gets_its_id(tmp_path, monkeypatch, when, mount): + fp = _file(tmp_path, "Artist/Album/01.flac") + _lidarr(monkeypatch, _root(tmp_path, mount)) + detail = _look_up(when, fp, tmp_path) + assert detail["trackfileId"] == 5 + + +def test_a_windows_lidarr_path_still_matches(tmp_path): + fp = _file(tmp_path, "Artist/Album/01.flac") + same = main._is_lidarr_copy(fp, "D:\\Music\\Artist\\Album\\01.flac", "D:\\Music\\Artist") + assert same is True + + +def _stale_library(tmp_path): + """A second copy flagged with the ID of Lidarr's good copy, as an older scan saved it; returns (config, flagged, good).""" + good = [_file(tmp_path, f"Artist/Album/{n}.flac") for n in ("01", "02")] + flagged = _file(tmp_path, "Incoming/Artist/Album/01.flac") + _file(tmp_path, "Incoming/Artist/Album/02.flac") + cfg = tmp_path / "config" + cfg.mkdir() + (cfg / "corrupt.txt").write_text(f"{flagged}\n") + (cfg / "corrupt_details.json").write_text(json.dumps({flagged: { + "reason": "damaged", "trackfileId": 5, "albumId": 7, "artistId": 1, + "lidarrPath": f"{OTHER_MOUNT}/Artist/Album/01.flac"}})) + (cfg / "corrupt_tracking.json").write_text(json.dumps({flagged: "2020-01-01T00:00:00"})) + return cfg, flagged, good + + +def _webui_delete(tmp_path, monkeypatch, cfg, flagged): + monkeypatch.setenv("LIDARR_URL", URL) + monkeypatch.delenv("LIDARR_BLOCKLIST", raising=False) + monkeypatch.setattr(main, "_load_lidarr_api_key", lambda: KEY) + main.delete_corrupt_files([flagged], str(cfg), music_dir=str(tmp_path / "data")) + + +def _webui_folder_delete(tmp_path, monkeypatch, cfg, flagged): + monkeypatch.setenv("LIDARR_URL", URL) + monkeypatch.delenv("LIDARR_BLOCKLIST", raising=False) + monkeypatch.setattr(main, "_load_lidarr_api_key", lambda: KEY) + main.delete_album_folders([os.path.dirname(flagged)], str(cfg), music_dir=str(tmp_path / "data"), mode="whole") + + +def _auto_delete(tmp_path, monkeypatch, cfg, flagged): + monkeypatch.setattr(main, "_reverify_before_delete", lambda to_delete, *a: to_delete) + main.run_auto_delete(str(cfg), str(cfg / "log.txt"), 1, lidarr_url=URL, lidarr_api_key=KEY) + + +@pytest.mark.parametrize("delete", [_webui_delete, _webui_folder_delete, _auto_delete]) +def test_a_wrong_match_saved_by_an_older_scan_deletes_the_flagged_file_not_lidarrs(tmp_path, monkeypatch, delete): + cfg, flagged, good = _stale_library(tmp_path) + _, deleted = _lidarr(monkeypatch, OTHER_MOUNT, tracks=("Album/01.flac", "Album/02.flac")) + delete(tmp_path, monkeypatch, cfg, flagged) + assert deleted == [] + assert not os.path.exists(flagged) + assert all(os.path.exists(g) for g in good) + + +@pytest.mark.parametrize("saved, root", [ + ({"artistId": 1}, OTHER_MOUNT), + ({"artistId": 2, "lidarrPath": f"{OTHER_MOUNT}/Artist/Album/01.flac"}, OTHER_MOUNT), + ({"artistId": 1, "lidarrPath": f"{OTHER_MOUNT}/Artist/Album/01.flac"}, f"{OTHER_MOUNT}/moved"), +]) +def test_a_saved_match_that_cannot_be_checked_is_looked_up_again(tmp_path, monkeypatch, saved, root): + fp = _file(tmp_path, "Artist/Album/01.flac") + _lidarr(monkeypatch, root) + details = {fp: dict(saved, reason="damaged", trackfileId=99, albumId=7)} + ready = main._lidarr_ids_before_delete(details, [fp], URL, KEY, str(tmp_path)) + assert ready is True + assert details[fp]["trackfileId"] == 5 + + +def test_a_saved_match_is_not_trusted_when_lidarr_cannot_be_read(tmp_path, monkeypatch): + fp = _file(tmp_path, "Artist/Album/01.flac") + _lidarr(monkeypatch, OTHER_MOUNT, up=False) + details = {fp: {"reason": "damaged", "trackfileId": 5, "albumId": 7, "artistId": 1, + "lidarrPath": f"{OTHER_MOUNT}/Artist/Album/01.flac"}} + ready = main._lidarr_ids_before_delete(details, [fp], URL, KEY, str(tmp_path)) + assert ready is False + + +def test_a_folder_delete_re_checks_a_saved_match_of_a_file_not_in_the_list(tmp_path, monkeypatch): + cfg, flagged, good = _stale_library(tmp_path) + unlisted = os.path.join(os.path.dirname(flagged), "02.flac") + (cfg / "corrupt_details.json").write_text(json.dumps({flagged: {"reason": "damaged"}, unlisted: { + "reason": "damaged", "trackfileId": 6, "albumId": 7, "artistId": 1, + "lidarrPath": f"{OTHER_MOUNT}/Artist/Album/02.flac"}})) + _, deleted = _lidarr(monkeypatch, OTHER_MOUNT, tracks=("Album/01.flac", "Album/02.flac")) + _webui_folder_delete(tmp_path, monkeypatch, cfg, flagged) + assert deleted == [] + assert all(os.path.exists(g) for g in good) + + +def test_a_folder_lookup_stops_when_saved_matches_cannot_be_checked(monkeypatch): + monkeypatch.setattr(main, "_drop_wrong_lidarr_matches", lambda *a: None) + file_to_tfid = main._build_file_to_tfid(["/data/a.flac"], set(), {"/data/a.flac": {"trackfileId": 5}}, URL, KEY) + assert file_to_tfid is None diff --git a/tests/test_lidarr_ids_before_delete.py b/tests/test_lidarr_ids_before_delete.py index f5f3d8a..a4d1676 100644 --- a/tests/test_lidarr_ids_before_delete.py +++ b/tests/test_lidarr_ids_before_delete.py @@ -211,9 +211,9 @@ def test_a_file_with_no_saved_details_gets_its_lidarr_id(tmp_path, monkeypatch): def test_only_the_files_being_deleted_are_looked_up(tmp_path, monkeypatch): - calls = _lidarr(monkeypatch, up=False) - details = {"/data/a.flac": {"reason": "damaged", "trackfileId": 5}, - "/data/b.flac": {"reason": "damaged"}} - ready = main._lidarr_ids_before_delete(details, ["/data/a.flac"], URL, KEY, str(tmp_path)) + calls = _lidarr(monkeypatch) + saved = {"reason": "damaged", "trackfileId": 5, "albumId": 7, "artistId": 1, "lidarrPath": LIDARR_TRACK} + details = {"/data/Artist/Album/01.flac": saved, "/data/b.flac": {"reason": "damaged"}} + ready = main._lidarr_ids_before_delete(details, ["/data/Artist/Album/01.flac"], URL, KEY, str(tmp_path)) assert ready is True - assert calls == [] + assert calls == ["GET artist"]