Let the Spotify worker resume during a rate-limit ban when Free can bridge
#798 follow-up. The enrichment worker's own loop already bridges to the no-creds Spotify Free source during a rate-limit ban (its guard checks is_spotify_metadata_available()). But the resume button's pre-check (_spotify_resume_pre_check) blocked resume on ANY rate-limit with no awareness of Free — so a Free-opted-in user who got rate-limited was locked out of restarting the worker, unable to fall through to the free API. Fix: the resume guard now mirrors the worker. Block only when rate-limited AND nothing can serve (plain auth, no Free) — where resuming would just sleep out the ban. When Free is available it serves during the ban (is_spotify_authenticated() is False while banned, so is_spotify_metadata_available() reports the free source), so resume is allowed and the worker bridges via Free, then returns to real auth once the ban lifts. Stays real-API-first; Free is only the bridge. The rule is pinned in a pure helper should_block_rate_limited_resume() next to the other gate functions, with 3 tests. Full suite clean (only pre-existing soundcloud /app env failures remain).
This commit is contained in:
parent
4039ec3573
commit
12038bde08
3 changed files with 52 additions and 4 deletions
|
|
@ -65,6 +65,20 @@ def should_offer_spotify_metadata(authenticated: bool, free_available: bool) ->
|
|||
return authenticated or free_available
|
||||
|
||||
|
||||
def should_block_rate_limited_resume(rate_limited: bool, metadata_available: bool) -> bool:
|
||||
"""Whether to refuse resuming the Spotify enrichment worker.
|
||||
|
||||
The worker's own loop bridges to the no-creds free source during a ban
|
||||
(its rate-limit guard checks ``is_spotify_metadata_available()``). The
|
||||
resume button must mirror that: block ONLY when rate-limited AND nothing
|
||||
can serve (plain auth, no free) — otherwise resuming just sleeps. When the
|
||||
free fallback is available, ``metadata_available`` is True during a ban
|
||||
(``is_spotify_authenticated()`` returns False while banned), so resume is
|
||||
allowed and the worker bridges via free.
|
||||
"""
|
||||
return rate_limited and not metadata_available
|
||||
|
||||
|
||||
# --------------------------------------------------------------------------
|
||||
# Normalizers (pure — unit-testable against captured fixtures)
|
||||
# --------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -10,12 +10,34 @@ from __future__ import annotations
|
|||
|
||||
from core.spotify_free_metadata import (
|
||||
normalize_artist,
|
||||
should_block_rate_limited_resume,
|
||||
should_offer_spotify_metadata,
|
||||
should_use_free_fallback,
|
||||
spotify_free_installed,
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# should_block_rate_limited_resume — the worker resume guard
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_resume_blocked_when_rate_limited_and_nothing_serves():
|
||||
# Plain auth, no free: resuming during a ban would just sleep → block.
|
||||
assert should_block_rate_limited_resume(rate_limited=True, metadata_available=False) is True
|
||||
|
||||
|
||||
def test_resume_allowed_when_free_can_bridge():
|
||||
# Rate-limited but free is available (metadata_available True during a ban
|
||||
# because is_spotify_authenticated() is False while banned) → allow resume,
|
||||
# the worker bridges via free.
|
||||
assert should_block_rate_limited_resume(rate_limited=True, metadata_available=True) is False
|
||||
|
||||
|
||||
def test_resume_never_blocked_when_not_rate_limited():
|
||||
assert should_block_rate_limited_resume(rate_limited=False, metadata_available=False) is False
|
||||
assert should_block_rate_limited_resume(rate_limited=False, metadata_available=True) is False
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# should_use_free_fallback — the activation gate
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -34883,12 +34883,24 @@ from core.enrichment.services import (
|
|||
|
||||
|
||||
def _spotify_resume_pre_check():
|
||||
"""Mirror the inline Spotify rate-limit guard from the legacy
|
||||
``/api/spotify-enrichment/resume`` route. Returns
|
||||
``(429, message)`` to short-circuit when banned, ``None`` when ok."""
|
||||
"""Guard the Spotify enrichment worker's resume button against a rate-limit
|
||||
ban. Returns ``(429, message)`` to short-circuit when banned, ``None`` ok.
|
||||
|
||||
Mirrors the worker's own loop: if the opt-in Spotify-Free fallback can
|
||||
serve enrichment, allow resume and let the worker bridge via the no-creds
|
||||
source during the ban. Block only when rate-limited AND nothing can serve
|
||||
(plain auth, no free) — where resuming would just sleep.
|
||||
"""
|
||||
try:
|
||||
if _spotify_rate_limited():
|
||||
return (429, 'Cannot resume while Spotify is rate limited')
|
||||
from core.spotify_free_metadata import should_block_rate_limited_resume
|
||||
try:
|
||||
metadata_available = bool(spotify_client.is_spotify_metadata_available())
|
||||
except Exception as e:
|
||||
logger.debug("spotify free-availability check failed: %s", e)
|
||||
metadata_available = False
|
||||
if should_block_rate_limited_resume(True, metadata_available):
|
||||
return (429, 'Cannot resume while Spotify is rate limited')
|
||||
except Exception as e:
|
||||
logger.debug("spotify rate-limit pre-check failed: %s", e)
|
||||
return None
|
||||
|
|
|
|||
Loading…
Reference in a new issue