Cache artist album lists across metadata sources

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]

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', ''):

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]

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,
+    )

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:

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