From 8382b8e24715f27d3b70b33b0ae2946869bb6abd Mon Sep 17 00:00:00 2001 From: Antti Kettunen Date: Fri, 17 Apr 2026 09:33:01 +0300 Subject: [PATCH] Refactor similar artist backfill Switch similar-artist backfill to the shared provider-priority flow instead of assuming iTunes as the fallback. Reuse the generic metadata search helpers, keep a compatibility alias for the old helper name, and update the scanner tests to cover the new path. Add a regression test that verifies backfill walks each available fallback provider and persists the resolved IDs per source. --- core/watchlist_scanner.py | 163 +++++++++++---------------- tests/test_watchlist_scanner_scan.py | 54 ++++++++- 2 files changed, 115 insertions(+), 102 deletions(-) diff --git a/core/watchlist_scanner.py b/core/watchlist_scanner.py index c293ab2d..bf062be6 100644 --- a/core/watchlist_scanner.py +++ b/core/watchlist_scanner.py @@ -30,63 +30,6 @@ DELAY_BETWEEN_ARTISTS = 4.0 # 4 seconds between different artists (was 2s, DELAY_BETWEEN_ALBUMS = 0.5 # 500ms between albums for same artist DELAY_BETWEEN_API_BATCHES = 1.0 # 1 second between API batch operations -# iTunes API retry configuration -ITUNES_MAX_RETRIES = 3 -ITUNES_BASE_DELAY = 1.0 # Base delay in seconds for exponential backoff - - -def _get_fallback_metadata_client(): - """Get the configured metadata client — delegates to centralized metadata_service.""" - from core.metadata_service import get_primary_source, get_primary_client - return get_primary_client(), get_primary_source() - - -def itunes_api_call_with_retry(func, *args, max_retries=ITUNES_MAX_RETRIES, **kwargs): - """ - Execute an iTunes API call with exponential backoff retry logic. - - Args: - func: The function to call - *args: Arguments to pass to the function - max_retries: Maximum number of retry attempts - **kwargs: Keyword arguments to pass to the function - - Returns: - The result of the function call, or None if all retries failed - """ - last_error = None - for attempt in range(max_retries): - try: - result = func(*args, **kwargs) - return result - except requests.exceptions.HTTPError as e: - # Handle rate limiting (429) and server errors (5xx) - if e.response is not None and e.response.status_code == 429: - delay = ITUNES_BASE_DELAY * (2 ** attempt) - logger.warning(f"[iTunes] Rate limited, retrying in {delay}s (attempt {attempt + 1}/{max_retries})") - time.sleep(delay) - last_error = e - elif e.response is not None and e.response.status_code >= 500: - delay = ITUNES_BASE_DELAY * (2 ** attempt) - logger.warning(f"[iTunes] Server error {e.response.status_code}, retrying in {delay}s (attempt {attempt + 1}/{max_retries})") - time.sleep(delay) - last_error = e - else: - raise # Don't retry on client errors (4xx except 429) - except requests.exceptions.RequestException as e: - # Retry on connection errors - delay = ITUNES_BASE_DELAY * (2 ** attempt) - logger.warning(f"[iTunes] Connection error, retrying in {delay}s (attempt {attempt + 1}/{max_retries}): {e}") - time.sleep(delay) - last_error = e - except Exception as e: - # Don't retry on other exceptions - raise - - if last_error: - logger.error(f"[iTunes] All {max_retries} retry attempts failed: {last_error}") - return None - def clean_track_name_for_search(track_name): """ @@ -1315,7 +1258,7 @@ class WatchlistScanner: spotify_authenticated = self.spotify_client and self.spotify_client.is_spotify_authenticated() if self.database.has_fresh_similar_artists(source_artist_id, days_threshold=30, require_spotify=spotify_authenticated, profile_id=artist_profile_id): logger.info("Similar artists for %s are cached and fresh (profile %s)", artist.artist_name, artist_profile_id) - self._backfill_similar_artists_itunes_ids(source_artist_id, profile_id=artist_profile_id) + self._backfill_similar_artists_fallback_ids(source_artist_id, profile_id=artist_profile_id) else: logger.info("Fetching similar artists for %s (profile %s)...", artist.artist_name, artist_profile_id) self.update_similar_artists(artist, profile_id=artist_profile_id, source_artist_id=source_artist_id) @@ -2313,57 +2256,79 @@ class WatchlistScanner: logger.error(f"Error fetching similar artists from MusicMap: {e}") return [] - def _backfill_similar_artists_itunes_ids(self, source_artist_id: str, profile_id: int = 1) -> int: - """ - Backfill missing iTunes IDs for cached similar artists. - This ensures seamless dual-source support without clearing cached data. + def _update_similar_artist_source_id(self, similar_artist_id: int, source: str, source_id: str) -> bool: + """Persist a resolved similar-artist ID for a supported source.""" + if source == 'deezer': + return self.database.update_similar_artist_deezer_id(similar_artist_id, source_id) + if source == 'itunes': + return self.database.update_similar_artist_itunes_id(similar_artist_id, source_id) + return False - Args: - source_artist_id: The source artist ID to backfill similar artists for - profile_id: Profile to scope the backfill to - - Returns: - Number of similar artists updated with iTunes IDs + def _backfill_similar_artists_fallback_ids(self, source_artist_id: str, profile_id: int = 1) -> int: """ + Backfill missing fallback-provider IDs for cached similar artists. + + Uses the configured source priority, filtered to providers that have + writable similar-artist ID columns. This keeps old cached rows usable + when the active metadata provider changes. + """ + backfill_sources = [source for source in self._discovery_source_priority() if source in {'itunes', 'deezer'}] + if not backfill_sources: + logger.debug("No fallback metadata providers available for similar-artist backfill") + return 0 + + updated_total = 0 + try: - # Get fallback metadata client (iTunes or Deezer) - fallback_client, fallback_source = _get_fallback_metadata_client() - - # Get similar artists that are missing IDs for the active fallback source - similar_artists = self.database.get_similar_artists_missing_fallback_ids(source_artist_id, fallback_source, profile_id=profile_id) - - if not similar_artists: - return 0 - - logger.info(f"Backfilling {fallback_source} IDs for {len(similar_artists)} similar artists") - - updated_count = 0 - for similar_artist in similar_artists: - try: - results = fallback_client.search_artists(similar_artist.similar_artist_name, limit=1) - if results and len(results) > 0: - found_id = results[0].id - # Update the similar artist with the correct source ID - if fallback_source == 'deezer': - success = self.database.update_similar_artist_deezer_id(similar_artist.id, found_id) - else: - success = self.database.update_similar_artist_itunes_id(similar_artist.id, found_id) - if success: - updated_count += 1 - logger.debug(f" Backfilled {fallback_source} ID {found_id} for {similar_artist.similar_artist_name}") - except Exception as e: - logger.debug(f" Could not backfill {fallback_source} ID for {similar_artist.similar_artist_name}: {e}") + for source in backfill_sources: + client = get_client_for_source(source) + if not client: + logger.debug("Skipping %s similar-artist backfill - client unavailable", source) continue - if updated_count > 0: - logger.info(f"Backfilled {updated_count} similar artists with {fallback_source} IDs") + similar_artists = self.database.get_similar_artists_missing_fallback_ids( + source_artist_id, + source, + profile_id=profile_id, + ) + if not similar_artists: + continue - return updated_count + logger.info("Backfilling %s IDs for %s similar artists", source, len(similar_artists)) + + updated_count = 0 + for similar_artist in similar_artists: + try: + results = self._search_artists_for_source(source, similar_artist.similar_artist_name, limit=1, client=client) + if not results: + continue + + found_id = self._extract_entity_id(results[0]) + if not found_id: + continue + + success = self._update_similar_artist_source_id(similar_artist.id, source, found_id) + if success: + updated_count += 1 + updated_total += 1 + logger.debug(" Backfilled %s ID %s for %s", source, found_id, similar_artist.similar_artist_name) + except Exception as e: + logger.debug(" Could not backfill %s ID for %s: %s", source, similar_artist.similar_artist_name, e) + continue + + if updated_count > 0: + logger.info("Backfilled %s similar artists with %s IDs", updated_count, source) + + return updated_total except Exception as e: - logger.error(f"Error backfilling similar artists {fallback_source if 'fallback_source' in dir() else 'fallback'} IDs: {e}") + logger.error("Error backfilling similar artists IDs: %s", e) return 0 + def _backfill_similar_artists_itunes_ids(self, source_artist_id: str, profile_id: int = 1) -> int: + """Backward-compatible alias for the provider-priority backfill path.""" + return self._backfill_similar_artists_fallback_ids(source_artist_id, profile_id=profile_id) + def update_similar_artists( self, watchlist_artist: WatchlistArtist, diff --git a/tests/test_watchlist_scanner_scan.py b/tests/test_watchlist_scanner_scan.py index bfffe188..10746b09 100644 --- a/tests/test_watchlist_scanner_scan.py +++ b/tests/test_watchlist_scanner_scan.py @@ -296,6 +296,54 @@ def test_fetch_similar_artists_from_musicmap_uses_provider_priority(monkeypatch) assert spotify_client.search_calls[-1][2]["allow_fallback"] is False +def test_backfill_similar_artists_fallback_ids_uses_provider_priority(monkeypatch): + def make_client(source): + client = types.SimpleNamespace(search_calls=[]) + + def search_artists(query, limit=1, **kwargs): + client.search_calls.append((query, limit, kwargs)) + safe_name = query.lower().replace(" ", "-") + return [types.SimpleNamespace(id=f"{source}-{safe_name}", name=query)] + + client.search_artists = search_artists + return client + + deezer_client = make_client("deezer") + itunes_client = make_client("itunes") + + deezer_artist = types.SimpleNamespace(id=11, similar_artist_name="Deezer Artist") + itunes_artist = types.SimpleNamespace(id=22, similar_artist_name="iTunes Artist") + + monkeypatch.setattr(watchlist_scanner_module, "get_primary_source", lambda: "deezer") + monkeypatch.setattr( + watchlist_scanner_module, + "get_client_for_source", + lambda source: { + "deezer": deezer_client, + "itunes": itunes_client, + }.get(source), + ) + + scanner = _build_scanner({"tracks": {"items": []}}, []) + scanner.database.get_similar_artists_missing_fallback_ids = ( + lambda source_artist_id, fallback_source, profile_id=1: [deezer_artist] if fallback_source == "deezer" else [itunes_artist] + ) + + update_calls = [] + scanner.database.update_similar_artist_deezer_id = lambda similar_artist_id, deezer_id: update_calls.append(("deezer", similar_artist_id, deezer_id)) or True + scanner.database.update_similar_artist_itunes_id = lambda similar_artist_id, itunes_id: update_calls.append(("itunes", similar_artist_id, itunes_id)) or True + + count = scanner._backfill_similar_artists_fallback_ids("source-artist", profile_id=7) + + assert count == 2 + assert update_calls == [ + ("deezer", 11, "deezer-deezer-artist"), + ("itunes", 22, "itunes-itunes-artist"), + ] + assert [call[0] for call in deezer_client.search_calls] == ["Deezer Artist"] + assert [call[0] for call in itunes_client.search_calls] == ["iTunes Artist"] + + def test_scan_watchlist_profile_loads_artists_and_applies_overrides(monkeypatch): artist = _build_artist() scanner = _build_scanner({"tracks": {"items": []}}, [artist]) @@ -352,7 +400,7 @@ def test_scan_watchlist_artists_scans_tracks_and_updates_state(monkeypatch): monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: True) monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True) monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True) - monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0) + monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0) scan_state = {} results = scanner.scan_watchlist_artists([artist], scan_state=scan_state) @@ -412,7 +460,7 @@ def test_scan_watchlist_artists_skips_placeholder_tracklists(monkeypatch): monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *args, **kwargs: add_calls.append((args, kwargs)) or True) monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True) monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True) - monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0) + monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0) scan_state = {} results = scanner.scan_watchlist_artists([artist], scan_state=scan_state) @@ -451,7 +499,7 @@ def test_scan_watchlist_artists_honors_cancel_check(monkeypatch): monkeypatch.setattr(scanner, "add_track_to_wishlist", lambda *_args, **_kwargs: False) monkeypatch.setattr(scanner, "update_artist_scan_timestamp", lambda *_args, **_kwargs: True) monkeypatch.setattr(scanner, "update_similar_artists", lambda *_args, **_kwargs: True) - monkeypatch.setattr(scanner, "_backfill_similar_artists_itunes_ids", lambda *_args, **_kwargs: 0) + monkeypatch.setattr(scanner, "_backfill_similar_artists_fallback_ids", lambda *_args, **_kwargs: 0) cancels = iter([False, True]) scan_state = {}