From 2a823676676e2bcff4d9f54b688fc580b6231d66 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 5 Apr 2019 15:41:28 +0200
Subject: [PATCH] Fix #788: fixed crashing import with empty album artist
---
api/funkwhale_api/music/metadata.py | 24 ++++++++-
api/funkwhale_api/music/tasks.py | 2 +-
api/tests/music/test_metadata.py | 75 +++++++++++++++++++++++++++++
3 files changed, 99 insertions(+), 2 deletions(-)
diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py
index 3347dfb6..eca4b475 100644
--- a/api/funkwhale_api/music/metadata.py
+++ b/api/funkwhale_api/music/metadata.py
@@ -431,7 +431,11 @@ class AlbumField(serializers.Field):
}
artists_field = ArtistField(for_album=True)
payload = artists_field.get_value(data)
- artists = artists_field.to_internal_value(payload)
+ try:
+ artists = artists_field.to_internal_value(payload)
+ except serializers.ValidationError as e:
+ artists = []
+ logger.debug("Ignoring validation error on album artists: %s", e)
album_serializer = AlbumSerializer(data=final)
album_serializer.is_valid(raise_exception=True)
album_serializer.validated_data["artists"] = artists
@@ -513,3 +517,21 @@ class TrackMetadataSerializer(serializers.Serializer):
album = AlbumField()
artists = ArtistField()
cover_data = CoverDataField()
+
+
+class FakeMetadata(Mapping):
+ def __init__(self, data, picture=None):
+ self.data = data
+ self.picture = None
+
+ def __getitem__(self, key):
+ return self.data[key]
+
+ def __len__(self):
+ return len(self.data)
+
+ def __iter__(self):
+ yield from self.data
+
+ def get_picture(self, *args):
+ return self.picture
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index f64b7eed..3f0d87e7 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -466,7 +466,7 @@ def _get_track(data):
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
)[0]
- album_artists = getter(data, "album", "artists", default=artists)
+ album_artists = getter(data, "album", "artists", default=artists) or artists
album_artist = album_artists[0]
album_artist_name = album_artist.get("name")
if album_artist_name == artist_name:
diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py
index e2c180a6..1d61901c 100644
--- a/api/tests/music/test_metadata.py
+++ b/api/tests/music/test_metadata.py
@@ -462,3 +462,78 @@ def test_album_serializer_validation(data, errored_field):
assert len(serializer.errors) == 1
assert errored_field in serializer.errors
+
+
+def test_fake_metadata_with_serializer():
+ data = {
+ "title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
+ "artist": "Edvard Grieg",
+ "album_artist": "Edvard Grieg; Musopen Symphony Orchestra",
+ "album": "Peer Gynt Suite no. 1, op. 46",
+ "date": "2012-08-15",
+ "position": "1",
+ "disc_number": "1",
+ "musicbrainz_albumid": "a766da8b-8336-47aa-a3ee-371cc41ccc75",
+ "mbid": "bd21ac48-46d8-4e78-925f-d9cc2a294656",
+ "musicbrainz_artistid": "013c8e5b-d72a-4cd3-8dee-6c64d6125823",
+ "musicbrainz_albumartistid": "013c8e5b-d72a-4cd3-8dee-6c64d6125823;5b4d7d2d-36df-4b38-95e3-a964234f520f",
+ "license": "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/",
+ "copyright": "Someone",
+ }
+
+ expected = {
+ "title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
+ "artists": [
+ {
+ "name": "Edvard Grieg",
+ "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
+ }
+ ],
+ "album": {
+ "title": "Peer Gynt Suite no. 1, op. 46",
+ "mbid": uuid.UUID("a766da8b-8336-47aa-a3ee-371cc41ccc75"),
+ "release_date": datetime.date(2012, 8, 15),
+ "artists": [
+ {
+ "name": "Edvard Grieg",
+ "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
+ },
+ {
+ "name": "Musopen Symphony Orchestra",
+ "mbid": uuid.UUID("5b4d7d2d-36df-4b38-95e3-a964234f520f"),
+ },
+ ],
+ },
+ "position": 1,
+ "disc_number": 1,
+ "mbid": uuid.UUID("bd21ac48-46d8-4e78-925f-d9cc2a294656"),
+ "license": "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/",
+ "copyright": "Someone",
+ "cover_data": None,
+ }
+ serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data))
+ assert serializer.is_valid(raise_exception=True) is True
+ assert serializer.validated_data == expected
+
+
+def test_serializer_album_artist_missing():
+ data = {
+ "title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
+ "artist": "Edvard Grieg",
+ "album": "Peer Gynt Suite no. 1, op. 46",
+ }
+
+ expected = {
+ "title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
+ "artists": [{"name": "Edvard Grieg", "mbid": None}],
+ "album": {
+ "title": "Peer Gynt Suite no. 1, op. 46",
+ "mbid": None,
+ "release_date": None,
+ "artists": [],
+ },
+ "cover_data": None,
+ }
+ serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data))
+ assert serializer.is_valid(raise_exception=True) is True
+ assert serializer.validated_data == expected
--
GitLab