From 9bf7881f7abac6fc35e132cbee276b5c2c2d6d6d Mon Sep 17 00:00:00 2001 From: BoulderBadgeDad Date: Thu, 11 Jun 2026 12:01:05 -0700 Subject: [PATCH] =?UTF-8?q?#704:=20add=20"Relocate"=20fix=20for=20AcoustID?= =?UTF-8?q?=20mismatches=20=E2=80=94=20retag=20+=20restage=20for=20re-impo?= =?UTF-8?q?rt?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'retag' fix corrects a mismatched file's tags/DB but leaves it in the WRONG artist/album folder, so the library shows the right title while the file sits under the previous track. AcoustID yields only title+artist (no reliable album), so an in-place move has no safe target. New 'relocate' action: retag the file, move it into Staging, drop the stale tracks row, and clean up the emptied folder. The auto-import worker (which watches Staging) re-identifies it with full metadata and files it correctly — reusing the import pipeline instead of guessing a destination. - core/repair_jobs/relocate.py: pure, injectable orchestration (retag -> move -> drop row) + collision-safe staging_destination. Row is dropped only AFTER a successful move, so a failed move never orphans the library entry. - _fix_acoustid_mismatch gains the 'relocate' branch (thin wrapper: resolve path, staging dir, drop-row closure, empty-parent cleanup). - UI: "Relocate" button on the AcoustID-mismatch fix modal. Tests (8): staging-dest collision suffixing; relocate happy path; tag-write failure still relocates; FAILED move does NOT drop the row; no-tags skips write; a real file move through safe_move_file; and a handler integration test (file moved to staging + tracks row deleted end-to-end). Repair + integrity suites green. --- core/repair_jobs/relocate.py | 69 +++++++++++++++++ core/repair_worker.py | 51 +++++++++++++ tests/repair_jobs/test_relocate.py | 117 +++++++++++++++++++++++++++++ webui/static/enrichment.js | 6 +- 4 files changed, 242 insertions(+), 1 deletion(-) create mode 100644 core/repair_jobs/relocate.py create mode 100644 tests/repair_jobs/test_relocate.py diff --git a/core/repair_jobs/relocate.py b/core/repair_jobs/relocate.py new file mode 100644 index 00000000..11766d75 --- /dev/null +++ b/core/repair_jobs/relocate.py @@ -0,0 +1,69 @@ +"""Relocate an AcoustID-mismatched file into Staging for clean re-import (#704). + +The existing 'retag' fix corrects a mismatched file's tags + DB record but leaves +the file in the WRONG artist/album folder on disk — so the library shows the right +title while the file sits under the previous track's artist/album. AcoustID only +yields a title + artist (not a reliable album), so an *in-place* move has no +trustworthy target. + +Instead: retag the file, move it into the staging folder, and drop the stale +``tracks`` row. The auto-import worker (which watches staging) then re-identifies +the file with full metadata and files it in the correct artist/album/track path — +reusing the battle-tested import pipeline rather than guessing a destination here. + +Side effects are injected so the orchestration is a pure, unit-testable seam. +""" + +from __future__ import annotations + +import os +from typing import Any, Callable, Dict, Optional + + +def staging_destination(staging_dir: str, filename: str, + exists: Callable[[str], bool]) -> str: + """A non-colliding path for ``filename`` inside ``staging_dir``. + + If the name is already taken, suffix it ``' (1)'``, ``' (2)'``, … before the + extension — never overwrite an unrelated file already waiting in staging. + """ + base, ext = os.path.splitext(filename) + dest = os.path.join(staging_dir, filename) + n = 1 + while exists(dest): + dest = os.path.join(staging_dir, f"{base} ({n}){ext}") + n += 1 + return dest + + +def relocate_mismatch_to_staging( + resolved_path: str, + staging_dir: str, + tag_updates: Optional[Dict[str, Any]], + *, + write_tags: Callable[[str, Dict[str, Any]], Any], + move_file: Callable[[str, str], Any], + drop_db_row: Callable[[], Any], + exists: Callable[[str], bool], +) -> str: + """Retag (best-effort) → move into staging → drop the stale DB row. + + Returns the staging destination path. Order matters: the DB row is dropped + only AFTER a successful move, so a failed move (which raises) leaves the + library entry intact rather than orphaning it. + """ + if tag_updates: + try: + write_tags(resolved_path, tag_updates) + except Exception: + # Tags are best-effort — re-import re-derives them from the source. + # The relocation itself is the point, so don't abort over a tag write. + pass + + dest = staging_destination(staging_dir, os.path.basename(resolved_path), exists) + move_file(resolved_path, dest) # may raise → row NOT dropped (intentional) + drop_db_row() + return dest + + +__all__ = ["staging_destination", "relocate_mismatch_to_staging"] diff --git a/core/repair_worker.py b/core/repair_worker.py index 60c81ff8..27dffe3b 100644 --- a/core/repair_worker.py +++ b/core/repair_worker.py @@ -1983,6 +1983,57 @@ class RepairWorker: return {'success': True, 'action': 'redownload', 'message': f'Added "{expected_title}" to wishlist, removed wrong file'} + if fix_action == 'relocate': + # #704: retag fixes the file's tags but leaves it in the WRONG + # artist/album folder. AcoustID gives only title+artist (no reliable + # album), so move the retagged file into staging and let auto-import + # re-file it correctly with full metadata. Drop the stale tracks row. + resolved = _resolve_file_path(file_path, self.transfer_folder, + config_manager=self._config_manager) + if not resolved or not os.path.exists(resolved): + return {'success': False, 'error': f'File not found: {file_path}'} + staging_path = './Staging' + if self._config_manager: + staging_path = self._config_manager.get('import.staging_path', './Staging') + staging_path = self._resolve_path(staging_path) + try: + os.makedirs(staging_path, exist_ok=True) + except OSError as e: + return {'success': False, 'error': f'Staging folder unavailable: {e}'} + + aid_title = details.get('acoustid_title', '') + aid_artist = details.get('acoustid_artist', '') + tag_updates = {} + if aid_title: + tag_updates['title'] = aid_title + if aid_artist: + tag_updates['artist_name'] = aid_artist + tag_updates['artists_list'] = _split_acoustid_credit(aid_artist) + + def _drop_row(): + if not track_id: + return + conn = self.db._get_connection() + try: + conn.cursor().execute("DELETE FROM tracks WHERE id = ?", (track_id,)) + conn.commit() + finally: + conn.close() + + from core.repair_jobs.relocate import relocate_mismatch_to_staging + from core.tag_writer import write_tags_to_file + from core.imports.file_ops import safe_move_file + try: + dest = relocate_mismatch_to_staging( + resolved, staging_path, tag_updates, + write_tags=write_tags_to_file, move_file=safe_move_file, + drop_db_row=_drop_row, exists=os.path.exists) + except Exception as e: + return {'success': False, 'error': f'Relocate failed: {e}'} + self._cleanup_empty_parents(resolved) # remove the now-empty wrong folder + return {'success': True, 'action': 'relocated', + 'message': f'Moved to staging for re-import: {os.path.basename(dest)}'} + # Default: retag — update DB record to match the actual audio content aid_title = details.get('acoustid_title', '') aid_artist = details.get('acoustid_artist', '') diff --git a/tests/repair_jobs/test_relocate.py b/tests/repair_jobs/test_relocate.py new file mode 100644 index 00000000..cb8be997 --- /dev/null +++ b/tests/repair_jobs/test_relocate.py @@ -0,0 +1,117 @@ +"""#704: relocate an AcoustID-mismatched file to staging for re-import — pure +orchestration (retag -> move -> drop row) with the side effects injected, plus a +real-file move through safe_move_file.""" + +from __future__ import annotations + +import os + +import pytest + +from core.repair_jobs.relocate import staging_destination, relocate_mismatch_to_staging + + +# ── staging_destination: never overwrite an unrelated staged file ────────── +def test_staging_destination_no_collision(): + assert staging_destination('/stg', 'Song.mp3', exists=lambda p: False) == os.path.join('/stg', 'Song.mp3') + + +def test_staging_destination_suffixes_on_collision(): + taken = {os.path.join('/stg', 'Song.mp3'), os.path.join('/stg', 'Song (1).mp3')} + assert staging_destination('/stg', 'Song.mp3', exists=lambda p: p in taken) == os.path.join('/stg', 'Song (2).mp3') + + +# ── relocate orchestration ───────────────────────────────────────────────── +class _Spy: + def __init__(self): self.tagged = None; self.moved = None; self.dropped = False + def write_tags(self, path, updates): self.tagged = (path, updates) + def move(self, src, dst): self.moved = (src, dst) + def drop(self): self.dropped = True + + +def test_relocate_happy_path(): + s = _Spy() + dest = relocate_mismatch_to_staging( + '/lib/Artist X/Album Y/03 - x.mp3', '/stg', {'title': 'Real Song'}, + write_tags=s.write_tags, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.tagged == ('/lib/Artist X/Album Y/03 - x.mp3', {'title': 'Real Song'}) + assert s.moved == ('/lib/Artist X/Album Y/03 - x.mp3', os.path.join('/stg', '03 - x.mp3')) + assert s.dropped is True + assert dest == os.path.join('/stg', '03 - x.mp3') + + +def test_relocate_tag_failure_still_relocates(): + s = _Spy() + def boom(*a): raise RuntimeError('tag write failed') + dest = relocate_mismatch_to_staging( + '/lib/x.mp3', '/stg', {'title': 'T'}, + write_tags=boom, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.moved and s.dropped and dest == os.path.join('/stg', 'x.mp3') + + +def test_relocate_failed_move_does_not_drop_row(): + # The library row must survive a failed move (no orphaning). + s = _Spy() + def bad_move(src, dst): raise OSError('cross-device move failed') + with pytest.raises(OSError): + relocate_mismatch_to_staging('/lib/x.mp3', '/stg', None, + write_tags=s.write_tags, move_file=bad_move, drop_db_row=s.drop, exists=lambda p: False) + assert s.dropped is False + + +def test_relocate_no_tag_updates_skips_write(): + s = _Spy() + relocate_mismatch_to_staging('/lib/x.mp3', '/stg', None, + write_tags=s.write_tags, move_file=s.move, drop_db_row=s.drop, exists=lambda p: False) + assert s.tagged is None and s.moved is not None + + +# ── real-file move through the actual safe_move_file ─────────────────────── +def test_real_file_moves_into_staging(tmp_path): + from core.imports.file_ops import safe_move_file + lib = tmp_path / 'lib' / 'Artist X' / 'Album Y'; lib.mkdir(parents=True) + src = lib / '03 - wrong.mp3'; src.write_bytes(b'\x00' * 64) + stg = tmp_path / 'Staging'; stg.mkdir() + dropped = [] + dest = relocate_mismatch_to_staging( + str(src), str(stg), None, + write_tags=lambda *a: None, move_file=safe_move_file, + drop_db_row=lambda: dropped.append(True), exists=os.path.exists) + assert not src.exists() # moved out of the wrong folder + assert os.path.exists(dest) # present in staging + assert os.path.dirname(dest) == str(stg) + assert dropped == [True] + + +# ── handler integration: _fix_acoustid_mismatch relocate end-to-end ───────── +def test_relocate_handler_moves_file_and_drops_row(tmp_path): + from database.music_database import MusicDatabase + from core.repair_worker import RepairWorker + + db = MusicDatabase(str(tmp_path / 'm.db')) + lib = tmp_path / 'music' / 'Wrong Artist' / 'Wrong Album' + lib.mkdir(parents=True) + wrong = lib / '03 - wrong.mp3' + wrong.write_bytes(b'\x00' * 64) + staging = tmp_path / 'Staging'; staging.mkdir() + + with db._get_connection() as conn: + conn.execute("INSERT OR REPLACE INTO artists (id, name, server_source) VALUES ('a1','Wrong Artist','plex')") + conn.execute("INSERT OR REPLACE INTO albums (id, title, artist_id) VALUES (10,'Wrong Album','a1')") + conn.execute("INSERT INTO tracks (id, album_id, artist_id, title, track_number, duration, file_path, server_source) " + "VALUES ('t1',10,'a1','Wrong Title',3,100,?, 'plex')", (str(wrong),)) + conn.commit() + + worker = RepairWorker(db) + worker._config_manager = type('C', (), { + 'get': staticmethod(lambda k, d=None: str(staging) if k == 'import.staging_path' else d)})() + + res = worker._fix_acoustid_mismatch( + 'track', 't1', str(wrong), + {'_fix_action': 'relocate', 'acoustid_title': 'Real Song', 'acoustid_artist': 'Real Artist'}) + + assert res['success'] is True and res['action'] == 'relocated' + assert not wrong.exists() # gone from the wrong album folder + assert (staging / '03 - wrong.mp3').exists() # now staged for re-import + with db._get_connection() as conn: + assert conn.execute("SELECT COUNT(*) FROM tracks WHERE id='t1'").fetchone()[0] == 0 # stale row dropped diff --git a/webui/static/enrichment.js b/webui/static/enrichment.js index a63d93e5..3ec070e0 100644 --- a/webui/static/enrichment.js +++ b/webui/static/enrichment.js @@ -3583,6 +3583,9 @@ function _promptAcoustidAction() { + @@ -3591,7 +3594,7 @@ function _promptAcoustidAction() {
- Retag = update metadata to match actual audio • Re-download = add correct track to wishlist & delete wrong file • Delete = remove file and DB entry + Retag = update metadata in place • Relocate = retag + move to Staging so it's re-imported into the correct artist/album • Re-download = add correct track to wishlist & delete wrong file • Delete = remove file and DB entry