diff --git a/core/sync/match_overrides.py b/core/sync/match_overrides.py index f83c007d..3bff435d 100644 --- a/core/sync/match_overrides.py +++ b/core/sync/match_overrides.py @@ -24,6 +24,10 @@ from __future__ import annotations from typing import Any, Callable, Dict, List, Optional +from utils.logging_config import get_logger + +logger = get_logger("sync.match_overrides") + def resolve_match_overrides( source_tracks: List[Dict[str, Any]], @@ -117,3 +121,76 @@ def record_manual_match( )) except Exception: return False + + +def resolve_durable_match_server_id( + db: Any, + profile_id: int, + source_track_id: str, + server_source: str, + valid_server_ids: set, +) -> Optional[str]: + """Current server track id for a DURABLE manual library match, or None. + + Unlike ``sync_match_cache`` (wiped on every rescan), the + ``manual_library_track_matches`` table survives a scan — so consulting it + here is what makes a user's Find & Add / manual match persist across a + library rescan (#787). If the stored ``library_track_id`` went stale + (a rescan re-keyed the track — common on Jellyfin/Navidrome), re-resolve + it from the stored file path and self-heal the row so the next lookup is + a direct hit. + + Pure helper: ``db`` is injected. ``valid_server_ids`` is the set of + string ids that currently exist in the server playlist's track list — + a re-resolved id is only returned if it's actually present. Never raises. + """ + if not source_track_id: + return None + finder = getattr(db, "find_manual_library_match_by_source_track_id", None) + if finder is None: + return None + try: + match = finder(profile_id, str(source_track_id), server_source or "") + except Exception: + return None + if not match: + return None + + lib_id = match.get("library_track_id") + if lib_id is not None and str(lib_id) in valid_server_ids: + return str(lib_id) + + # Stale pointer — re-resolve via the stored file path and self-heal. + file_path = match.get("library_file_path") + resolver = getattr(db, "find_track_id_by_file_path", None) + if file_path and resolver is not None: + try: + new_id = resolver(file_path) + except Exception: + new_id = None + if new_id and str(new_id) in valid_server_ids: + _self_heal_match_id(db, match, str(new_id)) + return str(new_id) + return None + + +def _self_heal_match_id(db: Any, match: Dict[str, Any], new_library_track_id: str) -> None: + """Rewrite a manual match's library_track_id after re-resolution. Best-effort.""" + saver = getattr(db, "save_manual_library_match", None) + if saver is None: + return + try: + saver( + match.get("profile_id", 1), + match.get("source", ""), + match.get("source_track_id", ""), + new_library_track_id, + source_title=match.get("source_title"), + source_artist=match.get("source_artist"), + source_album=match.get("source_album"), + source_context_json=match.get("source_context_json"), + server_source=match.get("server_source", ""), + library_file_path=match.get("library_file_path"), + ) + except Exception as e: + logger.debug("manual match self-heal failed: %s", e) diff --git a/database/music_database.py b/database/music_database.py index e978df0c..3e52c2dc 100644 --- a/database/music_database.py +++ b/database/music_database.py @@ -4621,21 +4621,29 @@ class MusicDatabase: CREATE INDEX IF NOT EXISTS idx_mltm_lib_track ON manual_library_track_matches (library_track_id) """) + # Stable re-resolution key: a library rescan can drop/re-key + # tracks (esp. Jellyfin/Navidrome GUIDs), leaving library_track_id + # dangling. Storing the file path lets us re-find the current + # track id after a scan so manual matches survive it. + cursor.execute("PRAGMA table_info(manual_library_track_matches)") + _mltm_cols = {r[1] for r in cursor.fetchall()} + if 'library_file_path' not in _mltm_cols: + cursor.execute("ALTER TABLE manual_library_track_matches ADD COLUMN library_file_path TEXT") except Exception as e: logger.error(f"Error creating manual_library_track_matches table: {e}") def save_manual_library_match(self, profile_id: int, source: str, source_track_id: str, library_track_id: str, **meta) -> bool: """Insert or replace a manual match. meta keys: source_title, source_artist, - source_album, source_context_json, server_source.""" + source_album, source_context_json, server_source, library_file_path.""" try: with self._get_connection() as conn: conn.execute(""" INSERT INTO manual_library_track_matches (profile_id, source, source_track_id, library_track_id, source_title, source_artist, source_album, - source_context_json, server_source, updated_at) - VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) + source_context_json, server_source, library_file_path, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, CURRENT_TIMESTAMP) ON CONFLICT(profile_id, source, source_track_id, server_source) DO UPDATE SET library_track_id = excluded.library_track_id, @@ -4643,18 +4651,46 @@ class MusicDatabase: source_artist = excluded.source_artist, source_album = excluded.source_album, source_context_json = excluded.source_context_json, + library_file_path = excluded.library_file_path, updated_at = CURRENT_TIMESTAMP """, ( profile_id, source, source_track_id, library_track_id, meta.get('source_title'), meta.get('source_artist'), meta.get('source_album'), meta.get('source_context_json'), - meta.get('server_source', ''), + meta.get('server_source', ''), meta.get('library_file_path'), )) return True except Exception as e: logger.error(f"save_manual_library_match error: {e}") return False + def find_track_id_by_file_path(self, file_path: str) -> Optional[str]: + """Return the current tracks.id for a file path, or None. + + Used to re-resolve a manual match whose stored library_track_id went + stale after a rescan re-keyed the track. Exact path first, then a + basename fallback (handles server-vs-local path differences).""" + if not file_path: + return None + try: + conn = self._get_connection() + cursor = conn.cursor() + cursor.execute("SELECT id FROM tracks WHERE file_path = ? LIMIT 1", (file_path,)) + row = cursor.fetchone() + if row: + return str(row[0]) + import os as _os + fname = _os.path.basename(str(file_path).replace('\\', '/')) + if fname: + cursor.execute("SELECT id FROM tracks WHERE file_path LIKE ? LIMIT 1", (f"%{fname}",)) + row = cursor.fetchone() + if row: + return str(row[0]) + return None + except Exception as e: + logger.error(f"find_track_id_by_file_path error: {e}") + return None + def get_manual_library_match(self, profile_id: int, source: str, source_track_id: str, server_source: str = '') -> Optional[Dict[str, Any]]: """Return match row dict or None.""" diff --git a/tests/sync/test_match_overrides.py b/tests/sync/test_match_overrides.py index 4779ca54..63a5e40a 100644 --- a/tests/sync/test_match_overrides.py +++ b/tests/sync/test_match_overrides.py @@ -1,6 +1,10 @@ from unittest.mock import MagicMock -from core.sync.match_overrides import record_manual_match, resolve_match_overrides +from core.sync.match_overrides import ( + record_manual_match, + resolve_durable_match_server_id, + resolve_match_overrides, +) # ────────────────────────────────────────────────────────────────────── @@ -191,3 +195,84 @@ def test_record_handles_empty_optional_strings(): assert kwargs["normalized_title"] == "" assert kwargs["normalized_artist"] == "" assert kwargs["server_track_title"] == "" + + +# ────────────────────────────────────────────────────────────────────── +# resolve_durable_match_server_id — manual match survives a rescan (#787) +# ────────────────────────────────────────────────────────────────────── + +class _FakeMatchDB: + """Minimal DB stub for the durable-match resolver.""" + def __init__(self, match=None, file_path_id=None): + self._match = match + self._file_path_id = file_path_id + self.saved = [] + + def find_manual_library_match_by_source_track_id(self, profile_id, source_track_id, server_source): + return dict(self._match) if self._match else None + + def find_track_id_by_file_path(self, file_path): + return self._file_path_id + + def save_manual_library_match(self, profile_id, source, source_track_id, library_track_id, **meta): + self.saved.append((library_track_id, meta)) + return True + + +def test_durable_match_returns_id_when_still_valid(): + # Stored library id still present in the server playlist → direct hit. + db = _FakeMatchDB(match={"library_track_id": "5001", "library_file_path": "/m/a.flac", + "profile_id": 1, "source": "spotify", "source_track_id": "iron", + "server_source": "plex"}) + out = resolve_durable_match_server_id(db, 1, "iron", "plex", {"5001", "5002"}) + assert out == "5001" + assert db.saved == [] # no self-heal needed + + +def test_durable_match_reresolves_stale_id_via_file_path_and_self_heals(): + # Rescan re-keyed the track: old id 5001 gone, file now lives at id 7777. + db = _FakeMatchDB( + match={"library_track_id": "5001", "library_file_path": "/m/a.flac", + "profile_id": 1, "source": "spotify", "source_track_id": "iron", "server_source": "plex"}, + file_path_id="7777", + ) + out = resolve_durable_match_server_id(db, 1, "iron", "plex", {"7777", "9000"}) + assert out == "7777" # re-resolved to the current id + assert db.saved and db.saved[0][0] == "7777" # self-healed the stored id + + +def test_durable_match_none_when_no_match(): + db = _FakeMatchDB(match=None) + assert resolve_durable_match_server_id(db, 1, "iron", "plex", {"5001"}) is None + + +def test_durable_match_none_when_stale_and_file_path_unresolvable(): + # Stale id AND the file path no longer resolves (file moved/removed) → no pair. + db = _FakeMatchDB( + match={"library_track_id": "5001", "library_file_path": "/m/gone.flac", + "profile_id": 1, "source": "spotify", "source_track_id": "iron", "server_source": "plex"}, + file_path_id=None, + ) + assert resolve_durable_match_server_id(db, 1, "iron", "plex", {"7777"}) is None + assert db.saved == [] + + +def test_durable_match_none_when_reresolved_id_not_in_playlist(): + # File resolves to a track, but that track isn't in THIS playlist → don't pair. + db = _FakeMatchDB( + match={"library_track_id": "5001", "library_file_path": "/m/a.flac", + "profile_id": 1, "source": "spotify", "source_track_id": "iron", "server_source": "plex"}, + file_path_id="7777", + ) + assert resolve_durable_match_server_id(db, 1, "iron", "plex", {"1234"}) is None + + +def test_durable_match_safe_when_db_lacks_methods(): + class Bare: + pass + assert resolve_durable_match_server_id(Bare(), 1, "iron", "plex", {"5001"}) is None + + +def test_durable_match_empty_source_id_returns_none(): + db = _FakeMatchDB(match={"library_track_id": "5001"}) + assert resolve_durable_match_server_id(db, 1, "", "plex", {"5001"}) is None diff --git a/tests/test_manual_library_match.py b/tests/test_manual_library_match.py index 6329a01c..f964454e 100644 --- a/tests/test_manual_library_match.py +++ b/tests/test_manual_library_match.py @@ -90,6 +90,34 @@ def test_save_upserts_existing(db): assert row["source_title"] == "Updated" +def test_save_persists_library_file_path(db): + """#787 durability: the file path is stored so a manual match can be + re-resolved after a rescan re-keys the track.""" + ok = db.save_manual_library_match(1, "spotify", "track-abc", 42, + library_file_path="/music/Artist/Album/01.flac") + assert ok is True + row = db.get_manual_library_match(1, "spotify", "track-abc") + assert row["library_file_path"] == "/music/Artist/Album/01.flac" + + +def test_find_track_id_by_file_path(db): + """Re-resolution primitive: locate the current tracks.id for a file path + (exact, then basename fallback).""" + with db._get_connection() as conn: + conn.execute("PRAGMA foreign_keys=OFF") + conn.execute( + "INSERT INTO tracks (id, album_id, artist_id, title, file_path) VALUES (?, ?, ?, ?, ?)", + ("7777", 1, 1, "Track", "/music/Artist/Album/01.flac"), + ) + # Exact path + assert db.find_track_id_by_file_path("/music/Artist/Album/01.flac") == "7777" + # Basename fallback (server vs local path shape) + assert db.find_track_id_by_file_path("/different/root/01.flac") == "7777" + # Unknown file → None + assert db.find_track_id_by_file_path("/music/nope.flac") is None + assert db.find_track_id_by_file_path("") is None + + def test_delete_by_id(db): db.save_manual_library_match(1, "spotify", "track-abc", 42) row = db.get_manual_library_match(1, "spotify", "track-abc") diff --git a/web_server.py b/web_server.py index 04390e3a..e3214b1b 100644 --- a/web_server.py +++ b/web_server.py @@ -18072,13 +18072,26 @@ def get_server_playlist_tracks(playlist_id): # exact/fuzzy passes entirely. Stale-cache safe — if the cached # server track no longer exists, the override is silently # skipped and normal matching runs. - from core.sync.match_overrides import resolve_match_overrides + from core.sync.match_overrides import resolve_durable_match_server_id, resolve_match_overrides _db_for_overrides = get_database() - _override_pairs = resolve_match_overrides( - source_tracks, - server_tracks, - lambda src_id: ((_db_for_overrides.read_sync_match_cache(src_id, active_server) or {}).get('server_track_id')), - ) + # Set of server track ids currently in this playlist — used to validate + # a re-resolved durable match actually exists before pairing it. + _server_ids = {str(t.get('id')) for t in server_tracks if isinstance(t, dict) and t.get('id') is not None} + _ov_profile = get_current_profile_id() + + def _override_lookup(src_id): + # 1) Fast override cache (cleared on every rescan). + cached = _db_for_overrides.read_sync_match_cache(src_id, active_server) or {} + if cached.get('server_track_id'): + return cached['server_track_id'] + # 2) Durable manual library match — survives a rescan (#787), so a + # Find & Add / manual match keeps pairing after a DB scan. Re- + # resolves a stale id via the stored file path when needed. + return resolve_durable_match_server_id( + _db_for_overrides, _ov_profile, src_id, active_server, _server_ids + ) + + _override_pairs = resolve_match_overrides(source_tracks, server_tracks, _override_lookup) combined = reconcile_playlist(source_tracks, server_tracks, _override_pairs) @@ -18194,27 +18207,55 @@ def server_playlist_replace_track(playlist_id): return jsonify({"success": False, "error": str(e)}), 500 -def _persist_find_and_add_match(source_track_id, server_source, server_track_id, server_track_title, source_title, source_artist): - """Wrap match-override persistence with the active DB. No-op when - source_track_id is missing (e.g. add to a non-mirrored playlist).""" +def _persist_find_and_add_match(source_track_id, server_source, server_track_id, server_track_title, source_title, source_artist, source='spotify'): + """Persist a Find & Add selection two ways. No-op when source_track_id is + missing (e.g. add to a non-mirrored playlist). + + 1. ``sync_match_cache`` override — fast auto-match on the next sync, but + wiped on every library rescan. + 2. A DURABLE manual library match (#787) — survives a rescan, so the + source→library pairing sticks in the sync display AND the download loop + stops re-fetching it. This is the unification the issue asked for: + Find & Add now also records a manual library match (one-way; the manual + match tool has no playlist to act on, so it doesn't reverse-create).""" if not source_track_id: return + db = get_database() try: from core.sync.match_overrides import record_manual_match ok = record_manual_match( - get_database(), + db, source_track_id=source_track_id, server_source=server_source, server_track_id=server_track_id, - server_track_title=server_track_title, source_title=source_title, source_artist=source_artist, + server_track_title=server_track_title, ) if ok: logger.info(f"[ServerPlaylist] Persisted Find & Add override: {source_track_id} → {server_track_id} ({server_source})") except Exception as e: logger.warning(f"[ServerPlaylist] Failed to persist Find & Add override: {e}") + # Durable manual library match — survives a rescan (the override above does not). + try: + from core.library import manual_library_match as _mlm + file_path = '' + try: + _rows = db.api_get_tracks_by_ids([server_track_id]) + if _rows: + file_path = _rows[0].get('file_path', '') or '' + except Exception as _fp_err: + logger.debug(f"[ServerPlaylist] file_path lookup for manual match failed: {_fp_err}") + _mlm.save_match( + db, get_current_profile_id(), (source or 'spotify'), str(source_track_id), str(server_track_id), + source_title=source_title, source_artist=source_artist, + server_source=server_source, library_file_path=file_path, + ) + logger.info(f"[ServerPlaylist] Recorded durable manual library match: {source_track_id} → {server_track_id}") + except Exception as e: + logger.warning(f"[ServerPlaylist] Failed to record durable manual match: {e}") + @app.route('/api/server/playlist//add-track', methods=['POST']) def server_playlist_add_track(playlist_id): @@ -18236,6 +18277,9 @@ def server_playlist_add_track(playlist_id): source_title = data.get('source_title') or '' source_artist = data.get('source_artist') or '' server_track_title = data.get('server_track_title') or '' + # Provider of the source track (spotify/deezer/...) for the durable + # manual match. Retrieval is source-agnostic, so this is metadata. + source_provider = data.get('source') or 'spotify' if not track_id: return jsonify({"success": False, "error": "track_id required"}), 400 @@ -18276,7 +18320,7 @@ def server_playlist_add_track(playlist_id): except Exception: _existing = set() if str(track_id) in _existing: - _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title or new_item.title, source_title, source_artist) + _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title or new_item.title, source_title, source_artist, source_provider) return jsonify({"success": True, "message": "Track linked"}) logger.info(f"[ServerPlaylist] Adding track: '{new_item.title}' (ratingKey={new_item.ratingKey}) to playlist '{playlist_name}'") @@ -18300,7 +18344,7 @@ def server_playlist_add_track(playlist_id): new_id = str(raw_playlist.ratingKey) logger.info(f"[ServerPlaylist] Added track to playlist, playlist ID: {new_id}") - _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title or new_item.title, source_title, source_artist) + _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title or new_item.title, source_title, source_artist, source_provider) return jsonify({"success": True, "message": "Track added", "new_playlist_id": new_id}) elif active_server == 'jellyfin' and media_server_engine.client('jellyfin'): @@ -18313,7 +18357,7 @@ def server_playlist_add_track(playlist_id): if plan['should_insert']: new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in plan['new_ids']] media_server_engine.client('jellyfin').update_playlist(playlist_name, new_track_objs) - _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist) + _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist, source_provider) return jsonify({"success": True, "message": "Track linked" if not plan['should_insert'] else "Track added"}) elif active_server == 'navidrome' and media_server_engine.client('navidrome'): @@ -18326,7 +18370,7 @@ def server_playlist_add_track(playlist_id): if plan['should_insert']: new_track_objs = [type('T', (), {'ratingKey': tid, 'title': ''})() for tid in plan['new_ids']] media_server_engine.client('navidrome').create_playlist(playlist_name, new_track_objs, playlist_id=playlist_id) - _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist) + _persist_find_and_add_match(source_track_id, active_server, track_id, server_track_title, source_title, source_artist, source_provider) return jsonify({"success": True, "message": "Track linked" if not plan['should_insert'] else "Track added"}) return jsonify({"success": False, "error": f"Unsupported server: {active_server}"}), 400 diff --git a/webui/static/pages-extra.js b/webui/static/pages-extra.js index 67b016ae..9b42763e 100644 --- a/webui/static/pages-extra.js +++ b/webui/static/pages-extra.js @@ -1836,6 +1836,9 @@ async function _serverSelectTrack(trackIndex, mode, newTrackId, el) { source_track_id: srcTrack.source_track_id || '', source_title: srcTrack.name || '', source_artist: srcTrack.artist || '', + // Provider of the source track, so the durable manual match + // (#787) records the right source. Retrieval is source-agnostic. + source: srcTrack.source || _serverEditorState.mirroredPlaylist?.source || '', }) }); }