Unverified Commit 0750aaca authored by Agate's avatar Agate 💬
Browse files

Fix #1104: invalid metadata when importing multi-artists tracks/albums

parent c8f9ac97
......@@ -189,6 +189,7 @@ CONF = {
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"artists": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date"},
......@@ -213,6 +214,7 @@ CONF = {
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"artists": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date"},
......@@ -237,6 +239,7 @@ CONF = {
"disc_number": {"field": "DISCNUMBER"},
"title": {},
"artist": {},
"artists": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date"},
......@@ -258,6 +261,7 @@ CONF = {
"disc_number": {"field": "TPOS"},
"title": {"field": "TIT2"},
"artist": {"field": "TPE1"},
"artists": {"field": "ARTISTS"},
"album_artist": {"field": "TPE2"},
"album": {"field": "TALB"},
"date": {"field": "TDRC"},
......@@ -280,6 +284,7 @@ CONF = {
"disc_number": {"field": "disk", "to_application": get_mp4_position},
"title": {"field": "©nam"},
"artist": {"field": "©ART"},
"artists": {"field": "----:com.apple.iTunes:ARTISTS"},
"album_artist": {"field": "aART"},
"album": {"field": "©alb"},
"date": {"field": "©day"},
......@@ -308,6 +313,7 @@ CONF = {
"disc_number": {"field": "discnumber"},
"title": {},
"artist": {},
"artists": {},
"album_artist": {"field": "albumartist"},
"album": {},
"date": {"field": "date"},
......@@ -468,9 +474,17 @@ class ArtistField(serializers.Field):
def get_value(self, data):
if self.for_album:
keys = [("names", "album_artist"), ("mbids", "musicbrainz_albumartistid")]
keys = [
("artists", "artists"),
("names", "album_artist"),
("mbids", "musicbrainz_albumartistid"),
]
else:
keys = [("names", "artist"), ("mbids", "musicbrainz_artistid")]
keys = [
("artists", "artists"),
("names", "artist"),
("mbids", "musicbrainz_artistid"),
]
final = {}
for field, key in keys:
......@@ -499,7 +513,16 @@ class ArtistField(serializers.Field):
# now, we split on artist names, using the same separator as the one used
# by mbids, if any
if used_separator and mbids:
names = []
if data.get("artists", None):
for separator in separators:
if separator in data["artists"]:
names = [n.strip() for n in data["artists"].split(separator)]
break
if not names:
names = [data["artists"]]
elif used_separator and mbids:
names = [n.strip() for n in data["names"].split(used_separator)]
else:
names = [data["names"]]
......
......@@ -517,43 +517,15 @@ def _get_track(data, attributed_to=None, **forced_values):
pass
# get / create artist and album artist
artists = getter(data, "artists", default=[])
if "artist" in forced_values:
artist = forced_values["artist"]
else:
artists = getter(data, "artists", default=[])
artist_data = artists[0]
artist_mbid = artist_data.get("mbid", None)
artist_fid = artist_data.get("fid", None)
artist_name = truncate(artist_data["name"], models.MAX_LENGTHS["ARTIST_NAME"])
if artist_mbid:
query = Q(mbid=artist_mbid)
else:
query = Q(name__iexact=artist_name)
if artist_fid:
query |= Q(fid=artist_fid)
defaults = {
"name": artist_name,
"mbid": artist_mbid,
"fid": artist_fid,
"from_activity_id": from_activity_id,
"attributed_to": artist_data.get("attributed_to", attributed_to),
}
if artist_data.get("fdate"):
defaults["creation_date"] = artist_data.get("fdate")
artist, created = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
artist = get_artist(
artist_data, attributed_to=attributed_to, from_activity_id=from_activity_id
)
if created:
tags_models.add_tags(artist, *artist_data.get("tags", []))
common_utils.attach_content(
artist, "description", artist_data.get("description")
)
common_utils.attach_file(
artist, "attachment_cover", artist_data.get("cover_data")
)
artist_name = artist.name
if "album" in forced_values:
album = forced_values["album"]
else:
......@@ -695,6 +667,12 @@ def _get_track(data, attributed_to=None, **forced_values):
if track_fid:
query |= Q(fid=track_fid)
if album and len(artists) > 1:
# we use the second artist to preserve featuring information
artist = artist = get_artist(
artists[1], attributed_to=attributed_to, from_activity_id=from_activity_id
)
defaults = {
"title": track_title,
"album": album,
......@@ -726,6 +704,41 @@ def _get_track(data, attributed_to=None, **forced_values):
return track
def get_artist(artist_data, attributed_to, from_activity_id):
artist_mbid = artist_data.get("mbid", None)
artist_fid = artist_data.get("fid", None)
artist_name = truncate(artist_data["name"], models.MAX_LENGTHS["ARTIST_NAME"])
if artist_mbid:
query = Q(mbid=artist_mbid)
else:
query = Q(name__iexact=artist_name)
if artist_fid:
query |= Q(fid=artist_fid)
defaults = {
"name": artist_name,
"mbid": artist_mbid,
"fid": artist_fid,
"from_activity_id": from_activity_id,
"attributed_to": artist_data.get("attributed_to", attributed_to),
}
if artist_data.get("fdate"):
defaults["creation_date"] = artist_data.get("fdate")
artist, created = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
)
if created:
tags_models.add_tags(artist, *artist_data.get("tags", []))
common_utils.attach_content(
artist, "description", artist_data.get("description")
)
common_utils.attach_file(
artist, "attachment_cover", artist_data.get("cover_data")
)
return artist
@receiver(signals.upload_import_status_updated)
def broadcast_import_status_update_to_owner(old_status, new_status, upload, **kwargs):
user = upload.library.actor.get_user()
......
No preview for this file type
No preview for this file type
No preview for this file type
......@@ -17,6 +17,7 @@ DATA_DIR = os.path.dirname(os.path.abspath(__file__))
("title", "Peer Gynt Suite no. 1, op. 46: I. Morning"),
("artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg; Musopen Symphony Orchestra"),
("artists", "Edvard Grieg; Musopen Symphony Orchestra"),
("album", "Peer Gynt Suite no. 1, op. 46"),
("date", "2012-08-15"),
("position", "1"),
......@@ -48,6 +49,7 @@ def test_can_get_metadata_all():
"title": "Peer Gynt Suite no. 1, op. 46: I. Morning",
"artist": "Edvard Grieg",
"album_artist": "Edvard Grieg; Musopen Symphony Orchestra",
"artists": "Edvard Grieg; Musopen Symphony Orchestra",
"album": "Peer Gynt Suite no. 1, op. 46",
"date": "2012-08-15",
"position": "1",
......@@ -70,6 +72,7 @@ def test_can_get_metadata_all():
("title", "Peer Gynt Suite no. 1, op. 46: I. Morning"),
("artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg; Musopen Symphony Orchestra"),
("artists", "Edvard Grieg; Musopen Symphony Orchestra"),
("album", "Peer Gynt Suite no. 1, op. 46"),
("date", "2012-08-15"),
("position", "1"),
......@@ -126,6 +129,7 @@ def test_can_get_metadata_from_ogg_theora_file(field, value):
("title", "Bend"),
("artist", "Binärpilot"),
("album_artist", "Binärpilot"),
("artists", "Binärpilot; Another artist"),
("album", "You Can't Stop Da Funk"),
("date", "2006-02-07"),
("position", "2/4"),
......@@ -202,6 +206,7 @@ def test_can_get_metadata_from_flac_file(field, value):
("title", "Peer Gynt Suite no. 1, op. 46: I. Morning"),
("artist", "Edvard Grieg"),
("album_artist", "Edvard Grieg; Musopen Symphony Orchestra"),
("artists", "Edvard Grieg; Musopen Symphony Orchestra"),
("album", "Peer Gynt Suite no. 1, op. 46"),
("date", "2012-08-15"),
("position", 1),
......@@ -283,7 +288,8 @@ def test_metadata_fallback_ogg_theora(mocker):
{
"name": "Binärpilot",
"mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
}
},
{"name": "Another artist", "mbid": None},
],
"album": {
"title": "You Can't Stop Da Funk",
......@@ -293,7 +299,8 @@ def test_metadata_fallback_ogg_theora(mocker):
{
"name": "Binärpilot",
"mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"),
}
},
{"name": "Another artist", "mbid": None},
],
},
"position": 2,
......@@ -313,7 +320,8 @@ def test_metadata_fallback_ogg_theora(mocker):
{
"name": "Edvard Grieg",
"mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
}
},
{"name": "Musopen Symphony Orchestra", "mbid": None},
],
"album": {
"title": "Peer Gynt Suite no. 1, op. 46",
......@@ -347,7 +355,8 @@ def test_metadata_fallback_ogg_theora(mocker):
{
"name": "Edvard Grieg",
"mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"),
}
},
{"name": "Musopen Symphony Orchestra", "mbid": None},
],
"album": {
"title": "Peer Gynt Suite no. 1, op. 46",
......
......@@ -143,6 +143,20 @@ def test_can_create_track_from_file_metadata_description(factories):
assert track.description.content_type == "text/plain"
def test_can_create_track_from_file_metadata_use_featuring(factories):
metadata = {
"title": "Whole Lotta Love",
"position": 1,
"disc_number": 1,
"description": {"text": "hello there", "content_type": "text/plain"},
"album": {"title": "Test album"},
"artists": [{"name": "Santana"}, {"name": "Anatnas"}],
}
track = tasks.get_track_from_import_metadata(metadata)
assert track.artist.name == "Anatnas"
def test_can_create_track_from_file_metadata_mbid(factories, mocker):
metadata = {
"title": "Test track",
......
Fixed invalid metadata when importing multi-artists tracks/albums (#1104)
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment