mirror of
https://github.com/beetbox/beets.git
synced 2026-08-21 06:19:15 -04:00
Fix KeyError in Deezer track conversion when artist is missing (#6929)
Fixes #4339. The album half of this issue is already fixed, `album_for_id` guards the `contributors` key. This covers the remaining call site, `_get_track`, which still assumes the key exists: ```python artist, artist_id = self.get_artist( track_data.get("contributors", [track_data["artist"]]) ) ``` Two problems. The default argument to `.get` is evaluated eagerly, so `track_data["artist"]` runs on every call, even when `contributors` is present. A track payload carrying `contributors` but no `artist` raises KeyError, despite the code reading as though the fallback only applies when `contributors` is missing. And the fallback itself depends on `artist` being there, which is the same assumption sampsyo asked the plugin to stop making. I mirrored the shape already merged for the album path: use `contributors` when present, fall back to `artist` when that's the one we have, and leave the artist fields unset when neither key exists. `str(artist_id)` now also only runs when there's an id, so a track with no artist info gets `None` instead of the string "None". The album path still calls `str(artist_id)` unconditionally and can store "None" the same way, but I left it alone to keep this to the one call site. Happy to follow up on it. `_get_track` had no test coverage, so I added three tests: contributors without artist, artist without contributors (the old fallback still behaves the same), and neither key. The first and third fail on master with KeyError at the `.get` line, and all three pass with the change. ruff check and format are clean on both files.
This commit is contained in:
@@ -187,16 +187,20 @@ class DeezerPlugin(SearchApiMetadataSourcePlugin[IDResponse]):
|
||||
|
||||
:param track_data: Deezer Track object dict
|
||||
"""
|
||||
artist, artist_id = self.get_artist(
|
||||
track_data.get("contributors", [track_data["artist"]])
|
||||
)
|
||||
contributors = track_data.get("contributors")
|
||||
if contributors is None and (artist_data := track_data.get("artist")):
|
||||
contributors = [artist_data]
|
||||
if contributors is not None:
|
||||
artist, artist_id = self.get_artist(contributors)
|
||||
else:
|
||||
artist, artist_id = None, None
|
||||
return TrackInfo(
|
||||
title=track_data["title"],
|
||||
track_id=track_data["id"],
|
||||
deezer_track_id=track_data["id"],
|
||||
isrc=track_data.get("isrc"),
|
||||
artist=artist,
|
||||
artist_id=str(artist_id),
|
||||
artist_id=str(artist_id) if artist_id is not None else None,
|
||||
length=track_data["duration"],
|
||||
index=track_data.get("track_position"),
|
||||
medium=track_data.get("disk_number"),
|
||||
|
||||
@@ -56,6 +56,11 @@ Bug fixes
|
||||
copyright/rights-statement text verbatim. It's now normalized to a concise
|
||||
label name, stripping copyright markers, years, and corporate, licensing, and
|
||||
territorial boilerplate. Affects both album and track metadata. :bug:`6796`
|
||||
- :doc:`plugins/deezer`: Track conversion no longer assumes the API sends both
|
||||
``contributors`` and ``artist``. The fallback to ``artist`` was evaluated even
|
||||
when ``contributors`` was present, so a track payload without ``artist``
|
||||
raised ``KeyError``. Albums were already guarded; this fixes the remaining
|
||||
call site. :bug:`4339`
|
||||
|
||||
..
|
||||
For plugin developers
|
||||
|
||||
@@ -39,3 +39,36 @@ class TestSearchQuery:
|
||||
)
|
||||
|
||||
assert query == 'album:"Album"'
|
||||
|
||||
|
||||
class TestGetTrack:
|
||||
def track_data(self, **fields):
|
||||
return {
|
||||
"id": 1,
|
||||
"title": "Title",
|
||||
"duration": 100,
|
||||
"link": "https://www.deezer.com/track/1",
|
||||
**fields,
|
||||
}
|
||||
|
||||
def test_uses_contributors_when_artist_is_missing(self, plugin):
|
||||
track = plugin._get_track(
|
||||
self.track_data(contributors=[{"id": 2, "name": "Artist"}])
|
||||
)
|
||||
|
||||
assert track.artist == "Artist"
|
||||
assert track.artist_id == "2"
|
||||
|
||||
def test_falls_back_to_artist_without_contributors(self, plugin):
|
||||
track = plugin._get_track(
|
||||
self.track_data(artist={"id": 2, "name": "Artist"})
|
||||
)
|
||||
|
||||
assert track.artist == "Artist"
|
||||
assert track.artist_id == "2"
|
||||
|
||||
def test_tolerates_missing_artist_and_contributors(self, plugin):
|
||||
track = plugin._get_track(self.track_data())
|
||||
|
||||
assert track.artist is None
|
||||
assert track.artist_id is None
|
||||
|
||||
Reference in New Issue
Block a user