See #170: cover on tracks and artists

api/funkwhale_api/common/migrations/0007_auto_20200116_1610.py

+# Generated by Django 2.2.9 on 2020-01-16 16:10
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('common', '0006_content'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='attachment',
+            name='url',
+            field=models.URLField(max_length=500, null=True),
+        ),
+    ]

api/funkwhale_api/common/models.py

@@ -175,7 +175,12 @@ def get_file_path(instance, filename):
 
 class AttachmentQuerySet(models.QuerySet):
     def attached(self, include=True):
-        related_fields = ["covered_album", "mutation_attachment"]
+        related_fields = [
+            "covered_album",
+            "mutation_attachment",
+            "covered_track",
+            "covered_artist",
+        ]
         query = None
         for field in related_fields:
             field_query = ~models.Q(**{field: None})
@@ -195,7 +200,7 @@ class AttachmentQuerySet(models.QuerySet):
 
 class Attachment(models.Model):
     # Remote URL where the attachment can be fetched
-    url = models.URLField(max_length=500, unique=True, null=True)
+    url = models.URLField(max_length=500, null=True)
     uuid = models.UUIDField(unique=True, db_index=True, default=uuid.uuid4)
     # Actor associated with the attachment
     actor = models.ForeignKey(

api/funkwhale_api/common/mutations.py

@@ -85,8 +85,6 @@ class MutationSerializer(serializers.Serializer):
 
 
 class UpdateMutationSerializer(serializers.ModelSerializer, MutationSerializer):
-    serialized_relations = {}
-
     def __init__(self, *args, **kwargs):
         # we force partial mode, because update mutations are partial
         kwargs.setdefault("partial", True)
@@ -105,13 +103,14 @@ class UpdateMutationSerializer(serializers.ModelSerializer, MutationSerializer):
         return super().validate(validated_data)
 
     def db_serialize(self, validated_data):
+        serialized_relations = self.get_serialized_relations()
         data = {}
         # ensure model fields are serialized properly
         for key, value in list(validated_data.items()):
             if not isinstance(value, models.Model):
                 data[key] = value
                 continue
-            field = self.serialized_relations[key]
+            field = serialized_relations[key]
             data[key] = getattr(value, field)
         return data
 
@@ -120,7 +119,7 @@ class UpdateMutationSerializer(serializers.ModelSerializer, MutationSerializer):
         # we use our serialized_relations configuration
         # to ensure we store ids instead of model instances in our json
         # payload
-        for field, attr in self.serialized_relations.items():
+        for field, attr in self.get_serialized_relations().items():
             try:
                 obj = data[field]
             except KeyError:
@@ -139,10 +138,13 @@ class UpdateMutationSerializer(serializers.ModelSerializer, MutationSerializer):
         return get_update_previous_state(
             obj,
             *list(validated_data.keys()),
-            serialized_relations=self.serialized_relations,
+            serialized_relations=self.get_serialized_relations(),
             handlers=self.get_previous_state_handlers(),
         )
 
+    def get_serialized_relations(self):
+        return {}
+
     def get_previous_state_handlers(self):
         return {}
 

api/funkwhale_api/common/utils.py

+from django.core.files.base import ContentFile
 from django.utils.deconstruct import deconstructible
 
 import bleach.sanitizer
+import logging
 import markdown
 import os
 import shutil
@@ -13,6 +15,8 @@ from django.conf import settings
 from django import urls
 from django.db import models, transaction
 
+logger = logging.getLogger(__name__)
+
 
 def rename_file(instance, field_name, new_name, allow_missing_file=False):
     field = getattr(instance, field_name)
@@ -306,3 +310,41 @@ def attach_content(obj, field, content_data):
     setattr(obj, field, content_obj)
     obj.save(update_fields=[field])
     return content_obj
+
+
+@transaction.atomic
+def attach_file(obj, field, file_data, fetch=False):
+    from . import models
+    from . import tasks
+
+    existing = getattr(obj, "{}_id".format(field))
+    if existing:
+        getattr(obj, field).delete()
+
+    if not file_data:
+        return
+
+    extensions = {"image/jpeg": "jpg", "image/png": "png", "image/gif": "gif"}
+    extension = extensions.get(file_data["mimetype"], "jpg")
+    attachment = models.Attachment(mimetype=file_data["mimetype"])
+
+    filename = "cover-{}.{}".format(obj.uuid, extension)
+    if "url" in file_data:
+        attachment.url = file_data["url"]
+    else:
+        f = ContentFile(file_data["content"])
+        attachment.file.save(filename, f, save=False)
+
+    if not attachment.file and fetch:
+        try:
+            tasks.fetch_remote_attachment(attachment, filename=filename, save=False)
+        except Exception as e:
+            logger.warn("Cannot download attachment at url %s: %s", attachment.url, e)
+            attachment = None
+
+    if attachment:
+        attachment.save()
+
+    setattr(obj, field, attachment)
+    obj.save(update_fields=[field])
+    return attachment

api/funkwhale_api/federation/serializers.py

@@ -22,7 +22,7 @@ logger = logging.getLogger(__name__)
 
 
 class LinkSerializer(jsonld.JsonLdSerializer):
-    type = serializers.ChoiceField(choices=[contexts.AS.Link])
+    type = serializers.ChoiceField(choices=[contexts.AS.Link, contexts.AS.Image])
     href = serializers.URLField(max_length=500)
     mediaType = serializers.CharField()
 
@@ -817,6 +817,17 @@ def include_content(repr, content_obj):
     repr["mediaType"] = "text/html"
 
 
+def include_image(repr, attachment):
+    if attachment:
+        repr["image"] = {
+            "type": "Image",
+            "href": attachment.download_url_original,
+            "mediaType": attachment.mimetype or "image/jpeg",
+        }
+    else:
+        repr["image"] = None
+
+
 class TruncatedCharField(serializers.CharField):
     def __init__(self, *args, **kwargs):
         self.truncate_length = kwargs.pop("truncate_length")
@@ -877,6 +888,23 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer):
         ]
 
     def validate_updated_data(self, instance, validated_data):
+        try:
+            attachment_cover = validated_data.pop("attachment_cover")
+        except KeyError:
+            return validated_data
+
+        if (
+            instance.attachment_cover
+            and instance.attachment_cover.url == attachment_cover["href"]
+        ):
+            # we already have the proper attachment
+            return validated_data
+        # create the attachment by hand so it can be attached as the cover
+        validated_data["attachment_cover"] = common_models.Attachment.objects.create(
+            mimetype=attachment_cover["mediaType"],
+            url=attachment_cover["href"],
+            actor=instance.attributed_to,
+        )
         return validated_data
 
     def validate(self, data):
@@ -890,15 +918,26 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer):
 
 
 class ArtistSerializer(MusicEntitySerializer):
+    image = LinkSerializer(
+        allowed_mimetypes=["image/*"], allow_null=True, required=False
+    )
     updateable_fields = [
         ("name", "name"),
         ("musicbrainzId", "mbid"),
         ("attributedTo", "attributed_to"),
+        ("image", "attachment_cover"),
     ]
 
     class Meta:
         model = music_models.Artist
-        jsonld_mapping = MUSIC_ENTITY_JSONLD_MAPPING
+        jsonld_mapping = common_utils.concat_dicts(
+            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),
+            },
+        )
 
     def to_representation(self, instance):
         d = {
@@ -913,6 +952,7 @@ class ArtistSerializer(MusicEntitySerializer):
             "tag": self.get_tags_repr(instance),
         }
         include_content(d, instance.description)
+        include_image(d, instance.attachment_cover)
         if self.context.get("include_ap_context", self.parent is None):
             d["@context"] = jsonld.get_default_context()
         return d
@@ -921,6 +961,7 @@ class ArtistSerializer(MusicEntitySerializer):
 class AlbumSerializer(MusicEntitySerializer):
     released = serializers.DateField(allow_null=True, required=False)
     artists = serializers.ListField(child=ArtistSerializer(), min_length=1)
+    # XXX: 1.0 rename to image
     cover = LinkSerializer(
         allowed_mimetypes=["image/*"], allow_null=True, required=False
     )
@@ -970,30 +1011,12 @@ class AlbumSerializer(MusicEntitySerializer):
                 "href": instance.attachment_cover.download_url_original,
                 "mediaType": instance.attachment_cover.mimetype or "image/jpeg",
             }
+            include_image(d, instance.attachment_cover)
+
         if self.context.get("include_ap_context", self.parent is None):
             d["@context"] = jsonld.get_default_context()
         return d
 
-    def validate_updated_data(self, instance, validated_data):
-        try:
-            attachment_cover = validated_data.pop("attachment_cover")
-        except KeyError:
-            return validated_data
-
-        if (
-            instance.attachment_cover
-            and instance.attachment_cover.url == attachment_cover["href"]
-        ):
-            # we already have the proper attachment
-            return validated_data
-        # create the attachment by hand so it can be attached as the album cover
-        validated_data["attachment_cover"] = common_models.Attachment.objects.create(
-            mimetype=attachment_cover["mediaType"],
-            url=attachment_cover["href"],
-            actor=instance.attributed_to,
-        )
-        return validated_data
-
 
 class TrackSerializer(MusicEntitySerializer):
     position = serializers.IntegerField(min_value=0, allow_null=True, required=False)
@@ -1002,6 +1025,9 @@ class TrackSerializer(MusicEntitySerializer):
     album = AlbumSerializer()
     license = serializers.URLField(allow_null=True, required=False)
     copyright = serializers.CharField(allow_null=True, required=False)
+    image = LinkSerializer(
+        allowed_mimetypes=["image/*"], allow_null=True, required=False
+    )
 
     updateable_fields = [
         ("name", "title"),
@@ -1011,6 +1037,7 @@ class TrackSerializer(MusicEntitySerializer):
         ("position", "position"),
         ("copyright", "copyright"),
         ("license", "license"),
+        ("image", "attachment_cover"),
     ]
 
     class Meta:
@@ -1024,6 +1051,7 @@ class TrackSerializer(MusicEntitySerializer):
                 "disc": jsonld.first_val(contexts.FW.disc),
                 "license": jsonld.first_id(contexts.FW.license),
                 "position": jsonld.first_val(contexts.FW.position),
+                "image": jsonld.first_obj(contexts.AS.image),
             },
         )
 
@@ -1054,6 +1082,7 @@ class TrackSerializer(MusicEntitySerializer):
             "tag": self.get_tags_repr(instance),
         }
         include_content(d, instance.description)
+        include_image(d, instance.attachment_cover)
         if self.context.get("include_ap_context", self.parent is None):
             d["@context"] = jsonld.get_default_context()
         return d

api/funkwhale_api/federation/views.py

@@ -222,9 +222,12 @@ class MusicLibraryViewSet(
                     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(
                         "tagged_items__tag",
@@ -283,6 +286,9 @@ class MusicUploadViewSet(
         "track__album__artist",
         "track__description",
         "track__album__attachment_cover",
+        "track__album__artist__attachment_cover",
+        "track__artist__attachment_cover",
+        "track__attachment_cover",
     )
     serializer_class = serializers.UploadSerializer
     lookup_field = "uuid"
@@ -303,7 +309,9 @@ class MusicArtistViewSet(
 ):
     authentication_classes = [authentication.SignatureAuthentication]
     renderer_classes = renderers.get_ap_renderers()
-    queryset = music_models.Artist.objects.local().select_related("description")
+    queryset = music_models.Artist.objects.local().select_related(
+        "description", "attachment_cover"
+    )
     serializer_class = serializers.ArtistSerializer
     lookup_field = "uuid"
 
@@ -314,7 +322,7 @@ class MusicAlbumViewSet(
     authentication_classes = [authentication.SignatureAuthentication]
     renderer_classes = renderers.get_ap_renderers()
     queryset = music_models.Album.objects.local().select_related(
-        "artist__description", "description"
+        "artist__description", "description", "artist__attachment_cover"
     )
     serializer_class = serializers.AlbumSerializer
     lookup_field = "uuid"
@@ -326,7 +334,14 @@ 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"
+        "album__artist",
+        "album__description",
+        "artist__description",
+        "description",
+        "attachment_cover",
+        "album__artist__attachment_cover",
+        "album__attachment_cover",
+        "artist__attachment_cover",
     )
     serializer_class = serializers.TrackSerializer
     lookup_field = "uuid"

api/funkwhale_api/manage/serializers.py

@@ -390,6 +390,7 @@ class ManageArtistSerializer(
     tracks = ManageNestedTrackSerializer(many=True)
     attributed_to = ManageBaseActorSerializer()
     tags = serializers.SerializerMethodField()
+    cover = music_serializers.cover_field
 
     class Meta:
         model = music_models.Artist
@@ -398,6 +399,7 @@ class ManageArtistSerializer(
             "tracks",
             "attributed_to",
             "tags",
+            "cover",
         ]
 
     def get_tags(self, obj):
@@ -447,6 +449,7 @@ class ManageTrackSerializer(
     attributed_to = ManageBaseActorSerializer()
     uploads_count = serializers.SerializerMethodField()
     tags = serializers.SerializerMethodField()
+    cover = music_serializers.cover_field
 
     class Meta:
         model = music_models.Track
@@ -456,6 +459,7 @@ class ManageTrackSerializer(
             "attributed_to",
             "uploads_count",
             "tags",
+            "cover",
         ]
 
     def get_uploads_count(self, obj):

api/funkwhale_api/manage/views.py

@@ -64,7 +64,7 @@ class ManageArtistViewSet(
     queryset = (
         music_models.Artist.objects.all()
         .order_by("-id")
-        .select_related("attributed_to")
+        .select_related("attributed_to", "attachment_cover",)
         .prefetch_related(
             "tracks",
             Prefetch(
@@ -164,7 +164,11 @@ class ManageTrackViewSet(
         music_models.Track.objects.all()
         .order_by("-id")
         .select_related(
-            "attributed_to", "artist", "album__artist", "album__attachment_cover"
+            "attributed_to",
+            "artist",
+            "album__artist",
+            "album__attachment_cover",
+            "attachment_cover",
         )
         .annotate(uploads_count=Coalesce(Subquery(uploads_subquery), 0))
         .prefetch_related(music_views.TAG_PREFETCH)

api/funkwhale_api/music/factories.py

@@ -64,6 +64,7 @@ class ArtistFactory(
     mbid = factory.Faker("uuid4")
     fid = factory.Faker("federation_url")
     playable = playable_factory("track__album__artist")
+    attachment_cover = factory.SubFactory(common_factories.AttachmentFactory)
 
     class Meta:
         model = "music.Artist"
@@ -111,6 +112,7 @@ class TrackFactory(
     album = factory.SubFactory(AlbumFactory)
     position = 1
     playable = playable_factory("track")
+    attachment_cover = factory.SubFactory(common_factories.AttachmentFactory)
 
     class Meta:
         model = "music.Track"

api/funkwhale_api/music/metadata.py

@@ -723,6 +723,7 @@ class TrackMetadataSerializer(serializers.Serializer):
                 continue
             if v in ["", None, []]:
                 validated_data.pop(field)
+        validated_data["album"]["cover_data"] = validated_data.pop("cover_data", None)
         return validated_data
 
 

api/funkwhale_api/music/migrations/0047_auto_20200116_1246.py

+# Generated by Django 2.2.9 on 2020-01-16 12:46
+
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('common', '0006_content'),
+        ('music', '0046_auto_20200113_1018'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='artist',
+            name='attachment_cover',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='covered_artist', to='common.Attachment'),
+        ),
+        migrations.AddField(
+            model_name='track',
+            name='attachment_cover',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='covered_track', to='common.Attachment'),
+        ),
+        migrations.AlterField(
+            model_name='album',
+            name='attachment_cover',
+            field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='covered_album', to='common.Attachment'),
+        ),
+    ]

api/funkwhale_api/music/models.py

@@ -230,6 +230,13 @@ class Artist(APIModelMixin):
     description = models.ForeignKey(
         "common.Content", null=True, blank=True, on_delete=models.SET_NULL
     )
+    attachment_cover = models.ForeignKey(
+        "common.Attachment",
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+        related_name="covered_artist",
+    )
 
     api = musicbrainz.api.artists
     objects = ArtistQuerySet.as_manager()
@@ -248,6 +255,10 @@ class Artist(APIModelMixin):
         kwargs.update({"name": name})
         return cls.objects.get_or_create(name__iexact=name, defaults=kwargs)
 
+    @property
+    def cover(self):
+        return self.attachment_cover
+
 
 def import_artist(v):
     a = Artist.get_or_create_from_api(mbid=v[0]["artist"]["id"])[0]
@@ -358,44 +369,6 @@ class Album(APIModelMixin):
     }
     objects = AlbumQuerySet.as_manager()
 
-    def get_image(self, data=None):
-        from funkwhale_api.common import tasks as common_tasks
-
-        attachment = None
-        if data:
-            extensions = {"image/jpeg": "jpg", "image/png": "png", "image/gif": "gif"}
-            extension = extensions.get(data["mimetype"], "jpg")
-            attachment = common_models.Attachment(mimetype=data["mimetype"])
-            f = None
-            filename = "{}.{}".format(self.uuid, extension)
-            if data.get("content"):
-                # we have to cover itself
-                f = ContentFile(data["content"])
-                attachment.file.save(filename, f, save=False)
-            elif data.get("url"):
-                attachment.url = data.get("url")
-                # we can fetch from a url
-                try:
-                    common_tasks.fetch_remote_attachment(
-                        attachment, filename=filename, save=False
-                    )
-                except Exception as e:
-                    logger.warn(
-                        "Cannot download cover at url %s: %s", data.get("url"), e
-                    )
-                    return
-
-        elif self.mbid:
-            image_data = musicbrainz.api.images.get_front(str(self.mbid))
-            f = ContentFile(image_data)
-            attachment = common_models.Attachment(mimetype="image/jpeg")
-            attachment.file.save("{0}.jpg".format(self.mbid), f, save=False)
-        if attachment and attachment.file:
-            attachment.save()
-            self.attachment_cover = attachment
-            self.save(update_fields=["attachment_cover"])
-            return self.attachment_cover.file
-
     @property
     def cover(self):
         return self.attachment_cover
@@ -518,6 +491,13 @@ class Track(APIModelMixin):
     description = models.ForeignKey(
         "common.Content", null=True, blank=True, on_delete=models.SET_NULL
     )
+    attachment_cover = models.ForeignKey(
+        "common.Attachment",
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+        related_name="covered_track",
+    )
 
     federation_namespace = "tracks"
     musicbrainz_model = "recording"
@@ -572,6 +552,10 @@ class Track(APIModelMixin):
         except AttributeError:
             return "{} - {}".format(self.artist.name, self.title)
 
+    @property
+    def cover(self):
+        return self.attachment_cover
+
     def get_activity_url(self):
         if self.mbid:
             return "https://musicbrainz.org/recording/{}".format(self.mbid)

api/funkwhale_api/music/mutations.py

@@ -59,17 +59,66 @@ class DescriptionMutation(mutations.UpdateMutationSerializer):
         return r
 
 
+class CoverMutation(mutations.UpdateMutationSerializer):
+    cover = common_serializers.RelatedField(
+        "uuid", queryset=common_models.Attachment.objects.all().local(), serializer=None
+    )
+
+    def get_serialized_relations(self):
+        serialized_relations = super().get_serialized_relations()
+        serialized_relations["cover"] = "uuid"
+        return serialized_relations
+
+    def get_previous_state_handlers(self):
+        handlers = super().get_previous_state_handlers()
+        handlers["cover"] = (
+            lambda obj: str(obj.attachment_cover.uuid) if obj.attachment_cover else None
+        )
+        return handlers
+
+    def update(self, instance, validated_data):
+        if "cover" in validated_data:
+            validated_data["attachment_cover"] = validated_data.pop("cover")
+        return super().update(instance, validated_data)
+
+    def mutation_post_init(self, mutation):
+        # link cover_attachment (if any) to mutation
+        if "cover" not in mutation.payload:
+            return
+        try:
+            attachment = common_models.Attachment.objects.get(
+                uuid=mutation.payload["cover"]
+            )
+        except common_models.Attachment.DoesNotExist:
+            return
+
+        common_models.MutationAttachment.objects.create(
+            attachment=attachment, mutation=mutation
+        )
+
+
 @mutations.registry.connect(
     "update",
     models.Track,
     perm_checkers={"suggest": can_suggest, "approve": can_approve},
 )
-class TrackMutationSerializer(TagMutation, DescriptionMutation):
-    serialized_relations = {"license": "code"}
-
+class TrackMutationSerializer(CoverMutation, TagMutation, DescriptionMutation):
     class Meta:
         model = models.Track
-        fields = ["license", "title", "position", "copyright", "tags", "description"]
+        fields = [
+            "license",
+            "title",
+            "position",
+            "copyright",
+            "tags",
+            "description",
+            "cover",
+        ]
+
+    def get_serialized_relations(self):
+        serialized_relations = super().get_serialized_relations()
+        serialized_relations["license"] = "code"
+        return serialized_relations
 
     def post_apply(self, obj, validated_data):
         routes.outbox.dispatch(
@@ -82,10 +131,10 @@ class TrackMutationSerializer(TagMutation, DescriptionMutation):
     models.Artist,
     perm_checkers={"suggest": can_suggest, "approve": can_approve},
 )
-class ArtistMutationSerializer(TagMutation, DescriptionMutation):
+class ArtistMutationSerializer(CoverMutation, TagMutation, DescriptionMutation):
     class Meta:
         model = models.Artist
-        fields = ["name", "tags", "description"]
+        fields = ["name", "tags", "description", "cover"]
 
     def post_apply(self, obj, validated_data):
         routes.outbox.dispatch(
@@ -98,45 +147,12 @@ class ArtistMutationSerializer(TagMutation, DescriptionMutation):
     models.Album,
     perm_checkers={"suggest": can_suggest, "approve": can_approve},
 )
-class AlbumMutationSerializer(TagMutation, DescriptionMutation):
-    cover = common_serializers.RelatedField(
-        "uuid", queryset=common_models.Attachment.objects.all().local(), serializer=None
-    )
-
-    serialized_relations = {"cover": "uuid"}
-
+class AlbumMutationSerializer(CoverMutation, TagMutation, DescriptionMutation):
     class Meta:
         model = models.Album
         fields = ["title", "release_date", "tags", "cover", "description"]
 
-    def get_previous_state_handlers(self):
-        handlers = super().get_previous_state_handlers()
-        handlers["cover"] = (
-            lambda obj: str(obj.attachment_cover.uuid) if obj.attachment_cover else None
-        )
-        return handlers
-
     def post_apply(self, obj, validated_data):
         routes.outbox.dispatch(
             {"type": "Update", "object": {"type": "Album"}}, context={"album": obj}
         )
-
-    def update(self, instance, validated_data):
-        if "cover" in validated_data:
-            validated_data["attachment_cover"] = validated_data.pop("cover")
-        return super().update(instance, validated_data)
-
-    def mutation_post_init(self, mutation):
-        # link cover_attachment (if any) to mutation
-        if "cover" not in mutation.payload:
-            return
-        try:
-            attachment = common_models.Attachment.objects.get(
-                uuid=mutation.payload["cover"]
-            )
-        except common_models.Attachment.DoesNotExist:
-            return
-
-        common_models.MutationAttachment.objects.create(
-            attachment=attachment, mutation=mutation
-        )

api/funkwhale_api/music/serializers.py

@@ -121,6 +121,7 @@ class ArtistWithAlbumsSerializer(OptionalDescriptionMixin, serializers.Serialize
     name = serializers.CharField()
     creation_date = serializers.DateTimeField()
     is_local = serializers.BooleanField()
+    cover = cover_field
 
     def get_tags(self, obj):
         tagged_items = getattr(obj, "_prefetched_tagged_items", [])
@@ -266,7 +267,7 @@ class TrackSerializer(OptionalDescriptionMixin, serializers.Serializer):
     disc_number = serializers.IntegerField()
     copyright = serializers.CharField()
     license = serializers.SerializerMethodField()
-
+    cover = cover_field
     get_attributed_to = serialize_attributed_to
 
     def get_artist(self, o):

api/funkwhale_api/music/tasks.py

@@ -11,6 +11,7 @@ from django.dispatch import receiver
 from musicbrainzngs import ResponseError
 from requests.exceptions import RequestException
 
+from funkwhale_api import musicbrainz
 from funkwhale_api.common import channels, preferences
 from funkwhale_api.common import utils as common_utils
 from funkwhale_api.federation import routes
@@ -28,33 +29,34 @@ from . import signals
 logger = logging.getLogger(__name__)
 
 
-def update_album_cover(
-    album, source=None, cover_data=None, musicbrainz=True, replace=False
-):
+def populate_album_cover(album, source=None, replace=False):
     if album.attachment_cover and not replace:
         return
-    if cover_data:
-        return album.get_image(data=cover_data)
-
     if source and source.startswith("file://"):
         # let's look for a cover in the same directory
         path = os.path.dirname(source.replace("file://", "", 1))
         logger.info("[Album %s] scanning covers from %s", album.pk, path)
         cover = get_cover_from_fs(path)
-        if cover:
-            return album.get_image(data=cover)
-    if musicbrainz and album.mbid:
-        try:
+        return common_utils.attach_file(album, "attachment_cover", cover)
+    if album.mbid:
         logger.info(
             "[Album %s] Fetching cover from musicbrainz release %s",
             album.pk,
             str(album.mbid),
         )
-            return album.get_image()
+        try:
+            image_data = musicbrainz.api.images.get_front(str(album.mbid))
         except ResponseError as exc:
             logger.warning(
                 "[Album %s] cannot fetch cover from musicbrainz: %s", album.pk, str(exc)
             )
+        else:
+            return common_utils.attach_file(
+                album,
+                "attachment_cover",
+                {"content": image_data, "mimetype": "image/jpeg"},
+                fetch=True,
+            )
 
 
 IMAGE_TYPES = [("jpg", "image/jpeg"), ("jpeg", "image/jpeg"), ("png", "image/png")]
@@ -274,10 +276,8 @@ def process_upload(upload, update_denormalization=True):
 
     # update album cover, if needed
     if not track.album.attachment_cover:
-        update_album_cover(
-            track.album,
-            source=final_metadata.get("upload_source"),
-            cover_data=final_metadata.get("cover_data"),
+        populate_album_cover(
+            track.album, source=final_metadata.get("upload_source"),
         )
 
     broadcast = getter(
@@ -299,6 +299,12 @@ def process_upload(upload, update_denormalization=True):
         )
 
 
+def get_cover(obj, field):
+    cover = obj.get(field)
+    if cover:
+        return {"mimetype": cover["mediaType"], "url": cover["href"]}
+
+
 def federation_audio_track_to_metadata(payload, references):
     """
     Given a valid payload as returned by federation.serializers.TrackSerializer.validated_data,
@@ -315,6 +321,7 @@ def federation_audio_track_to_metadata(payload, references):
         "mbid": str(payload.get("musicbrainzId"))
         if payload.get("musicbrainzId")
         else None,
+        "cover_data": get_cover(payload, "image"),
         "album": {
             "title": payload["album"]["name"],
             "fdate": payload["album"]["published"],
@@ -324,6 +331,7 @@ def federation_audio_track_to_metadata(payload, references):
             "mbid": str(payload["album"]["musicbrainzId"])
             if payload["album"].get("musicbrainzId")
             else None,
+            "cover_data": get_cover(payload["album"], "cover"),
             "release_date": payload["album"].get("released"),
             "tags": [t["name"] for t in payload["album"].get("tags", []) or []],
             "artists": [
@@ -331,6 +339,7 @@ def federation_audio_track_to_metadata(payload, references):
                     "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,
@@ -348,6 +357,7 @@ def federation_audio_track_to_metadata(payload, references):
                 "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"),
             }
             for a in payload["artists"]
         ],
@@ -356,9 +366,6 @@ def federation_audio_track_to_metadata(payload, references):
         "fdate": payload["published"],
         "tags": [t["name"] for t in payload.get("tags", []) or []],
     }
-    cover = payload["album"].get("cover")
-    if cover:
-        new_data["cover_data"] = {"mimetype": cover["mediaType"], "url": cover["href"]}
     return new_data
 
 
@@ -427,11 +434,7 @@ def get_track_from_import_metadata(
 ):
     track = _get_track(data, attributed_to=attributed_to, **forced_values)
     if update_cover and track and not track.album.attachment_cover:
-        update_album_cover(
-            track.album,
-            source=data.get("upload_source"),
-            cover_data=data.get("cover_data"),
-        )
+        populate_album_cover(track.album, source=data.get("upload_source"))
     return track
 
 
@@ -513,6 +516,9 @@ def _get_track(data, attributed_to=None, **forced_values):
             common_utils.attach_content(
                 artist, "description", artist_data.get("description")
             )
+            common_utils.attach_file(
+                artist, "attachment_cover", artist_data.get("cover_data")
+            )
 
     if "album" in forced_values:
         album = forced_values["album"]
@@ -550,6 +556,11 @@ def _get_track(data, attributed_to=None, **forced_values):
                 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"),
+                )
 
         # get / create album
         album_data = data["album"]
@@ -583,6 +594,9 @@ def _get_track(data, attributed_to=None, **forced_values):
             common_utils.attach_content(
                 album, "description", album_data.get("description")
             )
+            common_utils.attach_file(
+                album, "attachment_cover", album_data.get("cover_data")
+            )
 
     # get / create track
     track_title = (
@@ -643,6 +657,7 @@ def _get_track(data, attributed_to=None, **forced_values):
         )
         tags_models.add_tags(track, *tags)
         common_utils.attach_content(track, "description", data.get("description"))
+        common_utils.attach_file(track, "attachment_cover", data.get("cover_data"))
 
     return track
 

api/funkwhale_api/music/views.py

@@ -113,7 +113,7 @@ class ArtistViewSet(
 ):
     queryset = (
         models.Artist.objects.all()
-        .prefetch_related("attributed_to")
+        .prefetch_related("attributed_to", "attachment_cover")
         .prefetch_related(
             Prefetch(
                 "tracks",
@@ -295,7 +295,7 @@ class TrackViewSet(
     queryset = (
         models.Track.objects.all()
         .for_nested_serialization()
-        .prefetch_related("attributed_to")
+        .prefetch_related("attributed_to", "attachment_cover")
         .order_by("-creation_date")
     )
     serializer_class = serializers.TrackSerializer
@@ -558,7 +558,12 @@ class UploadViewSet(
     queryset = (
         models.Upload.objects.all()
         .order_by("-creation_date")
-        .prefetch_related("library", "track__artist", "track__album__artist")
+        .prefetch_related(
+            "library",
+            "track__artist",
+            "track__album__artist",
+            "track__attachment_cover",
+        )
     )
     serializer_class = serializers.UploadForOwnerSerializer
     permission_classes = [

api/requirements/test.txt

 # Test dependencies go here.
 
 flake8
-pytest>=5
+pytest>=5,<5.3.3
 pytest-django>=3.5.1
 pytest-mock
 pytest-sugar

api/tests/common/test_models.py

@@ -64,7 +64,7 @@ def test_attachment(factories, now):
 @pytest.mark.parametrize("args, expected", [([], [0]), ([True], [0]), ([False], [1])])
 def test_attachment_queryset_attached(args, expected, factories, queryset_equal_list):
     attachments = [
-        factories["music.Album"]().attachment_cover,
+        factories["music.Album"](artist__attachment_cover=None).attachment_cover,
         factories["common.Attachment"](),
     ]
 

api/tests/common/test_mutations.py

@@ -63,12 +63,13 @@ def test_db_serialize_update_mutation(factories, mutations_registry, mocker):
     user = factories["users.User"](email="hello@test.email", with_actor=True)
 
     class S(mutations.UpdateMutationSerializer):
-        serialized_relations = {"actor": "full_username"}
-
         class Meta:
             model = user.__class__
             fields = ["actor"]
 
+        def get_serialized_relations(self):
+            return {"actor": "full_username"}
+
     expected = {"actor": user.actor.full_username}
     assert S().db_serialize({"actor": user.actor}) == expected
 

api/tests/common/test_utils.py

+import io
 import pytest
 
 from funkwhale_api.common import utils
@@ -124,3 +125,51 @@ def test_join_url(start, end, expected):
 def test_render_html(text, content_type, expected):
     result = utils.render_html(text, content_type)
     assert result == expected
+
+
+def test_attach_file_url(factories):
+    album = factories["music.Album"]()
+    existing_attachment = album.attachment_cover
+    assert existing_attachment is not None
+
+    data = {"mimetype": "image/jpeg", "url": "https://example.com/test.jpg"}
+    new_attachment = utils.attach_file(album, "attachment_cover", data)
+
+    album.refresh_from_db()
+
+    with pytest.raises(existing_attachment.DoesNotExist):
+        existing_attachment.refresh_from_db()
+
+    assert album.attachment_cover == new_attachment
+    assert not new_attachment.file
+    assert new_attachment.url == data["url"]
+    assert new_attachment.mimetype == data["mimetype"]
+
+
+def test_attach_file_url_fetch(factories, r_mock):
+    album = factories["music.Album"]()
+
+    data = {"mimetype": "image/jpeg", "url": "https://example.com/test.jpg"}
+    r_mock.get(data["url"], body=io.BytesIO(b"content"))
+    new_attachment = utils.attach_file(album, "attachment_cover", data, fetch=True)
+
+    album.refresh_from_db()
+
+    assert album.attachment_cover == new_attachment
+    assert new_attachment.file.read() == b"content"
+    assert new_attachment.url == data["url"]
+    assert new_attachment.mimetype == data["mimetype"]
+
+
+def test_attach_file_content(factories, r_mock):
+    album = factories["music.Album"]()
+
+    data = {"mimetype": "image/jpeg", "content": b"content"}
+    new_attachment = utils.attach_file(album, "attachment_cover", data)
+
+    album.refresh_from_db()
+
+    assert album.attachment_cover == new_attachment
+    assert new_attachment.file.read() == b"content"
+    assert new_attachment.url is None
+    assert new_attachment.mimetype == data["mimetype"]