#798: Spotify Free as a real dropdown source + automatic rate-limit bridge
Consistency fix: Spotify Free is now its own entry in the metadata-source dropdown (alongside Spotify / iTunes / Deezer / MusicBrainz) instead of a side-toggle. Stored as fallback_source='spotify' + spotify_free=true so all downstream 'spotify' routing and the spotify_* columns are unchanged. Refined gate model (no toggle): - Connected user (has credentials) -> official; bridges to free AUTOMATICALLY during a rate-limit ban (no opt-in needed). - No-auth user -> must pick 'Spotify Free' in the dropdown; then free serves. - Never opted into Spotify (no creds, didn't pick it) -> free never runs, so no surprise scraping. _free_wanted() = has_credentials OR picked-spotify-free is the guard. - AUTHED + healthy -> official always; free never opens. UI: dropdown gains 'Spotify Free (no credentials)' (selectable when the package is installed — surfaced via status.free_installed, since selecting it is the opt-in and can't depend on having selected it); load/save map the dropdown value to the (fallback_source, spotify_free) pair; old checkbox removed. Gate model pinned by 6 scenario tests (connected/healthy, connected/ratelimited bridge, no-auth picked, no-auth not-opted-in, package-missing). 117 tests green.
This commit is contained in:
BoulderBadgeDad
parent
a387814deb
commit
217a5eda70
5 changed files with 126 additions and 36 deletions
|
|
@ -569,26 +569,43 @@ class SpotifyClient:
|
|||
return self._itunes # Fall back to iTunes if no Discogs token
|
||||
return self._itunes
|
||||
|
||||
def _free_enabled(self) -> bool:
|
||||
"""Opt-in switch for the no-creds SpotipyFree ('Spotify Free') source.
|
||||
Default OFF — the user explicitly enables it (Settings), so there's no
|
||||
surprise web-scraping for people who simply haven't connected Spotify."""
|
||||
def _free_selected(self) -> bool:
|
||||
"""Whether the user picked 'Spotify Free' as their metadata source
|
||||
(the no-creds option, for when they haven't connected Spotify)."""
|
||||
try:
|
||||
return bool(config_manager.get('metadata.spotify_free', False))
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
def _free_available(self) -> bool:
|
||||
"""Whether the no-creds fallback can be offered at all: enabled in
|
||||
config AND the SpotipyFree package is installed."""
|
||||
def _has_spotify_credentials(self) -> bool:
|
||||
"""Whether Spotify client credentials are configured (a 'connected'
|
||||
user, even if currently rate-limit-banned). Drives the auto-bridge."""
|
||||
try:
|
||||
cfg = config_manager.get_spotify_config() or {}
|
||||
return bool(cfg.get('client_id') and cfg.get('client_secret'))
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
def _free_installed(self) -> bool:
|
||||
from core.spotify_free_metadata import spotify_free_installed
|
||||
return self._free_enabled() and spotify_free_installed()
|
||||
return spotify_free_installed()
|
||||
|
||||
def _free_wanted(self) -> bool:
|
||||
"""Does the user want Spotify metadata at all? Either they have
|
||||
credentials (connected → eligible for the rate-limit auto-bridge) or
|
||||
they explicitly chose 'Spotify Free'. This is what keeps the free source
|
||||
from auto-scraping for someone who never opted into Spotify."""
|
||||
return self._has_spotify_credentials() or self._free_selected()
|
||||
|
||||
def _free_available(self) -> bool:
|
||||
"""Free CAN serve: package installed AND the user wants Spotify."""
|
||||
return self._free_installed() and self._free_wanted()
|
||||
|
||||
def is_spotify_metadata_available(self) -> bool:
|
||||
"""Whether SoulSync can serve Spotify metadata — real auth OR the
|
||||
no-creds fallback. Availability gates (search resolve, enrichment
|
||||
worker, watchlist) use THIS instead of ``is_spotify_authenticated()``
|
||||
so the fallback is reachable. Does NOT change auth semantics."""
|
||||
no-creds source. Availability gates (search resolve, enrichment worker,
|
||||
watchlist) use THIS instead of ``is_spotify_authenticated()`` so the
|
||||
free source is reachable. Does NOT change auth semantics."""
|
||||
from core.spotify_free_metadata import should_offer_spotify_metadata
|
||||
try:
|
||||
authed = self.is_spotify_authenticated()
|
||||
|
|
@ -597,13 +614,15 @@ class SpotifyClient:
|
|||
return should_offer_spotify_metadata(authed, self._free_available())
|
||||
|
||||
def _free_active(self) -> bool:
|
||||
"""Whether the no-creds SpotipyFree fallback may serve THIS request.
|
||||
"""Whether the no-creds source may serve THIS request: free available
|
||||
AND official can't (no auth, or rate-limited). ``is_spotify_authenticated()``
|
||||
already returns False during a rate-limit ban; the explicit rate-limit
|
||||
term covers the brief window before the auth cache refreshes. When authed
|
||||
+ healthy the official path returns first, so this never opens.
|
||||
|
||||
Only when official Spotify can't (no auth, or rate-limited — note
|
||||
``is_spotify_authenticated()`` already returns False during a rate-limit
|
||||
ban) AND the fallback is available. When authed + healthy the official
|
||||
path returns before any fallback, so this never opens. Delegates the
|
||||
pure decision to ``core.spotify_free_metadata.should_use_free_fallback``."""
|
||||
Two activations fall out of this: a no-auth user who chose Spotify Free
|
||||
(free is their source), and a connected user mid-rate-limit (free bridges
|
||||
the ban) — see _free_wanted()."""
|
||||
from core.spotify_free_metadata import should_use_free_fallback
|
||||
if not self._free_available():
|
||||
return False
|
||||
|
|
|
|||
|
|
@ -133,3 +133,56 @@ def test_normalize_artist_skips_imageless_sources():
|
|||
'visuals': {'avatarImage': {'sources': [{'height': 1}, {'url': 'u'}]}}}
|
||||
out = normalize_artist(raw)
|
||||
assert out['images'] == [{'url': 'u', 'height': None, 'width': None}]
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# SpotifyClient gate model — auto-bridge vs explicit opt-in vs no-surprise
|
||||
# (constructs the client via __new__ so no network/config init runs)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
from unittest.mock import patch # noqa: E402
|
||||
from core.spotify_client import SpotifyClient # noqa: E402
|
||||
import core.spotify_free_metadata as _sfm # noqa: E402
|
||||
|
||||
|
||||
def _gate(authed, has_creds, selected, installed, rate_limited):
|
||||
c = SpotifyClient.__new__(SpotifyClient)
|
||||
with patch.object(SpotifyClient, 'is_spotify_authenticated', return_value=authed), \
|
||||
patch('core.spotify_client.config_manager') as cm, \
|
||||
patch('core.spotify_client._is_globally_rate_limited', return_value=rate_limited), \
|
||||
patch.object(_sfm, 'spotify_free_installed', return_value=installed):
|
||||
cm.get_spotify_config.return_value = (
|
||||
{'client_id': 'x', 'client_secret': 'y'} if has_creds else {})
|
||||
cm.get.side_effect = lambda k, d=None: selected if k == 'metadata.spotify_free' else d
|
||||
return c.is_spotify_metadata_available(), c._free_active()
|
||||
|
||||
|
||||
def test_connected_healthy_uses_official():
|
||||
avail, free = _gate(authed=True, has_creds=True, selected=False, installed=True, rate_limited=False)
|
||||
assert avail is True and free is False # official; free never opens
|
||||
|
||||
|
||||
def test_connected_ratelimited_bridges_to_free():
|
||||
avail, free = _gate(authed=False, has_creds=True, selected=False, installed=True, rate_limited=True)
|
||||
assert avail is True and free is True # auto-bridge, no toggle needed
|
||||
|
||||
|
||||
def test_no_auth_picked_spotify_free_serves():
|
||||
avail, free = _gate(authed=False, has_creds=False, selected=True, installed=True, rate_limited=False)
|
||||
assert avail is True and free is True
|
||||
|
||||
|
||||
def test_no_auth_not_opted_in_does_nothing():
|
||||
# The key no-surprise guarantee: a user who never chose Spotify gets no free.
|
||||
avail, free = _gate(authed=False, has_creds=False, selected=False, installed=True, rate_limited=False)
|
||||
assert avail is False and free is False
|
||||
|
||||
|
||||
def test_selected_but_package_missing_is_graceful():
|
||||
avail, free = _gate(authed=False, has_creds=False, selected=True, installed=False, rate_limited=False)
|
||||
assert avail is False and free is False
|
||||
|
||||
|
||||
def test_connected_ratelimited_but_no_package_no_bridge():
|
||||
avail, free = _gate(authed=False, has_creds=True, selected=False, installed=False, rate_limited=True)
|
||||
assert avail is False and free is False
|
||||
|
|
|
|||
|
|
@ -2318,8 +2318,11 @@ def get_status():
|
|||
if t.get('status') in ('downloading', 'searching', 'post_processing', 'queued', 'pending'):
|
||||
active_dl_count += 1
|
||||
|
||||
# Spotify Free: tell the UI whether Spotify metadata is available even
|
||||
# without auth (so the Settings source selector can offer it).
|
||||
# Spotify Free: tell the UI (a) whether Spotify metadata is currently
|
||||
# available (auth or free), and (b) whether the SpotipyFree package is
|
||||
# installed — the latter is what makes 'Spotify Free' selectable in the
|
||||
# source dropdown (selecting it is the opt-in, so it can't depend on
|
||||
# already having selected it).
|
||||
spotify_status = dict(metadata_status['spotify'])
|
||||
try:
|
||||
spotify_status['metadata_available'] = bool(
|
||||
|
|
@ -2327,6 +2330,11 @@ def get_status():
|
|||
)
|
||||
except Exception:
|
||||
spotify_status['metadata_available'] = bool(spotify_status.get('authenticated'))
|
||||
try:
|
||||
from core.spotify_free_metadata import spotify_free_installed
|
||||
spotify_status['free_installed'] = spotify_free_installed()
|
||||
except Exception:
|
||||
spotify_status['free_installed'] = False
|
||||
|
||||
status_data = {
|
||||
'metadata_source': metadata_status['metadata_source'],
|
||||
|
|
|
|||
|
|
@ -3838,21 +3838,15 @@
|
|||
<label>Primary metadata source:</label>
|
||||
<select id="metadata-fallback-source">
|
||||
<option value="spotify">Spotify</option>
|
||||
<option value="spotify_free">Spotify Free (no credentials)</option>
|
||||
<option value="itunes">iTunes / Apple Music</option>
|
||||
<option value="deezer">Deezer</option>
|
||||
<option value="discogs">Discogs</option>
|
||||
<option value="musicbrainz">MusicBrainz</option>
|
||||
</select>
|
||||
</div>
|
||||
<div class="form-group">
|
||||
<label class="checkbox-label">
|
||||
<input type="checkbox" id="metadata-spotify-free">
|
||||
<span>Use Spotify Free when Spotify is unavailable or rate-limited</span>
|
||||
</label>
|
||||
<div class="callback-help">Fetches Spotify metadata without API credentials. Works two ways: as a full Spotify source if you haven't connected your account, and as a bridge that keeps search & enrichment going when your connected Spotify hits a rate-limit. Unofficial & best-effort (can break if Spotify changes); it can't search albums by name or access your library, and never runs while your authenticated Spotify is working normally.</div>
|
||||
</div>
|
||||
<div class="callback-info">
|
||||
<div class="callback-help">Choose the primary source for artist, album, and track metadata. Spotify needs an active session — or enable Spotify Free above. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.</div>
|
||||
<div class="callback-help">Choose the primary source for artist, album, and track metadata. <strong>Spotify</strong> needs a connected account. <strong>Spotify Free</strong> pulls the same Spotify data without credentials (unofficial & best-effort — can break if Spotify changes, can't search albums by name, no library access). Either way, a connected Spotify that hits a rate-limit is automatically bridged by the free source if it's installed. Discogs requires a personal token. MusicBrainz is always available but rate-limited to 1 req/sec.</div>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
|
|
|||
|
|
@ -64,9 +64,13 @@ function syncMetadataSourceSelection(source) {
|
|||
|
||||
function _isMetadataSourceSelectable(source) {
|
||||
if (source === 'spotify') {
|
||||
// Selectable with real auth OR when Spotify Free (no-creds) is available.
|
||||
return _lastStatusPayload?.spotify?.authenticated === true
|
||||
|| _lastStatusPayload?.spotify?.metadata_available === true;
|
||||
// Official Spotify needs a connected session.
|
||||
return _lastStatusPayload?.spotify?.authenticated === true;
|
||||
}
|
||||
if (source === 'spotify_free') {
|
||||
// No-creds Spotify only needs the SpotipyFree package installed —
|
||||
// selecting it IS the opt-in, so it must NOT depend on having selected it.
|
||||
return _lastStatusPayload?.spotify?.free_installed === true;
|
||||
}
|
||||
if (source === 'discogs') {
|
||||
const token = document.getElementById('discogs-token');
|
||||
|
|
@ -1045,10 +1049,13 @@ async function loadSettingsData() {
|
|||
// Populate Discogs settings
|
||||
document.getElementById('discogs-token').value = settings.discogs?.token || '';
|
||||
|
||||
// Populate Metadata source setting
|
||||
document.getElementById('metadata-fallback-source').value = settings.metadata?.fallback_source || 'deezer';
|
||||
const spotifyFreeToggle = document.getElementById('metadata-spotify-free');
|
||||
if (spotifyFreeToggle) spotifyFreeToggle.checked = settings.metadata?.spotify_free === true;
|
||||
// Populate Metadata source setting. 'Spotify Free' is stored as
|
||||
// fallback_source='spotify' + spotify_free=true (so all downstream
|
||||
// 'spotify' routing is unchanged) — map it back to the dropdown value.
|
||||
const _fbSrc = settings.metadata?.fallback_source || 'deezer';
|
||||
const _metaSel = (_fbSrc === 'spotify' && settings.metadata?.spotify_free === true)
|
||||
? 'spotify_free' : _fbSrc;
|
||||
document.getElementById('metadata-fallback-source').value = _metaSel;
|
||||
|
||||
// Populate Hydrabase settings
|
||||
const hbConfig = settings.hydrabase || {};
|
||||
|
|
@ -2759,12 +2766,19 @@ async function saveSettings(quiet = false) {
|
|||
const discogsTokenPresent = !!discogsTokenInput?.value?.trim();
|
||||
let metadataSource = metadataSourceSelect?.value || 'deezer';
|
||||
const spotifySessionActive = _lastStatusPayload?.spotify?.authenticated === true;
|
||||
const spotifyFreeInstalled = _lastStatusPayload?.spotify?.free_installed === true;
|
||||
if (metadataSource === 'spotify' && !spotifySessionActive) {
|
||||
metadataSource = _metadataSourceFallback('spotify');
|
||||
if (metadataSourceSelect) metadataSourceSelect.value = metadataSource;
|
||||
if (!quiet) {
|
||||
showToast('Spotify is disconnected, so the primary metadata source was switched.', 'warning');
|
||||
}
|
||||
} else if (metadataSource === 'spotify_free' && !spotifyFreeInstalled) {
|
||||
metadataSource = _metadataSourceFallback('spotify_free');
|
||||
if (metadataSourceSelect) metadataSourceSelect.value = metadataSource;
|
||||
if (!quiet) {
|
||||
showToast('Spotify Free needs the SpotipyFree package installed.', 'warning');
|
||||
}
|
||||
} else if (metadataSource === 'discogs' && !discogsTokenPresent) {
|
||||
metadataSource = _metadataSourceFallback('discogs');
|
||||
if (metadataSourceSelect) metadataSourceSelect.value = metadataSource;
|
||||
|
|
@ -2846,8 +2860,10 @@ async function saveSettings(quiet = false) {
|
|||
token: document.getElementById('discogs-token').value,
|
||||
},
|
||||
metadata: {
|
||||
fallback_source: metadataSource,
|
||||
spotify_free: document.getElementById('metadata-spotify-free')?.checked === true
|
||||
// 'Spotify Free' is stored as the spotify source + a flag, so all
|
||||
// downstream 'spotify' routing is unchanged.
|
||||
fallback_source: metadataSource === 'spotify_free' ? 'spotify' : metadataSource,
|
||||
spotify_free: metadataSource === 'spotify_free'
|
||||
},
|
||||
hydrabase: {
|
||||
url: document.getElementById('hydrabase-url').value,
|
||||
|
|
|
|||
Loading…
Reference in a new issue