Deezer cover-art download: fallback to original URL on CDN refusal
Defensive followup. If Deezer CDN ever refuses the upgraded 1900×1900 URL for a specific album (rare — empirically tested 4 albums and none hit it), pre-fix would have succeeded with the 1000×1000 URL and post-fix would have failed entirely. Both download sites now retry with the original URL when the upgraded URL fails: - `core/metadata/artwork.py::download_cover_art` — auto post-process flow. Resolves the original URL from album_info / context the same way the existing path does. - `core/tag_writer.py::download_cover_art` — captures the original URL before upgrade so the retry has it without a second context lookup. Strictly non-regressive: worst plausible post-fix case is now identical to pre-fix (cover at 1000×1000 succeeds). Fallback only fires on the rare CDN-refusal edge. Tests added (2): - `test_tag_writer_retries_with_original_on_failure` — upgraded URL raises, original succeeds, both attempts logged in call order - `test_tag_writer_no_fallback_for_non_dzcdn_url` — non-Deezer URLs go through unchanged, no fallback path triggered (single attempt) Verification: - 18/18 helper + integration tests pass - 2561 full suite passes - Ruff clean
This commit is contained in:
parent
80cf16339c
commit
8a4c0dc92a
3 changed files with 112 additions and 3 deletions
|
|
@ -341,8 +341,33 @@ def download_cover_art(album_info: dict, target_dir: str, context: dict = None):
|
|||
if not art_url:
|
||||
logger.warning("No cover art URL available for download.")
|
||||
return
|
||||
with urllib.request.urlopen(art_url, timeout=10) as response:
|
||||
image_data = response.read()
|
||||
# Fetch with one fallback level: if we upgraded a Deezer
|
||||
# URL above and the CDN happens to refuse the larger size
|
||||
# for this specific album, retry with the original URL so
|
||||
# we never regress vs. pre-upgrade behavior. Empirically
|
||||
# 1900 works for every album tested but defending against
|
||||
# the edge case keeps the fix strictly non-regressive.
|
||||
original_url = album_info.get("album_image_url")
|
||||
if context and not original_url:
|
||||
album_ctx = get_import_context_album(context)
|
||||
original_url = album_ctx.get("image_url") or original_url
|
||||
try:
|
||||
with urllib.request.urlopen(art_url, timeout=10) as response:
|
||||
image_data = response.read()
|
||||
except Exception as fetch_err:
|
||||
if (
|
||||
"dzcdn" in art_url
|
||||
and original_url
|
||||
and original_url != art_url
|
||||
):
|
||||
logger.info(
|
||||
"Deezer CDN refused upgraded cover URL (%s); "
|
||||
"retrying with original size", fetch_err,
|
||||
)
|
||||
with urllib.request.urlopen(original_url, timeout=10) as response:
|
||||
image_data = response.read()
|
||||
else:
|
||||
raise
|
||||
|
||||
if not image_data:
|
||||
return
|
||||
|
|
|
|||
|
|
@ -209,10 +209,14 @@ def download_cover_art(cover_url: str) -> Optional[Tuple[bytes, str]]:
|
|||
max). Mirrors the same upgrade in
|
||||
``core.metadata.artwork.download_cover_art`` so the
|
||||
enhanced-library-view "Write Tags to File" feature embeds the same
|
||||
high-resolution cover the auto post-process flow does.
|
||||
high-resolution cover the auto post-process flow does. Falls back
|
||||
to the original URL if the CDN refuses the upgraded size for a
|
||||
specific album — keeps the fix strictly non-regressive vs. the
|
||||
pre-upgrade behaviour.
|
||||
"""
|
||||
if not cover_url:
|
||||
return None
|
||||
original_url = cover_url
|
||||
if 'dzcdn' in cover_url:
|
||||
try:
|
||||
from core.deezer_client import _upgrade_deezer_cover_url
|
||||
|
|
@ -226,6 +230,21 @@ def download_cover_art(cover_url: str) -> Optional[Tuple[bytes, str]]:
|
|||
if image_data:
|
||||
return (image_data, mime_type)
|
||||
except Exception as e:
|
||||
# Deezer CDN refused upgraded size for this album — retry with
|
||||
# original URL so we never get less than pre-upgrade behaviour.
|
||||
if 'dzcdn' in cover_url and cover_url != original_url:
|
||||
logger.info(
|
||||
"Deezer CDN refused upgraded cover URL (%s); retrying with original size", e,
|
||||
)
|
||||
try:
|
||||
with urllib.request.urlopen(original_url, timeout=15) as response:
|
||||
image_data = response.read()
|
||||
mime_type = response.info().get_content_type() or 'image/jpeg'
|
||||
if image_data:
|
||||
return (image_data, mime_type)
|
||||
except Exception as fallback_err:
|
||||
logger.error(f"Error downloading cover art (fallback): {fallback_err}")
|
||||
return None
|
||||
logger.error(f"Error downloading cover art from {cover_url}: {e}")
|
||||
return None
|
||||
|
||||
|
|
|
|||
|
|
@ -138,3 +138,68 @@ class TestEmptyInputs:
|
|||
|
||||
def test_none(self):
|
||||
assert _upgrade_deezer_cover_url(None) is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Download fallback — if upgraded URL 403s, retry with original
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class TestDownloadFallbackOnCdnRefusal:
|
||||
"""If Deezer CDN refuses the upgraded 1900×1900 URL for some
|
||||
specific album (rare but possible — empirically tested 4 albums
|
||||
and none hit this, but defending the edge keeps the fix
|
||||
strictly non-regressive vs. pre-upgrade behaviour)."""
|
||||
|
||||
def test_tag_writer_retries_with_original_on_failure(self, monkeypatch):
|
||||
"""tag_writer.download_cover_art must fall back to the
|
||||
original URL when the upgraded URL fails."""
|
||||
from core import tag_writer
|
||||
|
||||
original_url = 'https://cdn-images.dzcdn.net/images/cover/abc/1000x1000-000000-80-0-0.jpg'
|
||||
upgraded_url = 'https://cdn-images.dzcdn.net/images/cover/abc/1900x1900-000000-80-0-0.jpg'
|
||||
|
||||
call_log = []
|
||||
|
||||
class _FakeResponse:
|
||||
def read(self): return b'cover-bytes'
|
||||
def info(self):
|
||||
class _Info:
|
||||
def get_content_type(_self): return 'image/jpeg'
|
||||
return _Info()
|
||||
def __enter__(self): return self
|
||||
def __exit__(self, *a): pass
|
||||
|
||||
def fake_urlopen(url, timeout=None):
|
||||
call_log.append(url)
|
||||
if url == upgraded_url:
|
||||
raise Exception("403 Forbidden")
|
||||
return _FakeResponse()
|
||||
|
||||
monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen)
|
||||
|
||||
result = tag_writer.download_cover_art(original_url)
|
||||
|
||||
assert result == (b'cover-bytes', 'image/jpeg')
|
||||
# Tried upgraded first, then fell back to original
|
||||
assert call_log == [upgraded_url, original_url]
|
||||
|
||||
def test_tag_writer_no_fallback_for_non_dzcdn_url(self, monkeypatch):
|
||||
"""Non-Deezer URLs go through unchanged — no upgrade, no
|
||||
fallback. Fast path preserved."""
|
||||
from core import tag_writer
|
||||
|
||||
spotify_url = 'https://i.scdn.co/image/abc'
|
||||
call_log = []
|
||||
|
||||
def fake_urlopen(url, timeout=None):
|
||||
call_log.append(url)
|
||||
raise Exception("network error")
|
||||
|
||||
monkeypatch.setattr('core.tag_writer.urllib.request.urlopen', fake_urlopen)
|
||||
|
||||
result = tag_writer.download_cover_art(spotify_url)
|
||||
|
||||
assert result is None
|
||||
# Single attempt — no Deezer fallback path triggered
|
||||
assert call_log == [spotify_url]
|
||||
|
|
|
|||
Loading…
Reference in a new issue