#704: add "Relocate" fix for AcoustID mismatches — retag + restage for re-import
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.
This commit is contained in:
parent
525222b04c
commit
9bf7881f7a
4 changed files with 242 additions and 1 deletions
69
core/repair_jobs/relocate.py
Normal file
69
core/repair_jobs/relocate.py
Normal file
|
|
@ -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"]
|
||||
|
|
@ -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', '')
|
||||
|
|
|
|||
117
tests/repair_jobs/test_relocate.py
Normal file
117
tests/repair_jobs/test_relocate.py
Normal file
|
|
@ -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
|
||||
|
|
@ -3583,6 +3583,9 @@ function _promptAcoustidAction() {
|
|||
<button id="_acid-retag" style="padding:10px 20px;border-radius:10px;border:1px solid rgba(102,126,234,0.4);background:rgba(102,126,234,0.15);color:#667eea;font-weight:600;cursor:pointer;font-family:inherit;">
|
||||
Retag
|
||||
</button>
|
||||
<button id="_acid-relocate" style="padding:10px 20px;border-radius:10px;border:1px solid rgba(245,158,11,0.4);background:rgba(245,158,11,0.12);color:#f59e0b;font-weight:600;cursor:pointer;font-family:inherit;">
|
||||
Relocate
|
||||
</button>
|
||||
<button id="_acid-redownload" style="padding:10px 20px;border-radius:10px;border:1px solid rgba(29,185,84,0.4);background:rgba(29,185,84,0.15);color:#1db954;font-weight:600;cursor:pointer;font-family:inherit;">
|
||||
Re-download
|
||||
</button>
|
||||
|
|
@ -3591,7 +3594,7 @@ function _promptAcoustidAction() {
|
|||
</button>
|
||||
</div>
|
||||
<div style="margin-top:12px;font-size:0.78em;color:rgba(255,255,255,0.35);line-height:1.4;">
|
||||
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
|
||||
</div>
|
||||
<button id="_acid-cancel" style="margin-top:12px;padding:6px 16px;border:none;background:none;color:rgba(255,255,255,0.4);cursor:pointer;font-size:0.82em;font-family:inherit;">
|
||||
Cancel
|
||||
|
|
@ -3601,6 +3604,7 @@ function _promptAcoustidAction() {
|
|||
document.body.appendChild(overlay);
|
||||
|
||||
overlay.querySelector('#_acid-retag').onclick = () => { overlay.remove(); resolve('retag'); };
|
||||
overlay.querySelector('#_acid-relocate').onclick = () => { overlay.remove(); resolve('relocate'); };
|
||||
overlay.querySelector('#_acid-redownload').onclick = () => { overlay.remove(); resolve('redownload'); };
|
||||
overlay.querySelector('#_acid-delete').onclick = () => { overlay.remove(); resolve('delete'); };
|
||||
overlay.querySelector('#_acid-cancel').onclick = () => { overlay.remove(); resolve(null); };
|
||||
|
|
|
|||
Loading…
Reference in a new issue