Merge pull request #298 from kettui/fix/respect-metadata-provider-in-album-completeness-job
Honor primary metadata source in album_completeness job and associated repair flow
This commit is contained in:
commit
3618f3fa7f
6 changed files with 569 additions and 136 deletions
|
|
@ -17,6 +17,9 @@ logger = get_logger("metadata_service")
|
|||
|
||||
MetadataProvider = Literal["spotify", "itunes", "auto"]
|
||||
|
||||
# Ordered by fallback preference. Higher-priority sources appear earlier.
|
||||
METADATA_SOURCE_PRIORITY = ('deezer', 'itunes', 'spotify', 'discogs', 'hydrabase')
|
||||
|
||||
_client_cache_lock = threading.RLock()
|
||||
_client_cache: Dict[str, Any] = {}
|
||||
|
||||
|
|
@ -62,7 +65,72 @@ def get_primary_client():
|
|||
|
||||
This is THE single source of truth for "which client should I call?"
|
||||
"""
|
||||
return _get_client_for_source(get_primary_source())
|
||||
return get_client_for_source(get_primary_source())
|
||||
|
||||
|
||||
def get_source_priority(preferred_source: str):
|
||||
"""Return supported sources with the preferred source first."""
|
||||
ordered = []
|
||||
|
||||
if preferred_source in METADATA_SOURCE_PRIORITY:
|
||||
ordered.append(preferred_source)
|
||||
|
||||
for source in METADATA_SOURCE_PRIORITY:
|
||||
if source not in ordered:
|
||||
ordered.append(source)
|
||||
|
||||
return ordered
|
||||
|
||||
|
||||
def get_client_for_source(source: str):
|
||||
"""Get the client object for an exact metadata source.
|
||||
|
||||
Returns the matching client or None if that source is unavailable.
|
||||
No fallback swaps.
|
||||
"""
|
||||
if source == 'spotify':
|
||||
try:
|
||||
import importlib
|
||||
ws = importlib.import_module('web_server')
|
||||
sc = getattr(ws, 'spotify_client', None)
|
||||
if sc and sc.is_spotify_authenticated():
|
||||
return sc
|
||||
except Exception:
|
||||
pass
|
||||
return None
|
||||
|
||||
if source == 'deezer':
|
||||
return get_deezer_client()
|
||||
|
||||
if source == 'discogs':
|
||||
return get_discogs_client()
|
||||
|
||||
if source == 'hydrabase':
|
||||
return get_hydrabase_client(allow_fallback=False)
|
||||
|
||||
if source == 'itunes':
|
||||
return get_itunes_client()
|
||||
|
||||
return None
|
||||
|
||||
|
||||
def get_album_tracks_for_source(source: str, album_id: str):
|
||||
"""Get album tracks for an exact source.
|
||||
|
||||
Returns Spotify-compatible dict/list data or None.
|
||||
No fallback swaps.
|
||||
"""
|
||||
client = get_client_for_source(source)
|
||||
if not client:
|
||||
return None
|
||||
|
||||
try:
|
||||
fetch = getattr(client, 'get_album_tracks_dict', None) if source == 'hydrabase' else getattr(client, 'get_album_tracks', None)
|
||||
if not fetch:
|
||||
return None
|
||||
return fetch(album_id)
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
def get_deezer_client():
|
||||
|
|
@ -122,8 +190,12 @@ def get_discogs_client(token: Optional[str] = None):
|
|||
return client
|
||||
|
||||
|
||||
def get_hydrabase_client():
|
||||
"""Return current Hydrabase client if connected, else iTunes fallback."""
|
||||
def get_hydrabase_client(allow_fallback: bool = True):
|
||||
"""Return current Hydrabase client if connected.
|
||||
|
||||
If allow_fallback is True, return iTunes fallback when Hydrabase is not
|
||||
connected. If False, return None instead.
|
||||
"""
|
||||
try:
|
||||
import importlib
|
||||
ws = importlib.import_module('web_server')
|
||||
|
|
@ -132,7 +204,9 @@ def get_hydrabase_client():
|
|||
return client
|
||||
except Exception:
|
||||
pass
|
||||
return get_itunes_client()
|
||||
if allow_fallback:
|
||||
return get_itunes_client()
|
||||
return None
|
||||
|
||||
|
||||
def clear_cached_metadata_clients():
|
||||
|
|
@ -206,7 +280,7 @@ class MetadataService:
|
|||
self.preferred_provider = preferred_provider
|
||||
self.spotify = SpotifyClient()
|
||||
self._fallback_source = get_primary_source()
|
||||
self.itunes = _get_client_for_source(self._fallback_source)
|
||||
self.itunes = get_client_for_source(self._fallback_source)
|
||||
|
||||
self._log_initialization()
|
||||
|
||||
|
|
@ -378,7 +452,7 @@ class MetadataService:
|
|||
self.spotify.reload_config()
|
||||
new_source = get_primary_source()
|
||||
self._fallback_source = new_source
|
||||
self.itunes = _get_client_for_source(new_source)
|
||||
self.itunes = get_client_for_source(new_source)
|
||||
self._log_initialization()
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1,5 +1,10 @@
|
|||
"""Album Completeness Checker Job — finds albums missing tracks."""
|
||||
|
||||
from core.metadata_service import (
|
||||
get_album_tracks_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
|
||||
|
|
@ -14,8 +19,9 @@ class AlbumCompletenessJob(RepairJob):
|
|||
description = 'Checks if all tracks from albums are present'
|
||||
help_text = (
|
||||
'Compares the number of tracks you have for each album against the expected total '
|
||||
'from the album tracklist (via Spotify, iTunes, or Deezer). Albums where tracks are '
|
||||
'missing get flagged as findings with details about which tracks are absent.\n\n'
|
||||
'from the active metadata provider first, then other supported sources if needed. '
|
||||
'Albums where tracks are missing get flagged as findings with details about which '
|
||||
'tracks are absent.\n\n'
|
||||
'Useful for catching partial downloads or albums where some tracks failed to download. '
|
||||
'You can use the Download Missing feature from the album page to fill gaps.\n\n'
|
||||
'Settings:\n'
|
||||
|
|
@ -40,10 +46,13 @@ class AlbumCompletenessJob(RepairJob):
|
|||
settings = self._get_settings(context)
|
||||
min_tracks = settings.get('min_tracks_for_check', 3)
|
||||
min_completion_pct = settings.get('min_completion_pct', 0)
|
||||
primary_source = self._get_primary_source()
|
||||
|
||||
# Fetch all albums with ANY external source ID — not just Spotify
|
||||
albums = []
|
||||
conn = None
|
||||
has_itunes = False
|
||||
has_deezer = False
|
||||
try:
|
||||
conn = context.db._get_connection()
|
||||
cursor = conn.cursor()
|
||||
|
|
@ -53,29 +62,44 @@ class AlbumCompletenessJob(RepairJob):
|
|||
columns = {row[1] for row in cursor.fetchall()}
|
||||
has_itunes = 'itunes_album_id' in columns
|
||||
has_deezer = 'deezer_id' in columns
|
||||
has_discogs = 'discogs_id' in columns
|
||||
has_hydrabase = 'soul_id' in columns
|
||||
|
||||
# Build SELECT with available source ID columns
|
||||
select_cols = [
|
||||
'al.id', 'al.title', 'ar.name', 'al.spotify_album_id', 'al.track_count',
|
||||
'COUNT(t.id) as actual_count', 'al.thumb_url', 'ar.thumb_url',
|
||||
('al.id', 'album_id'),
|
||||
('al.title', 'album_title'),
|
||||
('ar.name', 'artist_name'),
|
||||
('al.spotify_album_id', 'spotify_album_id'),
|
||||
('al.track_count', 'track_count'),
|
||||
('COUNT(t.id)', 'actual_count'),
|
||||
('al.thumb_url', 'album_thumb_url'),
|
||||
('ar.thumb_url', 'artist_thumb_url'),
|
||||
]
|
||||
if has_itunes:
|
||||
select_cols.append('al.itunes_album_id')
|
||||
select_cols.append(('al.itunes_album_id', 'itunes_album_id'))
|
||||
if has_deezer:
|
||||
select_cols.append('al.deezer_id')
|
||||
select_cols.append(('al.deezer_id', 'deezer_album_id'))
|
||||
if has_discogs:
|
||||
select_cols.append(('al.discogs_id', 'discogs_album_id'))
|
||||
if has_hydrabase:
|
||||
select_cols.append(('al.soul_id', 'hydrabase_album_id'))
|
||||
|
||||
# WHERE: album has at least one source ID
|
||||
where_parts = []
|
||||
if True: # spotify always exists
|
||||
where_parts.append("(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')")
|
||||
where_parts = ["(al.spotify_album_id IS NOT NULL AND al.spotify_album_id != '')"]
|
||||
if has_itunes:
|
||||
where_parts.append("(al.itunes_album_id IS NOT NULL AND al.itunes_album_id != '')")
|
||||
if has_deezer:
|
||||
where_parts.append("(al.deezer_id IS NOT NULL AND al.deezer_id != '')")
|
||||
if has_discogs:
|
||||
where_parts.append("(al.discogs_id IS NOT NULL AND al.discogs_id != '')")
|
||||
if has_hydrabase:
|
||||
where_parts.append("(al.soul_id IS NOT NULL AND al.soul_id != '')")
|
||||
where_clause = ' OR '.join(where_parts)
|
||||
|
||||
select_sql = ', '.join(f'{expr} AS {alias}' for expr, alias in select_cols)
|
||||
cursor.execute(f"""
|
||||
SELECT {', '.join(select_cols)}
|
||||
SELECT {select_sql}
|
||||
FROM albums al
|
||||
LEFT JOIN artists ar ON ar.id = al.artist_id
|
||||
LEFT JOIN tracks t ON t.album_id = al.id
|
||||
|
|
@ -83,6 +107,7 @@ class AlbumCompletenessJob(RepairJob):
|
|||
GROUP BY al.id
|
||||
""")
|
||||
albums = cursor.fetchall()
|
||||
column_index = {alias: idx for idx, (_, alias) in enumerate(select_cols)}
|
||||
except Exception as e:
|
||||
logger.error("Error fetching albums: %s", e, exc_info=True)
|
||||
result.errors += 1
|
||||
|
|
@ -100,27 +125,24 @@ class AlbumCompletenessJob(RepairJob):
|
|||
if context.report_progress:
|
||||
context.report_progress(phase=f'Checking {total} albums...', total=total)
|
||||
|
||||
# Determine column positions based on what we selected
|
||||
# Fixed: 0=id, 1=title, 2=artist, 3=spotify_id, 4=track_count, 5=actual, 6=album_thumb, 7=artist_thumb
|
||||
itunes_col = 8 if has_itunes else None
|
||||
deezer_col = (9 if has_itunes else 8) if has_deezer else None
|
||||
|
||||
for i, row in enumerate(albums):
|
||||
if context.check_stop():
|
||||
return result
|
||||
if i % 10 == 0 and context.wait_if_paused():
|
||||
return result
|
||||
|
||||
album_id = row[0]
|
||||
title = row[1]
|
||||
artist_name = row[2]
|
||||
spotify_album_id = row[3]
|
||||
db_track_count = row[4]
|
||||
actual_count = row[5]
|
||||
album_thumb = row[6]
|
||||
artist_thumb = row[7]
|
||||
itunes_album_id = row[itunes_col] if itunes_col is not None else None
|
||||
deezer_album_id = row[deezer_col] if deezer_col is not None else None
|
||||
album_id = row[column_index['album_id']]
|
||||
title = row[column_index['album_title']]
|
||||
artist_name = row[column_index['artist_name']]
|
||||
spotify_album_id = row[column_index['spotify_album_id']]
|
||||
db_track_count = row[column_index['track_count']]
|
||||
actual_count = row[column_index['actual_count']]
|
||||
album_thumb = row[column_index['album_thumb_url']]
|
||||
artist_thumb = row[column_index['artist_thumb_url']]
|
||||
itunes_album_id = row[column_index['itunes_album_id']] if 'itunes_album_id' in column_index else None
|
||||
deezer_album_id = row[column_index['deezer_album_id']] if 'deezer_album_id' in column_index else None
|
||||
discogs_album_id = row[column_index['discogs_album_id']] if 'discogs_album_id' in column_index else None
|
||||
hydrabase_album_id = row[column_index['hydrabase_album_id']] if 'hydrabase_album_id' in column_index else None
|
||||
|
||||
result.scanned += 1
|
||||
|
||||
|
|
@ -135,10 +157,16 @@ class AlbumCompletenessJob(RepairJob):
|
|||
# If we don't know the expected track count, try to get it from an API
|
||||
expected_total = db_track_count
|
||||
|
||||
album_ids = {
|
||||
'spotify': spotify_album_id or '',
|
||||
'itunes': itunes_album_id or '',
|
||||
'deezer': deezer_album_id or '',
|
||||
'discogs': discogs_album_id or '',
|
||||
'hydrabase': hydrabase_album_id or '',
|
||||
}
|
||||
|
||||
if not expected_total:
|
||||
expected_total = self._get_expected_total(
|
||||
context, spotify_album_id, itunes_album_id, deezer_album_id
|
||||
)
|
||||
expected_total = self._get_expected_total(context, primary_source, album_ids)
|
||||
|
||||
# Skip singles/EPs based on expected track count (not local count)
|
||||
if expected_total and expected_total < min_tracks:
|
||||
|
|
@ -169,9 +197,7 @@ class AlbumCompletenessJob(RepairJob):
|
|||
continue
|
||||
|
||||
# Album is incomplete — try to find which tracks are missing
|
||||
missing_tracks = self._find_missing_tracks(
|
||||
context, album_id, spotify_album_id, itunes_album_id, deezer_album_id
|
||||
)
|
||||
missing_tracks = self._find_missing_tracks(context, primary_source, album_id, album_ids)
|
||||
|
||||
if context.report_progress:
|
||||
context.report_progress(
|
||||
|
|
@ -180,8 +206,6 @@ class AlbumCompletenessJob(RepairJob):
|
|||
)
|
||||
if context.create_finding:
|
||||
try:
|
||||
# Use whichever source ID is available
|
||||
source_id = spotify_album_id or itunes_album_id or deezer_album_id or ''
|
||||
context.create_finding(
|
||||
job_id=self.job_id,
|
||||
finding_type='incomplete_album',
|
||||
|
|
@ -198,9 +222,13 @@ class AlbumCompletenessJob(RepairJob):
|
|||
'album_id': album_id,
|
||||
'album_title': title,
|
||||
'artist': artist_name,
|
||||
'primary_source': primary_source,
|
||||
'primary_album_id': self._get_album_id_for_source(primary_source, album_ids) or '',
|
||||
'spotify_album_id': spotify_album_id or '',
|
||||
'itunes_album_id': itunes_album_id or '',
|
||||
'deezer_album_id': deezer_album_id or '',
|
||||
'discogs_album_id': discogs_album_id or '',
|
||||
'hydrabase_album_id': hydrabase_album_id or '',
|
||||
'expected_tracks': expected_total,
|
||||
'actual_tracks': actual_count,
|
||||
'missing_tracks': missing_tracks,
|
||||
|
|
@ -220,42 +248,24 @@ class AlbumCompletenessJob(RepairJob):
|
|||
context.update_progress(total, total)
|
||||
|
||||
logger.info("Completeness check: %d albums checked, %d incomplete found",
|
||||
result.scanned, result.findings_created)
|
||||
result.scanned, result.findings_created)
|
||||
return result
|
||||
|
||||
def _get_expected_total(self, context, spotify_id, itunes_id, deezer_id):
|
||||
"""Try to get the expected track count from any available API source."""
|
||||
# Try Spotify first
|
||||
if spotify_id and context.spotify_client and not context.is_spotify_rate_limited():
|
||||
try:
|
||||
album_data = context.spotify_client.get_album(spotify_id)
|
||||
if album_data:
|
||||
total = album_data.get('total_tracks', 0)
|
||||
if total:
|
||||
return total
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# Try fallback client (iTunes or Deezer) — both return Spotify-compatible format
|
||||
# Match the ID to the actual client type to avoid passing iTunes ID to Deezer or vice versa
|
||||
if context.itunes_client:
|
||||
is_deezer = type(context.itunes_client).__name__ == 'DeezerClient'
|
||||
primary_id = deezer_id if is_deezer else itunes_id
|
||||
secondary_id = itunes_id if is_deezer else deezer_id
|
||||
for fid in [primary_id, secondary_id]:
|
||||
if not fid:
|
||||
continue
|
||||
try:
|
||||
api_tracks = context.itunes_client.get_album_tracks(fid)
|
||||
if api_tracks and 'items' in api_tracks:
|
||||
return len(api_tracks['items'])
|
||||
except Exception:
|
||||
pass
|
||||
def _get_expected_total(self, context, primary_source, album_ids):
|
||||
"""Try to get the expected track count from the active metadata provider first."""
|
||||
for source in get_source_priority(primary_source):
|
||||
album_id = self._get_album_id_for_source(source, album_ids)
|
||||
if not album_id:
|
||||
continue
|
||||
api_tracks = self._get_album_tracks(source, album_id)
|
||||
items = self._extract_track_items(api_tracks)
|
||||
if items:
|
||||
return len(items)
|
||||
|
||||
return 0
|
||||
|
||||
def _find_missing_tracks(self, context, album_id, spotify_id, itunes_id, deezer_id):
|
||||
"""Identify which specific tracks are missing using any available API source."""
|
||||
def _find_missing_tracks(self, context, primary_source, album_id, album_ids):
|
||||
"""Identify which specific tracks are missing using the active metadata provider first."""
|
||||
# Get track numbers we already have
|
||||
owned_numbers = set()
|
||||
conn = None
|
||||
|
|
@ -274,37 +284,23 @@ class AlbumCompletenessJob(RepairJob):
|
|||
if conn:
|
||||
conn.close()
|
||||
|
||||
# Try Spotify first
|
||||
api_tracks = None
|
||||
if spotify_id and context.spotify_client and not context.is_spotify_rate_limited():
|
||||
try:
|
||||
api_tracks = context.spotify_client.get_album_tracks(spotify_id)
|
||||
except Exception as e:
|
||||
logger.debug("Error getting Spotify album tracks for %s: %s", spotify_id, e)
|
||||
for source in get_source_priority(primary_source):
|
||||
source_album_id = self._get_album_id_for_source(source, album_ids)
|
||||
if not source_album_id:
|
||||
continue
|
||||
api_tracks = self._get_album_tracks(source, source_album_id)
|
||||
if self._extract_track_items(api_tracks):
|
||||
break
|
||||
|
||||
# Try fallback client (iTunes or Deezer)
|
||||
if not api_tracks or 'items' not in (api_tracks or {}):
|
||||
if context.itunes_client:
|
||||
is_deezer = type(context.itunes_client).__name__ == 'DeezerClient'
|
||||
primary_id = deezer_id if is_deezer else itunes_id
|
||||
secondary_id = itunes_id if is_deezer else deezer_id
|
||||
for fid in [primary_id, secondary_id]:
|
||||
if not fid:
|
||||
continue
|
||||
try:
|
||||
api_tracks = context.itunes_client.get_album_tracks(fid)
|
||||
if api_tracks and 'items' in api_tracks:
|
||||
break
|
||||
except Exception as e:
|
||||
logger.debug("Error getting fallback album tracks for %s: %s", fid, e)
|
||||
|
||||
if not api_tracks or 'items' not in api_tracks:
|
||||
items = self._extract_track_items(api_tracks)
|
||||
if not items:
|
||||
return []
|
||||
|
||||
# Both Spotify, iTunes, and Deezer return the same format:
|
||||
# All supported provider responses expose the same core fields once normalized.
|
||||
# items[].track_number, items[].name, items[].disc_number, items[].id, items[].artists
|
||||
missing_tracks = []
|
||||
for item in api_tracks['items']:
|
||||
for item in items:
|
||||
tn = item.get('track_number')
|
||||
if tn and tn not in owned_numbers:
|
||||
track_artists = []
|
||||
|
|
@ -317,6 +313,9 @@ class AlbumCompletenessJob(RepairJob):
|
|||
'track_number': tn,
|
||||
'name': item.get('name', ''),
|
||||
'disc_number': item.get('disc_number', 1),
|
||||
'source': item.get('_source', primary_source),
|
||||
'source_track_id': item.get('id', ''),
|
||||
'track_id': item.get('id', ''),
|
||||
'spotify_track_id': item.get('id', ''),
|
||||
'duration_ms': item.get('duration_ms', 0),
|
||||
'artists': track_artists,
|
||||
|
|
@ -331,6 +330,35 @@ class AlbumCompletenessJob(RepairJob):
|
|||
merged.update(cfg)
|
||||
return merged
|
||||
|
||||
def _get_primary_source(self) -> str:
|
||||
"""Return the active metadata source used for source prioritization."""
|
||||
try:
|
||||
return get_primary_source()
|
||||
except Exception:
|
||||
return 'deezer'
|
||||
|
||||
def _get_album_id_for_source(self, source: str, album_ids: dict) -> str:
|
||||
return album_ids.get(source, '')
|
||||
|
||||
def _get_album_tracks(self, source: str, album_id: str):
|
||||
"""Fetch album tracks from a specific source."""
|
||||
try:
|
||||
return get_album_tracks_for_source(source, album_id)
|
||||
except Exception as e:
|
||||
logger.debug("Error getting %s album tracks for %s: %s", source.capitalize(), album_id, e)
|
||||
return None
|
||||
|
||||
def _extract_track_items(self, api_tracks):
|
||||
"""Normalize album track responses to a list of item dicts."""
|
||||
if not api_tracks:
|
||||
return []
|
||||
if isinstance(api_tracks, dict):
|
||||
items = api_tracks.get('items') or []
|
||||
return items if items else []
|
||||
if isinstance(api_tracks, list):
|
||||
return api_tracks
|
||||
return []
|
||||
|
||||
def estimate_scope(self, context: JobContext) -> int:
|
||||
conn = None
|
||||
try:
|
||||
|
|
@ -346,6 +374,10 @@ class AlbumCompletenessJob(RepairJob):
|
|||
where_parts.append("(itunes_album_id IS NOT NULL AND itunes_album_id != '')")
|
||||
if 'deezer_id' in columns:
|
||||
where_parts.append("(deezer_id IS NOT NULL AND deezer_id != '')")
|
||||
if 'discogs_id' in columns:
|
||||
where_parts.append("(discogs_id IS NOT NULL AND discogs_id != '')")
|
||||
if 'soul_id' in columns:
|
||||
where_parts.append("(soul_id IS NOT NULL AND soul_id != '')")
|
||||
|
||||
cursor.execute(f"""
|
||||
SELECT COUNT(*) FROM albums
|
||||
|
|
|
|||
|
|
@ -18,6 +18,11 @@ from difflib import SequenceMatcher
|
|||
from datetime import datetime, timedelta
|
||||
from typing import Any, Dict, List, Optional, Tuple
|
||||
|
||||
from core.metadata_service import (
|
||||
get_album_tracks_for_source,
|
||||
get_source_priority,
|
||||
get_primary_source,
|
||||
)
|
||||
from core.repair_jobs import get_all_jobs
|
||||
from core.repair_jobs.base import JobContext, JobResult, RepairJob
|
||||
from utils.logging_config import get_logger
|
||||
|
|
@ -104,7 +109,6 @@ class RepairWorker:
|
|||
self._on_job_finish = None # (job_id, status, result) -> None
|
||||
|
||||
# Lazy client accessors
|
||||
self._spotify_client = None
|
||||
self._itunes_client = None
|
||||
self._mb_client = None
|
||||
self._acoustid_client = None
|
||||
|
|
@ -150,13 +154,12 @@ class RepairWorker:
|
|||
# ------------------------------------------------------------------
|
||||
@property
|
||||
def spotify_client(self):
|
||||
if self._spotify_client is None:
|
||||
try:
|
||||
from core.spotify_client import SpotifyClient
|
||||
self._spotify_client = SpotifyClient()
|
||||
except Exception as e:
|
||||
logger.error("Failed to initialize SpotifyClient: %s", e)
|
||||
return self._spotify_client
|
||||
try:
|
||||
from core.metadata_service import get_client_for_source
|
||||
return get_client_for_source('spotify')
|
||||
except Exception as e:
|
||||
logger.error("Failed to resolve shared Spotify client: %s", e)
|
||||
return None
|
||||
|
||||
@property
|
||||
def itunes_client(self):
|
||||
|
|
@ -1743,7 +1746,9 @@ class RepairWorker:
|
|||
track_number = mt.get('track_number', 0)
|
||||
disc_number = mt.get('disc_number', 1)
|
||||
track_artists = mt.get('artists', [])
|
||||
spotify_track_id = mt.get('spotify_track_id', '')
|
||||
source = mt.get('source', '') or 'spotify'
|
||||
source_track_id = mt.get('source_track_id', '') or mt.get('track_id', '') or mt.get('spotify_track_id', '')
|
||||
spotify_track_id = mt.get('spotify_track_id', '') or (source_track_id if source == 'spotify' else '')
|
||||
artist_search = track_artists[0] if track_artists else artist_name
|
||||
|
||||
if not track_name:
|
||||
|
|
@ -1809,7 +1814,7 @@ class RepairWorker:
|
|||
logger.warning("File operation failed for '%s': %s", track_name, result.get('error'))
|
||||
|
||||
# Phase 4: Wishlist fallback
|
||||
if spotify_track_id:
|
||||
if source_track_id:
|
||||
try:
|
||||
# Build album images from finding thumb URL
|
||||
album_images = []
|
||||
|
|
@ -1818,7 +1823,7 @@ class RepairWorker:
|
|||
album_images = [{'url': album_thumb, 'height': 300, 'width': 300}]
|
||||
|
||||
wishlist_data = {
|
||||
'id': spotify_track_id,
|
||||
'id': source_track_id,
|
||||
'name': track_name,
|
||||
'artists': [{'name': a} for a in track_artists] if track_artists else [{'name': artist_name}],
|
||||
'album': {
|
||||
|
|
@ -1832,6 +1837,7 @@ class RepairWorker:
|
|||
'duration_ms': mt.get('duration_ms', 0),
|
||||
'track_number': track_number,
|
||||
'disc_number': disc_number,
|
||||
'uri': f"{source}:track:{source_track_id}" if source and source_track_id else '',
|
||||
}
|
||||
source_info = {
|
||||
'album_title': album_title,
|
||||
|
|
@ -1839,6 +1845,8 @@ class RepairWorker:
|
|||
'track_number': track_number,
|
||||
'disc_number': disc_number,
|
||||
'spotify_album_id': spotify_album_id,
|
||||
'source': source,
|
||||
'source_track_id': source_track_id,
|
||||
'is_album': True,
|
||||
'reason': 'album_completeness_auto_fill',
|
||||
}
|
||||
|
|
@ -1860,7 +1868,7 @@ class RepairWorker:
|
|||
track_details.append({'track': track_name, 'status': 'skipped', 'reason': f'wishlist error: {e}'})
|
||||
else:
|
||||
skipped_count += 1
|
||||
track_details.append({'track': track_name, 'status': 'skipped', 'reason': 'no spotify_track_id for wishlist'})
|
||||
track_details.append({'track': track_name, 'status': 'skipped', 'reason': 'no source_track_id for wishlist'})
|
||||
|
||||
# Build result message
|
||||
parts = []
|
||||
|
|
@ -1885,11 +1893,18 @@ class RepairWorker:
|
|||
|
||||
def _refetch_missing_tracks(self, album_id, details):
|
||||
"""Re-fetch missing track list from APIs when the stored list is empty."""
|
||||
configured_primary_source = get_primary_source()
|
||||
spotify_album_id = details.get('spotify_album_id', '')
|
||||
itunes_album_id = details.get('itunes_album_id', '')
|
||||
deezer_album_id = details.get('deezer_album_id', '')
|
||||
logger.debug("Refetch missing tracks for album %s: spotify=%s, itunes=%s, deezer=%s",
|
||||
album_id, spotify_album_id, itunes_album_id, deezer_album_id)
|
||||
discogs_album_id = details.get('discogs_album_id', '')
|
||||
hydrabase_album_id = details.get('hydrabase_album_id', '')
|
||||
primary_source = details.get('primary_source') or configured_primary_source
|
||||
logger.debug(
|
||||
"Refetch missing tracks for album %s: primary=%s spotify=%s itunes=%s deezer=%s discogs=%s hydrabase=%s",
|
||||
album_id, primary_source, spotify_album_id, itunes_album_id, deezer_album_id, discogs_album_id,
|
||||
hydrabase_album_id
|
||||
)
|
||||
|
||||
# Get track numbers we already own
|
||||
owned_numbers = set()
|
||||
|
|
@ -1909,32 +1924,27 @@ class RepairWorker:
|
|||
if conn:
|
||||
conn.close()
|
||||
|
||||
# Try Spotify first
|
||||
current_source = primary_source
|
||||
api_tracks = None
|
||||
if spotify_album_id:
|
||||
try:
|
||||
sp = self.spotify_client
|
||||
if sp and hasattr(sp, 'get_album_tracks'):
|
||||
api_tracks = sp.get_album_tracks(spotify_album_id)
|
||||
except Exception as e:
|
||||
logger.debug("Refetch: Spotify album tracks failed for %s: %s", spotify_album_id, e)
|
||||
album_sources = {
|
||||
'spotify': spotify_album_id,
|
||||
'itunes': itunes_album_id,
|
||||
'deezer': deezer_album_id,
|
||||
'discogs': discogs_album_id,
|
||||
'hydrabase': hydrabase_album_id,
|
||||
}
|
||||
|
||||
# Try fallback client (iTunes/Deezer)
|
||||
if not api_tracks or 'items' not in (api_tracks or {}):
|
||||
itunes = self.itunes_client
|
||||
if itunes:
|
||||
is_deezer = type(itunes).__name__ == 'DeezerClient'
|
||||
primary_id = deezer_album_id if is_deezer else itunes_album_id
|
||||
secondary_id = itunes_album_id if is_deezer else deezer_album_id
|
||||
for fid in [primary_id, secondary_id]:
|
||||
if not fid:
|
||||
continue
|
||||
try:
|
||||
api_tracks = itunes.get_album_tracks(fid)
|
||||
if api_tracks and 'items' in api_tracks:
|
||||
break
|
||||
except Exception as e:
|
||||
logger.debug("Refetch: fallback album tracks failed for %s: %s", fid, e)
|
||||
for source in get_source_priority(primary_source):
|
||||
fid = album_sources.get(source, '')
|
||||
if not fid:
|
||||
continue
|
||||
try:
|
||||
api_tracks = get_album_tracks_for_source(source, fid)
|
||||
if api_tracks and 'items' in (api_tracks or {}):
|
||||
current_source = source
|
||||
break
|
||||
except Exception as e:
|
||||
logger.debug("Refetch: %s album tracks failed for %s: %s", source, fid, e)
|
||||
|
||||
if not api_tracks or 'items' not in api_tracks:
|
||||
return []
|
||||
|
|
@ -1953,6 +1963,9 @@ class RepairWorker:
|
|||
'track_number': tn,
|
||||
'name': item.get('name', ''),
|
||||
'disc_number': item.get('disc_number', 1),
|
||||
'source': current_source or 'spotify',
|
||||
'source_track_id': item.get('id', ''),
|
||||
'track_id': item.get('id', ''),
|
||||
'spotify_track_id': item.get('id', ''),
|
||||
'duration_ms': item.get('duration_ms', 0),
|
||||
'artists': track_artists,
|
||||
|
|
|
|||
|
|
@ -6426,10 +6426,10 @@ class MusicDatabase:
|
|||
with self._get_connection() as conn:
|
||||
cursor = conn.cursor()
|
||||
|
||||
# Use Spotify track ID as unique identifier
|
||||
# Use track ID as unique identifier. Field name stays legacy-compatible.
|
||||
track_id = spotify_track_data.get('id')
|
||||
if not track_id:
|
||||
logger.error("Cannot add track to wishlist: missing Spotify track ID")
|
||||
logger.error("Cannot add track to wishlist: missing track ID")
|
||||
return False
|
||||
|
||||
track_name = spotify_track_data.get('name', 'Unknown Track')
|
||||
|
|
|
|||
305
tests/test_album_completeness_job.py
Normal file
305
tests/test_album_completeness_job.py
Normal file
|
|
@ -0,0 +1,305 @@
|
|||
import sys
|
||||
import types
|
||||
|
||||
|
||||
if "spotipy" not in sys.modules:
|
||||
spotipy = types.ModuleType("spotipy")
|
||||
|
||||
class _DummySpotify:
|
||||
def __init__(self, *args, **kwargs):
|
||||
pass
|
||||
|
||||
oauth2 = types.ModuleType("spotipy.oauth2")
|
||||
|
||||
class _DummyOAuth:
|
||||
def __init__(self, *args, **kwargs):
|
||||
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_pkg = 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_pkg.settings = settings_mod
|
||||
sys.modules["config"] = config_pkg
|
||||
sys.modules["config.settings"] = settings_mod
|
||||
|
||||
from core.repair_jobs.album_completeness import AlbumCompletenessJob
|
||||
import core.repair_jobs.album_completeness as album_completeness_module
|
||||
|
||||
|
||||
class _FakeCursor:
|
||||
def __init__(self, owned_track_numbers):
|
||||
self._owned_track_numbers = owned_track_numbers
|
||||
self._last_query = ""
|
||||
|
||||
def execute(self, query, params=None):
|
||||
self._last_query = query
|
||||
return self
|
||||
|
||||
def fetchall(self):
|
||||
if "SELECT track_number" in self._last_query:
|
||||
return [(track_number,) for track_number in self._owned_track_numbers]
|
||||
return []
|
||||
|
||||
|
||||
class _FakeConnection:
|
||||
def __init__(self, owned_track_numbers):
|
||||
self._cursor = _FakeCursor(owned_track_numbers)
|
||||
|
||||
def cursor(self):
|
||||
return self._cursor
|
||||
|
||||
def close(self):
|
||||
return None
|
||||
|
||||
|
||||
class _FakeDB:
|
||||
def __init__(self, owned_track_numbers):
|
||||
self._owned_track_numbers = owned_track_numbers
|
||||
|
||||
def _get_connection(self):
|
||||
return _FakeConnection(self._owned_track_numbers)
|
||||
|
||||
|
||||
class _FakeSpotifyClient:
|
||||
def __init__(self, track_count=5):
|
||||
self.track_count = track_count
|
||||
self.calls = []
|
||||
|
||||
def is_spotify_authenticated(self):
|
||||
return True
|
||||
|
||||
def get_album_tracks(self, album_id):
|
||||
self.calls.append(album_id)
|
||||
return {
|
||||
"items": [
|
||||
{"id": f"sp-{i}", "name": f"Spotify Track {i}", "track_number": i, "disc_number": 1, "artists": []}
|
||||
for i in range(1, self.track_count + 1)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
class _FakeDeezerClient:
|
||||
def __init__(self, track_count=2):
|
||||
self.track_count = track_count
|
||||
self.calls = []
|
||||
|
||||
def is_authenticated(self):
|
||||
return True
|
||||
|
||||
def get_album_tracks(self, album_id):
|
||||
self.calls.append(album_id)
|
||||
return {
|
||||
"items": [
|
||||
{"id": f"dz-{i}", "name": f"Deezer Track {i}", "track_number": i, "disc_number": 1, "artists": []}
|
||||
for i in range(1, self.track_count + 1)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
class _FakeITunesClient:
|
||||
def __init__(self):
|
||||
self.calls = []
|
||||
|
||||
def is_authenticated(self):
|
||||
return True
|
||||
|
||||
def get_album_tracks(self, album_id):
|
||||
self.calls.append(album_id)
|
||||
return {"items": []}
|
||||
|
||||
|
||||
class _FakeDiscogsClient:
|
||||
def __init__(self, track_count=3):
|
||||
self.track_count = track_count
|
||||
self.calls = []
|
||||
|
||||
def get_album_tracks(self, album_id):
|
||||
self.calls.append(album_id)
|
||||
return {
|
||||
"items": [
|
||||
{"id": f"dg-{i}", "name": f"Discogs Track {i}", "track_number": i, "disc_number": 1, "artists": []}
|
||||
for i in range(1, self.track_count + 1)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
class _FakeHydrabaseClient:
|
||||
def __init__(self, track_count=4):
|
||||
self.track_count = track_count
|
||||
self.calls = []
|
||||
|
||||
def is_connected(self):
|
||||
return True
|
||||
|
||||
def get_album_tracks_dict(self, album_id):
|
||||
self.calls.append(album_id)
|
||||
return {
|
||||
"items": [
|
||||
{"id": f"hy-{i}", "name": f"Hydrabase Track {i}", "track_number": i, "disc_number": 1, "artists": []}
|
||||
for i in range(1, self.track_count + 1)
|
||||
]
|
||||
}
|
||||
|
||||
|
||||
def test_album_completeness_uses_primary_provider_first(monkeypatch):
|
||||
job = AlbumCompletenessJob()
|
||||
context = types.SimpleNamespace(
|
||||
db=_FakeDB(owned_track_numbers={1}),
|
||||
spotify_client=_FakeSpotifyClient(track_count=5),
|
||||
is_spotify_rate_limited=lambda: False,
|
||||
)
|
||||
|
||||
deezer_client = _FakeDeezerClient(track_count=2)
|
||||
itunes_client = _FakeITunesClient()
|
||||
calls = []
|
||||
album_ids = {
|
||||
"spotify": "spotify-album",
|
||||
"itunes": "itunes-album",
|
||||
"deezer": "deezer-album",
|
||||
"discogs": "",
|
||||
"hydrabase": "",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(album_completeness_module, "get_primary_source", lambda: "deezer")
|
||||
monkeypatch.setattr(
|
||||
album_completeness_module,
|
||||
"get_album_tracks_for_source",
|
||||
lambda source, album_id: (
|
||||
calls.append((source, album_id)) or
|
||||
(
|
||||
deezer_client.get_album_tracks(album_id) if source == "deezer" else
|
||||
itunes_client.get_album_tracks(album_id) if source == "itunes" else
|
||||
context.spotify_client.get_album_tracks(album_id) if source == "spotify" else
|
||||
{"items": []}
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
expected_total = job._get_expected_total(context, "deezer", album_ids)
|
||||
missing_tracks = job._find_missing_tracks(context, "deezer", 42, album_ids)
|
||||
|
||||
assert expected_total == 2
|
||||
assert calls == [("deezer", "deezer-album"), ("deezer", "deezer-album")]
|
||||
assert deezer_client.calls == ["deezer-album", "deezer-album"]
|
||||
assert context.spotify_client.calls == []
|
||||
assert itunes_client.calls == []
|
||||
|
||||
assert [track["track_number"] for track in missing_tracks] == [2]
|
||||
assert missing_tracks[0]["source"] == "deezer"
|
||||
assert missing_tracks[0]["source_track_id"] == "dz-2"
|
||||
assert missing_tracks[0]["spotify_track_id"] == "dz-2"
|
||||
|
||||
|
||||
def test_album_completeness_supports_discogs_primary(monkeypatch):
|
||||
job = AlbumCompletenessJob()
|
||||
context = types.SimpleNamespace(
|
||||
db=_FakeDB(owned_track_numbers={1}),
|
||||
spotify_client=_FakeSpotifyClient(track_count=5),
|
||||
is_spotify_rate_limited=lambda: False,
|
||||
)
|
||||
|
||||
discogs_client = _FakeDiscogsClient(track_count=3)
|
||||
itunes_client = _FakeITunesClient()
|
||||
deezer_client = _FakeDeezerClient(track_count=2)
|
||||
calls = []
|
||||
album_ids = {
|
||||
"spotify": "spotify-album",
|
||||
"itunes": "itunes-album",
|
||||
"deezer": "deezer-album",
|
||||
"discogs": "discogs-release",
|
||||
"hydrabase": "",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(album_completeness_module, "get_primary_source", lambda: "discogs")
|
||||
monkeypatch.setattr(
|
||||
album_completeness_module,
|
||||
"get_album_tracks_for_source",
|
||||
lambda source, album_id: (
|
||||
calls.append((source, album_id)) or
|
||||
(
|
||||
discogs_client.get_album_tracks(album_id) if source == "discogs" else
|
||||
itunes_client.get_album_tracks(album_id) if source == "itunes" else
|
||||
deezer_client.get_album_tracks(album_id) if source == "deezer" else
|
||||
context.spotify_client.get_album_tracks(album_id) if source == "spotify" else
|
||||
{"items": []}
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
expected_total = job._get_expected_total(context, "discogs", album_ids)
|
||||
missing_tracks = job._find_missing_tracks(context, "discogs", 42, album_ids)
|
||||
|
||||
assert expected_total == 3
|
||||
assert calls == [("discogs", "discogs-release"), ("discogs", "discogs-release")]
|
||||
assert discogs_client.calls == ["discogs-release", "discogs-release"]
|
||||
assert context.spotify_client.calls == []
|
||||
assert itunes_client.calls == []
|
||||
assert deezer_client.calls == []
|
||||
|
||||
assert [track["track_number"] for track in missing_tracks] == [2, 3]
|
||||
assert missing_tracks[0]["source"] == "discogs"
|
||||
assert missing_tracks[0]["source_track_id"] == "dg-2"
|
||||
|
||||
|
||||
def test_album_completeness_supports_hydrabase_primary(monkeypatch):
|
||||
job = AlbumCompletenessJob()
|
||||
context = types.SimpleNamespace(
|
||||
db=_FakeDB(owned_track_numbers={1}),
|
||||
spotify_client=_FakeSpotifyClient(track_count=5),
|
||||
is_spotify_rate_limited=lambda: False,
|
||||
)
|
||||
|
||||
hydrabase_client = _FakeHydrabaseClient(track_count=4)
|
||||
itunes_client = _FakeITunesClient()
|
||||
deezer_client = _FakeDeezerClient(track_count=2)
|
||||
calls = []
|
||||
album_ids = {
|
||||
"spotify": "spotify-album",
|
||||
"itunes": "itunes-album",
|
||||
"deezer": "deezer-album",
|
||||
"discogs": "",
|
||||
"hydrabase": "soul-album",
|
||||
}
|
||||
|
||||
monkeypatch.setattr(album_completeness_module, "get_primary_source", lambda: "hydrabase")
|
||||
monkeypatch.setattr(
|
||||
album_completeness_module,
|
||||
"get_album_tracks_for_source",
|
||||
lambda source, album_id: (
|
||||
calls.append((source, album_id)) or
|
||||
(
|
||||
hydrabase_client.get_album_tracks_dict(album_id) if source == "hydrabase" else
|
||||
itunes_client.get_album_tracks(album_id) if source == "itunes" else
|
||||
deezer_client.get_album_tracks(album_id) if source == "deezer" else
|
||||
context.spotify_client.get_album_tracks(album_id) if source == "spotify" else
|
||||
{"items": []}
|
||||
)
|
||||
)
|
||||
)
|
||||
|
||||
expected_total = job._get_expected_total(context, "hydrabase", album_ids)
|
||||
missing_tracks = job._find_missing_tracks(context, "hydrabase", 42, album_ids)
|
||||
|
||||
assert expected_total == 4
|
||||
assert calls == [("hydrabase", "soul-album"), ("hydrabase", "soul-album")]
|
||||
assert hydrabase_client.calls == ["soul-album", "soul-album"]
|
||||
assert context.spotify_client.calls == []
|
||||
assert itunes_client.calls == []
|
||||
assert deezer_client.calls == []
|
||||
|
||||
assert [track["track_number"] for track in missing_tracks] == [2, 3, 4]
|
||||
assert missing_tracks[0]["source"] == "hydrabase"
|
||||
assert missing_tracks[0]["source_track_id"] == "hy-2"
|
||||
|
|
@ -64051,7 +64051,15 @@ function _renderFindingDetail(f) {
|
|||
case 'incomplete_album':
|
||||
if (d.artist) rows.push(['Artist', d.artist]);
|
||||
if (d.album_title) rows.push(['Album', d.album_title]);
|
||||
if (d.spotify_album_id) rows.push(['Spotify ID', d.spotify_album_id]);
|
||||
if (d.primary_source && d.primary_album_id) {
|
||||
const primaryLabel = d.primary_source.charAt(0).toUpperCase() + d.primary_source.slice(1);
|
||||
rows.push([`${primaryLabel} ID`, d.primary_album_id]);
|
||||
if (d.spotify_album_id && d.primary_source !== 'spotify') {
|
||||
rows.push(['Spotify ID', d.spotify_album_id]);
|
||||
}
|
||||
} else if (d.spotify_album_id) {
|
||||
rows.push(['Spotify ID', d.spotify_album_id]);
|
||||
}
|
||||
let incHtml = media + _gridRows(rows);
|
||||
const actual = d.actual_tracks || 0, expected = d.expected_tracks || 0;
|
||||
if (expected > 0) {
|
||||
|
|
@ -64065,6 +64073,7 @@ function _renderFindingDetail(f) {
|
|||
incHtml += `<div class="repair-detail-sublist">${d.missing_tracks.map(t => `
|
||||
<div class="repair-detail-subitem">
|
||||
<strong>#${t.track_number || '?'} ${_escFinding(t.name || t.title || 'Unknown')}</strong>
|
||||
${t.source && t.source !== 'spotify' ? `<span>Source: ${_escFinding(t.source)}${t.source_track_id ? ` · ID: ${_escFinding(t.source_track_id)}` : ''}</span>` : ''}
|
||||
${t.duration_ms ? `<span>Duration: ${Math.round(t.duration_ms / 1000)}s</span>` : ''}
|
||||
</div>`).join('')}</div>`;
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue