Fix Spotify API calls leaking when Deezer/iTunes is primary source
Spotify was being called for album/artist data fetching across multiple background workers and the Artists page search even when the user had Deezer or iTunes set as their primary metadata source. Being authenticated for playlist sync was treated as permission to use Spotify for everything. - watchlist_scanner: add _spotify_is_primary_source() that checks both auth and primary source config; use it for all album/artist data fetching (discovery pool, recent album caching, playlist curation, similar artist ID matching, proactive ID backfill). _spotify_available_for_run() is kept for sync_spotify_library_cache which must run regardless of primary source - repair_jobs/metadata_gap_filler: gate Spotify ISRC lookup on primary source being 'spotify'; MusicBrainz lookup unaffected - repair_jobs/unknown_artist_fixer: replace hardcoded spotify_client with source-aware client selection — primary source ID tried first, each ID matched to its correct client (fixes latent bug passing Deezer IDs to Spotify) - web_server.py /api/match/search: Artists page search was hardcoded to spotify_client.search_artists(); now uses _get_metadata_fallback_client() so results come from the configured primary source
This commit is contained in:
parent
3618f3fa7f
commit
fe399636b2
5 changed files with 67 additions and 23 deletions
|
|
@ -2,6 +2,7 @@
|
||||||
|
|
||||||
import time
|
import time
|
||||||
|
|
||||||
|
from core.metadata_service import get_primary_source
|
||||||
from core.repair_jobs import register_job
|
from core.repair_jobs import register_job
|
||||||
from core.repair_jobs.base import JobContext, JobResult, RepairJob
|
from core.repair_jobs.base import JobContext, JobResult, RepairJob
|
||||||
from utils.logging_config import get_logger
|
from utils.logging_config import get_logger
|
||||||
|
|
@ -108,8 +109,10 @@ class MetadataGapFillerJob(RepairJob):
|
||||||
)
|
)
|
||||||
found_fields = {}
|
found_fields = {}
|
||||||
|
|
||||||
# Try Spotify enrichment first (most reliable for ISRC)
|
# Try Spotify enrichment for ISRC — only when Spotify is the configured primary source.
|
||||||
if spotify_track_id and context.spotify_client and not context.is_spotify_rate_limited():
|
# If Deezer/iTunes is primary, Spotify may still be authenticated for playlist sync
|
||||||
|
# but should not be called here, as it burns API quota unnecessarily.
|
||||||
|
if spotify_track_id and context.spotify_client and not context.is_spotify_rate_limited() and get_primary_source() == 'spotify':
|
||||||
try:
|
try:
|
||||||
track_data = context.spotify_client.get_track_details(spotify_track_id)
|
track_data = context.spotify_client.get_track_details(spotify_track_id)
|
||||||
if track_data:
|
if track_data:
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,7 @@ import shutil
|
||||||
import sys
|
import sys
|
||||||
import time
|
import time
|
||||||
|
|
||||||
|
from core.metadata_service import get_client_for_source, get_primary_client, get_primary_source
|
||||||
from core.repair_jobs import register_job
|
from core.repair_jobs import register_job
|
||||||
from core.repair_jobs.base import JobContext, JobResult, RepairJob
|
from core.repair_jobs.base import JobContext, JobResult, RepairJob
|
||||||
from utils.logging_config import get_logger
|
from utils.logging_config import get_logger
|
||||||
|
|
@ -268,12 +269,27 @@ class UnknownArtistFixerJob(RepairJob):
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.debug(f"Failed to read tags from {resolved_path}: {e}")
|
logger.debug(f"Failed to read tags from {resolved_path}: {e}")
|
||||||
|
|
||||||
# Priority 2: Look up by source track ID
|
# Priority 2: Look up by source track ID using the appropriate client.
|
||||||
source_id = (track.get('spotify_track_id') or track.get('deezer_track_id')
|
# Try the primary source's ID first, then fall back to any available ID
|
||||||
or track.get('itunes_track_id'))
|
# with its matching client so we never pass a Deezer/iTunes ID to Spotify
|
||||||
if source_id and context.spotify_client:
|
# (or vice-versa).
|
||||||
|
_primary = get_primary_source()
|
||||||
|
_id_candidates = []
|
||||||
|
for _src in [_primary] + [s for s in ('spotify', 'deezer', 'itunes') if s != _primary]:
|
||||||
|
_tid = track.get(f'{_src}_track_id')
|
||||||
|
if _tid:
|
||||||
|
_id_candidates.append((_src, _tid))
|
||||||
|
source_id = None
|
||||||
|
_lookup_client = None
|
||||||
|
for _src, _tid in _id_candidates:
|
||||||
|
_c = get_client_for_source(_src)
|
||||||
|
if _c:
|
||||||
|
source_id = _tid
|
||||||
|
_lookup_client = _c
|
||||||
|
break
|
||||||
|
if source_id and _lookup_client:
|
||||||
try:
|
try:
|
||||||
details = context.spotify_client.get_track_details(str(source_id))
|
details = _lookup_client.get_track_details(str(source_id))
|
||||||
if details and details.get('primary_artist'):
|
if details and details.get('primary_artist'):
|
||||||
artist = details['primary_artist']
|
artist = details['primary_artist']
|
||||||
if artist.lower() not in _UNKNOWN_NAMES:
|
if artist.lower() not in _UNKNOWN_NAMES:
|
||||||
|
|
@ -293,10 +309,11 @@ class UnknownArtistFixerJob(RepairJob):
|
||||||
except Exception as e:
|
except Exception as e:
|
||||||
logger.debug(f"Track ID lookup failed for {source_id}: {e}")
|
logger.debug(f"Track ID lookup failed for {source_id}: {e}")
|
||||||
|
|
||||||
# Priority 3: Search by title
|
# Priority 3: Search by title using the configured primary metadata source
|
||||||
if title and context.spotify_client:
|
_search_client = get_primary_client()
|
||||||
|
if title and _search_client:
|
||||||
try:
|
try:
|
||||||
results = context.spotify_client.search_tracks(title, limit=5)
|
results = _search_client.search_tracks(title, limit=5)
|
||||||
if results:
|
if results:
|
||||||
# Score candidates
|
# Score candidates
|
||||||
from difflib import SequenceMatcher
|
from difflib import SequenceMatcher
|
||||||
|
|
@ -319,7 +336,7 @@ class UnknownArtistFixerJob(RepairJob):
|
||||||
# Get full details for track_number
|
# Get full details for track_number
|
||||||
full_details = None
|
full_details = None
|
||||||
try:
|
try:
|
||||||
full_details = context.spotify_client.get_track_details(best.id)
|
full_details = _search_client.get_track_details(best.id)
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
album_data = full_details.get('album', {}) if full_details else {}
|
album_data = full_details.get('album', {}) if full_details else {}
|
||||||
|
|
|
||||||
|
|
@ -418,6 +418,26 @@ class WatchlistScanner:
|
||||||
return False
|
return False
|
||||||
return self.spotify_client.is_spotify_authenticated()
|
return self.spotify_client.is_spotify_authenticated()
|
||||||
|
|
||||||
|
def _spotify_is_primary_source(self) -> bool:
|
||||||
|
"""Check if Spotify is both authenticated and the configured primary metadata source.
|
||||||
|
|
||||||
|
Use this (not _spotify_available_for_run) when deciding whether to fetch
|
||||||
|
album/artist data from Spotify. Plain auth is not sufficient — the user
|
||||||
|
may have Spotify connected only for playlist sync while Deezer/iTunes
|
||||||
|
serves as the metadata source, and calling Spotify for data in that case
|
||||||
|
burns API quota unnecessarily.
|
||||||
|
|
||||||
|
_spotify_available_for_run() is still used for Spotify-specific features
|
||||||
|
(e.g. library-cache sync) that must run regardless of primary source.
|
||||||
|
"""
|
||||||
|
if not self._spotify_available_for_run():
|
||||||
|
return False
|
||||||
|
try:
|
||||||
|
from core.metadata_service import get_primary_source
|
||||||
|
return get_primary_source() == 'spotify'
|
||||||
|
except Exception:
|
||||||
|
return False
|
||||||
|
|
||||||
def _get_active_client_and_artist_id(self, watchlist_artist: WatchlistArtist):
|
def _get_active_client_and_artist_id(self, watchlist_artist: WatchlistArtist):
|
||||||
"""
|
"""
|
||||||
Get the appropriate client and artist ID based on active provider.
|
Get the appropriate client and artist ID based on active provider.
|
||||||
|
|
@ -627,7 +647,7 @@ class WatchlistScanner:
|
||||||
if self.spotify_client and self.spotify_client.is_rate_limited():
|
if self.spotify_client and self.spotify_client.is_rate_limited():
|
||||||
self._disable_spotify_for_run("global Spotify rate limit active")
|
self._disable_spotify_for_run("global Spotify rate limit active")
|
||||||
providers_to_backfill = ['itunes', 'deezer']
|
providers_to_backfill = ['itunes', 'deezer']
|
||||||
if self._spotify_available_for_run():
|
if self._spotify_is_primary_source():
|
||||||
providers_to_backfill.append('spotify')
|
providers_to_backfill.append('spotify')
|
||||||
try:
|
try:
|
||||||
from config.settings import config_manager as _cfg
|
from config.settings import config_manager as _cfg
|
||||||
|
|
@ -1764,8 +1784,8 @@ class WatchlistScanner:
|
||||||
searched_spotify_id = None
|
searched_spotify_id = None
|
||||||
searched_fallback_id = None
|
searched_fallback_id = None
|
||||||
try:
|
try:
|
||||||
# Try Spotify search
|
# Try Spotify search (only when Spotify is the configured primary source)
|
||||||
if self._spotify_available_for_run():
|
if self._spotify_is_primary_source():
|
||||||
searched_results = self.spotify_client.search_artists(artist_name, limit=1)
|
searched_results = self.spotify_client.search_artists(artist_name, limit=1)
|
||||||
if searched_results and len(searched_results) > 0:
|
if searched_results and len(searched_results) > 0:
|
||||||
searched_spotify_id = searched_results[0].id
|
searched_spotify_id = searched_results[0].id
|
||||||
|
|
@ -1801,8 +1821,8 @@ class WatchlistScanner:
|
||||||
'popularity': 0
|
'popularity': 0
|
||||||
}
|
}
|
||||||
|
|
||||||
# Try to match on Spotify
|
# Try to match on Spotify (only when Spotify is the configured primary source)
|
||||||
if self._spotify_available_for_run():
|
if self._spotify_is_primary_source():
|
||||||
try:
|
try:
|
||||||
spotify_results = self.spotify_client.search_artists(artist_name_to_match, limit=1)
|
spotify_results = self.spotify_client.search_artists(artist_name_to_match, limit=1)
|
||||||
if spotify_results and len(spotify_results) > 0:
|
if spotify_results and len(spotify_results) > 0:
|
||||||
|
|
@ -2013,7 +2033,7 @@ class WatchlistScanner:
|
||||||
logger.info("Populating discovery pool from similar artists...")
|
logger.info("Populating discovery pool from similar artists...")
|
||||||
|
|
||||||
# Determine which sources are available
|
# Determine which sources are available
|
||||||
spotify_available = self._spotify_available_for_run()
|
spotify_available = self._spotify_is_primary_source()
|
||||||
|
|
||||||
# Import fallback metadata client (iTunes or Deezer)
|
# Import fallback metadata client (iTunes or Deezer)
|
||||||
itunes_client, fallback_source = _get_fallback_metadata_client()
|
itunes_client, fallback_source = _get_fallback_metadata_client()
|
||||||
|
|
@ -2650,7 +2670,7 @@ class WatchlistScanner:
|
||||||
albums_checked = 0
|
albums_checked = 0
|
||||||
|
|
||||||
# Determine available sources
|
# Determine available sources
|
||||||
spotify_available = self._spotify_available_for_run()
|
spotify_available = self._spotify_is_primary_source()
|
||||||
|
|
||||||
# Get fallback metadata client (iTunes or Deezer)
|
# Get fallback metadata client (iTunes or Deezer)
|
||||||
itunes_client, fallback_source = _get_fallback_metadata_client()
|
itunes_client, fallback_source = _get_fallback_metadata_client()
|
||||||
|
|
@ -2885,7 +2905,7 @@ class WatchlistScanner:
|
||||||
f"{profile['avg_daily_plays']:.1f} avg daily plays")
|
f"{profile['avg_daily_plays']:.1f} avg daily plays")
|
||||||
|
|
||||||
# Determine available sources
|
# Determine available sources
|
||||||
spotify_available = self._spotify_available_for_run()
|
spotify_available = self._spotify_is_primary_source()
|
||||||
itunes_client, fallback_source = _get_fallback_metadata_client()
|
itunes_client, fallback_source = _get_fallback_metadata_client()
|
||||||
|
|
||||||
# Process each available source
|
# Process each available source
|
||||||
|
|
|
||||||
|
|
@ -15803,8 +15803,8 @@ def search_match():
|
||||||
artist_matches = hydrabase_client.search_artists(query, limit=8)
|
artist_matches = hydrabase_client.search_artists(query, limit=8)
|
||||||
provider = 'hydrabase'
|
provider = 'hydrabase'
|
||||||
else:
|
else:
|
||||||
artist_matches = spotify_client.search_artists(query, limit=8)
|
artist_matches = _get_metadata_fallback_client().search_artists(query, limit=8)
|
||||||
provider = _detect_provider(artist_matches, spotify_client)
|
provider = _get_metadata_fallback_source()
|
||||||
results = []
|
results = []
|
||||||
|
|
||||||
for artist in artist_matches:
|
for artist in artist_matches:
|
||||||
|
|
@ -15840,8 +15840,8 @@ def search_match():
|
||||||
if not artist_id:
|
if not artist_id:
|
||||||
return jsonify({"error": "Artist ID required for album search"}), 400
|
return jsonify({"error": "Artist ID required for album search"}), 400
|
||||||
# Get artist's albums and filter by query
|
# Get artist's albums and filter by query
|
||||||
album_matches = spotify_client.get_artist_albums(artist_id)
|
album_matches = _get_metadata_fallback_client().get_artist_albums(artist_id)
|
||||||
provider = _detect_provider(album_matches, spotify_client)
|
provider = _get_metadata_fallback_source()
|
||||||
|
|
||||||
results = []
|
results = []
|
||||||
for album in album_matches:
|
for album in album_matches:
|
||||||
|
|
|
||||||
|
|
@ -3600,6 +3600,10 @@ function closeHelperSearch() {
|
||||||
|
|
||||||
const WHATS_NEW = {
|
const WHATS_NEW = {
|
||||||
'2.2': [
|
'2.2': [
|
||||||
|
// --- April 15, 2026 ---
|
||||||
|
{ date: 'April 15, 2026' },
|
||||||
|
{ title: 'Fix Spotify API Leaking When Deezer/iTunes is Primary', desc: 'Spotify was being called for watchlist album scanning, similar artist discovery, repair jobs, and the Artists page search even when another source was set as primary. All data-fetching now respects the configured primary source. Spotify playlist sync is unaffected' },
|
||||||
|
|
||||||
// --- April 14, 2026 ---
|
// --- April 14, 2026 ---
|
||||||
{ date: 'April 14, 2026' },
|
{ date: 'April 14, 2026' },
|
||||||
{ title: 'Fix Staging Files Ignoring Path Template', desc: 'Files matched from the Staging folder were copied to the transfer root with their original filename instead of applying the configured path template. Post-processing now receives full artist/album context for staging matches' },
|
{ title: 'Fix Staging Files Ignoring Path Template', desc: 'Files matched from the Staging folder were copied to the transfer root with their original filename instead of applying the configured path template. Post-processing now receives full artist/album context for staging matches' },
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue