From c84396e669ec3e4bb0c9e7e4c2d925facf6df25c Mon Sep 17 00:00:00 2001 From: Eliot Berriot <contact@eliotberriot.com> Date: Mon, 25 Nov 2019 09:49:06 +0100 Subject: [PATCH] Attachments --- api/config/api_urls.py | 1 + api/config/settings/common.py | 17 ++- api/funkwhale_api/common/admin.py | 17 +++ api/funkwhale_api/common/factories.py | 11 ++ api/funkwhale_api/common/middleware.py | 7 +- .../migrations/0004_auto_20191111_1338.py | 35 ++++++ api/funkwhale_api/common/models.py | 108 ++++++++++++++++++ .../common/scripts/create_image_variations.py | 7 +- api/funkwhale_api/common/serializers.py | 35 ++++++ api/funkwhale_api/common/session.py | 9 +- api/funkwhale_api/common/tasks.py | 43 ++++++- api/funkwhale_api/common/views.py | 39 +++++++ api/funkwhale_api/favorites/views.py | 4 +- api/funkwhale_api/federation/actors.py | 5 +- api/funkwhale_api/federation/library.py | 9 +- api/funkwhale_api/federation/models.py | 1 - api/funkwhale_api/federation/serializers.py | 11 +- api/funkwhale_api/federation/tasks.py | 14 +-- api/funkwhale_api/federation/utils.py | 2 - api/funkwhale_api/federation/views.py | 28 ++++- api/funkwhale_api/federation/webfinger.py | 4 +- api/funkwhale_api/manage/views.py | 12 +- .../management/commands/mrf_check.py | 7 +- api/funkwhale_api/music/factories.py | 3 +- .../migrations/0042_album_attachment_cover.py | 20 ++++ .../migrations/0043_album_cover_attachment.py | 41 +++++++ api/funkwhale_api/music/models.py | 73 ++++++------ api/funkwhale_api/music/serializers.py | 85 ++++++++------ api/funkwhale_api/music/spa_views.py | 28 ++--- api/funkwhale_api/music/tasks.py | 9 +- api/funkwhale_api/music/views.py | 6 +- api/funkwhale_api/playlists/models.py | 10 +- api/funkwhale_api/playlists/serializers.py | 2 +- api/funkwhale_api/subsonic/serializers.py | 6 +- api/funkwhale_api/subsonic/views.py | 16 ++- api/tests/common/test_models.py | 25 ++++ api/tests/common/test_serializers.py | 70 ++++++++++++ api/tests/common/test_tasks.py | 23 ++++ api/tests/common/test_views.py | 68 +++++++++++ api/tests/federation/test_serializers.py | 6 +- api/tests/manage/test_serializers.py | 17 +-- api/tests/music/test_models.py | 2 +- api/tests/music/test_music.py | 4 +- api/tests/music/test_serializers.py | 24 ++-- api/tests/music/test_spa_views.py | 16 +-- api/tests/music/test_tasks.py | 17 +-- api/tests/music/test_views.py | 28 +++-- api/tests/playlists/test_serializers.py | 12 +- api/tests/subsonic/test_views.py | 2 +- docs/swagger.yml | 103 +++++++++++++---- 50 files changed, 880 insertions(+), 262 deletions(-) create mode 100644 api/funkwhale_api/common/migrations/0004_auto_20191111_1338.py create mode 100644 api/funkwhale_api/music/migrations/0042_album_attachment_cover.py create mode 100644 api/funkwhale_api/music/migrations/0043_album_cover_attachment.py diff --git a/api/config/api_urls.py b/api/config/api_urls.py index cdcc743912..dc5ef22a30 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -28,6 +28,7 @@ router.register( r"playlist-tracks", playlists_views.PlaylistTrackViewSet, "playlist-tracks" ) router.register(r"mutations", common_views.MutationViewSet, "mutations") +router.register(r"attachments", common_views.AttachmentViewSet, "attachments") v1_patterns = router.urls subsonic_router = routers.SimpleRouter(trailing_slash=False) diff --git a/api/config/settings/common.py b/api/config/settings/common.py index e5ac5a3440..1ec11e7ff5 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -392,6 +392,11 @@ MEDIA_ROOT = env("MEDIA_ROOT", default=str(APPS_DIR("media"))) # See: https://docs.djangoproject.com/en/dev/ref/settings/#media-url MEDIA_URL = env("MEDIA_URL", default="/media/") FILE_UPLOAD_PERMISSIONS = 0o644 + +ATTACHMENTS_UNATTACHED_PRUNE_DELAY = env.int( + "ATTACHMENTS_UNATTACHED_PRUNE_DELAY", default=3600 * 24 +) + # URL Configuration # ------------------------------------------------------------------------------ ROOT_URLCONF = "config.urls" @@ -558,6 +563,11 @@ CELERY_BROKER_URL = env( CELERY_TASK_DEFAULT_RATE_LIMIT = 1 CELERY_TASK_TIME_LIMIT = 300 CELERY_BEAT_SCHEDULE = { + "common.prune_unattached_attachments": { + "task": "common.prune_unattached_attachments", + "schedule": crontab(minute="0", hour="*"), + "options": {"expires": 60 * 60}, + }, "federation.clean_music_cache": { "task": "federation.clean_music_cache", "schedule": crontab(minute="0", hour="*/2"), @@ -856,6 +866,7 @@ ACCOUNT_USERNAME_BLACKLIST = [ ] + env.list("ACCOUNT_USERNAME_BLACKLIST", default=[]) EXTERNAL_REQUESTS_VERIFY_SSL = env.bool("EXTERNAL_REQUESTS_VERIFY_SSL", default=True) +EXTERNAL_REQUESTS_TIMEOUT = env.int("EXTERNAL_REQUESTS_TIMEOUT", default=5) # XXX: deprecated, see #186 API_AUTHENTICATION_REQUIRED = env.bool("API_AUTHENTICATION_REQUIRED", True) @@ -878,7 +889,11 @@ VERSATILEIMAGEFIELD_RENDITION_KEY_SETS = { ("square_crop", "crop__400x400"), ("medium_square_crop", "crop__200x200"), ("small_square_crop", "crop__50x50"), - ] + ], + "attachment_square": [ + ("original", "url"), + ("medium_square_crop", "crop__200x200"), + ], } VERSATILEIMAGEFIELD_SETTINGS = {"create_images_on_demand": False} RSA_KEY_SIZE = 2048 diff --git a/api/funkwhale_api/common/admin.py b/api/funkwhale_api/common/admin.py index 3ec6f1f449..39a1125311 100644 --- a/api/funkwhale_api/common/admin.py +++ b/api/funkwhale_api/common/admin.py @@ -45,3 +45,20 @@ class MutationAdmin(ModelAdmin): search_fields = ["created_by__preferred_username"] list_filter = ["type", "is_approved", "is_applied"] actions = [apply] + + +@register(models.Attachment) +class AttachmentAdmin(ModelAdmin): + list_display = [ + "uuid", + "actor", + "url", + "file", + "size", + "mimetype", + "creation_date", + "last_fetch_date", + ] + list_select_related = True + search_fields = ["actor__domain__name"] + list_filter = ["mimetype"] diff --git a/api/funkwhale_api/common/factories.py b/api/funkwhale_api/common/factories.py index 6919f9c377..85a441e852 100644 --- a/api/funkwhale_api/common/factories.py +++ b/api/funkwhale_api/common/factories.py @@ -23,3 +23,14 @@ class MutationFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): return self.target = extracted self.save() + + +@registry.register +class AttachmentFactory(NoUpdateOnCreate, factory.django.DjangoModelFactory): + url = factory.Faker("federation_url") + uuid = factory.Faker("uuid4") + actor = factory.SubFactory(federation_factories.ActorFactory) + file = factory.django.ImageField() + + class Meta: + model = "common.Attachment" diff --git a/api/funkwhale_api/common/middleware.py b/api/funkwhale_api/common/middleware.py index d8573baaae..3b9fc17e5b 100644 --- a/api/funkwhale_api/common/middleware.py +++ b/api/funkwhale_api/common/middleware.py @@ -1,6 +1,5 @@ import html import io -import requests import time import xml.sax.saxutils @@ -11,6 +10,7 @@ from django import urls from rest_framework import views from . import preferences +from . import session from . import throttling from . import utils @@ -76,10 +76,7 @@ def get_spa_html(spa_url): if cached: return cached - response = requests.get( - utils.join_url(spa_url, "index.html"), - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, - ) + response = session.get_session().get(utils.join_url(spa_url, "index.html"),) response.raise_for_status() content = response.text caches["local"].set(cache_key, content, settings.FUNKWHALE_SPA_HTML_CACHE_DURATION) diff --git a/api/funkwhale_api/common/migrations/0004_auto_20191111_1338.py b/api/funkwhale_api/common/migrations/0004_auto_20191111_1338.py new file mode 100644 index 0000000000..f35c9e11fb --- /dev/null +++ b/api/funkwhale_api/common/migrations/0004_auto_20191111_1338.py @@ -0,0 +1,35 @@ +# Generated by Django 2.2.6 on 2019-11-11 13:38 + +import django.contrib.postgres.fields.jsonb +import django.core.serializers.json +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import funkwhale_api.common.models +import funkwhale_api.common.validators +import uuid +import versatileimagefield.fields + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0003_cit_extension'), + ] + + operations = [ + migrations.CreateModel( + name='Attachment', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('url', models.URLField(max_length=500, unique=True, null=True)), + ('uuid', models.UUIDField(db_index=True, default=uuid.uuid4, unique=True)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('last_fetch_date', models.DateTimeField(blank=True, null=True)), + ('size', models.IntegerField(blank=True, null=True)), + ('mimetype', models.CharField(blank=True, max_length=200, null=True)), + ('file', versatileimagefield.fields.VersatileImageField(max_length=255, upload_to=funkwhale_api.common.models.get_file_path, validators=[funkwhale_api.common.validators.ImageDimensionsValidator(min_height=50, min_width=50), funkwhale_api.common.validators.FileValidator(allowed_extensions=['png', 'jpg', 'jpeg'], max_size=5242880)])), + ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='attachments', to='federation.Actor', null=True)), + ], + ), + ] diff --git a/api/funkwhale_api/common/models.py b/api/funkwhale_api/common/models.py index 9fd1a3c764..f764bf251e 100644 --- a/api/funkwhale_api/common/models.py +++ b/api/funkwhale_api/common/models.py @@ -1,4 +1,6 @@ import uuid +import magic +import mimetypes from django.contrib.postgres.fields import JSONField from django.contrib.contenttypes.fields import GenericForeignKey @@ -9,11 +11,18 @@ from django.db import connections, models, transaction from django.db.models import Lookup from django.db.models.fields import Field from django.db.models.sql.compiler import SQLCompiler +from django.dispatch import receiver from django.utils import timezone from django.urls import reverse +from versatileimagefield.fields import VersatileImageField +from versatileimagefield.image_warmer import VersatileImageFieldWarmer + from funkwhale_api.federation import utils as federation_utils +from . import utils +from . import validators + @Field.register_lookup class NotEqual(Lookup): @@ -150,3 +159,102 @@ class Mutation(models.Model): self.applied_date = timezone.now() self.save(update_fields=["is_applied", "applied_date", "previous_state"]) return previous_state + + +def get_file_path(instance, filename): + return utils.ChunkedPath("attachments")(instance, filename) + + +class AttachmentQuerySet(models.QuerySet): + def attached(self, include=True): + related_fields = ["covered_album"] + query = None + for field in related_fields: + field_query = ~models.Q(**{field: None}) + query = query | field_query if query else field_query + + if include is False: + query = ~query + + return self.filter(query) + + +class Attachment(models.Model): + # Remote URL where the attachment can be fetched + url = models.URLField(max_length=500, unique=True, null=True) + uuid = models.UUIDField(unique=True, db_index=True, default=uuid.uuid4) + # Actor associated with the attachment + actor = models.ForeignKey( + "federation.Actor", + related_name="attachments", + on_delete=models.CASCADE, + null=True, + ) + creation_date = models.DateTimeField(default=timezone.now) + last_fetch_date = models.DateTimeField(null=True, blank=True) + # File size + size = models.IntegerField(null=True, blank=True) + mimetype = models.CharField(null=True, blank=True, max_length=200) + + file = VersatileImageField( + upload_to=get_file_path, + max_length=255, + validators=[ + validators.ImageDimensionsValidator(min_width=50, min_height=50), + validators.FileValidator( + allowed_extensions=["png", "jpg", "jpeg"], max_size=1024 * 1024 * 5, + ), + ], + ) + + objects = AttachmentQuerySet.as_manager() + + def save(self, **kwargs): + if self.file and not self.size: + self.size = self.file.size + + if self.file and not self.mimetype: + self.mimetype = self.guess_mimetype() + + return super().save() + + @property + def is_local(self): + return federation_utils.is_local(self.fid) + + def guess_mimetype(self): + f = self.file + b = min(1000000, f.size) + t = magic.from_buffer(f.read(b), mime=True) + if not t.startswith("image/"): + # failure, we try guessing by extension + mt, _ = mimetypes.guess_type(f.name) + if mt: + t = mt + return t + + @property + def download_url_original(self): + if self.file: + return federation_utils.full_url(self.file.url) + proxy_url = reverse("api:v1:attachments-proxy", kwargs={"uuid": self.uuid}) + return federation_utils.full_url(proxy_url + "?next=original") + + @property + def download_url_medium_square_crop(self): + if self.file: + return federation_utils.full_url(self.file.crop["200x200"].url) + proxy_url = reverse("api:v1:attachments-proxy", kwargs={"uuid": self.uuid}) + return federation_utils.full_url(proxy_url + "?next=medium_square_crop") + + +@receiver(models.signals.post_save, sender=Attachment) +def warm_attachment_thumbnails(sender, instance, **kwargs): + if not instance.file or not settings.CREATE_IMAGE_THUMBNAILS: + return + warmer = VersatileImageFieldWarmer( + instance_or_queryset=instance, + rendition_key_set="attachment_square", + image_attr="file", + ) + num_created, failed_to_create = warmer.warm() diff --git a/api/funkwhale_api/common/scripts/create_image_variations.py b/api/funkwhale_api/common/scripts/create_image_variations.py index 5e941ce1fe..31bf0269c3 100644 --- a/api/funkwhale_api/common/scripts/create_image_variations.py +++ b/api/funkwhale_api/common/scripts/create_image_variations.py @@ -4,11 +4,16 @@ Compute different sizes of image used for Album covers and User avatars from versatileimagefield.image_warmer import VersatileImageFieldWarmer +from funkwhale_api.common.models import Attachment from funkwhale_api.music.models import Album from funkwhale_api.users.models import User -MODELS = [(Album, "cover", "square"), (User, "avatar", "square")] +MODELS = [ + (Album, "cover", "square"), + (User, "avatar", "square"), + (Attachment, "file", "attachment_square"), +] def main(command, **kwargs): diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py index 59b513f37a..3f128c18d2 100644 --- a/api/funkwhale_api/common/serializers.py +++ b/api/funkwhale_api/common/serializers.py @@ -272,3 +272,38 @@ class APIMutationSerializer(serializers.ModelSerializer): if value not in self.context["registry"]: raise serializers.ValidationError("Invalid mutation type {}".format(value)) return value + + +class AttachmentSerializer(serializers.Serializer): + uuid = serializers.UUIDField(read_only=True) + size = serializers.IntegerField(read_only=True) + mimetype = serializers.CharField(read_only=True) + creation_date = serializers.DateTimeField(read_only=True) + file = StripExifImageField(write_only=True) + urls = serializers.SerializerMethodField() + + def get_urls(self, o): + urls = {} + urls["source"] = o.url + urls["original"] = o.download_url_original + urls["medium_square_crop"] = o.download_url_medium_square_crop + return urls + + def to_representation(self, o): + repr = super().to_representation(o) + # XXX: BACKWARD COMPATIBILITY + # having the attachment urls in a nested JSON obj is better, + # but we can't do this without breaking clients + # So we extract the urls and include these in the parent payload + repr.update({k: v for k, v in repr["urls"].items() if k != "source"}) + # also, our legacy images had lots of variations (400x400, 200x200, 50x50) + # but we removed some of these, so we emulate these by hand (by redirecting) + # to actual, existing attachment variations + repr["square_crop"] = repr["medium_square_crop"] + repr["small_square_crop"] = repr["medium_square_crop"] + return repr + + def create(self, validated_data): + return models.Attachment.objects.create( + file=validated_data["file"], actor=validated_data["actor"] + ) diff --git a/api/funkwhale_api/common/session.py b/api/funkwhale_api/common/session.py index 4d5d0bb60b..23fd283061 100644 --- a/api/funkwhale_api/common/session.py +++ b/api/funkwhale_api/common/session.py @@ -4,6 +4,13 @@ from django.conf import settings import funkwhale_api +class FunkwhaleSession(requests.Session): + def request(self, *args, **kwargs): + kwargs.setdefault("verify", settings.EXTERNAL_REQUESTS_VERIFY_SSL) + kwargs.setdefault("timeout", settings.EXTERNAL_REQUESTS_TIMEOUT) + return super().request(*args, **kwargs) + + def get_user_agent(): return "python-requests (funkwhale/{}; +{})".format( funkwhale_api.__version__, settings.FUNKWHALE_URL @@ -11,6 +18,6 @@ def get_user_agent(): def get_session(): - s = requests.Session() + s = FunkwhaleSession() s.headers["User-Agent"] = get_user_agent() return s diff --git a/api/funkwhale_api/common/tasks.py b/api/funkwhale_api/common/tasks.py index 994b0bdfff..c7deee7f5e 100644 --- a/api/funkwhale_api/common/tasks.py +++ b/api/funkwhale_api/common/tasks.py @@ -1,14 +1,23 @@ +import datetime +import logging +import tempfile + +from django.conf import settings +from django.core.files import File from django.db import transaction from django.dispatch import receiver - +from django.utils import timezone from funkwhale_api.common import channels from funkwhale_api.taskapp import celery from . import models from . import serializers +from . import session from . import signals +logger = logging.getLogger(__name__) + @celery.app.task(name="common.apply_mutation") @transaction.atomic @@ -57,3 +66,35 @@ def broadcast_mutation_update(mutation, old_is_approved, new_is_approved, **kwar }, }, ) + + +def fetch_remote_attachment(attachment, filename=None, save=True): + if attachment.file: + # already there, no need to fetch + return + + s = session.get_session() + attachment.last_fetch_date = timezone.now() + with tempfile.TemporaryFile() as tf: + with s.get(attachment.url, timeout=5, stream=True) as r: + for chunk in r.iter_content(): + tf.write(chunk) + tf.seek(0) + attachment.file.save( + filename or attachment.url.split("/")[-1], File(tf), save=save + ) + + +@celery.app.task(name="common.prune_unattached_attachments") +def prune_unattached_attachments(): + limit = timezone.now() - datetime.timedelta( + seconds=settings.ATTACHMENTS_UNATTACHED_PRUNE_DELAY + ) + candidates = models.Attachment.objects.attached(False).filter( + creation_date__lte=limit + ) + + total = candidates.count() + logger.info("Deleting %s unattached attachments…", total) + result = candidates.delete() + logger.info("Deletion done: %s", result) diff --git a/api/funkwhale_api/common/views.py b/api/funkwhale_api/common/views.py index d197dbe908..1109589d0e 100644 --- a/api/funkwhale_api/common/views.py +++ b/api/funkwhale_api/common/views.py @@ -11,6 +11,8 @@ from rest_framework import response from rest_framework import views from rest_framework import viewsets +from funkwhale_api.users.oauth import permissions as oauth_permissions + from . import filters from . import models from . import mutations @@ -140,3 +142,40 @@ class RateLimitView(views.APIView): "scopes": throttling.get_status(ident, time.time()), } return response.Response(data, status=200) + + +class AttachmentViewSet( + mixins.RetrieveModelMixin, + mixins.CreateModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet, +): + lookup_field = "uuid" + queryset = models.Attachment.objects.all() + serializer_class = serializers.AttachmentSerializer + permission_classes = [oauth_permissions.ScopePermission] + required_scope = "libraries" + anonymous_policy = "setting" + + @action(detail=True, methods=["get"]) + @transaction.atomic + def proxy(self, request, *args, **kwargs): + instance = self.get_object() + + size = request.GET.get("next", "original").lower() + if size not in ["original", "medium_square_crop"]: + size = "original" + + tasks.fetch_remote_attachment(instance) + data = self.serializer_class(instance).data + redirect = response.Response(status=302) + redirect["Location"] = data["urls"][size] + return redirect + + def perform_create(self, serializer): + return serializer.save(actor=self.request.user.actor) + + def perform_destroy(self, instance): + if instance.actor is None or instance.actor != self.request.user.actor: + raise exceptions.PermissionDenied() + instance.delete() diff --git a/api/funkwhale_api/favorites/views.py b/api/funkwhale_api/favorites/views.py index 7d1991bc67..3e1f337593 100644 --- a/api/funkwhale_api/favorites/views.py +++ b/api/funkwhale_api/favorites/views.py @@ -54,7 +54,9 @@ class TrackFavoriteViewSet( ) tracks = Track.objects.with_playable_uploads( music_utils.get_actor_from_request(self.request) - ).select_related("artist", "album__artist", "attributed_to") + ).select_related( + "artist", "album__artist", "attributed_to", "album__attachment_cover" + ) queryset = queryset.prefetch_related(Prefetch("track", queryset=tracks)) return queryset diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 95997f9525..39161a9cb6 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -14,10 +14,7 @@ logger = logging.getLogger(__name__) def get_actor_data(actor_url): response = session.get_session().get( - actor_url, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, - headers={"Accept": "application/activity+json"}, + actor_url, headers={"Accept": "application/activity+json"}, ) response.raise_for_status() try: diff --git a/api/funkwhale_api/federation/library.py b/api/funkwhale_api/federation/library.py index e7f8373fa3..1fef43c22b 100644 --- a/api/funkwhale_api/federation/library.py +++ b/api/funkwhale_api/federation/library.py @@ -1,5 +1,4 @@ import requests -from django.conf import settings from funkwhale_api.common import session @@ -12,8 +11,6 @@ def get_library_data(library_url, actor): response = session.get_session().get( library_url, auth=auth, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, headers={"Content-Type": "application/activity+json"}, ) except requests.ConnectionError: @@ -35,11 +32,7 @@ def get_library_data(library_url, actor): def get_library_page(library, page_url, actor): auth = signing.get_auth(actor.private_key, actor.private_key_id) response = session.get_session().get( - page_url, - auth=auth, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, - headers={"Content-Type": "application/activity+json"}, + page_url, auth=auth, headers={"Content-Type": "application/activity+json"}, ) serializer = serializers.CollectionPageSerializer( data=response.json(), diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index fd52e17c86..12057253ff 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -541,7 +541,6 @@ class LibraryTrack(models.Model): auth=auth, stream=True, timeout=20, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, headers={"Content-Type": "application/activity+json"}, ) with remote_response as r: diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 697c2acc91..5a62f4ce3c 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -824,8 +824,8 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer): def get_tags_repr(self, instance): return [ - {"type": "Hashtag", "name": "#{}".format(tag)} - for tag in sorted(instance.tagged_items.values_list("tag__name", flat=True)) + {"type": "Hashtag", "name": "#{}".format(item.tag.name)} + for item in sorted(instance.tagged_items.all(), key=lambda i: i.tag.name) ] @@ -902,12 +902,11 @@ class AlbumSerializer(MusicEntitySerializer): else None, "tag": self.get_tags_repr(instance), } - if instance.cover: + if instance.attachment_cover: d["cover"] = { "type": "Link", - "href": utils.full_url(instance.cover.url), - "mediaType": mimetypes.guess_type(instance.cover_path)[0] - or "image/jpeg", + "href": instance.attachment_cover.download_url_original, + "mediaType": instance.attachment_cover.mimetype or "image/jpeg", } if self.context.get("include_ap_context", self.parent is None): d["@context"] = jsonld.get_default_context() diff --git a/api/funkwhale_api/federation/tasks.py b/api/funkwhale_api/federation/tasks.py index 163ac77885..6a03438c4c 100644 --- a/api/funkwhale_api/federation/tasks.py +++ b/api/funkwhale_api/federation/tasks.py @@ -88,7 +88,7 @@ def dispatch_inbox(activity, call_handlers=True): context={ "activity": activity, "actor": activity.actor, - "inbox_items": activity.inbox_items.filter(is_read=False), + "inbox_items": activity.inbox_items.filter(is_read=False).order_by("id"), }, call_handlers=call_handlers, ) @@ -142,8 +142,6 @@ def deliver_to_remote(delivery): auth=auth, json=delivery.activity.payload, url=delivery.inbox_url, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, headers={"Content-Type": "application/activity+json"}, ) logger.debug("Remote answered with %s", response.status_code) @@ -163,9 +161,7 @@ def deliver_to_remote(delivery): def fetch_nodeinfo(domain_name): s = session.get_session() wellknown_url = "https://{}/.well-known/nodeinfo".format(domain_name) - response = s.get( - url=wellknown_url, timeout=5, verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL - ) + response = s.get(url=wellknown_url) response.raise_for_status() serializer = serializers.NodeInfoSerializer(data=response.json()) serializer.is_valid(raise_exception=True) @@ -175,9 +171,7 @@ def fetch_nodeinfo(domain_name): nodeinfo_url = link["href"] break - response = s.get( - url=nodeinfo_url, timeout=5, verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL - ) + response = s.get(url=nodeinfo_url) response.raise_for_status() return response.json() @@ -308,8 +302,6 @@ def fetch(fetch): response = session.get_session().get( auth=auth, url=fetch.url, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, headers={"Content-Type": "application/activity+json"}, ) logger.debug("Remote answered with %s", response.status_code) diff --git a/api/funkwhale_api/federation/utils.py b/api/funkwhale_api/federation/utils.py index c2eacfe9d6..74e9fef692 100644 --- a/api/funkwhale_api/federation/utils.py +++ b/api/funkwhale_api/federation/utils.py @@ -84,8 +84,6 @@ def retrieve_ap_object( response = session.get_session().get( fid, auth=auth, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, headers={ "Accept": "application/activity+json", "Content-Type": "application/activity+json", diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 85594e02bf..7aebc29718 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -1,5 +1,6 @@ from django import forms from django.core import paginator +from django.db.models import Prefetch from django.http import HttpResponse from django.urls import reverse from rest_framework import exceptions, mixins, permissions, response, viewsets @@ -163,7 +164,7 @@ class MusicLibraryViewSet( FederationMixin, mixins.RetrieveModelMixin, viewsets.GenericViewSet ): authentication_classes = [authentication.SignatureAuthentication] - renderer_classes = renderers.get_ap_renderers() + # renderer_classes = renderers.get_ap_renderers() serializer_class = serializers.LibrarySerializer queryset = music_models.Library.objects.all().select_related("actor") lookup_field = "uuid" @@ -176,7 +177,25 @@ class MusicLibraryViewSet( "actor": lb.actor, "name": lb.name, "summary": lb.description, - "items": lb.uploads.for_federation().order_by("-creation_date"), + "items": lb.uploads.for_federation() + .order_by("-creation_date") + .prefetch_related( + Prefetch( + "track", + queryset=music_models.Track.objects.select_related( + "album__artist__attributed_to", + "artist__attributed_to", + "album__attributed_to", + "attributed_to", + "album__attachment_cover", + ).prefetch_related( + "tagged_items__tag", + "album__tagged_items__tag", + "album__artist__tagged_items__tag", + "artist__tagged_items__tag", + ), + ) + ), "item_serializer": serializers.UploadSerializer, } page = request.GET.get("page") @@ -219,7 +238,10 @@ 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" + "library__actor", + "track__artist", + "track__album__artist", + "track__album__attachment_cover", ) serializer_class = serializers.UploadSerializer lookup_field = "uuid" diff --git a/api/funkwhale_api/federation/webfinger.py b/api/funkwhale_api/federation/webfinger.py index 874b3c1587..765c5e5356 100644 --- a/api/funkwhale_api/federation/webfinger.py +++ b/api/funkwhale_api/federation/webfinger.py @@ -41,9 +41,7 @@ def get_resource(resource_string): url = "https://{}/.well-known/webfinger?resource={}".format( hostname, resource_string ) - response = session.get_session().get( - url, verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, timeout=5 - ) + response = session.get_session().get(url) response.raise_for_status() serializer = serializers.ActorWebfingerSerializer(data=response.json()) serializer.is_valid(raise_exception=True) diff --git a/api/funkwhale_api/manage/views.py b/api/funkwhale_api/manage/views.py index 200dccf1fd..c946c37e28 100644 --- a/api/funkwhale_api/manage/views.py +++ b/api/funkwhale_api/manage/views.py @@ -69,9 +69,9 @@ class ManageArtistViewSet( "tracks", Prefetch( "albums", - queryset=music_models.Album.objects.annotate( - tracks_count=Count("tracks") - ), + queryset=music_models.Album.objects.select_related( + "attachment_cover" + ).annotate(tracks_count=Count("tracks")), ), music_views.TAG_PREFETCH, ) @@ -110,7 +110,7 @@ class ManageAlbumViewSet( queryset = ( music_models.Album.objects.all() .order_by("-id") - .select_related("attributed_to", "artist") + .select_related("attributed_to", "artist", "attachment_cover") .prefetch_related("tracks", music_views.TAG_PREFETCH) ) serializer_class = serializers.ManageAlbumSerializer @@ -153,7 +153,9 @@ class ManageTrackViewSet( queryset = ( music_models.Track.objects.all() .order_by("-id") - .select_related("attributed_to", "artist", "album__artist") + .select_related( + "attributed_to", "artist", "album__artist", "album__attachment_cover" + ) .annotate(uploads_count=Coalesce(Subquery(uploads_subquery), 0)) .prefetch_related(music_views.TAG_PREFETCH) ) diff --git a/api/funkwhale_api/moderation/management/commands/mrf_check.py b/api/funkwhale_api/moderation/management/commands/mrf_check.py index b518daa08c..6462bd9a0b 100644 --- a/api/funkwhale_api/moderation/management/commands/mrf_check.py +++ b/api/funkwhale_api/moderation/management/commands/mrf_check.py @@ -6,8 +6,6 @@ import logging from django.core.management.base import BaseCommand, CommandError from django.core import validators -from django.conf import settings - from funkwhale_api.common import session from funkwhale_api.federation import models from funkwhale_api.moderation import mrf @@ -84,10 +82,7 @@ class Command(BaseCommand): content = models.Activity.objects.get(uuid=input).payload elif is_url(input): response = session.get_session().get( - input, - timeout=5, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, - headers={"Content-Type": "application/activity+json"}, + input, headers={"Content-Type": "application/activity+json"}, ) response.raise_for_status() content = response.json() diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 52e5020bbf..07f0d9aa37 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -4,6 +4,7 @@ import factory from funkwhale_api.factories import registry, NoUpdateOnCreate +from funkwhale_api.common import factories as common_factories from funkwhale_api.federation import factories as federation_factories from funkwhale_api.music import licenses from funkwhale_api.tags import factories as tags_factories @@ -81,7 +82,7 @@ class AlbumFactory( title = factory.Faker("sentence", nb_words=3) mbid = factory.Faker("uuid4") release_date = factory.Faker("date_object") - cover = factory.django.ImageField() + attachment_cover = factory.SubFactory(common_factories.AttachmentFactory) artist = factory.SubFactory(ArtistFactory) release_group_id = factory.Faker("uuid4") fid = factory.Faker("federation_url") diff --git a/api/funkwhale_api/music/migrations/0042_album_attachment_cover.py b/api/funkwhale_api/music/migrations/0042_album_attachment_cover.py new file mode 100644 index 0000000000..b27dc03eb5 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0042_album_attachment_cover.py @@ -0,0 +1,20 @@ +# Generated by Django 2.2.6 on 2019-11-12 09:56 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('common', '0004_auto_20191111_1338'), + ('music', '0041_auto_20191021_1705'), + ] + + operations = [ + migrations.AddField( + model_name='album', + name='attachment_cover', + field=models.ForeignKey(null=True, on_delete=django.db.models.deletion.SET_NULL, to='common.Attachment', blank=True), + ), + ] diff --git a/api/funkwhale_api/music/migrations/0043_album_cover_attachment.py b/api/funkwhale_api/music/migrations/0043_album_cover_attachment.py new file mode 100644 index 0000000000..f5da4072aa --- /dev/null +++ b/api/funkwhale_api/music/migrations/0043_album_cover_attachment.py @@ -0,0 +1,41 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations + + +def create_attachments(apps, schema_editor): + Album = apps.get_model("music", "Album") + Attachment = apps.get_model("common", "Attachment") + + album_attachment_mapping = {} + def get_mimetype(path): + if path.lower().endswith('.png'): + return "image/png" + return "image/jpeg" + + for album in Album.objects.filter(attachment_cover=None).exclude(cover="").exclude(cover=None): + album_attachment_mapping[album] = Attachment( + file=album.cover, + size=album.cover.size, + mimetype=get_mimetype(album.cover.path), + ) + + Attachment.objects.bulk_create(album_attachment_mapping.values(), batch_size=2000) + # map each attachment to the corresponding album + # and bulk save + for album, attachment in album_attachment_mapping.items(): + album.attachment_cover = attachment + + Album.objects.bulk_update(album_attachment_mapping.keys(), fields=['attachment_cover'], batch_size=2000) + + +def rewind(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + + dependencies = [("music", "0042_album_attachment_cover")] + + operations = [migrations.RunPython(create_attachments, rewind)] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index f40a4b1860..a1f4a7c642 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -20,7 +20,6 @@ from django.urls import reverse from django.utils import timezone from versatileimagefield.fields import VersatileImageField -from versatileimagefield.image_warmer import VersatileImageFieldWarmer from funkwhale_api import musicbrainz from funkwhale_api.common import fields @@ -286,9 +285,17 @@ class Album(APIModelMixin): artist = models.ForeignKey(Artist, related_name="albums", on_delete=models.CASCADE) release_date = models.DateField(null=True, blank=True, db_index=True) release_group_id = models.UUIDField(null=True, blank=True) + # XXX: 1.0 clean this uneeded field in favor of attachment_cover cover = VersatileImageField( upload_to="albums/covers/%Y/%m/%d", null=True, blank=True ) + attachment_cover = models.ForeignKey( + "common.Attachment", + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="covered_album", + ) TYPE_CHOICES = (("album", "Album"),) type = models.CharField(choices=TYPE_CHOICES, max_length=30, default="album") @@ -334,40 +341,46 @@ 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: - response = session.get_session().get( - data.get("url"), - timeout=3, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, + common_tasks.fetch_remote_attachment( + attachment, filename=filename, save=False ) - response.raise_for_status() except Exception as e: logger.warn( "Cannot download cover at url %s: %s", data.get("url"), e ) return - else: - f = ContentFile(response.content) - if f: - self.cover.save("{}.{}".format(self.uuid, extension), f, save=False) - self.save(update_fields=["cover"]) - return self.cover.file - if self.mbid: + + elif self.mbid: image_data = musicbrainz.api.images.get_front(str(self.mbid)) f = ContentFile(image_data) - self.cover.save("{0}.jpg".format(self.mbid), f, save=False) - self.save(update_fields=["cover"]) - if self.cover: - return self.cover.file + 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 def __str__(self): return self.title @@ -378,16 +391,6 @@ class Album(APIModelMixin): def get_moderation_url(self): return "/manage/library/albums/{}".format(self.pk) - @property - def cover_path(self): - if not self.cover: - return None - try: - return self.cover.path - except NotImplementedError: - # external storage - return self.cover.name - @classmethod def get_or_create_from_title(cls, title, **kwargs): kwargs.update({"title": title}) @@ -415,7 +418,9 @@ def import_album(v): class TrackQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): def for_nested_serialization(self): - return self.prefetch_related("artist", "album__artist") + return self.prefetch_related( + "artist", "album__artist", "album__attachment_cover" + ) def annotate_playable_by_actor(self, actor): @@ -729,7 +734,6 @@ class Upload(models.Model): return parsed.hostname def download_audio_from_remote(self, actor): - from funkwhale_api.common import session from funkwhale_api.federation import signing if actor: @@ -743,7 +747,6 @@ class Upload(models.Model): stream=True, timeout=20, headers={"Content-Type": "application/octet-stream"}, - verify=settings.EXTERNAL_REQUESTS_VERIFY_SSL, ) with remote_response as r: remote_response.raise_for_status() @@ -1307,13 +1310,3 @@ def update_request_status(sender, instance, created, **kwargs): # let's mark the request as imported since the import is over instance.import_request.status = "imported" return instance.import_request.save(update_fields=["status"]) - - -@receiver(models.signals.post_save, sender=Album) -def warm_album_covers(sender, instance, **kwargs): - if not instance.cover or not settings.CREATE_IMAGE_THUMBNAILS: - return - album_covers_warmer = VersatileImageFieldWarmer( - instance_or_queryset=instance, rendition_key_set="square", image_attr="cover" - ) - num_created, failed_to_create = album_covers_warmer.warm() diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 9ea2dd2cbe..66790cf6d6 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -4,7 +4,6 @@ from django.db import transaction from django import urls from django.conf import settings from rest_framework import serializers -from versatileimagefield.serializers import VersatileImageFieldSerializer from funkwhale_api.activity import serializers as activity_serializers from funkwhale_api.common import serializers as common_serializers @@ -17,7 +16,25 @@ from funkwhale_api.tags.models import Tag from . import filters, models, tasks -cover_field = VersatileImageFieldSerializer(allow_null=True, sizes="square") +class NullToEmptDict(object): + def get_attribute(self, o): + attr = super().get_attribute(o) + if attr is None: + return {} + return attr + + def to_representation(self, v): + if not v: + return v + return super().to_representation(v) + + +class CoverField(NullToEmptDict, common_serializers.AttachmentSerializer): + # XXX: BACKWARD COMPATIBILITY + pass + + +cover_field = CoverField() def serialize_attributed_to(self, obj): @@ -450,12 +467,12 @@ class OembedSerializer(serializers.Serializer): embed_type = "track" embed_id = track.pk data["title"] = "{} by {}".format(track.title, track.artist.name) - if track.album.cover: - data["thumbnail_url"] = federation_utils.full_url( - track.album.cover.crop["400x400"].url - ) - data["thumbnail_width"] = 400 - data["thumbnail_height"] = 400 + if track.album.attachment_cover: + data[ + "thumbnail_url" + ] = track.album.attachment_cover.download_url_medium_square_crop + data["thumbnail_width"] = 200 + data["thumbnail_height"] = 200 data["description"] = track.full_name data["author_name"] = track.artist.name data["height"] = 150 @@ -476,12 +493,12 @@ class OembedSerializer(serializers.Serializer): ) embed_type = "album" embed_id = album.pk - if album.cover: - data["thumbnail_url"] = federation_utils.full_url( - album.cover.crop["400x400"].url - ) - data["thumbnail_width"] = 400 - data["thumbnail_height"] = 400 + if album.attachment_cover: + data[ + "thumbnail_url" + ] = album.attachment_cover.download_url_medium_square_crop + data["thumbnail_width"] = 200 + data["thumbnail_height"] = 200 data["title"] = "{} by {}".format(album.title, album.artist.name) data["description"] = "{} by {}".format(album.title, album.artist.name) data["author_name"] = album.artist.name @@ -501,19 +518,14 @@ class OembedSerializer(serializers.Serializer): ) embed_type = "artist" embed_id = artist.pk - album = ( - artist.albums.filter(cover__isnull=False) - .exclude(cover="") - .order_by("-id") - .first() - ) - - if album and album.cover: - data["thumbnail_url"] = federation_utils.full_url( - album.cover.crop["400x400"].url - ) - data["thumbnail_width"] = 400 - data["thumbnail_height"] = 400 + album = artist.albums.exclude(attachment_cover=None).order_by("-id").first() + + if album and album.attachment_cover: + data[ + "thumbnail_url" + ] = album.attachment_cover.download_url_medium_square_crop + data["thumbnail_width"] = 200 + data["thumbnail_height"] = 200 data["title"] = artist.name data["description"] = artist.name data["author_name"] = artist.name @@ -533,19 +545,22 @@ class OembedSerializer(serializers.Serializer): ) embed_type = "playlist" embed_id = obj.pk - playlist_tracks = obj.playlist_tracks.exclude(track__album__cover="") - playlist_tracks = playlist_tracks.exclude(track__album__cover=None) - playlist_tracks = playlist_tracks.select_related("track__album").order_by( - "index" + playlist_tracks = obj.playlist_tracks.exclude( + track__album__attachment_cover=None ) + playlist_tracks = playlist_tracks.select_related( + "track__album__attachment_cover" + ).order_by("index") first_playlist_track = playlist_tracks.first() if first_playlist_track: - data["thumbnail_url"] = federation_utils.full_url( - first_playlist_track.track.album.cover.crop["400x400"].url + data[ + "thumbnail_url" + ] = ( + first_playlist_track.track.album.attachment_cover.download_url_medium_square_crop ) - data["thumbnail_width"] = 400 - data["thumbnail_height"] = 400 + data["thumbnail_width"] = 200 + data["thumbnail_height"] = 200 data["title"] = obj.name data["description"] = obj.name data["author_name"] = obj.name diff --git a/api/funkwhale_api/music/spa_views.py b/api/funkwhale_api/music/spa_views.py index 5215dcdd86..cbf1431629 100644 --- a/api/funkwhale_api/music/spa_views.py +++ b/api/funkwhale_api/music/spa_views.py @@ -57,14 +57,12 @@ def library_track(request, pk): ), }, ] - if obj.album.cover: + if obj.album.attachment_cover: metas.append( { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, obj.album.cover.crop["400x400"].url - ), + "content": obj.album.attachment_cover.download_url_medium_square_crop, } ) @@ -126,14 +124,12 @@ def library_album(request, pk): } ) - if obj.cover: + if obj.attachment_cover: metas.append( { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, obj.cover.crop["400x400"].url - ), + "content": obj.attachment_cover.download_url_medium_square_crop, } ) @@ -166,7 +162,7 @@ def library_artist(request, pk): ) # we use latest album's cover as artist image latest_album = ( - obj.albums.exclude(cover="").exclude(cover=None).order_by("release_date").last() + obj.albums.exclude(attachment_cover=None).order_by("release_date").last() ) metas = [ {"tag": "meta", "property": "og:url", "content": artist_url}, @@ -174,14 +170,12 @@ def library_artist(request, pk): {"tag": "meta", "property": "og:type", "content": "profile"}, ] - if latest_album and latest_album.cover: + if latest_album and latest_album.attachment_cover: metas.append( { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, latest_album.cover.crop["400x400"].url - ), + "content": latest_album.attachment_cover.download_url_medium_square_crop, } ) @@ -217,8 +211,7 @@ def library_playlist(request, pk): utils.spa_reverse("library_playlist", kwargs={"pk": obj.pk}), ) # we use the first playlist track's album's cover as image - playlist_tracks = obj.playlist_tracks.exclude(track__album__cover="") - playlist_tracks = playlist_tracks.exclude(track__album__cover=None) + playlist_tracks = obj.playlist_tracks.exclude(track__album__attachment_cover=None) playlist_tracks = playlist_tracks.select_related("track__album").order_by("index") first_playlist_track = playlist_tracks.first() metas = [ @@ -232,10 +225,7 @@ def library_playlist(request, pk): { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, - first_playlist_track.track.album.cover.crop["400x400"].url, - ), + "content": first_playlist_track.track.album.attachment_cover.download_url_medium_square_crop, } ) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 20265b3206..1bf0f823e0 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -21,7 +21,6 @@ from . import licenses from . import models from . import metadata from . import signals -from . import serializers logger = logging.getLogger(__name__) @@ -29,7 +28,7 @@ logger = logging.getLogger(__name__) def update_album_cover( album, source=None, cover_data=None, musicbrainz=True, replace=False ): - if album.cover and not replace: + if album.attachment_cover and not replace: return if cover_data: return album.get_image(data=cover_data) @@ -257,7 +256,7 @@ def process_upload(upload, update_denormalization=True): ) # update album cover, if needed - if not track.album.cover: + if not track.album.attachment_cover: update_album_cover( track.album, source=final_metadata.get("upload_source"), @@ -404,7 +403,7 @@ def sort_candidates(candidates, important_fields): @transaction.atomic def get_track_from_import_metadata(data, update_cover=False, attributed_to=None): track = _get_track(data, attributed_to=attributed_to) - if update_cover and track and not track.album.cover: + if update_cover and track and not track.album.attachment_cover: update_album_cover( track.album, source=data.get("upload_source"), @@ -584,6 +583,8 @@ def broadcast_import_status_update_to_owner(old_status, new_status, upload, **kw if not user: return + from . import serializers + group = "user.{}.imports".format(user.pk) channels.group_send( group, diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index c451d6b76d..2260d92682 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -128,7 +128,9 @@ class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelV def get_queryset(self): queryset = super().get_queryset() - albums = models.Album.objects.with_tracks_count() + albums = models.Album.objects.with_tracks_count().select_related( + "attachment_cover" + ) albums = albums.annotate_playable_by_actor( utils.get_actor_from_request(self.request) ) @@ -149,7 +151,7 @@ class AlbumViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelVi queryset = ( models.Album.objects.all() .order_by("-creation_date") - .prefetch_related("artist", "attributed_to") + .prefetch_related("artist", "attributed_to", "attachment_cover") ) serializer_class = serializers.AlbumSerializer permission_classes = [oauth_permissions.ScopePermission] diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index 37d498c4f0..affa86ea54 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -17,7 +17,8 @@ class PlaylistQuerySet(models.QuerySet): def with_covers(self): album_prefetch = models.Prefetch( - "album", queryset=music_models.Album.objects.only("cover", "artist_id") + "album", + queryset=music_models.Album.objects.select_related("attachment_cover"), ) track_prefetch = models.Prefetch( "track", @@ -29,8 +30,7 @@ class PlaylistQuerySet(models.QuerySet): plt_prefetch = models.Prefetch( "playlist_tracks", queryset=PlaylistTrack.objects.all() - .exclude(track__album__cover=None) - .exclude(track__album__cover="") + .exclude(track__album__attachment_cover=None) .order_by("index") .only("id", "playlist_id", "track_id") .prefetch_related(track_prefetch), @@ -179,7 +179,9 @@ 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") + tracks = tracks.select_related( + "artist", "album__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 dc61950dde..727edd525b 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -145,7 +145,7 @@ class PlaylistSerializer(serializers.ModelSerializer): for plt in plts: if plt.track.album.artist_id in excluded_artists: continue - url = plt.track.album.cover.crop["200x200"].url + url = plt.track.album.attachment_cover.download_url_medium_square_crop if url in covers: continue covers.append(url) diff --git a/api/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index cae99e2426..7b6e37686b 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -89,7 +89,7 @@ class GetArtistSerializer(serializers.Serializer): "created": to_subsonic_date(album.creation_date), "songCount": len(album.tracks.all()), } - if album.cover: + if album.attachment_cover_id: album_data["coverArt"] = "al-{}".format(album.id) if album.release_date: album_data["year"] = album.release_date.year @@ -122,7 +122,7 @@ def get_track_data(album, track, upload): "artistId": album.artist.pk, "type": "music", } - if track.album.cover: + if track.album.attachment_cover_id: data["coverArt"] = "al-{}".format(track.album.id) if upload.bitrate: data["bitrate"] = int(upload.bitrate / 1000) @@ -141,7 +141,7 @@ def get_album2_data(album): "artist": album.artist.name, "created": to_subsonic_date(album.creation_date), } - if album.cover: + if album.attachment_cover_id: payload["coverArt"] = "al-{}".format(album.id) try: diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index db2620100e..53861572aa 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -16,7 +16,12 @@ from rest_framework.serializers import ValidationError import funkwhale_api from funkwhale_api.activity import record -from funkwhale_api.common import fields, preferences, utils as common_utils +from funkwhale_api.common import ( + fields, + preferences, + utils as common_utils, + tasks as common_tasks, +) from funkwhale_api.favorites.models import TrackFavorite from funkwhale_api.moderation import filters as moderation_filters from funkwhale_api.music import models as music_models @@ -732,20 +737,23 @@ class SubsonicViewSet(viewsets.GenericViewSet): try: album_id = int(id.replace("al-", "")) album = ( - music_models.Album.objects.exclude(cover__isnull=True) - .exclude(cover="") + music_models.Album.objects.exclude(attachment_cover=None) + .select_related("attachment_cover") .get(pk=album_id) ) except (TypeError, ValueError, music_models.Album.DoesNotExist): return response.Response( {"error": {"code": 70, "message": "cover art not found."}} ) - cover = album.cover + attachment = album.attachment_cover else: return response.Response( {"error": {"code": 70, "message": "cover art not found."}} ) + if not attachment.file: + common_tasks.fetch_remote_attachment(attachment) + cover = attachment.file mapping = {"nginx": "X-Accel-Redirect", "apache2": "X-Sendfile"} path = music_views.get_file_path(cover) file_header = mapping[settings.REVERSE_PROXY_TYPE] diff --git a/api/tests/common/test_models.py b/api/tests/common/test_models.py index a2ea89ef2a..b5a56614d5 100644 --- a/api/tests/common/test_models.py +++ b/api/tests/common/test_models.py @@ -46,3 +46,28 @@ def test_get_moderation_url(factory_name, factories, expected): obj = factories[factory_name]() assert obj.get_moderation_url() == expected.format(obj=obj) + + +def test_attachment(factories, now): + attachment = factories["common.Attachment"]() + + assert attachment.uuid is not None + assert attachment.mimetype == "image/jpeg" + assert attachment.file is not None + assert attachment.url is not None + assert attachment.actor is not None + assert attachment.creation_date > now + assert attachment.last_fetch_date is None + assert attachment.size > 0 + + +@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["common.Attachment"](), + ] + + queryset = attachments[0].__class__.objects.attached(*args).order_by("id") + expected_objs = [attachments[i] for i in expected] + assert queryset == expected_objs diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py index 6d20443af6..067a9e4a20 100644 --- a/api/tests/common/test_serializers.py +++ b/api/tests/common/test_serializers.py @@ -2,11 +2,13 @@ import os import PIL from django.core.files.uploadedfile import SimpleUploadedFile +from django.urls import reverse import django_filters from funkwhale_api.common import serializers from funkwhale_api.users import models +from funkwhale_api.federation import utils as federation_utils class TestActionFilterSet(django_filters.FilterSet): @@ -182,3 +184,71 @@ def test_strip_exif_field(): cleaned = PIL.Image.open(field.to_internal_value(uploaded)) assert cleaned._getexif() is None + + +def test_attachment_serializer_existing_file(factories, to_api_date): + attachment = factories["common.Attachment"]() + expected = { + "uuid": str(attachment.uuid), + "size": attachment.size, + "mimetype": attachment.mimetype, + "creation_date": to_api_date(attachment.creation_date), + "urls": { + "source": attachment.url, + "original": federation_utils.full_url(attachment.file.url), + "medium_square_crop": federation_utils.full_url( + attachment.file.crop["200x200"].url + ), + }, + # XXX: BACKWARD COMPATIBILITY + "original": federation_utils.full_url(attachment.file.url), + "medium_square_crop": federation_utils.full_url( + attachment.file.crop["200x200"].url + ), + "small_square_crop": federation_utils.full_url( + attachment.file.crop["200x200"].url + ), + "square_crop": federation_utils.full_url(attachment.file.crop["200x200"].url), + } + + serializer = serializers.AttachmentSerializer(attachment) + + assert serializer.data == expected + + +def test_attachment_serializer_remote_file(factories, to_api_date): + attachment = factories["common.Attachment"](file=None) + proxy_url = reverse("api:v1:attachments-proxy", kwargs={"uuid": attachment.uuid}) + expected = { + "uuid": str(attachment.uuid), + "size": attachment.size, + "mimetype": attachment.mimetype, + "creation_date": to_api_date(attachment.creation_date), + # everything is the same, except for the urls field because: + # - the file isn't available on the local pod + # - we need to return different URLs so that the client can trigger + # a fetch and get redirected to the desired version + # + "urls": { + "source": attachment.url, + "original": federation_utils.full_url(proxy_url + "?next=original"), + "medium_square_crop": federation_utils.full_url( + proxy_url + "?next=medium_square_crop" + ), + }, + # XXX: BACKWARD COMPATIBILITY + "original": federation_utils.full_url(proxy_url + "?next=original"), + "medium_square_crop": federation_utils.full_url( + proxy_url + "?next=medium_square_crop" + ), + "square_crop": federation_utils.full_url( + proxy_url + "?next=medium_square_crop" + ), + "small_square_crop": federation_utils.full_url( + proxy_url + "?next=medium_square_crop" + ), + } + + serializer = serializers.AttachmentSerializer(attachment) + + assert serializer.data == expected diff --git a/api/tests/common/test_tasks.py b/api/tests/common/test_tasks.py index f097c44231..fc62d901b9 100644 --- a/api/tests/common/test_tasks.py +++ b/api/tests/common/test_tasks.py @@ -1,4 +1,5 @@ import pytest +import datetime from funkwhale_api.common import serializers from funkwhale_api.common import signals @@ -63,3 +64,25 @@ def test_cannot_apply_already_applied_migration(factories): mutation = factories["common.Mutation"](payload={}, is_applied=True) with pytest.raises(mutation.__class__.DoesNotExist): tasks.apply_mutation(mutation_id=mutation.pk) + + +def test_prune_unattached_attachments(factories, settings, now): + settings.ATTACHMENTS_UNATTACHED_PRUNE_DELAY = 5 + attachments = [ + # attached, kept + factories["music.Album"]().attachment_cover, + # recent, kept + factories["common.Attachment"](), + # too old, pruned + factories["common.Attachment"]( + creation_date=now + - datetime.timedelta(seconds=settings.ATTACHMENTS_UNATTACHED_PRUNE_DELAY) + ), + ] + + tasks.prune_unattached_attachments() + + attachments[0].refresh_from_db() + attachments[1].refresh_from_db() + with pytest.raises(attachments[2].DoesNotExist): + attachments[2].refresh_from_db() diff --git a/api/tests/common/test_views.py b/api/tests/common/test_views.py index c1cbfd761e..0ca7cbfd93 100644 --- a/api/tests/common/test_views.py +++ b/api/tests/common/test_views.py @@ -1,4 +1,6 @@ +import io import pytest + from django.urls import reverse from funkwhale_api.common import serializers @@ -181,3 +183,69 @@ def test_rate_limit(logged_in_api_client, now_time, settings, mocker): assert response.status_code == 200 assert response.data == expected get_status.assert_called_once_with(expected_ident, now_time) + + +@pytest.mark.parametrize( + "next, expected", + [ + ("original", "original"), + ("medium_square_crop", "medium_square_crop"), + ("unknown", "original"), + ], +) +def test_attachment_proxy_redirects_original( + next, expected, factories, logged_in_api_client, mocker, avatar, r_mock, now +): + attachment = factories["common.Attachment"](file=None) + + avatar_content = avatar.read() + fetch_remote_attachment = mocker.spy(tasks, "fetch_remote_attachment") + m = r_mock.get(attachment.url, body=io.BytesIO(avatar_content)) + proxy_url = reverse("api:v1:attachments-proxy", kwargs={"uuid": attachment.uuid}) + + response = logged_in_api_client.get(proxy_url, {"next": next}) + attachment.refresh_from_db() + + urls = serializers.AttachmentSerializer(attachment).data["urls"] + + assert attachment.file.read() == avatar_content + assert attachment.last_fetch_date == now + fetch_remote_attachment.assert_called_once_with(attachment) + assert len(m.request_history) == 1 + assert response.status_code == 302 + assert response["Location"] == urls[expected] + + +def test_attachment_create(logged_in_api_client, avatar): + actor = logged_in_api_client.user.create_actor() + url = reverse("api:v1:attachments-list") + content = avatar.read() + avatar.seek(0) + payload = {"file": avatar} + response = logged_in_api_client.post(url, payload) + + assert response.status_code == 201 + attachment = actor.attachments.latest("id") + assert attachment.file.read() == content + assert attachment.file.size == len(content) + + +def test_attachment_destroy(factories, logged_in_api_client): + actor = logged_in_api_client.user.create_actor() + attachment = factories["common.Attachment"](actor=actor) + url = reverse("api:v1:attachments-detail", kwargs={"uuid": attachment.uuid}) + response = logged_in_api_client.delete(url) + + assert response.status_code == 204 + with pytest.raises(attachment.DoesNotExist): + attachment.refresh_from_db() + + +def test_attachment_destroy_not_owner(factories, logged_in_api_client): + logged_in_api_client.user.create_actor() + attachment = factories["common.Attachment"]() + url = reverse("api:v1:attachments-detail", kwargs={"uuid": attachment.uuid}) + response = logged_in_api_client.delete(url) + + assert response.status_code == 403 + attachment.refresh_from_db() diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index a0b773fb4e..34460a5a66 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -589,7 +589,7 @@ def test_activity_pub_album_serializer_to_ap(factories): "cover": { "type": "Link", "mediaType": "image/jpeg", - "href": utils.full_url(album.cover.url), + "href": utils.full_url(album.attachment_cover.file.url), }, "musicbrainzId": album.mbid, "published": album.creation_date.isoformat(), @@ -729,8 +729,8 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): assert str(track.mbid) == data["musicbrainzId"] assert album.from_activity == activity - assert album.cover.read() == b"coucou" - assert album.cover_path.endswith(".png") + assert album.attachment_cover.file.read() == b"coucou" + assert album.attachment_cover.file.path.endswith(".png") assert album.title == data["album"]["name"] assert album.fid == data["album"]["id"] assert str(album.mbid) == data["album"]["musicbrainzId"] diff --git a/api/tests/manage/test_serializers.py b/api/tests/manage/test_serializers.py index d8a2ee8f9e..37490383aa 100644 --- a/api/tests/manage/test_serializers.py +++ b/api/tests/manage/test_serializers.py @@ -1,7 +1,8 @@ import pytest -from funkwhale_api.manage import serializers +from funkwhale_api.common import serializers as common_serializers from funkwhale_api.federation import tasks as federation_tasks +from funkwhale_api.manage import serializers def test_manage_upload_action_delete(factories): @@ -339,12 +340,7 @@ def test_manage_nested_album_serializer(factories, now, to_api_date): "mbid": album.mbid, "creation_date": to_api_date(album.creation_date), "release_date": album.release_date.isoformat(), - "cover": { - "original": album.cover.url, - "square_crop": album.cover.crop["400x400"].url, - "medium_square_crop": album.cover.crop["200x200"].url, - "small_square_crop": album.cover.crop["50x50"].url, - }, + "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "tracks_count": 44, } s = serializers.ManageNestedAlbumSerializer(album) @@ -380,12 +376,7 @@ def test_manage_album_serializer(factories, now, to_api_date): "mbid": album.mbid, "creation_date": to_api_date(album.creation_date), "release_date": album.release_date.isoformat(), - "cover": { - "original": album.cover.url, - "square_crop": album.cover.crop["400x400"].url, - "medium_square_crop": album.cover.crop["200x200"].url, - "small_square_crop": album.cover.crop["50x50"].url, - }, + "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "artist": serializers.ManageNestedArtistSerializer(album.artist).data, "tracks": [serializers.ManageNestedTrackSerializer(track).data], "attributed_to": serializers.ManageBaseActorSerializer( diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py index a5090e89ac..c723b0104a 100644 --- a/api/tests/music/test_models.py +++ b/api/tests/music/test_models.py @@ -192,7 +192,7 @@ def test_album_get_image_content(factories): album.get_image(data={"content": b"test", "mimetype": "image/jpeg"}) album.refresh_from_db() - assert album.cover.read() == b"test" + assert album.attachment_cover.file.read() == b"test" def test_library(factories): diff --git a/api/tests/music/test_music.py b/api/tests/music/test_music.py index 727214af56..0709ab3404 100644 --- a/api/tests/music/test_music.py +++ b/api/tests/music/test_music.py @@ -132,11 +132,11 @@ def test_can_download_image_file_for_album(binary_cover, mocker, factories): album.get_image() album.save() - assert album.cover.file.read() == binary_cover + assert album.attachment_cover.file.read() == binary_cover def test_album_get_image_doesnt_crash_with_empty_data(mocker, factories): - album = factories["music.Album"](mbid=None, cover=None) + album = factories["music.Album"](mbid=None, attachment_cover=None) assert ( album.get_image(data={"content": "", "url": "", "mimetype": "image/png"}) is None diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 4eaf54ad54..a3e94e768c 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -1,5 +1,6 @@ import pytest +from funkwhale_api.common import serializers as common_serializers from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.music import licenses from funkwhale_api.music import models @@ -42,12 +43,7 @@ def test_artist_album_serializer(factories, to_api_date): "creation_date": to_api_date(album.creation_date), "tracks_count": 1, "is_playable": None, - "cover": { - "original": album.cover.url, - "square_crop": album.cover.crop["400x400"].url, - "medium_square_crop": album.cover.crop["200x200"].url, - "small_square_crop": album.cover.crop["50x50"].url, - }, + "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "release_date": to_api_date(album.release_date), "is_local": album.is_local, } @@ -172,12 +168,7 @@ def test_album_serializer(factories, to_api_date): "artist": serializers.serialize_artist_simple(album.artist), "creation_date": to_api_date(album.creation_date), "is_playable": False, - "cover": { - "original": album.cover.url, - "square_crop": album.cover.crop["400x400"].url, - "medium_square_crop": album.cover.crop["200x200"].url, - "small_square_crop": album.cover.crop["50x50"].url, - }, + "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "release_date": to_api_date(album.release_date), "tracks": [serializers.serialize_album_track(t) for t in [track2, track1]], "is_local": album.is_local, @@ -189,6 +180,15 @@ def test_album_serializer(factories, to_api_date): assert serializer.data == expected +def test_album_serializer_empty_cover(factories, to_api_date): + # XXX: BACKWARD COMPATIBILITY + album = factories["music.Album"](attachment_cover=None) + + serializer = serializers.AlbumSerializer(album) + + assert serializer.data["cover"] == {} + + def test_track_serializer(factories, to_api_date): actor = factories["federation.Actor"]() upload = factories["music.Upload"]( diff --git a/api/tests/music/test_spa_views.py b/api/tests/music/test_spa_views.py index bf85ab8882..42f7234602 100644 --- a/api/tests/music/test_spa_views.py +++ b/api/tests/music/test_spa_views.py @@ -49,9 +49,7 @@ def test_library_track(spa_html, no_api_auth, client, factories, settings): { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, track.album.cover.crop["400x400"].url - ), + "content": track.album.attachment_cover.download_url_medium_square_crop, }, { "tag": "meta", @@ -116,9 +114,7 @@ def test_library_album(spa_html, no_api_auth, client, factories, settings): { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, album.cover.crop["400x400"].url - ), + "content": album.attachment_cover.download_url_medium_square_crop, }, { "tag": "link", @@ -166,9 +162,7 @@ def test_library_artist(spa_html, no_api_auth, client, factories, settings): { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, album.cover.crop["400x400"].url - ), + "content": album.attachment_cover.download_url_medium_square_crop, }, { "tag": "link", @@ -217,9 +211,7 @@ def test_library_playlist(spa_html, no_api_auth, client, factories, settings): { "tag": "meta", "property": "og:image", - "content": utils.join_url( - settings.FUNKWHALE_URL, track.album.cover.crop["400x400"].url - ), + "content": track.album.attachment_cover.download_url_medium_square_crop, }, { "tag": "link", diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index fbbb0197ae..65ed3f9b93 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -285,8 +285,11 @@ def test_can_create_track_from_file_metadata_federation(factories, mocker, r_moc assert track.fid == metadata["fid"] assert track.creation_date == metadata["fdate"] assert track.position == 4 - assert track.album.cover.read() == b"coucou" - assert track.album.cover_path.endswith(".png") + assert track.album.attachment_cover.file.read() == b"coucou" + assert track.album.attachment_cover.file.path.endswith(".png") + assert track.album.attachment_cover.url == metadata["cover_data"]["url"] + assert track.album.attachment_cover.mimetype == metadata["cover_data"]["mimetype"] + assert track.album.fid == metadata["album"]["fid"] assert track.album.title == metadata["album"]["title"] assert track.album.creation_date == metadata["album"]["fdate"] @@ -312,7 +315,7 @@ def test_upload_import(now, factories, temp_signal, mocker): update_album_cover = mocker.patch("funkwhale_api.music.tasks.update_album_cover") get_picture = mocker.patch("funkwhale_api.music.metadata.Metadata.get_picture") get_track_from_import_metadata = mocker.spy(tasks, "get_track_from_import_metadata") - track = factories["music.Track"](album__cover="") + track = factories["music.Track"](album__attachment_cover=None) upload = factories["music.Upload"]( track=None, import_metadata={"funkwhale": {"track": {"uuid": str(track.uuid)}}} ) @@ -531,7 +534,7 @@ def test_upload_import_error_metadata(factories, now, temp_signal, mocker): def test_upload_import_updates_cover_if_no_cover(factories, mocker, now): mocked_update = mocker.patch("funkwhale_api.music.tasks.update_album_cover") - album = factories["music.Album"](cover="") + album = factories["music.Album"](attachment_cover=None) track = factories["music.Track"](album=album) upload = factories["music.Upload"]( track=None, import_metadata={"funkwhale": {"track": {"uuid": track.uuid}}} @@ -541,7 +544,7 @@ def test_upload_import_updates_cover_if_no_cover(factories, mocker, now): def test_update_album_cover_mbid(factories, mocker): - album = factories["music.Album"](cover="") + album = factories["music.Album"](attachment_cover=None) mocked_get = mocker.patch("funkwhale_api.music.models.Album.get_image") tasks.update_album_cover(album=album) @@ -550,7 +553,7 @@ def test_update_album_cover_mbid(factories, mocker): def test_update_album_cover_file_data(factories, mocker): - album = factories["music.Album"](cover="", mbid=None) + album = factories["music.Album"](attachment_cover=None, mbid=None) mocked_get = mocker.patch("funkwhale_api.music.models.Album.get_image") tasks.update_album_cover(album=album, cover_data={"hello": "world"}) @@ -563,7 +566,7 @@ def test_update_album_cover_file_cover_separate_file(ext, mimetype, factories, m image_path = os.path.join(DATA_DIR, "cover.{}".format(ext)) with open(image_path, "rb") as f: image_content = f.read() - album = factories["music.Album"](cover="", mbid=None) + album = factories["music.Album"](attachment_cover=None, mbid=None) mocked_get = mocker.patch("funkwhale_api.music.models.Album.get_image") mocker.patch("funkwhale_api.music.metadata.Metadata.get_picture", return_value=None) diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 22f7da6352..ad3bf04133 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -780,10 +780,10 @@ def test_oembed_track(factories, no_api_auth, api_client, settings): "title": "{} by {}".format(track.title, track.artist.name), "description": track.full_name, "thumbnail_url": federation_utils.full_url( - track.album.cover.crop["400x400"].url + track.album.attachment_cover.file.crop["200x200"].url ), - "thumbnail_height": 400, - "thumbnail_width": 400, + "thumbnail_height": 200, + "thumbnail_width": 200, "html": '<iframe width="600" height="150" scrolling="no" frameborder="no" src="{}"></iframe>'.format( iframe_src ), @@ -815,9 +815,11 @@ def test_oembed_album(factories, no_api_auth, api_client, settings): "width": 600, "title": "{} by {}".format(album.title, album.artist.name), "description": "{} by {}".format(album.title, album.artist.name), - "thumbnail_url": federation_utils.full_url(album.cover.crop["400x400"].url), - "thumbnail_height": 400, - "thumbnail_width": 400, + "thumbnail_url": federation_utils.full_url( + album.attachment_cover.file.crop["200x200"].url + ), + "thumbnail_height": 200, + "thumbnail_width": 200, "html": '<iframe width="600" height="400" scrolling="no" frameborder="no" src="{}"></iframe>'.format( iframe_src ), @@ -850,9 +852,11 @@ def test_oembed_artist(factories, no_api_auth, api_client, settings): "width": 600, "title": artist.name, "description": artist.name, - "thumbnail_url": federation_utils.full_url(album.cover.crop["400x400"].url), - "thumbnail_height": 400, - "thumbnail_width": 400, + "thumbnail_url": federation_utils.full_url( + album.attachment_cover.file.crop["200x200"].url + ), + "thumbnail_height": 200, + "thumbnail_width": 200, "html": '<iframe width="600" height="400" scrolling="no" frameborder="no" src="{}"></iframe>'.format( iframe_src ), @@ -886,10 +890,10 @@ def test_oembed_playlist(factories, no_api_auth, api_client, settings): "title": playlist.name, "description": playlist.name, "thumbnail_url": federation_utils.full_url( - track.album.cover.crop["400x400"].url + track.album.attachment_cover.file.crop["200x200"].url ), - "thumbnail_height": 400, - "thumbnail_width": 400, + "thumbnail_height": 200, + "thumbnail_width": 200, "html": '<iframe width="600" height="400" scrolling="no" frameborder="no" src="{}"></iframe>'.format( iframe_src ), diff --git a/api/tests/playlists/test_serializers.py b/api/tests/playlists/test_serializers.py index f84df4bb23..79105d3ccf 100644 --- a/api/tests/playlists/test_serializers.py +++ b/api/tests/playlists/test_serializers.py @@ -95,7 +95,7 @@ def test_playlist_serializer_include_covers(factories, api_request): playlist = factories["playlists.Playlist"]() t1 = factories["music.Track"]() t2 = factories["music.Track"]() - t3 = factories["music.Track"](album__cover=None) + t3 = factories["music.Track"](album__attachment_cover=None) t4 = factories["music.Track"]() t5 = factories["music.Track"]() t6 = factories["music.Track"]() @@ -106,11 +106,11 @@ def test_playlist_serializer_include_covers(factories, api_request): qs = playlist.__class__.objects.with_covers().with_tracks_count() expected = [ - request.build_absolute_uri(t1.album.cover.crop["200x200"].url), - request.build_absolute_uri(t2.album.cover.crop["200x200"].url), - request.build_absolute_uri(t4.album.cover.crop["200x200"].url), - request.build_absolute_uri(t5.album.cover.crop["200x200"].url), - request.build_absolute_uri(t6.album.cover.crop["200x200"].url), + t1.album.attachment_cover.download_url_medium_square_crop, + t2.album.attachment_cover.download_url_medium_square_crop, + t4.album.attachment_cover.download_url_medium_square_crop, + t5.album.attachment_cover.download_url_medium_square_crop, + t6.album.attachment_cover.download_url_medium_square_crop, ] serializer = serializers.PlaylistSerializer(qs.get(), context={"request": request}) diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index 3dd9f6cc63..0e7acfd66d 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -724,7 +724,7 @@ def test_get_cover_art_album(factories, logged_in_api_client): assert response.status_code == 200 assert response["Content-Type"] == "" assert response["X-Accel-Redirect"] == music_views.get_file_path( - album.cover + album.attachment_cover.file ).decode("utf-8") diff --git a/docs/swagger.yml b/docs/swagger.yml index a6952098b1..eff119aa54 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -185,6 +185,8 @@ tags: url: https://docs.funkwhale.audio/users/managing.html - name: Content curation description: Favorites, playlists, radios + - name: Other + description: Other endpoints that don't fit in the categories above paths: /api/v1/oauth/apps/: @@ -1022,6 +1024,54 @@ paths: 204: $ref: "#/responses/204" + /api/v1/attachments/: + post: + tags: + - "Other" + description: + Upload a new file as an attachment that can be later associated with other objects. + responses: + 201: + $ref: "#/responses/201" + 400: + $ref: "#/responses/400" + requestBody: + required: true + content: + multipart/form-data: + schema: + type: object + properties: + file: + type: string + format: binary + + /api/v1/attachments/{uuid}/: + parameters: + - name: uuid + in: path + required: true + schema: + type: "string" + format: "uuid" + get: + summary: Retrieve an attachment + tags: + - "Other" + responses: + 200: + content: + application/json: + schema: + $ref: "#/definitions/Attachment" + delete: + summary: Delete an attachment + tags: + - "Other" + responses: + 204: + $ref: "#/responses/204" + parameters: ObjectId: name: id @@ -1114,6 +1164,12 @@ properties: - "audio/mpeg" - "audio/x-flac" - "audio/flac" + image_mimetype: + type: string + example: "image/png" + enum: + - "image/png" + - "image/jpeg" import_status: type: string example: "finished" @@ -1180,28 +1236,33 @@ definitions: format: "uri" description: "Link to the previous page of results" - - Image: + Attachment: type: "object" properties: - original: - type: "string" - description: "URL to the original image" - example: "https://mydomain/media/albums/covers/ec2c53aeaac6.jpg" - small_square_crop: - type: "string" - description: "URL to a small, squared thumbnail of the image" - example: "https://mydomain/media/__sized__/albums/covers/ec2c53aeaac6-crop-c0-5__0-5-50x50-70.jpg" - - medium_square_crop: - type: "string" - description: "URL to a medium, squared thumbnail of the image" - example: "https://mydomain/media/__sized__/albums/covers/ec2c53aeaac6-crop-c0-5__0-5-200x200-70.jpg" - - square_crop: + uuid: + type: string + format: uuid + size: + type: "integer" + format: "int64" + example: 2787000 + description: "Size of the file, in bytes" + mimetype: + $ref: "#/properties/image_mimetype" + creation_date: type: "string" - description: "URL to a large, squared thumbnail of the image" - example: "https://mydomain/media/__sized__/albums/covers/ec2c53aeaac6-crop-c0-5__0-5-400x400-70.jpg" + format: "date-time" + urls: + type: "object" + properties: + original: + type: "string" + description: "URL to the original image" + example: "https://mydomain/media/attachments/ec2c53aeaac6.jpg" + medium_square_crop: + type: "string" + description: "URL to a medium, squared thumbnail of the image" + example: "https://mydomain/media/__sized__/attachments/ec2c53aeaac6-crop-c0-5__0-5-200x200-70.jpg" Actor: type: object @@ -1317,7 +1378,7 @@ definitions: is_playable: type: "boolean" cover: - $ref: "#/definitions/Image" + $ref: "#/definitions/Attachment" is_local: type: "boolean" description: "Indicates if the object was initally created locally or on another server" @@ -1508,7 +1569,7 @@ definitions: uuid: type: string format: uuid - size: + size:size: type: "integer" format: "int64" example: 278987000 -- GitLab