Honor manual library matches across source labels
Manual matches can be created from sync history as mirrored while wishlist and download flows later see the same track as wishlist or a provider source. Add a shared track-level lookup that falls back from exact source/id to source_track_id and title/artist, then use it for wishlist adds, cleanup, and download analysis so mapped tracks are not re-added or redownloaded. Add coverage for mirrored-source matches being honored by wishlist cleanup and download batches, including the internal wishlist force-download path.
This commit is contained in:
Broque Thomas
parent
52dcdbe0f7
commit
aaf312cd34
7 changed files with 288 additions and 14 deletions
|
|
@ -382,8 +382,10 @@ def run_full_missing_tracks_process(batch_id, playlist_id, tracks_json, deps: Ma
|
|||
|
||||
# Manual library matches are authoritative unless the user explicitly
|
||||
# requested a force re-download from the normal download modal.
|
||||
_stid = track_data.get('spotify_track_id') or track_data.get('id', '')
|
||||
if not ignore_manual_matches and _stid and _mlm.get_match(db, batch_profile_id, batch_source, _stid):
|
||||
_stid = track_data.get('spotify_track_id') or track_data.get('source_track_id') or track_data.get('id', '')
|
||||
if not ignore_manual_matches and _stid and _mlm.get_match_for_track(
|
||||
db, batch_profile_id, track_data, default_source=batch_source
|
||||
):
|
||||
logger.info(f"[Manual Match] '{track_name}' already matched in library — skipping download")
|
||||
try:
|
||||
deps.check_and_remove_track_from_wishlist_by_metadata(track_data)
|
||||
|
|
|
|||
|
|
@ -42,6 +42,93 @@ def get_match(
|
|||
return getter(profile_id, source, source_track_id, server_source)
|
||||
|
||||
|
||||
def _first_artist_name(track: dict[str, Any]) -> str:
|
||||
artists = track.get("artists") or []
|
||||
if isinstance(artists, list) and artists:
|
||||
first = artists[0]
|
||||
if isinstance(first, dict):
|
||||
return (first.get("name") or "").strip()
|
||||
return str(first).strip()
|
||||
return (track.get("artist") or track.get("artist_name") or "").strip()
|
||||
|
||||
|
||||
def _track_source_candidates(track: dict[str, Any], default_source: str = "") -> list[str]:
|
||||
candidates = [
|
||||
track.get("provider"),
|
||||
track.get("source"),
|
||||
default_source,
|
||||
"spotify",
|
||||
]
|
||||
out = []
|
||||
for source in candidates:
|
||||
source = (source or "").strip()
|
||||
if source and source not in out:
|
||||
out.append(source)
|
||||
return out
|
||||
|
||||
|
||||
def _track_id_candidates(track: dict[str, Any]) -> list[str]:
|
||||
candidates = [
|
||||
track.get("source_track_id"),
|
||||
track.get("spotify_track_id"),
|
||||
track.get("track_id"),
|
||||
track.get("id"),
|
||||
track.get("musicbrainz_recording_id"),
|
||||
track.get("deezer_id") or track.get("deezer_track_id"),
|
||||
track.get("itunes_track_id"),
|
||||
track.get("tidal_id") or track.get("tidal_track_id"),
|
||||
track.get("qobuz_id") or track.get("qobuz_track_id"),
|
||||
track.get("amazon_id") or track.get("amazon_track_id"),
|
||||
]
|
||||
out = []
|
||||
for value in candidates:
|
||||
value = str(value).strip() if value is not None else ""
|
||||
if value and value not in out:
|
||||
out.append(value)
|
||||
return out
|
||||
|
||||
|
||||
def get_match_for_track(
|
||||
db,
|
||||
profile_id: int,
|
||||
track: dict[str, Any],
|
||||
*,
|
||||
default_source: str = "",
|
||||
server_source: str = "",
|
||||
) -> Optional[dict]:
|
||||
"""Return a manual match for a wishlist/sync track.
|
||||
|
||||
Exact source+ID matches are preferred, but source labels can legitimately
|
||||
change between UI surfaces (for example ``mirrored`` in sync history versus
|
||||
``wishlist`` in the wishlist batch). Fall back to track ID and finally
|
||||
title/artist so saved manual matches are honored consistently.
|
||||
"""
|
||||
if not isinstance(track, dict):
|
||||
return None
|
||||
|
||||
sources = _track_source_candidates(track, default_source)
|
||||
track_ids = _track_id_candidates(track)
|
||||
for track_id in track_ids:
|
||||
for source in sources:
|
||||
match = get_match(db, profile_id, source, track_id, server_source)
|
||||
if match:
|
||||
return match
|
||||
|
||||
id_getter = getattr(db, "find_manual_library_match_by_source_track_id", None)
|
||||
if id_getter is not None:
|
||||
for track_id in track_ids:
|
||||
match = id_getter(profile_id, track_id, server_source)
|
||||
if match:
|
||||
return match
|
||||
|
||||
title = (track.get("name") or track.get("title") or track.get("track_name") or "").strip()
|
||||
artist = _first_artist_name(track)
|
||||
metadata_getter = getattr(db, "find_manual_library_match_by_metadata", None)
|
||||
if metadata_getter is not None and title and artist:
|
||||
return metadata_getter(profile_id, title, artist, server_source)
|
||||
return None
|
||||
|
||||
|
||||
def delete_match(db, match_id: int, profile_id: int) -> bool:
|
||||
"""Delete match by PK id, scoped to profile."""
|
||||
return db.delete_manual_library_match(match_id, profile_id)
|
||||
|
|
|
|||
|
|
@ -256,8 +256,7 @@ def remove_tracks_already_in_library(
|
|||
continue
|
||||
|
||||
# Manual match check — skip fuzzy search if user already linked this track.
|
||||
_track_source = track.get('provider') or 'spotify'
|
||||
if _mlm.get_match(music_database, profile_id, _track_source, spotify_track_id):
|
||||
if _mlm.get_match_for_track(music_database, profile_id, track, default_source='wishlist'):
|
||||
try:
|
||||
removed = wishlist_service.mark_track_download_result(spotify_track_id, success=True)
|
||||
if removed:
|
||||
|
|
|
|||
|
|
@ -4219,6 +4219,64 @@ class MusicDatabase:
|
|||
logger.error(f"get_manual_library_match error: {e}")
|
||||
return None
|
||||
|
||||
def find_manual_library_match_by_source_track_id(self, profile_id: int,
|
||||
source_track_id: str,
|
||||
server_source: str = '') -> Optional[Dict[str, Any]]:
|
||||
"""Return a manual match for this source track ID across source labels.
|
||||
|
||||
The UI may save a match from sync history as ``mirrored`` while the
|
||||
wishlist/download flow later sees the same track under ``wishlist`` or
|
||||
the provider name. The source remains useful metadata, but the stored
|
||||
track ID is the stable identity we need to honor.
|
||||
"""
|
||||
if not source_track_id:
|
||||
return None
|
||||
try:
|
||||
with self._get_connection() as conn:
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("""
|
||||
SELECT * FROM manual_library_track_matches
|
||||
WHERE profile_id = ?
|
||||
AND source_track_id = ?
|
||||
AND (server_source = ? OR server_source = '')
|
||||
ORDER BY
|
||||
CASE WHEN server_source = ? THEN 0 ELSE 1 END,
|
||||
updated_at DESC
|
||||
LIMIT 1
|
||||
""", (profile_id, source_track_id, server_source or '', server_source or ''))
|
||||
row = cursor.fetchone()
|
||||
return dict(row) if row else None
|
||||
except Exception as e:
|
||||
logger.error(f"find_manual_library_match_by_source_track_id error: {e}")
|
||||
return None
|
||||
|
||||
def find_manual_library_match_by_metadata(self, profile_id: int,
|
||||
source_title: str,
|
||||
source_artist: str,
|
||||
server_source: str = '') -> Optional[Dict[str, Any]]:
|
||||
"""Return a manual match by title/artist when provider IDs differ."""
|
||||
if not source_title or not source_artist:
|
||||
return None
|
||||
try:
|
||||
with self._get_connection() as conn:
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("""
|
||||
SELECT * FROM manual_library_track_matches
|
||||
WHERE profile_id = ?
|
||||
AND source_title = ? COLLATE NOCASE
|
||||
AND source_artist = ? COLLATE NOCASE
|
||||
AND (server_source = ? OR server_source = '')
|
||||
ORDER BY
|
||||
CASE WHEN server_source = ? THEN 0 ELSE 1 END,
|
||||
updated_at DESC
|
||||
LIMIT 1
|
||||
""", (profile_id, source_title, source_artist, server_source or '', server_source or ''))
|
||||
row = cursor.fetchone()
|
||||
return dict(row) if row else None
|
||||
except Exception as e:
|
||||
logger.error(f"find_manual_library_match_by_metadata error: {e}")
|
||||
return None
|
||||
|
||||
def delete_manual_library_match(self, match_id: int, profile_id: int) -> bool:
|
||||
"""Delete match by PK id, scoped to profile_id."""
|
||||
try:
|
||||
|
|
@ -7315,12 +7373,12 @@ class MusicDatabase:
|
|||
logger.error("Cannot add track to wishlist: missing track ID")
|
||||
return False
|
||||
|
||||
track_source = spotify_track_data.get('provider') or spotify_track_data.get('source') or 'spotify'
|
||||
if self.get_manual_library_match(profile_id, track_source, track_id):
|
||||
from core.library import manual_library_match as _mlm
|
||||
if _mlm.get_match_for_track(self, profile_id, spotify_track_data):
|
||||
logger.info(
|
||||
"Skipping wishlist add for manually matched track: '%s' (%s:%s)",
|
||||
spotify_track_data.get('name', 'Unknown Track'),
|
||||
track_source,
|
||||
spotify_track_data.get('provider') or spotify_track_data.get('source') or 'unknown',
|
||||
track_id,
|
||||
)
|
||||
return True
|
||||
|
|
|
|||
|
|
@ -43,6 +43,7 @@ class _FakeDB:
|
|||
self.album_confidence = album_confidence
|
||||
self.sync_history_calls = []
|
||||
self.track_results_calls = []
|
||||
self.manual_matches = []
|
||||
|
||||
def check_track_exists(self, title, artist, confidence_threshold=0.7, server_source=None, album=None):
|
||||
key = (title.lower().strip(), artist.lower().strip())
|
||||
|
|
@ -71,6 +72,23 @@ class _FakeDB:
|
|||
def update_sync_history_track_results(self, batch_id, results_json):
|
||||
self.track_results_calls.append((batch_id, results_json))
|
||||
|
||||
def get_manual_library_match(self, profile_id, source, source_track_id, server_source=''):
|
||||
for match in self.manual_matches:
|
||||
if (
|
||||
match["profile_id"] == profile_id
|
||||
and match["source"] == source
|
||||
and match["source_track_id"] == source_track_id
|
||||
and match.get("server_source", "") == server_source
|
||||
):
|
||||
return match
|
||||
return None
|
||||
|
||||
def find_manual_library_match_by_source_track_id(self, profile_id, source_track_id, server_source=''):
|
||||
for match in self.manual_matches:
|
||||
if match["profile_id"] == profile_id and match["source_track_id"] == source_track_id:
|
||||
return match
|
||||
return None
|
||||
|
||||
|
||||
class _DBTrack:
|
||||
def __init__(self, title):
|
||||
|
|
@ -301,7 +319,7 @@ def test_manual_match_overrides_internal_force_download(monkeypatch):
|
|||
db = _FakeDB()
|
||||
monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db)
|
||||
monkeypatch.setattr(
|
||||
'core.library.manual_library_match.get_match',
|
||||
'core.library.manual_library_match.get_match_for_track',
|
||||
lambda *_args, **_kwargs: {'id': 1, 'library_track_id': 42},
|
||||
)
|
||||
|
||||
|
|
@ -324,6 +342,38 @@ def test_manual_match_overrides_internal_force_download(monkeypatch):
|
|||
assert removed == ['T1']
|
||||
|
||||
|
||||
def test_manual_match_saved_under_mirrored_source_overrides_wishlist_batch(monkeypatch):
|
||||
"""Wishlist batches honor matches saved from mirrored sync history."""
|
||||
db = _FakeDB()
|
||||
db.manual_matches.append({
|
||||
"id": 1,
|
||||
"profile_id": 1,
|
||||
"source": "mirrored",
|
||||
"source_track_id": "track-abc",
|
||||
"server_source": "",
|
||||
"library_track_id": 42,
|
||||
})
|
||||
monkeypatch.setattr('database.music_database.MusicDatabase', lambda: db)
|
||||
|
||||
removed = []
|
||||
_seed_batch(
|
||||
'B2m',
|
||||
force_download_all=True,
|
||||
ignore_manual_matches=False,
|
||||
profile_id=1,
|
||||
batch_source='wishlist',
|
||||
)
|
||||
deps = _build_deps(wishlist_remove=lambda td: removed.append(td.get('name')))
|
||||
tracks = [{'id': 'track-abc', 'name': 'Coffee Break', 'artists': [{'name': 'Zeds Dead'}], 'provider': 'wishlist'}]
|
||||
|
||||
mw.run_full_missing_tracks_process('B2m', 'wishlist', tracks, deps)
|
||||
|
||||
assert download_batches['B2m']['queue'] == []
|
||||
assert download_batches['B2m']['analysis_results'][0]['found'] is True
|
||||
assert download_batches['B2m']['analysis_results'][0]['match_reason'] == 'manual_library_match'
|
||||
assert removed == ['Coffee Break']
|
||||
|
||||
|
||||
def test_explicit_force_download_ignores_manual_match(monkeypatch):
|
||||
"""User-facing Force Download All can intentionally bypass manual matches."""
|
||||
db = _FakeDB()
|
||||
|
|
@ -331,7 +381,7 @@ def test_explicit_force_download_ignores_manual_match(monkeypatch):
|
|||
|
||||
calls = []
|
||||
monkeypatch.setattr(
|
||||
'core.library.manual_library_match.get_match',
|
||||
'core.library.manual_library_match.get_match_for_track',
|
||||
lambda *_args, **_kwargs: calls.append(True) or {'id': 1, 'library_track_id': 42},
|
||||
)
|
||||
|
||||
|
|
|
|||
|
|
@ -79,6 +79,58 @@ def test_get_returns_none_when_absent(db):
|
|||
assert db.get_manual_library_match(1, "spotify", "nonexistent") is None
|
||||
|
||||
|
||||
def test_get_match_for_track_falls_back_across_source_labels(db):
|
||||
db.save_manual_library_match(
|
||||
1,
|
||||
"mirrored",
|
||||
"track-abc",
|
||||
42,
|
||||
source_title="Coffee Break",
|
||||
source_artist="Zeds Dead",
|
||||
)
|
||||
|
||||
row = mlm.get_match_for_track(
|
||||
db,
|
||||
1,
|
||||
{
|
||||
"id": "track-abc",
|
||||
"name": "Coffee Break",
|
||||
"artists": [{"name": "Zeds Dead"}],
|
||||
"provider": "wishlist",
|
||||
},
|
||||
default_source="wishlist",
|
||||
)
|
||||
|
||||
assert row is not None
|
||||
assert row["library_track_id"] == 42
|
||||
|
||||
|
||||
def test_get_match_for_track_falls_back_to_source_title_artist(db):
|
||||
db.save_manual_library_match(
|
||||
1,
|
||||
"mirrored",
|
||||
"old-id",
|
||||
42,
|
||||
source_title="Coffee Break",
|
||||
source_artist="Zeds Dead",
|
||||
)
|
||||
|
||||
row = mlm.get_match_for_track(
|
||||
db,
|
||||
1,
|
||||
{
|
||||
"id": "new-id",
|
||||
"name": "Coffee Break",
|
||||
"artists": [{"name": "Zeds Dead"}],
|
||||
"provider": "musicbrainz",
|
||||
},
|
||||
default_source="wishlist",
|
||||
)
|
||||
|
||||
assert row is not None
|
||||
assert row["library_track_id"] == 42
|
||||
|
||||
|
||||
def test_add_to_wishlist_skips_manual_matched_track(db):
|
||||
db.save_manual_library_match(1, "spotify", "track-abc", 42)
|
||||
|
||||
|
|
@ -98,6 +150,32 @@ def test_add_to_wishlist_skips_manual_matched_track(db):
|
|||
assert db.get_wishlist_tracks(profile_id=1) == []
|
||||
|
||||
|
||||
def test_add_to_wishlist_skips_manual_match_saved_from_mirrored_source(db):
|
||||
db.save_manual_library_match(
|
||||
1,
|
||||
"mirrored",
|
||||
"track-abc",
|
||||
42,
|
||||
source_title="Coffee Break",
|
||||
source_artist="Zeds Dead",
|
||||
)
|
||||
|
||||
ok = db.add_to_wishlist(
|
||||
track_data={
|
||||
"id": "track-abc",
|
||||
"name": "Coffee Break",
|
||||
"artists": [{"name": "Zeds Dead"}],
|
||||
"album": {"name": "Coffee Break"},
|
||||
"provider": "wishlist",
|
||||
},
|
||||
failure_reason="Download failed",
|
||||
profile_id=1,
|
||||
)
|
||||
|
||||
assert ok is True
|
||||
assert db.get_wishlist_tracks(profile_id=1) == []
|
||||
|
||||
|
||||
def test_get_match_returns_none_when_db_lacks_manual_match_method():
|
||||
class _MinimalDB:
|
||||
pass
|
||||
|
|
@ -167,7 +245,7 @@ def test_wishlist_skips_manual_matched_track():
|
|||
mock_music_db = MagicMock()
|
||||
mock_music_db.check_track_exists = MagicMock()
|
||||
|
||||
with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}):
|
||||
with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}):
|
||||
from core.wishlist.processing import remove_tracks_already_in_library
|
||||
removed = remove_tracks_already_in_library(
|
||||
mock_wishlist_svc,
|
||||
|
|
@ -199,7 +277,7 @@ def test_wishlist_falls_through_when_no_match():
|
|||
mock_music_db = MagicMock()
|
||||
mock_music_db.check_track_exists.return_value = (None, 0.0)
|
||||
|
||||
with patch("core.library.manual_library_match.get_match", return_value=None):
|
||||
with patch("core.library.manual_library_match.get_match_for_track", return_value=None):
|
||||
from core.wishlist.processing import remove_tracks_already_in_library
|
||||
removed = remove_tracks_already_in_library(
|
||||
mock_wishlist_svc,
|
||||
|
|
@ -273,7 +351,7 @@ def test_master_analysis_marks_found():
|
|||
mock_deps.start_next_batch_of_downloads = MagicMock()
|
||||
mock_deps.reset_wishlist_auto_processing = MagicMock()
|
||||
|
||||
with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}), \
|
||||
with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}), \
|
||||
patch("database.music_database.MusicDatabase", return_value=mock_db):
|
||||
run_full_missing_tracks_process(batch_id, "playlist-1", [track_data], mock_deps)
|
||||
|
||||
|
|
@ -345,7 +423,7 @@ def test_master_analysis_manual_match_wins_over_internal_force_download():
|
|||
mock_deps.start_next_batch_of_downloads = MagicMock()
|
||||
mock_deps.reset_wishlist_auto_processing = MagicMock()
|
||||
|
||||
with patch("core.library.manual_library_match.get_match", return_value={"id": 1, "library_track_id": 42}), \
|
||||
with patch("core.library.manual_library_match.get_match_for_track", return_value={"id": 1, "library_track_id": 42}), \
|
||||
patch("database.music_database.MusicDatabase", return_value=mock_db):
|
||||
run_full_missing_tracks_process(batch_id, "playlist-1", [track_data], mock_deps)
|
||||
|
||||
|
|
|
|||
|
|
@ -240,7 +240,7 @@ def test_automatic_wishlist_cleanup_after_db_update_removes_manual_matches(monke
|
|||
)
|
||||
music_db = _CleanupMusicDatabase()
|
||||
monkeypatch.setattr(
|
||||
"core.library.manual_library_match.get_match",
|
||||
"core.library.manual_library_match.get_match_for_track",
|
||||
lambda *_args, **_kwargs: {"id": 1, "library_track_id": 42},
|
||||
)
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue