From 030374c5b0456b4f28c90eeed368a9723cffa46e Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 17 Apr 2026 08:05:56 +0300 Subject: [PATCH] Tune discovery fan-out and caching Reduce request volume in the discovery helpers while keeping the source-priority model intact. - make cache_discovery_recent_albums source-priority aware - cap Spotify artist-album pagination in the discovery and incremental paths - reduce the similar-artist sample size for the cache-refresh helper - keep Spotify strict where fallback would contaminate source-specific IDs - add regression coverage for source order, strict Spotify lookups, and pagination caps --- core/watchlist_scanner.py | 254 +++++++++++++++------------ tests/test_watchlist_scanner_scan.py | 109 ++++++++++++ 2 files changed, 249 insertions(+), 114 deletions(-) diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index 8e2defef..1b4f76ae 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -469,6 +469,15 @@ class WatchlistScanner: 'discogs': 'discogs_artist_id', }.get(source) + @staticmethod + def _similar_artist_id_attribute_for_source(source: str) -> Optional[str]: + """Return the similar-artist attribute that stores the given source ID.""" + return { + 'spotify': 'similar_artist_spotify_id', + 'itunes': 'similar_artist_itunes_id', + 'deezer': 'similar_artist_deezer_id', + }.get(source) + @staticmethod def _extract_entity_id(value: Any) -> Optional[str]: """Extract an ID from a dataclass, dict, or plain object.""" @@ -692,7 +701,9 @@ class WatchlistScanner: artist_id: str, album_type: str = 'album,single,ep', limit: int = 50, + # Only applies to Spotify currently skip_cache: bool = True, + # Only applies to Spotify currently max_pages: int = 0, ) -> List[Any]: """Fetch artist albums for a specific source, keeping Spotify strict.""" @@ -2956,17 +2967,14 @@ class WatchlistScanner: """ Cache recent albums from watchlist and similar artists for discover page. - Supports both Spotify and iTunes sources - iTunes is always processed (baseline), - Spotify is added when authenticated. Same pattern as discovery pool. + Uses the configured source priority and caches the first source that + can return albums for each artist. """ try: from datetime import datetime, timedelta logger.info("Caching recent albums for discover page...") - if self.spotify_client and self.spotify_client.is_rate_limited(): - self._disable_spotify_for_run("global Spotify rate limit active") - # Clear existing cache for this profile self.database.clear_discovery_recent_albums(profile_id=profile_id) @@ -2983,21 +2991,20 @@ class WatchlistScanner: except Exception: pass cutoff_date = datetime.now() - timedelta(days=days_lookback) - cached_count = {'spotify': 0, 'itunes': 0, 'deezer': 0} + discovery_sources = self._discovery_source_priority() + if not discovery_sources: + logger.warning("No music sources available to cache recent albums") + return + + cached_count = {source: 0 for source in discovery_sources} albums_checked = 0 - # Determine available sources - spotify_available = self._spotify_is_primary_source() - - # Get fallback metadata client (iTunes or Deezer) - itunes_client, fallback_source = _get_fallback_metadata_client() - # Get artists to check (scoped to profile) watchlist_artists = self.database.get_watchlist_artists(profile_id=profile_id) - similar_artists = self.database.get_top_similar_artists(limit=50, profile_id=profile_id) + # We only need a modest sample here; this path fans out into per-source album lookups. + similar_artists = self.database.get_top_similar_artists(limit=25, profile_id=profile_id) logger.info(f"Checking albums from {len(watchlist_artists)} watchlist + {len(similar_artists)} similar artists") - logger.info(f"Sources: Spotify={spotify_available}, {fallback_source}=True") def process_album(album, artist_name, artist_spotify_id, artist_itunes_id, source, artist_deezer_id=None): """Helper to process and cache a single album""" @@ -3043,118 +3050,137 @@ class WatchlistScanner: # Process watchlist artists for artist in watchlist_artists: - # Always process fallback source (iTunes or Deezer) as baseline - fallback_id = artist.itunes_artist_id if fallback_source == 'itunes' else artist.deezer_artist_id - if not fallback_id: - # Try to resolve fallback ID on-the-fly (with retry for rate limiting) - try: - results = itunes_api_call_with_retry( - itunes_client.search_artists, artist.artist_name, limit=1 - ) - if results and len(results) > 0: - fallback_id = results[0].id - fallback_resolved += 1 - logger.debug(f"[{fallback_source}] Resolved ID for {artist.artist_name}: {fallback_id}") - else: - fallback_failed_resolve += 1 - logger.info(f"[{fallback_source}] No artist found for: {artist.artist_name}") - except Exception as e: - fallback_failed_resolve += 1 - logger.info(f"[{fallback_source}] Failed to resolve {artist.artist_name}: {e}") + selected_source = None + selected_artist_id = None + selected_albums = [] + selected_watchlist_id = None - if fallback_id: - try: - albums = itunes_api_call_with_retry( - itunes_client.get_artist_albums, fallback_id, album_type='album,single,ep', limit=20 - ) - for album in albums or []: - process_album( - album, artist.artist_name, artist.spotify_artist_id, - fallback_id if fallback_source == 'itunes' else None, - fallback_source, - artist_deezer_id=fallback_id if fallback_source == 'deezer' else None - ) - except Exception as e: - logger.info(f"[{fallback_source}] Error fetching albums for {artist.artist_name}: {e}") + for source in discovery_sources: + source_attr = self._artist_id_attribute_for_source(source) + stored_id = getattr(artist, source_attr, None) if source_attr else None + cache_callback = None + if source == 'spotify': + cache_callback = lambda found_id, watchlist_id=artist.id: self._cache_watchlist_artist_source_id(artist, 'spotify', found_id) + elif source == 'itunes': + cache_callback = lambda found_id, watchlist_id=artist.id: self._cache_watchlist_artist_source_id(artist, 'itunes', found_id) + elif source == 'deezer': + cache_callback = lambda found_id, watchlist_id=artist.id: self._cache_watchlist_artist_source_id(artist, 'deezer', found_id) - # Process Spotify if authenticated - if spotify_available and artist.spotify_artist_id: - try: - albums = self.spotify_client.get_artist_albums( - artist.spotify_artist_id, - album_type='album,single,ep', - limit=20, - skip_cache=True, - max_pages=2, - ) - for album in albums or []: - process_album(album, artist.artist_name, artist.spotify_artist_id, fallback_id if fallback_source == 'itunes' else None, 'spotify') - except Exception as e: - logger.debug(f"Error fetching Spotify albums for {artist.artist_name}: {e}") + artist_id = self._resolve_artist_id_for_source( + source, + artist.artist_name, + stored_id=stored_id, + cache_callback=cache_callback, + ) + if not artist_id: + continue + + albums = self._get_artist_albums_for_source( + source, + artist_id, + album_type='album,single,ep', + limit=20, + skip_cache=True, + max_pages=2, + ) + if not albums: + logger.debug(f"No recent albums found for {artist.artist_name} on {source}") + continue + + selected_source = source + selected_artist_id = artist_id + selected_albums = albums + if source == 'spotify': + selected_watchlist_id = artist_id + elif source == 'itunes': + selected_watchlist_id = artist.itunes_artist_id or artist_id + elif source == 'deezer': + selected_watchlist_id = getattr(artist, 'deezer_artist_id', None) or artist_id + break + + if not selected_source or not selected_artist_id or not selected_albums: + time.sleep(DELAY_BETWEEN_ARTISTS) + continue + + for album in selected_albums: + process_album( + album, + artist.artist_name, + selected_watchlist_id if selected_source == 'spotify' else artist.spotify_artist_id, + selected_watchlist_id if selected_source == 'itunes' else None, + selected_source, + artist_deezer_id=selected_watchlist_id if selected_source == 'deezer' else None, + ) time.sleep(DELAY_BETWEEN_ARTISTS) # Process similar artists for artist in similar_artists: - # Always process fallback source (iTunes or Deezer) as baseline - fallback_id = artist.similar_artist_itunes_id if fallback_source == 'itunes' else getattr(artist, 'similar_artist_deezer_id', None) - if not fallback_id: - # Try to resolve fallback ID on-the-fly (with retry for rate limiting) - try: - results = itunes_api_call_with_retry( - itunes_client.search_artists, artist.similar_artist_name, limit=1 - ) - if results and len(results) > 0: - fallback_id = results[0].id - # Cache for future - if fallback_source == 'deezer': - self.database.update_similar_artist_deezer_id(artist.id, fallback_id) - else: - self.database.update_similar_artist_itunes_id(artist.id, fallback_id) - fallback_resolved += 1 - logger.debug(f"[{fallback_source}] Resolved ID for similar artist {artist.similar_artist_name}: {fallback_id}") - else: - fallback_failed_resolve += 1 - logger.info(f"[{fallback_source}] No artist found for similar: {artist.similar_artist_name}") - except Exception as e: - fallback_failed_resolve += 1 - logger.info(f"[{fallback_source}] Failed to resolve similar {artist.similar_artist_name}: {e}") + selected_source = None + selected_artist_id = None + selected_albums = [] + selected_similar_id = None - if fallback_id: - try: - albums = itunes_api_call_with_retry( - itunes_client.get_artist_albums, fallback_id, album_type='album,single,ep', limit=20 - ) - for album in albums or []: - process_album( - album, artist.similar_artist_name, artist.similar_artist_spotify_id, - fallback_id if fallback_source == 'itunes' else None, - fallback_source, - artist_deezer_id=fallback_id if fallback_source == 'deezer' else None - ) - except Exception as e: - logger.info(f"[{fallback_source}] Error fetching albums for similar {artist.similar_artist_name}: {e}") + for source in discovery_sources: + source_attr = self._similar_artist_id_attribute_for_source(source) + stored_id = getattr(artist, source_attr, None) if source_attr else None + cache_callback = None + if source == 'itunes': + cache_callback = lambda found_id, similar_id=artist.id: self.database.update_similar_artist_itunes_id(similar_id, found_id) + elif source == 'deezer': + cache_callback = lambda found_id, similar_id=artist.id: self.database.update_similar_artist_deezer_id(similar_id, found_id) - # Process Spotify if authenticated - if spotify_available and artist.similar_artist_spotify_id: - try: - albums = self.spotify_client.get_artist_albums( - artist.similar_artist_spotify_id, - album_type='album,single,ep', - limit=20, - skip_cache=True, - max_pages=2, - ) - for album in albums or []: - process_album(album, artist.similar_artist_name, artist.similar_artist_spotify_id, fallback_id if fallback_source == 'itunes' else None, 'spotify') - except Exception as e: - logger.debug(f"Error fetching Spotify albums for {artist.similar_artist_name}: {e}") + artist_id = self._resolve_artist_id_for_source( + source, + artist.similar_artist_name, + stored_id=stored_id, + cache_callback=cache_callback, + ) + if not artist_id: + continue + + albums = self._get_artist_albums_for_source( + source, + artist_id, + album_type='album,single,ep', + limit=20, + skip_cache=True, + max_pages=2, + ) + if not albums: + logger.debug(f"No recent albums found for similar {artist.similar_artist_name} on {source}") + continue + + selected_source = source + selected_artist_id = artist_id + selected_albums = albums + if source == 'spotify': + selected_similar_id = artist_id + elif source == 'itunes': + selected_similar_id = artist.similar_artist_itunes_id or artist_id + elif source == 'deezer': + selected_similar_id = getattr(artist, 'similar_artist_deezer_id', None) or artist_id + break + + if not selected_source or not selected_artist_id or not selected_albums: + time.sleep(DELAY_BETWEEN_ARTISTS) + continue + + for album in selected_albums: + process_album( + album, + artist.similar_artist_name, + selected_similar_id if selected_source == 'spotify' else artist.similar_artist_spotify_id, + selected_similar_id if selected_source == 'itunes' else None, + selected_source, + artist_deezer_id=selected_similar_id if selected_source == 'deezer' else None, + ) time.sleep(DELAY_BETWEEN_ARTISTS) - total_cached = cached_count['spotify'] + cached_count.get(fallback_source, 0) - logger.info(f"Cached {total_cached} recent albums (Spotify: {cached_count['spotify']}, {fallback_source}: {cached_count.get(fallback_source, 0)}) from {albums_checked} albums checked") - logger.info(f"[{fallback_source}] ID resolution stats: {fallback_resolved} resolved, {fallback_failed_resolve} failed") + total_cached = sum(cached_count.values()) + logger.info(f"Cached {total_cached} recent albums from {albums_checked} albums checked") + logger.info(f"Recent albums ID resolution stats: {fallback_resolved} resolved, {fallback_failed_resolve} failed") except Exception as e: logger.error(f"Error caching discovery recent albums: {e}") diff --git a/tests/test_watchlist_scanner_scan.py b/tests/test_watchlist_scanner_scan.py index 5cc12222..4916a937 100644 --- a/tests/test_watchlist_scanner_scan.py +++ b/tests/test_watchlist_scanner_scan.py @@ -125,6 +125,7 @@ class _FakeDB: self.similar_calls = [] self.discovery_pool_calls = [] self.discovery_pool_timestamp_calls = [] + self.discovery_recent_calls = [] self.db_albums = [] def get_watchlist_artists(self, profile_id=None): @@ -144,6 +145,13 @@ class _FakeDB: self.discovery_pool_calls.append((track_data, source, profile_id)) return True + def clear_discovery_recent_albums(self, profile_id=1): + return True + + def cache_discovery_recent_album(self, album_data, source='spotify', profile_id=1): + self.discovery_recent_calls.append((album_data, source, profile_id)) + return True + def cleanup_old_discovery_tracks(self, days_threshold=365): return 0 @@ -718,6 +726,107 @@ def test_populate_discovery_pool_uses_strict_spotify_for_database_album_search(m ) +def test_cache_discovery_recent_albums_uses_primary_source_first(monkeypatch): + monkeypatch.setattr(watchlist_scanner_module, "DELAY_BETWEEN_ARTISTS", 0) + monkeypatch.setattr(watchlist_scanner_module, "time", types.SimpleNamespace(sleep=lambda *_args, **_kwargs: None)) + monkeypatch.setattr(watchlist_scanner_module, "get_primary_source", lambda: "deezer") + monkeypatch.setattr(watchlist_scanner_module, "get_source_priority", lambda primary: [primary, "spotify", "itunes"]) + + artist = _build_artist("Artist One") + album = types.SimpleNamespace( + id="dz-album-1", + name="Recent Deezer Album", + album_type="album", + release_date="2026-04-01", + image_url="https://example.com/deezer-album.jpg", + ) + + deezer_client = _FakeSourceClient( + artist_id="dz-artist", + albums=[album], + image_url="https://example.com/deezer-artist.jpg", + ) + spotify_client = _FakeSourceClient( + artist_id="sp-artist", + albums=[types.SimpleNamespace(id="sp-album-1", name="Spotify Album", album_type="album")], + image_url="https://example.com/spotify-artist.jpg", + ) + + def fake_get_client_for_source(source): + return { + "deezer": deezer_client, + "spotify": spotify_client, + }.get(source) + + monkeypatch.setattr(watchlist_scanner_module, "get_client_for_source", fake_get_client_for_source) + + scanner = _build_scanner({"tracks": {"items": []}}, [artist]) + scanner.database.get_top_similar_artists = lambda limit=50, profile_id=1: [] + + scanner.cache_discovery_recent_albums(profile_id=1) + + assert scanner.database.discovery_recent_calls + assert scanner.database.discovery_recent_calls[0][1] == "deezer" + assert deezer_client.album_calls + assert spotify_client.search_calls == [] + assert spotify_client.album_calls == [] + + +def test_cache_discovery_recent_albums_falls_back_to_spotify_when_primary_has_no_albums(monkeypatch): + monkeypatch.setattr(watchlist_scanner_module, "DELAY_BETWEEN_ARTISTS", 0) + monkeypatch.setattr(watchlist_scanner_module, "time", types.SimpleNamespace(sleep=lambda *_args, **_kwargs: None)) + monkeypatch.setattr(watchlist_scanner_module, "get_primary_source", lambda: "deezer") + monkeypatch.setattr(watchlist_scanner_module, "get_source_priority", lambda primary: [primary, "spotify", "itunes"]) + + artist = _build_artist("Fallback Artist") + artist.spotify_artist_id = None + deezer_client = _FakeSourceClient( + artist_id="dz-artist", + albums=[], + image_url="https://example.com/deezer-artist.jpg", + ) + spotify_album = types.SimpleNamespace( + id="sp-album-1", + name="Spotify Recent Album", + album_type="album", + release_date="2026-04-01", + image_url="https://example.com/spotify-album.jpg", + ) + spotify_client = _FakeSourceClient( + artist_id="sp-artist", + albums=[spotify_album], + image_url="https://example.com/spotify-artist.jpg", + album_payload={ + "id": "sp-album-1", + "name": "Spotify Recent Album", + "images": [{"url": "https://example.com/spotify-album.jpg"}], + "release_date": "2026-04-01", + "popularity": 50, + "tracks": {"items": [{"id": "sp-track-1", "name": "Spotify Track", "artists": [{"name": "Fallback Artist"}]}]}, + "artists": [{"id": "sp-artist"}], + }, + ) + + def fake_get_client_for_source(source): + return { + "deezer": deezer_client, + "spotify": spotify_client, + }.get(source) + + monkeypatch.setattr(watchlist_scanner_module, "get_client_for_source", fake_get_client_for_source) + + scanner = _build_scanner({"tracks": {"items": []}}, [artist]) + scanner.database.get_top_similar_artists = lambda limit=50, profile_id=1: [] + + scanner.cache_discovery_recent_albums(profile_id=1) + + assert scanner.database.discovery_recent_calls + assert scanner.database.discovery_recent_calls[0][1] == "spotify" + assert deezer_client.album_calls + assert spotify_client.search_calls == [("Fallback Artist", 1, {"allow_fallback": False})] + assert spotify_client.album_calls + + def test_match_to_spotify_uses_strict_lookup(): spotify_client = _FakeSpotifyClient( search_results=[types.SimpleNamespace(id="fallback-id", name="Artist One")]