MirrorMirror
/
funkwhale
mirror of https://dev.funkwhale.audio/funkwhale/funkwhale.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Merge branch '718-error-handling' into 'develop' 

Resolve "Upload problem due to a formatting error in the date tag"

Closes #718, #583, #501, #252, #544 (yes, that's a lot :D)

So, this MR changes the way we parse audio metadata and handle validation for this data:

- There is now a strict and explicit separation between the retrieve step (extracting raw metadata value from files), and the validation step (ensure the raw metadata is actually valid, convert it to our internal types, etc.)
- The date parsing issue from #718 should be solved (and Funkwhale should not crash anymore against invalid dates, skipping those instead)
- We are now ready to deal with multiple artists at the metadata parsing level (cf !515). It should also be easier to exclude featuring artists now (cf #736), but it's not implemented here
- When invalid metadata makes the import fail, we store the errors on the upload so we can display those to the user
- Tables that list uploads in the Web UI leverage this new data to display more detailed errors and advices, as shown on the screenshot below

## Link to the detail modal

![image](/uploads/20eab969c7cb46fe68a402d8a509815d/image.png)

## Import error detail

![image](/uploads/7cebae6dd05f437345fa38340e8675c7/image.png)

![image](/uploads/946c8ae59cd89fe29e701700fbd5063f/image.png)

As you can see, we display the type of errors, the failing tags and the actual error, as well as some link to the documentation, support forum and some debug info you can easily copy-paste to get help.

See merge request funkwhale/funkwhale!703
This commit is contained in:
Eliot Berriot 2019-04-04 16:07:44 +02:00
parent 7bb0fa2e64 05f0129025
commit 47b3b1ae0a
9 changed files with 916 additions and 397 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

329
api/funkwhale_api/music/metadata.py
View File

@ -8,7 +8,8 @@ import mutagen.oggtheora
import mutagen.oggvorbis
import mutagen.flac
from django import forms
from rest_framework import serializers
from rest_framework.compat import Mapping
logger = logging.getLogger(__name__)
NODEFAULT = object()
@ -122,85 +123,23 @@ def get_mp3_recording_id(f, k):
raise TagNotFound(k)
def convert_position(v):
try:
return int(v)
except ValueError:
# maybe the position is of the form "1/4"
pass
try:
return int(v.split("/")[0])
except (ValueError, AttributeError, IndexError):
pass
class FirstUUIDField(forms.UUIDField):
def to_python(self, value):
try:
# sometimes, Picard leaves two uuids in the field, separated
# by a slash or a ;
value = value.split(";")[0].split("/")[0].strip()
except (AttributeError, IndexError, TypeError):
pass
return super().to_python(value)
def get_date(value):
ADDITIONAL_FORMATS = ["%Y-%d-%m %H:%M"] # deezer date format
try:
parsed = pendulum.parse(str(value))
return datetime.date(parsed.year, parsed.month, parsed.day)
except pendulum.exceptions.ParserError:
pass
for date_format in ADDITIONAL_FORMATS:
try:
parsed = datetime.datetime.strptime(value, date_format)
except ValueError:
continue
else:
return datetime.date(parsed.year, parsed.month, parsed.day)
raise ParseError("{} cannot be parsed as a date".format(value))
def split_and_return_first(separator):
def inner(v):
return v.split(separator)[0].strip()
return inner
VALIDATION = {
"musicbrainz_artistid": FirstUUIDField(),
"musicbrainz_albumid": FirstUUIDField(),
"musicbrainz_recordingid": FirstUUIDField(),
"musicbrainz_albumartistid": FirstUUIDField(),
}
VALIDATION = {}
CONF = {
"OggOpus": {
"getter": lambda f, k: f[k][0],
"fields": {
"track_number": {
"field": "TRACKNUMBER",
"to_application": convert_position,
},
"disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
"position": {"field": "TRACKNUMBER"},
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"album_artist": {
"field": "albumartist",
"to_application": split_and_return_first(";"),
},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date", "to_application": get_date},
"date": {"field": "date"},
"musicbrainz_albumid": {},
"musicbrainz_artistid": {},
"musicbrainz_albumartistid": {},
"musicbrainz_recordingid": {"field": "musicbrainz_trackid"},
"mbid": {"field": "musicbrainz_trackid"},
"license": {},
"copyright": {},
},
@ -208,23 +147,17 @@ CONF = {
"OggVorbis": {
"getter": lambda f, k: f[k][0],
"fields": {
"track_number": {
"field": "TRACKNUMBER",
"to_application": convert_position,
},
"disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
"position": {"field": "TRACKNUMBER"},
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"album_artist": {
"field": "albumartist",
"to_application": split_and_return_first(";"),
},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date", "to_application": get_date},
"date": {"field": "date"},
"musicbrainz_albumid": {},
"musicbrainz_artistid": {},
"musicbrainz_albumartistid": {},
"musicbrainz_recordingid": {"field": "musicbrainz_trackid"},
"mbid": {"field": "musicbrainz_trackid"},
"license": {},
"copyright": {},
"pictures": {
@ -236,20 +169,17 @@ CONF = {
"OggTheora": {
"getter": lambda f, k: f[k][0],
"fields": {
"track_number": {
"field": "TRACKNUMBER",
"to_application": convert_position,
},
"disc_number": {"field": "DISCNUMBER", "to_application": convert_position},
"position": {"field": "TRACKNUMBER"},
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date", "to_application": get_date},
"date": {"field": "date"},
"musicbrainz_albumid": {"field": "MusicBrainz Album Id"},
"musicbrainz_artistid": {"field": "MusicBrainz Artist Id"},
"musicbrainz_albumartistid": {"field": "MusicBrainz Album Artist Id"},
"musicbrainz_recordingid": {"field": "MusicBrainz Track Id"},
"mbid": {"field": "MusicBrainz Track Id"},
"license": {},
"copyright": {},
},
@ -258,20 +188,17 @@ CONF = {
"getter": get_id3_tag,
"clean_pictures": clean_id3_pictures,
"fields": {
"track_number": {"field": "TRCK", "to_application": convert_position},
"disc_number": {"field": "TPOS", "to_application": convert_position},
"position": {"field": "TRCK"},
"disc_number": {"field": "TPOS"},
"title": {"field": "TIT2"},
"artist": {"field": "TPE1"},
"album_artist": {"field": "TPE2"},
"album": {"field": "TALB"},
"date": {"field": "TDRC", "to_application": get_date},
"date": {"field": "TDRC"},
"musicbrainz_albumid": {"field": "MusicBrainz Album Id"},
"musicbrainz_artistid": {"field": "MusicBrainz Artist Id"},
"musicbrainz_albumartistid": {"field": "MusicBrainz Album Artist Id"},
"musicbrainz_recordingid": {
"field": "UFID",
"getter": get_mp3_recording_id,
},
"mbid": {"field": "UFID", "getter": get_mp3_recording_id},
"pictures": {},
"license": {"field": "WCOP"},
"copyright": {"field": "TCOP"},
@ -281,20 +208,17 @@ CONF = {
"getter": get_flac_tag,
"clean_pictures": clean_flac_pictures,
"fields": {
"track_number": {
"field": "tracknumber",
"to_application": convert_position,
},
"disc_number": {"field": "discnumber", "to_application": convert_position},
"position": {"field": "tracknumber"},
"disc_number": {"field": "discnumber"},
"title": {},
"artist": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date", "to_application": get_date},
"date": {"field": "date"},
"musicbrainz_albumid": {},
"musicbrainz_artistid": {},
"musicbrainz_albumartistid": {},
"musicbrainz_recordingid": {"field": "musicbrainz_trackid"},
"mbid": {"field": "musicbrainz_trackid"},
"test": {},
"pictures": {},
"license": {},
@ -304,7 +228,7 @@ CONF = {
}
ALL_FIELDS = [
"track_number",
"position",
"disc_number",
"title",
"artist",
@ -314,13 +238,13 @@ ALL_FIELDS = [
"musicbrainz_albumid",
"musicbrainz_artistid",
"musicbrainz_albumartistid",
"musicbrainz_recordingid",
"mbid",
"license",
"copyright",
]
class Metadata(object):
class Metadata(Mapping):
def __init__(self, filething, kind=mutagen.File):
self._file = kind(filething)
if self._file is None:
@ -368,6 +292,21 @@ class Metadata(object):
else:
return self.fallback.get(key, default=default)
def all(self):
"""
Return a dict with all support metadata fields, if they are available
"""
final = {}
for field in self._conf["fields"]:
if field in ["pictures"]:
continue
value = self.get(field, None)
if value is None:
continue
final[field] = str(value)
return final
def _get_from_self(self, key, default=NODEFAULT):
try:
field_conf = self._conf["fields"][key]
@ -390,25 +329,6 @@ class Metadata(object):
v = field.to_python(v)
return v
def all(self, ignore_parse_errors=True):
"""
Return a dict containing all metadata of the file
"""
data = {}
for field in ALL_FIELDS:
try:
data[field] = self.get(field, None)
except (TagNotFound, forms.ValidationError):
data[field] = None
except ParseError as e:
if not ignore_parse_errors:
raise
logger.warning("Unparsable field {}: {}".format(field, str(e)))
data[field] = None
return data
def get_picture(self, *picture_types):
if not picture_types:
raise ValueError("You need to request at least one picture type")
@ -430,3 +350,166 @@ class Metadata(object):
for p in pictures:
if p["type"] == ptype:
return p
def __getitem__(self, key):
return self.get(key)
def __len__(self):
return 1
def __iter__(self):
for field in self._conf["fields"]:
yield field
class ArtistField(serializers.Field):
def __init__(self, *args, **kwargs):
self.for_album = kwargs.pop("for_album", False)
super().__init__(*args, **kwargs)
def get_value(self, data):
if self.for_album:
keys = [("names", "album_artist"), ("mbids", "musicbrainz_albumartistid")]
else:
keys = [("names", "artist"), ("mbids", "musicbrainz_artistid")]
final = {}
for field, key in keys:
final[field] = data.get(key, None)
return final
def to_internal_value(self, data):
# we have multiple values that can be separated by various separators
separators = [";", "/"]
# we get a list like that if tagged via musicbrainz
# ae29aae4-abfb-4609-8f54-417b1f4d64cc; 3237b5a8-ae44-400c-aa6d-cea51f0b9074;
raw_mbids = data["mbids"]
used_separator = None
mbids = [raw_mbids]
if raw_mbids:
for separator in separators:
if separator in raw_mbids:
used_separator = separator
mbids = [m.strip() for m in raw_mbids.split(separator)]
break
# now, we split on artist names, using the same separator as the one used
# by mbids, if any
if used_separator and mbids:
names = [n.strip() for n in data["names"].split(used_separator)]
else:
names = [data["names"]]
final = []
for i, name in enumerate(names):
try:
mbid = mbids[i]
except IndexError:
mbid = None
artist = {"name": name, "mbid": mbid}
final.append(artist)
field = serializers.ListField(child=ArtistSerializer(), min_length=1)
return field.to_internal_value(final)
class AlbumField(serializers.Field):
def get_value(self, data):
return data
def to_internal_value(self, data):
try:
title = data.get("album")
except TagNotFound:
raise serializers.ValidationError("Missing album tag")
final = {
"title": title,
"release_date": data.get("date", None),
"mbid": data.get("musicbrainz_albumid", None),
}
artists_field = ArtistField(for_album=True)
payload = artists_field.get_value(data)
artists = artists_field.to_internal_value(payload)
album_serializer = AlbumSerializer(data=final)
album_serializer.is_valid(raise_exception=True)
album_serializer.validated_data["artists"] = artists
return album_serializer.validated_data
class CoverDataField(serializers.Field):
def get_value(self, data):
return data
def to_internal_value(self, data):
return data.get_picture("cover_front", "other")
class PermissiveDateField(serializers.CharField):
def to_internal_value(self, value):
if not value:
return None
value = super().to_internal_value(str(value))
ADDITIONAL_FORMATS = [
"%Y-%d-%m %H:%M", # deezer date format
"%Y-%W", # weird date format based on week number, see #718
]
for date_format in ADDITIONAL_FORMATS:
try:
parsed = datetime.datetime.strptime(value, date_format)
except ValueError:
continue
else:
return datetime.date(parsed.year, parsed.month, parsed.day)
try:
parsed = pendulum.parse(str(value))
return datetime.date(parsed.year, parsed.month, parsed.day)
except pendulum.exceptions.ParserError:
pass
return None
class ArtistSerializer(serializers.Serializer):
name = serializers.CharField()
mbid = serializers.UUIDField(required=False, allow_null=True)
class AlbumSerializer(serializers.Serializer):
title = serializers.CharField()
mbid = serializers.UUIDField(required=False, allow_null=True)
release_date = PermissiveDateField(required=False, allow_null=True)
class PositionField(serializers.CharField):
def to_internal_value(self, v):
v = super().to_internal_value(v)
if not v:
return v
try:
return int(v)
except ValueError:
# maybe the position is of the form "1/4"
pass
try:
return int(v.split("/")[0])
except (ValueError, AttributeError, IndexError):
pass
class TrackMetadataSerializer(serializers.Serializer):
title = serializers.CharField()
position = PositionField(allow_null=True, required=False)
disc_number = PositionField(allow_null=True, required=False)
copyright = serializers.CharField(allow_null=True, required=False)
license = serializers.CharField(allow_null=True, required=False)
mbid = serializers.UUIDField(allow_null=True, required=False)
album = AlbumField()
artists = ArtistField()
cover_data = CoverDataField()

145
api/funkwhale_api/music/tasks.py
View File

@ -151,10 +151,11 @@ class UploadImportError(ValueError):
super().__init__(code)
def fail_import(upload, error_code):
def fail_import(upload, error_code, detail=None, **fields):
old_status = upload.import_status
upload.import_status = "errored"
upload.import_details = {"error_code": error_code}
upload.import_details = {"error_code": error_code, "detail": detail}
upload.import_details.update(fields)
upload.import_date = timezone.now()
upload.save(update_fields=["import_details", "import_status", "import_date"])
@ -182,19 +183,29 @@ def process_upload(upload):
old_status = upload.import_status
audio_file = upload.get_audio_file()
additional_data = {}
m = metadata.Metadata(audio_file)
try:
serializer = metadata.TrackMetadataSerializer(data=m)
serializer.is_valid()
except Exception:
fail_import(upload, "unknown_error")
raise
if not serializer.is_valid():
detail = serializer.errors
try:
metadata_dump = m.all()
except Exception as e:
logger.warn("Cannot dump metadata for file %s: %s", audio_file, str(e))
return fail_import(
upload, "invalid_metadata", detail=detail, file_metadata=metadata_dump
)
final_metadata = collections.ChainMap(
additional_data, serializer.validated_data, import_metadata
)
additional_data["upload_source"] = upload.source
try:
if not audio_file:
# we can only rely on user proveded data
final_metadata = import_metadata
else:
# we use user provided data and data from the file itself
m = metadata.Metadata(audio_file)
file_metadata = m.all()
final_metadata = collections.ChainMap(
additional_data, import_metadata, file_metadata
)
additional_data["cover_data"] = m.get_picture("cover_front", "other")
additional_data["upload_source"] = upload.source
track = get_track_from_import_metadata(final_metadata)
except UploadImportError as e:
return fail_import(upload, e.code)
@ -276,43 +287,45 @@ def federation_audio_track_to_metadata(payload):
Given a valid payload as returned by federation.serializers.TrackSerializer.validated_data,
returns a correct metadata payload for use with get_track_from_import_metadata.
"""
musicbrainz_recordingid = payload.get("musicbrainzId")
musicbrainz_artistid = payload["artists"][0].get("musicbrainzId")
musicbrainz_albumartistid = payload["album"]["artists"][0].get("musicbrainzId")
musicbrainz_albumid = payload["album"].get("musicbrainzId")
new_data = {
"title": payload["name"],
"album": payload["album"]["name"],
"track_number": payload.get("position") or 1,
"position": payload.get("position") or 1,
"disc_number": payload.get("disc"),
"artist": payload["artists"][0]["name"],
"album_artist": payload["album"]["artists"][0]["name"],
"date": payload["album"].get("released"),
"license": payload.get("license"),
"copyright": payload.get("copyright"),
# musicbrainz
"musicbrainz_recordingid": str(musicbrainz_recordingid)
if musicbrainz_recordingid
else None,
"musicbrainz_artistid": str(musicbrainz_artistid)
if musicbrainz_artistid
else None,
"musicbrainz_albumartistid": str(musicbrainz_albumartistid)
if musicbrainz_albumartistid
else None,
"musicbrainz_albumid": str(musicbrainz_albumid)
if musicbrainz_albumid
"mbid": str(payload.get("musicbrainzId"))
if payload.get("musicbrainzId")
else None,
"album": {
"title": payload["album"]["name"],
"fdate": payload["album"]["published"],
"fid": payload["album"]["id"],
"mbid": str(payload["album"]["musicbrainzId"])
if payload["album"].get("musicbrainzId")
else None,
"release_date": payload["album"].get("released"),
"artists": [
{
"fid": a["id"],
"name": a["name"],
"fdate": a["published"],
"mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None,
}
for a in payload["album"]["artists"]
],
},
"artists": [
{
"fid": a["id"],
"name": a["name"],
"fdate": a["published"],
"mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None,
}
for a in payload["artists"]
],
# federation
"fid": payload["id"],
"artist_fid": payload["artists"][0]["id"],
"album_artist_fid": payload["album"]["artists"][0]["id"],
"album_fid": payload["album"]["id"],
"fdate": payload["published"],
"album_fdate": payload["album"]["published"],
"album_artist_fdate": payload["album"]["artists"][0]["published"],
"artist_fdate": payload["artists"][0]["published"],
}
cover = payload["album"].get("cover")
if cover:
@ -405,8 +418,8 @@ def _get_track(data):
return track
from_activity_id = data.get("from_activity_id", None)
track_mbid = data.get("musicbrainz_recordingid", None)
album_mbid = data.get("musicbrainz_albumid", None)
track_mbid = data.get("mbid", None)
album_mbid = getter(data, "album", "mbid")
track_fid = getter(data, "fid")
query = None
@ -428,9 +441,12 @@ def _get_track(data):
pass
# get / create artist and album artist
artist_mbid = data.get("musicbrainz_artistid", None)
artist_fid = data.get("artist_fid", None)
artist_name = data["artist"]
artists = getter(data, "artists", default=[])
artist = artists[0]
artist_mbid = artist.get("mbid", None)
artist_fid = artist.get("fid", None)
artist_name = artist["name"]
if artist_mbid:
query = Q(mbid=artist_mbid)
else:
@ -443,20 +459,22 @@ def _get_track(data):
"fid": artist_fid,
"from_activity_id": from_activity_id,
}
if data.get("artist_fdate"):
defaults["creation_date"] = data.get("artist_fdate")
if artist.get("fdate"):
defaults["creation_date"] = artist.get("fdate")
artist = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
)[0]
album_artist_name = data.get("album_artist") or artist_name
album_artists = getter(data, "album", "artists", default=artists)
album_artist = album_artists[0]
album_artist_name = album_artist.get("name")
if album_artist_name == artist_name:
album_artist = artist
else:
query = Q(name__iexact=album_artist_name)
album_artist_mbid = data.get("musicbrainz_albumartistid", None)
album_artist_fid = data.get("album_artist_fid", None)
album_artist_mbid = album_artist.get("mbid", None)
album_artist_fid = album_artist.get("fid", None)
if album_artist_mbid:
query |= Q(mbid=album_artist_mbid)
if album_artist_fid:
@ -467,16 +485,17 @@ def _get_track(data):
"fid": album_artist_fid,
"from_activity_id": from_activity_id,
}
if data.get("album_artist_fdate"):
defaults["creation_date"] = data.get("album_artist_fdate")
if album_artist.get("fdate"):
defaults["creation_date"] = album_artist.get("fdate")
album_artist = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
)[0]
# get / create album
album_title = data["album"]
album_fid = data.get("album_fid", None)
album = data["album"]
album_title = album["title"]
album_fid = album.get("fid", None)
if album_mbid:
query = Q(mbid=album_mbid)
@ -489,12 +508,12 @@ def _get_track(data):
"title": album_title,
"artist": album_artist,
"mbid": album_mbid,
"release_date": data.get("date"),
"release_date": album.get("release_date"),
"fid": album_fid,
"from_activity_id": from_activity_id,
}
if data.get("album_fdate"):
defaults["creation_date"] = data.get("album_fdate")
if album.get("fdate"):
defaults["creation_date"] = album.get("fdate")
album = get_best_candidate_or_create(
models.Album, query, defaults=defaults, sort_fields=["mbid", "fid"]
@ -502,10 +521,8 @@ def _get_track(data):
# get / create track
track_title = data["title"]
track_number = data.get("track_number", 1)
query = Q(
title__iexact=track_title, artist=artist, album=album, position=track_number
)
position = data.get("position", 1)
query = Q(title__iexact=track_title, artist=artist, album=album, position=position)
if track_mbid:
query |= Q(mbid=track_mbid)
if track_fid:
@ -515,7 +532,7 @@ def _get_track(data):
"album": album,
"mbid": track_mbid,
"artist": artist,
"position": track_number,
"position": position,
"disc_number": data.get("disc_number"),
"fid": track_fid,
"from_activity_id": from_activity_id,

416
api/tests/music/test_metadata.py
View File

@ -11,45 +11,22 @@ from funkwhale_api.music import metadata
DATA_DIR = os.path.dirname(os.path.abspath(__file__))
def test_get_all_metadata_at_once():
path = os.path.join(DATA_DIR, "test.ogg")
data = metadata.Metadata(path)
expected = {
"title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
"artist": "Edvard Grieg",
"album_artist": "Edvard Grieg",
"album": "Peer Gynt Suite no. 1, op. 46",
"date": datetime.date(2012, 8, 15),
"track_number": 1,
"disc_number": 1,
"musicbrainz_albumid": uuid.UUID("a766da8b-8336-47aa-a3ee-371cc41ccc75"),
"musicbrainz_recordingid": uuid.UUID("bd21ac48-46d8-4e78-925f-d9cc2a294656"),
"musicbrainz_artistid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
"musicbrainz_albumartistid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
"license": "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/",
"copyright": "Someone",
}
assert data.all() == expected
@pytest.mark.parametrize(
"field,value",
[
("title", "Peer Gynt Suite no. 1, op. 46: I. Morning"),
("artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg; Musopen Symphony Orchestra"),
("album", "Peer Gynt Suite no. 1, op. 46"),
("date", datetime.date(2012, 8, 15)),
("track_number", 1),
("disc_number", 1),
("musicbrainz_albumid", uuid.UUID("a766da8b-8336-47aa-a3ee-371cc41ccc75")),
("musicbrainz_recordingid", uuid.UUID("bd21ac48-46d8-4e78-925f-d9cc2a294656")),
("musicbrainz_artistid", uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823")),
("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",
uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
"013c8e5b-d72a-4cd3-8dee-6c64d6125823;5b4d7d2d-36df-4b38-95e3-a964234f520f",
),
("license", "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/"),
("copyright", "Someone"),
@ -62,22 +39,44 @@ def test_can_get_metadata_from_ogg_file(field, value):
assert data.get(field) == value
def test_can_get_metadata_all():
path = os.path.join(DATA_DIR, "test.ogg")
data = metadata.Metadata(path)
expected = {
"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",
}
assert data.all() == expected
@pytest.mark.parametrize(
"field,value",
[
("title", "Peer Gynt Suite no. 1, op. 46: I. Morning"),
("artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg; Musopen Symphony Orchestra"),
("album", "Peer Gynt Suite no. 1, op. 46"),
("date", datetime.date(2012, 8, 15)),
("track_number", 1),
("disc_number", 1),
("musicbrainz_albumid", uuid.UUID("a766da8b-8336-47aa-a3ee-371cc41ccc75")),
("musicbrainz_recordingid", uuid.UUID("bd21ac48-46d8-4e78-925f-d9cc2a294656")),
("musicbrainz_artistid", uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823")),
("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",
uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
"013c8e5b-d72a-4cd3-8dee-6c64d6125823;5b4d7d2d-36df-4b38-95e3-a964234f520f",
),
("license", "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/"),
("copyright", "Someone"),
@ -97,16 +96,13 @@ def test_can_get_metadata_from_opus_file(field, value):
("artist", "Die Toten Hosen"),
("album_artist", "Die Toten Hosen"),
("album", "Ballast der Republik"),
("date", datetime.date(2012, 5, 4)),
("track_number", 1),
("disc_number", 1),
("musicbrainz_albumid", uuid.UUID("1f0441ad-e609-446d-b355-809c445773cf")),
("musicbrainz_recordingid", uuid.UUID("124d0150-8627-46bc-bc14-789a3bc960c8")),
("musicbrainz_artistid", uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1")),
(
"musicbrainz_albumartistid",
uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"),
),
("date", "2012-05-04"),
("position", "1/16"),
("disc_number", "1/2"),
("musicbrainz_albumid", "1f0441ad-e609-446d-b355-809c445773cf"),
("mbid", "124d0150-8627-46bc-bc14-789a3bc960c8"),
("musicbrainz_artistid", "c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"),
("musicbrainz_albumartistid", "c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"),
# somehow, I cannot successfully create an ogg theora file
# with the proper license field
# ("license", "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/"),
@ -126,16 +122,13 @@ def test_can_get_metadata_from_ogg_theora_file(field, value):
("artist", "Binärpilot"),
("album_artist", "Binärpilot"),
("album", "You Can't Stop Da Funk"),
("date", datetime.date(2006, 2, 7)),
("track_number", 2),
("disc_number", 1),
("musicbrainz_albumid", uuid.UUID("ce40cdb1-a562-4fd8-a269-9269f98d4124")),
("musicbrainz_recordingid", uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb")),
("musicbrainz_artistid", uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13")),
(
"musicbrainz_albumartistid",
uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
),
("date", "2006-02-07"),
("position", "2/4"),
("disc_number", "1/1"),
("musicbrainz_albumid", "ce40cdb1-a562-4fd8-a269-9269f98d4124"),
("mbid", "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"),
("musicbrainz_artistid", "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
("musicbrainz_albumartistid", "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
("license", "https://creativecommons.org/licenses/by-nc-nd/2.5/"),
("copyright", "Someone"),
],
@ -144,7 +137,7 @@ def test_can_get_metadata_from_id3_mp3_file(field, value):
path = os.path.join(DATA_DIR, "test.mp3")
data = metadata.Metadata(path)
assert data.get(field) == value
assert str(data.get(field)) == value
@pytest.mark.parametrize(
@ -170,16 +163,13 @@ def test_can_get_pictures(name):
("artist", "Nine Inch Nails"),
("album_artist", "Nine Inch Nails"),
("album", "The Slip"),
("date", datetime.date(2008, 5, 5)),
("track_number", 1),
("disc_number", 1),
("musicbrainz_albumid", uuid.UUID("12b57d46-a192-499e-a91f-7da66790a1c1")),
("musicbrainz_recordingid", uuid.UUID("30f3f33e-8d0c-4e69-8539-cbd701d18f28")),
("musicbrainz_artistid", uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da")),
(
"musicbrainz_albumartistid",
uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da"),
),
("date", "2008-05-05"),
("position", "1"),
("disc_number", "1"),
("musicbrainz_albumid", "12b57d46-a192-499e-a91f-7da66790a1c1"),
("mbid", "30f3f33e-8d0c-4e69-8539-cbd701d18f28"),
("musicbrainz_artistid", "b7ffd2af-418f-4be2-bdd1-22f8b48613da"),
("musicbrainz_albumartistid", "b7ffd2af-418f-4be2-bdd1-22f8b48613da"),
("license", "http://creativecommons.org/licenses/by-nc-sa/3.0/us/"),
("copyright", "2008 nin"),
],
@ -199,54 +189,18 @@ def test_can_get_metadata_from_flac_file_not_crash_if_empty():
data.get("test")
@pytest.mark.parametrize(
"field_name",
[
"musicbrainz_artistid",
"musicbrainz_albumid",
"musicbrainz_recordingid",
"musicbrainz_albumartistid",
],
)
def test_mbid_clean_keeps_only_first(field_name):
u1 = str(uuid.uuid4())
u2 = str(uuid.uuid4())
field = metadata.VALIDATION[field_name]
result = field.to_python("/".join([u1, u2]))
assert str(result) == u1
@pytest.mark.parametrize(
"raw,expected",
[
("2017", datetime.date(2017, 1, 1)),
("2017-12-31", datetime.date(2017, 12, 31)),
("2017-14-01 01:32", datetime.date(2017, 1, 14)), # deezer format
("2017-02", datetime.date(2017, 1, 1)), # weird format that exists
("nonsense", None),
],
)
def test_date_parsing(raw, expected):
assert metadata.get_date(raw) == expected
def test_date_parsing_failure():
with pytest.raises(metadata.ParseError):
metadata.get_date("noop")
def test_metadata_all_ignore_parse_errors_true(mocker):
path = os.path.join(DATA_DIR, "sample.flac")
data = metadata.Metadata(path)
mocker.patch.object(data, "get", side_effect=metadata.ParseError("Failure"))
assert data.all()["date"] is None
def test_metadata_all_ignore_parse_errors_false(mocker):
path = os.path.join(DATA_DIR, "sample.flac")
data = metadata.Metadata(path)
mocker.patch.object(data, "get", side_effect=metadata.ParseError("Failure"))
with pytest.raises(metadata.ParseError):
data.all(ignore_parse_errors=False)
assert metadata.PermissiveDateField().to_internal_value(raw) == expected
def test_metadata_fallback_ogg_theora(mocker):
@ -264,3 +218,247 @@ def test_metadata_fallback_ogg_theora(mocker):
assert data.get("pictures", "default") == expected_result
fallback_get.assert_called_once_with("pictures", "default")
@pytest.mark.parametrize(
"path, expected",
[
(
"test.mp3",
{
"title": "Bend",
"artists": [
{
"name": "Binärpilot",
"mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
}
],
"album": {
"title": "You Can't Stop Da Funk",
"mbid": uuid.UUID("ce40cdb1-a562-4fd8-a269-9269f98d4124"),
"release_date": datetime.date(2006, 2, 7),
"artists": [
{
"name": "Binärpilot",
"mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
}
],
},
"position": 2,
"disc_number": 1,
"mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"),
"license": "https://creativecommons.org/licenses/by-nc-nd/2.5/",
"copyright": "Someone",
},
),
(
"test.ogg",
{
"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",
},
),
(
"test.opus",
{
"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",
},
),
(
"test_theora.ogg",
{
"title": "Drei Kreuze (dass wir hier sind)",
"artists": [
{
"name": "Die Toten Hosen",
"mbid": uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"),
}
],
"album": {
"title": "Ballast der Republik",
"mbid": uuid.UUID("1f0441ad-e609-446d-b355-809c445773cf"),
"release_date": datetime.date(2012, 5, 4),
"artists": [
{
"name": "Die Toten Hosen",
"mbid": uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"),
}
],
},
"position": 1,
"disc_number": 1,
"mbid": uuid.UUID("124d0150-8627-46bc-bc14-789a3bc960c8"),
# somehow, I cannot successfully create an ogg theora file
# with the proper license field
# ("license", "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/"),
"copyright": "℗ 2012 JKP GmbH & Co. KG",
},
),
(
"sample.flac",
{
"title": "999,999",
"artists": [
{
"name": "Nine Inch Nails",
"mbid": uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da"),
}
],
"album": {
"title": "The Slip",
"mbid": uuid.UUID("12b57d46-a192-499e-a91f-7da66790a1c1"),
"release_date": datetime.date(2008, 5, 5),
"artists": [
{
"name": "Nine Inch Nails",
"mbid": uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da"),
}
],
},
"position": 1,
"disc_number": 1,
"mbid": uuid.UUID("30f3f33e-8d0c-4e69-8539-cbd701d18f28"),
"license": "http://creativecommons.org/licenses/by-nc-sa/3.0/us/",
"copyright": "2008 nin",
},
),
],
)
def test_track_metadata_serializer(path, expected, mocker):
path = os.path.join(DATA_DIR, path)
data = metadata.Metadata(path)
get_picture = mocker.patch.object(data, "get_picture")
expected["cover_data"] = get_picture.return_value
serializer = metadata.TrackMetadataSerializer(data=data)
assert serializer.is_valid(raise_exception=True) is True
assert serializer.validated_data == expected
get_picture.assert_called_once_with("cover_front", "other")
@pytest.mark.parametrize(
"raw, expected",
[
(
{
"names": "Hello; World",
"mbids": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb; f269d497-1cc0-4ae4-a0c4-157ec7d73fcd ",
},
[
{
"name": "Hello",
"mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"),
},
{
"name": "World",
"mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"),
},
],
),
(
{
"names": "Hello; World; Foo",
"mbids": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb; f269d497-1cc0-4ae4-a0c4-157ec7d73fcd ",
},
[
{
"name": "Hello",
"mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"),
},
{
"name": "World",
"mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"),
},
{"name": "Foo", "mbid": None},
],
),
],
)
def test_artists_cleaning(raw, expected):
field = metadata.ArtistField()
assert field.to_internal_value(raw) == expected
@pytest.mark.parametrize(
"data, errored_field",
[
({"name": "Hello", "mbid": "wrong-uuid"}, "mbid"), # wrong uuid
({"name": "", "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"}, "name"),
],
)
def test_artist_serializer_validation(data, errored_field):
serializer = metadata.ArtistSerializer(data=data)
assert serializer.is_valid() is False
assert len(serializer.errors) == 1
assert errored_field in serializer.errors
@pytest.mark.parametrize(
"data, errored_field",
[
({"title": "Hello", "mbid": "wrong"}, "mbid"), # wrong uuid
(
{"title": "", "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"},
"title",
), # empty title
],
)
def test_album_serializer_validation(data, errored_field):
serializer = metadata.AlbumSerializer(data=data)
assert serializer.is_valid() is False
assert len(serializer.errors) == 1
assert errored_field in serializer.errors

235
api/tests/music/test_tasks.py
View File

@ -20,15 +20,13 @@ DATA_DIR = os.path.dirname(os.path.abspath(__file__))
def test_can_create_track_from_file_metadata_no_mbid(db, mocker):
metadata = {
"title": "Test track",
"artist": "Test artist",
"album": "Test album",
"date": datetime.date(2012, 8, 15),
"track_number": 4,
"artists": [{"name": "Test artist"}],
"album": {"title": "Test album", "release_date": datetime.date(2012, 8, 15)},
"position": 4,
"disc_number": 2,
"license": "Hello world: http://creativecommons.org/licenses/by-sa/4.0/",
"copyright": "2018 Someone",
}
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
match_license = mocker.spy(licenses, "match")
track = tasks.get_track_from_import_metadata(metadata)
@ -39,10 +37,10 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker):
assert track.disc_number == 2
assert track.license.code == "cc-by-sa-4.0"
assert track.copyright == metadata["copyright"]
assert track.album.title == metadata["album"]
assert track.album.title == metadata["album"]["title"]
assert track.album.mbid is None
assert track.album.release_date == datetime.date(2012, 8, 15)
assert track.artist.name == metadata["artist"]
assert track.artist.name == metadata["artists"][0]["name"]
assert track.artist.mbid is None
match_license.assert_called_once_with(metadata["license"], metadata["copyright"])
@ -50,33 +48,38 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker):
def test_can_create_track_from_file_metadata_mbid(factories, mocker):
metadata = {
"title": "Test track",
"artist": "Test artist",
"album_artist": "Test album artist",
"album": "Test album",
"date": datetime.date(2012, 8, 15),
"track_number": 4,
"musicbrainz_albumid": "ce40cdb1-a562-4fd8-a269-9269f98d4124",
"musicbrainz_recordingid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb",
"musicbrainz_artistid": "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13",
"musicbrainz_albumartistid": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e13",
"artists": [
{"name": "Test artist", "mbid": "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"}
],
"album": {
"title": "Test album",
"release_date": datetime.date(2012, 8, 15),
"mbid": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e15",
"artists": [
{
"name": "Test album artist",
"mbid": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e13",
}
],
},
"position": 4,
"mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb",
"cover_data": {"content": b"image_content", "mimetype": "image/png"},
}
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
track = tasks.get_track_from_import_metadata(metadata)
assert track.title == metadata["title"]
assert track.mbid == metadata["musicbrainz_recordingid"]
assert track.mbid == metadata["mbid"]
assert track.position == 4
assert track.disc_number is None
assert track.album.title == metadata["album"]
assert track.album.mbid == metadata["musicbrainz_albumid"]
assert track.album.artist.mbid == metadata["musicbrainz_albumartistid"]
assert track.album.artist.name == metadata["album_artist"]
assert track.album.title == metadata["album"]["title"]
assert track.album.mbid == metadata["album"]["mbid"]
assert track.album.artist.mbid == metadata["album"]["artists"][0]["mbid"]
assert track.album.artist.name == metadata["album"]["artists"][0]["name"]
assert track.album.release_date == datetime.date(2012, 8, 15)
assert track.artist.name == metadata["artist"]
assert track.artist.mbid == metadata["musicbrainz_artistid"]
assert track.artist.name == metadata["artists"][0]["name"]
assert track.artist.mbid == metadata["artists"][0]["mbid"]
def test_can_create_track_from_file_metadata_mbid_existing_album_artist(
@ -85,22 +88,21 @@ def test_can_create_track_from_file_metadata_mbid_existing_album_artist(
artist = factories["music.Artist"]()
album = factories["music.Album"]()
metadata = {
"artist": "",
"album": "",
"album": {
"mbid": album.mbid,
"title": "",
"artists": [{"name": "", "mbid": album.artist.mbid}],
},
"title": "Hello",
"track_number": 4,
"musicbrainz_albumid": album.mbid,
"musicbrainz_recordingid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb",
"musicbrainz_artistid": artist.mbid,
"musicbrainz_albumartistid": album.artist.mbid,
"position": 4,
"artists": [{"mbid": artist.mbid, "name": ""}],
"mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb",
}
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
track = tasks.get_track_from_import_metadata(metadata)
assert track.title == metadata["title"]
assert track.mbid == metadata["musicbrainz_recordingid"]
assert track.mbid == metadata["mbid"]
assert track.position == 4
assert track.album == album
assert track.artist == artist
@ -112,18 +114,17 @@ def test_can_create_track_from_file_metadata_fid_existing_album_artist(
artist = factories["music.Artist"]()
album = factories["music.Album"]()
metadata = {
"artist": "",
"album": "",
"artists": [{"name": "", "fid": artist.fid}],
"album": {
"title": "",
"fid": album.fid,
"artists": [{"name": "", "fid": album.artist.fid}],
},
"title": "Hello",
"track_number": 4,
"position": 4,
"fid": "https://hello",
"album_fid": album.fid,
"artist_fid": artist.fid,
"album_artist_fid": album.artist.fid,
}
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
track = tasks.get_track_from_import_metadata(metadata)
assert track.title == metadata["title"]
@ -139,13 +140,11 @@ def test_can_create_track_from_file_metadata_distinct_release_mbid(factories):
album = factories["music.Album"](artist=artist)
track = factories["music.Track"](album=album, artist=artist)
metadata = {
"artist": artist.name,
"album": album.title,
"artists": [{"name": artist.name, "mbid": artist.mbid}],
"album": {"title": album.title, "mbid": str(uuid.uuid4())},
"title": track.title,
"track_number": 4,
"position": 4,
"fid": "https://hello",
"musicbrainz_artistid": artist.mbid,
"musicbrainz_albumid": str(uuid.uuid4()),
}
new_track = tasks.get_track_from_import_metadata(metadata)
@ -162,12 +161,10 @@ def test_can_create_track_from_file_metadata_distinct_position(factories):
album = factories["music.Album"](artist=artist)
track = factories["music.Track"](album=album, artist=artist)
metadata = {
"artist": artist.name,
"album": album.title,
"artists": [{"name": artist.name, "mbid": artist.mbid}],
"album": {"title": album.title, "mbid": album.mbid},
"title": track.title,
"track_number": track.position + 1,
"musicbrainz_artistid": artist.mbid,
"musicbrainz_albumid": album.mbid,
"position": track.position + 1,
}
new_track = tasks.get_track_from_import_metadata(metadata)
@ -177,23 +174,28 @@ def test_can_create_track_from_file_metadata_distinct_position(factories):
def test_can_create_track_from_file_metadata_federation(factories, mocker, r_mock):
metadata = {
"artist": "Artist",
"album": "Album",
"album_artist": "Album artist",
"artists": [
{"name": "Artist", "fid": "https://artist.fid", "fdate": timezone.now()}
],
"album": {
"title": "Album",
"fid": "https://album.fid",
"fdate": timezone.now(),
"artists": [
{
"name": "Album artist",
"fid": "https://album.artist.fid",
"fdate": timezone.now(),
}
],
},
"title": "Hello",
"track_number": 4,
"position": 4,
"fid": "https://hello",
"album_fid": "https://album.fid",
"artist_fid": "https://artist.fid",
"album_artist_fid": "https://album.artist.fid",
"fdate": timezone.now(),
"album_fdate": timezone.now(),
"album_artist_fdate": timezone.now(),
"artist_fdate": timezone.now(),
"cover_data": {"url": "https://cover/hello.png", "mimetype": "image/png"},
}
r_mock.get(metadata["cover_data"]["url"], body=io.BytesIO(b"coucou"))
mocker.patch("funkwhale_api.music.metadata.Metadata.all", return_value=metadata)
track = tasks.get_track_from_import_metadata(metadata, update_cover=True)
@ -203,15 +205,15 @@ def test_can_create_track_from_file_metadata_federation(factories, mocker, r_moc
assert track.position == 4
assert track.album.cover.read() == b"coucou"
assert track.album.cover.path.endswith(".png")
assert track.album.fid == metadata["album_fid"]
assert track.album.title == metadata["album"]
assert track.album.creation_date == metadata["album_fdate"]
assert track.album.artist.fid == metadata["album_artist_fid"]
assert track.album.artist.name == metadata["album_artist"]
assert track.album.artist.creation_date == metadata["album_artist_fdate"]
assert track.artist.fid == metadata["artist_fid"]
assert track.artist.name == metadata["artist"]
assert track.artist.creation_date == metadata["artist_fdate"]
assert track.album.fid == metadata["album"]["fid"]
assert track.album.title == metadata["album"]["title"]
assert track.album.creation_date == metadata["album"]["fdate"]
assert track.album.artist.fid == metadata["album"]["artists"][0]["fid"]
assert track.album.artist.name == metadata["album"]["artists"][0]["name"]
assert track.album.artist.creation_date == metadata["album"]["artists"][0]["fdate"]
assert track.artist.fid == metadata["artists"][0]["fid"]
assert track.artist.name == metadata["artists"][0]["name"]
assert track.artist.creation_date == metadata["artists"][0]["fdate"]
def test_sort_candidates(factories):
@ -391,7 +393,38 @@ def test_upload_import_error(factories, now, temp_signal):
assert upload.import_status == "errored"
assert upload.import_date == now
assert upload.import_details == {"error_code": "track_uuid_not_found"}
assert upload.import_details == {
"error_code": "track_uuid_not_found",
"detail": None,
}
handler.assert_called_once_with(
upload=upload,
old_status="pending",
new_status="errored",
sender=None,
signal=signals.upload_import_status_updated,
)
def test_upload_import_error_metadata(factories, now, temp_signal, mocker):
path = os.path.join(DATA_DIR, "test.ogg")
upload = factories["music.Upload"](audio_file__frompath=path)
mocker.patch.object(
metadata.AlbumField,
"to_internal_value",
side_effect=metadata.serializers.ValidationError("Hello"),
)
with temp_signal(signals.upload_import_status_updated) as handler:
tasks.process_upload(upload_id=upload.pk)
upload.refresh_from_db()
assert upload.import_status == "errored"
assert upload.import_date == now
assert upload.import_details == {
"error_code": "invalid_metadata",
"detail": {"album": ["Hello"]},
"file_metadata": metadata.Metadata(path).all(),
}
handler.assert_called_once_with(
upload=upload,
old_status="pending",
@ -494,31 +527,43 @@ def test_federation_audio_track_to_metadata(now):
serializer = federation_serializers.TrackSerializer(data=payload)
serializer.is_valid(raise_exception=True)
expected = {
"artist": payload["artists"][0]["name"],
"album": payload["album"]["name"],
"album_artist": payload["album"]["artists"][0]["name"],
"title": payload["name"],
"date": released,
"track_number": payload["position"],
"position": payload["position"],
"disc_number": payload["disc"],
"license": "http://creativecommons.org/licenses/by-sa/4.0/",
"copyright": "2018 Someone",
# musicbrainz
"musicbrainz_albumid": payload["album"]["musicbrainzId"],
"musicbrainz_recordingid": payload["musicbrainzId"],
"musicbrainz_artistid": payload["artists"][0]["musicbrainzId"],
"musicbrainz_albumartistid": payload["album"]["artists"][0]["musicbrainzId"],
# federation
"fid": payload["id"],
"album_fid": payload["album"]["id"],
"artist_fid": payload["artists"][0]["id"],
"album_artist_fid": payload["album"]["artists"][0]["id"],
"mbid": payload["musicbrainzId"],
"fdate": serializer.validated_data["published"],
"artist_fdate": serializer.validated_data["artists"][0]["published"],
"album_artist_fdate": serializer.validated_data["album"]["artists"][0][
"published"
"fid": payload["id"],
"album": {
"title": payload["album"]["name"],
"release_date": released,
"mbid": payload["album"]["musicbrainzId"],
"fid": payload["album"]["id"],
"fdate": serializer.validated_data["album"]["published"],
"artists": [
{
"name": a["name"],
"mbid": a["musicbrainzId"],
"fid": a["id"],
"fdate": serializer.validated_data["album"]["artists"][i][
"published"
],
}
for i, a in enumerate(payload["album"]["artists"])
],
},
# musicbrainz
# federation
"artists": [
{
"name": a["name"],
"mbid": a["musicbrainzId"],
"fid": a["id"],
"fdate": serializer.validated_data["artists"][i]["published"],
}
for i, a in enumerate(payload["artists"])
],
"album_fdate": serializer.validated_data["album"]["published"],
"cover_data": {
"mimetype": serializer.validated_data["album"]["cover"]["mediaType"],
"url": serializer.validated_data["album"]["cover"]["href"],
@ -528,10 +573,6 @@ def test_federation_audio_track_to_metadata(now):
result = tasks.federation_audio_track_to_metadata(serializer.validated_data)
assert result == expected
# ensure we never forget to test a mandatory field
for k in metadata.ALL_FIELDS:
assert k in result
def test_scan_library_fetches_page_and_calls_scan_page(now, mocker, factories, r_mock):
scan = factories["music.LibraryScan"]()

1
changes/changelog.d/252.feature Normal file
View File

@ -0,0 +1 @@
Improved error handling and display during import (#252, #718, #583, #501, #544)

1
changes/changelog.d/718.bugfix Normal file
View File

@ -0,0 +1 @@
Fixed crashing upload processing on invalid date format (#718)

9
changes/notes.rst
View File

@ -35,6 +35,15 @@ enabled in a future release).
If you want to start building an app on top of Funkwhale's API, please check-out
`https://docs.funkwhale.audio/api.html`_ and `https://docs.funkwhale.audio/developers/authentication.html`_.
Better error handling and display during import
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Funkwhale should now be more resilient to missing tags in imported files, and give
you more insights when something goes wrong, including the specific tags that were missing
or invalid, and additional debug information to share in your support requests.
This information is available in all pages that list uploads, when clicking on the button next to the upload status.
Prune library command
^^^^^^^^^^^^^^^^^^^^^

27
docs/users/upload.rst
View File

@ -154,3 +154,30 @@ Then select the files you want to delete using the checkboxes on the left ; you
Finally, select "Delete" in the "Action" menu and click "Go".
This operation does *not* remove metadata, meaning that deleted tracks will remain visible in your library. They just won't be playable anymore.
Common errors during import
---------------------------
.. _invalid_metadata:
Invalid metadata
::::::::::::::::
This error occurs when the uploaded file miss some mandatory tags, or when some tags have
incorrect values (e.g an empty title or artist name).
To fix this issue, please retag the file properly as described in :ref:`upload-tagging`
and reupload it.
.. _unknown_error:
Unkwown error
:::::::::::::
This error can happen for multiple reasons and likely indicates an issue with the Funkwhale
server (e.g. misconfiguration) or with Funkwhale itself.
If the issue persists when you relaunch the import, get in touch with our instance admin
or open a support thread on our forum.

150
front/src/views/content/libraries/FilesTable.vue
View File

@ -35,6 +35,88 @@
</div>
</div>
</div>
<modal :show.sync="showUploadDetailModal">
<div class="header">
<translate translate-context="Popup/Import/Title">Import detail</translate>
</div>
<div class="content" v-if="detailedUpload">
<div class="description">
<div class="ui message" v-if="detailedUpload.import_status === 'pending'">
<translate translate-context="Popup/Import/Message">Upload is still pending and will soon be processed by the server.</translate>
</div>
<div class="ui success message" v-if="detailedUpload.import_status === 'finished'">
<translate translate-context="Popup/Import/Message">Upload was successfully processed by the server.</translate>
</div>
<div class="ui warning message" v-if="detailedUpload.import_status === 'skipped'">
<translate translate-context="Popup/Import/Message">Upload was skipped because a similar one is already available in one of your libraries.</translate>
</div>
<div class="ui error message" v-if="detailedUpload.import_status === 'errored'">
<translate translate-context="Popup/Import/Message">An error occured during upload processing. You will find more information below.</translate>
</div>
<template v-if="detailedUpload.import_status === 'errored'">
<table class="ui very basic collapsing celled table">
<tbody>
<tr>
<td>
<translate translate-context="Popup/Import/Table.Label/Noun">Error type</translate>
</td>
<td>
{{ getErrorData(detailedUpload).label }}
</td>
</tr>
<tr>
<td>
<translate translate-context="Popup/Import/Table.Label/Noun">Error detail</translate>
</td>
<td>
{{ getErrorData(detailedUpload).detail }}
<ul v-if="getErrorData(detailedUpload).errorRows.length > 0">
<li v-for="row in getErrorData(detailedUpload).errorRows">
{{ row.key}}: {{ row.value}}
</li>
</ul>
</td>
</tr>
<tr>
<td>
<translate translate-context="Popup/Import/Table.Label/Noun">Getting help</translate>
</td>
<td>
<ul>
<li>
<a :href="getErrorData(detailedUpload).documentationUrl" target="_blank">
<translate translate-context="Popup/Import/Table.Label/Value">Read our documentation for this error</translate>
</a>
</li>
<li>
<a :href="getErrorData(detailedUpload).supportUrl" target="_blank">
<translate translate-context="Popup/Import/Table.Label/Value">Open a support thread (include the debug information below in your message)</translate>
</a>
</li>
</ul>
</td>
</tr>
<tr>
<td>
<translate translate-context="Popup/Import/Table.Label/Noun">Debug information</translate>
</td>
<td>
<div class="ui form">
<textarea class="ui textarea" rows="10" :value="getErrorData(detailedUpload).debugInfo"></textarea>
</div>
</td>
</tr>
</tbody>
</table>
</template>
</div>
</div>
<div class="actions">
<div class="ui deny button">
<translate translate-context="*/*/Button.Label/Verb">Close</translate>
</div>
</div>
</modal>
<div class="dimmable">
<div v-if="isLoading" class="ui active inverted dimmer">
<div class="ui loader"></div>
@ -80,11 +162,13 @@
<td>
<human-date :date="scope.obj.creation_date"></human-date>
</td>
<td :title="labels.importStatuses[scope.obj.import_status].help">
<span class="discrete link" @click="addSearchToken('status', scope.obj.import_status)">
<td>
<span class="discrete link" @click="addSearchToken('status', scope.obj.import_status)" :title="labels.importStatuses[scope.obj.import_status].help">
{{ labels.importStatuses[scope.obj.import_status].label }}
<i class="question circle outline icon"></i>
</span>
<button class="ui tiny basic icon button" :title="labels.statusDetailTitle" @click="detailedUpload = scope.obj; showUploadDetailModal = true">
<i class="question circle outline icon"></i>
</button>
</td>
<td v-if="scope.obj.duration">
{{ time.parse(scope.obj.duration) }}
@ -132,7 +216,33 @@ import ActionTable from '@/components/common/ActionTable'
import OrderingMixin from '@/components/mixins/Ordering'
import TranslationsMixin from '@/components/mixins/Translations'
import SmartSearchMixin from '@/components/mixins/SmartSearch'
import Modal from '@/components/semantic/Modal'
function getErrors(payload) {
let errors = []
for (var k in payload) {
if (payload.hasOwnProperty(k)) {
let value = payload[k]
if (Array.isArray(value)) {
errors.push({
key: k,
value: value.join(', ')
})
} else {
// possibly artists, so nested errors
if (typeof value === 'object') {
getErrors(value).forEach((e) => {
errors.push({
key: `${k} / ${e.key}`,
value: e.value
})
})
}
}
}
}
return errors
}
export default {
mixins: [OrderingMixin, TranslationsMixin, SmartSearchMixin],
props: {
@ -142,11 +252,14 @@ export default {
},
components: {
Pagination,
ActionTable
ActionTable,
Modal
},
data () {
return {
time,
detailedUpload: null,
showUploadDetailModal: false,
isLoading: false,
result: null,
page: 1,
@ -193,12 +306,41 @@ export default {
},
selectPage: function (page) {
this.page = page
},
getErrorData (upload) {
let payload = upload.import_details || {}
let d = {
supportUrl: 'https://governance.funkwhale.audio/g/246YOJ1m/funkwhale-support',
errorRows: []
}
if (!payload.error_code) {
d.errorCode = 'unknown_error'
} else {
d.errorCode = payload.error_code
}
d.documentationUrl = `https://docs.funkwhale.audio/users/upload.html#${d.errorCode}`
if (d.errorCode === 'invalid_metadata') {
d.label = this.$pgettext('Popup/Import/Error.Label', 'Invalid metadata')
d.detail = this.$pgettext('Popup/Import/Error.Label', 'The metadata included in the file is invalid or some mandatory fields are missing.')
let detail = payload.detail || {}
d.errorRows = getErrors(detail)
} else {
d.label = this.$pgettext('Popup/Import/Error.Label', 'Unkwown error')
d.detail = this.$pgettext('Popup/Import/Error.Label', 'An unkwown error occured')
}
let debugInfo = {
source: upload.source,
...payload,
}
d.debugInfo = JSON.stringify(debugInfo, null, 4)
return d
}
},
computed: {
labels () {
return {
searchPlaceholder: this.$pgettext('Content/Library/Input.Placeholder', 'Search by title, artist, album…'),
statusDetailTitle: this.$pgettext('Content/Library/Link.Title', 'Click to display more information about the import process for this upload'),
importStatuses: {
skipped: {
label: this.$pgettext('Content/Library/*', 'Skipped'),