Centralize metadata source selection in core/metadata_service.py
All metadata source decisions now flow through get_primary_source() and
get_primary_client() in core/metadata_service.py. Previously 6 different
files reimplemented this logic with inconsistent defaults ('itunes' vs
'deezer') and auth checks, causing bugs when any one was missed.
Changes:
- metadata_service.py: Added canonical get_primary_source/get_primary_client
- web_server.py: _get_metadata_fallback_source() and _get_active_discovery_source()
are now thin wrappers delegating to metadata_service
- seasonal_discovery.py: _get_source() delegates to metadata_service
- personalized_playlists.py: _get_active_source() delegates to metadata_service
- spotify_client.py: Fixed _fallback_source default from 'itunes' to 'deezer'
- watchlist_scanner.py: _get_fallback_metadata_client() delegates to metadata_service
Future changes to source selection only need to update one file.
This commit is contained in:
parent
52a5d93018
commit
498c22e7c3
6 changed files with 110 additions and 65 deletions
|
|
@ -1,9 +1,10 @@
|
||||||
"""
|
"""
|
||||||
Metadata Service - Hot-swappable Spotify/iTunes/Deezer provider
|
Metadata Service - Centralized metadata source selection
|
||||||
|
|
||||||
Automatically uses Spotify when authenticated, falls back to the configured
|
ALL metadata source decisions flow through this module. Other files import
|
||||||
fallback source (iTunes or Deezer) when not.
|
get_primary_source() and get_primary_client() instead of reimplementing
|
||||||
Provides unified interface for all metadata operations.
|
the logic. This prevents bugs where different files have different defaults
|
||||||
|
or auth checks.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from typing import List, Optional, Dict, Any, Literal
|
from typing import List, Optional, Dict, Any, Literal
|
||||||
|
|
@ -16,37 +17,106 @@ logger = get_logger("metadata_service")
|
||||||
MetadataProvider = Literal["spotify", "itunes", "auto"]
|
MetadataProvider = Literal["spotify", "itunes", "auto"]
|
||||||
|
|
||||||
|
|
||||||
def _get_configured_fallback_source():
|
# =============================================================================
|
||||||
"""Get the configured metadata fallback source ('itunes' or 'deezer')."""
|
# CANONICAL SOURCE SELECTION — all code should use these two functions
|
||||||
|
# =============================================================================
|
||||||
|
|
||||||
|
def get_primary_source() -> str:
|
||||||
|
"""Get the user's configured primary metadata source.
|
||||||
|
|
||||||
|
Returns 'spotify', 'deezer', 'itunes', 'discogs', or 'hydrabase'.
|
||||||
|
If the user selected Spotify but it's not authenticated, falls back to 'deezer'.
|
||||||
|
|
||||||
|
This is THE single source of truth for "which metadata source should I use?"
|
||||||
|
All other modules should import this function instead of reading config directly.
|
||||||
|
"""
|
||||||
try:
|
try:
|
||||||
from config.settings import config_manager
|
from config.settings import config_manager
|
||||||
return config_manager.get('metadata.fallback_source', 'itunes') or 'itunes'
|
source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer'
|
||||||
except Exception:
|
except Exception:
|
||||||
return 'itunes'
|
return 'deezer'
|
||||||
|
|
||||||
|
# Validate Spotify selection — can't use it if not authenticated
|
||||||
|
if source == 'spotify':
|
||||||
|
try:
|
||||||
|
import importlib
|
||||||
|
ws = importlib.import_module('web_server')
|
||||||
|
sc = getattr(ws, 'spotify_client', None)
|
||||||
|
if not sc or not sc.is_spotify_authenticated():
|
||||||
|
return 'deezer'
|
||||||
|
except Exception:
|
||||||
|
return 'deezer'
|
||||||
|
|
||||||
|
return source
|
||||||
|
|
||||||
|
|
||||||
def _create_fallback_client():
|
def get_primary_client():
|
||||||
"""Create the configured fallback metadata client."""
|
"""Get the client object for the user's configured primary metadata source.
|
||||||
source = _get_configured_fallback_source()
|
|
||||||
|
Returns a SpotifyClient, DeezerClient, iTunesClient, DiscogsClient,
|
||||||
|
or HydrabaseClient instance.
|
||||||
|
|
||||||
|
This is THE single source of truth for "which client should I call?"
|
||||||
|
"""
|
||||||
|
source = get_primary_source()
|
||||||
|
|
||||||
|
if source == 'spotify':
|
||||||
|
try:
|
||||||
|
import importlib
|
||||||
|
ws = importlib.import_module('web_server')
|
||||||
|
sc = getattr(ws, 'spotify_client', None)
|
||||||
|
if sc and sc.is_spotify_authenticated():
|
||||||
|
return sc
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
# Spotify selected but unavailable — fall back to Deezer
|
||||||
|
from core.deezer_client import DeezerClient
|
||||||
|
return DeezerClient()
|
||||||
|
|
||||||
if source == 'deezer':
|
if source == 'deezer':
|
||||||
from core.deezer_client import DeezerClient
|
from core.deezer_client import DeezerClient
|
||||||
return DeezerClient()
|
return DeezerClient()
|
||||||
|
|
||||||
|
if source == 'discogs':
|
||||||
|
try:
|
||||||
|
from config.settings import config_manager
|
||||||
|
token = config_manager.get('discogs.token', '')
|
||||||
|
if token:
|
||||||
|
from core.discogs_client import DiscogsClient
|
||||||
|
return DiscogsClient(token=token)
|
||||||
|
except Exception:
|
||||||
|
pass
|
||||||
|
return iTunesClient()
|
||||||
|
|
||||||
if source == 'hydrabase':
|
if source == 'hydrabase':
|
||||||
try:
|
try:
|
||||||
from core.hydrabase_client import HydrabaseClient
|
|
||||||
# Hydrabase client is managed globally — try to import the running instance
|
|
||||||
import importlib
|
import importlib
|
||||||
ws_module = importlib.import_module('web_server')
|
ws = importlib.import_module('web_server')
|
||||||
client = getattr(ws_module, 'hydrabase_client', None)
|
client = getattr(ws, 'hydrabase_client', None)
|
||||||
if client and client.is_connected():
|
if client and client.is_connected():
|
||||||
return client
|
return client
|
||||||
except Exception:
|
except Exception:
|
||||||
pass
|
pass
|
||||||
# Hydrabase not available — fall back to iTunes
|
|
||||||
return iTunesClient()
|
return iTunesClient()
|
||||||
|
|
||||||
|
# Default: iTunes
|
||||||
return iTunesClient()
|
return iTunesClient()
|
||||||
|
|
||||||
|
|
||||||
|
# =============================================================================
|
||||||
|
# LEGACY ALIASES — kept for backward compatibility, delegate to canonical funcs
|
||||||
|
# =============================================================================
|
||||||
|
|
||||||
|
def _get_configured_fallback_source():
|
||||||
|
"""Legacy alias for get_primary_source(). Use get_primary_source() instead."""
|
||||||
|
return get_primary_source()
|
||||||
|
|
||||||
|
|
||||||
|
def _create_fallback_client():
|
||||||
|
"""Legacy alias for get_primary_client(). Use get_primary_client() instead."""
|
||||||
|
return get_primary_client()
|
||||||
|
|
||||||
|
|
||||||
class MetadataService:
|
class MetadataService:
|
||||||
"""
|
"""
|
||||||
Unified metadata service that seamlessly switches between Spotify and
|
Unified metadata service that seamlessly switches between Spotify and
|
||||||
|
|
@ -94,10 +164,8 @@ class MetadataService:
|
||||||
return "spotify"
|
return "spotify"
|
||||||
elif self.preferred_provider == "itunes":
|
elif self.preferred_provider == "itunes":
|
||||||
return self._fallback_source
|
return self._fallback_source
|
||||||
else: # auto
|
else: # auto — use the centralized source selection
|
||||||
# Use is_spotify_authenticated() to check actual Spotify auth status
|
return get_primary_source()
|
||||||
# (is_authenticated() always returns True due to fallback)
|
|
||||||
return "spotify" if self.spotify.is_spotify_authenticated() else self._fallback_source
|
|
||||||
|
|
||||||
def _get_client(self):
|
def _get_client(self):
|
||||||
"""Get the appropriate client based on provider selection"""
|
"""Get the appropriate client based on provider selection"""
|
||||||
|
|
|
||||||
|
|
@ -101,17 +101,9 @@ class PersonalizedPlaylistsService:
|
||||||
self.spotify_client = spotify_client
|
self.spotify_client = spotify_client
|
||||||
|
|
||||||
def _get_active_source(self) -> str:
|
def _get_active_source(self) -> str:
|
||||||
"""
|
"""Determine which music source is active — delegates to centralized metadata_service."""
|
||||||
Determine which music source is active — respects user's configured primary source.
|
from core.metadata_service import get_primary_source
|
||||||
"""
|
return get_primary_source()
|
||||||
try:
|
|
||||||
from config.settings import config_manager
|
|
||||||
source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer'
|
|
||||||
if source == 'spotify' and not (self.spotify_client and hasattr(self.spotify_client, 'is_spotify_authenticated') and self.spotify_client.is_spotify_authenticated()):
|
|
||||||
return 'deezer'
|
|
||||||
return source
|
|
||||||
except Exception:
|
|
||||||
return 'deezer'
|
|
||||||
|
|
||||||
def _build_track_dict(self, row, source: str) -> Dict:
|
def _build_track_dict(self, row, source: str) -> Dict:
|
||||||
"""Build a standardized track dictionary from a database row."""
|
"""Build a standardized track dictionary from a database row."""
|
||||||
|
|
|
||||||
|
|
@ -96,16 +96,9 @@ class SeasonalDiscoveryService:
|
||||||
self._ensure_database_schema()
|
self._ensure_database_schema()
|
||||||
|
|
||||||
def _get_source(self):
|
def _get_source(self):
|
||||||
"""Determine active music source — respects user's configured primary source"""
|
"""Determine active music source — delegates to centralized metadata_service."""
|
||||||
try:
|
from core.metadata_service import get_primary_source
|
||||||
from config.settings import config_manager
|
return get_primary_source()
|
||||||
source = config_manager.get('metadata.fallback_source', 'deezer') or 'deezer'
|
|
||||||
# If user selected spotify, verify it's actually authenticated
|
|
||||||
if source == 'spotify' and not (self.spotify_client and self.spotify_client.is_spotify_authenticated()):
|
|
||||||
return 'deezer'
|
|
||||||
return source
|
|
||||||
except Exception:
|
|
||||||
return 'deezer'
|
|
||||||
|
|
||||||
def _ensure_database_schema(self):
|
def _ensure_database_schema(self):
|
||||||
"""Create seasonal content tables if they don't exist"""
|
"""Create seasonal content tables if they don't exist"""
|
||||||
|
|
|
||||||
|
|
@ -506,11 +506,11 @@ class SpotifyClient:
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def _fallback_source(self) -> str:
|
def _fallback_source(self) -> str:
|
||||||
"""Get configured metadata fallback source ('itunes', 'deezer', or 'discogs')"""
|
"""Get configured primary metadata source for internal fallback routing."""
|
||||||
try:
|
try:
|
||||||
return config_manager.get('metadata.fallback_source', 'itunes') or 'itunes'
|
return config_manager.get('metadata.fallback_source', 'deezer') or 'deezer'
|
||||||
except Exception:
|
except Exception:
|
||||||
return 'itunes'
|
return 'deezer'
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def _fallback(self):
|
def _fallback(self):
|
||||||
|
|
|
||||||
|
|
@ -30,18 +30,9 @@ ITUNES_BASE_DELAY = 1.0 # Base delay in seconds for exponential backoff
|
||||||
|
|
||||||
|
|
||||||
def _get_fallback_metadata_client():
|
def _get_fallback_metadata_client():
|
||||||
"""Get the configured metadata fallback client (iTunes or Deezer)."""
|
"""Get the configured metadata client — delegates to centralized metadata_service."""
|
||||||
try:
|
from core.metadata_service import get_primary_source, get_primary_client
|
||||||
from config.settings import config_manager
|
return get_primary_client(), get_primary_source()
|
||||||
source = config_manager.get('metadata.fallback_source', 'itunes') or 'itunes'
|
|
||||||
if source == 'deezer':
|
|
||||||
from core.deezer_client import DeezerClient
|
|
||||||
return DeezerClient(), 'deezer'
|
|
||||||
from core.itunes_client import iTunesClient
|
|
||||||
return iTunesClient(), 'itunes'
|
|
||||||
except Exception:
|
|
||||||
from core.itunes_client import iTunesClient
|
|
||||||
return iTunesClient(), 'itunes'
|
|
||||||
|
|
||||||
|
|
||||||
def itunes_api_call_with_retry(func, *args, max_retries=ITUNES_MAX_RETRIES, **kwargs):
|
def itunes_api_call_with_retry(func, *args, max_retries=ITUNES_MAX_RETRIES, **kwargs):
|
||||||
|
|
|
||||||
|
|
@ -33369,11 +33369,12 @@ def _get_deezer_client():
|
||||||
|
|
||||||
def _get_metadata_fallback_source():
|
def _get_metadata_fallback_source():
|
||||||
"""Get the configured primary metadata source.
|
"""Get the configured primary metadata source.
|
||||||
Returns 'spotify', 'itunes', 'deezer', 'discogs', or 'hydrabase'."""
|
Returns 'spotify', 'itunes', 'deezer', 'discogs', or 'hydrabase'.
|
||||||
try:
|
|
||||||
return config_manager.get('metadata.fallback_source', 'deezer') or 'deezer'
|
NOTE: This is a thin wrapper — canonical logic lives in core.metadata_service.get_primary_source().
|
||||||
except Exception:
|
Kept as a local function because 70+ callers reference it by name."""
|
||||||
return 'deezer'
|
from core.metadata_service import get_primary_source
|
||||||
|
return get_primary_source()
|
||||||
|
|
||||||
def _get_metadata_fallback_client():
|
def _get_metadata_fallback_client():
|
||||||
"""Get the active metadata client based on settings.
|
"""Get the active metadata client based on settings.
|
||||||
|
|
@ -40240,11 +40241,11 @@ def _get_active_discovery_source():
|
||||||
Determine which music source is active for discovery.
|
Determine which music source is active for discovery.
|
||||||
Returns the user's configured primary metadata source.
|
Returns the user's configured primary metadata source.
|
||||||
If the selected source requires auth and isn't available, falls back.
|
If the selected source requires auth and isn't available, falls back.
|
||||||
|
|
||||||
|
NOTE: Thin wrapper — canonical logic lives in core.metadata_service.get_primary_source().
|
||||||
"""
|
"""
|
||||||
source = _get_metadata_fallback_source()
|
from core.metadata_service import get_primary_source
|
||||||
if source == 'spotify' and not (spotify_client and spotify_client.is_spotify_authenticated()):
|
return get_primary_source()
|
||||||
return 'deezer'
|
|
||||||
return source
|
|
||||||
|
|
||||||
|
|
||||||
@app.route('/api/discover/hero', methods=['GET'])
|
@app.route('/api/discover/hero', methods=['GET'])
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue