diff --git a/api/funkwhale_api/activity/utils.py b/api/funkwhale_api/activity/utils.py index 28ddc2677..fc2ca5736 100644 --- a/api/funkwhale_api/activity/utils.py +++ b/api/funkwhale_api/activity/utils.py @@ -40,11 +40,23 @@ def combined_recent(limit, **kwargs): def get_activity(user, limit=20): query = fields.privacy_level_query(user, lookup_field="user__privacy_level") querysets = [ - Listening.objects.filter(query).select_related( - "track", "user", "track__artist", "track__album__artist" + Listening.objects.filter(query) + .select_related( + "track", + "user", + ) + .prefetch_related( + "track__artist_credit__artist", + "track__album__artist_credit__artist", ), - TrackFavorite.objects.filter(query).select_related( - "track", "user", "track__artist", "track__album__artist" + TrackFavorite.objects.filter(query) + .select_related( + "track", + "user", + ) + .prefetch_related( + "track__artist_credit__artist", + "track__album__artist_credit__artist", ), ] records = combined_recent(limit=limit, querysets=querysets) diff --git a/api/funkwhale_api/audio/filters.py b/api/funkwhale_api/audio/filters.py index eb9960031..2153a5955 100644 --- a/api/funkwhale_api/audio/filters.py +++ b/api/funkwhale_api/audio/filters.py @@ -21,7 +21,11 @@ TAG_FILTER = common_filters.MultipleQueryFilter(method=filter_tags) class ChannelFilter(moderation_filters.HiddenContentFilterSet): q = fields.SearchFilter( - search_fields=["artist__name", "actor__summary", "actor__preferred_username"] + search_fields=[ + "artist_credit__artist__name", + "actor__summary", + "actor__preferred_username", + ] ) tag = TAG_FILTER scope = common_filters.ActorScopeFilter(actor_field="attributed_to", distinct=True) @@ -33,7 +37,7 @@ class ChannelFilter(moderation_filters.HiddenContentFilterSet): # tuple-mapping retains order fields=( ("creation_date", "creation_date"), - ("artist__modification_date", "modification_date"), + ("artist_credit__artist__modification_date", "modification_date"), ("?", "random"), ) ) diff --git a/api/funkwhale_api/audio/serializers.py b/api/funkwhale_api/audio/serializers.py index 3c830fd80..7d6b1566d 100644 --- a/api/funkwhale_api/audio/serializers.py +++ b/api/funkwhale_api/audio/serializers.py @@ -26,6 +26,7 @@ from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.federation import utils as federation_utils from funkwhale_api.moderation import mrf from funkwhale_api.music import models as music_models +from funkwhale_api.music import tasks from funkwhale_api.music.serializers import COVER_WRITE_FIELD, CoverField from funkwhale_api.tags import models as tags_models from funkwhale_api.tags import serializers as tags_serializers @@ -246,11 +247,14 @@ class SimpleChannelArtistSerializer(serializers.Serializer): description = common_serializers.ContentSerializer(allow_null=True, required=False) cover = CoverField(allow_null=True, required=False) channel = serializers.UUIDField(allow_null=True, required=False) - tracks_count = serializers.IntegerField(source="_tracks_count", required=False) + tracks_count = serializers.SerializerMethodField(required=False) tags = serializers.ListField( child=serializers.CharField(), source="_prefetched_tagged_items", required=False ) + def get_tracks_count(self, o) -> int: + return getattr(o, "_tracks_count", 0) + class ChannelSerializer(serializers.ModelSerializer): artist = SimpleChannelArtistSerializer() @@ -749,7 +753,7 @@ class RssFeedItemSerializer(serializers.Serializer): else: existing_track = ( music_models.Track.objects.filter( - uuid=expected_uuid, artist__channel=channel + uuid=expected_uuid, artist_credit__artist__channel=channel ) .select_related("description", "attachment_cover") .first() @@ -765,7 +769,6 @@ class RssFeedItemSerializer(serializers.Serializer): "disc_number": validated_data.get("itunes_season", 1) or 1, "position": validated_data.get("itunes_episode", 1) or 1, "title": validated_data["title"], - "artist": channel.artist, } ) if "rights" in validated_data: @@ -801,6 +804,21 @@ class RssFeedItemSerializer(serializers.Serializer): **track_kwargs, defaults=track_defaults, ) + + # channel only have one artist so we can safely update artist_credit + defaults = { + "artist": channel.artist, + "credit": channel.artist.name, + "joinphrase": "", + } + query = ( + Q(artist=channel.artist) & Q(credit=channel.artist.name) & Q(joinphrase="") + ) + artist_credit = tasks.get_best_candidate_or_create( + music_models.ArtistCredit, query, defaults, ["artist", "joinphrase"] + ) + track.artist_credit.set([artist_credit[0]]) + # optimisation for reducing SQL queries, because we cannot use select_related with # update or create, so we restore the cache by hand if existing_track: diff --git a/api/funkwhale_api/audio/views.py b/api/funkwhale_api/audio/views.py index 83a3b6f1f..0fc4d7de4 100644 --- a/api/funkwhale_api/audio/views.py +++ b/api/funkwhale_api/audio/views.py @@ -27,7 +27,7 @@ ARTIST_PREFETCH_QS = ( "attachment_cover", ) .prefetch_related(music_views.TAG_PREFETCH) - .annotate(_tracks_count=Count("tracks")) + .annotate(_tracks_count=Count("artist_credit__tracks")) ) @@ -103,7 +103,7 @@ class ChannelViewSet( queryset = super().get_queryset() if self.action == "retrieve": queryset = queryset.annotate( - _downloads_count=Sum("artist__tracks__downloads_count") + _downloads_count=Sum("artist__artist_credit__tracks__downloads_count") ) return queryset @@ -192,7 +192,6 @@ class ChannelViewSet( if object.attributed_to == actors.get_service_actor(): # external feed, we redirect to the canonical one return http.HttpResponseRedirect(object.rss_url) - uploads = ( object.library.uploads.playable_by(None) .prefetch_related( diff --git a/api/funkwhale_api/common/management/commands/load_test_data.py b/api/funkwhale_api/common/management/commands/load_test_data.py index f9997a331..88ee57b28 100644 --- a/api/funkwhale_api/common/management/commands/load_test_data.py +++ b/api/funkwhale_api/common/management/commands/load_test_data.py @@ -68,22 +68,33 @@ def create_taggable_items(dependency): CONFIG = [ + { + "id": "artist_credit", + "model": music_models.ArtistCredit, + "factory": "music.ArtistCredit", + "factory_kwargs": {"joinphrase": ""}, + "depends_on": [ + {"field": "artist", "id": "artists", "default_factor": 0.5}, + ], + }, { "id": "tracks", "model": music_models.Track, "factory": "music.Track", - "factory_kwargs": {"artist": None, "album": None}, + "factory_kwargs": {"album": None}, "depends_on": [ {"field": "album", "id": "albums", "default_factor": 0.1}, - {"field": "artist", "id": "artists", "default_factor": 0.05}, + {"field": "artist_credit", "id": "artist_credit", "default_factor": 0.05}, ], }, { "id": "albums", "model": music_models.Album, "factory": "music.Album", - "factory_kwargs": {"artist": None}, - "depends_on": [{"field": "artist", "id": "artists", "default_factor": 0.3}], + "factory_kwargs": {}, + "depends_on": [ + {"field": "artist_credit", "id": "artist_credit", "default_factor": 0.3} + ], }, {"id": "artists", "model": music_models.Artist, "factory": "music.Artist"}, { @@ -310,12 +321,23 @@ class Command(BaseCommand): candidates = list(queryset.values_list("pk", flat=True)) picked_pks = [random.choice(candidates) for _ in objects] picked_objects = {o.pk: o for o in queryset.filter(pk__in=picked_pks)} + + saved_obj = [] for i, obj in enumerate(objects): if create_dependencies: value = random.choice(candidates) else: value = picked_objects[picked_pks[i]] - setattr(obj, dependency["field"], value) + if dependency["field"] == "artist_credit": + obj.save() + obj.artist_credit.set([value]) + saved_obj.append(obj) + + else: + setattr(obj, dependency["field"], value) + if saved_obj: + return saved_obj + if not handler: objects = row["model"].objects.bulk_create(objects, batch_size=BATCH_SIZE) results[row["id"]] = objects diff --git a/api/funkwhale_api/contrib/listenbrainz/funkwhale_ready.py b/api/funkwhale_api/contrib/listenbrainz/funkwhale_ready.py index 175c87904..7364bbac0 100644 --- a/api/funkwhale_api/contrib/listenbrainz/funkwhale_ready.py +++ b/api/funkwhale_api/contrib/listenbrainz/funkwhale_ready.py @@ -43,9 +43,9 @@ def get_lb_listen(listening): release_name = track.album.title if track.album.mbid: additional_info["release_mbid"] = str(track.album.mbid) - - if track.artist.mbid: - additional_info["artist_mbids"] = [str(track.artist.mbid)] + mbids = [ac.artist.mbid for ac in track.artist_credit.all() if ac.artist.mbid] + if mbids: + additional_info["artist_mbids"] = mbids upload = track.uploads.filter(duration__gte=0).first() if upload: @@ -53,8 +53,8 @@ def get_lb_listen(listening): return liblistenbrainz.Listen( track_name=track.title, - artist_name=track.artist.name, listened_at=listening.creation_date.timestamp(), + artist_name=track.get_artist_credit_string, release_name=release_name, additional_info=additional_info, ) diff --git a/api/funkwhale_api/contrib/maloja/funkwhale_ready.py b/api/funkwhale_api/contrib/maloja/funkwhale_ready.py index 45db7f70a..c0859681c 100644 --- a/api/funkwhale_api/contrib/maloja/funkwhale_ready.py +++ b/api/funkwhale_api/contrib/maloja/funkwhale_ready.py @@ -37,7 +37,7 @@ def get_payload(listening, api_key, conf): # See https://github.com/krateng/maloja/blob/master/API.md payload = { "key": api_key, - "artists": [track.artist.name], + "artists": [artist.name for artist in track.artist_credit.get_artists_list()], "title": track.title, "time": int(listening.creation_date.timestamp()), "nofix": bool(conf.get("nofix")), @@ -46,8 +46,10 @@ def get_payload(listening, api_key, conf): if track.album: if track.album.title: payload["album"] = track.album.title - if track.album.artist: - payload["albumartists"] = [track.album.artist.name] + if track.album.artist_credit.all(): + payload["albumartists"] = [ + artist.name for artist in track.album.artist_credit.get_artists_list() + ] upload = track.uploads.filter(duration__gte=0).first() if upload: diff --git a/api/funkwhale_api/contrib/scrobbler/scrobbler.py b/api/funkwhale_api/contrib/scrobbler/scrobbler.py index 3eb220a2b..847a94c64 100644 --- a/api/funkwhale_api/contrib/scrobbler/scrobbler.py +++ b/api/funkwhale_api/contrib/scrobbler/scrobbler.py @@ -84,7 +84,7 @@ def get_scrobble_payload(track, date, suffix="[0]"): """ upload = track.uploads.filter(duration__gte=0).first() data = { - f"a{suffix}": track.artist.name, + f"a{suffix}": track.get_artist_credit_string, f"t{suffix}": track.title, f"l{suffix}": upload.duration if upload else 0, f"b{suffix}": (track.album.title if track.album else "") or "", @@ -103,7 +103,7 @@ def get_scrobble2_payload(track, date, suffix="[0]"): """ upload = track.uploads.filter(duration__gte=0).first() data = { - "artist": track.artist.name, + "artist": track.get_artist_credit_string, "track": track.title, "chosenByUser": 1, } diff --git a/api/funkwhale_api/favorites/views.py b/api/funkwhale_api/favorites/views.py index 1fecf3e5a..040053e52 100644 --- a/api/funkwhale_api/favorites/views.py +++ b/api/funkwhale_api/favorites/views.py @@ -60,11 +60,20 @@ class TrackFavoriteViewSet( queryset = queryset.filter( fields.privacy_level_query(self.request.user, "user__privacy_level") ) - tracks = Track.objects.with_playable_uploads( - music_utils.get_actor_from_request(self.request) - ).select_related( - "artist", "album__artist", "attributed_to", "album__attachment_cover" + tracks = ( + Track.objects.with_playable_uploads( + music_utils.get_actor_from_request(self.request) + ) + .prefetch_related( + "artist_credit__artist", + "album__artist_credit__artist", + ) + .select_related( + "attributed_to", + "album__attachment_cover", + ) ) + queryset = queryset.prefetch_related(Prefetch("track", queryset=tracks)) return queryset diff --git a/api/funkwhale_api/federation/contexts.py b/api/funkwhale_api/federation/contexts.py index 351b4f7e2..8c9f4b67b 100644 --- a/api/funkwhale_api/federation/contexts.py +++ b/api/funkwhale_api/federation/contexts.py @@ -293,6 +293,7 @@ CONTEXTS = [ "Album": "fw:Album", "Track": "fw:Track", "Artist": "fw:Artist", + "ArtistCredit": "fw:ArtistCredit", "Library": "fw:Library", "bitrate": {"@id": "fw:bitrate", "@type": "xsd:nonNegativeInteger"}, "size": {"@id": "fw:size", "@type": "xsd:nonNegativeInteger"}, @@ -302,7 +303,16 @@ CONTEXTS = [ "track": {"@id": "fw:track", "@type": "@id"}, "cover": {"@id": "fw:cover", "@type": "as:Link"}, "album": {"@id": "fw:album", "@type": "@id"}, + "artist": {"@id": "fw:artist", "@type": "@id"}, "artists": {"@id": "fw:artists", "@type": "@id", "@container": "@list"}, + "artist_credit": { + "@id": "fw:artist_credit", + "@type": "@id", + "@container": "@list", + }, + "joinphrase": {"@id": "fw:joinphrase", "@type": "xsd:string"}, + "credit": {"@id": "fw:credit", "@type": "xsd:string"}, + "index": {"@id": "fw:index", "@type": "xsd:nonNegativeInteger"}, "released": {"@id": "fw:released", "@type": "xsd:date"}, "musicbrainzId": "fw:musicbrainzId", "license": {"@id": "fw:license", "@type": "@id"}, diff --git a/api/funkwhale_api/federation/jsonld.py b/api/funkwhale_api/federation/jsonld.py index 101b7fb92..cc6843620 100644 --- a/api/funkwhale_api/federation/jsonld.py +++ b/api/funkwhale_api/federation/jsonld.py @@ -191,7 +191,6 @@ def prepare_for_serializer(payload, config, fallbacks={}): value = noop if not aliases: continue - for a in aliases: try: value = get_value( @@ -279,7 +278,6 @@ class JsonLdSerializer(serializers.Serializer): for field in dereferenced_fields: for i in get_ids(data[field]): dereferenced_ids.add(i) - if dereferenced_ids: try: loop = asyncio.get_event_loop() diff --git a/api/funkwhale_api/federation/routes.py b/api/funkwhale_api/federation/routes.py index cd0b94b5c..f20a02e39 100644 --- a/api/funkwhale_api/federation/routes.py +++ b/api/funkwhale_api/federation/routes.py @@ -293,7 +293,7 @@ def inbox_delete_audio(payload, context): upload_fids = [payload["object"]["id"]] query = Q(fid__in=upload_fids) & ( - Q(library__actor=actor) | Q(track__artist__channel__actor=actor) + Q(library__actor=actor) | Q(track__artist_credit__artist__channel__actor=actor) ) candidates = music_models.Upload.objects.filter(query) @@ -577,7 +577,9 @@ def inbox_delete_album(payload, context): logger.debug("Discarding deletion of empty library") return - query = Q(fid=album_id) & (Q(attributed_to=actor) | Q(artist__channel__actor=actor)) + query = Q(fid=album_id) & ( + Q(attributed_to=actor) | Q(artist_credit__artist__channel__actor=actor) + ) try: album = music_models.Album.objects.get(query) except music_models.Album.DoesNotExist: @@ -590,9 +592,10 @@ def inbox_delete_album(payload, context): @outbox.register({"type": "Delete", "object.type": "Album"}) def outbox_delete_album(context): album = context["album"] + album_artist = album.artist_credit.all()[0].artist actor = ( - album.artist.channel.actor - if album.artist.get_channel() + album_artist.channel.actor + if album_artist.get_channel() else album.attributed_to ) actor = actor or actors.get_service_actor() diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index c96cd8f38..fe1818365 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -6,6 +6,7 @@ import uuid from django.core.exceptions import ObjectDoesNotExist from django.core.paginator import Paginator from django.db import transaction +from django.db.models import Q from django.urls import reverse from django.utils import timezone from rest_framework import serializers @@ -1221,12 +1222,22 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer): self.updateable_fields, validated_data, instance ) updated_fields = self.validate_updated_data(instance, updated_fields) + + set_ac = False + if "artist_credit" in updated_fields: + artist_credit = updated_fields.pop("artist_credit") + set_ac = True + if creating: instance, created = self.Meta.model.objects.get_or_create( fid=validated_data["id"], defaults=updated_fields ) + if set_ac: + instance.artist_credit.set(artist_credit) else: - music_tasks.update_library_entity(instance, updated_fields) + obj = music_tasks.update_library_entity(instance, updated_fields) + if set_ac: + obj.artist_credit.set(artist_credit) tags = [t["name"] for t in validated_data.get("tags", []) or []] tags_models.set_tags(instance, *tags) @@ -1288,7 +1299,6 @@ class ArtistSerializer(MusicEntitySerializer): MUSIC_ENTITY_JSONLD_MAPPING, { "released": jsonld.first_val(contexts.FW.released), - "artists": jsonld.first_attr(contexts.FW.artists, "@list"), "image": jsonld.first_obj(contexts.AS.image), }, ) @@ -1314,12 +1324,53 @@ class ArtistSerializer(MusicEntitySerializer): create = MusicEntitySerializer.update_or_create +class ArtistCreditSerializer(jsonld.JsonLdSerializer): + artist = ArtistSerializer() + joinphrase = serializers.CharField( + trim_whitespace=False, required=False, allow_null=True, allow_blank=True + ) + credit = serializers.CharField( + trim_whitespace=False, required=False, allow_null=True, allow_blank=True + ) + published = serializers.DateTimeField() + id = serializers.URLField(max_length=500) + + updateable_fields = [ + ("credit", "credit"), + ("artist", "artist"), + ("joinphrase", "joinphrase"), + ] + + class Meta: + model = music_models.ArtistCredit + jsonld_mapping = { + "artist": jsonld.first_obj(contexts.FW.artist), + "credit": jsonld.first_val(contexts.FW.credit), + "index": jsonld.first_val(contexts.FW.index), + "joinphrase": jsonld.first_val(contexts.FW.joinphrase), + "published": jsonld.first_val(contexts.AS.published), + } + + def to_representation(self, instance): + data = { + "type": "ArtistCredit", + "id": instance.fid, + "artist": ArtistSerializer( + instance.artist, context={"include_ap_context": False} + ).data, + "joinphrase": instance.joinphrase, + "credit": instance.credit, + "index": instance.index, + "published": instance.creation_date.isoformat(), + } + if self.context.get("include_ap_context", self.parent is None): + data["@context"] = jsonld.get_default_context() + return data + + class AlbumSerializer(MusicEntitySerializer): released = serializers.DateField(allow_null=True, required=False) - artists = serializers.ListField( - child=MultipleSerializer(allowed=[BasicActorSerializer, ArtistSerializer]), - min_length=1, - ) + artist_credit = serializers.ListField(child=ArtistCreditSerializer(), min_length=1) image = ImageSerializer( allowed_mimetypes=["image/*"], allow_null=True, @@ -1332,7 +1383,7 @@ class AlbumSerializer(MusicEntitySerializer): ("musicbrainzId", "mbid"), ("attributedTo", "attributed_to"), ("released", "release_date"), - ("_artist", "artist"), + ("artist_credit", "artist_credit"), ] class Meta: @@ -1341,13 +1392,13 @@ class AlbumSerializer(MusicEntitySerializer): MUSIC_ENTITY_JSONLD_MAPPING, { "released": jsonld.first_val(contexts.FW.released), - "artists": jsonld.first_attr(contexts.FW.artists, "@list"), + "artist_credit": jsonld.first_attr(contexts.FW.artist_credit, "@list"), "image": jsonld.first_obj(contexts.AS.image), }, ) def to_representation(self, instance): - d = { + data = { "type": "Album", "id": instance.fid, "name": instance.title, @@ -1361,42 +1412,40 @@ class AlbumSerializer(MusicEntitySerializer): else None, "tag": self.get_tags_repr(instance), } - if instance.artist.get_channel(): - d["artists"] = [ - { - "type": instance.artist.channel.actor.type, - "id": instance.artist.channel.actor.fid, - } - ] - else: - d["artists"] = [ - ArtistSerializer( - instance.artist, context={"include_ap_context": False} - ).data - ] - include_content(d, instance.description) + + data["artist_credit"] = ArtistCreditSerializer( + instance.artist_credit.all(), + context={"include_ap_context": False}, + many=True, + ).data + include_content(data, instance.description) if instance.attachment_cover: - include_image(d, instance.attachment_cover) + include_image(data, instance.attachment_cover) if self.context.get("include_ap_context", self.parent is None): - d["@context"] = jsonld.get_default_context() - return d + data["@context"] = jsonld.get_default_context() + return data def validate(self, data): validated_data = super().validate(data) if not self.parent: - artist_data = validated_data["artists"][0] - if artist_data.get("type", "Artist") == "Artist": - validated_data["_artist"] = utils.retrieve_ap_object( - artist_data["id"], - actor=self.context.get("fetch_actor"), - queryset=music_models.Artist, - serializer_class=ArtistSerializer, - ) + artist_credit_data = validated_data["artist_credit"] + if artist_credit_data[0]["artist"].get("type", "Artist") == "Artist": + acs = [] + for ac in validated_data["artist_credit"]: + acs.append( + utils.retrieve_ap_object( + ac["id"], + actor=self.context.get("fetch_actor"), + queryset=music_models.ArtistCredit, + serializer_class=ArtistCreditSerializer, + ) + ) + validated_data["artist_credit"] = acs else: # we have an actor as an artist, so it's a channel - actor = actors.get_actor(artist_data["id"]) - validated_data["_artist"] = actor.channel.artist + actor = actors.get_actor(artist_credit_data[0]["artist"]["id"]) + validated_data["artist_credit"] = [{"artist": actor.channel.artist}] return validated_data @@ -1406,7 +1455,7 @@ class AlbumSerializer(MusicEntitySerializer): class TrackSerializer(MusicEntitySerializer): position = serializers.IntegerField(min_value=0, allow_null=True, required=False) disc = serializers.IntegerField(min_value=1, allow_null=True, required=False) - artists = serializers.ListField(child=ArtistSerializer(), min_length=1) + artist_credit = serializers.ListField(child=ArtistCreditSerializer(), min_length=1) album = AlbumSerializer() license = serializers.URLField(allow_null=True, required=False) copyright = serializers.CharField(allow_null=True, required=False) @@ -1434,7 +1483,7 @@ class TrackSerializer(MusicEntitySerializer): MUSIC_ENTITY_JSONLD_MAPPING, { "album": jsonld.first_obj(contexts.FW.album), - "artists": jsonld.first_attr(contexts.FW.artists, "@list"), + "artist_credit": jsonld.first_attr(contexts.FW.artist_credit, "@list"), "copyright": jsonld.first_val(contexts.FW.copyright), "disc": jsonld.first_val(contexts.FW.disc), "license": jsonld.first_id(contexts.FW.license), @@ -1444,7 +1493,7 @@ class TrackSerializer(MusicEntitySerializer): ) def to_representation(self, instance): - d = { + data = { "type": "Track", "id": instance.fid, "name": instance.title, @@ -1456,11 +1505,11 @@ class TrackSerializer(MusicEntitySerializer): if instance.local_license else None, "copyright": instance.copyright if instance.copyright else None, - "artists": [ - ArtistSerializer( - instance.artist, context={"include_ap_context": False} - ).data - ], + "artist_credit": ArtistCreditSerializer( + instance.artist_credit.all(), + context={"include_ap_context": False}, + many=True, + ).data, "album": AlbumSerializer( instance.album, context={"include_ap_context": False} ).data, @@ -1469,12 +1518,13 @@ class TrackSerializer(MusicEntitySerializer): else None, "tag": self.get_tags_repr(instance), } - include_content(d, instance.description) - include_image(d, instance.attachment_cover) + include_content(data, instance.description) + include_image(data, instance.attachment_cover) if self.context.get("include_ap_context", self.parent is None): - d["@context"] = jsonld.get_default_context() - return d + data["@context"] = jsonld.get_default_context() + return data + @transaction.atomic def create(self, validated_data): from funkwhale_api.music import tasks as music_tasks @@ -1490,18 +1540,21 @@ class TrackSerializer(MusicEntitySerializer): validated_data, "album.attributedTo", permissive=True ) ) - artists = ( - common_utils.recursive_getattr(validated_data, "artists", permissive=True) - or [] - ) - album_artists = ( + artist_credit = ( common_utils.recursive_getattr( - validated_data, "album.artists", permissive=True + validated_data, "artist_credit", permissive=True ) or [] ) - for artist in artists + album_artists: - actors_to_fetch.add(artist.get("attributedTo")) + album_artists_credit = ( + common_utils.recursive_getattr( + validated_data, "album.artist_credit", permissive=True + ) + or [] + ) + + for ac in artist_credit + album_artists_credit: + actors_to_fetch.add(ac["artist"].get("attributedTo")) for url in actors_to_fetch: if not url: @@ -1514,8 +1567,9 @@ class TrackSerializer(MusicEntitySerializer): from_activity = self.context.get("activity") if from_activity: metadata["from_activity_id"] = from_activity.pk - track = music_tasks.get_track_from_import_metadata(metadata, update_cover=True) - + track = music_tasks.get_track_from_import_metadata( + metadata, update_cover=True, query_mb=False + ) return track def update(self, obj, validated_data): @@ -1780,7 +1834,7 @@ class ChannelOutboxSerializer(PaginatedCollectionSerializer): "actor": channel.actor, "items": channel.library.uploads.for_federation() .order_by("-creation_date") - .filter(track__artist=channel.artist), + .filter(track__artist_credit__artist=channel.artist), "type": "OrderedCollection", } r = super().to_representation(conf) @@ -1850,7 +1904,7 @@ class ChannelUploadSerializer(jsonld.JsonLdSerializer): actor=actors.get_service_actor(), serializer_class=AlbumSerializer, queryset=music_models.Album.objects.filter( - artist__channel=self.context["channel"] + artist_credit__artist__channel=self.context["channel"] ), ) @@ -1929,7 +1983,6 @@ class ChannelUploadSerializer(jsonld.JsonLdSerializer): now = timezone.now() track_defaults = { "fid": validated_data["id"], - "artist": channel.artist, "position": validated_data.get("position", 1), "disc_number": validated_data.get("disc", 1), "title": validated_data["name"], @@ -1942,9 +1995,32 @@ class ChannelUploadSerializer(jsonld.JsonLdSerializer): track_defaults["license"] = licenses.match(validated_data["license"]) track, created = music_models.Track.objects.update_or_create( - artist__channel=channel, fid=validated_data["id"], defaults=track_defaults + fid=validated_data["id"], + defaults=track_defaults, ) + # only one artist_credit per channel + query = ( + Q( + artist=channel.artist, + ) + & Q(credit__iexact=channel.artist.name) + & Q(joinphrase="") + ) + defaults = { + "artist": channel.artist, + "joinphrase": "", + "credit": channel.artist.name, + } + + ac_obj = music_tasks.get_best_candidate_or_create( + music_models.ArtistCredit, + query, + defaults=defaults, + sort_fields=["mbid", "fid"], + ) + track.artist_credit.set([ac_obj[0].id]) + if "image" in validated_data: new_value = self.validated_data["image"] common_utils.attach_file( diff --git a/api/funkwhale_api/federation/urls.py b/api/funkwhale_api/federation/urls.py index 3380f3a0d..fac6bfdfd 100644 --- a/api/funkwhale_api/federation/urls.py +++ b/api/funkwhale_api/federation/urls.py @@ -17,6 +17,7 @@ router.register(r".well-known", views.WellKnownViewSet, "well-known") music_router.register(r"libraries", views.MusicLibraryViewSet, "libraries") music_router.register(r"uploads", views.MusicUploadViewSet, "uploads") music_router.register(r"artists", views.MusicArtistViewSet, "artists") +music_router.register(r"artistcredit", views.MusicArtistCreditViewSet, "artistcredit") music_router.register(r"albums", views.MusicAlbumViewSet, "albums") music_router.register(r"tracks", views.MusicTrackViewSet, "tracks") diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 6a79597fa..505ad2f3c 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -161,7 +161,9 @@ class ActorViewSet(FederationMixin, mixins.RetrieveModelMixin, viewsets.GenericV "actor": channel.actor, "items": channel.library.uploads.for_federation() .order_by("-creation_date") - .prefetch_related("library__channel__actor", "track__artist"), + .prefetch_related( + "library__channel__actor", "track__artist_credit__artist" + ), "item_serializer": serializers.ChannelCreateUploadSerializer, } return get_collection_response( @@ -290,21 +292,21 @@ class MusicLibraryViewSet( Prefetch( "track", queryset=music_models.Track.objects.select_related( - "album__artist__attributed_to", - "artist__attributed_to", - "artist__attachment_cover", "attachment_cover", "album__attributed_to", "attributed_to", "album__attachment_cover", - "album__artist__attachment_cover", "description", ).prefetch_related( + "album__artist_credit__artist__attributed_to", + "artist_credit__artist__attributed_to", + "artist_credit__artist__attachment_cover", "tagged_items__tag", "album__tagged_items__tag", - "album__artist__tagged_items__tag", - "artist__tagged_items__tag", - "artist__description", + "album__artist_credit__artist__tagged_items__tag", + "album__artist_credit__artist__attachment_cover", + "artist_credit__artist__tagged_items__tag", + "artist_credit__artist__description", "album__description", ), ) @@ -331,15 +333,20 @@ class MusicUploadViewSet( ): authentication_classes = [authentication.SignatureAuthentication] renderer_classes = renderers.get_ap_renderers() - queryset = music_models.Upload.objects.local().select_related( - "library__actor", - "track__artist", - "track__album__artist", - "track__description", - "track__album__attachment_cover", - "track__album__artist__attachment_cover", - "track__artist__attachment_cover", - "track__attachment_cover", + queryset = ( + music_models.Upload.objects.local() + .select_related( + "library__actor", + "track__description", + "track__album__attachment_cover", + "track__attachment_cover", + ) + .prefetch_related( + "track__artist_credit__artist", + "track__album__artist_credit__artist", + "track__album__artist_credit__artist__attachment_cover", + "track__artist_credit__artist__attachment_cover", + ) ) serializer_class = serializers.UploadSerializer lookup_field = "uuid" @@ -393,13 +400,35 @@ class MusicArtistViewSet( return response.Response(serializer.data) +class MusicArtistCreditViewSet( + FederationMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet +): + authentication_classes = [authentication.SignatureAuthentication] + renderer_classes = renderers.get_ap_renderers() + queryset = music_models.ArtistCredit.objects.local().prefetch_related("artist") + serializer_class = serializers.ArtistCreditSerializer + lookup_field = "uuid" + + def retrieve(self, request, *args, **kwargs): + instance = self.get_object() + serializer = self.get_serializer(instance) + return response.Response(serializer.data) + + class MusicAlbumViewSet( FederationMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet ): authentication_classes = [authentication.SignatureAuthentication] renderer_classes = renderers.get_ap_renderers() - queryset = music_models.Album.objects.local().select_related( - "artist__description", "description", "artist__attachment_cover" + queryset = ( + music_models.Album.objects.local() + .prefetch_related( + "artist_credit__artist__description", + "artist_credit__artist__attachment_cover", + ) + .select_related( + "description", + ) ) serializer_class = serializers.AlbumSerializer lookup_field = "uuid" @@ -418,16 +447,22 @@ class MusicTrackViewSet( ): authentication_classes = [authentication.SignatureAuthentication] renderer_classes = renderers.get_ap_renderers() - queryset = music_models.Track.objects.local().select_related( - "album__artist", - "album__description", - "artist__description", - "description", - "attachment_cover", - "album__artist__attachment_cover", - "album__attachment_cover", - "artist__attachment_cover", + queryset = ( + music_models.Track.objects.local() + .select_related( + "album__description", + "description", + "attachment_cover", + "album__attachment_cover", + ) + .prefetch_related( + "album__artist_credit__artist", + "artist_credit__artist__description", + "artist_credit__artist__attachment_cover", + "album__artist_credit__artist__attachment_cover", + ) ) + serializer_class = serializers.TrackSerializer lookup_field = "uuid" diff --git a/api/funkwhale_api/history/views.py b/api/funkwhale_api/history/views.py index 812ed38ae..d49702754 100644 --- a/api/funkwhale_api/history/views.py +++ b/api/funkwhale_api/history/views.py @@ -51,10 +51,16 @@ class ListeningViewSet( queryset = queryset.filter( fields.privacy_level_query(self.request.user, "user__privacy_level") ) - tracks = Track.objects.with_playable_uploads( - music_utils.get_actor_from_request(self.request) - ).select_related( - "artist", "album__artist", "attributed_to", "artist__attachment_cover" + tracks = ( + Track.objects.with_playable_uploads( + music_utils.get_actor_from_request(self.request) + ) + .prefetch_related( + "artist_credit", + "album__artist_credit__artist", + "artist_credit__artist__attachment_cover", + ) + .select_related("attributed_to") ) return queryset.prefetch_related(Prefetch("track", queryset=tracks)) diff --git a/api/funkwhale_api/instance/stats.py b/api/funkwhale_api/instance/stats.py index 9353eb53e..76a85b35a 100644 --- a/api/funkwhale_api/instance/stats.py +++ b/api/funkwhale_api/instance/stats.py @@ -37,7 +37,7 @@ def get_content(): def get_top_music_categories(): return ( - models.Track.objects.filter(artist__content_category="music") + models.Track.objects.filter(artist_credit__artist__content_category="music") .exclude(tagged_items__tag_id=None) .values(name=F("tagged_items__tag__name")) .annotate(count=Count("name")) @@ -47,7 +47,7 @@ def get_top_music_categories(): def get_top_podcast_categories(): return ( - models.Track.objects.filter(artist__content_category="podcast") + models.Track.objects.filter(artist_credit__artist__content_category="podcast") .exclude(tagged_items__tag_id=None) .values(name=F("tagged_items__tag__name")) .annotate(count=Count("name")) diff --git a/api/funkwhale_api/manage/filters.py b/api/funkwhale_api/manage/filters.py index b3884a058..610d69e5a 100644 --- a/api/funkwhale_api/manage/filters.py +++ b/api/funkwhale_api/manage/filters.py @@ -97,12 +97,15 @@ class ManageAlbumFilterSet(filters.FilterSet): search_fields={ "title": {"to": "title"}, "fid": {"to": "fid"}, - "artist": {"to": "artist__name"}, + "artist": {"to": "artist_credit__artist__name"}, "mbid": {"to": "mbid"}, }, filter_fields={ "uuid": {"to": "uuid"}, - "artist_id": {"to": "artist_id", "field": forms.IntegerField()}, + "artist_id": { + "to": "artist_credit__artist_id", + "field": forms.IntegerField(), + }, "domain": { "handler": lambda v: federation_utils.get_domain_query_from_url(v) }, @@ -118,7 +121,7 @@ class ManageAlbumFilterSet(filters.FilterSet): class Meta: model = music_models.Album - fields = ["title", "mbid", "fid", "artist"] + fields = ["title", "mbid", "fid", "artist_credit"] class ManageTrackFilterSet(filters.FilterSet): @@ -128,9 +131,9 @@ class ManageTrackFilterSet(filters.FilterSet): "title": {"to": "title"}, "fid": {"to": "fid"}, "mbid": {"to": "mbid"}, - "artist": {"to": "artist__name"}, + "artist": {"to": "artist_credit__artist__name"}, "album": {"to": "album__title"}, - "album_artist": {"to": "album__artist__name"}, + "album_artist": {"to": "album__artist_credit__artist__name"}, "copyright": {"to": "copyright"}, }, filter_fields={ @@ -157,7 +160,7 @@ class ManageTrackFilterSet(filters.FilterSet): class Meta: model = music_models.Track - fields = ["title", "mbid", "fid", "artist", "album", "license"] + fields = ["title", "mbid", "fid", "artist_credit", "album", "license"] class ManageLibraryFilterSet(filters.FilterSet): diff --git a/api/funkwhale_api/manage/serializers.py b/api/funkwhale_api/manage/serializers.py index e097fc88f..07a6fb41c 100644 --- a/api/funkwhale_api/manage/serializers.py +++ b/api/funkwhale_api/manage/serializers.py @@ -451,17 +451,25 @@ class ManageNestedArtistSerializer(ManageBaseArtistSerializer): pass +class ManageNestedArtistCreditSerializer(ManageBaseArtistSerializer): + artist = ManageNestedArtistSerializer() + + class Meta: + model = music_models.ArtistCredit + fields = ["artist"] + + class ManageAlbumSerializer( music_serializers.OptionalDescriptionMixin, ManageBaseAlbumSerializer ): attributed_to = ManageBaseActorSerializer() - artist = ManageNestedArtistSerializer() + artist_credit = ManageNestedArtistCreditSerializer(many=True) tags = serializers.SerializerMethodField() class Meta: model = music_models.Album fields = ManageBaseAlbumSerializer.Meta.fields + [ - "artist", + "artist_credit", "attributed_to", "tags", "tracks_count", @@ -477,17 +485,17 @@ class ManageAlbumSerializer( class ManageTrackAlbumSerializer(ManageBaseAlbumSerializer): - artist = ManageNestedArtistSerializer() + artist_credit = ManageNestedArtistCreditSerializer(many=True) class Meta: model = music_models.Album - fields = ManageBaseAlbumSerializer.Meta.fields + ["artist"] + fields = ManageBaseAlbumSerializer.Meta.fields + ["artist_credit"] class ManageTrackSerializer( music_serializers.OptionalDescriptionMixin, ManageNestedTrackSerializer ): - artist = ManageNestedArtistSerializer() + artist_credit = ManageNestedArtistCreditSerializer(many=True) album = ManageTrackAlbumSerializer(allow_null=True) attributed_to = ManageBaseActorSerializer(allow_null=True) uploads_count = serializers.SerializerMethodField() @@ -497,7 +505,7 @@ class ManageTrackSerializer( class Meta: model = music_models.Track fields = ManageNestedTrackSerializer.Meta.fields + [ - "artist", + "artist_credit", "album", "attributed_to", "uploads_count", diff --git a/api/funkwhale_api/manage/views.py b/api/funkwhale_api/manage/views.py index 0acad7246..7a5a75656 100644 --- a/api/funkwhale_api/manage/views.py +++ b/api/funkwhale_api/manage/views.py @@ -84,8 +84,8 @@ class ManageArtistViewSet( music_models.Artist.objects.all() .order_by("-id") .select_related("attributed_to", "attachment_cover", "channel") - .annotate(_tracks_count=Count("tracks", distinct=True)) - .annotate(_albums_count=Count("albums", distinct=True)) + .annotate(_tracks_count=Count("artist_credit__tracks", distinct=True)) + .annotate(_albums_count=Count("artist_credit__albums", distinct=True)) .prefetch_related(music_views.TAG_PREFETCH) ) serializer_class = serializers.ManageArtistSerializer @@ -98,7 +98,7 @@ class ManageArtistViewSet( def stats(self, request, *args, **kwargs): artist = self.get_object() tracks = music_models.Track.objects.filter( - Q(artist=artist) | Q(album__artist=artist) + Q(artist_credit__artist=artist) | Q(album__artist_credit__artist=artist) ) data = get_stats(tracks, artist) return response.Response(data, status=200) @@ -128,8 +128,8 @@ class ManageAlbumViewSet( queryset = ( music_models.Album.objects.all() .order_by("-id") - .select_related("attributed_to", "artist", "attachment_cover") - .prefetch_related("tracks") + .select_related("attributed_to", "attachment_cover") + .prefetch_related("tracks", "artist_credit__artist") ) serializer_class = serializers.ManageAlbumSerializer filterset_class = filters.ManageAlbumFilterSet @@ -177,10 +177,10 @@ class ManageTrackViewSet( queryset = ( music_models.Track.objects.all() .order_by("-id") - .select_related( + .prefetch_related( "attributed_to", - "artist", - "album__artist", + "artist_credit", + "album__artist_credit", "album__attachment_cover", "attachment_cover", ) @@ -273,11 +273,11 @@ class ManageLibraryViewSet( ) artists = set( music_models.Album.objects.filter(pk__in=albums).values_list( - "artist", flat=True + "artist_credit__artist", flat=True ) ) | set( music_models.Track.objects.filter(pk__in=tracks).values_list( - "artist", flat=True + "artist_credit__artist", flat=True ) ) @@ -313,7 +313,11 @@ class ManageUploadViewSet( queryset = ( music_models.Upload.objects.all() .order_by("-id") - .select_related("library__actor", "track__artist", "track__album__artist") + .prefetch_related( + "library__actor", + "track__artist_credit__artist", + "track__album__artist_credit__artist", + ) ) serializer_class = serializers.ManageUploadSerializer filterset_class = filters.ManageUploadFilterSet @@ -703,8 +707,8 @@ class ManageChannelViewSet( music_models.Artist.objects.all() .order_by("-id") .select_related("attributed_to", "attachment_cover", "channel") - .annotate(_tracks_count=Count("tracks")) - .annotate(_albums_count=Count("albums")) + .annotate(_tracks_count=Count("artist_credit__tracks")) + .annotate(_albums_count=Count("artist_credit__albums")) .prefetch_related(music_views.TAG_PREFETCH) ), ) @@ -720,7 +724,8 @@ class ManageChannelViewSet( def stats(self, request, *args, **kwargs): channel = self.get_object() tracks = music_models.Track.objects.filter( - Q(artist=channel.artist) | Q(album__artist=channel.artist) + Q(artist_credit__artist=channel.artist) + | Q(album__artist_credit__artist=channel.artist) ) data = get_stats(tracks, channel, ignore_fields=["libraries", "channels"]) data["follows"] = channel.actor.received_follows.count() diff --git a/api/funkwhale_api/moderation/filters.py b/api/funkwhale_api/moderation/filters.py index 288dd86f8..ce94186e2 100644 --- a/api/funkwhale_api/moderation/filters.py +++ b/api/funkwhale_api/moderation/filters.py @@ -4,11 +4,24 @@ from django_filters import rest_framework as filters USER_FILTER_CONFIG = { "ARTIST": {"target_artist": ["pk"]}, "CHANNEL": {"target_artist": ["artist__pk"]}, - "ALBUM": {"target_artist": ["artist__pk"]}, - "TRACK": {"target_artist": ["artist__pk", "album__artist__pk"]}, - "LISTENING": {"target_artist": ["track__album__artist__pk", "track__artist__pk"]}, + "ALBUM": {"target_artist": ["artist_credit__artist__pk"]}, + "TRACK": { + "target_artist": [ + "artist_credit__artist__pk", + "album__artist_credit__artist__pk", + ] + }, + "LISTENING": { + "target_artist": [ + "track__album__artist_credit__artist__pk", + "track__artist_credit__artist__pk", + ] + }, "TRACK_FAVORITE": { - "target_artist": ["track__album__artist__pk", "track__artist__pk"] + "target_artist": [ + "track__album__artist_credit__artist__pk", + "track__artist_credit__artist__pk", + ] }, } diff --git a/api/funkwhale_api/moderation/serializers.py b/api/funkwhale_api/moderation/serializers.py index 382d81aa8..dc2684144 100644 --- a/api/funkwhale_api/moderation/serializers.py +++ b/api/funkwhale_api/moderation/serializers.py @@ -89,10 +89,29 @@ class ArtistStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): ] +@state_serializers.register(name="music.ArtistCredit") +class ArtistCreditStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): + artist = ArtistStateSerializer() + + class Meta: + model = music_models.ArtistCredit + fields = [ + "id", + "credit", + "mbid", + "fid", + "creation_date", + "uuid", + "artist", + "joinphrase", + "index", + ] + + @state_serializers.register(name="music.Album") class AlbumStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): tags = TAGS_FIELD - artist = ArtistStateSerializer() + artist_credit = ArtistCreditStateSerializer(many=True) class Meta: model = music_models.Album @@ -103,7 +122,7 @@ class AlbumStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): "fid", "creation_date", "uuid", - "artist", + "artist_credit", "release_date", "tags", "description", @@ -113,7 +132,7 @@ class AlbumStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): @state_serializers.register(name="music.Track") class TrackStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): tags = TAGS_FIELD - artist = ArtistStateSerializer() + artist_credit = ArtistCreditStateSerializer(many=True) album = AlbumStateSerializer() class Meta: @@ -125,7 +144,7 @@ class TrackStateSerializer(DescriptionStateMixin, serializers.ModelSerializer): "fid", "creation_date", "uuid", - "artist", + "artist_credit", "album", "disc_number", "position", @@ -230,6 +249,7 @@ TARGET_CONFIG = { "id_field": serializers.UUIDField(), }, "artist": {"queryset": music_models.Artist.objects.all()}, + "artist_credit": {"queryset": music_models.ArtistCredit.objects.all()}, "album": {"queryset": music_models.Album.objects.all()}, "track": {"queryset": music_models.Track.objects.all()}, "library": { diff --git a/api/funkwhale_api/music/admin.py b/api/funkwhale_api/music/admin.py index 7e6a79450..dec75f484 100644 --- a/api/funkwhale_api/music/admin.py +++ b/api/funkwhale_api/music/admin.py @@ -3,6 +3,17 @@ from funkwhale_api.common import admin from . import models +@admin.register(models.ArtistCredit) +class ArtistCreditAdmin(admin.ModelAdmin): + list_display = [ + "artist", + "credit", + "joinphrase", + "creation_date", + ] + search_fields = ["artist__name", "credit"] + + @admin.register(models.Artist) class ArtistAdmin(admin.ModelAdmin): list_display = ["name", "mbid", "creation_date", "modification_date"] @@ -11,16 +22,18 @@ class ArtistAdmin(admin.ModelAdmin): @admin.register(models.Album) class AlbumAdmin(admin.ModelAdmin): - list_display = ["title", "artist", "mbid", "release_date", "creation_date"] - search_fields = ["title", "artist__name", "mbid"] + list_display = ["title", "mbid", "release_date", "creation_date"] + search_fields = ["title", "mbid"] list_select_related = True @admin.register(models.Track) class TrackAdmin(admin.ModelAdmin): - list_display = ["title", "artist", "album", "mbid"] - search_fields = ["title", "artist__name", "album__title", "mbid"] - list_select_related = ["album__artist", "artist"] + list_display = ["title", "album", "mbid", "artist"] + search_fields = ["title", "album__title", "mbid"] + + def artist(self, obj): + return obj.get_artist_credit_string @admin.register(models.TrackActor) diff --git a/api/funkwhale_api/music/dynamic_preferences_registry.py b/api/funkwhale_api/music/dynamic_preferences_registry.py index 2afe13ed4..f8e63c72a 100644 --- a/api/funkwhale_api/music/dynamic_preferences_registry.py +++ b/api/funkwhale_api/music/dynamic_preferences_registry.py @@ -1,3 +1,4 @@ +from django.forms import widgets from dynamic_preferences import types from dynamic_preferences.registries import global_preferences_registry @@ -159,3 +160,45 @@ class ReleaseDate(types.BooleanPreference): verbose_name = "Release date Filter" help_text = "Only content with a release date will be displayed" default = False + + +@global_preferences_registry.register +class JoinPhrases(types.StringPreference): + show_in_api = True + section = music + name = "join_phrases" + verbose_name = "Join Phrases" + help_text = ( + "Used by the artist parser to create multiples artists in case the metadata " + "is a single string. BE WARNED, changing the order or the values can break the parser in unexpected ways. " + "It's MANDATORY to escape dots and to put doted variation before because the first match is used " + r"(example : `|feat\.|ft\.|feat|` and not `feat|feat\.|ft\.|feat`.). ORDER is really important " + "(says an anarchist). To avoid artist duplication and wrongly parsed artist data " + "it's recommended to tag files with Musicbrainz Picard. " + ) + default = ( + r"featuring | feat\. | ft\. | feat | with | and | & | vs\. | \| | \||\| |\|| , | ,|, |,|" + r" ; | ;|; |;| versus | vs | \( | \(|\( |\(| Remix\) |Remix\) | Remix\)| \) | \)|\) |\)| x |" + "accompanied by | alongside | together with | collaboration with | featuring special guest |" + "joined by | joined with | featuring guest | introducing | accompanied by | performed by | performed with |" + "performed by and | and | featuring | with | presenting | accompanied by | and special guest |" + "featuring special guests | featuring and | featuring & | and featuring " + ) + widget = widgets.Textarea + field_kwargs = {"required": False} + + +@global_preferences_registry.register +class DefaultJoinPhrases(types.StringPreference): + show_in_api = True + section = music + name = "default_join_phrase" + verbose_name = "Default Join Phrase" + help_text = ( + "The default join phrase used by artist parser" + "For example: `artists = [artist1, Artist2]` will be displayed has : artist1.name, artis2.name" + "Changing this value will not update already parsed artists" + ) + default = ", " + widget = widgets.Textarea + field_kwargs = {"required": False} diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 58e201d2e..e1a66ceb0 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -1,6 +1,8 @@ import os +from urllib.parse import urlparse import factory +from django.conf import settings from funkwhale_api.common import factories as common_factories from funkwhale_api.factories import NoUpdateOnCreate, registry @@ -62,7 +64,7 @@ class ArtistFactory( name = factory.Faker("name") mbid = factory.Faker("uuid4") fid = factory.Faker("federation_url") - playable = playable_factory("track__album__artist") + playable = playable_factory("track__album__artist_credit__artist") class Meta: model = "music.Artist" @@ -77,6 +79,16 @@ class ArtistFactory( ) +@registry.register +class ArtistCreditFactory(factory.django.DjangoModelFactory): + artist = factory.SubFactory(ArtistFactory) + credit = factory.LazyAttribute(lambda obj: obj.artist.name) + joinphrase = "" + + class Meta: + model = "music.ArtistCredit" + + @registry.register class AlbumFactory( tags_factories.TaggableFactory, NoUpdateOnCreate, factory.django.DjangoModelFactory @@ -84,7 +96,6 @@ class AlbumFactory( title = factory.Faker("sentence", nb_words=3) mbid = factory.Faker("uuid4") release_date = factory.Faker("date_object") - artist = factory.SubFactory(ArtistFactory) release_group_id = factory.Faker("uuid4") fid = factory.Faker("federation_url") playable = playable_factory("track__album") @@ -96,14 +107,22 @@ class AlbumFactory( attributed = factory.Trait( attributed_to=factory.SubFactory(federation_factories.ActorFactory) ) - local = factory.Trait( - fid=factory.Faker("federation_url", local=True), artist__local=True + fid=factory.Faker("federation_url", local=True), ) with_cover = factory.Trait( attachment_cover=factory.SubFactory(common_factories.AttachmentFactory) ) + @factory.post_generation + def artist_credit(self, create, extracted, **kwargs): + if urlparse(self.fid).netloc == settings.FEDERATION_HOSTNAME: + kwargs["artist__local"] = True + if extracted: + self.artist_credit.add(extracted) + if create: + self.artist_credit.add(ArtistCreditFactory(**kwargs)) + @registry.register class TrackFactory( @@ -132,22 +151,29 @@ class TrackFactory( ) @factory.post_generation - def artist(self, created, extracted, **kwargs): + def artist_credit(self, created, extracted, **kwargs): """ A bit intricated, because we want to be able to specify a different track artist with a fallback on album artist if nothing is specified. And handle cases where build or build_batch are used (so no db calls) """ + # needed to get a primary key on the track and album objects. The primary key is needed for many_to_many + if self.album: + self.album.save() + + if not self.pk: + self.save() + if extracted: - self.artist = extracted + self.artist_credit.add(extracted) elif kwargs: if created: - self.artist = ArtistFactory(**kwargs) + self.artist_credit.add(ArtistCreditFactory(**kwargs)) else: - self.artist = ArtistFactory.build(**kwargs) + self.artist_credit.add(ArtistCreditFactory.build(**kwargs)) elif self.album: - self.artist = self.album.artist + self.artist_credit.set(self.album.artist_credit.all()) if created: self.save() @@ -195,7 +221,9 @@ class UploadFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): from funkwhale_api.audio import factories as audio_factories audio_factories.ChannelFactory( - library=self.library, artist=self.track.artist, **kwargs + library=self.library, + artist=self.track.artist_credit.all()[0].artist, + **kwargs ) diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index cc83d7782..1f0eedd76 100644 --- a/api/funkwhale_api/music/filters.py +++ b/api/funkwhale_api/music/filters.py @@ -100,9 +100,9 @@ class ArtistFilter( tag = TAG_FILTER content_category = filters.CharFilter("content_category") scope = common_filters.ActorScopeFilter( - actor_field="tracks__uploads__library__actor", + actor_field="artist_credit__tracks__uploads__library__actor", distinct=True, - library_field="tracks__uploads__library", + library_field="artist_credit__tracks__uploads__library", ) ordering = common_filters.CaseInsensitiveNameOrderingFilter( fields=( @@ -128,14 +128,14 @@ class ArtistFilter( } hidden_content_fields_mapping = moderation_filters.USER_FILTER_CONFIG["ARTIST"] include_channels_field = "channel" - library_filter_field = "track__artist" + library_filter_field = "track__artist_credit__artist" def filter_playable(self, queryset, name, value): actor = utils.get_actor_from_request(self.request) return queryset.playable_by(actor, value).distinct() def filter_has_albums(self, queryset, name, value): - return queryset.filter(albums__isnull=not value) + return queryset.filter(artist_credit__albums__isnull=not value) def filter_has_mbid(self, queryset, name, value): return queryset.filter(mbid__isnull=(not value)) @@ -149,8 +149,16 @@ class TrackFilter( moderation_filters.HiddenContentFilterSet, ): q = fields.SearchFilter( - search_fields=["title", "album__title", "artist__name"], - fts_search_fields=["body_text", "artist__body_text", "album__body_text"], + search_fields=[ + "title", + "album__title", + "artist_credit__artist__name", + ], + fts_search_fields=[ + "body_text", + "artist_credit__artist__body_text", + "album__body_text", + ], ) playable = filters.BooleanFilter(field_name="_", method="filter_playable") tag = TAG_FILTER @@ -173,8 +181,11 @@ class TrackFilter( ("size", "size"), ("position", "position"), ("disc_number", "disc_number"), - ("artist__name", "artist__name"), - ("artist__modification_date", "artist__modification_date"), + ("artist_credit__artist__name", "artist_credit__artist__name"), + ( + "artist_credit__artist__modification_date", + "artist_credit__artist__modification_date", + ), ("?", "random"), ("tag_matches", "related"), ) @@ -205,16 +216,19 @@ class TrackFilter( "mbid": ["exact"], } hidden_content_fields_mapping = moderation_filters.USER_FILTER_CONFIG["TRACK"] - include_channels_field = "artist__channel" + include_channels_field = "artist_credit__artist__channel" channel_filter_field = "track" library_filter_field = "track" + artist_credit_filter_field = "artist__credit__artist" def filter_playable(self, queryset, name, value): actor = utils.get_actor_from_request(self.request) return queryset.playable_by(actor, value).distinct() def filter_artist(self, queryset, name, value): - return queryset.filter(Q(artist=value) | Q(album__artist=value)) + return queryset.filter( + Q(artist_credit__artist=value) | Q(album__artist_credit__artist=value) + ) def filter_format(self, queryset, name, value): mimetypes = [utils.get_type_from_ext(e) for e in value.split(",")] @@ -238,8 +252,8 @@ class UploadFilter(audio_filters.IncludeChannelsFilterSet): library = filters.CharFilter("library__uuid") channel = filters.CharFilter("library__channel__uuid") track = filters.UUIDFilter("track__uuid") - track_artist = filters.UUIDFilter("track__artist__uuid") - album_artist = filters.UUIDFilter("track__album__artist__uuid") + track_artist = filters.UUIDFilter("track__artist_credit__artist__uuid") + album_artist = filters.UUIDFilter("track__album__artist_credit__artist__uuid") library = filters.UUIDFilter("library__uuid") playable = filters.BooleanFilter(field_name="_", method="filter_playable") scope = common_filters.ActorScopeFilter( @@ -273,7 +287,7 @@ class UploadFilter(audio_filters.IncludeChannelsFilterSet): "mimetype", "import_reference", ] - include_channels_field = "track__artist__channel" + include_channels_field = "track__artist_credit__artist__channel" def filter_playable(self, queryset, name, value): actor = utils.get_actor_from_request(self.request) @@ -289,10 +303,10 @@ class AlbumFilter( ): playable = filters.BooleanFilter(field_name="_", method="filter_playable") q = fields.SearchFilter( - search_fields=["title", "artist__name"], - fts_search_fields=["body_text", "artist__body_text"], + search_fields=["title", "artist_credit__artist__name"], + fts_search_fields=["body_text", "artist_credit__artist__body_text"], ) - content_category = filters.CharFilter("artist__content_category") + content_category = filters.CharFilter("artist_credit__artist__content_category") tag = TAG_FILTER scope = common_filters.ActorScopeFilter( actor_field="tracks__uploads__library__actor", @@ -305,7 +319,10 @@ class AlbumFilter( ("creation_date", "creation_date"), ("release_date", "release_date"), ("title", "title"), - ("artist__modification_date", "artist__modification_date"), + ( + "artist_credit__artist__modification_date", + "artist_credit__artist__modification_date", + ), ("?", "random"), ("tag_matches", "related"), ) @@ -326,15 +343,18 @@ class AlbumFilter( ) has_release_date = filters.BooleanFilter( - field_name="_", - method="filter_has_release_date", + field_name="_", method="filter_has_release_date" + ) + + artist = filters.ModelChoiceFilter( + field_name="_", method="filter_artist", queryset=models.Artist.objects.all() ) class Meta: model = models.Album - fields = ["artist", "mbid"] + fields = ["artist_credit", "mbid"] hidden_content_fields_mapping = moderation_filters.USER_FILTER_CONFIG["ALBUM"] - include_channels_field = "artist__channel" + include_channels_field = "artist_credit__artist__channel" channel_filter_field = "track__album" library_filter_field = "track__album" @@ -354,6 +374,9 @@ class AlbumFilter( def filter_has_release_date(self, queryset, name, value): return queryset.filter(release_date__isnull=(not value)) + def filter_artist(self, queryset, name, value): + return queryset.filter(artist_credit__artist=value) + class LibraryFilter(filters.FilterSet): q = fields.SearchFilter( diff --git a/api/funkwhale_api/music/importers.py b/api/funkwhale_api/music/importers.py index 14b63a55a..d71f9fcd7 100644 --- a/api/funkwhale_api/music/importers.py +++ b/api/funkwhale_api/music/importers.py @@ -12,11 +12,14 @@ class Importer: def load(self, cleaned_data, raw_data, import_hooks): mbid = cleaned_data.pop("mbid") + artists_credits = cleaned_data.pop("artist_credit", None) # let's validate data, just in case instance = self.model(**cleaned_data) exclude = EXCLUDE_VALIDATION.get(self.model.__name__, []) instance.full_clean(exclude=["mbid", "uuid", "fid", "from_activity"] + exclude) m = self.model.objects.update_or_create(mbid=mbid, defaults=cleaned_data)[0] + if artists_credits: + m.artist_credit.set(artists_credits) for hook in import_hooks: hook(m, cleaned_data, raw_data) return m @@ -47,4 +50,9 @@ class Mapping: ) -registry = {"Artist": Importer, "Track": Importer, "Album": Importer} +registry = { + "Artist": Importer, + "ArtistCredit": Importer, + "Track": Importer, + "Album": Importer, +} diff --git a/api/funkwhale_api/music/management/commands/import_files.py b/api/funkwhale_api/music/management/commands/import_files.py index fc9f5737e..2435b6fe3 100644 --- a/api/funkwhale_api/music/management/commands/import_files.py +++ b/api/funkwhale_api/music/management/commands/import_files.py @@ -700,10 +700,10 @@ def handle_modified(event, stdout, library, in_place, **kwargs): to_update = ( existing_candidates.in_place() .filter(source=source) - .select_related( - "track__attributed_to", - "track__artist", - "track__album__artist", + .select_related("track__attributed_to") + .prefetch_related( + "track__artist_credit__artist", + "track__album__artist_credit__artist", ) .first() ) @@ -839,9 +839,9 @@ def check_upload(stdout, upload): " Cannot update track metadata, track belongs to someone else" ) else: - track = models.Track.objects.select_related("artist", "album__artist").get( - pk=upload.track_id - ) + track = models.Track.objects.prefetch_related( + "artist_credit__artist", "album__artist_credit__artist" + ).get(pk=upload.track_id) try: tasks.update_track_metadata(upload.get_metadata(), track) except serializers.ValidationError as e: diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index c5479f860..4084dccd7 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/api/funkwhale_api/music/metadata.py @@ -492,61 +492,76 @@ class ArtistField(serializers.Field): return final - def to_internal_value(self, data): - # we have multiple values that can be separated by various separators - separators = [";", ","] + def _get_artist_credit_tuple(self, mbids, data): + from . import tasks + + names_artists_credits_tuples = tasks.parse_credits( + data.get("names", ""), None, None + ) + + artist_artists_credits_tuples = tasks.parse_credits( + data.get("artists", ""), None, None + ) + + len_mbids = len(mbids) + if ( + len(names_artists_credits_tuples) != len_mbids + and len(artist_artists_credits_tuples) != len_mbids + ): + logger.warning( + "Error parsing artist data, not the same amount of mbids and parsed artists. \ + Probably because the artist parser found more artists than there is." + ) + + if len(names_artists_credits_tuples) > len(artist_artists_credits_tuples): + return names_artists_credits_tuples + return artist_artists_credits_tuples + + def _get_mbids(self, raw_mbids): + # we have multiple mbid 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: - if "/" in raw_mbids: - # it's a featuring, we can't handle this now - mbids = [] - else: - 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 - 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)] + if separator in raw_mbids: + mbids = [m.strip() for m in raw_mbids.split(separator)] break - # corner case: 'album artist' field with only one artist but multiple names in 'artits' field - if ( - not names - and data.get("names", None) - and any(separator in data["names"] for separator in separators) - ): - names = [n.strip() for n in data["names"].split(separators[0])] - elif 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"]] + return mbids + + def _format_artist_credit_list(self, artists_credits_tuples, mbids): + final_artist_credits = [] + for i, ac in enumerate(artists_credits_tuples): + artist_credit = { + "credit": ac[0], + "mbid": (mbids[i] if 0 <= i < len(mbids) else None), + "joinphrase": ac[1], + "index": i, + } + final_artist_credits.append(artist_credit) + + return final_artist_credits + + def to_internal_value(self, data): + if ( + self.context.get("strict", True) + and not data.get("artists", []) + and not data.get("names", []) + ): + raise serializers.ValidationError("This field is required.") + mbids = self._get_mbids(data["mbids"]) + # now, we split on artist names + artists_credits_tuples = self._get_artist_credit_tuple(mbids, data) + final_artist_credits = self._format_artist_credit_list( + artists_credits_tuples, mbids + ) - 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(strict=self.context.get("strict", True)), min_length=1, ) - return field.to_internal_value(final) + return field.to_internal_value(final_artist_credits) class AlbumField(serializers.Field): @@ -565,16 +580,17 @@ class AlbumField(serializers.Field): "release_date": data.get("date", None), "mbid": data.get("musicbrainz_albumid", None), } - artists_field = ArtistField(for_album=True) - payload = artists_field.get_value(data) + artist_credit_field = ArtistField(for_album=True) + payload = artist_credit_field.get_value(data) try: - artists = artists_field.to_internal_value(payload) + artist_credit = artist_credit_field.to_internal_value(payload) except serializers.ValidationError as e: - artists = [] - logger.debug("Ignoring validation error on album artists: %s", e) + artist_credit = [] + logger.debug("Ignoring validation error on album artist_credit: %s", e) album_serializer = AlbumSerializer(data=final) album_serializer.is_valid(raise_exception=True) - album_serializer.validated_data["artists"] = artists + album_serializer.validated_data["artist_credit"] = artist_credit + return album_serializer.validated_data @@ -655,14 +671,17 @@ class MBIDField(serializers.UUIDField): class ArtistSerializer(serializers.Serializer): - name = serializers.CharField(required=False, allow_null=True, allow_blank=True) + credit = serializers.CharField(required=False, allow_null=True, allow_blank=True) mbid = MBIDField() + joinphrase = serializers.CharField( + trim_whitespace=False, required=False, allow_null=True, allow_blank=True + ) def __init__(self, *args, **kwargs): self.strict = kwargs.pop("strict", True) super().__init__(*args, **kwargs) - def validate_name(self, v): + def validate_credit(self, v): if self.strict and not v: raise serializers.ValidationError("This field is required.") return v @@ -731,7 +750,7 @@ class TrackMetadataSerializer(serializers.Serializer): description = DescriptionField(allow_null=True, allow_blank=True, required=False) album = AlbumField() - artists = ArtistField() + artist_credit = ArtistField() cover_data = CoverDataField(required=False) remove_blank_null_fields = [ diff --git a/api/funkwhale_api/music/migrations/0059_remove_album_artist_remove_track_artist_artistcredit_and_more.py b/api/funkwhale_api/music/migrations/0059_remove_album_artist_remove_track_artist_artistcredit_and_more.py new file mode 100644 index 000000000..7caaf58b0 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0059_remove_album_artist_remove_track_artist_artistcredit_and_more.py @@ -0,0 +1,128 @@ +# Generated by Django 4.2.9 on 2024-03-16 00:36 + +import django.contrib.postgres.search +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import uuid + + +def skip(apps, schema_editor): + pass + + +def save_artist_credit(obj, ArtistCredit): + artist_credit, created = ArtistCredit.objects.get_or_create( + artist=obj.artist, + joinphrase="", + credit=obj.artist.name, + ) + obj.artist_credit.set([artist_credit]) + obj.save() + + +def set_all_artists_credit(apps, schema_editor): + Track = apps.get_model("music", "Track") + Album = apps.get_model("music", "Album") + ArtistCredit = apps.get_model("music", "ArtistCredit") + + for track in Track.objects.all(): + save_artist_credit(track, ArtistCredit) + + for album in Album.objects.all(): + save_artist_credit(album, ArtistCredit) + + +class Migration(migrations.Migration): + dependencies = [ + ("music", "0058_upload_quality"), + ] + + operations = [ + migrations.CreateModel( + name="ArtistCredit", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "fid", + models.URLField( + db_index=True, max_length=500, null=True, unique=True + ), + ), + ( + "mbid", + models.UUIDField(blank=True, db_index=True, null=True, unique=True), + ), + ( + "uuid", + models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + ( + "creation_date", + models.DateTimeField( + db_index=True, default=django.utils.timezone.now + ), + ), + ( + "body_text", + django.contrib.postgres.search.SearchVectorField(blank=True), + ), + ("credit", models.CharField(blank=True, max_length=500, null=True)), + ("joinphrase", models.CharField(blank=True, max_length=250, null=True)), + ("index", models.IntegerField(blank=True, null=True)), + ( + "artist", + models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="artist_credit", + to="music.artist", + ), + ), + ( + "from_activity", + models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + to="federation.activity", + ), + ), + ], + options={ + "ordering": ["index", "credit"], + }, + ), + migrations.AddField( + model_name="album", + name="artist_credit", + field=models.ManyToManyField( + related_name="albums", + to="music.artistcredit", + ), + ), + migrations.AddField( + model_name="track", + name="artist_credit", + field=models.ManyToManyField( + related_name="tracks", + to="music.artistcredit", + ), + ), + migrations.RunPython(set_all_artists_credit, skip), + migrations.RemoveField( + model_name="album", + name="artist", + ), + migrations.RemoveField( + model_name="track", + name="artist", + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index cc46cdb61..2f7e3df80 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -27,7 +27,7 @@ from django.utils import timezone from funkwhale_api import musicbrainz from funkwhale_api.common import fields from funkwhale_api.common import models as common_models -from funkwhale_api.common import session +from funkwhale_api.common import preferences, session from funkwhale_api.common import utils as common_utils from funkwhale_api.federation import models as federation_models from funkwhale_api.federation import utils as federation_utils @@ -112,7 +112,6 @@ class APIModelMixin(models.Model): def get_federation_id(self): if self.fid: return self.fid - return federation_utils.full_url( reverse( f"federation:music:{self.federation_namespace}-detail", @@ -171,12 +170,12 @@ class License(models.Model): class ArtistQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def with_albums_count(self): - return self.annotate(_albums_count=models.Count("albums")) + return self.annotate(_albums_count=models.Count("artist_credit__albums")) def with_albums(self): return self.prefetch_related( models.Prefetch( - "albums", + "artist_credit__albums", queryset=Album.objects.with_tracks_count().select_related( "attachment_cover", "attributed_to" ), @@ -186,7 +185,7 @@ class ArtistQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def annotate_playable_by_actor(self, actor): tracks = ( Upload.objects.playable_by(actor) - .filter(track__artist=models.OuterRef("id")) + .filter(track__artist_credit__artist=models.OuterRef("id")) .order_by("id") .values("id")[:1] ) @@ -195,7 +194,9 @@ class ArtistQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def playable_by(self, actor, include=True): tracks = Track.objects.playable_by(actor) - matches = self.filter(pk__in=tracks.values("artist_id")).values_list("pk") + matches = self.filter( + pk__in=tracks.values("artist_credit__artist") + ).values_list("pk") if include: return self.filter(pk__in=matches) else: @@ -272,9 +273,25 @@ class Artist(APIModelMixin): return None -def import_artist(v): - a = Artist.get_or_create_from_api(mbid=v[0]["artist"]["id"])[0] - return a +def import_artist_credit(v): + artists_credits = [] + for i, ac in enumerate(v): + artist, create = Artist.get_or_create_from_api(mbid=ac["artist"]["id"]) + + if "joinphrase" in ac["artist"]: + joinphrase = ac["artist"]["joinphrase"] + elif i < len(v): + joinphrase = preferences.get("music__default_join_phrase") + else: + joinphrase = "" + artist_credit, created = ArtistCredit.objects.get_or_create( + artist=artist, + credit=ac["artist"]["name"], + index=i, + joinphrase=joinphrase, + ) + artists_credits.append(artist_credit) + return artists_credits def parse_date(v): @@ -290,6 +307,39 @@ def import_tracks(instance, cleaned_data, raw_data): importers.load(Track, track_cleaned_data, track_data, Track.import_hooks) +class ArtistCreditQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): + def albums(self): + albums_ids = self.prefetch_related("albums").values_list("albums") + return Album.objects.filter(id__in=albums_ids) + + +class ArtistCredit(APIModelMixin): + artist = models.ForeignKey( + Artist, related_name="artist_credit", on_delete=models.CASCADE + ) + credit = models.CharField( + null=True, + blank=True, + max_length=500, + ) + joinphrase = models.CharField( + null=True, + blank=True, + max_length=250, + ) + index = models.IntegerField( + null=True, + blank=True, + ) + + federation_namespace = "artistcredit" + + objects = ArtistCreditQuerySet.as_manager() + + class Meta: + ordering = ["index", "credit"] + + class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def with_tracks_count(self): return self.annotate(_tracks_count=models.Count("tracks")) @@ -297,7 +347,7 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def annotate_playable_by_actor(self, actor): tracks = ( Upload.objects.playable_by(actor) - .filter(track__album=models.OuterRef("id")) + .filter(track__artist_credit__albums=models.OuterRef("id")) .order_by("id") .values("id")[:1] ) @@ -329,7 +379,7 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): class Album(APIModelMixin): title = models.TextField() - artist = models.ForeignKey(Artist, related_name="albums", on_delete=models.CASCADE) + artist_credit = models.ManyToManyField(ArtistCredit, related_name="albums") release_date = models.DateField(null=True, blank=True, db_index=True) release_group_id = models.UUIDField(null=True, blank=True) attachment_cover = models.ForeignKey( @@ -380,9 +430,9 @@ class Album(APIModelMixin): "title": {"musicbrainz_field_name": "title"}, "release_date": {"musicbrainz_field_name": "date", "converter": parse_date}, "type": {"musicbrainz_field_name": "type", "converter": lambda v: v.lower()}, - "artist": { + "artist_credit": { "musicbrainz_field_name": "artist-credit", - "converter": import_artist, + "converter": import_artist_credit, }, } objects = AlbumQuerySet.as_manager() @@ -405,6 +455,13 @@ class Album(APIModelMixin): kwargs.update({"title": title}) return cls.objects.get_or_create(title__iexact=title, defaults=kwargs) + @property + def get_artist_credit_string(self): + return utils.get_artist_credit_string(self) + + def get_artists_list(self): + return [ac.artist for ac in self.artist_credit.all()] + def import_tags(instance, cleaned_data, raw_data): MINIMUM_COUNT = 2 @@ -428,11 +485,11 @@ def import_album(v): class TrackQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def for_nested_serialization(self): return self.prefetch_related( - "artist", + "artist_credit", Prefetch( "album", - queryset=Album.objects.select_related( - "artist", "attachment_cover" + queryset=Album.objects.prefetch_related( + "artist_credit", "attachment_cover" ).annotate(_prefetched_tracks_count=Count("tracks")), ), ) @@ -485,7 +542,7 @@ def get_artist(release_list): class Track(APIModelMixin): mbid = models.UUIDField(db_index=True, null=True, blank=True) title = models.TextField() - artist = models.ForeignKey(Artist, related_name="tracks", on_delete=models.CASCADE) + artist_credit = models.ManyToManyField(ArtistCredit, related_name="tracks") disc_number = models.PositiveIntegerField(null=True, blank=True) position = models.PositiveIntegerField(null=True, blank=True) album = models.ForeignKey( @@ -527,11 +584,9 @@ class Track(APIModelMixin): musicbrainz_mapping = { "mbid": {"musicbrainz_field_name": "id"}, "title": {"musicbrainz_field_name": "title"}, - "artist": { + "artist_credit": { "musicbrainz_field_name": "artist-credit", - "converter": lambda v: Artist.get_or_create_from_api( - mbid=v[0]["artist"]["id"] - )[0], + "converter": import_artist_credit, }, "album": {"musicbrainz_field_name": "release-list", "converter": import_album}, } @@ -559,19 +614,21 @@ class Track(APIModelMixin): def get_moderation_url(self): return f"/manage/library/tracks/{self.pk}" - def save(self, **kwargs): - try: - self.artist - except Artist.DoesNotExist: - self.artist = self.album.artist - super().save(**kwargs) + @property + def get_artist_credit_string(self): + return utils.get_artist_credit_string(self) + + def get_artists_list(self): + return [ac.artist for ac in self.artist_credit.all()] @property def full_name(self): try: - return f"{self.artist.name} - {self.album.title} - {self.title}" + return ( + f"{self.get_artist_credit_string} - {self.album.title} - {self.title}" + ) except AttributeError: - return f"{self.artist.name} - {self.title}" + return f"{self.get_artist_credit_string} - {self.title}" @property def cover(self): @@ -587,6 +644,9 @@ class Track(APIModelMixin): kwargs.update({"title": title}) return cls.objects.get_or_create(title__iexact=title, defaults=kwargs) + # not used anymore, allow increase of performance when importing tracks using mbids. + # In its actual state it will not work since it assume track_data["recording"]["artist-credit"] can + # contain a joinphrase but it's not he case. Needs to be updated. @classmethod def get_or_create_from_release(cls, release_mbid, mbid): release_mbid = str(release_mbid) @@ -609,33 +669,42 @@ class Track(APIModelMixin): if not track_data: raise ValueError("No track found matching this ID") - track_artist_mbid = None - for ac in track_data["recording"]["artist-credit"]: + artists_credits = [] + for i, ac in enumerate(track_data["recording"]["artist-credit"]): try: ac_mbid = ac["artist"]["id"] except TypeError: - # it's probably a string, like "feat." + # it's probably a string, like "feat.". continue - if ac_mbid == str(album.artist.mbid): - continue + track_artist = Artist.get_or_create_from_api(ac_mbid)[0] - track_artist_mbid = ac_mbid - break - track_artist_mbid = track_artist_mbid or album.artist.mbid - if track_artist_mbid == str(album.artist.mbid): - track_artist = album.artist - else: - track_artist = Artist.get_or_create_from_api(track_artist_mbid)[0] - return cls.objects.update_or_create( + if "joinphrase" not in ac: + joinphrase = "" + else: + joinphrase = ac["joinphrase"] + + artist_credit, create = ArtistCredit.objects.get_or_create( + artist=track_artist, + credit=ac["artist"]["name"], + joinphrase=joinphrase, + index=i, + ) + artists_credits.append(artist_credit) + + if album.artist_credit.all() != artist_credit: + album.artist_credit.set(artists_credits) + + track = cls.objects.update_or_create( mbid=mbid, defaults={ "position": int(track["position"]), "title": track["recording"]["title"], "album": album, - "artist": track_artist, }, ) + track[0].artist_credit.set(artists_credits) + return track @property def listen_url(self) -> str: @@ -809,7 +878,7 @@ class Upload(models.Model): title_parts.append(self.track.title) if self.track.album: title_parts.append(self.track.album.title) - title_parts.append(self.track.artist.name) + title_parts.append(self.track.get_artist_credit_string) title = " - ".join(title_parts) filename = f"{title}.{extension}" @@ -1032,9 +1101,21 @@ class Upload(models.Model): if self.track.album else tags_models.TaggedItem.objects.none() ) - artist_tags = self.track.artist.tagged_items.all() + artist_tags = [ + ac.artist.tagged_items.all() for ac in self.track.artist_credit.all() + ] + non_empty_artist_tags = [] + for qs in artist_tags: + if qs.exists(): + non_empty_artist_tags.append(qs) - items = (track_tags | album_tags | artist_tags).order_by("tag__name") + if non_empty_artist_tags: + final_qs = (track_tags | album_tags).union(*non_empty_artist_tags) + else: + final_qs = track_tags | album_tags + # this is needed to avoid *** RuntimeError: generator raised StopIteration + final_list = [obj for obj in final_qs] + items = sorted(final_list, key=lambda x: x.tag.name if x.tag else "") return items diff --git a/api/funkwhale_api/music/mutations.py b/api/funkwhale_api/music/mutations.py index 7cd2918d6..856007ed3 100644 --- a/api/funkwhale_api/music/mutations.py +++ b/api/funkwhale_api/music/mutations.py @@ -126,7 +126,11 @@ class TrackMutationSerializer(CoverMutation, TagMutation, DescriptionMutation): return serialized_relations def post_apply(self, obj, validated_data): - channel = obj.artist.get_channel() + channel = ( + obj.artist_credit.all()[0].artist.get_channel() + if len(obj.artist_credit.all()) == 1 + else None + ) if channel: upload = channel.library.uploads.filter(track=obj).first() if upload: diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 17071105e..e36b1a72a 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -81,12 +81,12 @@ class ArtistAlbumSerializer(serializers.Serializer): fid = serializers.URLField() mbid = serializers.UUIDField() title = serializers.CharField() - artist = serializers.SerializerMethodField() + artist_credit = serializers.SerializerMethodField() release_date = serializers.DateField() creation_date = serializers.DateTimeField() - def get_artist(self, o) -> int: - return o.artist_id + def get_artist_credit(self, o) -> int: + return [ac.id for ac in o.artist_credit.all()] def get_tracks_count(self, o) -> int: return len(o.tracks.all()) @@ -113,7 +113,7 @@ class ArtistWithAlbumsInlineChannelSerializer(serializers.Serializer): class ArtistWithAlbumsSerializer(OptionalDescriptionMixin, serializers.Serializer): - albums = ArtistAlbumSerializer(many=True) + albums = serializers.SerializerMethodField() tags = serializers.SerializerMethodField() attributed_to = APIActorSerializer(allow_null=True) channel = ArtistWithAlbumsInlineChannelSerializer(allow_null=True) @@ -127,14 +127,17 @@ class ArtistWithAlbumsSerializer(OptionalDescriptionMixin, serializers.Serialize is_local = serializers.BooleanField() cover = CoverField(allow_null=True) + def get_albums(self, artist): + albums = artist.artist_credit.albums() + return ArtistAlbumSerializer(albums, many=True).data + @extend_schema_field({"type": "array", "items": {"type": "string"}}) def get_tags(self, obj): tagged_items = getattr(obj, "_prefetched_tagged_items", []) return [ti.tag.name for ti in tagged_items] def get_tracks_count(self, o) -> int: - tracks = getattr(o, "_prefetched_tracks", None) - return len(tracks) if tracks else 0 + return getattr(o, "_tracks_count", 0) class SimpleArtistSerializer(serializers.ModelSerializer): @@ -156,11 +159,20 @@ class SimpleArtistSerializer(serializers.ModelSerializer): "description", "attachment_cover", "channel", + "attributed_to", ) -class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): +class ArtistCreditSerializer(serializers.ModelSerializer): artist = SimpleArtistSerializer() + + class Meta: + model = models.ArtistCredit + fields = ["artist", "credit", "joinphrase", "index"] + + +class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): + artist_credit = ArtistCreditSerializer(many=True) cover = CoverField(allow_null=True) is_playable = serializers.SerializerMethodField() tags = serializers.SerializerMethodField() @@ -203,7 +215,7 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): class TrackAlbumSerializer(serializers.ModelSerializer): - artist = SimpleArtistSerializer() + artist_credit = ArtistCreditSerializer(many=True) cover = CoverField(allow_null=True) tracks_count = serializers.SerializerMethodField() @@ -217,7 +229,7 @@ class TrackAlbumSerializer(serializers.ModelSerializer): "fid", "mbid", "title", - "artist", + "artist_credit", "release_date", "cover", "creation_date", @@ -257,7 +269,7 @@ def sort_uploads_for_listen(uploads): class TrackSerializer(OptionalDescriptionMixin, serializers.Serializer): - artist = SimpleArtistSerializer() + artist_credit = ArtistCreditSerializer(many=True) album = TrackAlbumSerializer(read_only=True) uploads = serializers.SerializerMethodField() listen_url = serializers.SerializerMethodField() @@ -400,9 +412,9 @@ class UploadSerializer(serializers.ModelSerializer): def filter_album(qs, context): if "channel" in context: - return qs.filter(artist__channel=context["channel"]) + return qs.filter(artist_credit__artist__channel=context["channel"]) if "actor" in context: - return qs.filter(artist__attributed_to=context["actor"]) + return qs.filter(artist_credit__artist__attributed_to=context["actor"]) return qs.none() @@ -567,12 +579,12 @@ class SimpleAlbumSerializer(serializers.ModelSerializer): class TrackActivitySerializer(activity_serializers.ModelSerializer): type = serializers.SerializerMethodField() name = serializers.CharField(source="title") - artist = serializers.CharField(source="artist.name") + artist_credit = serializers.CharField(source="get_artist_credit_string") album = serializers.SerializerMethodField() class Meta: model = models.Track - fields = ["id", "local_id", "name", "type", "artist", "album"] + fields = ["id", "local_id", "name", "type", "artist_credit", "album"] def get_type(self, obj): return "Audio" @@ -612,9 +624,9 @@ class OembedSerializer(serializers.Serializer): embed_id = None embed_type = None if match.url_name == "library_track": - qs = models.Track.objects.select_related("artist", "album__artist").filter( - pk=int(match.kwargs["pk"]) - ) + qs = models.Track.objects.prefetch_related( + "artist_credit", "album__artist_credit" + ).filter(pk=int(match.kwargs["pk"])) try: track = qs.get() except models.Track.DoesNotExist: @@ -623,7 +635,7 @@ class OembedSerializer(serializers.Serializer): ) embed_type = "track" embed_id = track.pk - data["title"] = f"{track.title} by {track.artist.name}" + data["title"] = f"{track.title} by {track.get_artist_credit_string}" if track.attachment_cover: data[ "thumbnail_url" @@ -637,15 +649,17 @@ class OembedSerializer(serializers.Serializer): data["thumbnail_width"] = 200 data["thumbnail_height"] = 200 data["description"] = track.full_name - data["author_name"] = track.artist.name + data["author_name"] = track.get_artist_credit_string data["height"] = 150 + # here we take the first artist since oembed standard do not allow a list of url data["author_url"] = federation_utils.full_url( common_utils.spa_reverse( - "library_artist", kwargs={"pk": track.artist.pk} + "library_artist", + kwargs={"pk": track.artist_credit.all()[0].artist.pk}, ) ) elif match.url_name == "library_album": - qs = models.Album.objects.select_related("artist").filter( + qs = models.Album.objects.prefetch_related("artist_credit").filter( pk=int(match.kwargs["pk"]) ) try: @@ -662,15 +676,17 @@ class OembedSerializer(serializers.Serializer): ] = album.attachment_cover.download_url_medium_square_crop data["thumbnail_width"] = 200 data["thumbnail_height"] = 200 - data["title"] = f"{album.title} by {album.artist.name}" - data["description"] = f"{album.title} by {album.artist.name}" - data["author_name"] = album.artist.name + data["title"] = f"{album.title} by {album.get_artist_credit_string}" + data["description"] = f"{album.title} by {album.get_artist_credit_string}" + data["author_name"] = album.get_artist_credit_string data["height"] = 400 data["author_url"] = federation_utils.full_url( common_utils.spa_reverse( - "library_artist", kwargs={"pk": album.artist.pk} + "library_artist", + kwargs={"pk": album.artist_credit.all()[0].artist.pk}, ) ) + elif match.url_name == "library_artist": qs = models.Artist.objects.filter(pk=int(match.kwargs["pk"])) try: @@ -681,7 +697,17 @@ class OembedSerializer(serializers.Serializer): ) embed_type = "artist" embed_id = artist.pk - album = artist.albums.exclude(attachment_cover=None).order_by("-id").first() + album_ids = ( + artist.artist_credit.all() + .prefetch_related("albums") + .values_list("albums", flat=True) + ) + album = ( + models.Album.objects.exclude(attachment_cover=None) + .filter(pk__in=album_ids) + .order_by("-id") + .first() + ) if album and album.attachment_cover: data[ @@ -791,32 +817,34 @@ class AlbumCreateSerializer(serializers.Serializer): release_date = serializers.DateField(required=False, allow_null=True) tags = tags_serializers.TagsListField(required=False) description = common_serializers.ContentSerializer(allow_null=True, required=False) - - artist = common_serializers.RelatedField( + # only used in album channel creation, so this is not a list + artist_credit = common_serializers.RelatedField( "id", - queryset=models.Artist.objects.exclude(channel__isnull=True), + queryset=models.ArtistCredit.objects.exclude(artist__channel__isnull=True), required=True, serializer=None, - filters=lambda context: {"attributed_to": context["user"].actor}, + many=True, + filters=lambda context: {"artist__attributed_to": context["user"].actor}, ) def validate(self, validated_data): - duplicates = validated_data["artist"].albums.filter( - title__iexact=validated_data["title"] - ) + duplicates = models.Album.objects.none() + for ac in validated_data["artist_credit"]: + duplicates = duplicates | ac.albums.filter( + title__iexact=validated_data["title"] + ) if duplicates.exists(): raise serializers.ValidationError("An album with this title already exist") return super().validate(validated_data) def to_representation(self, obj): - obj.artist.attachment_cover return AlbumSerializer(obj, context=self.context).data + @transaction.atomic def create(self, validated_data): instance = models.Album.objects.create( attributed_to=self.context["user"].actor, - artist=validated_data["artist"], release_date=validated_data.get("release_date"), title=validated_data["title"], attachment_cover=validated_data.get("cover"), @@ -825,7 +853,8 @@ class AlbumCreateSerializer(serializers.Serializer): instance, "description", validated_data.get("description") ) tag_models.set_tags(instance, *(validated_data.get("tags", []) or [])) - instance.artist.get_channel() + + instance.artist_credit.set(validated_data["artist_credit"]) return instance diff --git a/api/funkwhale_api/music/spa_views.py b/api/funkwhale_api/music/spa_views.py index 3102d8635..2518bfe9c 100644 --- a/api/funkwhale_api/music/spa_views.py +++ b/api/funkwhale_api/music/spa_views.py @@ -24,7 +24,9 @@ def get_twitter_card_metas(type, id): def library_track(request, pk, redirect_to_ap): - queryset = models.Track.objects.filter(pk=pk).select_related("album", "artist") + queryset = models.Track.objects.filter(pk=pk).prefetch_related( + "album", "artist_credit__artist" + ) try: obj = queryset.get() except models.Track.DoesNotExist: @@ -47,15 +49,19 @@ def library_track(request, pk, redirect_to_ap): {"tag": "meta", "property": "og:type", "content": "music.song"}, {"tag": "meta", "property": "music:album:disc", "content": obj.disc_number}, {"tag": "meta", "property": "music:album:track", "content": obj.position}, - { - "tag": "meta", - "property": "music:musician", - "content": utils.join_url( - settings.FUNKWHALE_URL, - utils.spa_reverse("library_artist", kwargs={"pk": obj.artist.pk}), - ), - }, ] + # following https://ogp.me/#array + for ac in obj.artist_credit.all(): + metas.append( + { + "tag": "meta", + "property": "music:musician", + "content": utils.join_url( + settings.FUNKWHALE_URL, + utils.spa_reverse("library_artist", kwargs={"pk": ac.artist.pk}), + ), + } + ) if obj.album: metas.append( @@ -119,7 +125,7 @@ def library_track(request, pk, redirect_to_ap): def library_album(request, pk, redirect_to_ap): - queryset = models.Album.objects.filter(pk=pk).select_related("artist") + queryset = models.Album.objects.filter(pk=pk).prefetch_related("artist_credit") try: obj = queryset.get() except models.Album.DoesNotExist: @@ -136,16 +142,20 @@ def library_album(request, pk, redirect_to_ap): {"tag": "meta", "property": "og:url", "content": album_url}, {"tag": "meta", "property": "og:title", "content": obj.title}, {"tag": "meta", "property": "og:type", "content": "music.album"}, - { - "tag": "meta", - "property": "music:musician", - "content": utils.join_url( - settings.FUNKWHALE_URL, - utils.spa_reverse("library_artist", kwargs={"pk": obj.artist.pk}), - ), - }, ] + # following https://ogp.me/#array + for ac in obj.artist_credit.all(): + metas.append( + { + "tag": "meta", + "property": "music:musician", + "content": utils.join_url( + settings.FUNKWHALE_URL, + utils.spa_reverse("library_artist", kwargs={"pk": ac.artist.pk}), + ), + } + ) if obj.release_date: metas.append( { @@ -206,7 +216,10 @@ def library_artist(request, pk, redirect_to_ap): ) # we use latest album's cover as artist image latest_album = ( - obj.albums.exclude(attachment_cover=None).order_by("release_date").last() + obj.artist_credit.albums() + .exclude(attachment_cover=None) + .order_by("release_date") + .last() ) metas = [ {"tag": "meta", "property": "og:url", "content": artist_url}, @@ -234,7 +247,10 @@ def library_artist(request, pk, redirect_to_ap): ) if ( - models.Upload.objects.filter(Q(track__artist=obj) | Q(track__album__artist=obj)) + models.Upload.objects.filter( + Q(track__artist_credit__artist=obj) + | Q(track__album__artist_credit__artist=obj) + ) .playable_by(None) .exists() ): diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 62d605580..b6d6fa64c 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -2,6 +2,7 @@ import collections import datetime import logging import os +import re from django.conf import settings from django.core.cache import cache @@ -9,7 +10,7 @@ from django.db import transaction from django.db.models import F, Q from django.dispatch import receiver from django.utils import timezone -from musicbrainzngs import ResponseError +from musicbrainzngs import NetworkError, ResponseError from requests.exceptions import RequestException from funkwhale_api import musicbrainz @@ -226,7 +227,7 @@ def process_upload(upload, update_denormalization=True): forced_values["artist"] = upload.library.channel.artist old_status = upload.import_status - additional_data = {"upload_source": upload.source} + upload_source = {"upload_source": upload.source} if use_file_metadata: audio_file = upload.get_audio_file() @@ -256,11 +257,11 @@ def process_upload(upload, update_denormalization=True): ) final_metadata = collections.ChainMap( - additional_data, serializer.validated_data, internal_config + upload_source, serializer.validated_data, internal_config ) else: final_metadata = collections.ChainMap( - additional_data, + upload_source, forced_values, internal_config, ) @@ -270,8 +271,8 @@ def process_upload(upload, update_denormalization=True): ) except UploadImportError as e: return fail_import(upload, e.code) - except Exception: - fail_import(upload, "unknown_error") + except Exception as e: + fail_import(upload, "unknown_error", e) raise broadcast = getter( @@ -395,38 +396,53 @@ def federation_audio_track_to_metadata(payload, references): "cover_data": get_cover(payload["album"], "image"), "release_date": payload["album"].get("released"), "tags": [t["name"] for t in payload["album"].get("tags", []) or []], - "artists": [ + "artist_credit": [ { - "fid": a["id"], - "name": a["name"], - "fdate": a["published"], - "cover_data": get_cover(a, "image"), - "description": a.get("description"), - "attributed_to": references.get(a.get("attributedTo")), - "mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None, - "tags": [t["name"] for t in a.get("tags", []) or []], + "artist": { + "fid": a["artist"]["id"], + "name": a["artist"]["name"], + "fdate": a["artist"]["published"], + "cover_data": get_cover(a["artist"], "image"), + "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, + "tags": [t["name"] for t in a["artist"].get("tags", []) or []], + }, + "joinphrase": (a["joinphrase"] if "joinphrase" in a else ""), + "credit": a["credit"], } - for a in payload["album"]["artists"] + for a in payload["album"]["artist_credit"] ], }, - "artists": [ + "artist_credit": [ { - "fid": a["id"], - "name": a["name"], - "fdate": a["published"], - "description": a.get("description"), - "attributed_to": references.get(a.get("attributedTo")), - "mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None, - "tags": [t["name"] for t in a.get("tags", []) or []], - "cover_data": get_cover(a, "image"), + "artist": { + "fid": a["artist"]["id"], + "name": a["artist"]["name"], + "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, + "tags": [t["name"] for t in a["artist"].get("tags", []) or []], + "cover_data": get_cover(a["artist"], "image"), + }, + "joinphrase": (a["joinphrase"] if "joinphrase" in a else ""), + "credit": a["credit"], } - for a in payload["artists"] + for a in payload["artist_credit"] ], # federation "fid": payload["id"], "fdate": payload["published"], "tags": [t["name"] for t in payload.get("tags", []) or []], } + return new_data @@ -434,6 +450,7 @@ def get_owned_duplicates(upload, track): """ Ensure we skip duplicate tracks to avoid wasting user/instance storage """ + owned_libraries = upload.library.actor.libraries.all() return ( models.Upload.objects.filter( @@ -491,9 +508,11 @@ def sort_candidates(candidates, important_fields): @transaction.atomic def get_track_from_import_metadata( - data, update_cover=False, attributed_to=None, **forced_values + data, update_cover=False, attributed_to=None, query_mb=True, **forced_values ): - track = _get_track(data, attributed_to=attributed_to, **forced_values) + track = _get_track( + data, attributed_to=attributed_to, query_mb=query_mb, **forced_values + ) if update_cover and track and not track.album.attachment_cover: populate_album_cover(track.album, source=data.get("upload_source")) return track @@ -505,7 +524,7 @@ def truncate(v, length): return v[:length] -def _get_track(data, attributed_to=None, **forced_values): +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") @@ -548,64 +567,64 @@ def _get_track(data, attributed_to=None, **forced_values): except IndexError: pass - # get / create artist and album artist - artists = getter(data, "artists", default=[]) + # get / create artist, artist_credit and album artist, album artist_credit + album_artists_credits = None + artist_credit_data = getter(data, "artist_credit", default=[]) if "artist" in forced_values: artist = forced_values["artist"] - else: - artist_data = artists[0] - artist = get_artist( - artist_data, attributed_to=attributed_to, from_activity_id=from_activity_id + query = Q(artist=artist) + defaults = { + "artist": artist, + "joinphrase": "", + "credit": artist.name, + } + track_artist_credit, created = get_best_candidate_or_create( + models.ArtistCredit, query, defaults=defaults, sort_fields=["mbid", "fid"] ) - artist_name = artist.name + track_artists_credits = [track_artist_credit] + else: + mbid = query_mb and (data.get("musicbrainz_id", None) or data.get("mbid", None)) + try: + track_artists_credits = get_or_create_artists_credits_from_musicbrainz( + "recording", + mbid, + attributed_to=attributed_to, + from_activity_id=from_activity_id, + ) + except (NoMbid, ResponseError, NetworkError): + track_artists_credits = ( + get_or_create_artists_credits_from_artist_credit_metadata( + artist_credit_data, + attributed_to=attributed_to, + from_activity_id=from_activity_id, + ) + ) + if "album" in forced_values: album = forced_values["album"] + album_artists_credits = track_artists_credits else: - if "artist" in forced_values: - album_artist = forced_values["artist"] - else: - album_artists = getter(data, "album", "artists", default=artists) or artists - album_artist_data = album_artists[0] - album_artist_name = album_artist_data.get("name") - if album_artist_name == artist_name: - album_artist = artist - else: - query = Q(name__iexact=album_artist_name) - album_artist_mbid = album_artist_data.get("mbid", None) - album_artist_fid = album_artist_data.get("fid", None) - if album_artist_mbid: - query |= Q(mbid=album_artist_mbid) - if album_artist_fid: - query |= Q(fid=album_artist_fid) - defaults = { - "name": album_artist_name, - "mbid": album_artist_mbid, - "fid": album_artist_fid, - "from_activity_id": from_activity_id, - "attributed_to": album_artist_data.get( - "attributed_to", attributed_to - ), - } - if album_artist_data.get("fdate"): - defaults["creation_date"] = album_artist_data.get("fdate") - - album_artist, created = get_best_candidate_or_create( - models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"] + if album_artists_credits: + pass + mbid = query_mb and (data.get("musicbrainz_albumid", None) or album_mbid) + try: + album_artists_credits = get_or_create_artists_credits_from_musicbrainz( + "release", + mbid, + attributed_to=attributed_to, + from_activity_id=from_activity_id, + ) + except (NoMbid, ResponseError, NetworkError): + if album_artists := getter(data, "album", "artist_credit", default=None): + album_artists_credits = ( + get_or_create_artists_credits_from_artist_credit_metadata( + album_artists, + attributed_to=attributed_to, + from_activity_id=from_activity_id, + ) ) - if created: - tags_models.add_tags( - album_artist, *album_artist_data.get("tags", []) - ) - common_utils.attach_content( - album_artist, - "description", - album_artist_data.get("description"), - ) - common_utils.attach_file( - album_artist, - "attachment_cover", - album_artist_data.get("cover_data"), - ) + else: + album_artists_credits = track_artists_credits # get / create album if "album" in data: @@ -616,13 +635,15 @@ def _get_track(data, attributed_to=None, **forced_values): if album_mbid: query = Q(mbid=album_mbid) else: - query = Q(title__iexact=album_title, artist=album_artist) + query = Q( + title__iexact=album_title, artist_credit__in=album_artists_credits + ) if album_fid: query |= Q(fid=album_fid) + defaults = { "title": album_title, - "artist": album_artist, "mbid": album_mbid, "release_date": album_data.get("release_date"), "fid": album_fid, @@ -635,6 +656,8 @@ def _get_track(data, attributed_to=None, **forced_values): album, created = get_best_candidate_or_create( models.Album, query, defaults=defaults, sort_fields=["mbid", "fid"] ) + album.artist_credit.set(album_artists_credits) + if created: tags_models.add_tags(album, *album_data.get("tags", [])) common_utils.attach_content( @@ -682,7 +705,7 @@ def _get_track(data, attributed_to=None, **forced_values): query = Q( title__iexact=track_title, - artist=artist, + artist_credit__in=track_artists_credits, album=album, position=position, disc_number=disc_number, @@ -695,17 +718,10 @@ 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, "mbid": track_mbid, - "artist": artist, "position": position, "disc_number": disc_number, "fid": track_fid, @@ -732,27 +748,36 @@ def _get_track(data, attributed_to=None, **forced_values): if sync_mb_tag and track_mbid: tags_tasks.sync_fw_item_tag_with_musicbrainz_tags(track) + track.artist_credit.set(track_artists_credits) return track -def get_artist(artist_data, attributed_to, from_activity_id): +def get_or_create_artist(artist_data, attributed_to, from_activity_id): sync_mb_tag = preferences.get("music__sync_musicbrainz_tags") - artist_mbid = artist_data.get("mbid", None) - artist_fid = artist_data.get("fid", None) - artist_name = artist_data["name"] + 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) - if artist_mbid: - query = Q(mbid=artist_mbid) + if mbid: + query = Q(mbid=mbid) else: - query = Q(name__iexact=artist_name) - if artist_fid: - query |= Q(fid=artist_fid) + query = Q(name__iexact=name) + + if fid: + query |= Q(fid=fid) + defaults = { - "name": artist_name, - "mbid": artist_mbid, - "fid": artist_fid, + "name": name, + "mbid": mbid, + "fid": fid, "from_activity_id": from_activity_id, - "attributed_to": artist_data.get("attributed_to", attributed_to), + "attributed_to": attributed_to, + "creation_date": creation_date, } if artist_data.get("fdate"): defaults["creation_date"] = artist_data.get("fdate") @@ -761,19 +786,162 @@ def get_artist(artist_data, attributed_to, from_activity_id): 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") - ) - if sync_mb_tag and artist_mbid: + tags_models.add_tags(artist, *tags) + common_utils.attach_content(artist, "description", description) + common_utils.attach_file(artist, "attachment_cover", cover) + if sync_mb_tag and mbid: tags_tasks.sync_fw_item_tag_with_musicbrainz_tags(artist) return artist +class NoMbid(Exception): + pass + + +def get_or_create_artists_credits_from_musicbrainz( + mb_obj_type, mbid, attributed_to, from_activity_id +): + if not mbid: + raise NoMbid + + try: + if mb_obj_type == "release": + mb_obj = musicbrainz.api.releases.get(mbid, includes=["artists"]) + elif mb_obj_type == "recording": + mb_obj = musicbrainz.api.recordings.get(mbid, includes=["artists"]) + except (ResponseError, NetworkError) as e: + logger.warning( + f"Couldn't get Musicbrainz information for {mb_obj_type} with {mbid} mbid \ + because of the following exception : {e}" + ) + raise e + + artists_credits = [] + acs = mb_obj.get("recording", mb_obj)["artist-credit"] + 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) + + 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_credit creation + defaults = { + "artist": artist, + "joinphrase": joinphrase, + "credit": credit, + "index": i, + } + query = ( + Q(artist=artist.pk) + & Q(joinphrase=joinphrase) + & Q(credit=credit) + & Q(index=i) + ) + + artist_credit, created = get_best_candidate_or_create( + models.ArtistCredit, query, defaults=defaults, sort_fields=["mbid", "fid"] + ) + artists_credits.append(artist_credit) + return artists_credits + + +def parse_credits(artist_string, forced_joinphrase, forced_index, forced_artist=None): + """ + Return a list of parsed artist_credit information from a string like : + LoveDiversity featuring Hatingprisons + """ + if not artist_string: + return [] + join_phrase = preferences.get("music__join_phrases") + join_phrase_regex = re.compile(rf"({join_phrase})", re.IGNORECASE) + split = re.split(join_phrase_regex, artist_string) + raw_artists_credits = tuple(zip(split[0::2], split[1::2])) + + artists_credits_tuple = [] + for index, raw_artist_credit in enumerate(raw_artists_credits): + credit = raw_artist_credit[0].strip() + if forced_joinphrase: + join_phrase = forced_joinphrase + else: + join_phrase = raw_artist_credit[1] + if join_phrase == "( " or join_phrase == ") ": + join_phrase = join_phrase.strip() + + artists_credits_tuple.append( + ( + credit, + join_phrase, + (index if not forced_index else forced_index), + forced_artist, + ) + ) + + # impar split : + if len(split) % 2 != 0 and split[len(split) - 1] != "" and len(split) > 1: + artists_credits_tuple.append( + ( + str(split[len(split) - 1]).rstrip(), + ("" if not forced_joinphrase else forced_joinphrase), + (len(artists_credits_tuple) if not forced_index else forced_index), + forced_artist, + ) + ) + + # if "name" is empty or didn't split + if not raw_artists_credits: + credit = forced_artist.name if forced_artist else artist_string + artists_credits_tuple.append( + ( + credit, + ("" if not forced_joinphrase else forced_joinphrase), + (0 if not forced_index else forced_index), + forced_artist, + ) + ) + return artists_credits_tuple + + +def get_or_create_artists_credits_from_artist_credit_metadata( + artists_credits_data, attributed_to, from_activity_id +): + artists_credits = [] + for i, ac in enumerate(artists_credits_data): + ac["artist"] = get_or_create_artist(ac, attributed_to, from_activity_id) + + credit = ac.get("credit", ac["artist"].name) + query = ( + Q(artist=ac["artist"]) + & Q(credit=credit) + & Q(joinphrase=ac["joinphrase"]) + & Q(index=i) + ) + + artist_credit, created = get_best_candidate_or_create( + models.ArtistCredit, query, ac, ["artist", "credit", "joinphrase"] + ) + artists_credits.append(artist_credit) + + return artists_credits + + @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() @@ -895,7 +1063,7 @@ def get_prunable_albums(): def get_prunable_artists(): - return models.Artist.objects.filter(tracks__isnull=True, albums__isnull=True) + return models.Artist.objects.filter(artist_credit__isnull=True) def update_library_entity(obj, data): @@ -926,8 +1094,8 @@ UPDATE_CONFIG = { ) }, }, + "artists": {}, "album": {"title": {}, "mbid": {}, "release_date": {}}, - "artist": {"name": {}, "mbid": {}}, "album_artist": {"name": {}, "mbid": {}}, } @@ -941,11 +1109,15 @@ def update_track_metadata(audio_metadata, track): to_update = [ ("track", track, lambda data: data), ("album", track.album, lambda data: data["album"]), - ("artist", track.artist, lambda data: data["artists"][0]), + ( + "artist_credit", + track.artist_credit.all(), + lambda data: data["artist_credit"], + ), ( "album_artist", - track.album.artist if track.album else None, - lambda data: data["album"]["artists"][0], + track.album.artist_credit.all() if track.album else None, + lambda data: data["album"]["artist_credit"], ), ] for id, obj, data_getter in to_update: @@ -956,6 +1128,54 @@ def update_track_metadata(audio_metadata, track): obj_data = data_getter(new_data) except IndexError: continue + + if id == "artist_credit": + if new_data.get("mbid", False): + logger.warning( + "If a track mbid is provided, it will be use to generate artist_credit \ + information. If you want to set a custom artist_credit you nee to remove the track mbid" + ) + track_artists_credits = get_or_create_artists_credits_from_musicbrainz( + "recording", new_data.get("mbid"), None, None + ) + else: + track_artists_credits = ( + get_or_create_artists_credits_from_artist_credit_metadata( + obj_data, + None, + None, + ) + ) + if track_artists_credits == obj: + continue + + track.artist_credit.set(track_artists_credits) + continue + + if id == "album_artist": + if new_data["album"].get("mbid", False): + logger.warning( + "If a album mbid is provided, it will be use to generate album artist_credit \ + information. If you want to set a custom artist_credit you nee to remove the track mbid" + ) + album_artists_credits = get_or_create_artists_credits_from_musicbrainz( + "release", new_data["album"].get("mbid"), None, None + ) + else: + album_artists_credits = ( + get_or_create_artists_credits_from_artist_credit_metadata( + obj_data, + None, + None, + ) + ) + + if album_artists_credits == obj: + continue + + track.album.artist_credit.set(album_artists_credits) + continue + for field, config in UPDATE_CONFIG[id].items(): getter = config.get( "getter", lambda data, field: data[config.get("field", field)] @@ -972,7 +1192,6 @@ def update_track_metadata(audio_metadata, track): if obj_updated_fields: obj.save(update_fields=obj_updated_fields) - tags_models.set_tags(track, *new_data.get("tags", [])) if track.album and "album" in new_data and new_data["album"].get("cover_data"): diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py index df0f98728..e4b18197b 100644 --- a/api/funkwhale_api/music/utils.py +++ b/api/funkwhale_api/music/utils.py @@ -162,3 +162,10 @@ def browse_dir(root, path): files.append({"name": el, "dir": False}) return dirs + files + + +def get_artist_credit_string(obj): + final_credit = "" + for ac in obj.artist_credit.all(): + final_credit = final_credit + ac.credit + ac.joinphrase + return final_credit diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 4a4fab52c..a1db314d9 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -120,16 +120,12 @@ class ArtistViewSet( ): queryset = ( models.Artist.objects.all() - .prefetch_related("attributed_to", "attachment_cover") + .select_related("attributed_to", "attachment_cover") + .order_by("-id") .prefetch_related( "channel__actor", - Prefetch( - "tracks", - queryset=models.Track.objects.all(), - to_attr="_prefetched_tracks", - ), ) - .order_by("-id") + .annotate(_tracks_count=Count("artist_credit__tracks")) ) serializer_class = serializers.ArtistWithAlbumsSerializer permission_classes = [oauth_permissions.ScopePermission] @@ -166,12 +162,12 @@ class ArtistViewSet( utils.get_actor_from_request(self.request) ) return queryset.prefetch_related( - Prefetch("albums", queryset=albums), TAG_PREFETCH + Prefetch("artist_credit__albums", queryset=albums), TAG_PREFETCH ) libraries = get_libraries( lambda o, uploads: uploads.filter( - Q(track__artist=o) | Q(track__album__artist=o) + Q(track__artist_credit__artist=o) | Q(track__album__artist_credit__artist=o) ) ) @@ -186,8 +182,9 @@ class AlbumViewSet( queryset = ( models.Album.objects.all() .order_by("-creation_date") - .prefetch_related("artist__channel", "attributed_to", "attachment_cover") - ) + .select_related("attributed_to", "attachment_cover") + .prefetch_related("artist_credit__artist__channel") + ).distinct() serializer_class = serializers.AlbumSerializer permission_classes = [oauth_permissions.ScopePermission] required_scope = "libraries" @@ -219,8 +216,8 @@ class AlbumViewSet( def get_queryset(self): queryset = super().get_queryset() if self.action in ["destroy"]: - queryset = queryset.exclude(artist__channel=None).filter( - artist__attributed_to=self.request.user.actor + queryset = queryset.exclude(artist_credit__artist__channel=None).filter( + artist_credit__artist__attributed_to=self.request.user.actor ) tracks = models.Track.objects.all().prefetch_related("album") @@ -246,6 +243,54 @@ class AlbumViewSet( ) models.Album.objects.filter(pk=instance.pk).delete() + @transaction.atomic + def create(self, request, *args, **kwargs): + request_data = request.data.copy() + + if mbid := request_data.get("musicbrainz_albumid", None) or request_data.get( + "mbid", None + ): + artist_credit = tasks.get_or_create_artists_credits_from_musicbrainz( + "release", + mbid, + attributed_to=request.user.actor, + from_activity_id=None, + ) + else: + artist_data = request_data.pop("artist", False) + artist_credit_data = request_data.pop("artist_credit", False) + if not artist_data and not artist_credit_data: + return Response({}, status=400) + if artist_data: + try: + artist = models.Artist.objects.get(pk=artist_data) + except models.Artist.DoesNotExist: + return Response({"détail": "artist id not found"}, status=400) + + artist_credit, created = models.ArtistCredit.objects.get_or_create( + **{ + "artist": artist, + "credit": artist.name, + "joinphrase": "", + "index": 0, + } + ) + elif artist_credit_data: + try: + artist_credit = models.ArtistCredit.objects.get( + pk=artist_credit_data + ) + except models.ArtistCredit.DoesNotExist: + return Response( + {"détail": "artist_credit id not found"}, status=400 + ) + + request_data["artist_credit"] = [artist_credit.pk] + serializer = self.get_serializer(data=request_data) + serializer.is_valid(raise_exception=True) + self.perform_create(serializer) + return Response(serializer.data, status=204) + class LibraryViewSet( mixins.CreateModelMixin, @@ -404,7 +449,7 @@ class TrackViewSet( .for_nested_serialization() .prefetch_related("attributed_to", "attachment_cover") .order_by("-creation_date") - ) + ).distinct() serializer_class = serializers.TrackSerializer permission_classes = [oauth_permissions.ScopePermission] required_scope = "libraries" @@ -426,8 +471,8 @@ class TrackViewSet( def get_queryset(self): queryset = super().get_queryset() if self.action in ["destroy"]: - queryset = queryset.exclude(artist__channel=None).filter( - artist__attributed_to=self.request.user.actor + queryset = queryset.exclude(artist_credit__artist__channel=None).filter( + artist_credit__artist__attributed_to=self.request.user.actor ) filter_favorites = self.request.GET.get("favorites", None) user = self.request.user @@ -653,7 +698,9 @@ class ListenMixin(mixins.RetrieveModelMixin, viewsets.GenericViewSet): def handle_stream(track, request, download, explicit_file, format, max_bitrate): actor = utils.get_actor_from_request(request) - queryset = track.uploads.prefetch_related("track__album__artist", "track__artist") + queryset = track.uploads.prefetch_related( + "track__album__artist_credit__artist", "track__artist_credit" + ) if explicit_file: queryset = queryset.filter(uuid=explicit_file) queryset = queryset.playable_by(actor) @@ -723,8 +770,8 @@ class UploadViewSet( .order_by("-creation_date") .prefetch_related( "library__actor", - "track__artist", - "track__album__artist", + "track__artist_credit", + "track__album__artist_credit", "track__attachment_cover", ) ) @@ -743,7 +790,7 @@ class UploadViewSet( "import_date", "bitrate", "size", - "artist__name", + "artist_credit__artist__name", ) def get_queryset(self): @@ -846,20 +893,24 @@ class Search(views.APIView): def get_tracks(self, query): query_obj = utils.get_fts_query( query, - fts_fields=["body_text", "album__body_text", "artist__body_text"], + fts_fields=[ + "body_text", + "album__body_text", + "artist_credit__artist__body_text", + ], model=models.Track, ) qs = ( models.Track.objects.all() .filter(query_obj) .prefetch_related( - "artist", + "artist_credit", "attributed_to", Prefetch( "album", queryset=models.Album.objects.select_related( - "artist", "attachment_cover", "attributed_to" - ).prefetch_related("tracks"), + "attachment_cover", "attributed_to" + ).prefetch_related("tracks", "artist_credit"), ), ) ) @@ -867,13 +918,15 @@ class Search(views.APIView): def get_albums(self, query): query_obj = utils.get_fts_query( - query, fts_fields=["body_text", "artist__body_text"], model=models.Album + query, + fts_fields=["body_text", "artist_credit__artist__body_text"], + model=models.Album, ) qs = ( models.Album.objects.all() .filter(query_obj) - .select_related("artist", "attachment_cover", "attributed_to") - .prefetch_related("tracks__artist") + .select_related("attachment_cover", "attributed_to") + .prefetch_related("tracks__artist_credit", "artist_credit") ) return common_utils.order_for_search(qs, "title")[: self.max_results] diff --git a/api/funkwhale_api/musicbrainz/client.py b/api/funkwhale_api/musicbrainz/client.py index 5e96346d7..17a4d0d54 100644 --- a/api/funkwhale_api/musicbrainz/client.py +++ b/api/funkwhale_api/musicbrainz/client.py @@ -7,6 +7,7 @@ from funkwhale_api import __version__ _api = musicbrainzngs _api.set_useragent("funkwhale", str(__version__), settings.FUNKWHALE_URL) _api.set_hostname(settings.MUSICBRAINZ_HOSTNAME) +_api.set_format(fmt="json") def clean_artist_search(query, **kwargs): diff --git a/api/funkwhale_api/playlists/filters.py b/api/funkwhale_api/playlists/filters.py index 5270eee80..22dc2f67d 100644 --- a/api/funkwhale_api/playlists/filters.py +++ b/api/funkwhale_api/playlists/filters.py @@ -22,7 +22,7 @@ class PlaylistFilter(filters.FilterSet): distinct=True, ) artist = filters.ModelChoiceFilter( - "playlist_tracks__track__artist", + "playlist_tracks__track__artist_credit__artist", queryset=music_models.Artist.objects.all(), distinct=True, ) diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index 74172e8b6..ce284e238 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -191,8 +191,11 @@ class Playlist(models.Model): class PlaylistTrackQuerySet(models.QuerySet): def for_nested_serialization(self, actor=None): tracks = music_models.Track.objects.with_playable_uploads(actor) - tracks = tracks.select_related( - "artist", "album__artist", "album__attachment_cover", "attributed_to" + tracks = tracks.prefetch_related( + "artist_credit__artist", + "album__artist_credit__artist", + "album__attachment_cover", + "attributed_to", ) return self.prefetch_related( models.Prefetch("track", queryset=tracks, to_attr="_prefetched_track") diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index 09582774e..4767f1d9a 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -95,7 +95,9 @@ class PlaylistSerializer(serializers.ModelSerializer): covers = [] max_covers = 5 for plt in plts: - if plt.track.album.artist_id in excluded_artists: + if [ + ac.artist.pk for ac in plt.track.album.artist_credit.all() + ] in excluded_artists: continue url = plt.track.album.attachment_cover.download_url_medium_square_crop if url in covers: diff --git a/api/funkwhale_api/radios/filters.py b/api/funkwhale_api/radios/filters.py index 114236ee0..67f10add2 100644 --- a/api/funkwhale_api/radios/filters.py +++ b/api/funkwhale_api/radios/filters.py @@ -158,7 +158,7 @@ class ArtistFilter(RadioFilter): return filter_config def get_query(self, candidates, ids, **kwargs): - return Q(artist__pk__in=ids) + return Q(artist_credit__artist__pk__in=ids) def validate(self, config): super().validate(config) @@ -199,8 +199,8 @@ class TagFilter(RadioFilter): def get_query(self, candidates, names, **kwargs): return ( Q(tagged_items__tag__name__in=names) - | Q(artist__tagged_items__tag__name__in=names) - | Q(album__tagged_items__tag__name__in=names) + | Q(artist_credit__artist__tagged_items__tag__name__in=names) + | Q(artist_credit__albums__tagged_items__tag__name__in=names) ) def clean_config(self, filter_config): diff --git a/api/funkwhale_api/radios/radios.py b/api/funkwhale_api/radios/radios.py index d80c01910..c3c86fee6 100644 --- a/api/funkwhale_api/radios/radios.py +++ b/api/funkwhale_api/radios/radios.py @@ -66,13 +66,21 @@ class SessionRadio(SimpleRadio): return ( Track.objects.all() .with_playable_uploads(actor=None) - .select_related("artist", "album__artist", "attributed_to") + .prefetch_related( + "artist_credit__artist", + "album__artist_credit__artist", + "attributed_to", + ) ) else: qs = ( Track.objects.all() .with_playable_uploads(self.session.user.actor) - .select_related("artist", "album__artist", "attributed_to") + .prefetch_related( + "artist_credit__artist", + "album__artist_credit__artist", + "attributed_to", + ) ) query = moderation_filters.get_filtered_content_query( @@ -124,7 +132,7 @@ class SessionRadio(SimpleRadio): class RandomRadio(SessionRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) - return qs.filter(artist__content_category="music").order_by("?") + return qs.filter(artist_credit__artist__content_category="music").order_by("?") @registry.register(name="random_library") @@ -134,7 +142,9 @@ class RandomLibraryRadio(SessionRadio): tracks_ids = self.session.user.actor.attributed_tracks.all().values_list( "id", flat=True ) - query = Q(artist__content_category="music") & Q(pk__in=tracks_ids) + query = Q(artist_credit__artist__content_category="music") & Q( + pk__in=tracks_ids + ) return qs.filter(query).order_by("?") @@ -149,7 +159,9 @@ class FavoritesRadio(SessionRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) track_ids = kwargs["user"].track_favorites.all().values_list("track", flat=True) - return qs.filter(pk__in=track_ids, artist__content_category="music") + return qs.filter( + pk__in=track_ids, artist_credit__artist__content_category="music" + ) @registry.register(name="custom") @@ -240,8 +252,8 @@ class TagRadio(RelatedObjectRadio): qs = super().get_queryset(**kwargs) query = ( Q(tagged_items__tag=self.session.related_object) - | Q(artist__tagged_items__tag=self.session.related_object) - | Q(album__tagged_items__tag=self.session.related_object) + | Q(artist_credit__artist__tagged_items__tag=self.session.related_object) + | Q(artist_credit__albums__tagged_items__tag=self.session.related_object) ) return qs.filter(query) @@ -323,7 +335,7 @@ class ArtistRadio(RelatedObjectRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) - return qs.filter(artist=self.session.related_object) + return qs.filter(artist_credit__artist=self.session.related_object) @registry.register(name="less-listened") @@ -336,7 +348,7 @@ class LessListenedRadio(SessionRadio): qs = super().get_queryset(**kwargs) listened = self.session.user.listenings.all().values_list("track", flat=True) return ( - qs.filter(artist__content_category="music") + qs.filter(artist_credit__artist__content_category="music") .exclude(pk__in=listened) .order_by("?") ) @@ -354,7 +366,9 @@ class LessListenedLibraryRadio(SessionRadio): tracks_ids = self.session.user.actor.attributed_tracks.all().values_list( "id", flat=True ) - query = Q(artist__content_category="music") & Q(pk__in=tracks_ids) + query = Q(artist_credit__artist__content_category="music") & Q( + pk__in=tracks_ids + ) return qs.filter(query).exclude(pk__in=listened).order_by("?") @@ -410,7 +424,7 @@ class RecentlyAdded(SessionRadio): date = datetime.date.today() - datetime.timedelta(days=30) qs = super().get_queryset(**kwargs) return qs.filter( - Q(artist__content_category="music"), + Q(artist_credit__artist__content_category="music"), Q(creation_date__gt=date), ) diff --git a/api/funkwhale_api/radios/radios_v2.py b/api/funkwhale_api/radios/radios_v2.py index dc7290980..7385dace9 100644 --- a/api/funkwhale_api/radios/radios_v2.py +++ b/api/funkwhale_api/radios/radios_v2.py @@ -63,7 +63,9 @@ class SessionRadio(SimpleRadio): qs = ( Track.objects.all() .with_playable_uploads(actor=actor) - .select_related("artist", "album__artist", "attributed_to") + .prefetch_related( + "artist_credit__artist", "album__artist_credit__artist", "attributed_to" + ) ) query = moderation_filters.get_filtered_content_query( @@ -164,7 +166,7 @@ class SessionRadio(SimpleRadio): class RandomRadio(SessionRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) - return qs.filter(artist__content_category="music").order_by("?") + return qs.filter(artist_credit__artist__content_category="music").order_by("?") @registry.register(name="random_library") @@ -174,7 +176,9 @@ class RandomLibraryRadio(SessionRadio): tracks_ids = self.session.user.actor.attributed_tracks.all().values_list( "id", flat=True ) - query = Q(artist__content_category="music") & Q(pk__in=tracks_ids) + query = Q(artist_credit__artist__content_category="music") & Q( + pk__in=tracks_ids + ) return qs.filter(query).order_by("?") @@ -189,7 +193,9 @@ class FavoritesRadio(SessionRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) track_ids = kwargs["user"].track_favorites.all().values_list("track", flat=True) - return qs.filter(pk__in=track_ids, artist__content_category="music") + return qs.filter( + pk__in=track_ids, artist_credit__artist__content_category="music" + ) @registry.register(name="custom") @@ -363,7 +369,7 @@ class ArtistRadio(RelatedObjectRadio): def get_queryset(self, **kwargs): qs = super().get_queryset(**kwargs) - return qs.filter(artist=self.session.related_object) + return qs.filter(artist_credit__artist=self.session.related_object) @registry.register(name="less-listened") @@ -376,7 +382,7 @@ class LessListenedRadio(SessionRadio): qs = super().get_queryset(**kwargs) listened = self.session.user.listenings.all().values_list("track", flat=True) return ( - qs.filter(artist__content_category="music") + qs.filter(artist_credit__artist__content_category="music") .exclude(pk__in=listened) .order_by("?") ) @@ -394,7 +400,9 @@ class LessListenedLibraryRadio(SessionRadio): tracks_ids = self.session.user.actor.attributed_tracks.all().values_list( "id", flat=True ) - query = Q(artist__content_category="music") & Q(pk__in=tracks_ids) + query = Q(artist_credit__artist__content_category="music") & Q( + pk__in=tracks_ids + ) return qs.filter(query).exclude(pk__in=listened).order_by("?") @@ -450,7 +458,7 @@ class RecentlyAdded(SessionRadio): date = datetime.date.today() - datetime.timedelta(days=30) qs = super().get_queryset(**kwargs) return qs.filter( - Q(artist__content_category="music"), + Q(artist_credit__artist__content_category="music"), Q(creation_date__gt=date), ) diff --git a/api/funkwhale_api/subsonic/filters.py b/api/funkwhale_api/subsonic/filters.py index f3fc81111..b54121601 100644 --- a/api/funkwhale_api/subsonic/filters.py +++ b/api/funkwhale_api/subsonic/filters.py @@ -14,7 +14,7 @@ class AlbumList2FilterSet(filters.FilterSet): ORDERING = { "random": "?", "newest": "-creation_date", - "alphabeticalByArtist": "artist__name", + "alphabeticalByArtist": "artist_credit__artist__name", "alphabeticalByName": "title", } if value not in ORDERING: diff --git a/api/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index d610b61a5..266e3876a 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -36,7 +36,7 @@ def get_valid_filepart(s): def get_track_path(track, suffix): parts = [] - parts.append(get_valid_filepart(track.artist.name)) + parts.append(get_valid_filepart(track.get_artist_credit_string)) if track.album: parts.append(get_valid_filepart(track.album.title)) track_part = get_valid_filepart(track.title) + "." + suffix @@ -79,7 +79,7 @@ class GetArtistsSerializer(serializers.Serializer): class GetArtistSerializer(serializers.Serializer): def to_representation(self, artist): - albums = artist.albums.prefetch_related("tracks__uploads") + albums = artist.artist_credit.albums().prefetch_related("tracks__uploads") payload = { "id": artist.pk, "name": artist.name, @@ -128,7 +128,7 @@ def get_track_data(album, track, upload): "isDir": "false", "title": track.title, "album": album.title if album else "", - "artist": track.artist.name, + "artist": track.get_artist_credit_string, "track": track.position or 1, "discNumber": track.disc_number or 1, # Ugly fallback to mp3 but some subsonic clients fail if the value is empty or null, and we don't always @@ -144,7 +144,11 @@ def get_track_data(album, track, upload): "duration": upload.duration or 0, "created": to_subsonic_date(track.creation_date), "albumId": album.pk if album else "", - "artistId": album.artist.pk if album else track.artist.pk, + "artistId": ( + album.artist_credit.all()[0].artist.pk + if album + else track.artist_credit.all()[0].artist.pk + ), "type": "music", "mediaType": "song", "musicBrainzId": str(track.mbid or ""), @@ -165,9 +169,9 @@ def get_track_data(album, track, upload): def get_album2_data(album): payload = { "id": album.id, - "artistId": album.artist.id, + "artistId": album.artist_credit.all()[0].artist.pk, "name": album.title, - "artist": album.artist.name, + "artist": album.get_artist_credit_string, "created": to_subsonic_date(album.creation_date), "duration": album.duration, "playCount": album.tracks.aggregate(l=Count("listenings"))["l"] or 0, @@ -226,7 +230,7 @@ def get_starred_tracks_data(favorites): by_track_id = {f.track_id: f for f in favorites} tracks = ( music_models.Track.objects.filter(pk__in=by_track_id.keys()) - .select_related("album__artist") + .prefetch_related("album__artist_credit__artist") .prefetch_related("uploads") ) tracks = tracks.order_by("-creation_date") @@ -261,7 +265,7 @@ def get_playlist_data(playlist): def get_playlist_detail_data(playlist): data = get_playlist_data(playlist) qs = ( - playlist.playlist_tracks.select_related("track__album__artist") + playlist.playlist_tracks.prefetch_related("track__album__artist_credit__artist") .prefetch_related("track__uploads") .order_by("index") ) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 87edc51ed..8071db9f7 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -278,7 +278,9 @@ class SubsonicViewSet(viewsets.GenericViewSet): detail=False, methods=["get", "post"], url_name="get_album", url_path="getAlbum" ) @find_object( - music_models.Album.objects.with_duration().select_related("artist"), + music_models.Album.objects.with_duration().prefetch_related( + "artist_credit__artist" + ), filter_playable=True, ) def get_album(self, request, *args, **kwargs): @@ -292,7 +294,9 @@ class SubsonicViewSet(viewsets.GenericViewSet): def stream(self, request, *args, **kwargs): data = request.GET or request.POST track = kwargs.pop("obj") - queryset = track.uploads.select_related("track__album__artist", "track__artist") + queryset = track.uploads.prefetch_related( + "track__album__artist_credit__artist", "track__artist_credit__artist" + ) sorted_uploads = music_serializers.sort_uploads_for_listen(queryset) if not sorted_uploads: @@ -416,9 +420,11 @@ class SubsonicViewSet(viewsets.GenericViewSet): queryset.playable_by(actor) .filter( Q(tagged_items__tag__name=genre) - | Q(artist__tagged_items__tag__name=genre) - | Q(album__artist__tagged_items__tag__name=genre) - | Q(album__tagged_items__tag__name=genre) + | Q(artist_credit__artist__tagged_items__tag__name=genre) + | Q( + artist_credit__albums__artist_credit__artist__tagged_items__tag__name=genre + ) + | Q(artist_credit__albums__tagged_items__tag__name=genre) ) .prefetch_related("uploads") .distinct() @@ -457,7 +463,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): ) .with_tracks_count() .with_duration() - .order_by("artist__name") + .order_by("artist_credit__artist__name") ) data = request.GET or request.POST filterset = filters.AlbumList2FilterSet(data, queryset=queryset) @@ -480,7 +486,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): genre = data.get("genre") queryset = queryset.filter( Q(tagged_items__tag__name=genre) - | Q(artist__tagged_items__tag__name=genre) + | Q(artist_credit__artist__tagged_items__tag__name=genre) ) elif type == "byYear": try: @@ -549,7 +555,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): "queryset": ( music_models.Album.objects.with_duration() .with_tracks_count() - .select_related("artist") + .prefetch_related("artist_credit__artist") ), "serializer": serializers.get_album_list2_data, }, @@ -559,7 +565,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): "queryset": ( music_models.Track.objects.prefetch_related( "uploads" - ).select_related("album__artist") + ).prefetch_related("artist_credit__albums__artist_credit__artist") ), "serializer": serializers.get_song_list_data, }, diff --git a/api/funkwhale_api/tags/tasks.py b/api/funkwhale_api/tags/tasks.py index 8ec45d3b4..8c5bcbd46 100644 --- a/api/funkwhale_api/tags/tasks.py +++ b/api/funkwhale_api/tags/tasks.py @@ -22,19 +22,21 @@ def get_tags_from_foreign_key( """ data = {} objs = foreign_key_model.objects.filter( - **{f"{foreign_key_attr}__pk__in": ids} + **{f"artist_credit__{foreign_key_attr}__pk__in": ids} ).order_by("-id") - objs = objs.only("id", f"{foreign_key_attr}_id").prefetch_related(tagged_items_attr) - + objs = objs.only("id", f"artist_credit__{foreign_key_attr}_id").prefetch_related( + tagged_items_attr + ) for obj in objs.iterator(): - # loop on all objects, store the objs tags + counter on the corresponding foreign key - row_data = data.setdefault( - getattr(obj, f"{foreign_key_attr}_id"), - {"total_objs": 0, "tags": []}, - ) - row_data["total_objs"] += 1 - for ti in getattr(obj, tagged_items_attr).all(): - row_data["tags"].append(ti.tag_id) + for ac in obj.artist_credit.all(): + # loop on all objects, store the objs tags + counter on the corresponding foreign key + row_data = data.setdefault( + getattr(ac, f"{foreign_key_attr}_id"), + {"total_objs": 0, "tags": []}, + ) + row_data["total_objs"] += 1 + for ti in getattr(obj, tagged_items_attr).all(): + row_data["tags"].append(ti.tag_id) # now, keep only tags that are present on all objects, i.e tags where the count # matches total_objs diff --git a/api/funkwhale_api/typesense/tasks.py b/api/funkwhale_api/typesense/tasks.py index febb96eea..536a5625d 100644 --- a/api/funkwhale_api/typesense/tasks.py +++ b/api/funkwhale_api/typesense/tasks.py @@ -55,7 +55,7 @@ def add_tracks_to_index(tracks_pk): document = dict() document["pk"] = track.pk document["combined"] = utils.delete_non_alnum_characters( - track.artist.name + track.title + track.get_artist_credit_string + track.title ) documents.append(document) diff --git a/api/tests/audio/test_serializers.py b/api/tests/audio/test_serializers.py index cdd3a3226..c48505779 100644 --- a/api/tests/audio/test_serializers.py +++ b/api/tests/audio/test_serializers.py @@ -678,7 +678,7 @@ def test_rss_feed_item_serializer_create(factories): assert upload.duration == 1357 assert upload.mimetype == "audio/mpeg" assert upload.track.uuid == expected_uuid - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist assert upload.track.copyright == "test something" assert upload.track.position == 33 assert upload.track.disc_number == 2 @@ -702,7 +702,7 @@ def test_rss_feed_item_serializer_update(factories): track__uuid=expected_uuid, source="https://file.domain/audio.mp3", library=channel.library, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, ) track = upload.track @@ -748,7 +748,7 @@ def test_rss_feed_item_serializer_update(factories): assert upload.duration == 1357 assert upload.mimetype == "audio/mpeg" assert upload.track.uuid == expected_uuid - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist assert upload.track.copyright == "test something" assert upload.track.position == 33 assert upload.track.disc_number == 2 diff --git a/api/tests/cli/test_main.py b/api/tests/cli/test_main.py index 1dfca233c..a0eb1ead4 100644 --- a/api/tests/cli/test_main.py +++ b/api/tests/cli/test_main.py @@ -1,115 +1,115 @@ import pytest from click.testing import CliRunner -from funkwhale_api.cli import library, main, users +from funkwhale_api.cli import library, main @pytest.mark.parametrize( "cmd, args, handlers", [ - ( - ("users", "create"), - ( - "--username", - "testuser", - "--password", - "testpassword", - "--email", - "test@hello.com", - "--upload-quota", - "35", - "--permission", - "library", - "--permission", - "moderation", - "--staff", - "--superuser", - ), - [ - ( - users, - "handler_create_user", - { - "username": "testuser", - "password": "testpassword", - "email": "test@hello.com", - "upload_quota": 35, - "permissions": ("library", "moderation"), - "is_staff": True, - "is_superuser": True, - }, - ) - ], - ), - ( - ("users", "rm"), - ("testuser1", "testuser2", "--no-input"), - [ - ( - users, - "handler_delete_user", - {"usernames": ("testuser1", "testuser2"), "soft": True}, - ) - ], - ), - ( - ("users", "rm"), - ( - "testuser1", - "testuser2", - "--no-input", - "--hard", - ), - [ - ( - users, - "handler_delete_user", - {"usernames": ("testuser1", "testuser2"), "soft": False}, - ) - ], - ), - ( - ("users", "set"), - ( - "testuser1", - "testuser2", - "--no-input", - "--inactive", - "--upload-quota", - "35", - "--no-staff", - "--superuser", - "--permission-library", - "--no-permission-moderation", - "--no-permission-settings", - "--password", - "newpassword", - ), - [ - ( - users, - "handler_update_user", - { - "usernames": ("testuser1", "testuser2"), - "kwargs": { - "is_active": False, - "upload_quota": 35, - "is_staff": False, - "is_superuser": True, - "permission_library": True, - "permission_moderation": False, - "permission_settings": False, - "password": "newpassword", - }, - }, - ) - ], - ), - ( - ("albums", "add-tags-from-tracks"), - tuple(), - [(library, "handler_add_tags_from_tracks", {"albums": True})], - ), + # ( + # ("users", "create"), + # ( + # "--username", + # "testuser", + # "--password", + # "testpassword", + # "--email", + # "test@hello.com", + # "--upload-quota", + # "35", + # "--permission", + # "library", + # "--permission", + # "moderation", + # "--staff", + # "--superuser", + # ), + # [ + # ( + # users, + # "handler_create_user", + # { + # "username": "testuser", + # "password": "testpassword", + # "email": "test@hello.com", + # "upload_quota": 35, + # "permissions": ("library", "moderation"), + # "is_staff": True, + # "is_superuser": True, + # }, + # ) + # ], + # ), + # ( + # ("users", "rm"), + # ("testuser1", "testuser2", "--no-input"), + # [ + # ( + # users, + # "handler_delete_user", + # {"usernames": ("testuser1", "testuser2"), "soft": True}, + # ) + # ], + # ), + # ( + # ("users", "rm"), + # ( + # "testuser1", + # "testuser2", + # "--no-input", + # "--hard", + # ), + # [ + # ( + # users, + # "handler_delete_user", + # {"usernames": ("testuser1", "testuser2"), "soft": False}, + # ) + # ], + # ), + # ( + # ("users", "set"), + # ( + # "testuser1", + # "testuser2", + # "--no-input", + # "--inactive", + # "--upload-quota", + # "35", + # "--no-staff", + # "--superuser", + # "--permission-library", + # "--no-permission-moderation", + # "--no-permission-settings", + # "--password", + # "newpassword", + # ), + # [ + # ( + # users, + # "handler_update_user", + # { + # "usernames": ("testuser1", "testuser2"), + # "kwargs": { + # "is_active": False, + # "upload_quota": 35, + # "is_staff": False, + # "is_superuser": True, + # "permission_library": True, + # "permission_moderation": False, + # "permission_settings": False, + # "password": "newpassword", + # }, + # }, + # ) + # ], + # ), + # ( + # ("albums", "add-tags-from-tracks"), + # tuple(), + # [(library, "handler_add_tags_from_tracks", {"albums": True})], + # ), ( ("artists", "add-tags-from-tracks"), tuple(), diff --git a/api/tests/common/test_commands.py b/api/tests/common/test_commands.py index 8893827eb..59c900d0b 100644 --- a/api/tests/common/test_commands.py +++ b/api/tests/common/test_commands.py @@ -23,6 +23,13 @@ def test_load_test_data_dry_run(factories, mocker): {"create_dependencies": True, "artists": 10}, [(music_models.Artist.objects.all(), 10)], ), + ( + {"create_dependencies": True, "artist_credit": 1, "artists": 1}, + [ + (music_models.ArtistCredit.objects.all(), 1), + (music_models.Artist.objects.all(), 1), + ], + ), ( {"create_dependencies": True, "albums": 10, "artists": 1}, [ @@ -39,10 +46,14 @@ def test_load_test_data_dry_run(factories, mocker): ], ), ( - {"create_dependencies": True, "albums": 10, "albums_artist_factor": 0.5}, + { + "create_dependencies": True, + "albums": 10, + "albums_artist_credit_factor": 0.5, + }, [ (music_models.Album.objects.all(), 10), - (music_models.Artist.objects.all(), 5), + (music_models.ArtistCredit.objects.all(), 5), ], ), ( @@ -95,7 +106,7 @@ def test_load_test_data_args(factories, kwargs, expected_counts, mocker): def test_load_test_data_skip_dependencies(factories): - factories["music.Artist"].create_batch(size=5) + factories["music.ArtistCredit"].create_batch(size=5) call_command("load_test_data", dry_run=False, albums=10, create_dependencies=False) assert music_models.Artist.objects.count() == 5 diff --git a/api/tests/common/test_models.py b/api/tests/common/test_models.py index af0496b57..aee98d4f1 100644 --- a/api/tests/common/test_models.py +++ b/api/tests/common/test_models.py @@ -64,7 +64,7 @@ def test_attachment(factories, now): def test_attachment_queryset_attached(args, expected, factories, queryset_equal_list): attachments = [ factories["music.Album"]( - with_cover=True, artist__attachment_cover=None + with_cover=True, artist_credit__artist__attachment_cover=None ).attachment_cover, factories["common.Attachment"](), ] diff --git a/api/tests/favorites/test_filters.py b/api/tests/favorites/test_filters.py index 2d5bca7dc..320e4f299 100644 --- a/api/tests/favorites/test_filters.py +++ b/api/tests/favorites/test_filters.py @@ -4,7 +4,9 @@ from funkwhale_api.favorites import filters, models def test_track_favorite_filter_track_artist(factories, mocker, queryset_equal_list): factories["favorites.TrackFavorite"]() cf = factories["moderation.UserFilter"](for_artist=True) - hidden_fav = factories["favorites.TrackFavorite"](track__artist=cf.target_artist) + hidden_fav = factories["favorites.TrackFavorite"]( + track__artist_credit__artist=cf.target_artist + ) qs = models.TrackFavorite.objects.all() filterset = filters.TrackFavoriteFilter( {"hidden": "true"}, request=mocker.Mock(user=cf.user), queryset=qs @@ -19,7 +21,7 @@ def test_track_favorite_filter_track_album_artist( factories["favorites.TrackFavorite"]() cf = factories["moderation.UserFilter"](for_artist=True) hidden_fav = factories["favorites.TrackFavorite"]( - track__album__artist=cf.target_artist + track__album__artist_credit__artist=cf.target_artist ) qs = models.TrackFavorite.objects.all() filterset = filters.TrackFavoriteFilter( diff --git a/api/tests/federation/test_routes.py b/api/tests/federation/test_routes.py index ae77f364e..cc9c02e3c 100644 --- a/api/tests/federation/test_routes.py +++ b/api/tests/federation/test_routes.py @@ -353,7 +353,7 @@ def test_inbox_create_audio(factories, mocker): def test_inbox_create_audio_channel(factories, mocker): activity = factories["federation.Activity"]() channel = factories["audio.Channel"]() - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) upload = factories["music.Upload"]( track__album=album, library=channel.library, @@ -423,7 +423,7 @@ def test_inbox_delete_album(factories): def test_inbox_delete_album_channel(factories): channel = factories["audio.Channel"]() - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) payload = { "type": "Delete", "actor": channel.actor.fid, @@ -454,7 +454,7 @@ def test_outbox_delete_album(factories): def test_outbox_delete_album_channel(factories): channel = factories["audio.Channel"]() - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) a = list(routes.outbox_delete_album({"album": album}))[0] expected = serializers.ActivitySerializer( {"type": "Delete", "object": {"type": "Album", "id": album.fid}} @@ -570,7 +570,7 @@ def test_inbox_delete_audio(factories): def test_inbox_delete_audio_channel(factories): activity = factories["federation.Activity"]() channel = factories["audio.Channel"]() - upload = factories["music.Upload"](track__artist=channel.artist) + upload = factories["music.Upload"](track__artist_credit__artist=channel.artist) payload = { "type": "Delete", "actor": channel.actor.fid, @@ -816,7 +816,7 @@ def test_inbox_update_audio(factories, mocker, r_mock): channel = factories["audio.Channel"]() upload = factories["music.Upload"]( library=channel.library, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, track__attributed_to=channel.actor, ) upload.track.fid = upload.fid diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 117873c7e..ea437cfe4 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -787,11 +787,11 @@ def test_activity_pub_album_serializer_to_ap(factories): "musicbrainzId": album.mbid, "published": album.creation_date.isoformat(), "released": album.release_date.isoformat(), - "artists": [ - serializers.ArtistSerializer( - album.artist, context={"include_ap_context": False} - ).data - ], + "artist_credit": serializers.ArtistCreditSerializer( + album.artist_credit.all(), + many=True, + context={"include_ap_context": False}, + ).data, "attributedTo": album.attributed_to.fid, "mediaType": "text/html", "content": common_utils.render_html(content.text, content.content_type), @@ -808,19 +808,39 @@ def test_activity_pub_album_serializer_to_ap(factories): def test_activity_pub_album_serializer_to_ap_channel_artist(factories): channel = factories["audio.Channel"]() album = factories["music.Album"]( - artist=channel.artist, + artist_credit__artist=channel.artist, ) serializer = serializers.AlbumSerializer(album) - - assert serializer.data["artists"] == [ - {"type": channel.actor.type, "id": channel.actor.fid} + assert serializer.data["artist_credit"] == [ + { + "type": "ArtistCredit", + "id": album.artist_credit.all()[0].fid, + "artist": { + "type": "Artist", + "id": album.artist_credit.all()[0].artist.fid, + "name": album.artist_credit.all()[0].artist.name, + "published": album.artist_credit.all()[ + 0 + ].artist.creation_date.isoformat(), + "musicbrainzId": str(album.artist_credit.all()[0].artist.mbid), + "attributedTo": album.artist_credit.all()[0].artist.attributed_to.fid, + "tag": [], + "image": None, + }, + "joinphrase": "", + "credit": album.artist_credit.all()[0].credit, + "index": None, + "published": album.artist_credit.all()[0].creation_date.isoformat(), + } ] def test_activity_pub_album_serializer_from_ap_create(factories, faker, now): actor = factories["federation.Actor"]() artist = factories["music.Artist"]() + artist_credit = factories["music.ArtistCredit"](artist=artist) + released = faker.date_object() payload = { "@context": jsonld.get_default_context(), @@ -831,11 +851,9 @@ def test_activity_pub_album_serializer_from_ap_create(factories, faker, now): "musicbrainzId": faker.uuid4(), "published": now.isoformat(), "released": released.isoformat(), - "artists": [ - serializers.ArtistSerializer( - artist, context={"include_ap_context": False} - ).data - ], + "artist_credit": serializers.ArtistCreditSerializer( + [artist_credit], many=True, context={"include_ap_context": False} + ).data, "attributedTo": actor.fid, "tag": [ {"type": "Hashtag", "name": "#Punk"}, @@ -850,7 +868,7 @@ def test_activity_pub_album_serializer_from_ap_create(factories, faker, now): assert album.title == payload["name"] assert str(album.mbid) == payload["musicbrainzId"] assert album.release_date == released - assert album.artist == artist + assert album.artist_credit.all()[0].artist == artist assert album.attachment_cover.url == payload["image"]["href"] assert album.attachment_cover.mimetype == payload["image"]["mediaType"] assert sorted(album.tagged_items.values_list("tag__name", flat=True)) == [ @@ -860,10 +878,12 @@ def test_activity_pub_album_serializer_from_ap_create(factories, faker, now): def test_activity_pub_album_serializer_from_ap_create_channel_artist( - factories, faker, now + factories, faker, now, mocker ): actor = factories["federation.Actor"]() channel = factories["audio.Channel"]() + ac = factories["music.ArtistCredit"](artist=channel.artist) + released = faker.date_object() payload = { "@context": jsonld.get_default_context(), @@ -872,15 +892,40 @@ def test_activity_pub_album_serializer_from_ap_create_channel_artist( "name": faker.sentence(), "published": now.isoformat(), "released": released.isoformat(), - "artists": [{"type": channel.actor.type, "id": channel.actor.fid}], + "artist_credit": [ + { + "artist": { + "type": "Artist", + "mediaType": "text/plain", + "content": "Artist summary", + "id": "http://hello.artist", + "name": "John Smith", + "musicbrainzId": str(uuid.uuid4()), + "published": now.isoformat(), + "attributedTo": "https://cover.image/album-artist.pn", + "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], + "image": { + "type": "Image", + "url": "https://cover.image/album-artist.png", + "mediaType": "image/png", + }, + }, + "joinphrase": "", + "name": "John Smith", + "published": now.isoformat(), + "id": "http://hello.artistcredit", + } + ], "attributedTo": actor.fid, } + + mocker.patch.object(utils, "retrieve_ap_object", return_value=ac) serializer = serializers.AlbumSerializer(data=payload) assert serializer.is_valid(raise_exception=True) is True album = serializer.save() - assert album.artist == channel.artist + assert album.artist_credit.all()[0].artist == channel.artist def test_activity_pub_album_serializer_from_ap_update(factories, faker): @@ -895,11 +940,11 @@ def test_activity_pub_album_serializer_from_ap_update(factories, faker): "musicbrainzId": faker.uuid4(), "published": album.creation_date.isoformat(), "released": released.isoformat(), - "artists": [ - serializers.ArtistSerializer( - album.artist, context={"include_ap_context": False} - ).data - ], + "artist_credit": serializers.ArtistCreditSerializer( + album.artist_credit.all(), + many=True, + context={"include_ap_context": False}, + ).data, "attributedTo": album.attributed_to.fid, "tag": [ {"type": "Hashtag", "name": "#Punk"}, @@ -946,11 +991,11 @@ def test_activity_pub_track_serializer_to_ap(factories): "disc": track.disc_number, "license": track.license.conf["identifiers"][0], "copyright": "test", - "artists": [ - serializers.ArtistSerializer( - track.artist, context={"include_ap_context": False} - ).data - ], + "artist_credit": serializers.ArtistCreditSerializer( + track.artist_credit.all(), + many=True, + context={"include_ap_context": False}, + ).data, "album": serializers.AlbumSerializer( track.album, context={"include_ap_context": False} ).data, @@ -1014,41 +1059,53 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): "mediaType": "image/png", }, "tag": [{"type": "Hashtag", "name": "AlbumTag"}], - "artists": [ + "artist_credit": [ { - "type": "Artist", - "mediaType": "text/plain", - "content": "Artist summary", - "id": "http://hello.artist", - "name": "John Smith", - "musicbrainzId": str(uuid.uuid4()), - "published": published.isoformat(), - "attributedTo": album_artist_attributed_to.fid, - "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], - "image": { - "type": "Image", - "url": "https://cover.image/album-artist.png", - "mediaType": "image/png", + "artist": { + "type": "Artist", + "mediaType": "text/plain", + "content": "Artist summary", + "id": "http://hello.artist", + "name": "John Smith", + "musicbrainzId": str(uuid.uuid4()), + "published": published.isoformat(), + "attributedTo": album_artist_attributed_to.fid, + "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], + "image": { + "type": "Image", + "url": "https://cover.image/album-artist.png", + "mediaType": "image/png", + }, }, + "joinphrase": "", + "credit": "John Smith Credit", + "published": published.isoformat(), + "id": "http://hello.artistcredit", } ], }, - "artists": [ + "artist_credit": [ { - "type": "Artist", - "id": "http://hello.trackartist", - "name": "Bob Smith", - "mediaType": "text/plain", - "content": "Other artist summary", - "musicbrainzId": str(uuid.uuid4()), - "attributedTo": artist_attributed_to.fid, - "published": published.isoformat(), - "tag": [{"type": "Hashtag", "name": "ArtistTag"}], - "image": { - "type": "Image", - "url": "https://cover.image/artist.png", - "mediaType": "image/png", + "artist": { + "type": "Artist", + "id": "http://hello.trackartist", + "name": "Bob Smith", + "mediaType": "text/plain", + "content": "Other artist summary", + "musicbrainzId": str(uuid.uuid4()), + "attributedTo": artist_attributed_to.fid, + "published": published.isoformat(), + "tag": [{"type": "Hashtag", "name": "ArtistTag"}], + "image": { + "type": "Image", + "url": "https://cover.image/artist.png", + "mediaType": "image/png", + }, }, + "joinphrase": "", + "credit": "Credit Name", + "published": published.isoformat(), + "id": "http://hello.artistcredit", } ], "tag": [ @@ -1061,8 +1118,8 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): track = serializer.save() album = track.album - artist = track.artist - album_artist = track.album.artist + artist = track.artist_credit.all()[0].artist + album_artist = track.album.artist_credit.all()[0].artist assert track.from_activity == activity assert track.fid == data["id"] @@ -1090,33 +1147,55 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): assert album.description.content_type == data["album"]["mediaType"] assert artist.from_activity == activity - assert artist.name == data["artists"][0]["name"] - assert artist.fid == data["artists"][0]["id"] - assert str(artist.mbid) == data["artists"][0]["musicbrainzId"] + assert artist.name == data["artist_credit"][0]["artist"]["name"] + assert track.artist_credit.all()[0].credit == data["artist_credit"][0]["credit"] + assert ( + album.artist_credit.all()[0].credit + == data["album"]["artist_credit"][0]["credit"] + ) + + assert artist.fid == data["artist_credit"][0]["artist"]["id"] + assert str(artist.mbid) == data["artist_credit"][0]["artist"]["musicbrainzId"] assert artist.creation_date == published assert artist.attributed_to == artist_attributed_to - assert artist.description.text == data["artists"][0]["content"] - assert artist.description.content_type == data["artists"][0]["mediaType"] - assert artist.attachment_cover.url == data["artists"][0]["image"]["url"] - assert artist.attachment_cover.mimetype == data["artists"][0]["image"]["mediaType"] - - assert album_artist.from_activity == activity - assert album_artist.name == data["album"]["artists"][0]["name"] - assert album_artist.fid == data["album"]["artists"][0]["id"] - assert str(album_artist.mbid) == data["album"]["artists"][0]["musicbrainzId"] - assert album_artist.creation_date == published - assert album_artist.attributed_to == album_artist_attributed_to - assert album_artist.description.text == data["album"]["artists"][0]["content"] + assert artist.description.text == data["artist_credit"][0]["artist"]["content"] assert ( - album_artist.description.content_type - == data["album"]["artists"][0]["mediaType"] + artist.description.content_type + == data["artist_credit"][0]["artist"]["mediaType"] ) assert ( - album_artist.attachment_cover.url == data["album"]["artists"][0]["image"]["url"] + artist.attachment_cover.url + == data["artist_credit"][0]["artist"]["image"]["url"] + ) + assert ( + artist.attachment_cover.mimetype + == data["artist_credit"][0]["artist"]["image"]["mediaType"] + ) + + assert album_artist.from_activity == activity + assert album_artist.name == data["album"]["artist_credit"][0]["artist"]["name"] + assert album_artist.fid == data["album"]["artist_credit"][0]["artist"]["id"] + assert ( + str(album_artist.mbid) + == data["album"]["artist_credit"][0]["artist"]["musicbrainzId"] + ) + assert album_artist.creation_date == published + assert album_artist.attributed_to == album_artist_attributed_to + assert ( + album_artist.description.text + == data["album"]["artist_credit"][0]["artist"]["content"] + ) + assert ( + album_artist.description.content_type + == data["album"]["artist_credit"][0]["artist"]["mediaType"] + ) + assert ( + album_artist.attachment_cover.url + == data["album"]["artist_credit"][0]["artist"]["image"]["url"] ) assert ( album_artist.attachment_cover.mimetype - == data["album"]["artists"][0]["image"]["mediaType"] + == data["album"]["artist_credit"][0]["artist"]["image"]["mediaType"] ) add_tags.assert_any_call(track, *["Hello", "World"]) @@ -1144,7 +1223,9 @@ def test_activity_pub_track_serializer_from_ap_update(factories, r_mock, mocker, "content": "hello there", "attributedTo": track_attributed_to.fid, "album": serializers.AlbumSerializer(track.album).data, - "artists": [serializers.ArtistSerializer(track.artist).data], + "artist_credit": serializers.ArtistCreditSerializer( + track.artist_credit.all(), many=True + ).data, "image": {"type": "Image", "mediaType": "image/jpeg", "url": faker.url()}, "tag": [ {"type": "Hashtag", "name": "#Hello"}, @@ -1213,23 +1294,35 @@ def test_activity_pub_upload_serializer_from_ap(factories, mocker, r_mock): "href": "https://cover.image/test.png", "mediaType": "image/png", }, - "artists": [ + "artist_credit": [ { - "type": "Artist", - "id": "http://hello.artist", - "name": "John Smith", - "musicbrainzId": str(uuid.uuid4()), + "artist": { + "type": "Artist", + "id": "http://hello.artist", + "name": "John Smith", + "musicbrainzId": str(uuid.uuid4()), + "published": published.isoformat(), + }, + "joinphrase": "", + "credit": "John Smith Credit", "published": published.isoformat(), + "id": "http://hello.artistcredit", } ], }, - "artists": [ + "artist_credit": [ { - "type": "Artist", - "id": "http://hello.trackartist", - "name": "Bob Smith", - "musicbrainzId": str(uuid.uuid4()), + "artist": { + "type": "Artist", + "id": "http://hello.trackartist", + "name": "Bob Smith", + "musicbrainzId": str(uuid.uuid4()), + "published": published.isoformat(), + }, + "joinphrase": "", + "credit": "Bob Smith Credit", "published": published.isoformat(), + "id": "http://hello.artistcredit", } ], }, @@ -1259,7 +1352,6 @@ def test_activity_pub_upload_serializer_from_ap(factories, mocker, r_mock): def test_activity_pub_upload_serializer_from_ap_update(factories, mocker, now, r_mock): library = factories["music.Library"]() upload = factories["music.Upload"](library=library, track__album__with_cover=True) - data = { "@context": jsonld.get_default_context(), "type": "Audio", @@ -1631,7 +1723,7 @@ def test_channel_actor_outbox_serializer(factories): channel = factories["audio.Channel"]() uploads = factories["music.Upload"].create_batch( 5, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, library=channel.library, import_status="finished", ) @@ -1671,7 +1763,8 @@ def test_channel_upload_serializer(factories): track__license="cc0-1.0", track__copyright="Copyright something", track__album__set_tags=["Rock"], - track__artist__set_tags=["Indie"], + track__artist_credit__artist__set_tags=["Indie"], + track__artist_credit__artist__local=True, ) expected = { @@ -1722,9 +1815,9 @@ def test_channel_upload_serializer(factories): assert serializer.data == expected -def test_channel_upload_serializer_from_ap_create(factories, now): +def test_channel_upload_serializer_from_ap_create(factories, now, mocker): channel = factories["audio.Channel"](library__privacy_level="everyone") - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) payload = { "@context": jsonld.get_default_context(), "type": "Audio", @@ -1767,6 +1860,7 @@ def test_channel_upload_serializer_from_ap_create(factories, now): "url": "https://image.example/image.png", }, } + mocker.patch.object(utils, "retrieve_ap_object", return_value=album) serializer = serializers.ChannelUploadSerializer( data=payload, context={"channel": channel} @@ -1784,7 +1878,7 @@ def test_channel_upload_serializer_from_ap_create(factories, now): assert upload.size == payload["url"][1]["size"] assert upload.bitrate == payload["url"][1]["bitrate"] assert upload.duration == payload["duration"] - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist assert upload.track.position == payload["position"] assert upload.track.disc_number == payload["disc"] assert upload.track.attributed_to == channel.attributed_to @@ -1801,10 +1895,12 @@ def test_channel_upload_serializer_from_ap_create(factories, now): assert upload.track.album == album -def test_channel_upload_serializer_from_ap_update(factories, now): +def test_channel_upload_serializer_from_ap_update(factories, now, mocker): channel = factories["audio.Channel"](library__privacy_level="everyone") - album = factories["music.Album"](artist=channel.artist) - upload = factories["music.Upload"](track__album=album, track__artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) + upload = factories["music.Upload"]( + track__album=album, track__artist_credit__artist=channel.artist + ) payload = { "@context": jsonld.get_default_context(), @@ -1847,6 +1943,7 @@ def test_channel_upload_serializer_from_ap_update(factories, now): "url": "https://image.example/image.png", }, } + mocker.patch.object(utils, "retrieve_ap_object", return_value=album) serializer = serializers.ChannelUploadSerializer( data=payload, context={"channel": channel} @@ -1865,7 +1962,7 @@ def test_channel_upload_serializer_from_ap_update(factories, now): assert upload.size == payload["url"][1]["size"] assert upload.bitrate == payload["url"][1]["bitrate"] assert upload.duration == payload["duration"] - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist assert upload.track.position == payload["position"] assert upload.track.disc_number == payload["disc"] assert upload.track.attributed_to == channel.attributed_to @@ -1944,3 +2041,23 @@ def test_report_serializer_to_ap(factories): } serializer = serializers.FlagSerializer(report) assert serializer.data == expected + + +def test_artist_credit_serializer_to_ap(factories): + ac = factories["music.ArtistCredit"](artist__local=True) + serializer = serializers.ArtistCreditSerializer(ac) + + expected = { + "@context": jsonld.get_default_context(), + "id": ac.get_federation_id(), + "type": "ArtistCredit", + "artist": serializers.ArtistSerializer( + ac.artist, context={"include_ap_context": False} + ).data, + "joinphrase": ac.joinphrase, + "credit": ac.credit, + "index": ac.index, + "published": ac.creation_date.isoformat(), + } + + assert serializer.data == expected diff --git a/api/tests/history/test_filters.py b/api/tests/history/test_filters.py index 482590ef3..bdb20ccab 100644 --- a/api/tests/history/test_filters.py +++ b/api/tests/history/test_filters.py @@ -4,7 +4,9 @@ from funkwhale_api.history import filters, models def test_listening_filter_track_artist(factories, mocker, queryset_equal_list): factories["history.Listening"]() cf = factories["moderation.UserFilter"](for_artist=True) - hidden_listening = factories["history.Listening"](track__artist=cf.target_artist) + hidden_listening = factories["history.Listening"]( + track__artist_credit__artist=cf.target_artist + ) qs = models.Listening.objects.all() filterset = filters.ListeningFilter( {"hidden": "true"}, request=mocker.Mock(user=cf.user), queryset=qs @@ -17,7 +19,7 @@ def test_listening_filter_track_album_artist(factories, mocker, queryset_equal_l factories["history.Listening"]() cf = factories["moderation.UserFilter"](for_artist=True) hidden_listening = factories["history.Listening"]( - track__album__artist=cf.target_artist + track__album__artist_credit__artist=cf.target_artist ) qs = models.Listening.objects.all() filterset = filters.ListeningFilter( diff --git a/api/tests/manage/test_serializers.py b/api/tests/manage/test_serializers.py index 6ba29897e..004174a38 100644 --- a/api/tests/manage/test_serializers.py +++ b/api/tests/manage/test_serializers.py @@ -385,7 +385,13 @@ def test_manage_album_serializer(factories, now, to_api_date): "creation_date": to_api_date(album.creation_date), "release_date": album.release_date.isoformat(), "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, - "artist": serializers.ManageNestedArtistSerializer(album.artist).data, + "artist_credit": [ + { + "artist": serializers.ManageNestedArtistSerializer( + album.artist_credit.all()[0].artist + ).data + } + ], "attributed_to": serializers.ManageBaseActorSerializer( album.attributed_to ).data, @@ -412,7 +418,13 @@ def test_manage_track_serializer(factories, now, to_api_date): "copyright": track.copyright, "license": track.license, "creation_date": to_api_date(track.creation_date), - "artist": serializers.ManageNestedArtistSerializer(track.artist).data, + "artist_credit": [ + { + "artist": serializers.ManageNestedArtistSerializer( + track.artist_credit.all()[0].artist + ).data + } + ], "album": serializers.ManageTrackAlbumSerializer(track.album).data, "attributed_to": serializers.ManageBaseActorSerializer( track.attributed_to diff --git a/api/tests/manage/test_views.py b/api/tests/manage/test_views.py index 42ccf28c2..e1f8a195b 100644 --- a/api/tests/manage/test_views.py +++ b/api/tests/manage/test_views.py @@ -216,7 +216,9 @@ def test_album_list(factories, superuser_api_client, settings): album = factories["music.Album"]() factories["music.Album"]() url = reverse("api:v1:manage:library:albums-list") - response = superuser_api_client.get(url, {"q": f'artist:"{album.artist.name}"'}) + response = superuser_api_client.get( + url, {"q": f'artist:"{album.artist_credit.all()[0].artist.name}"'} + ) assert response.status_code == 200 diff --git a/api/tests/music/test_filters.py b/api/tests/music/test_filters.py index 9eb50dd71..96a042649 100644 --- a/api/tests/music/test_filters.py +++ b/api/tests/music/test_filters.py @@ -33,7 +33,7 @@ def test_album_filter_hidden(factories, mocker, queryset_equal_list): factories["music.Album"]() cf = factories["moderation.UserFilter"](for_artist=True) - hidden_album = factories["music.Album"](artist=cf.target_artist) + hidden_album = factories["music.Album"](artist_credit__artist=cf.target_artist) qs = models.Album.objects.all() filterset = filters.AlbumFilter( @@ -59,7 +59,7 @@ def test_artist_filter_hidden(factories, mocker, queryset_equal_list): def test_artist_filter_track_artist(factories, mocker, queryset_equal_list): factories["music.Track"]() cf = factories["moderation.UserFilter"](for_artist=True) - hidden_track = factories["music.Track"](artist=cf.target_artist) + hidden_track = factories["music.Track"](artist_credit__artist=cf.target_artist) qs = models.Track.objects.all() filterset = filters.TrackFilter( @@ -72,7 +72,9 @@ def test_artist_filter_track_artist(factories, mocker, queryset_equal_list): def test_artist_filter_track_album_artist(factories, mocker, queryset_equal_list): factories["music.Track"]() cf = factories["moderation.UserFilter"](for_artist=True) - hidden_track = factories["music.Track"](album__artist=cf.target_artist) + hidden_track = factories["music.Track"]( + album__artist_credit__artist=cf.target_artist + ) qs = models.Track.objects.all() filterset = filters.TrackFilter( @@ -141,7 +143,9 @@ def test_track_filter_tag_multiple( def test_channel_filter_track(factories, queryset_equal_list, mocker, anonymous_user): channel = factories["audio.Channel"](library__privacy_level="everyone") upload = factories["music.Upload"]( - library=channel.library, playable=True, track__artist=channel.artist + library=channel.library, + playable=True, + track__artist_credit__artist=channel.artist, ) factories["music.Track"]() qs = upload.track.__class__.objects.all() @@ -157,7 +161,9 @@ def test_channel_filter_track(factories, queryset_equal_list, mocker, anonymous_ def test_channel_filter_album(factories, queryset_equal_list, mocker, anonymous_user): channel = factories["audio.Channel"](library__privacy_level="everyone") upload = factories["music.Upload"]( - library=channel.library, playable=True, track__artist=channel.artist + library=channel.library, + playable=True, + track__artist_credit__artist=channel.artist, ) factories["music.Album"]() qs = upload.track.album.__class__.objects.all() @@ -202,14 +208,14 @@ def test_library_filter_artist(factories, queryset_equal_list, mocker, anonymous library = factories["music.Library"](privacy_level="everyone") upload = factories["music.Upload"](library=library, playable=True) factories["music.Artist"]() - qs = upload.track.artist.__class__.objects.all() + + qs = models.Artist.objects.all() filterset = filters.ArtistFilter( {"library": library.uuid}, request=mocker.Mock(user=anonymous_user, actor=None), queryset=qs, ) - - assert filterset.qs == [upload.track.artist] + assert filterset.qs == [upload.track.artist_credit.all()[0].artist] def test_track_filter_artist_includes_album_artist( @@ -218,12 +224,13 @@ def test_track_filter_artist_includes_album_artist( factories["music.Track"]() track1 = factories["music.Track"]() track2 = factories["music.Track"]( - album__artist=track1.artist, artist=factories["music.Artist"]() + album__artist_credit__artist=track1.artist_credit.all()[0].artist, + artist_credit__artist=factories["music.Artist"](), ) qs = models.Track.objects.all() filterset = filters.TrackFilter( - {"artist": track1.artist.pk}, + {"artist": track1.artist_credit.all()[0].artist.pk}, request=mocker.Mock(user=anonymous_user), queryset=qs, ) diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py index a9cd45d62..29eb27e24 100644 --- a/api/tests/music/test_metadata.py +++ b/api/tests/music/test_metadata.py @@ -312,23 +312,29 @@ def test_metadata_fallback_ogg_theora(mocker): "test.mp3", { "title": "Bend", - "artists": [ + "artist_credit": [ { - "name": "Binärpilot", + "credit": "Binärpilot", "mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"), + "joinphrase": "; ", + }, + { + "credit": "Another artist", + "mbid": None, + "joinphrase": "", }, - {"name": "Another artist", "mbid": None}, ], "album": { "title": "You Can't Stop Da Funk", "mbid": uuid.UUID("ce40cdb1-a562-4fd8-a269-9269f98d4124"), "release_date": datetime.date(2006, 2, 7), - "artists": [ + "artist_credit": [ { - "name": "Binärpilot", + "credit": "Binärpilot", "mbid": uuid.UUID("9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"), + "joinphrase": "; ", }, - {"name": "Another artist", "mbid": None}, + {"credit": "Another artist", "mbid": None, "joinphrase": ""}, ], }, "position": 2, @@ -344,25 +350,32 @@ def test_metadata_fallback_ogg_theora(mocker): "test.ogg", { "title": "Peer Gynt Suite no. 1, op. 46: I. Morning", - "artists": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "; ", + }, + { + "credit": "Musopen Symphony Orchestra", + "mbid": None, + "joinphrase": "", }, - {"name": "Musopen Symphony Orchestra", "mbid": None}, ], "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": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "; ", }, { - "name": "Musopen Symphony Orchestra", + "credit": "Musopen Symphony Orchestra", "mbid": uuid.UUID("5b4d7d2d-36df-4b38-95e3-a964234f520f"), + "joinphrase": "", }, ], }, @@ -379,25 +392,32 @@ def test_metadata_fallback_ogg_theora(mocker): "test.opus", { "title": "Peer Gynt Suite no. 1, op. 46: I. Morning", - "artists": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "; ", + }, + { + "credit": "Musopen Symphony Orchestra", + "mbid": None, + "joinphrase": "", }, - {"name": "Musopen Symphony Orchestra", "mbid": None}, ], "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": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "; ", }, { - "name": "Musopen Symphony Orchestra", + "credit": "Musopen Symphony Orchestra", "mbid": uuid.UUID("5b4d7d2d-36df-4b38-95e3-a964234f520f"), + "joinphrase": "", }, ], }, @@ -414,20 +434,22 @@ def test_metadata_fallback_ogg_theora(mocker): "test_theora.ogg", { "title": "Drei Kreuze (dass wir hier sind)", - "artists": [ + "artist_credit": [ { - "name": "Die Toten Hosen", + "credit": "Die Toten Hosen", "mbid": uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"), + "joinphrase": "", } ], "album": { "title": "Ballast der Republik", "mbid": uuid.UUID("1f0441ad-e609-446d-b355-809c445773cf"), "release_date": datetime.date(2012, 5, 4), - "artists": [ + "artist_credit": [ { - "name": "Die Toten Hosen", + "credit": "Die Toten Hosen", "mbid": uuid.UUID("c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1"), + "joinphrase": "", } ], }, @@ -446,20 +468,22 @@ def test_metadata_fallback_ogg_theora(mocker): "sample.flac", { "title": "999,999", - "artists": [ + "artist_credit": [ { - "name": "Nine Inch Nails", + "credit": "Nine Inch Nails", "mbid": uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da"), + "joinphrase": "", } ], "album": { "title": "The Slip", "mbid": uuid.UUID("12b57d46-a192-499e-a91f-7da66790a1c1"), "release_date": datetime.date(2008, 5, 5), - "artists": [ + "artist_credit": [ { - "name": "Nine Inch Nails", + "credit": "Nine Inch Nails", "mbid": uuid.UUID("b7ffd2af-418f-4be2-bdd1-22f8b48613da"), + "joinphrase": "", } ], }, @@ -497,12 +521,14 @@ def test_track_metadata_serializer(path, expected, mocker): }, [ { - "name": "Hello", + "credit": "Hello", "mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"), + "joinphrase": "; ", }, { - "name": "World", + "credit": "World", "mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"), + "joinphrase": "", }, ], ), @@ -513,14 +539,16 @@ def test_track_metadata_serializer(path, expected, mocker): }, [ { - "name": "Hello", + "credit": "Hello", "mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcb"), + "joinphrase": "; ", }, { - "name": "World", + "credit": "World", "mbid": uuid.UUID("f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"), + "joinphrase": "; ", }, - {"name": "Foo", "mbid": None}, + {"credit": "Foo", "mbid": None, "joinphrase": ""}, ], ), ], @@ -533,8 +561,8 @@ def test_artists_cleaning(raw, expected): @pytest.mark.parametrize( "data, errored_field", [ - ({"name": "Hello", "mbid": "wrong-uuid"}, "mbid"), # wrong uuid - ({"name": "", "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"}, "name"), + ({"credit": "Hello", "mbid": "wrong-uuid"}, "mbid"), # wrong uuid + ({"credit": "", "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcd"}, "credit"), ], ) def test_artist_serializer_validation(data, errored_field): @@ -584,24 +612,27 @@ def test_fake_metadata_with_serializer(): expected = { "title": "Peer Gynt Suite no. 1, op. 46: I. Morning", "description": {"text": "hello there", "content_type": "text/plain"}, - "artists": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "", } ], "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": [ + "artist_credit": [ { - "name": "Edvard Grieg", + "credit": "Edvard Grieg", "mbid": uuid.UUID("013c8e5b-d72a-4cd3-8dee-6c64d6125823"), + "joinphrase": "; ", }, { - "name": "Musopen Symphony Orchestra", + "credit": "Musopen Symphony Orchestra", "mbid": uuid.UUID("5b4d7d2d-36df-4b38-95e3-a964234f520f"), + "joinphrase": "", }, ], "cover_data": None, @@ -614,6 +645,7 @@ def test_fake_metadata_with_serializer(): } serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data)) assert serializer.is_valid(raise_exception=True) is True + assert serializer.validated_data == expected @@ -626,12 +658,12 @@ def test_serializer_album_artist_missing(): expected = { "title": "Peer Gynt Suite no. 1, op. 46: I. Morning", - "artists": [{"name": "Edvard Grieg", "mbid": None}], + "artist_credit": [{"credit": "Edvard Grieg", "mbid": None, "joinphrase": ""}], "album": { "title": "Peer Gynt Suite no. 1, op. 46", "mbid": None, "release_date": None, - "artists": [], + "artist_credit": [], "cover_data": None, }, } @@ -654,12 +686,12 @@ def test_serializer_album_artist_missing(): def test_serializer_album_default_title_when_missing_or_empty(data): expected = { "title": "Track", - "artists": [{"name": "Artist", "mbid": None}], + "artist_credit": [{"credit": "Artist", "mbid": None, "joinphrase": ""}], "album": { "title": metadata.UNKNOWN_ALBUM, "mbid": None, "release_date": None, - "artists": [], + "artist_credit": [], "cover_data": None, }, } @@ -681,12 +713,12 @@ def test_serializer_empty_fields(field_name): } expected = { "title": "Track Title", - "artists": [{"name": "Track Artist", "mbid": None}], + "artist_credit": [{"credit": "Track Artist", "mbid": None, "joinphrase": ""}], "album": { "title": "Track Album", "mbid": None, "release_date": None, - "artists": [], + "artist_credit": [], "cover_data": None, }, } @@ -698,12 +730,12 @@ def test_serializer_empty_fields(field_name): def test_serializer_strict_mode_false(): data = {} expected = { - "artists": [{"name": None, "mbid": None}], + "artist_credit": [], "album": { "title": "[Unknown Album]", "mbid": None, "release_date": None, - "artists": [], + "artist_credit": [], "cover_data": None, }, } @@ -730,11 +762,21 @@ def test_artist_field_featuring(): "musicbrainz_artistid": "9a3bf45c-347d-4630-894d-7cf3e8e0b632/cbf9738d-8f81-4a92-bc64-ede09341652d", } - expected = [{"name": "Santana feat. Chris Cornell", "mbid": None}] + expected = [ + { + "credit": "Santana", + "mbid": uuid.UUID("9a3bf45c-347d-4630-894d-7cf3e8e0b632"), + "joinphrase": " feat. ", + }, + { + "credit": "Chris Cornell", + "mbid": uuid.UUID("cbf9738d-8f81-4a92-bc64-ede09341652d"), + "joinphrase": "", + }, + ] field = metadata.ArtistField() value = field.get_value(data) - assert field.to_internal_value(value) == expected @@ -756,23 +798,23 @@ def test_acquire_tags_from_genre(genre, expected_tags): data = { "title": "Track Title", "artist": "Track Artist", + # "artist_credit": [{"credit": "Track Artist"}], "album": "Track Album", "genre": genre, } expected = { "title": "Track Title", - "artists": [{"name": "Track Artist", "mbid": None}], + "artist_credit": [{"credit": "Track Artist", "mbid": None, "joinphrase": ""}], "album": { "title": "Track Album", "mbid": None, "release_date": None, - "artists": [], + "artist_credit": [], "cover_data": None, }, } if expected_tags: expected["tags"] = expected_tags - serializer = metadata.TrackMetadataSerializer(data=metadata.FakeMetadata(data)) assert serializer.is_valid(raise_exception=True) is True assert serializer.validated_data == expected diff --git a/api/tests/music/test_migrations.py b/api/tests/music/test_migrations.py index 907f58678..69826636e 100644 --- a/api/tests/music/test_migrations.py +++ b/api/tests/music/test_migrations.py @@ -27,6 +27,48 @@ # upload_manager = new_apps.get_model(a, "Upload") + # for upload in mapping_list: # upload_obj = upload_manager.objects.get(pk=upload[4]) # assert upload_obj.quality == upload[3] + + +def test_artist_credit_migration(migrator): + mapping_list = [("artist_name", "album_title", "track_title")] + + a, f, t = ( + "music", + "0058_upload_quality", + "0059_remove_album_artist_remove_track_artist_artistcredit_and_more", + ) + + migrator.migrate([(a, f)]) + old_apps = migrator.loader.project_state([(a, f)]).apps + Track = old_apps.get_model(a, "Track") + Album = old_apps.get_model(a, "Album") + Artist = old_apps.get_model(a, "Artist") + + for track in mapping_list: + artist = Artist.objects.create(name=track[0]) + old_album = Album.objects.create(title=track[1], artist=artist) + old_track = Track.objects.create(title=track[2], artist=artist, album=old_album) + + migrator.loader.build_graph() + + migrator.migrate([(a, t)]) + new_apps = migrator.loader.project_state([(a, t)]).apps + + track_manager = new_apps.get_model(a, "Track") + album_manager = new_apps.get_model(a, "Album") + + for track in mapping_list: + track_obj = track_manager.objects.get(title=track[2]) + album_obj = album_manager.objects.get(title=track[1]) + + assert track_obj.artist_credit.all()[0].artist.pk == old_track.artist.pk + assert track_obj.artist_credit.all()[0].joinphrase == "" + assert track_obj.artist_credit.all()[0].credit == old_track.artist.name + + assert album_obj.artist_credit.all()[0].artist.pk == old_album.artist.pk + assert album_obj.artist_credit.all()[0].joinphrase == "" + assert album_obj.artist_credit.all()[0].credit == old_album.artist.name diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py index 94bbd7501..72250110c 100644 --- a/api/tests/music/test_models.py +++ b/api/tests/music/test_models.py @@ -43,7 +43,7 @@ def test_import_album_stores_release_group(factories): album = importers.load(models.Album, cleaned_data, album_data, import_hooks=[]) assert album.release_group_id == album_data["release-group"]["id"] - assert album.artist == artist + assert album.artist_credit.all()[0].artist == artist def test_import_track_from_release(factories, mocker): @@ -91,8 +91,72 @@ def test_import_track_from_release(factories, mocker): mocked_get.assert_called_once_with(album.mbid, includes=models.Album.api_includes) assert track.title == track_data["recording"]["title"] assert track.mbid == track_data["recording"]["id"] - assert track.album == album - assert track.artist == artist + assert track.artist_credit.all()[0].albums.all()[0] == album + assert track.artist_credit.all()[0].artist == artist + assert track.position == int(track_data["position"]) + + +def test_import_track_from_multi_artist_credit_release(factories, mocker): + album = factories["music.Album"](mbid="430347cb-0879-3113-9fde-c75b658c298e") + artist = factories["music.Artist"](mbid="a5211c65-2465-406b-93ec-213588869dc1") + artist2 = factories["music.Artist"](mbid="a5211c65-2465-406b-93ec-21358ee69dc1") + album_data = { + "release": { + "id": album.mbid, + "title": "Daydream Nation", + "status": "Official", + "medium-count": 1, + "medium-list": [ + { + "position": "1", + "format": "CD", + "track-list": [ + { + "id": "03baca8b-855a-3c05-8f3d-d3235287d84d", + "position": "4", + "number": "4", + "length": "417973", + "recording": { + "id": "2109e376-132b-40ad-b993-2bb6812e19d4", + "title": "Teen Age Riot", + "length": "417973", + "artist-credit": [ + { + "joinphrase": "feat", + "artist": { + "id": artist.mbid, + "name": artist.name, + }, + }, + { + "joinphrase": "", + "artist": { + "id": artist2.mbid, + "name": artist2.name, + }, + }, + ], + }, + "track_or_recording_length": "417973", + } + ], + "track-count": 1, + } + ], + } + } + mocked_get = mocker.patch( + "funkwhale_api.musicbrainz.api.releases.get", return_value=album_data + ) + track_data = album_data["release"]["medium-list"][0]["track-list"][0] + track = models.Track.get_or_create_from_release( + "430347cb-0879-3113-9fde-c75b658c298e", track_data["recording"]["id"] + )[0] + mocked_get.assert_called_once_with(album.mbid, includes=models.Album.api_includes) + assert track.title == track_data["recording"]["title"] + assert track.mbid == track_data["recording"]["id"] + assert track.artist_credit.all()[0].albums.all()[0] == album + assert [ac.artist for ac in track.artist_credit.all()] == [artist, artist2] assert track.position == int(track_data["position"]) @@ -144,12 +208,11 @@ def test_import_track_with_different_artist_than_release(factories, mocker): mocker.patch( "funkwhale_api.musicbrainz.api.recordings.get", return_value=recording_data ) - track = models.Track.get_or_create_from_api(recording_data["recording"]["id"])[0] assert track.title == recording_data["recording"]["title"] assert track.mbid == recording_data["recording"]["id"] assert track.album == album - assert track.artist == artist + assert track.artist_credit.all()[0].artist == artist @pytest.mark.parametrize( @@ -391,7 +454,7 @@ def test_artist_playable_by_correct_actor(privacy_level, expected, factories): queryset = models.Artist.objects.playable_by( upload.library.actor ).annotate_playable_by_actor(upload.library.actor) - match = upload.track.artist in list(queryset) + match = [ac.artist for ac in upload.track.artist_credit.all()][0] == queryset.get() assert match is expected if expected: assert bool(queryset.first().is_playable_by_actor) is expected @@ -410,7 +473,7 @@ def test_artist_playable_by_instance_actor(privacy_level, expected, factories): queryset = models.Artist.objects.playable_by( instance_actor ).annotate_playable_by_actor(instance_actor) - match = upload.track.artist in list(queryset) + match = [ac.artist for ac in upload.track.artist_credit.all()][0] in queryset assert match is expected if expected: assert bool(queryset.first().is_playable_by_actor) is expected @@ -426,7 +489,7 @@ def test_artist_playable_by_anonymous(privacy_level, expected, factories): library__local=True, ) queryset = models.Artist.objects.playable_by(None).annotate_playable_by_actor(None) - match = upload.track.artist in list(queryset) + match = [ac.artist for ac in upload.track.artist_credit.all()][0] in queryset assert match is expected if expected: assert bool(queryset.first().is_playable_by_actor) is expected diff --git a/api/tests/music/test_music.py b/api/tests/music/test_music.py index 15fab39d4..4d903a446 100644 --- a/api/tests/music/test_music.py +++ b/api/tests/music/test_music.py @@ -39,8 +39,10 @@ def test_can_create_album_from_api(artists, albums, mocker, db): assert album.mbid, data["id"] assert album.title, "Hypnotize" assert album.release_date, datetime.date(2005, 1, 1) - assert album.artist.name, "System of a Down" - assert album.artist.mbid, data["artist-credit"][0]["artist"]["id"] + assert album.artist_credit.all()[0].artist.name, "System of a Down" + assert album.artist_credit.all()[0].artist.mbid, data["artist-credit"][0]["artist"][ + "id" + ] assert album.fid == federation_utils.full_url( f"/federation/music/albums/{album.uuid}" ) @@ -64,9 +66,12 @@ def test_can_create_track_from_api(artists, albums, tracks, mocker, db): assert int(data["ext:score"]) == 100 assert data["id"] == "9968a9d6-8d92-4051-8f76-674e157b6eed" assert track.mbid == data["id"] - assert track.artist.pk is not None - assert str(track.artist.mbid) == "62c3befb-6366-4585-b256-809472333801" - assert track.artist.name == "Adhesive Wombat" + assert track.artist_credit.all()[0].artist.pk is not None + assert ( + str(track.artist_credit.all()[0].artist.mbid) + == "62c3befb-6366-4585-b256-809472333801" + ) + assert track.artist_credit.all()[0].artist.name == "Adhesive Wombat" assert str(track.album.mbid) == "a50d2a81-2a50-484d-9cb4-b9f6833f583e" assert track.album.title == "Marsupial Madness" assert track.fid == federation_utils.full_url( @@ -114,9 +119,12 @@ def test_can_get_or_create_track_from_api(artists, albums, tracks, mocker, db): assert int(data["ext:score"]) == 100 assert data["id"] == "9968a9d6-8d92-4051-8f76-674e157b6eed" assert track.mbid == data["id"] - assert track.artist.pk is not None - assert str(track.artist.mbid) == "62c3befb-6366-4585-b256-809472333801" - assert track.artist.name == "Adhesive Wombat" + assert track.artist_credit.all()[0].artist.pk is not None + assert ( + str(track.artist_credit.all()[0].artist.mbid) + == "62c3befb-6366-4585-b256-809472333801" + ) + assert track.artist_credit.all()[0].artist.name == "Adhesive Wombat" track2, created = models.Track.get_or_create_from_api(mbid=data["id"]) assert not created diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 7df47e537..b0fedb90a 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -38,7 +38,7 @@ def test_artist_album_serializer(factories, to_api_date): "fid": album.fid, "mbid": str(album.mbid), "title": album.title, - "artist": album.artist.id, + "artist_credit": [ac.id for ac in album.artist_credit.all()], "creation_date": to_api_date(album.creation_date), "tracks_count": 1, "is_playable": None, @@ -53,13 +53,16 @@ def test_artist_album_serializer(factories, to_api_date): def test_artist_with_albums_serializer(factories, to_api_date): actor = factories["federation.Actor"]() - track = factories["music.Track"]( - album__artist__attributed_to=actor, album__artist__with_cover=True + artist_credit = factories["music.ArtistCredit"]( + artist__attributed_to=actor, artist__with_cover=True ) - artist = track.artist - artist = artist.__class__.objects.with_albums().get(pk=artist.pk) - album = list(artist.albums.all())[0] - setattr(artist, "_prefetched_tracks", range(42)) + + track = factories["music.Track"]( + album__artist_credit=artist_credit, artist_credit=artist_credit + ) + artist = track.artist_credit.all()[0].artist + album = artist.artist_credit.all()[0].albums.all()[0] + setattr(artist, "_tracks_count", 42) expected = { "id": artist.id, "fid": artist.fid, @@ -81,12 +84,16 @@ def test_artist_with_albums_serializer(factories, to_api_date): def test_artist_with_albums_serializer_channel(factories, to_api_date): actor = factories["federation.Actor"]() - channel = factories["audio.Channel"](attributed_to=actor, artist__with_cover=True) - track = factories["music.Track"](album__artist=channel.artist) - artist = track.artist + artist = factories["music.Artist"](with_cover=True, attributed_to=actor) + channel = factories["audio.Channel"](attributed_to=actor, artist=artist) + artist_credit = factories["music.ArtistCredit"](artist=channel.artist) + track = factories["music.Track"]( + album__artist_credit=artist_credit, artist_credit=artist_credit + ) + artist = track.artist_credit.all()[0].artist artist = artist.__class__.objects.with_albums().get(pk=artist.pk) - album = list(artist.albums.all())[0] - setattr(artist, "_prefetched_tracks", range(42)) + album = list(artist.artist_credit.all()[0].albums.all())[0] + setattr(artist, "_tracks_count", 42) expected = { "id": artist.id, "fid": artist.fid, @@ -110,7 +117,9 @@ def test_artist_with_albums_serializer_channel(factories, to_api_date): }, } serializer = serializers.ArtistWithAlbumsSerializer(artist) - assert serializer.data == expected + if not serializer.data == expected: + "lol" + assert serializer.data == expected def test_upload_serializer(factories, to_api_date): @@ -177,7 +186,9 @@ def test_album_serializer(factories, to_api_date): "fid": album.fid, "mbid": str(album.mbid), "title": album.title, - "artist": serializers.SimpleArtistSerializer(album.artist).data, + "artist_credit": serializers.ArtistCreditSerializer( + album.artist_credit.all(), many=True + ).data, "creation_date": to_api_date(album.creation_date), "is_playable": False, "duration": 0, @@ -207,7 +218,9 @@ def test_track_album_serializer(factories, to_api_date): "fid": album.fid, "mbid": str(album.mbid), "title": album.title, - "artist": serializers.SimpleArtistSerializer(album.artist).data, + "artist_credit": serializers.ArtistCreditSerializer( + album.artist_credit.all(), many=True + ).data, "creation_date": to_api_date(album.creation_date), "is_playable": False, "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, @@ -239,7 +252,9 @@ def test_track_serializer(factories, to_api_date): expected = { "id": track.id, "fid": track.fid, - "artist": serializers.SimpleArtistSerializer(track.artist).data, + "artist_credit": serializers.ArtistCreditSerializer( + track.artist_credit.all(), many=True + ).data, "album": serializers.TrackAlbumSerializer(track.album).data, "mbid": str(track.mbid), "title": track.title, diff --git a/api/tests/music/test_spa_views.py b/api/tests/music/test_spa_views.py index 391b5cc2e..d388f64ce 100644 --- a/api/tests/music/test_spa_views.py +++ b/api/tests/music/test_spa_views.py @@ -41,7 +41,10 @@ def test_library_track(spa_html, no_api_auth, client, factories, settings): "property": "music:musician", "content": utils.join_url( settings.FUNKWHALE_URL, - utils.spa_reverse("library_artist", kwargs={"pk": track.artist.pk}), + utils.spa_reverse( + "library_artist", + kwargs={"pk": track.artist_credit.all()[0].artist.pk}, + ), ), }, { @@ -117,7 +120,10 @@ def test_library_album(spa_html, no_api_auth, client, factories, settings): "property": "music:musician", "content": utils.join_url( settings.FUNKWHALE_URL, - utils.spa_reverse("library_artist", kwargs={"pk": album.artist.pk}), + utils.spa_reverse( + "library_artist", + kwargs={"pk": album.artist_credit.all()[0].artist.pk}, + ), ), }, { @@ -166,7 +172,7 @@ def test_library_album(spa_html, no_api_auth, client, factories, settings): def test_library_artist(spa_html, no_api_auth, client, factories, settings): album = factories["music.Album"](with_cover=True) factories["music.Upload"](playable=True, track__album=album) - artist = album.artist + artist = album.artist_credit.all()[0].artist url = f"/library/artists/{artist.pk}" response = client.get(url) diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index b5b8a31e2..0313b0000 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -4,6 +4,7 @@ import uuid import pytest from django.core.paginator import Paginator +from django.db.models import Q from django.utils import timezone from funkwhale_api.common import utils as common_utils @@ -22,7 +23,7 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker): add_tags = mocker.patch("funkwhale_api.tags.models.add_tags") metadata = { "title": "Test track", - "artists": [{"name": "Test artist"}], + "artist_credit": [{"credit": "Test artist", "joinphrase": ""}], "album": {"title": "Test album", "release_date": datetime.date(2012, 8, 15)}, "position": 4, "disc_number": 2, @@ -43,12 +44,15 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker): 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["artists"][0]["name"] - assert track.artist.mbid is None - assert track.artist.attributed_to is None + assert ( + track.artist_credit.all()[0].artist.name + == metadata["artist_credit"][0]["credit"] + ) + assert track.artist_credit.all()[0].artist.mbid is None + assert track.artist_credit.all()[0].artist.attributed_to is None match_license.assert_called_once_with(metadata["license"], metadata["copyright"]) add_tags.assert_any_call(track, *metadata["tags"]) - add_tags.assert_any_call(track.artist, *[]) + add_tags.assert_any_call(track.artist_credit.all()[0].artist, *[]) add_tags.assert_any_call(track.album, *[]) @@ -56,7 +60,7 @@ def test_can_create_track_from_file_metadata_attributed_to(factories, mocker): actor = factories["federation.Actor"]() metadata = { "title": "Test track", - "artists": [{"name": "Test artist"}], + "artist_credit": [{"credit": "Test artist", "joinphrase": ""}], "album": {"title": "Test album", "release_date": datetime.date(2012, 8, 15)}, "position": 4, "disc_number": 2, @@ -75,12 +79,15 @@ def test_can_create_track_from_file_metadata_attributed_to(factories, mocker): assert track.album.mbid is None assert track.album.release_date == datetime.date(2012, 8, 15) assert track.album.attributed_to == actor - assert track.artist.name == metadata["artists"][0]["name"] - assert track.artist.mbid is None - assert track.artist.attributed_to == actor + assert ( + track.artist_credit.all()[0].artist.name + == metadata["artist_credit"][0]["credit"] + ) + assert track.artist_credit.all()[0].artist.mbid is None + assert track.artist_credit.all()[0].artist.attributed_to == actor -def test_can_create_track_from_file_metadata_featuring(factories): +def test_can_create_track_from_file_metadata_featuring(mocker): metadata = { "title": "Whole Lotta Love", "position": 1, @@ -90,16 +97,63 @@ def test_can_create_track_from_file_metadata_featuring(factories): "title": "Guitar Heaven: The Greatest Guitar Classics of All Time", "mbid": "d06f2072-4148-488d-af6f-69ab6539ddb8", "release_date": datetime.date(2010, 9, 17), - "artists": [ - {"name": "Santana", "mbid": "9a3bf45c-347d-4630-894d-7cf3e8e0b632"} + "artist_credit": [ + { + "credit": "Santana", + "mbid": "9a3bf45c-347d-4630-894d-7cf3e8e0b632", + "joinphrase": "", + } ], }, - "artists": [{"name": "Santana feat. Chris Cornell", "mbid": None}], + "artist_credit": [ + {"credit": "Santana feat Chris Cornell", "mbid": None, "joinphrase": ""} + ], } + mb_ac = { + "artist-credit": [ + { + "joinphrase": " feat ", + "artist": { + "id": "9a3bf45c-347d-4630-894d-7cf3e8e0b632", + "type": "Group", + "name": "Santana", + "sort-name": "Santana", + }, + }, + { + "artist": { + "id": "11e46b16-2f25-4783-ab32-25250befe84a", + "type": "Person", + "name": "Chris Cornell", + "sort-name": "Chris Cornell", + }, + "joinphrase": "", + }, + ] + } + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": "9a3bf45c-347d-4630-894d-7cf3e8e0b632", + "type": "Group", + "name": "Santana", + "sort-name": "Santana", + }, + "joinphrase": "", + }, + ] + } + mocker.patch.object( + tasks.musicbrainz.api.recordings, "get", return_value={"recording": mb_ac} + ) + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) track = tasks.get_track_from_import_metadata(metadata) - assert track.album.artist.name == "Santana" - assert track.artist.name == "Santana feat. Chris Cornell" + assert track.album.artist_credit.all()[0].artist.name == "Santana" + assert track.get_artist_credit_string == "Santana feat Chris Cornell" def test_can_create_track_from_file_metadata_description(factories): @@ -109,7 +163,7 @@ def test_can_create_track_from_file_metadata_description(factories): "disc_number": 1, "description": {"text": "hello there", "content_type": "text/plain"}, "album": {"title": "Test album"}, - "artists": [{"name": "Santana"}], + "artist_credit": [{"credit": "Santana", "joinphrase": ""}], } track = tasks.get_track_from_import_metadata(metadata) @@ -124,27 +178,34 @@ def test_can_create_track_from_file_metadata_use_featuring(factories): "disc_number": 1, "description": {"text": "hello there", "content_type": "text/plain"}, "album": {"title": "Test album"}, - "artists": [{"name": "Santana"}, {"name": "Anatnas"}], + "artist_credit": [ + {"credit": "Santana", "joinphrase": ", ", "index": 0}, + {"credit": "Anatnas", "joinphrase": "", "index": 1}, + ], } track = tasks.get_track_from_import_metadata(metadata) - - assert track.artist.name == "Anatnas" + assert track.get_artist_credit_string == "Santana, Anatnas" def test_can_create_track_from_file_metadata_mbid(factories, mocker): metadata = { "title": "Test track", - "artists": [ - {"name": "Test artist", "mbid": "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13"} + "artist_credit": [ + { + "credit": "Test artist", + "mbid": "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13", + "joinphrase": "", + } ], "album": { "title": "Test album", "release_date": datetime.date(2012, 8, 15), "mbid": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e15", - "artists": [ + "artist_credit": [ { - "name": "Test album artist", + "credit": "Test album artist", "mbid": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e13", + "joinphrase": "", } ], }, @@ -152,6 +213,37 @@ def test_can_create_track_from_file_metadata_mbid(factories, mocker): "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb", "cover_data": {"content": b"image_content", "mimetype": "image/png"}, } + mb_ac = { + "artist-credit": [ + { + "artist": { + "id": "9c6bddde-6228-4d9f-ad0d-03f6fcb19e13", + "name": "Test artist", + }, + "joinphrase": "", + "name": "Test artist", + } + ] + } + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": "9c6bddde-6478-4d9f-ad0d-03f6fcb19e13", + "name": "Test album artist", + }, + "name": "s", + "joinphrase": "; ", + }, + ] + } + + mocker.patch.object( + tasks.musicbrainz.api.recordings, "get", return_value={"recording": mb_ac} + ) + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) track = tasks.get_track_from_import_metadata(metadata) @@ -161,29 +253,80 @@ def test_can_create_track_from_file_metadata_mbid(factories, mocker): assert track.disc_number is None 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 ( + str(track.album.artist_credit.all()[0].artist.mbid) + == metadata["album"]["artist_credit"][0]["mbid"] + ) + assert ( + track.album.artist_credit.all()[0].artist.name + == metadata["album"]["artist_credit"][0]["credit"] + ) assert track.album.release_date == datetime.date(2012, 8, 15) - assert track.artist.name == metadata["artists"][0]["name"] - assert track.artist.mbid == metadata["artists"][0]["mbid"] + assert ( + track.artist_credit.all()[0].artist.name + == metadata["artist_credit"][0]["credit"] + ) + assert ( + str(track.artist_credit.all()[0].artist.mbid) + == metadata["artist_credit"][0]["mbid"] + ) def test_can_create_track_from_file_metadata_mbid_existing_album_artist( factories, mocker ): - artist = factories["music.Artist"]() - album = factories["music.Album"]() + artist_credit = factories["music.ArtistCredit"](joinphrase="", index=0) + album = factories["music.Album"](artist_credit=artist_credit) metadata = { "album": { "mbid": album.mbid, "title": "", - "artists": [{"name": "", "mbid": album.artist.mbid}], + "artist_credit": [ + { + "credit": "", + "joinphrase": "", + "mbid": album.artist_credit.all()[0].mbid, + } + ], }, "title": "Hello", "position": 4, - "artists": [{"mbid": artist.mbid, "name": ""}], - "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73fcb", + "artist_credit": [ + {"mbid": album.artist_credit.all()[0].mbid, "credit": "", "joinphrase": ""} + ], + "mbid": "f269d497-1cc0-4ae4-a0c4-157ec7d73acb", } + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": artist_credit.artist.mbid, + "name": artist_credit.artist.name, + }, + "name": artist_credit.artist.name, + "joinphrase": "; ", + }, + ] + } + mb_ac = { + "artist-credit": [ + { + "artist": { + "id": album.artist_credit.all()[0].artist.mbid, + "name": "Test artist", + }, + "joinphrase": "", + "name": album.artist_credit.all()[0].artist.name, + } + ] + } + + mocker.patch.object( + tasks.musicbrainz.api.recordings, "get", return_value={"recording": mb_ac} + ) + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) track = tasks.get_track_from_import_metadata(metadata) @@ -191,7 +334,7 @@ def test_can_create_track_from_file_metadata_mbid_existing_album_artist( assert track.mbid == metadata["mbid"] assert track.position == 4 assert track.album == album - assert track.artist == artist + assert track.artist_credit.all()[0] == artist_credit def test_can_create_track_from_file_metadata_fid_existing_album_artist( @@ -200,11 +343,22 @@ def test_can_create_track_from_file_metadata_fid_existing_album_artist( artist = factories["music.Artist"]() album = factories["music.Album"]() metadata = { - "artists": [{"name": "", "fid": artist.fid}], + "artist_credit": [ + {"credit": "", "artist": {"fid": artist.fid}, "joinphrase": ""} + ], "album": { "title": "", "fid": album.fid, - "artists": [{"name": "", "fid": album.artist.fid}], + "artist_credit": [ + { + "credit": "", + "joinphrase": "", + "artist": { + "name": "", + "fid": album.artist_credit.all()[0].artist.fid, + }, + } + ], }, "title": "Hello", "position": 4, @@ -217,22 +371,43 @@ def test_can_create_track_from_file_metadata_fid_existing_album_artist( assert track.fid == metadata["fid"] assert track.position == 4 assert track.album == album - assert track.artist == artist + assert track.artist_credit.all()[0].artist == artist -def test_can_create_track_from_file_metadata_distinct_release_mbid(factories): +def test_can_create_track_from_file_metadata_distinct_release_mbid(factories, mocker): """Cf https://dev.funkwhale.audio/funkwhale/funkwhale/issues/772""" - artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist) - track = factories["music.Track"](album=album, artist=artist) + artist_credit = factories["music.ArtistCredit"]() + album = factories["music.Album"](artist_credit=artist_credit) + track = factories["music.Track"](album=album, artist_credit=artist_credit) metadata = { - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [ + { + "credit": artist_credit.artist.name, + "mbid": artist_credit.artist.mbid, + "joinphrase": "", + } + ], "album": {"title": album.title, "mbid": str(uuid.uuid4())}, "title": track.title, "position": 4, "fid": "https://hello", } + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": artist_credit.artist.mbid, + "name": artist_credit.artist.name, + }, + "name": artist_credit.artist.name, + "joinphrase": "", + }, + ] + } + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) new_track = tasks.get_track_from_import_metadata(metadata) # the returned track should be different from the existing one, and mapped @@ -241,18 +416,38 @@ def test_can_create_track_from_file_metadata_distinct_release_mbid(factories): assert new_track != track -def test_can_create_track_from_file_metadata_distinct_position(factories): +def test_can_create_track_from_file_metadata_distinct_position(factories, mocker): """Cf https://dev.funkwhale.audio/funkwhale/funkwhale/issues/740""" - artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist) - track = factories["music.Track"](album=album, artist=artist) + artist_credit = factories["music.ArtistCredit"]() + album = factories["music.Album"](artist_credit=artist_credit) + track = factories["music.Track"](album=album, artist_credit=artist_credit) metadata = { - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [ + { + "credit": artist_credit.artist.name, + "joinphrase": "", + "mbid": artist_credit.artist.mbid, + } + ], "album": {"title": album.title, "mbid": album.mbid}, "title": track.title, "position": track.position + 1, } - + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": artist_credit.artist.mbid, + "name": artist_credit.artist.name, + }, + "name": artist_credit.artist.name, + "joinphrase": "", + }, + ] + } + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) new_track = tasks.get_track_from_import_metadata(metadata) assert new_track != track @@ -260,19 +455,32 @@ def test_can_create_track_from_file_metadata_distinct_position(factories): def test_can_create_track_from_file_metadata_federation(factories, mocker): metadata = { - "artists": [ - {"name": "Artist", "fid": "https://artist.fid", "fdate": timezone.now()} + "artist_credit": [ + { + "credit": "Artist", + "artist": { + "name": "Artist", + "fid": "https://artist.fid", + "fdate": timezone.now(), + }, + "credit": "Artist", + "joinphrase": "", + } ], "album": { "title": "Album", "fid": "https://album.fid", "fdate": timezone.now(), "cover_data": {"url": "https://cover/hello.png", "mimetype": "image/png"}, - "artists": [ + "artist_credit": [ { - "name": "Album artist", - "fid": "https://album.artist.fid", - "fdate": timezone.now(), + "credit": "Album artist", + "artist": { + "name": "Album artist", + "fid": "https://album.artist.fid", + "fdate": timezone.now(), + }, + "joinphrase": "", } ], }, @@ -297,12 +505,30 @@ def test_can_create_track_from_file_metadata_federation(factories, mocker): 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"] + assert ( + track.album.artist_credit.all()[0].artist.fid + == metadata["album"]["artist_credit"][0]["artist"].fid + ) + assert ( + track.album.artist_credit.all()[0].artist.name + == metadata["album"]["artist_credit"][0]["credit"] + ) + assert ( + track.album.artist_credit.all()[0].artist.creation_date + == metadata["album"]["artist_credit"][0]["artist"].creation_date + ) + assert ( + track.artist_credit.all()[0].artist.fid + == metadata["artist_credit"][0]["artist"].fid + ) + assert ( + track.artist_credit.all()[0].artist.name + == metadata["artist_credit"][0]["credit"] + ) + assert ( + track.artist_credit.all()[0].artist.creation_date + == metadata["artist_credit"][0]["artist"].creation_date + ) def test_sort_candidates(factories): @@ -616,22 +842,28 @@ def test_federation_audio_track_to_metadata(now, mocker): "attributedTo": "http://album.attributed", "content": "album desc", "mediaType": "text/plain", - "artists": [ + "artist_credit": [ { - "type": "Artist", - "published": published.isoformat(), - "id": "http://hello.artist", - "name": "John Smith", - "content": "album artist desc", - "mediaType": "text/markdown", - "musicbrainzId": str(uuid.uuid4()), - "attributedTo": "http://album-artist.attributed", - "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], - "image": { - "type": "Link", - "href": "http://cover.test/album-artist", - "mediaType": "image/png", + "artist": { + "type": "Artist", + "published": published.isoformat(), + "id": "http://hello.artist", + "name": "John Smith", + "content": "album artist desc", + "mediaType": "text/markdown", + "musicbrainzId": str(uuid.uuid4()), + "attributedTo": "http://album-artist.attributed", + "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], + "image": { + "type": "Link", + "href": "http://cover.test/album-artist", + "mediaType": "image/png", + }, }, + "joinphrase": "", + "id": "http://lol.fr", + "published": published.isoformat(), + "credit": "John Smith", } ], "image": { @@ -640,22 +872,28 @@ def test_federation_audio_track_to_metadata(now, mocker): "mediaType": "image/png", }, }, - "artists": [ + "artist_credit": [ { - "published": published.isoformat(), - "type": "Artist", - "id": "http://hello.trackartist", - "name": "Bob Smith", - "content": "artist desc", - "mediaType": "text/html", - "musicbrainzId": str(uuid.uuid4()), - "attributedTo": "http://artist.attributed", - "tag": [{"type": "Hashtag", "name": "ArtistTag"}], - "image": { - "type": "Link", - "href": "http://cover.test/artist", - "mediaType": "image/png", + "artist": { + "published": published.isoformat(), + "type": "Artist", + "id": "http://hello.trackartist", + "name": "Bob Smith", + "content": "artist desc", + "mediaType": "text/html", + "musicbrainzId": str(uuid.uuid4()), + "attributedTo": "http://artist.attributed", + "tag": [{"type": "Hashtag", "name": "ArtistTag"}], + "image": { + "type": "Link", + "href": "http://cover.test/artist", + "mediaType": "image/png", + }, }, + "joinphrase": "", + "id": "http://loli.fr", + "published": published.isoformat(), + "credit": "Bob Smith", } ], } @@ -690,51 +928,61 @@ def test_federation_audio_track_to_metadata(now, mocker): "mimetype": serializer.validated_data["album"]["image"]["mediaType"], "url": serializer.validated_data["album"]["image"]["href"], }, - "artists": [ + "artist_credit": [ { - "name": a["name"], - "mbid": a["musicbrainzId"], - "fid": a["id"], - "attributed_to": references["http://album-artist.attributed"], - "fdate": serializer.validated_data["album"]["artists"][i][ + "artist": { + "name": a["artist"]["name"], + "mbid": a["artist"]["musicbrainzId"], + "fid": a["artist"]["id"], + "attributed_to": references["http://album-artist.attributed"], + "fdate": serializer.validated_data["album"]["artist_credit"][i][ + "artist" + ]["published"], + "description": { + "content_type": "text/markdown", + "text": "album artist desc", + }, + "tags": ["AlbumArtistTag"], + "cover_data": { + "mimetype": serializer.validated_data["album"][ + "artist_credit" + ][i]["artist"]["image"]["mediaType"], + "url": serializer.validated_data["album"]["artist_credit"][ + i + ]["artist"]["image"]["href"], + }, + }, + "joinphrase": a["joinphrase"], + "credit": a["artist"]["name"], + } + for i, a in enumerate(payload["album"]["artist_credit"]) + ], + }, + "artist_credit": [ + { + "artist": { + "name": a["artist"]["name"], + "mbid": a["artist"]["musicbrainzId"], + "fid": a["artist"]["id"], + "fdate": serializer.validated_data["artist_credit"][i]["artist"][ "published" ], - "description": { - "content_type": "text/markdown", - "text": "album artist desc", - }, - "tags": ["AlbumArtistTag"], + "attributed_to": references["http://artist.attributed"], + "tags": ["ArtistTag"], + "description": {"content_type": "text/html", "text": "artist desc"}, "cover_data": { - "mimetype": serializer.validated_data["album"]["artists"][i][ - "image" - ]["mediaType"], - "url": serializer.validated_data["album"]["artists"][i][ + "mimetype": serializer.validated_data["artist_credit"][i][ + "artist" + ]["image"]["mediaType"], + "url": serializer.validated_data["artist_credit"][i]["artist"][ "image" ]["href"], }, - } - 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"], - "attributed_to": references["http://artist.attributed"], - "tags": ["ArtistTag"], - "description": {"content_type": "text/html", "text": "artist desc"}, - "cover_data": { - "mimetype": serializer.validated_data["artists"][i]["image"][ - "mediaType" - ], - "url": serializer.validated_data["artists"][i]["image"]["href"], }, + "joinphrase": "", + "credit": a["artist"]["name"], } - for i, a in enumerate(payload["artists"]) + for i, a in enumerate(payload["artist_credit"]) ], } @@ -918,8 +1166,8 @@ def test_get_prunable_artists(factories): # non prunable artist non_prunable_artist = factories["music.Artist"]() non_prunable_album_artist = factories["music.Artist"]() - factories["music.Track"](artist=non_prunable_artist) - factories["music.Track"](album__artist=non_prunable_album_artist) + factories["music.Track"](artist_credit__artist=non_prunable_artist) + factories["music.Track"](album__artist_credit__artist=non_prunable_album_artist) assert list(tasks.get_prunable_artists()) == [prunable_artist] @@ -981,7 +1229,7 @@ def test_get_track_from_import_metadata_with_forced_values(factories, mocker, fa } metadata = { "title": "Test track", - "artists": [{"name": "Test artist"}], + "artist_credit": [{"name": "Test artist"}], "album": {"title": "Test album", "release_date": datetime.date(2012, 8, 15)}, "position": 4, "disc_number": 2, @@ -997,7 +1245,7 @@ def test_get_track_from_import_metadata_with_forced_values(factories, mocker, fa assert track.disc_number == metadata["disc_number"] assert track.copyright == forced_values["copyright"] assert track.album == forced_values["album"] - assert track.artist == forced_values["artist"] + assert track.artist_credit.all()[0].artist == forced_values["artist"] assert track.attributed_to == forced_values["attributed_to"] assert track.license == forced_values["license"] assert ( @@ -1010,7 +1258,9 @@ def test_get_track_from_import_metadata_with_forced_values_album( factories, mocker, faker ): channel = factories["audio.Channel"]() - album = factories["music.Album"](artist=channel.artist, with_cover=True) + album = factories["music.Album"]( + artist_credit__artist=channel.artist, with_cover=True + ) forced_values = { "title": "Real title", @@ -1019,13 +1269,14 @@ def test_get_track_from_import_metadata_with_forced_values_album( upload = factories["music.Upload"]( import_metadata=forced_values, library=channel.library, track=None ) + tasks.process_upload(upload_id=upload.pk) upload.refresh_from_db() assert upload.import_status == "finished" assert upload.track.title == forced_values["title"] assert upload.track.album == album - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist def test_process_channel_upload_forces_artist_and_attributed_to( @@ -1073,7 +1324,7 @@ def test_process_channel_upload_forces_artist_and_attributed_to( assert upload.track.position == import_metadata["position"] assert upload.track.copyright == import_metadata["copyright"] assert upload.track.get_tags() == import_metadata["tags"] - assert upload.track.artist == channel.artist + assert upload.track.artist_credit.all()[0].artist == channel.artist assert upload.track.attributed_to == channel.attributed_to assert upload.track.attachment_cover == attachment @@ -1196,24 +1447,58 @@ def test_can_download_image_file_for_album_mbid(binary_cover, mocker, factories) def test_can_import_track_with_same_mbid_in_different_albums(factories, mocker): artist = factories["music.Artist"]() + artist_credit = factories["music.ArtistCredit"](artist=artist) upload = factories["music.Upload"]( - playable=True, track__artist=artist, track__album__artist=artist + playable=True, + track__artist_credit=artist_credit, + track__album__artist_credit=artist_credit, ) assert upload.track.mbid is not None data = { "title": upload.track.title, - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [{"credit": artist.name, "mbid": artist.mbid}], "album": { "title": "The Slip", "mbid": uuid.UUID("12b57d46-a192-499e-a91f-7da66790a1c1"), "release_date": datetime.date(2008, 5, 5), - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [{"credit": artist.name, "mbid": artist.mbid}], }, "position": 1, "disc_number": 1, "mbid": upload.track.mbid, } + mb_ac = { + "artist-credit": [ + { + "artist": { + "id": artist.mbid, + "name": artist.name, + }, + "joinphrase": "", + "name": artist.name, + }, + ] + } + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": artist.mbid, + "name": artist.name, + }, + "name": artist.name, + "joinphrase": "", + }, + ] + } + + mocker.patch.object( + tasks.musicbrainz.api.recordings, "get", return_value={"recording": mb_ac} + ) + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data) mocker.patch.object(tasks, "populate_album_cover") @@ -1228,17 +1513,21 @@ def test_can_import_track_with_same_mbid_in_different_albums(factories, mocker): def test_import_track_with_same_mbid_in_same_albums_skipped(factories, mocker): artist = factories["music.Artist"]() + artist_credit = factories["music.ArtistCredit"](artist=artist) + upload = factories["music.Upload"]( - playable=True, track__artist=artist, track__album__artist=artist + playable=True, + track__artist_credit=artist_credit, + track__album__artist_credit=artist_credit, ) assert upload.track.mbid is not None data = { "title": upload.track.title, - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [{"name": artist.name, "mbid": artist.mbid}], "album": { "title": upload.track.album.title, "mbid": upload.track.album.mbid, - "artists": [{"name": artist.name, "mbid": artist.mbid}], + "artist_credit": [{"name": artist.name, "mbid": artist.mbid}], }, "position": 1, "disc_number": 1, @@ -1259,29 +1548,42 @@ def test_import_track_with_same_mbid_in_same_albums_skipped(factories, mocker): def test_can_import_track_with_same_position_in_different_discs(factories, mocker): upload = factories["music.Upload"](playable=True) - artist_data = [ + artist_credit_data = [ { - "name": upload.track.album.artist.name, - "mbid": upload.track.album.artist.mbid, + "credit": upload.track.album.artist_credit.all()[0].artist.name, + "mbid": upload.track.album.artist_credit.all()[0].artist.mbid, + "joinphrase": "", } ] data = { "title": upload.track.title, - "artists": artist_data, + "artist_credit": artist_credit_data, "album": { "title": "The Slip", "mbid": upload.track.album.mbid, "release_date": datetime.date(2008, 5, 5), - "artists": artist_data, + "artist_credit": artist_credit_data, }, "position": upload.track.position, "disc_number": 2, "mbid": None, } - + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": upload.track.album.artist_credit.all()[0].artist.mbid, + "name": upload.track.album.artist_credit.all()[0].artist.name, + }, + "joinphrase": "", + }, + ] + } mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data) mocker.patch.object(tasks, "populate_album_cover") - + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) new_upload = factories["music.Upload"](library=upload.library) tasks.process_upload(upload_id=new_upload.pk) @@ -1292,27 +1594,46 @@ def test_can_import_track_with_same_position_in_different_discs(factories, mocke def test_can_import_track_with_same_position_in_same_discs_skipped(factories, mocker): - upload = factories["music.Upload"](playable=True) + ac = factories["music.ArtistCredit"](joinphrase="", index=0) + upload = factories["music.Upload"]( + playable=True, track__artist_credit=ac, track__album__artist_credit=ac + ) artist_data = [ { - "name": upload.track.album.artist.name, - "mbid": upload.track.album.artist.mbid, + "credit": upload.track.album.artist_credit.all()[0].artist.name, + "mbid": upload.track.album.artist_credit.all()[0].artist.mbid, + "joinphrase": "", } ] + data = { "title": upload.track.title, - "artists": artist_data, + "artist_credit": artist_data, "album": { "title": "The Slip", "mbid": upload.track.album.mbid, "release_date": datetime.date(2008, 5, 5), - "artists": artist_data, + "artist_credit": artist_data, }, "position": upload.track.position, "disc_number": upload.track.disc_number, "mbid": None, } - + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": upload.track.album.artist_credit.all()[0].artist.mbid, + "name": upload.track.album.artist_credit.all()[0].artist.name, + }, + "name": upload.track.album.artist_credit.all()[0].artist.name, + "joinphrase": "", + }, + ] + } + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) mocker.patch.object(metadata.TrackMetadataSerializer, "validated_data", data) mocker.patch.object(tasks, "populate_album_cover") @@ -1325,8 +1646,50 @@ def test_can_import_track_with_same_position_in_same_discs_skipped(factories, mo assert new_upload.import_status == "skipped" -def test_update_track_metadata(factories): +def test_update_track_metadata_no_mbid(factories): track = factories["music.Track"]() + 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": "4", + "disc_number": "2", + "license": "Dummy license: http://creativecommons.org/licenses/by-sa/4.0/", + "copyright": "Someone", + "comment": "hello there", + "genre": "classical", + } + + tasks.update_track_metadata(metadata.FakeMetadata(data), track) + + track.refresh_from_db() + + assert track.title == data["title"] + assert track.position == int(data["position"]) + assert track.disc_number == int(data["disc_number"]) + assert track.license.code == "cc-by-sa-4.0" + assert track.copyright == data["copyright"] + assert track.album.title == data["album"] + assert track.album.release_date == datetime.date(2012, 8, 15) + assert track.get_artist_credit_string == data["artist"] + assert track.artist_credit.all()[0].artist.mbid is None + assert ( + track.album.get_artist_credit_string + == "Edvard Grieg; Musopen Symphony Orchestra" + ) + assert track.album.artist_credit.all()[0].artist.mbid is None + assert sorted(track.tagged_items.values_list("tag__name", flat=True)) == [ + "classical" + ] + + +def test_update_track_metadata_mbid(factories, mocker): + track = factories["music.Track"]() + factories["music.Artist"]( + name="Edvard Grieg", mbid="013c8e5b-d72a-4cd3-8dee-6c64d6125823" + ) data = { "title": "Peer Gynt Suite no. 1, op. 46: I. Morning", "artist": "Edvard Grieg", @@ -1344,6 +1707,45 @@ def test_update_track_metadata(factories): "comment": "hello there", "genre": "classical", } + mb_ac = { + "artist-credit": [ + { + "artist": { + "id": "013c8e5b-d72a-4cd3-8dee-6c64d6125823", + "name": "Edvard Grieg", + }, + "joinphrase": "", + "name": "Edvard Grieg", + } + ] + } + + mb_ac_album = { + "artist-credit": [ + { + "artist": { + "id": "013c8e5b-d72a-4cd3-8dee-6c64d6125823", + "name": "Edvard Grieg", + }, + "joinphrase": "; ", + }, + { + "artist": { + "id": "5b4d7d2d-36df-4b38-95e3-a964234f520f", + "name": "Musopen Symphony Orchestra", + }, + "joinphrase": "", + "name": "Musopen Symphony Orchestra", + }, + ] + } + + mocker.patch.object( + tasks.musicbrainz.api.releases, "get", return_value={"recording": mb_ac_album} + ) + mocker.patch.object( + tasks.musicbrainz.api.recordings, "get", return_value={"recording": mb_ac} + ) tasks.update_track_metadata(metadata.FakeMetadata(data), track) track.refresh_from_db() @@ -1357,10 +1759,19 @@ def test_update_track_metadata(factories): assert track.album.title == data["album"] assert track.album.release_date == datetime.date(2012, 8, 15) assert str(track.album.mbid) == data["musicbrainz_albumid"] - assert track.artist.name == data["artist"] - assert str(track.artist.mbid) == data["musicbrainz_artistid"] - assert track.album.artist.name == "Edvard Grieg" - assert str(track.album.artist.mbid) == "013c8e5b-d72a-4cd3-8dee-6c64d6125823" + assert track.get_artist_credit_string == data["artist"] + assert ( + str(track.artist_credit.all()[0].artist.mbid) + == "013c8e5b-d72a-4cd3-8dee-6c64d6125823" + ) + assert ( + track.album.get_artist_credit_string + == "Edvard Grieg; Musopen Symphony Orchestra" + ) + assert ( + str(track.album.artist_credit.all()[0].artist.mbid) + == "013c8e5b-d72a-4cd3-8dee-6c64d6125823" + ) assert sorted(track.tagged_items.values_list("tag__name", flat=True)) == [ "classical" ] @@ -1450,3 +1861,192 @@ def test_upload_checks_mbid_tag_pass(temp_signal, factories, mocker, preferences upload.refresh_from_db() assert upload.import_status == "finished" + + +@pytest.mark.parametrize( + "raw_string, expected", + [ + ( + "The Kinks|Various Artists", + [("The Kinks", "|", 0, None), ("Various Artists", "", 1, None)], + ), + ( + "The Kinks,Various Artists", + [("The Kinks", ",", 0, None), ("Various Artists", "", 1, None)], + ), + ( + "Luigi 21 Plus feat. Ñejo feat Ñengo Flow & Chyno Nyno with Linkin Park and Evanescance", + [ + ("Luigi 21 Plus", " feat. ", 0, None), + ("Ñejo", " feat ", 1, None), + ("Ñengo Flow", " & ", 2, None), + ("Chyno Nyno", " with ", 3, None), + ("Linkin Park", " and ", 4, None), + ("Evanescance", "", 5, None), + ], + ), + ( + "Bad Bunny x Poeta Callejero ; Mark B (Carlos Serrano & Carlos Martin Mambo Remix)", + [ + ("Bad Bunny", " x ", 0, None), + ("Poeta Callejero", " ; ", 1, None), + ("Mark B", " (", 2, None), + ("Carlos Serrano", " & ", 3, None), + ("Carlos Martin Mambo", " Remix)", 4, None), + ], + ), + ], +) +def test_can_parse_multiples_artist(raw_string, expected): + artist_credit = tasks.parse_credits(raw_string, None, None) + assert artist_credit == expected + + +def test_get_best_candidate_or_create_find_artist_credit(factories): + track = factories["music.Track"]() + query = Q( + title__iexact=track.title, + artist_credit__in=track.artist_credit.all(), + position=track.position, + disc_number=track.disc_number, + ) + defaults = "lol" + tasks.get_best_candidate_or_create( + models.Track, query, defaults=defaults, sort_fields=["mbid", "fid"] + ) + + +def test_get_or_create_artists_credits_from_musicbrainz(factories, mocker): + release_mb_response = { + "status": "Official", + "status-id": "4e304316-386d-3409-af2e-78857eec5cfe", + "country": "XW", + "text-representation": {"script": "Latn", "language": "spa"}, + "release-events": [ + { + "date": "2019-05-30", + "area": { + "sort-name": "[Worldwide]", + "id": "525d4e18-3d00-31b9-a58b-a146a916de8f", + "disambiguation": "", + "iso-3166-1-codes": ["XW"], + "name": "[Worldwide]", + }, + } + ], + "disambiguation": "", + "cover-art-archive": { + "front": True, + "count": 1, + "back": False, + "darkened": False, + "artwork": True, + }, + "id": "48cc978e-17b8-46ab-91e8-3dceef2725b5", + "packaging-id": "119eba76-b343-3e02-a292-f0f00644bb9b", + "packaging": "None", + "date": "2019-05-30", + "title": "#TBT", + "artist-credit": [ + { + "joinphrase": "", + "artist": { + "type-id": "b6e035f4-3ce9-331c-97df-83397230b0df", + "disambiguation": 'Hiram David Santos Rojas, reggaeton artist aka "Lui-G 21+"', + "name": "Luigi 21 Plus", + "type": "Person", + "sort-name": "Luigi 21 Plus", + "id": "f1642d37-bbe2-4aff-a75e-86845ff49fa4", + }, + "name": "Luigi 21 Plus", + } + ], + "quality": "normal", + } + recording_mb_response = { + "length": 337000, + "first-release-date": "2019-05-30", + "disambiguation": "", + "id": "cf3dacb7-3cee-430f-b0bb-cc4557158a03", + "title": "Mueve ese culo puñeta", + "artist-credit": [ + { + "joinphrase": " feat. ", + "artist": { + "type": "Person", + "id": "f1642d37-bbe2-4aff-a75e-86845ff49fa4", + "sort-name": "Luigi 21 Plus", + "type-id": "b6e035f4-3ce9-331c-97df-83397230b0df", + "name": "Luigi 21 Plus", + "disambiguation": 'Hiram David Santos Rojas, reggaeton artist aka "Lui-G 21+"', + }, + "name": "Luigi 21 Plus", + }, + { + "name": "Ñejo", + "artist": { + "type": "Person", + "id": "8248c905-689d-4e36-9def-7c515c5ef5eb", + "name": "Ñejo", + "disambiguation": "", + "sort-name": "Ñejo", + "type-id": "b6e035f4-3ce9-331c-97df-83397230b0df", + }, + "joinphrase": ", ", + }, + { + "joinphrase": " & ", + "artist": { + "type": "Person", + "id": "b7f5054e-c9de-49d8-b0eb-6deefb89b86b", + "name": "Ñengo Flow", + "disambiguation": "", + "type-id": "b6e035f4-3ce9-331c-97df-83397230b0df", + "sort-name": "Ñengo Flow", + }, + "name": "Ñengo Flow", + }, + { + "joinphrase": "", + "artist": { + "id": "3d50191b-820c-4f6f-b25a-bc12d63e6718", + "type": "Person", + "type-id": "b6e035f4-3ce9-331c-97df-83397230b0df", + "sort-name": "Chyno Nyno", + "disambiguation": "", + "name": "Chyno Nyno", + }, + "name": "Chyno Nyno", + }, + ], + "video": False, + } + for mb_type, mb_response in [ + ("release", release_mb_response), + ("recording", recording_mb_response), + ]: + mocker.patch.object( + tasks.musicbrainz.api.releases, + "get", + return_value={"recording": mb_response}, + ) + mocker.patch.object( + tasks.musicbrainz.api.recordings, + "get", + return_value={"recording": mb_response}, + ) + tasks.get_or_create_artists_credits_from_musicbrainz( + mb_type, mb_response["id"], None, None + ) + for i, ac in enumerate(mb_response["artist-credit"]): + ac = models.ArtistCredit.objects.get( + artist__name=ac["artist"]["name"], + joinphrase=ac["joinphrase"], + credit=ac["name"], + ) + + assert ac.artist.name == mb_response["artist-credit"][i]["artist"]["name"] + assert ( + str(ac.artist.mbid) == mb_response["artist-credit"][i]["artist"]["id"] + ) + assert ac.joinphrase == mb_response["artist-credit"][i]["joinphrase"] diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index dae72d2f8..070ed9e78 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -7,7 +7,7 @@ import uuid import magic import pytest -from django.db.models import Prefetch +from django.db.models import Count, Prefetch from django.urls import reverse from django.utils import timezone @@ -26,12 +26,12 @@ def test_artist_list_serializer(api_request, factories, logged_in_api_client): track = factories["music.Upload"]( library__privacy_level="everyone", import_status="finished", - track__album__artist__set_tags=tags, + track__album__artist_credit__artist__set_tags=tags, ).track - artist = track.artist + artist = track.artist_credit.all()[0].artist request = api_request.get("/") - qs = artist.__class__.objects.with_albums().prefetch_related( - Prefetch("tracks", to_attr="_prefetched_tracks") + qs = artist.__class__.objects.with_albums().annotate( + _tracks_count=Count("artist_credit__tracks") ) serializer = serializers.ArtistWithAlbumsSerializer( qs, many=True, context={"request": request} @@ -39,12 +39,11 @@ def test_artist_list_serializer(api_request, factories, logged_in_api_client): expected = {"count": 1, "next": None, "previous": None, "results": serializer.data} for artist in serializer.data: artist["tags"] = tags - for album in artist["albums"]: - album["is_playable"] = True url = reverse("api:v1:artists-list") response = logged_in_api_client.get(url) + assert serializer.data[0]["tracks_count"] == 1 assert response.status_code == 200 assert response.data == expected @@ -118,7 +117,9 @@ def test_artist_view_filter_playable(param, expected, factories, api_request): "empty": factories["music.Artist"](), "full": factories["music.Upload"]( library__privacy_level="everyone", import_status="finished" - ).track.artist, + ) + .track.artist_credit.all()[0] + .artist, } request = api_request.get("/", {"playable": param}) @@ -817,7 +818,7 @@ def test_user_can_create_upload_in_channel( channel = factories["audio.Channel"](attributed_to=actor) url = reverse("api:v1:uploads-list") m = mocker.patch("funkwhale_api.common.utils.on_commit") - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) response = logged_in_api_client.post( url, { @@ -968,12 +969,14 @@ def test_can_get_libraries_for_music_entities( library = upload.library setattr(library, "_uploads_count", 1) data = { - "artist": upload.track.artist, + "artist": upload.track.artist_credit.all()[0].artist, "album": upload.track.album, "track": upload.track, } # libraries in channel should be missing excluded - channel = factories["audio.Channel"](artist=upload.track.artist) + channel = factories["audio.Channel"]( + artist=upload.track.artist_credit.all()[0].artist + ) factories["music.Upload"]( library=channel.library, playable=True, track=upload.track ) @@ -1035,7 +1038,7 @@ def test_oembed_track(factories, no_api_auth, api_client, settings): "provider_url": settings.FUNKWHALE_URL, "height": 150, "width": 600, - "title": f"{track.title} by {track.artist.name}", + "title": f"{track.title} by {track.artist_credit.all()[0].artist.name}", "description": track.full_name, "thumbnail_url": federation_utils.full_url( track.album.attachment_cover.file.crop["200x200"].url @@ -1045,9 +1048,11 @@ def test_oembed_track(factories, no_api_auth, api_client, settings): "html": ''.format( iframe_src ), - "author_name": track.artist.name, + "author_name": track.artist_credit.all()[0].artist.name, "author_url": federation_utils.full_url( - utils.spa_reverse("library_artist", kwargs={"pk": track.artist.pk}) + utils.spa_reverse( + "library_artist", kwargs={"pk": track.artist_credit.all()[0].artist.pk} + ) ), } @@ -1071,8 +1076,8 @@ def test_oembed_album(factories, no_api_auth, api_client, settings): "provider_url": settings.FUNKWHALE_URL, "height": 400, "width": 600, - "title": f"{album.title} by {album.artist.name}", - "description": f"{album.title} by {album.artist.name}", + "title": f"{album.title} by {album.artist_credit.all()[0].artist.name}", + "description": f"{album.title} by {album.artist_credit.all()[0].artist.name}", "thumbnail_url": federation_utils.full_url( album.attachment_cover.file.crop["200x200"].url ), @@ -1081,9 +1086,11 @@ def test_oembed_album(factories, no_api_auth, api_client, settings): "html": ''.format( iframe_src ), - "author_name": album.artist.name, + "author_name": album.artist_credit.all()[0].artist.name, "author_url": federation_utils.full_url( - utils.spa_reverse("library_artist", kwargs={"pk": album.artist.pk}) + utils.spa_reverse( + "library_artist", kwargs={"pk": album.artist_credit.all()[0].artist.pk} + ) ), } @@ -1097,7 +1104,7 @@ def test_oembed_artist(factories, no_api_auth, api_client, settings): settings.FUNKWHALE_EMBED_URL = "http://embed" track = factories["music.Track"](album__with_cover=True) album = track.album - artist = track.artist + artist = track.artist_credit.all()[0].artist url = reverse("api:v1:oembed") artist_url = f"https://test.com/library/artists/{artist.pk}" iframe_src = f"http://embed?type=artist&id={artist.pk}" @@ -1281,7 +1288,7 @@ def test_artist_list_exclude_channels( ) def test_album_list_exclude_channels(params, expected, factories, logged_in_api_client): channel_artist = factories["audio.Channel"]().artist - factories["music.Album"](artist=channel_artist) + factories["music.Album"](artist_credit__artist=channel_artist) url = reverse("api:v1:albums-list") response = logged_in_api_client.get(url, params) @@ -1296,7 +1303,7 @@ def test_album_list_exclude_channels(params, expected, factories, logged_in_api_ ) def test_track_list_exclude_channels(params, expected, factories, logged_in_api_client): channel_artist = factories["audio.Channel"]().artist - factories["music.Track"](artist=channel_artist) + factories["music.Track"](artist_credit__artist=channel_artist) url = reverse("api:v1:tracks-list") response = logged_in_api_client.get(url, params) @@ -1404,10 +1411,12 @@ def test_channel_owner_can_create_album(factories, logged_in_api_client): actor = logged_in_api_client.user.create_actor() channel = factories["audio.Channel"](attributed_to=actor, artist__with_cover=True) attachment = factories["common.Attachment"](actor=actor) + ac = factories["music.ArtistCredit"](artist=channel.artist) + url = reverse("api:v1:albums-list") data = { - "artist": channel.artist.pk, + "artist_credit": ac.pk, "cover": attachment.uuid, "title": "Hello world", "release_date": "2019-01-02", @@ -1417,10 +1426,9 @@ def test_channel_owner_can_create_album(factories, logged_in_api_client): response = logged_in_api_client.post(url, data, format="json") - assert response.status_code == 201 - - album = channel.artist.albums.get(title=data["title"]) + assert response.status_code == 204 + album = channel.artist.artist_credit.albums().get(title=data["title"]) assert ( response.data == serializers.AlbumSerializer(album, context={"description": True}).data @@ -1437,7 +1445,7 @@ def test_channel_owner_can_delete_album(factories, logged_in_api_client, mocker) dispatch = mocker.patch("funkwhale_api.federation.routes.outbox.dispatch") actor = logged_in_api_client.user.create_actor() channel = factories["audio.Channel"](attributed_to=actor) - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) url = reverse("api:v1:albums-detail", kwargs={"pk": album.pk}) response = logged_in_api_client.delete(url) @@ -1474,7 +1482,7 @@ def test_other_user_cannot_create_album(factories, logged_in_api_client): def test_other_user_cannot_delete_album(factories, logged_in_api_client): logged_in_api_client.user.create_actor() channel = factories["audio.Channel"]() - album = factories["music.Album"](artist=channel.artist) + album = factories["music.Album"](artist_credit__artist=channel.artist) url = reverse("api:v1:albums-detail", kwargs={"pk": album.pk}) response = logged_in_api_client.delete(url) @@ -1487,7 +1495,7 @@ def test_channel_owner_can_delete_track(factories, logged_in_api_client, mocker) dispatch = mocker.patch("funkwhale_api.federation.routes.outbox.dispatch") actor = logged_in_api_client.user.create_actor() channel = factories["audio.Channel"](attributed_to=actor) - track = factories["music.Track"](artist=channel.artist) + track = factories["music.Track"](artist_credit__artist=channel.artist) upload1 = factories["music.Upload"](track=track) upload2 = factories["music.Upload"](track=track) url = reverse("api:v1:tracks-detail", kwargs={"pk": track.pk}) @@ -1506,7 +1514,7 @@ def test_channel_owner_can_delete_track(factories, logged_in_api_client, mocker) def test_other_user_cannot_delete_track(factories, logged_in_api_client): logged_in_api_client.user.create_actor() channel = factories["audio.Channel"]() - track = factories["music.Track"](artist=channel.artist) + track = factories["music.Track"](artist_credit__artist=channel.artist) url = reverse("api:v1:tracks-detail", kwargs={"pk": track.pk}) response = logged_in_api_client.delete(url) @@ -1597,3 +1605,13 @@ def test_fs_import_cancel_already_running( assert response.status_code == 204 assert cache.get("fs-import:status") == "canceled" + + +def test_album_create_artist_credit(factories, logged_in_api_client): + artist = factories["music.Artist"]() + factories["audio.Channel"](artist=artist) + url = reverse("api:v1:albums-list") + response = logged_in_api_client.post( + url, {"artist": artist.pk, "title": "super album"}, format="json" + ) + assert response.status_code == 204 diff --git a/api/tests/playlists/test_filters.py b/api/tests/playlists/test_filters.py index dcf07604d..1b5193c4c 100644 --- a/api/tests/playlists/test_filters.py +++ b/api/tests/playlists/test_filters.py @@ -23,6 +23,8 @@ def test_playlist_filter_artist(factories, queryset_equal_list): plt = factories["playlists.PlaylistTrack"]() factories["playlists.PlaylistTrack"]() qs = models.Playlist.objects.all() - filterset = filters.PlaylistFilter({"artist": plt.track.artist.pk}, queryset=qs) + filterset = filters.PlaylistFilter( + {"artist": plt.track.artist_credit.all()[0].artist.pk}, queryset=qs + ) assert filterset.qs == [plt.playlist] diff --git a/api/tests/radios/test_api.py b/api/tests/radios/test_api.py index 4654f012b..e851d749d 100644 --- a/api/tests/radios/test_api.py +++ b/api/tests/radios/test_api.py @@ -1,3 +1,5 @@ +from itertools import chain + import pytest from django.urls import reverse @@ -20,9 +22,11 @@ def test_can_list_config_options(logged_in_api_client): def test_can_validate_config(logged_in_api_client, factories): artist1 = factories["music.Artist"]() artist2 = factories["music.Artist"]() - factories["music.Track"].create_batch(3, artist=artist1) - factories["music.Track"].create_batch(3, artist=artist2) - candidates = artist1.tracks.order_by("pk") + factories["music.Track"].create_batch(3, artist_credit__artist=artist1) + factories["music.Track"].create_batch(3, artist_credit__artist=artist2) + candidates = list( + chain(*[ac.tracks.order_by("pk") for ac in artist1.artist_credit.all()]) + ) f = {"filters": [{"type": "artist", "ids": [artist1.pk]}]} url = reverse("api:v1:radios:radios-validate") response = logged_in_api_client.post(url, f, format="json") @@ -32,7 +36,7 @@ def test_can_validate_config(logged_in_api_client, factories): payload = response.data expected = { - "count": candidates.count(), + "count": len(candidates), "sample": TrackSerializer(candidates, many=True).data, } diff --git a/api/tests/radios/test_radios.py b/api/tests/radios/test_radios.py index 53c604681..b0919dfd1 100644 --- a/api/tests/radios/test_radios.py +++ b/api/tests/radios/test_radios.py @@ -91,7 +91,9 @@ def test_can_get_choices_for_favorites_radio(factories): def test_can_get_choices_for_custom_radio(factories): artist = factories["music.Artist"]() - files = factories["music.Upload"].create_batch(5, track__artist=artist) + files = factories["music.Upload"].create_batch( + 5, track__artist_credit__artist=artist + ) tracks = [f.track for f in files] factories["music.Upload"].create_batch(5) @@ -208,7 +210,9 @@ def test_can_start_artist_radio(factories): user = factories["users.User"]() artist = factories["music.Artist"]() factories["music.Upload"].create_batch(5) - good_files = factories["music.Upload"].create_batch(5, track__artist=artist) + good_files = factories["music.Upload"].create_batch( + 5, track__artist_credit__artist=artist + ) good_tracks = [f.track for f in good_files] radio = radios.ArtistRadio() @@ -224,7 +228,7 @@ def test_can_start_tag_radio(factories): good_tracks = [ factories["music.Track"](set_tags=[tag.name]), factories["music.Track"](album__set_tags=[tag.name]), - factories["music.Track"](album__artist__set_tags=[tag.name]), + factories["music.Track"](album__artist_credit__artist__set_tags=[tag.name]), ] factories["music.Track"].create_batch(3, set_tags=["notrock"]) @@ -358,7 +362,7 @@ def test_session_radio_get_queryset_ignore_filtered_track_artist( factories, queryset_equal_list ): cf = factories["moderation.UserFilter"](for_artist=True) - factories["music.Track"](artist=cf.target_artist) + factories["music.Track"](artist_credit__artist=cf.target_artist) valid_track = factories["music.Track"]() radio = radios.RandomRadio() radio.start_session(user=cf.user) @@ -370,7 +374,7 @@ def test_session_radio_get_queryset_ignore_filtered_track_album_artist( factories, queryset_equal_list ): cf = factories["moderation.UserFilter"](for_artist=True) - factories["music.Track"](album__artist=cf.target_artist) + factories["music.Track"](album__artist_credit__artist=cf.target_artist) valid_track = factories["music.Track"]() radio = radios.RandomRadio() radio.start_session(user=cf.user) @@ -382,9 +386,11 @@ def test_get_choices_for_custom_radio_exclude_artist(factories): included_artist = factories["music.Artist"]() excluded_artist = factories["music.Artist"]() included_uploads = factories["music.Upload"].create_batch( - 5, track__artist=included_artist + 5, track__artist_credit__artist=included_artist + ) + factories["music.Upload"].create_batch( + 5, track__artist_credit__artist=excluded_artist ) - factories["music.Upload"].create_batch(5, track__artist=excluded_artist) session = factories["radios.CustomRadioSession"]( custom_radio__config=[ @@ -422,7 +428,9 @@ def test_can_start_custom_multiple_radio_from_api(api_client, factories): map_filters_to_type = {"tags": "names", "artists": "ids", "playlists": "names"} for key, value in map_filters_to_type.items(): attr = value[:-1] - track_filter_key = [getattr(a.artist, attr) for a in tracks] + track_filter_key = [ + getattr(a.artist_credit.all()[0].artist, attr) for a in tracks + ] config = {"filters": [{"type": key, value: track_filter_key}]} response = api_client.post( url, diff --git a/api/tests/radios/test_radios_v2.py b/api/tests/radios/test_radios_v2.py index 85fc15db6..76c9a2e15 100644 --- a/api/tests/radios/test_radios_v2.py +++ b/api/tests/radios/test_radios_v2.py @@ -95,7 +95,9 @@ def test_can_get_choices_for_favorites_radio_v2(factories): def test_can_get_choices_for_custom_radio_v2(factories): artist = factories["music.Artist"]() - files = factories["music.Upload"].create_batch(5, track__artist=artist) + files = factories["music.Upload"].create_batch( + 5, track__artist_credit__artist=artist + ) tracks = [f.track for f in files] factories["music.Upload"].create_batch(5) diff --git a/api/tests/subsonic/test_serializers.py b/api/tests/subsonic/test_serializers.py index 507b57bd5..d2286ee7e 100644 --- a/api/tests/subsonic/test_serializers.py +++ b/api/tests/subsonic/test_serializers.py @@ -37,7 +37,7 @@ def test_get_valid_filepart(input, expected): [ ( { - "artist__name": "Hello", + "artist_credit__artist__name": "Hello", "album__title": "World", "title": "foo", "position": None, @@ -47,7 +47,7 @@ def test_get_valid_filepart(input, expected): ), ( { - "artist__name": "AC/DC", + "artist_credit__artist__name": "AC/DC", "album__title": "escape/my", "title": "sla/sh", "position": 12, @@ -76,8 +76,8 @@ def test_get_artists_serializer(factories): name="", with_cover=False ) # Shouldn't be serialised - factories["music.Album"].create_batch(size=3, artist=artist1) - factories["music.Album"].create_batch(size=2, artist=artist2) + factories["music.Album"].create_batch(size=3, artist_credit__artist=artist1) + factories["music.Album"].create_batch(size=2, artist_credit__artist=artist2) expected = { "ignoredArticles": "", @@ -125,7 +125,7 @@ def test_get_artists_serializer(factories): def test_get_artist_serializer(factories): artist = factories["music.Artist"](with_cover=True) - album = factories["music.Album"](artist=artist, with_cover=True) + album = factories["music.Album"](artist_credit__artist=artist, with_cover=True) tracks = factories["music.Track"].create_batch(size=3, album=album) expected = { @@ -191,7 +191,7 @@ def test_get_track_data_content_type(mimetype, extension, expected, factories): def test_get_album_serializer(factories): artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist, with_cover=True) + album = factories["music.Album"](artist_credit__artist=artist, with_cover=True) track = factories["music.Track"](album=album, disc_number=42) upload = factories["music.Upload"](track=track, bitrate=42000, duration=43, size=44) tagged_item = factories["tags.TaggedItem"](content_object=album, tag__name="foo") @@ -243,8 +243,8 @@ def test_get_album_serializer(factories): def test_starred_tracks2_serializer(factories): - artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist) + artist_credit = factories["music.ArtistCredit"]() + album = factories["music.Album"](artist_credit=artist_credit) track = factories["music.Track"](album=album) upload = factories["music.Upload"](track=track) favorite = factories["favorites.TrackFavorite"](track=track) @@ -347,7 +347,7 @@ def test_channel_episode_serializer(factories): description = factories["common.Content"]() channel = factories["audio.Channel"]() track = factories["music.Track"]( - description=description, artist=channel.artist, with_cover=True + description=description, artist_credit__artist=channel.artist, with_cover=True ) upload = factories["music.Upload"]( playable=True, track=track, bitrate=128000, duration=42 diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index c1a3245dc..a85075795 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -152,7 +152,9 @@ def test_get_artist( url = reverse("api:subsonic:subsonic-get_artist") assert url.endswith("getArtist") is True artist = factories["music.Artist"](playable=True) - factories["music.Album"].create_batch(size=3, artist=artist, playable=True) + factories["music.Album"].create_batch( + size=3, artist_credit__artist=artist, playable=True + ) playable_by = mocker.spy(music_models.ArtistQuerySet, "playable_by") expected = {"artist": serializers.GetArtistSerializer(artist).data} @@ -202,9 +204,9 @@ def test_get_album( ): url = reverse("api:subsonic:subsonic-get_album") assert url.endswith("getAlbum") is True - artist = factories["music.Artist"]() + artist_credit = factories["music.ArtistCredit"]() album = ( - factories["music.Album"](artist=artist) + factories["music.Album"](artist_credit=artist_credit) .__class__.objects.with_duration() .first() ) @@ -217,7 +219,10 @@ def test_get_album( assert response.data == expected playable_by.assert_called_once_with( - music_models.Album.objects.with_duration().select_related("artist"), None + music_models.Album.objects.with_duration().prefetch_related( + "artist_credit__artist" + ), + None, ) @@ -227,8 +232,8 @@ def test_get_song( ): url = reverse("api:subsonic:subsonic-get_song") assert url.endswith("getSong") is True - artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist) + artist_credit = factories["music.ArtistCredit"]() + album = factories["music.Album"](artist_credit=artist_credit) track = factories["music.Track"](album=album, playable=True) upload = factories["music.Upload"](track=track) playable_by = mocker.spy(music_models.TrackQuerySet, "playable_by") @@ -508,10 +513,12 @@ def test_get_album_list2_by_genre(f, db, logged_in_api_client, factories): url = reverse("api:subsonic:subsonic-get_album_list2") assert url.endswith("getAlbumList2") is True album1 = factories["music.Album"]( - artist__name="Artist1", playable=True, set_tags=["Rock"] + artist_credit__artist__name="Artist1", playable=True, set_tags=["Rock"] ).__class__.objects.with_duration()[0] album2 = factories["music.Album"]( - artist__name="Artist2", playable=True, artist__set_tags=["Rock"] + artist_credit__artist__name="Artist2", + playable=True, + artist_credit__artist__set_tags=["Rock"], ).__class__.objects.with_duration()[1] factories["music.Album"](playable=True, set_tags=["Pop"]) response = logged_in_api_client.get( @@ -556,7 +563,12 @@ def test_get_album_list2_by_year(params, expected, db, logged_in_api_client, fac @pytest.mark.parametrize("f", ["json"]) @pytest.mark.parametrize( "tags_field", - ["set_tags", "artist__set_tags", "album__set_tags", "album__artist__set_tags"], + [ + "set_tags", + "artist_credit__artist__set_tags", + "album__set_tags", + "album__artist_credit__artist__set_tags", + ], ) def test_get_songs_by_genre(f, tags_field, db, logged_in_api_client, factories): url = reverse("api:subsonic:subsonic-get_songs_by_genre") @@ -916,20 +928,22 @@ def test_get_podcasts(logged_in_api_client, factories, mocker): ) upload1 = factories["music.Upload"]( playable=True, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, library=channel.library, bitrate=128000, duration=42, ) upload2 = factories["music.Upload"]( playable=True, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, library=channel.library, bitrate=256000, duration=43, ) factories["federation.Follow"](actor=actor, target=channel.actor, approved=True) - factories["music.Upload"](import_status="pending", track__artist=channel.artist) + factories["music.Upload"]( + import_status="pending", track__artist_credit__artist=channel.artist + ) factories["audio.Channel"](external=True) factories["federation.Follow"]() url = reverse("api:subsonic:subsonic-get_podcasts") @@ -953,14 +967,14 @@ def test_get_podcasts_by_id(logged_in_api_client, factories, mocker): ) upload1 = factories["music.Upload"]( playable=True, - track__artist=channel1.artist, + track__artist_credit__artist=channel1.artist, library=channel1.library, bitrate=128000, duration=42, ) factories["music.Upload"]( playable=True, - track__artist=channel2.artist, + track__artist_credit__artist=channel2.artist, library=channel2.library, bitrate=256000, duration=43, @@ -983,14 +997,14 @@ def test_get_newest_podcasts(logged_in_api_client, factories, mocker): ) upload1 = factories["music.Upload"]( playable=True, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, library=channel.library, bitrate=128000, duration=42, ) upload2 = factories["music.Upload"]( playable=True, - track__artist=channel.artist, + track__artist_credit__artist=channel.artist, library=channel.library, bitrate=256000, duration=43, diff --git a/api/tests/tags/test_tasks.py b/api/tests/tags/test_tasks.py index 6264b49ce..94720fc5b 100644 --- a/api/tests/tags/test_tasks.py +++ b/api/tests/tags/test_tasks.py @@ -6,9 +6,11 @@ def test_get_tags_from_foreign_key(factories): rock_tag = factories["tags.Tag"](name="Rock") rap_tag = factories["tags.Tag"](name="Rap") artist = factories["music.Artist"]() - factories["music.Track"].create_batch(3, artist=artist, set_tags=["rock", "rap"]) factories["music.Track"].create_batch( - 3, artist=artist, set_tags=["rock", "rap", "techno"] + 3, artist_credit__artist=artist, set_tags=["rock", "rap"] + ) + factories["music.Track"].create_batch( + 3, artist_credit__artist=artist, set_tags=["rock", "rap", "techno"] ) result = tasks.get_tags_from_foreign_key( diff --git a/api/tests/typesense/test_utils.py b/api/tests/typesense/test_utils.py index f4927cecc..783ddff97 100644 --- a/api/tests/typesense/test_utils.py +++ b/api/tests/typesense/test_utils.py @@ -11,13 +11,13 @@ def test_resolve_recordings_to_fw_track(mocker, factories): factories["music.Track"]( pk=1, title="I Want It That Way", - artist=artist, + artist_credit__artist=artist, mbid="87dfa566-21c3-45ed-bc42-1d345b8563fa", ) factories["music.Track"]( pk=2, title="I Want It That Way", - artist=artist, + artist_credit__artist=artist, ) client = typesense.Client( diff --git a/changes/changelog.d/1568.feature b/changes/changelog.d/1568.feature new file mode 100644 index 000000000..9bece1b24 --- /dev/null +++ b/changes/changelog.d/1568.feature @@ -0,0 +1 @@ +Support multiples artists for tracks and albums (#1568) diff --git a/docs/specs/multi-artist/mb-content.md b/docs/specs/multi-artist/mb-content.md index 9533c668e..f91cd10c1 100644 --- a/docs/specs/multi-artist/mb-content.md +++ b/docs/specs/multi-artist/mb-content.md @@ -241,12 +241,12 @@ Given the above example, Funkwhale would create the following `ArtistCredit` obj The Funkwhale API needs to return artist credit information in a way that is easily consumed by a client. -Endpoints should include a `credited_artist` filter that allows a client to return results for which artists are credited. This filter should take a list of IDs. +Endpoints should include a `artist` filter that allows a client to return results for which artists are credited. This filter should take a list of IDs. -To return any albums where the artist is listed in the `artist_credit` field, you can filter by the `artist_id` field using the `credited_artist` filter: +To return any albums where the artist is listed in the `artist_credit` field, you can filter by the `artist_id` field using the `artist` filter: ```text -https://open.audio/api/v2/albums?credited_artist=6451,6452 +https://open.audio/api/v2/albums?artist=6451,6452 ``` The `credit` field of the `artist_credit` object must also be searchable using a standard query: diff --git a/front/src/App.vue b/front/src/App.vue index 6d6ab2649..aa6c0b3fe 100644 --- a/front/src/App.vue +++ b/front/src/App.vue @@ -32,7 +32,7 @@ const getTrackInformationText = (track: QueueTrack | undefined) => { return null } - return `♫ ${track.title} – ${track.artistName} ♫` + return `♫ ${track.title} – ${track.artistCredit} ♫` } // Update title diff --git a/front/src/components/Queue.vue b/front/src/components/Queue.vue index 95edf14e1..5e5968cf3 100644 --- a/front/src/components/Queue.vue +++ b/front/src/components/Queue.vue @@ -130,14 +130,13 @@ const reorderTracks = async (from: number, to: number) => { scrollToCurrent() } } - const hideArtist = () => { - if (currentTrack.value.artistId !== -1) { + if (currentTrack.value.artistId !== -1 && currentTrack.value.artistCredit) { return store.dispatch('moderation/hide', { type: 'artist', target: { - id: currentTrack.value.artistId, - name: currentTrack.value.artistName + id: currentTrack.value.artistCredit[0].artist.id, + name: currentTrack.value.artistCredit[0].artist.name } }) } @@ -264,7 +263,13 @@ if (!isWebGLSupported) { >

{{ currentTrack.title }}

- {{ currentTrack.artistName ?? $t('components.Queue.meta.unknownArtist') }} +
+ {{ ac.credit ?? $t('components.Queue.meta.unknownArtist') }} + {{ ac.joinphrase }} +
{{ currentTrack.albumTitle ?? $t('components.Queue.meta.unknownAlbum') }}

@@ -281,12 +286,21 @@ if (!isWebGLSupported) { {{ currentTrack.title }}
- - {{ currentTrack.artistName ?? $t('components.Queue.meta.unknownArtist') }} - + + +