From e7faa9f02f42a95c1169036e7b284080d4576486 Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Wed, 15 Apr 2026 19:48:59 +0300 Subject: [PATCH] Use metadata source priority in track number repair job Use the shared metadata source priority when resolving album IDs, album searches, and tracklists in track number repair. Keeps Deezer and iTunes ahead of Spotify where configured, while still allowing the job to fall back through other supported sources. --- core/repair_jobs/track_number_repair.py | 232 +++++++++++++----------- tests/test_track_number_repair.py | 116 ++++++++++++ 2 files changed, 242 insertions(+), 106 deletions(-) create mode 100644 tests/test_track_number_repair.py diff --git a/core/repair_jobs/track_number_repair.py b/core/repair_jobs/track_number_repair.py index ac39b9d4..65080101 100644 --- a/core/repair_jobs/track_number_repair.py +++ b/core/repair_jobs/track_number_repair.py @@ -1,8 +1,9 @@ """Track Number Repair Job — fixes embedded track number tags and filename prefixes. Detects albums where 3+ files share the same track number (the "all tracks = 01" -bug pattern), then uses cascading API lookups (Spotify → iTunes → MusicBrainz → -AudioDB) to resolve the correct tracklist and repair each file. +bug pattern), then uses cascading API lookups in metadata-source priority order +before falling back to MusicBrainz and AudioDB to resolve the correct tracklist +and repair each file. """ import os @@ -10,6 +11,12 @@ import re from difflib import SequenceMatcher from typing import Any, Dict, List, Optional, Tuple +from core.metadata_service import ( + get_album_tracks_for_source, + get_client_for_source, + get_primary_source, + get_source_priority, +) from core.repair_jobs import register_job from core.repair_jobs.base import JobContext, JobResult, RepairJob from utils.logging_config import get_logger @@ -24,6 +31,14 @@ _PLACEHOLDER_IDS = { 'unknown', 'none', 'null', '', } +_SOURCE_ALBUM_ID_COLUMNS = ( + ('spotify', 'spotify_album_id'), + ('itunes', 'itunes_album_id'), + ('deezer', 'deezer_id'), + ('discogs', 'discogs_id'), + ('hydrabase', 'soul_id'), +) + @register_job class TrackNumberRepairJob(RepairJob): @@ -32,8 +47,9 @@ class TrackNumberRepairJob(RepairJob): description = 'Detects mismatched track numbers using API lookups (dry run by default)' help_text = ( 'Scans album folders and compares each file\'s track number against the correct ' - 'tracklist from Spotify or iTunes. If a file\'s embedded track number doesn\'t match ' - 'the API data, the job creates a finding showing what needs to change.\n\n' + 'tracklist from the configured metadata sources. If a file\'s embedded track ' + 'number doesn\'t match the API data, the job creates a finding showing what ' + 'needs to change.\n\n' 'In dry run mode (default), no files are modified — you review each proposed change ' 'in the Findings tab and decide what to approve. Disable dry run in settings to let ' 'the job automatically rename and re-number files.\n\n' @@ -187,7 +203,7 @@ class TrackNumberRepairJob(RepairJob): logger.info("Anomaly detected in %s — %d files share track number(s): %s", os.path.basename(folder_path), sum(duped.values()), duped) - # Resolve album tracklist via cascading fallbacks + # Resolve album tracklist via source-aware cascading fallbacks api_tracks = self._resolve_album_tracklist(file_track_data, folder_path, context, scan_state) if not api_tracks: result.skipped += len(filenames) @@ -253,38 +269,24 @@ class TrackNumberRepairJob(RepairJob): cache = scan_state['album_tracks_cache'] folder_name = os.path.basename(folder_path) + primary_source = get_primary_source() + source_priority = get_source_priority(primary_source) - # Fallback 0: Check DB first — if these files are tracked and their album - # has a spotify_album_id, use that directly without reading file tags - db_album_id, db_spotify_album_id, db_itunes_album_id = _lookup_album_ids_from_db( - file_track_data, context - ) - if db_spotify_album_id and _is_valid_album_id(db_spotify_album_id): - tracks = _get_album_tracklist(db_spotify_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via DB spotify_album_id: %s", folder_name, db_spotify_album_id) - return tracks - if db_itunes_album_id and _is_valid_album_id(db_itunes_album_id): - tracks = _get_album_tracklist(db_itunes_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via DB itunes_album_id: %s", folder_name, db_itunes_album_id) - return tracks + # Fallback 0: Check DB first. If any tracked file already has source IDs, + # prefer the configured source order and use the first available album ID. + source_album_ids = _lookup_album_ids_from_db(file_track_data, context) # Collect available IDs from file tags (fallback when DB has no IDs) - spotify_album_id = None - itunes_album_id = None spotify_track_id = None mb_album_id = None album_name = None artist_name = None for fpath, fname, _ in file_track_data: - if not spotify_album_id or not itunes_album_id: + if 'spotify' not in source_album_ids or 'itunes' not in source_album_ids: aid, source = _read_album_id_from_file(fpath) - if aid and source == 'spotify' and not spotify_album_id: - spotify_album_id = aid - elif aid and source == 'itunes' and not itunes_album_id: - itunes_album_id = aid + if aid and source in ('spotify', 'itunes') and source not in source_album_ids: + source_album_ids[source] = aid if not spotify_track_id: spotify_track_id = _read_spotify_track_id_from_file(fpath) @@ -295,31 +297,27 @@ class TrackNumberRepairJob(RepairJob): if not album_name: album_name, artist_name = _read_album_artist_from_file(fpath) - if spotify_album_id and itunes_album_id and spotify_track_id and mb_album_id and album_name: + if source_album_ids and spotify_track_id and mb_album_id and album_name: break - # Fallback 1: Spotify album ID from file tags - if spotify_album_id and _is_valid_album_id(spotify_album_id): - tracks = _get_album_tracklist(spotify_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via Spotify album ID: %s", folder_name, spotify_album_id) - return tracks + # Fallback 1: Album IDs from DB / file tags, using source priority + for source in source_priority: + album_id = source_album_ids.get(source) + if album_id and _is_valid_album_id(album_id): + tracks = _get_album_tracklist(source, album_id, cache) + if tracks: + logger.info("[Repair] %s — resolved via %s album ID: %s", + folder_name, source, album_id) + return tracks - # Fallback 2: iTunes album ID - if itunes_album_id and _is_valid_album_id(itunes_album_id): - tracks = _get_album_tracklist(itunes_album_id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via iTunes album ID: %s", folder_name, itunes_album_id) - return tracks - - # Fallback 3: Spotify track ID → discover album ID - client = context.spotify_client - if spotify_track_id and client and client.is_spotify_authenticated() and not context.is_spotify_rate_limited(): + # Fallback 2: Spotify track ID → discover album ID + client = get_client_for_source('spotify') + if spotify_track_id and client: try: track_details = client.get_track_details(spotify_track_id) if track_details and track_details.get('album', {}).get('id'): real_album_id = track_details['album']['id'] - tracks = _get_album_tracklist(real_album_id, context, cache) + tracks = _get_album_tracklist('spotify', real_album_id, cache) if tracks: logger.info("[Repair] %s — resolved via Spotify track ID %s → album %s", folder_name, spotify_track_id, real_album_id) @@ -327,29 +325,35 @@ class TrackNumberRepairJob(RepairJob): except Exception as e: logger.debug("Spotify track lookup failed for %s: %s", spotify_track_id, e) - # Fallback 4: Search Spotify/iTunes by album name + artist - if album_name and client and not context.is_spotify_rate_limited(): - try: - query = f"{artist_name} {album_name}" if artist_name else album_name - results = client.search_albums(query, limit=5) - if results: - best = results[0] - tracks = _get_album_tracklist(best.id, context, cache) - if tracks: - logger.info("[Repair] %s — resolved via album search: '%s' → %s", - folder_name, query, best.id) - return tracks - except Exception as e: - logger.debug("Album search failed for '%s': %s", album_name, e) + # Fallback 3: Search metadata sources by album name + artist + if album_name: + query = f"{artist_name} {album_name}" if artist_name else album_name + for source in source_priority: + client = get_client_for_source(source) + if not client or not hasattr(client, 'search_albums'): + continue + try: + results = client.search_albums(query, limit=5) + if results: + best = results[0] + best_album_id = getattr(best, 'id', None) if not isinstance(best, dict) else best.get('id') + if best_album_id: + tracks = _get_album_tracklist(source, str(best_album_id), cache) + if tracks: + logger.info("[Repair] %s — resolved via %s album search: '%s' → %s", + folder_name, source, query, best_album_id) + return tracks + except Exception as e: + logger.debug("%s album search failed for '%s': %s", source.capitalize(), album_name, e) - # Fallback 5: MusicBrainz album ID from tags + # Fallback 4: MusicBrainz album ID from tags if mb_album_id: tracks = _get_tracklist_from_musicbrainz(mb_album_id, context, cache) if tracks: logger.info("[Repair] %s — resolved via MusicBrainz album ID: %s", folder_name, mb_album_id) return tracks - # Fallback 6: AudioDB → MusicBrainz + # Fallback 5: AudioDB → MusicBrainz if album_name and artist_name: adb_mb_id = _get_musicbrainz_id_via_audiodb(artist_name, album_name, context) if adb_mb_id and adb_mb_id != mb_album_id: @@ -800,25 +804,36 @@ def _update_db_file_path(db, old_path: str, new_path: str): def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], - context: JobContext) -> Tuple[Optional[str], Optional[str], Optional[str]]: + context: JobContext) -> Dict[str, Optional[str]]: """Look up album IDs from the database using file paths. Checks if any of the files in this folder are tracked in the DB, and if so, - returns the album's (album_id, spotify_album_id, itunes_album_id). + returns a mapping of metadata source -> album ID. This avoids expensive file tag reads and API calls when the DB already knows. """ if not context.db: - return None, None, None + return {} conn = None try: conn = context.db._get_connection() cursor = conn.cursor() + cursor.execute("PRAGMA table_info(albums)") + album_columns = {row[1] for row in cursor.fetchall()} + + selected_sources = [ + (source, column) + for source, column in _SOURCE_ALBUM_ID_COLUMNS + if column in album_columns + ] + if not selected_sources: + return {} # Try each file path until we find one tracked in the DB for fpath, _, _ in file_track_data: - cursor.execute(""" - SELECT t.album_id, al.spotify_album_id, al.itunes_album_id + select_cols = ", ".join(f"al.{column}" for _source, column in selected_sources) + cursor.execute(f""" + SELECT {select_cols} FROM tracks t JOIN albums al ON al.id = t.album_id WHERE t.file_path = ? @@ -826,7 +841,11 @@ def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], """, (fpath,)) row = cursor.fetchone() if row: - return row[0], row[1], row[2] + return { + source: str(row[idx]) + for idx, (source, _column) in enumerate(selected_sources) + if row[idx] + } except Exception as e: logger.debug("Error looking up album IDs from DB: %s", e) @@ -834,7 +853,7 @@ def _lookup_album_ids_from_db(file_track_data: List[Tuple[str, str, Any]], if conn: conn.close() - return None, None, None + return {} def _lookup_album_artist_art(file_track_data: List[Tuple[str, str, Any]], @@ -1018,52 +1037,53 @@ def _repair_single_track(file_path: str, filename: str, api_tracks: List[Dict], return True -def _get_album_tracklist(album_id: str, context: JobContext, - cache: dict) -> Optional[List[Dict]]: - """Fetch an album tracklist from Spotify or iTunes, with per-scan caching. +def _normalize_album_track_items(data) -> List[Dict[str, Any]]: + """Normalize album track payloads to a list of dicts.""" + if isinstance(data, list): + return data + if not isinstance(data, dict): + return [] + items = data.get('items') + if isinstance(items, list): + return items + tracks = data.get('tracks') + if isinstance(tracks, list): + return tracks + if isinstance(tracks, dict): + nested_items = tracks.get('items') + if isinstance(nested_items, list): + return nested_items + return [] + + +def _get_album_tracklist(source: str, album_id: str, cache: dict) -> Optional[List[Dict]]: + """Fetch an album tracklist from a specific source, with per-scan caching. Returns a list of dicts with at least 'name' and 'track_number' keys, or None if lookup fails. """ - if album_id in cache: - return cache[album_id] + cache_key = f"{source}:{album_id}" + if cache_key in cache: + return cache[cache_key] result = None - # Try Spotify first - client = context.spotify_client - if client and client.is_spotify_authenticated() and not context.is_spotify_rate_limited(): - try: - data = client.get_album_tracks(album_id) - if data and 'items' in data and data['items']: - result = [ - { - 'name': item.get('name', ''), - 'track_number': item.get('track_number'), - 'disc_number': item.get('disc_number', 1), - } - for item in data['items'] - ] - except Exception as e: - logger.debug("Spotify get_album_tracks failed for %s: %s", album_id, e) + try: + data = get_album_tracks_for_source(source, album_id) + items = _normalize_album_track_items(data) + if items: + result = [ + { + 'name': item.get('name', '') if isinstance(item, dict) else getattr(item, 'name', ''), + 'track_number': item.get('track_number') if isinstance(item, dict) else getattr(item, 'track_number', None), + 'disc_number': item.get('disc_number', 1) if isinstance(item, dict) else getattr(item, 'disc_number', 1), + } + for item in items + ] + except Exception as e: + logger.debug("%s get_album_tracks failed for %s: %s", source.capitalize(), album_id, e) - # Fallback to iTunes - if not result and context.itunes_client: - try: - data = context.itunes_client.get_album_tracks(album_id) - if data and 'items' in data and data['items']: - result = [ - { - 'name': item.get('name', ''), - 'track_number': item.get('track_number'), - 'disc_number': item.get('disc_number', 1), - } - for item in data['items'] - ] - except Exception as e: - logger.debug("iTunes get_album_tracks failed for %s: %s", album_id, e) - - cache[album_id] = result + cache[cache_key] = result return result diff --git a/tests/test_track_number_repair.py b/tests/test_track_number_repair.py new file mode 100644 index 00000000..7b5cfb77 --- /dev/null +++ b/tests/test_track_number_repair.py @@ -0,0 +1,116 @@ +import sys +import types +from types import SimpleNamespace + +# Stub optional Spotify dependency so metadata_service can import in tests. +if 'spotipy' not in sys.modules: + spotipy = types.ModuleType('spotipy') + oauth2 = types.ModuleType('spotipy.oauth2') + + class _DummySpotify: + pass + + class _DummyOAuth: + pass + + spotipy.Spotify = _DummySpotify + oauth2.SpotifyOAuth = _DummyOAuth + oauth2.SpotifyClientCredentials = _DummyOAuth + spotipy.oauth2 = oauth2 + sys.modules['spotipy'] = spotipy + sys.modules['spotipy.oauth2'] = oauth2 + +if 'config.settings' not in sys.modules: + config_mod = types.ModuleType('config') + settings_mod = types.ModuleType('config.settings') + + class _DummyConfigManager: + def get(self, key, default=None): + return default + + settings_mod.config_manager = _DummyConfigManager() + config_mod.settings = settings_mod + sys.modules['config'] = config_mod + sys.modules['config.settings'] = settings_mod + +from core.repair_jobs import track_number_repair as tnr + + +class _FakeSearchClient: + def __init__(self, search_results=None): + self.search_results = search_results or [] + self.search_calls = [] + + def search_albums(self, query, limit=10): + self.search_calls.append((query, limit)) + return self.search_results + + +def _make_context(): + return SimpleNamespace(db=None, mb_client=None) + + +def test_resolve_album_tracklist_prefers_primary_source_album_id(monkeypatch): + context = _make_context() + file_track_data = [('/music/test.flac', '01 - Track.flac', None)] + cache = {'album_tracks_cache': {}, 'title_similarity': 0.8} + calls = [] + + monkeypatch.setattr(tnr, 'get_primary_source', lambda: 'deezer') + monkeypatch.setattr( + tnr, + '_lookup_album_ids_from_db', + lambda *_args, **_kwargs: {'spotify': 'sp-album', 'itunes': 'it-album', 'deezer': 'dz-album'}, + ) + monkeypatch.setattr(tnr, '_read_album_id_from_file', lambda *_args, **_kwargs: (None, None)) + monkeypatch.setattr(tnr, '_read_spotify_track_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_musicbrainz_album_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_album_artist_from_file', lambda *_args, **_kwargs: (None, None)) + + def fake_get_album_tracks_for_source(source, album_id): + calls.append((source, album_id)) + return {'items': [{'name': f'{source} track', 'track_number': 1, 'disc_number': 1}]} + + monkeypatch.setattr(tnr, 'get_album_tracks_for_source', fake_get_album_tracks_for_source) + + result = tnr.TrackNumberRepairJob()._resolve_album_tracklist(file_track_data, '/music', context, cache) + + assert result is not None + assert result[0]['name'] == 'deezer track' + assert calls == [('deezer', 'dz-album')] + + +def test_resolve_album_tracklist_uses_source_priority_for_search(monkeypatch): + context = _make_context() + file_track_data = [('/music/test.flac', 'Track.flac', None)] + cache = {'album_tracks_cache': {}, 'title_similarity': 0.8} + track_calls = [] + + discogs_client = _FakeSearchClient([SimpleNamespace(id='dg-album')]) + spotify_client = _FakeSearchClient([SimpleNamespace(id='sp-album')]) + + monkeypatch.setattr(tnr, 'get_primary_source', lambda: 'discogs') + monkeypatch.setattr(tnr, '_lookup_album_ids_from_db', lambda *_args, **_kwargs: {}) + monkeypatch.setattr(tnr, '_read_album_id_from_file', lambda *_args, **_kwargs: (None, None)) + monkeypatch.setattr(tnr, '_read_spotify_track_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_musicbrainz_album_id_from_file', lambda *_args, **_kwargs: None) + monkeypatch.setattr(tnr, '_read_album_artist_from_file', lambda *_args, **_kwargs: ('Album Title', 'Artist Name')) + monkeypatch.setattr( + tnr, + 'get_client_for_source', + lambda source: {'discogs': discogs_client, 'spotify': spotify_client}.get(source), + ) + + def fake_get_album_tracks_for_source(source, album_id): + track_calls.append((source, album_id)) + return {'items': [{'name': f'{source} track', 'track_number': 1, 'disc_number': 1}]} + + monkeypatch.setattr(tnr, 'get_album_tracks_for_source', fake_get_album_tracks_for_source) + + result = tnr.TrackNumberRepairJob()._resolve_album_tracklist(file_track_data, '/music', context, cache) + + assert result is not None + assert result[0]['name'] == 'discogs track' + assert discogs_client.search_calls == [('Artist Name Album Title', 5)] + assert spotify_client.search_calls == [] + assert track_calls == [('discogs', 'dg-album')]