diff --git a/core/metadata/cache.py b/core/metadata/cache.py index 1d296da8..1653fc26 100644 --- a/core/metadata/cache.py +++ b/core/metadata/cache.py @@ -830,6 +830,36 @@ class MetadataCache: logger.error(f"Cache health stats error: {e}") return {} + def purge_artist_album_lists(self, source: str = 'spotify') -> int: + """One-time repair: delete cached artist-ALBUM-LIST entries. + + Partial watchlist probes (limit=5, max_pages=1) used to be stored in + the same unqualified ``_albums_`` slot the artist + detail page reads, so every watchlist artist's page showed only the + newest handful of releases. The writer is fixed to skip truncated + fetches; this clears the already-poisoned entries (30-day TTL would + otherwise keep them for weeks). Lists rebuild lazily on the next + artist-page visit. Returns the number of entries removed.""" + try: + db = self._get_db() + conn = db._get_connection() + try: + cursor = conn.cursor() + cursor.execute( + "DELETE FROM metadata_cache_entities " + "WHERE source = ? AND entity_type = 'artist' " + "AND entity_id LIKE '%\\_albums\\_%' ESCAPE '\\'", + (source,), + ) + count = cursor.rowcount + conn.commit() + return count + finally: + conn.close() + except Exception as e: + logger.debug("artist album-list purge failed: %s", e) + return 0 + def clear(self, source: str = None, entity_type: str = None) -> int: """Clear cache entries. Optional filters by source and/or entity_type.""" try: diff --git a/core/spotify_client.py b/core/spotify_client.py index 0dafdcde..d40d8780 100644 --- a/core/spotify_client.py +++ b/core/spotify_client.py @@ -1859,6 +1859,7 @@ class SpotifyClient: try: albums = [] raw_items = [] + truncated = False # did we stop while more pages existed? # Spotify caps artist_albums at 10 per page results = self.sp.artist_albums(artist_id, album_type=album_type, limit=min(limit, 10)) pages_fetched = 1 @@ -1871,6 +1872,7 @@ class SpotifyClient: # Stop if we've hit the page limit (0 = unlimited) if max_pages and pages_fetched >= max_pages: + truncated = bool(results.get('next')) break # Get next batch if available — throttle pagination to respect rate limits @@ -1893,8 +1895,17 @@ class SpotifyClient: (f" (page limit: {max_pages})" if max_pages else "")) # Cache the full artist albums result (wrapped in dict for cache compatibility) + # Only cache COMPLETE discographies. The cache key carries no + # limit/page info, so a partial probe (the watchlist's + # new-release check: limit=5, max_pages=1) stored here used to + # POISON the slot — the artist detail page then showed only + # the 5-10 newest releases for every watchlist artist until + # the 30-day TTL expired ("Taylor Swift has 8 albums, nothing + # before 2022"). Individual albums are still cached — they're + # complete entities regardless of how many pages we walked. if raw_items: - cache.store_entity('spotify', 'artist', cache_key, {'name': f'albums_{artist_id}', '_albums': raw_items}) + if not truncated: + cache.store_entity('spotify', 'artist', cache_key, {'name': f'albums_{artist_id}', '_albums': raw_items}) # 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_albums_cache_poisoning.py b/tests/test_artist_albums_cache_poisoning.py new file mode 100644 index 00000000..7e1d4c12 --- /dev/null +++ b/tests/test_artist_albums_cache_poisoning.py @@ -0,0 +1,84 @@ +"""Artist album-list cache poisoning (Boulder: 'Taylor Swift has 8 albums, +nothing before 2022'). + +get_artist_albums caches its result under an UNQUALIFIED key (no limit/page +info). The watchlist's new-release probe (limit=5, max_pages=1) stored its +truncated page in that slot, so the artist detail page — which reads the +cache — showed only the newest handful of releases for every watchlist +artist. The writer must never cache a fetch that stopped while more pages +existed; complete fetches (even small real discographies) stay cacheable. +""" + +from __future__ import annotations + +from unittest.mock import MagicMock + +import core.spotify_client as sc +from core.spotify_client import SpotifyClient + + +def _album(i): + return { + 'id': f'al{i}', 'name': f'Album {i}', 'album_type': 'album', + 'artists': [{'id': 'ar1', 'name': 'Taylor Swift'}], + 'release_date': '2024-01-01', 'total_tracks': 12, 'images': [], + 'external_urls': {}, + } + + +def _client(monkeypatch, pages): + """Fake sp.artist_albums + sp.next over a list of page dicts.""" + client = SpotifyClient.__new__(SpotifyClient) + fake = MagicMock() + fake.artist_albums.return_value = pages[0] + fake.next.side_effect = pages[1:] + client.sp = fake + monkeypatch.setattr(client, 'is_spotify_authenticated', lambda: True) + monkeypatch.setattr(sc, '_last_api_call_time', 0) + store_calls = [] + cache = MagicMock() + cache.get_entity.return_value = None + cache.store_entity.side_effect = lambda *a, **k: store_calls.append(a) + monkeypatch.setattr(sc, 'get_metadata_cache', lambda: cache) + return client, store_calls + + +def test_truncated_fetch_is_not_cached(monkeypatch): + # Two pages exist; max_pages=1 stops with a 'next' pending -> truncated. + pages = [ + {'items': [_album(1), _album(2)], 'next': 'page2-url'}, + {'items': [_album(3)], 'next': None}, + ] + client, stores = _client(monkeypatch, pages) + + albums = client.get_artist_albums('ar1', limit=5, skip_cache=True, max_pages=1) + + assert len(albums) == 2 # the probe still works + album_list_stores = [s for s in stores if s[1] == 'artist'] + assert album_list_stores == [] # but never poisons the slot + + +def test_complete_fetch_is_cached(monkeypatch): + pages = [ + {'items': [_album(1), _album(2)], 'next': 'page2-url'}, + {'items': [_album(3)], 'next': None}, + ] + client, stores = _client(monkeypatch, pages) + + albums = client.get_artist_albums('ar1', limit=50, skip_cache=True, max_pages=0) + + assert len(albums) == 3 + album_list_stores = [s for s in stores if s[1] == 'artist'] + assert len(album_list_stores) == 1 # full discography cached + + +def test_small_real_discography_with_page_cap_still_cached(monkeypatch): + # Artist genuinely has one page; max_pages=1 didn't truncate anything. + pages = [{'items': [_album(1)], 'next': None}] + client, stores = _client(monkeypatch, pages) + + albums = client.get_artist_albums('ar1', limit=5, skip_cache=True, max_pages=1) + + assert len(albums) == 1 + album_list_stores = [s for s in stores if s[1] == 'artist'] + assert len(album_list_stores) == 1 # complete -> cacheable diff --git a/web_server.py b/web_server.py index f2dad731..8e6542b6 100644 --- a/web_server.py +++ b/web_server.py @@ -35645,6 +35645,24 @@ def start_runtime_services(): logger.info("Starting OAuth callback servers...") start_oauth_callback_servers() + # One-time repair: purge artist album-list cache entries poisoned by + # partial watchlist probes (limit=5/max_pages=1 results stored in the + # full-discography slot — artist pages showed only the newest handful + # of releases for every watchlist artist). The writer is fixed; this + # clears what's already bad. Guarded so it runs once per install. + try: + if not config_manager.get('maintenance.album_cache_purge_v1', False): + from core.metadata.cache import get_metadata_cache as _gmc + _purged = _gmc().purge_artist_album_lists('spotify') + config_manager.set('maintenance.album_cache_purge_v1', True) + if _purged: + logger.warning( + "[Startup] Purged %d poisoned artist album-list cache " + "entries (partial watchlist probes); artist pages will " + "refetch full discographies lazily", _purged) + except Exception as _purge_err: + logger.debug("album cache purge skipped: %s", _purge_err) + # Startup diagnostics: Check and recover stuck flags logger.info("Running startup diagnostics...") stuck_flags_recovered = check_and_recover_stuck_flags()