Merge pull request #318 from kettui/feat/artist-discography-refactoring
Centralize metadata lookup for artist discography
This commit is contained in:
commit
c473bf777c
4 changed files with 645 additions and 257 deletions
|
|
@ -3,11 +3,12 @@ Metadata Service - Centralized metadata source selection
|
|||
|
||||
ALL metadata source decisions flow through this module. Other files import
|
||||
get_primary_source() and get_primary_client() instead of reimplementing
|
||||
the logic. This prevents bugs where different files have different defaults
|
||||
or auth checks.
|
||||
the logic. This prevents bugs where different files have different defaults,
|
||||
auth checks, or source-fallback behavior.
|
||||
"""
|
||||
|
||||
import threading
|
||||
from dataclasses import dataclass
|
||||
from typing import List, Optional, Dict, Any, Literal
|
||||
from core.spotify_client import SpotifyClient
|
||||
from core.itunes_client import iTunesClient
|
||||
|
|
@ -24,6 +25,17 @@ _client_cache_lock = threading.RLock()
|
|||
_client_cache: Dict[str, Any] = {}
|
||||
|
||||
|
||||
@dataclass(frozen=True)
|
||||
class MetadataLookupOptions:
|
||||
"""Generic metadata lookup policy shared by metadata services."""
|
||||
|
||||
source_override: Optional[str] = None
|
||||
allow_fallback: bool = True
|
||||
skip_cache: bool = False
|
||||
max_pages: int = 0
|
||||
limit: int = 50
|
||||
|
||||
|
||||
# =============================================================================
|
||||
# CANONICAL SOURCE SELECTION — all code should use these two functions
|
||||
# =============================================================================
|
||||
|
|
@ -135,6 +147,253 @@ def get_album_tracks_for_source(source: str, album_id: str):
|
|||
return None
|
||||
|
||||
|
||||
def get_artist_albums_for_source(
|
||||
source: str,
|
||||
artist_id: str,
|
||||
artist_name: str = '',
|
||||
album_type: str = 'album,single',
|
||||
limit: int = 50,
|
||||
skip_cache: bool = False,
|
||||
max_pages: int = 0,
|
||||
):
|
||||
"""Get artist albums for an exact source.
|
||||
|
||||
Returns a provider-native album list or None if the source is unavailable.
|
||||
Tries the requested artist ID first, then falls back to artist-name
|
||||
search using the same flow for every provider when artist_name is provided.
|
||||
|
||||
Set skip_cache=True only for freshness-sensitive flows that need newly
|
||||
released albums to show up immediately.
|
||||
"""
|
||||
client = get_client_for_source(source)
|
||||
if not client or not artist_id or not hasattr(client, 'get_artist_albums'):
|
||||
return None
|
||||
|
||||
def _fetch_for_artist(target_artist_id: str):
|
||||
kwargs = {
|
||||
'album_type': album_type,
|
||||
'limit': limit,
|
||||
}
|
||||
if source == 'spotify':
|
||||
kwargs['allow_fallback'] = False
|
||||
kwargs['skip_cache'] = skip_cache
|
||||
kwargs['max_pages'] = max_pages
|
||||
return client.get_artist_albums(target_artist_id, **kwargs)
|
||||
|
||||
try:
|
||||
albums = _fetch_for_artist(artist_id) or []
|
||||
if albums:
|
||||
return albums
|
||||
|
||||
if not artist_name:
|
||||
return albums
|
||||
|
||||
search_results = _search_artists_for_source(source, client, artist_name, limit=5)
|
||||
if not search_results:
|
||||
return albums
|
||||
|
||||
best = _pick_best_artist_match(search_results, artist_name)
|
||||
if not best:
|
||||
return albums
|
||||
|
||||
found_artist_id = _extract_lookup_value(best, 'id', 'artist_id')
|
||||
if not found_artist_id:
|
||||
return albums
|
||||
|
||||
resolved = _fetch_for_artist(found_artist_id) or []
|
||||
if resolved:
|
||||
logger.debug("Found %s artist '%s' (id=%s)", source, _extract_lookup_value(best, 'name', 'artist_name', 'title'), found_artist_id)
|
||||
return resolved
|
||||
except Exception:
|
||||
return None
|
||||
|
||||
|
||||
def _get_source_chain_for_lookup(options: MetadataLookupOptions) -> List[str]:
|
||||
primary_source = get_primary_source()
|
||||
source_chain = list(get_source_priority(primary_source))
|
||||
override = (options.source_override or '').strip().lower()
|
||||
|
||||
if override:
|
||||
source_chain = [override] + [source for source in source_chain if source != override]
|
||||
|
||||
if not options.allow_fallback:
|
||||
source_chain = source_chain[:1]
|
||||
|
||||
return source_chain
|
||||
|
||||
|
||||
def _extract_lookup_value(value: Any, *names: str, default: Any = None) -> Any:
|
||||
if value is None:
|
||||
return default
|
||||
|
||||
for name in names:
|
||||
if isinstance(value, dict):
|
||||
if name in value and value[name] is not None:
|
||||
return value[name]
|
||||
else:
|
||||
candidate = getattr(value, name, None)
|
||||
if candidate is not None:
|
||||
return candidate
|
||||
return default
|
||||
|
||||
|
||||
def _normalize_artist_name(value: Any) -> str:
|
||||
return (value or '').strip().casefold()
|
||||
|
||||
|
||||
def _search_artists_for_source(source: str, client: Any, artist_name: str, limit: int = 5) -> List[Any]:
|
||||
if not client or not hasattr(client, 'search_artists'):
|
||||
return []
|
||||
|
||||
try:
|
||||
kwargs = {'limit': limit}
|
||||
if source == 'spotify':
|
||||
kwargs['allow_fallback'] = False
|
||||
return client.search_artists(artist_name, **kwargs) or []
|
||||
except Exception as exc:
|
||||
logger.debug("Could not search %s for %s: %s", source, artist_name, exc)
|
||||
return []
|
||||
|
||||
|
||||
def _pick_best_artist_match(search_results: List[Any], artist_name: str) -> Optional[Any]:
|
||||
"""Prefer an exact artist-name match, otherwise use the first result."""
|
||||
if not search_results:
|
||||
return None
|
||||
|
||||
target_name = _normalize_artist_name(artist_name)
|
||||
for artist in search_results:
|
||||
candidate_name = _normalize_artist_name(
|
||||
_extract_lookup_value(artist, 'name', 'artist_name', 'title')
|
||||
)
|
||||
if candidate_name == target_name:
|
||||
return artist
|
||||
|
||||
return search_results[0]
|
||||
|
||||
|
||||
def _build_discography_release_dict(release: Any, artist_id: str) -> Optional[Dict[str, Any]]:
|
||||
release_id = _extract_lookup_value(release, 'id', 'album_id', 'release_id')
|
||||
if not release_id:
|
||||
return None
|
||||
|
||||
artist_ids = _extract_lookup_value(release, 'artist_ids') or []
|
||||
if isinstance(artist_ids, (str, bytes)):
|
||||
artist_ids = [artist_ids]
|
||||
else:
|
||||
try:
|
||||
artist_ids = list(artist_ids)
|
||||
except TypeError:
|
||||
artist_ids = [artist_ids]
|
||||
|
||||
if artist_ids and str(artist_ids[0]) != str(artist_id):
|
||||
return None
|
||||
|
||||
album_type = _extract_lookup_value(release, 'album_type', default='album') or 'album'
|
||||
release_date = _extract_lookup_value(release, 'release_date')
|
||||
|
||||
return {
|
||||
'id': release_id,
|
||||
'name': _extract_lookup_value(release, 'name', 'title', default=release_id),
|
||||
'release_date': release_date,
|
||||
'album_type': album_type,
|
||||
'image_url': _extract_lookup_value(release, 'image_url', 'thumb_url', 'cover_image'),
|
||||
'total_tracks': _extract_lookup_value(release, 'total_tracks', default=0) or 0,
|
||||
'external_urls': _extract_lookup_value(release, 'external_urls', default={}) or {},
|
||||
}
|
||||
|
||||
|
||||
def _sort_discography_releases(releases: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
|
||||
def get_release_year(item):
|
||||
if item.get('release_date'):
|
||||
try:
|
||||
return int(str(item['release_date'])[:4])
|
||||
except (ValueError, IndexError, TypeError):
|
||||
return 0
|
||||
return 0
|
||||
|
||||
return sorted(releases, key=get_release_year, reverse=True)
|
||||
|
||||
|
||||
def get_artist_discography(
|
||||
artist_id: str,
|
||||
artist_name: str = '',
|
||||
options: Optional[MetadataLookupOptions] = None,
|
||||
) -> Dict[str, Any]:
|
||||
"""Get a normalized artist discography with source resolution and fallback.
|
||||
|
||||
Each provider uses the same lookup flow:
|
||||
1. try the requested artist ID
|
||||
2. if that misses, search by artist name
|
||||
3. retry with the provider-specific artist ID from the search result
|
||||
"""
|
||||
options = options or MetadataLookupOptions()
|
||||
source_priority = _get_source_chain_for_lookup(options)
|
||||
|
||||
albums: List[Any] = []
|
||||
active_source: Optional[str] = None
|
||||
|
||||
if not albums:
|
||||
for source in source_priority:
|
||||
client = get_client_for_source(source)
|
||||
if not client:
|
||||
continue
|
||||
|
||||
try:
|
||||
albums = get_artist_albums_for_source(
|
||||
source,
|
||||
artist_id,
|
||||
artist_name=artist_name,
|
||||
limit=options.limit,
|
||||
skip_cache=options.skip_cache,
|
||||
max_pages=options.max_pages,
|
||||
) or []
|
||||
except Exception as exc:
|
||||
logger.debug("%s direct lookup failed for artist %s: %s", source, artist_id, exc)
|
||||
albums = []
|
||||
|
||||
if albums:
|
||||
active_source = source
|
||||
logger.info("Got %s albums from %s for artist %s", len(albums), source, artist_id)
|
||||
break
|
||||
|
||||
album_list: List[Dict[str, Any]] = []
|
||||
singles_list: List[Dict[str, Any]] = []
|
||||
seen_albums = set()
|
||||
|
||||
for release in albums or []:
|
||||
release_data = _build_discography_release_dict(release, artist_id)
|
||||
if not release_data:
|
||||
continue
|
||||
|
||||
release_id = release_data['id']
|
||||
if release_id in seen_albums:
|
||||
continue
|
||||
seen_albums.add(release_id)
|
||||
|
||||
album_type = release_data.get('album_type') or 'album'
|
||||
if album_type in ['single', 'ep']:
|
||||
singles_list.append(release_data)
|
||||
else:
|
||||
album_list.append(release_data)
|
||||
|
||||
album_list = _sort_discography_releases(album_list)
|
||||
singles_list = _sort_discography_releases(singles_list)
|
||||
|
||||
logger.debug(
|
||||
"Total albums returned for artist %s: %s (source=%s)",
|
||||
artist_id,
|
||||
len(album_list) + len(singles_list),
|
||||
active_source,
|
||||
)
|
||||
|
||||
return {
|
||||
'albums': album_list,
|
||||
'singles': singles_list,
|
||||
'source': active_source or (source_priority[0] if source_priority else 'unknown'),
|
||||
'source_priority': source_priority,
|
||||
}
|
||||
|
||||
|
||||
def get_deezer_client():
|
||||
"""Get cached Deezer client.
|
||||
|
||||
|
|
@ -192,18 +451,32 @@ def get_discogs_client(token: Optional[str] = None):
|
|||
return client
|
||||
|
||||
|
||||
def get_hydrabase_client(allow_fallback: bool = True):
|
||||
"""Return current Hydrabase client if connected.
|
||||
def is_hydrabase_enabled() -> bool:
|
||||
"""Return True when Hydrabase is connected and allowed for metadata use."""
|
||||
try:
|
||||
import importlib
|
||||
ws = importlib.import_module('web_server')
|
||||
client = getattr(ws, 'hydrabase_client', None)
|
||||
if not client or not client.is_connected():
|
||||
return False
|
||||
return bool(getattr(ws, 'dev_mode_enabled', False))
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
|
||||
def get_hydrabase_client(allow_fallback: bool = True, require_enabled: bool = True):
|
||||
"""Return current Hydrabase client if connected and enabled.
|
||||
|
||||
If allow_fallback is True, return iTunes fallback when Hydrabase is not
|
||||
connected. If False, return None instead.
|
||||
connected or not enabled. If False, return None instead.
|
||||
"""
|
||||
try:
|
||||
import importlib
|
||||
ws = importlib.import_module('web_server')
|
||||
client = getattr(ws, 'hydrabase_client', None)
|
||||
if client and client.is_connected():
|
||||
return client
|
||||
if not require_enabled or bool(getattr(ws, 'dev_mode_enabled', False)):
|
||||
return client
|
||||
except Exception:
|
||||
pass
|
||||
if allow_fallback:
|
||||
|
|
|
|||
|
|
@ -105,3 +105,39 @@ def test_deezer_client_cache_tracks_token(monkeypatch):
|
|||
|
||||
assert first is not second
|
||||
assert calls["deezer"] == 2
|
||||
|
||||
|
||||
class _FakeHydrabaseClient:
|
||||
def __init__(self, connected=True):
|
||||
self._connected = connected
|
||||
|
||||
def is_connected(self):
|
||||
return self._connected
|
||||
|
||||
|
||||
def test_hydrabase_enabled_requires_connection_and_dev_mode(monkeypatch):
|
||||
fake_ws = types.ModuleType("web_server")
|
||||
fake_ws.hydrabase_client = _FakeHydrabaseClient(connected=True)
|
||||
fake_ws.dev_mode_enabled = True
|
||||
monkeypatch.setitem(sys.modules, "web_server", fake_ws)
|
||||
|
||||
assert metadata_service.is_hydrabase_enabled() is True
|
||||
|
||||
fake_ws.dev_mode_enabled = False
|
||||
assert metadata_service.is_hydrabase_enabled() is False
|
||||
|
||||
fake_ws.dev_mode_enabled = True
|
||||
fake_ws.hydrabase_client = _FakeHydrabaseClient(connected=False)
|
||||
assert metadata_service.is_hydrabase_enabled() is False
|
||||
|
||||
|
||||
def test_get_client_for_source_hydrabase_requires_enablement(monkeypatch):
|
||||
fake_ws = types.ModuleType("web_server")
|
||||
fake_ws.hydrabase_client = _FakeHydrabaseClient(connected=True)
|
||||
fake_ws.dev_mode_enabled = False
|
||||
monkeypatch.setitem(sys.modules, "web_server", fake_ws)
|
||||
|
||||
assert metadata_service.get_client_for_source("hydrabase") is None
|
||||
|
||||
fake_ws.dev_mode_enabled = True
|
||||
assert metadata_service.get_client_for_source("hydrabase") is fake_ws.hydrabase_client
|
||||
|
|
|
|||
250
tests/test_metadata_service_discography.py
Normal file
250
tests/test_metadata_service_discography.py
Normal file
|
|
@ -0,0 +1,250 @@
|
|||
import sys
|
||||
import types
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
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 import metadata_service
|
||||
from core.metadata_service import MetadataLookupOptions
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _clear_metadata_client_cache():
|
||||
metadata_service.clear_cached_metadata_clients()
|
||||
yield
|
||||
metadata_service.clear_cached_metadata_clients()
|
||||
|
||||
|
||||
class _FakeSourceClient:
|
||||
def __init__(self, album_results=None, artist_search_results=None, discography_results=None):
|
||||
self.album_results = list(album_results or [])
|
||||
self.artist_search_results = list(artist_search_results or [])
|
||||
self.discography_results = list(discography_results or [])
|
||||
self.album_calls = []
|
||||
self.artist_search_calls = []
|
||||
self.discography_calls = []
|
||||
|
||||
def get_artist_albums(self, artist_id, **kwargs):
|
||||
self.album_calls.append((artist_id, dict(kwargs)))
|
||||
return list(self.album_results)
|
||||
|
||||
def search_artists(self, query, **kwargs):
|
||||
self.artist_search_calls.append((query, dict(kwargs)))
|
||||
return list(self.artist_search_results)
|
||||
|
||||
def search_discography(self, query, **kwargs):
|
||||
self.discography_calls.append((query, dict(kwargs)))
|
||||
return list(self.discography_results)
|
||||
|
||||
|
||||
def _album(album_id, name, release_date, album_type="album"):
|
||||
return types.SimpleNamespace(
|
||||
id=album_id,
|
||||
name=name,
|
||||
release_date=release_date,
|
||||
album_type=album_type,
|
||||
image_url=f"https://img.example/{album_id}.jpg",
|
||||
total_tracks=12,
|
||||
external_urls={"spotify": f"https://example/{album_id}"},
|
||||
artist_ids=["artist-1"],
|
||||
)
|
||||
|
||||
|
||||
def _artist(artist_id, name):
|
||||
return types.SimpleNamespace(id=artist_id, name=name)
|
||||
|
||||
|
||||
def test_get_artist_discography_uses_primary_then_fallback(monkeypatch):
|
||||
deezer = _FakeSourceClient()
|
||||
spotify = _FakeSourceClient(
|
||||
album_results=[
|
||||
_album("album-old", "Older Album", "2022-01-01"),
|
||||
_album("single-one", "Single One", "2024-06-01", album_type="single"),
|
||||
_album("album-new", "New Album", "2024-08-01"),
|
||||
]
|
||||
)
|
||||
itunes = _FakeSourceClient()
|
||||
clients = {
|
||||
"deezer": deezer,
|
||||
"spotify": spotify,
|
||||
"itunes": itunes,
|
||||
}
|
||||
|
||||
monkeypatch.setattr(metadata_service, "get_primary_source", lambda: "deezer")
|
||||
monkeypatch.setattr(metadata_service, "get_source_priority", lambda primary: [primary, "spotify", "itunes"])
|
||||
monkeypatch.setattr(metadata_service, "get_client_for_source", lambda source: clients.get(source))
|
||||
|
||||
result = metadata_service.get_artist_discography("artist-1", "Artist One", MetadataLookupOptions())
|
||||
|
||||
assert result["source"] == "spotify"
|
||||
assert result["source_priority"] == ["deezer", "spotify", "itunes"]
|
||||
assert [album["id"] for album in result["albums"]] == ["album-new", "album-old"]
|
||||
assert [single["id"] for single in result["singles"]] == ["single-one"]
|
||||
assert spotify.album_calls == [(
|
||||
"artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
"allow_fallback": False,
|
||||
"skip_cache": False,
|
||||
"max_pages": 0,
|
||||
},
|
||||
)]
|
||||
|
||||
|
||||
def test_get_artist_discography_uses_name_search_when_direct_lookup_missing(monkeypatch):
|
||||
class _SearchThenAlbumClient(_FakeSourceClient):
|
||||
def get_artist_albums(self, artist_id, **kwargs):
|
||||
self.album_calls.append((artist_id, dict(kwargs)))
|
||||
if artist_id == "deezer-artist-1":
|
||||
return [_album("deezer-album-1", "Deezer Album", "2023-05-01")]
|
||||
return []
|
||||
|
||||
deezer = _SearchThenAlbumClient(
|
||||
artist_search_results=[_artist("deezer-artist-1", "Artist One")],
|
||||
)
|
||||
clients = {"deezer": deezer}
|
||||
|
||||
monkeypatch.setattr(metadata_service, "get_primary_source", lambda: "deezer")
|
||||
monkeypatch.setattr(metadata_service, "get_source_priority", lambda primary: [primary])
|
||||
monkeypatch.setattr(metadata_service, "get_client_for_source", lambda source: clients.get(source))
|
||||
|
||||
result = metadata_service.get_artist_discography("artist-1", "Artist One", MetadataLookupOptions())
|
||||
|
||||
assert result["source"] == "deezer"
|
||||
assert [album["id"] for album in result["albums"]] == ["deezer-album-1"]
|
||||
assert deezer.artist_search_calls == [("Artist One", {"limit": 5})]
|
||||
assert deezer.album_calls == [
|
||||
(
|
||||
"artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
},
|
||||
),
|
||||
(
|
||||
"deezer-artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
},
|
||||
),
|
||||
]
|
||||
|
||||
|
||||
def test_get_artist_discography_respects_source_override_without_fallback(monkeypatch):
|
||||
deezer = _FakeSourceClient()
|
||||
itunes = _FakeSourceClient(album_results=[_album("itunes-album-1", "iTunes Album", "2024-02-01")])
|
||||
spotify = _FakeSourceClient()
|
||||
clients = {
|
||||
"deezer": deezer,
|
||||
"itunes": itunes,
|
||||
"spotify": spotify,
|
||||
}
|
||||
|
||||
monkeypatch.setattr(metadata_service, "get_primary_source", lambda: "deezer")
|
||||
monkeypatch.setattr(metadata_service, "get_source_priority", lambda primary: [primary, "spotify", "itunes"])
|
||||
monkeypatch.setattr(metadata_service, "get_client_for_source", lambda source: clients.get(source))
|
||||
|
||||
result = metadata_service.get_artist_discography(
|
||||
"artist-1",
|
||||
"Artist One",
|
||||
MetadataLookupOptions(source_override="itunes", allow_fallback=False),
|
||||
)
|
||||
|
||||
assert result["source"] == "itunes"
|
||||
assert result["source_priority"] == ["itunes"]
|
||||
assert [album["id"] for album in result["albums"]] == ["itunes-album-1"]
|
||||
assert deezer.album_calls == []
|
||||
assert spotify.album_calls == []
|
||||
assert itunes.album_calls == [
|
||||
(
|
||||
"artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
},
|
||||
)
|
||||
]
|
||||
|
||||
|
||||
def test_get_artist_discography_uses_hydrabase_fast_path_when_active(monkeypatch):
|
||||
class _HydrabaseLikeClient(_FakeSourceClient):
|
||||
def get_artist_albums(self, artist_id, **kwargs):
|
||||
self.album_calls.append((artist_id, dict(kwargs)))
|
||||
if artist_id == "hydrabase-artist-1":
|
||||
return [
|
||||
_album("hydrabase-album-1", "Hydra Album", "2024-03-01"),
|
||||
_album("hydrabase-single-1", "Hydra Single", "2024-04-01", album_type="single"),
|
||||
]
|
||||
return []
|
||||
|
||||
hydrabase = _HydrabaseLikeClient(
|
||||
artist_search_results=[_artist("hydrabase-artist-1", "Artist One")],
|
||||
)
|
||||
clients = {"deezer": None, "spotify": None, "itunes": None}
|
||||
|
||||
monkeypatch.setattr(metadata_service, "get_primary_source", lambda: "deezer")
|
||||
monkeypatch.setattr(metadata_service, "get_source_priority", lambda primary: [primary, "spotify", "itunes", "hydrabase"])
|
||||
def fake_get_client_for_source(source):
|
||||
if source == "hydrabase":
|
||||
return hydrabase
|
||||
return clients.get(source)
|
||||
|
||||
monkeypatch.setattr(metadata_service, "get_client_for_source", fake_get_client_for_source)
|
||||
|
||||
result = metadata_service.get_artist_discography("artist-1", "Artist One", MetadataLookupOptions())
|
||||
|
||||
assert result["source"] == "hydrabase"
|
||||
assert [album["id"] for album in result["albums"]] == ["hydrabase-album-1"]
|
||||
assert [single["id"] for single in result["singles"]] == ["hydrabase-single-1"]
|
||||
assert hydrabase.album_calls == [
|
||||
(
|
||||
"artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
},
|
||||
),
|
||||
(
|
||||
"hydrabase-artist-1",
|
||||
{
|
||||
"album_type": "album,single",
|
||||
"limit": 50,
|
||||
},
|
||||
)
|
||||
]
|
||||
assert hydrabase.artist_search_calls == [("Artist One", {"limit": 5})]
|
||||
331
web_server.py
331
web_server.py
|
|
@ -5958,15 +5958,11 @@ _COMPARISON_MAX_ENTRIES = 50
|
|||
_comparison_lock = threading.Lock()
|
||||
|
||||
def _is_hydrabase_active():
|
||||
"""Check if Hydrabase should be used as the PRIMARY metadata source (replaces Spotify).
|
||||
Only active in dev mode — the legacy 'Hydrabase replaces everything' behavior.
|
||||
When selected as a fallback source, Hydrabase works through the normal fallback
|
||||
path (_get_metadata_fallback_client) just like iTunes/Deezer — not as primary."""
|
||||
"""Check if Hydrabase is connected and enabled for metadata use."""
|
||||
try:
|
||||
if hydrabase_client is None or not hydrabase_client.is_connected():
|
||||
return False
|
||||
return dev_mode_enabled
|
||||
except (NameError, Exception):
|
||||
from core.metadata_service import is_hydrabase_enabled
|
||||
return is_hydrabase_enabled()
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
def _run_background_comparison(query, hydrabase_counts=None):
|
||||
|
|
@ -11053,247 +11049,79 @@ def get_artist_discography(artist_id):
|
|||
"""Get an artist's complete discography (albums and singles)"""
|
||||
try:
|
||||
# Get optional artist name for fallback searches
|
||||
artist_name = request.args.get('artist_name', '')
|
||||
artist_name = request.args.get('artist_name', '').strip()
|
||||
# Optional source override from multi-source search tabs
|
||||
source_override = request.args.get('source', '')
|
||||
source_override = request.args.get('source', '').strip().lower()
|
||||
|
||||
# Mirror to Hydrabase P2P network
|
||||
if hydrabase_worker and dev_mode_enabled and artist_name:
|
||||
hydrabase_worker.enqueue(artist_name, 'artist.albums')
|
||||
|
||||
# Determine which source to use
|
||||
spotify_available = spotify_client and spotify_client.is_spotify_authenticated()
|
||||
|
||||
# Import fallback client for non-Spotify lookups
|
||||
fallback_client = _get_metadata_fallback_client()
|
||||
fallback_source = _get_metadata_fallback_source()
|
||||
|
||||
print(f"Fetching discography for artist: {artist_id} (name: {artist_name}, spotify: {spotify_available}, source_override: {source_override or 'auto'})")
|
||||
|
||||
albums = []
|
||||
active_source = None
|
||||
|
||||
# Source override: when user clicked from a specific search tab, use that source directly
|
||||
if source_override and source_override in ('spotify', 'itunes', 'deezer'):
|
||||
try:
|
||||
if source_override == 'spotify' and spotify_available:
|
||||
albums = spotify_client.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = 'spotify'
|
||||
elif source_override == 'itunes':
|
||||
itunes_cl = _get_itunes_client()
|
||||
albums = itunes_cl.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = 'itunes'
|
||||
elif source_override == 'deezer':
|
||||
deezer_cl = _get_deezer_client()
|
||||
albums = deezer_cl.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = 'deezer'
|
||||
elif source_override == 'discogs':
|
||||
discogs_cl = _get_discogs_client()
|
||||
albums = discogs_cl.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = 'discogs'
|
||||
elif source_override == 'hydrabase':
|
||||
plugin = request.args.get('plugin', '').lower()
|
||||
if plugin == 'deezer':
|
||||
hb_cl = _get_deezer_client()
|
||||
elif plugin == 'itunes' or artist_id.isdigit():
|
||||
hb_cl = _get_itunes_client()
|
||||
else:
|
||||
hb_cl = spotify_client
|
||||
albums = hb_cl.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = plugin or 'hydrabase'
|
||||
|
||||
# If direct ID lookup failed but we have artist name, search by name
|
||||
if not albums and artist_name:
|
||||
if source_override == 'itunes':
|
||||
cl = _get_itunes_client()
|
||||
elif source_override == 'hydrabase':
|
||||
plugin = request.args.get('plugin', '').lower()
|
||||
if plugin == 'deezer':
|
||||
cl = _get_deezer_client()
|
||||
else:
|
||||
cl = _get_itunes_client()
|
||||
elif source_override == 'deezer':
|
||||
cl = _get_deezer_client()
|
||||
elif source_override == 'discogs':
|
||||
cl = _get_discogs_client()
|
||||
elif source_override == 'spotify' and spotify_available:
|
||||
cl = spotify_client
|
||||
else:
|
||||
cl = None
|
||||
|
||||
if cl:
|
||||
search_artists = cl.search_artists(artist_name, limit=5)
|
||||
if search_artists:
|
||||
best = next((a for a in search_artists if a.name.lower() == artist_name.lower()), search_artists[0])
|
||||
albums = cl.get_artist_albums(best.id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = source_override
|
||||
|
||||
if albums:
|
||||
print(f"Got {len(albums)} albums from {active_source} (source override)")
|
||||
except Exception as e:
|
||||
print(f"Source override ({source_override}) lookup failed: {e}")
|
||||
|
||||
# Try Hydrabase first when active (and no source override)
|
||||
if not albums and _is_hydrabase_active() and artist_name:
|
||||
try:
|
||||
albums = hydrabase_client.search_discography(artist_name, limit=50)
|
||||
if albums:
|
||||
active_source = 'hydrabase'
|
||||
print(f"Got {len(albums)} albums from Hydrabase")
|
||||
except Exception as e:
|
||||
print(f"Hydrabase discography failed: {e}")
|
||||
|
||||
# Try to get albums from the appropriate source
|
||||
# Check if the ID looks like Spotify (alphanumeric) or iTunes (numeric only)
|
||||
is_numeric_id = artist_id.isdigit()
|
||||
|
||||
if not albums and spotify_available and not is_numeric_id:
|
||||
# Try Spotify first for alphanumeric IDs
|
||||
try:
|
||||
albums = spotify_client.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = 'spotify'
|
||||
print(f"Got {len(albums)} albums from Spotify")
|
||||
except Exception as e:
|
||||
print(f"Spotify lookup failed: {e}")
|
||||
|
||||
# Try fallback source if Spotify didn't work or if it's a numeric ID
|
||||
if not albums:
|
||||
try:
|
||||
if is_numeric_id:
|
||||
# It's a numeric ID (iTunes/Deezer), use directly
|
||||
albums = fallback_client.get_artist_albums(artist_id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = fallback_source
|
||||
print(f"Got {len(albums)} albums from {fallback_source} (direct ID)")
|
||||
elif artist_name:
|
||||
# Search fallback by name
|
||||
print(f"Trying {fallback_source} search by name: '{artist_name}'")
|
||||
fallback_artists = fallback_client.search_artists(artist_name, limit=5)
|
||||
if fallback_artists:
|
||||
# Find best match
|
||||
best_match = None
|
||||
for artist in fallback_artists:
|
||||
if artist.name.lower() == artist_name.lower():
|
||||
best_match = artist
|
||||
break
|
||||
if not best_match:
|
||||
best_match = fallback_artists[0]
|
||||
|
||||
print(f"Found {fallback_source} artist: {best_match.name} (ID: {best_match.id})")
|
||||
albums = fallback_client.get_artist_albums(best_match.id, album_type='album,single')
|
||||
if albums:
|
||||
active_source = fallback_source
|
||||
print(f"Got {len(albums)} albums from {fallback_source} (name search)")
|
||||
except Exception as e:
|
||||
print(f"{fallback_source} lookup failed: {e}")
|
||||
|
||||
print(f"Total albums returned: {len(albums)} (source: {active_source})")
|
||||
|
||||
if not albums:
|
||||
return jsonify({
|
||||
"albums": [],
|
||||
"singles": [],
|
||||
"source": active_source or "unknown"
|
||||
})
|
||||
|
||||
# Separate albums from singles/EPs
|
||||
album_list = []
|
||||
singles_list = []
|
||||
|
||||
# Track seen albums to avoid duplicates (especially for "appears_on")
|
||||
seen_albums = set()
|
||||
|
||||
for album in albums:
|
||||
# Skip duplicates
|
||||
if album.id in seen_albums:
|
||||
continue
|
||||
seen_albums.add(album.id)
|
||||
|
||||
# Skip albums where this artist isn't the primary (first-listed) artist
|
||||
if hasattr(album, 'artist_ids') and album.artist_ids:
|
||||
if album.artist_ids[0] != artist_id:
|
||||
continue
|
||||
|
||||
album_data = {
|
||||
"id": album.id,
|
||||
"name": album.name,
|
||||
"release_date": album.release_date if hasattr(album, 'release_date') else None,
|
||||
"album_type": album.album_type if hasattr(album, 'album_type') else 'album',
|
||||
"image_url": album.image_url if hasattr(album, 'image_url') else None,
|
||||
"total_tracks": album.total_tracks if hasattr(album, 'total_tracks') else 0,
|
||||
"external_urls": album.external_urls if hasattr(album, 'external_urls') else {}
|
||||
}
|
||||
|
||||
# Skip obvious compilation issues but be more lenient for now
|
||||
if hasattr(album, 'album_type') and album.album_type == 'compilation':
|
||||
print(f"Found compilation: '{album.name}' - including for now")
|
||||
|
||||
# Categorize by album type
|
||||
if hasattr(album, 'album_type'):
|
||||
if album.album_type in ['single', 'ep']:
|
||||
singles_list.append(album_data)
|
||||
else: # 'album' or approved 'compilation'
|
||||
album_list.append(album_data)
|
||||
effective_override_source = source_override
|
||||
if source_override == 'hydrabase':
|
||||
plugin = request.args.get('plugin', '').strip().lower()
|
||||
if plugin == 'deezer':
|
||||
effective_override_source = 'deezer'
|
||||
elif plugin == 'itunes' or artist_id.isdigit():
|
||||
effective_override_source = 'itunes'
|
||||
else:
|
||||
# Default to album if no type specified
|
||||
album_list.append(album_data)
|
||||
|
||||
# Sort by release date (newest first)
|
||||
def get_release_year(item):
|
||||
if item['release_date']:
|
||||
try:
|
||||
# Handle different date formats (YYYY, YYYY-MM, YYYY-MM-DD)
|
||||
return int(item['release_date'][:4])
|
||||
except (ValueError, IndexError):
|
||||
return 0
|
||||
return 0
|
||||
|
||||
album_list.sort(key=get_release_year, reverse=True)
|
||||
singles_list.sort(key=get_release_year, reverse=True)
|
||||
|
||||
print(f"Found {len(album_list)} albums and {len(singles_list)} singles for artist {artist_id}")
|
||||
|
||||
# Debug: Log the final album list
|
||||
for album in album_list:
|
||||
print(f"Album: {album['name']} ({album['album_type']}) - {album['release_date']}")
|
||||
for single in singles_list:
|
||||
print(f"Single/EP: {single['name']} ({single['album_type']}) - {single['release_date']}")
|
||||
|
||||
effective_override_source = 'spotify'
|
||||
|
||||
from core.metadata_service import MetadataLookupOptions, get_artist_discography as _get_artist_discography
|
||||
|
||||
discography = _get_artist_discography(
|
||||
artist_id,
|
||||
artist_name=artist_name,
|
||||
options=MetadataLookupOptions(
|
||||
source_override=effective_override_source,
|
||||
allow_fallback=True,
|
||||
skip_cache=False,
|
||||
max_pages=0,
|
||||
limit=50,
|
||||
),
|
||||
)
|
||||
|
||||
album_list = discography['albums']
|
||||
singles_list = discography['singles']
|
||||
active_source = discography['source']
|
||||
source_priority = discography['source_priority']
|
||||
|
||||
# Gather artist enrichment info from cache + library
|
||||
artist_info = {}
|
||||
try:
|
||||
cache = get_metadata_cache()
|
||||
cache_sources = []
|
||||
if active_source:
|
||||
cache_sources.append(active_source)
|
||||
for source in source_priority:
|
||||
if source not in cache_sources:
|
||||
cache_sources.append(source)
|
||||
|
||||
# Try metadata cache for genres, image, followers
|
||||
cached = cache.get_entity(active_source or 'spotify', 'artist', artist_id)
|
||||
if not cached and active_source != 'spotify':
|
||||
cached = cache.get_entity('spotify', 'artist', artist_id)
|
||||
if not cached:
|
||||
cached = None
|
||||
for src in cache_sources:
|
||||
cached = cache.get_entity(src, 'artist', artist_id)
|
||||
if cached:
|
||||
break
|
||||
if not cached and artist_name:
|
||||
# Try by name across all sources
|
||||
for src in ['spotify', 'itunes', 'deezer']:
|
||||
if artist_name:
|
||||
db_tmp = get_database()
|
||||
conn_tmp = db_tmp._get_connection()
|
||||
try:
|
||||
cur = conn_tmp.cursor()
|
||||
cur.execute("""
|
||||
SELECT genres, image_url, followers, popularity, external_urls
|
||||
FROM metadata_cache_entities
|
||||
WHERE entity_type = 'artist' AND name COLLATE NOCASE = ? AND source = ?
|
||||
LIMIT 1
|
||||
""", (artist_name, src))
|
||||
row = cur.fetchone()
|
||||
if row:
|
||||
cached = dict(row)
|
||||
break
|
||||
finally:
|
||||
conn_tmp.close()
|
||||
for src in cache_sources:
|
||||
db_tmp = get_database()
|
||||
conn_tmp = db_tmp._get_connection()
|
||||
try:
|
||||
cur = conn_tmp.cursor()
|
||||
cur.execute("""
|
||||
SELECT genres, image_url, followers, popularity, external_urls
|
||||
FROM metadata_cache_entities
|
||||
WHERE entity_type = 'artist' AND name COLLATE NOCASE = ? AND source = ?
|
||||
LIMIT 1
|
||||
""", (artist_name, src))
|
||||
row = cur.fetchone()
|
||||
if row:
|
||||
cached = dict(row)
|
||||
break
|
||||
finally:
|
||||
conn_tmp.close()
|
||||
if cached:
|
||||
try:
|
||||
artist_info['genres'] = json.loads(cached.get('genres', '[]')) if isinstance(cached.get('genres'), str) else (cached.get('genres') or [])
|
||||
|
|
@ -11366,14 +11194,12 @@ def get_artist_discography(artist_id):
|
|||
return jsonify({
|
||||
"albums": album_list,
|
||||
"singles": singles_list,
|
||||
"source": active_source or "spotify",
|
||||
"source": active_source or (source_priority[0] if source_priority else "unknown"),
|
||||
"artist_info": artist_info,
|
||||
})
|
||||
|
||||
except Exception as e:
|
||||
print(f"Error fetching artist discography: {e}")
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
logger.exception("Error fetching artist discography for %s", artist_id)
|
||||
return jsonify({"error": str(e)}), 500
|
||||
|
||||
def _resolve_db_album_id(album_id, artist_id=None):
|
||||
|
|
@ -11413,20 +11239,20 @@ def _resolve_db_album_id(album_id, artist_id=None):
|
|||
album_title = row['title']
|
||||
artist_name = row['artist_name']
|
||||
query = f"{artist_name} {album_title}"
|
||||
print(f"Searching for album by name: '{query}'")
|
||||
logger.debug("Searching for album by name: %s", query)
|
||||
results = spotify_client.search_albums(query, limit=5)
|
||||
if results:
|
||||
# Pick the best match (search already ranks by relevance)
|
||||
for album in results:
|
||||
if album.name.lower().strip() == album_title.lower().strip():
|
||||
print(f"Found exact album match: {album.name} (ID: {album.id})")
|
||||
logger.debug("Found exact album match: %s (id=%s)", album.name, album.id)
|
||||
return album.id
|
||||
# Fall back to first result if no exact title match
|
||||
print(f"No exact match, using best result: {results[0].name} (ID: {results[0].id})")
|
||||
logger.debug("No exact match, using best result: %s (id=%s)", results[0].name, results[0].id)
|
||||
return results[0].id
|
||||
|
||||
except Exception as e:
|
||||
print(f"Error resolving DB album ID {album_id}: {e}")
|
||||
logger.debug("Error resolving DB album ID %s: %s", album_id, e)
|
||||
return None
|
||||
|
||||
|
||||
|
|
@ -11466,7 +11292,7 @@ def get_artist_album_tracks(artist_id, album_id):
|
|||
'uri': '',
|
||||
'album': album_info
|
||||
})
|
||||
print(f"Hydrabase returned {len(formatted_tracks)} tracks for album: {album_info['name']}")
|
||||
logger.info("Hydrabase returned %s tracks for album %s", len(formatted_tracks), album_info['name'])
|
||||
return jsonify({
|
||||
'success': True,
|
||||
'album': album_info,
|
||||
|
|
@ -11494,7 +11320,12 @@ def get_artist_album_tracks(artist_id, album_id):
|
|||
elif source_override == 'discogs':
|
||||
client = _get_discogs_client()
|
||||
|
||||
print(f"Fetching tracks for album: {album_id} by artist: {artist_id} (source: {source_override or 'auto'})")
|
||||
logger.debug(
|
||||
"Fetching tracks for album %s by artist %s (source=%s)",
|
||||
album_id,
|
||||
artist_id,
|
||||
source_override or 'auto',
|
||||
)
|
||||
|
||||
# Get album information first
|
||||
album_data = client.get_album(album_id)
|
||||
|
|
@ -11504,7 +11335,7 @@ def get_artist_album_tracks(artist_id, album_id):
|
|||
if not album_data:
|
||||
resolved_album_id = _resolve_db_album_id(album_id, artist_id)
|
||||
if resolved_album_id and resolved_album_id != album_id:
|
||||
print(f"Resolved DB album ID {album_id} -> external ID {resolved_album_id}")
|
||||
logger.debug("Resolved DB album ID %s -> external ID %s", album_id, resolved_album_id)
|
||||
album_data = client.get_album(resolved_album_id)
|
||||
|
||||
if not album_data:
|
||||
|
|
@ -11558,7 +11389,7 @@ def get_artist_album_tracks(artist_id, album_id):
|
|||
}
|
||||
formatted_tracks.append(formatted_track)
|
||||
|
||||
print(f"Successfully formatted {len(formatted_tracks)} tracks for album: {album_info['name']}")
|
||||
logger.info("Successfully formatted %s tracks for album %s", len(formatted_tracks), album_info['name'])
|
||||
|
||||
return jsonify({
|
||||
'success': True,
|
||||
|
|
@ -11567,9 +11398,7 @@ def get_artist_album_tracks(artist_id, album_id):
|
|||
})
|
||||
|
||||
except Exception as e:
|
||||
print(f"Error fetching album tracks: {e}")
|
||||
import traceback
|
||||
traceback.print_exc()
|
||||
logger.exception("Error fetching album tracks for artist %s album %s", artist_id, album_id)
|
||||
return jsonify({"error": str(e)}), 500
|
||||
|
||||
@app.route('/api/artist/<artist_id>/download-discography', methods=['POST'])
|
||||
|
|
|
|||
Loading…
Reference in a new issue