From 76d3e25fd421445815391cd73bb349fddf79d9a8 Mon Sep 17 00:00:00 2001 From: ramonskie Date: Thu, 11 Jun 2026 22:33:04 +0200 Subject: [PATCH] Cache artist album lists across metadata sources --- core/deezer_client.py | 20 ++- core/discogs_client.py | 46 +++++-- core/itunes_client.py | 16 ++- core/metadata/artist_album_cache.py | 90 +++++++++++++ core/spotify_client.py | 26 +++- ...test_artist_album_list_cache_nonspotify.py | 118 ++++++++++++++++++ 6 files changed, 293 insertions(+), 23 deletions(-) create mode 100644 core/metadata/artist_album_cache.py create mode 100644 tests/test_artist_album_list_cache_nonspotify.py diff --git a/core/deezer_client.py b/core/deezer_client.py index 07162228..5df8c078 100644 --- a/core/deezer_client.py +++ b/core/deezer_client.py @@ -6,6 +6,7 @@ from typing import Dict, List, Optional, Any from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("deezer_client") @@ -873,6 +874,23 @@ class DeezerClient: Matches iTunesClient.get_artist_albums() interface. Paginates through all results up to the requested limit.""" + cache = get_metadata_cache() + cached_items = get_cached_artist_album_items(cache, 'deezer', artist_id, album_type=album_type, limit=limit) + if cached_items: + try: + requested_types = [t.strip() for t in album_type.split(',')] + cached_albums = [] + for album_data in cached_items: + album = Album.from_deezer_album(album_data) + if album_type != 'album,single': + if album.album_type not in requested_types: + if not (album.album_type == 'ep' and 'single' in requested_types): + continue + cached_albums.append(album) + return cached_albums[:limit] + except Exception as e: + logger.debug("Deezer artist albums cache reuse failed: %s", e) + albums = [] all_raw = [] requested_types = [t.strip() for t in album_type.split(',')] @@ -900,7 +918,6 @@ class DeezerClient: break # Last page offset += len(data['data']) - cache = get_metadata_cache() # Deezer's /artist/{id}/albums endpoint doesn't include artist info on each album. # Inject it so cached album entities have artist_name for discover page display. artist_stub = None @@ -914,6 +931,7 @@ class DeezerClient: entries.append((str(ad['id']), ad)) if entries: cache.store_entities_bulk('deezer', 'album', entries, skip_if_exists=True) + store_artist_album_items(cache, 'deezer', artist_id, all_raw, album_type=album_type, limit=limit) logger.info(f"Retrieved {len(albums)} albums for artist {artist_id}") return albums[:limit] diff --git a/core/discogs_client.py b/core/discogs_client.py index 54b95775..7bafa486 100644 --- a/core/discogs_client.py +++ b/core/discogs_client.py @@ -12,6 +12,7 @@ import re import time import threading import requests +from core.metadata.artist_album_cache import get_cached_artist_album_payload, store_artist_album_items from core.metadata.cache import get_metadata_cache from typing import List, Dict, Any, Optional from dataclasses import dataclass @@ -728,26 +729,45 @@ class DiscogsClient: def get_artist_albums(self, artist_id: str, album_type: str = 'album,single', limit: int = 50) -> List[Album]: """Get releases by an artist. Prefers master releases, filters features.""" - # First get the artist name for feature filtering. Strip Discogs - # disambiguation suffix so feature-vs-primary matching below - # compares against the canonical name, not "Beyoncé*". - artist_data = self._api_get(f'/artists/{artist_id}') - artist_name = _clean_discogs_artist_name( - artist_data.get('name', '') if artist_data else '' - ).lower() + cache = get_metadata_cache() + cached_payload = get_cached_artist_album_payload(cache, 'discogs', artist_id, album_type=album_type, limit=limit) + releases = cached_payload.get('_releases') if cached_payload else None + artist_name = '' + if cached_payload: + artist_name = str(cached_payload.get('artist_name') or '').lower() - data = self._api_get(f'/artists/{artist_id}/releases', { - 'sort': 'year', 'sort_order': 'desc', 'per_page': min(limit * 3, 200), - }) - if not data or not data.get('releases'): - return [] + if not isinstance(releases, list) or not releases: + # First get the artist name for feature filtering. Strip Discogs + # disambiguation suffix so feature-vs-primary matching below + # compares against the canonical name, not "Beyoncé*". + artist_data = self._api_get(f'/artists/{artist_id}') + artist_name = _clean_discogs_artist_name( + artist_data.get('name', '') if artist_data else '' + ).lower() + + data = self._api_get(f'/artists/{artist_id}/releases', { + 'sort': 'year', 'sort_order': 'desc', 'per_page': min(limit * 3, 200), + }) + if not data or not data.get('releases'): + return [] + releases = data.get('releases') or [] + store_artist_album_items( + cache, + 'discogs', + artist_id, + releases, + album_type=album_type, + limit=limit, + items_field='_releases', + extra_fields={'artist_name': artist_name}, + ) # Separate masters from individual releases — prefer masters (canonical versions) masters = [] releases_no_master = [] master_titles = set() - for item in data['releases']: + for item in releases: # Skip non-main roles role = item.get('role', 'Main').lower() if role not in ('main', ''): diff --git a/core/itunes_client.py b/core/itunes_client.py index 6f08dba3..25f9c73a 100644 --- a/core/itunes_client.py +++ b/core/itunes_client.py @@ -5,6 +5,7 @@ import threading from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("itunes_client") @@ -1036,7 +1037,15 @@ class iTunesClient: """ import re - results = self._lookup(id=artist_id, entity='album', limit=min(limit, 200)) + cache = get_metadata_cache() + cache_limit = min(limit, 200) + cached_items = get_cached_artist_album_items(cache, 'itunes', artist_id, album_type=album_type, limit=cache_limit) + cache_hit = False + if cached_items: + results = cached_items + cache_hit = True + else: + results = self._lookup(id=artist_id, entity='album', limit=cache_limit) seen_albums = {} # Track albums by normalized name, prefer explicit versions def normalize_album_name(name: str) -> str: @@ -1098,7 +1107,7 @@ class iTunesClient: # If this is an explicit album, validate it has tracks before keeping it # (Some iTunes explicit albums are broken and return 0 tracks) - if is_explicit: + if is_explicit and not cache_hit: try: test_tracks = self._lookup(id=album.id, entity='song') track_count = len([t for t in test_tracks if t.get('wrapperType') == 'track']) @@ -1124,8 +1133,9 @@ class iTunesClient: if album_data.get('wrapperType') == 'collection' and album_data.get('collectionId'): album_entries.append((str(album_data['collectionId']), album_data)) if album_entries: - cache = get_metadata_cache() cache.store_entities_bulk('itunes', 'album', album_entries, skip_if_exists=True) + if not cache_hit: + store_artist_album_items(cache, 'itunes', artist_id, results, album_type=album_type, limit=cache_limit) logger.info(f"Retrieved {len(albums)} unique albums for artist {artist_id} (filtered from {len(results)} results)") return albums[:limit] diff --git a/core/metadata/artist_album_cache.py b/core/metadata/artist_album_cache.py new file mode 100644 index 00000000..2a138b5c --- /dev/null +++ b/core/metadata/artist_album_cache.py @@ -0,0 +1,90 @@ +"""Shared artist album-list cache helpers for metadata clients.""" + +from __future__ import annotations + +from typing import Any, Optional + + +def make_artist_album_cache_key( + artist_id: str, + album_type: str = 'album,single', + limit: int = 200, + *, + include_limit: bool = True, +) -> str: + """Return the metadata-cache key for an artist album-list query.""" + safe_album_type = str(album_type or 'album,single').replace(',', '_') + base_key = f"{artist_id}_albums_{safe_album_type}" + return f"{base_key}_{limit}" if include_limit else base_key + + +def get_cached_artist_album_items( + cache: Any, + source: str, + artist_id: str, + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, + items_field: str = '_albums', +) -> Optional[list[dict[str, Any]]]: + """Return cached raw artist album-list items, or None on miss/invalid shape.""" + cached = cache.get_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + ) + if not isinstance(cached, dict): + return None + + items = cached.get(items_field) + return items if isinstance(items, list) and items else None + + +def get_cached_artist_album_payload( + cache: Any, + source: str, + artist_id: str, + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, +) -> Optional[dict[str, Any]]: + """Return the raw cached artist album-list payload, or None on miss.""" + cached = cache.get_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + ) + return cached if isinstance(cached, dict) else None + + +def store_artist_album_items( + cache: Any, + source: str, + artist_id: str, + items: list[dict[str, Any]], + *, + album_type: str = 'album,single', + limit: int = 200, + include_limit: bool = True, + items_field: str = '_albums', + extra_fields: Optional[dict[str, Any]] = None, +) -> None: + """Store raw artist album-list items in the metadata cache.""" + if not items: + return + + payload: dict[str, Any] = { + 'name': f'albums_{artist_id}', + items_field: items, + } + if extra_fields: + payload.update(extra_fields) + + cache.store_entity( + source, + 'artist', + make_artist_album_cache_key(artist_id, album_type, limit, include_limit=include_limit), + payload, + ) diff --git a/core/spotify_client.py b/core/spotify_client.py index 17824d09..b5318037 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -8,6 +8,7 @@ from functools import wraps from dataclasses import dataclass from utils.logging_config import get_logger from config.settings import config_manager +from core.metadata.artist_album_cache import get_cached_artist_album_items, store_artist_album_items from core.metadata.cache import get_metadata_cache logger = get_logger("spotify_client") @@ -1897,15 +1898,20 @@ class SpotifyClient: cache = get_metadata_cache() fallback_src = self._fallback_source source = fallback_src if self._is_itunes_id(artist_id) else 'spotify' - cache_key = f"{artist_id}_albums_{album_type.replace(',', '_')}" # Check cache first (unless caller needs fresh data) if not skip_cache: - cached = cache.get_entity(source, 'artist', cache_key) - if cached: + cached_items = get_cached_artist_album_items( + cache, + source, + artist_id, + album_type=album_type, + limit=limit, + include_limit=False, + ) + if cached_items: try: - albums_list = cached.get('_albums', cached) if isinstance(cached, dict) else cached - return [Album.from_spotify_album(ad) for ad in albums_list] + return [Album.from_spotify_album(ad) for ad in cached_items] except Exception as e: logger.debug("artist albums cache reuse: %s", e) @@ -1959,7 +1965,15 @@ class SpotifyClient: # complete entities regardless of how many pages we walked. if raw_items: if not truncated: - cache.store_entity('spotify', 'artist', cache_key, {'name': f'albums_{artist_id}', '_albums': raw_items}) + store_artist_album_items( + cache, + 'spotify', + artist_id, + raw_items, + album_type=album_type, + limit=limit, + include_limit=False, + ) # Also cache individual albums opportunistically entries = [(ad.get('id'), ad) for ad in raw_items if ad.get('id')] if entries: diff --git a/tests/test_artist_album_list_cache_nonspotify.py b/tests/test_artist_album_list_cache_nonspotify.py new file mode 100644 index 00000000..a10d0855 --- /dev/null +++ b/tests/test_artist_album_list_cache_nonspotify.py @@ -0,0 +1,118 @@ +from __future__ import annotations + +from unittest.mock import MagicMock + +import core.deezer_client as deezer_mod +import core.discogs_client as discogs_mod +import core.itunes_client as itunes_mod +from core.metadata.artist_album_cache import ( + get_cached_artist_album_items, + get_cached_artist_album_payload, + make_artist_album_cache_key, + store_artist_album_items, +) + + +class MemoryCache: + def __init__(self): + self.entities = {} + + def get_entity(self, source, entity_type, entity_id): + return self.entities.get((source, entity_type, entity_id)) + + def store_entity(self, source, entity_type, entity_id, raw_data): + self.entities[(source, entity_type, entity_id)] = raw_data + + def store_entities_bulk(self, *args, **kwargs): + return None + + +def test_artist_album_cache_helper_round_trips_items_and_payload(): + cache = MemoryCache() + + store_artist_album_items( + cache, + 'deezer', + 'artist-1', + [{'id': 'album-1'}], + album_type='album,single', + limit=200, + extra_fields={'artist_name': 'Artist'}, + ) + + assert make_artist_album_cache_key('artist-1', 'album,single', 200) == 'artist-1_albums_album_single_200' + assert make_artist_album_cache_key('artist-1', 'album,single', 200, include_limit=False) == 'artist-1_albums_album_single' + assert get_cached_artist_album_items(cache, 'deezer', 'artist-1', limit=200) == [{'id': 'album-1'}] + assert get_cached_artist_album_payload(cache, 'deezer', 'artist-1', limit=200)['artist_name'] == 'Artist' + + +def test_deezer_artist_albums_reuses_list_cache(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(deezer_mod, 'get_metadata_cache', lambda: cache) + client = deezer_mod.DeezerClient.__new__(deezer_mod.DeezerClient) + client._api_get = MagicMock(return_value={ + 'data': [{ + 'id': 123, + 'title': 'Cached Deezer Album', + 'record_type': 'album', + 'release_date': '2024-01-01', + 'nb_tracks': 10, + 'artist': {'id': 7, 'name': 'Artist'}, + }] + }) + + first = client.get_artist_albums('7', limit=200) + second = client.get_artist_albums('7', limit=200) + + assert [album.name for album in first] == ['Cached Deezer Album'] + assert [album.name for album in second] == ['Cached Deezer Album'] + client._api_get.assert_called_once() + + +def test_itunes_artist_albums_reuses_list_cache_and_skips_validation(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(itunes_mod, 'get_metadata_cache', lambda: cache) + client = itunes_mod.iTunesClient.__new__(itunes_mod.iTunesClient) + client._lookup = MagicMock(side_effect=[ + [{ + 'wrapperType': 'collection', + 'collectionId': 456, + 'collectionName': 'Cached iTunes Album', + 'artistId': 8, + 'artistName': 'Artist', + 'trackCount': 10, + 'collectionExplicitness': 'notExplicit', + 'releaseDate': '2024-01-01T00:00:00Z', + }] + ]) + + first = client.get_artist_albums('8', limit=200) + second = client.get_artist_albums('8', limit=200) + + assert [album.name for album in first] == ['Cached iTunes Album'] + assert [album.name for album in second] == ['Cached iTunes Album'] + client._lookup.assert_called_once_with(id='8', entity='album', limit=200) + + +def test_discogs_artist_albums_reuses_list_cache(monkeypatch): + cache = MemoryCache() + monkeypatch.setattr(discogs_mod, 'get_metadata_cache', lambda: cache) + client = discogs_mod.DiscogsClient.__new__(discogs_mod.DiscogsClient) + client._api_get = MagicMock(side_effect=[ + {'name': 'Artist'}, + {'releases': [{ + 'id': 789, + 'type': 'master', + 'role': 'Main', + 'title': 'Cached Discogs Album', + 'artist': 'Artist', + 'year': 2024, + }]}, + ]) + + first = client.get_artist_albums('9', limit=50) + second = client.get_artist_albums('9', limit=50) + + assert [album.name for album in first] == ['Cached Discogs Album'] + assert [album.name for album in second] == ['Cached Discogs Album'] + assert client._api_get.call_count == 2