Resolve "regression:multiple albums with same name and artsit creating during import"

This commit is contained in:
petitminion 2024-12-28 19:09:43 +00:00
parent 788e84d70c
commit 4bfa1feacf
3 changed files with 105 additions and 42 deletions

View File

@ -380,9 +380,9 @@ def federation_audio_track_to_metadata(payload, references):
"copyright": payload.get("copyright"),
"description": payload.get("description"),
"attributed_to": references.get(payload.get("attributedTo")),
"mbid": str(payload.get("musicbrainzId"))
if payload.get("musicbrainzId")
else None,
"mbid": (
str(payload.get("musicbrainzId")) if payload.get("musicbrainzId") else None
),
"cover_data": get_cover(payload, "image"),
"album": {
"title": payload["album"]["name"],
@ -390,9 +390,11 @@ def federation_audio_track_to_metadata(payload, references):
"fid": payload["album"]["id"],
"description": payload["album"].get("description"),
"attributed_to": references.get(payload["album"].get("attributedTo")),
"mbid": str(payload["album"]["musicbrainzId"])
if payload["album"].get("musicbrainzId")
else None,
"mbid": (
str(payload["album"]["musicbrainzId"])
if payload["album"].get("musicbrainzId")
else None
),
"cover_data": get_cover(payload["album"], "image"),
"release_date": payload["album"].get("released"),
"tags": [t["name"] for t in payload["album"].get("tags", []) or []],
@ -407,9 +409,11 @@ def federation_audio_track_to_metadata(payload, references):
"attributed_to": references.get(
a["artist"].get("attributedTo")
),
"mbid": str(a["artist"]["musicbrainzId"])
if a["artist"].get("musicbrainzId")
else None,
"mbid": (
str(a["artist"]["musicbrainzId"])
if a["artist"].get("musicbrainzId")
else None
),
"tags": [t["name"] for t in a["artist"].get("tags", []) or []],
},
"joinphrase": (a["joinphrase"] if "joinphrase" in a else ""),
@ -426,9 +430,11 @@ def federation_audio_track_to_metadata(payload, references):
"fdate": a["artist"]["published"],
"description": a["artist"].get("description"),
"attributed_to": references.get(a["artist"].get("attributedTo")),
"mbid": str(a["artist"]["musicbrainzId"])
if a["artist"].get("musicbrainzId")
else None,
"mbid": (
str(a["artist"]["musicbrainzId"])
if a["artist"].get("musicbrainzId")
else None
),
"tags": [t["name"] for t in a["artist"].get("tags", []) or []],
"cover_data": get_cover(a["artist"], "image"),
},
@ -528,6 +534,7 @@ def _get_track(data, attributed_to=None, query_mb=True, **forced_values):
sync_mb_tag = preferences.get("music__sync_musicbrainz_tags")
track_uuid = getter(data, "funkwhale", "track", "uuid")
logger.debug(f"Getting track from import metadata: {data}")
if track_uuid:
# easy case, we have a reference to a uuid of a track that
# already exists in our database
@ -567,7 +574,7 @@ def _get_track(data, attributed_to=None, query_mb=True, **forced_values):
except IndexError:
pass
# get / create artist, artist_credit and album artist, album artist_credit
# get / create artist, artist_credit
album_artists_credits = None
artist_credit_data = getter(data, "artist_credit", default=[])
if "artist" in forced_values:
@ -600,6 +607,7 @@ def _get_track(data, attributed_to=None, query_mb=True, **forced_values):
)
)
# get / create album artist, album artist_credit
if "album" in forced_values:
album = forced_values["album"]
album_artists_credits = track_artists_credits
@ -752,16 +760,16 @@ def _get_track(data, attributed_to=None, query_mb=True, **forced_values):
return track
def get_or_create_artist(artist_data, attributed_to, from_activity_id):
def get_or_create_artist_from_ac(ac_data, attributed_to, from_activity_id):
sync_mb_tag = preferences.get("music__sync_musicbrainz_tags")
mbid = artist_data.get("artist", {}).get("mbid", None)
fid = artist_data.get("artist", {}).get("fid", None)
name = artist_data.get("artist", {}).get("name", artist_data["credit"])
creation_date = artist_data.get("artist", {}).get("fdate", timezone.now())
description = artist_data.get("artist", {}).get("description", None)
attributed_to = artist_data.get("artist", {}).get("attributed_to", attributed_to)
tags = artist_data.get("artist", {}).get("tags", [])
cover = artist_data.get("artist", {}).get("cover_data", None)
mbid = ac_data.get("artist", {}).get("mbid", None)
fid = ac_data.get("artist", {}).get("fid", None)
name = ac_data.get("artist", {}).get("name", ac_data.get("credit", None))
creation_date = ac_data.get("artist", {}).get("fdate", timezone.now())
description = ac_data.get("artist", {}).get("description", None)
attributed_to = ac_data.get("artist", {}).get("attributed_to", attributed_to)
tags = ac_data.get("artist", {}).get("tags", [])
cover = ac_data.get("artist", {}).get("cover_data", None)
if mbid:
query = Q(mbid=mbid)
@ -779,8 +787,8 @@ def get_or_create_artist(artist_data, attributed_to, from_activity_id):
"attributed_to": attributed_to,
"creation_date": creation_date,
}
if artist_data.get("fdate"):
defaults["creation_date"] = artist_data.get("fdate")
if ac_data.get("fdate"):
defaults["creation_date"] = ac_data.get("fdate")
artist, created = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"]
@ -819,28 +827,19 @@ def get_or_create_artists_credits_from_musicbrainz(
artists_credits = []
acs = mb_obj.get("recording", mb_obj)["artist-credit"]
logger.debug(f"MusicBrainz responded with : {mb_obj}")
for i, ac in enumerate(acs):
if isinstance(ac, str):
continue
artist_mbid = ac["artist"]["id"]
artist_name = ac["artist"]["name"]
credit = ac.get("name", artist_name)
joinphrase = ac["joinphrase"]
# artist creation
query = Q(mbid=artist_mbid)
# mb use "name" instead of "credit" and id instead of mbdi
credit = ac.get("name", ac.get("credit", artist_name))
ac["credit"] = credit
ac["artist"]["mbid"] = ac["artist"]["id"]
defaults = {
"name": artist_name,
"mbid": artist_mbid,
"from_activity_id": from_activity_id,
"attributed_to": attributed_to,
}
artist, created = get_best_candidate_or_create(
models.Artist, query, defaults=defaults, sort_fields=["mbid"]
)
# we could import artist tag, description, cover here.
artist = get_or_create_artist_from_ac(ac, attributed_to, from_activity_id)
# artist_credit creation
defaults = {
@ -924,14 +923,15 @@ def get_or_create_artists_credits_from_artist_credit_metadata(
):
artists_credits = []
for i, ac in enumerate(artists_credits_data):
ac["artist"] = get_or_create_artist(ac, attributed_to, from_activity_id)
ac["artist"] = get_or_create_artist_from_ac(ac, attributed_to, from_activity_id)
ac["index"] = ac.get("index", i)
credit = ac.get("credit", ac["artist"].name)
query = (
Q(artist=ac["artist"])
& Q(credit=credit)
& Q(joinphrase=ac["joinphrase"])
& Q(index=i)
& Q(index=ac.get("index", i))
)
artist_credit, created = get_best_candidate_or_create(

View File

@ -283,7 +283,7 @@ def test_can_create_track_from_file_metadata_mbid_existing_album_artist(
"title": "",
"artist_credit": [
{
"credit": "",
"credit": "lol",
"joinphrase": "",
"mbid": album.artist_credit.all()[0].mbid,
}
@ -1279,6 +1279,68 @@ def test_get_track_from_import_metadata_with_forced_values_album(
assert upload.track.artist_credit.all()[0].artist == channel.artist
def test_get_track_same_album(factories, mocker, faker):
metadata = {
"title": "Whole Lotta Love",
"position": 1,
"disc_number": 1,
"album": {
"title": "Guitar Heaven: The Greatest Guitar Classics of All Time",
"release_date": datetime.date(2010, 9, 17),
"artist_credit": [
{
"credit": "Santana",
"joinphrase": "",
"artist": {
"name": "artist name",
},
}
],
},
"artist_credit": [
{
"credit": "Santana",
"joinphrase": "",
"artist": {
"name": "artist name",
},
}
],
}
metadata2 = {
"title": "Whole Lotta Love 2",
"position": 2,
"disc_number": 1,
"album": {
"title": "Guitar Heaven: The Greatest Guitar Classics of All Time",
"release_date": datetime.date(2010, 9, 17),
"artist_credit": [
{
"credit": "Santana",
"joinphrase": "",
"artist": {
"name": "artist name",
},
}
],
},
"artist_credit": [
{
"credit": "Santana",
"joinphrase": "",
"artist": {
"name": "artist name",
},
}
],
}
track = tasks._get_track(metadata)
track.refresh_from_db()
track2 = tasks._get_track(metadata2)
track2.refresh_from_db()
assert track.album == track2.album
def test_process_channel_upload_forces_artist_and_attributed_to(
factories, mocker, faker
):

View File

@ -0,0 +1 @@
regression:multiple albums with same name and artsit creating during import (#2365)