From b38cec3f69a6c521932d2042124751ecd703f77e Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Tue, 4 Feb 2020 14:24:20 +0100
Subject: [PATCH] Fixed a federation issue related to images
---
api/funkwhale_api/federation/serializers.py | 63 +++++++++++++++------
api/funkwhale_api/music/tasks.py | 7 ++-
api/tests/federation/test_serializers.py | 41 +++++++-------
3 files changed, 72 insertions(+), 39 deletions(-)
diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 96ff2fcd3..c057799a6 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -31,17 +31,9 @@ class TruncatedCharField(serializers.CharField):
return v
-class LinkSerializer(jsonld.JsonLdSerializer):
- type = serializers.ChoiceField(choices=[contexts.AS.Link, contexts.AS.Image])
- href = serializers.URLField(max_length=500)
+class MediaSerializer(jsonld.JsonLdSerializer):
mediaType = serializers.CharField()
- class Meta:
- jsonld_mapping = {
- "href": jsonld.first_id(contexts.AS.href),
- "mediaType": jsonld.first_val(contexts.AS.mediaType),
- }
-
def __init__(self, *args, **kwargs):
self.allowed_mimetypes = kwargs.pop("allowed_mimetypes", [])
super().__init__(*args, **kwargs)
@@ -62,6 +54,43 @@ class LinkSerializer(jsonld.JsonLdSerializer):
)
+class LinkSerializer(MediaSerializer):
+ type = serializers.ChoiceField(choices=[contexts.AS.Link])
+ href = serializers.URLField(max_length=500)
+
+ class Meta:
+ jsonld_mapping = {
+ "href": jsonld.first_id(contexts.AS.href),
+ "mediaType": jsonld.first_val(contexts.AS.mediaType),
+ }
+
+
+class ImageSerializer(MediaSerializer):
+ type = serializers.ChoiceField(choices=[contexts.AS.Image, contexts.AS.Link])
+ href = serializers.URLField(max_length=500, required=False)
+ url = serializers.URLField(max_length=500, required=False)
+
+ class Meta:
+ jsonld_mapping = {
+ "url": jsonld.first_id(contexts.AS.url),
+ "href": jsonld.first_id(contexts.AS.href),
+ "mediaType": jsonld.first_val(contexts.AS.mediaType),
+ }
+
+ def validate(self, data):
+ validated_data = super().validate(data)
+ if "url" not in validated_data:
+ try:
+ validated_data["url"] = validated_data["href"]
+ except KeyError:
+ if self.required:
+ raise serializers.ValidationError(
+ "You need to provide a url or href"
+ )
+
+ return validated_data
+
+
class EndpointsSerializer(jsonld.JsonLdSerializer):
sharedInbox = serializers.URLField(max_length=500, required=False)
@@ -95,7 +124,7 @@ class ActorSerializer(jsonld.JsonLdSerializer):
following = serializers.URLField(max_length=500, required=False, allow_null=True)
publicKey = PublicKeySerializer(required=False)
endpoints = EndpointsSerializer(required=False)
- icon = LinkSerializer(
+ icon = ImageSerializer(
allowed_mimetypes=["image/*"], allow_null=True, required=False
)
@@ -202,7 +231,7 @@ class ActorSerializer(jsonld.JsonLdSerializer):
common_utils.attach_file(
actor,
"attachment_icon",
- {"url": new_value["href"], "mimetype": new_value["mediaType"]}
+ {"url": new_value["url"], "mimetype": new_value["mediaType"]}
if new_value
else None,
)
@@ -853,7 +882,7 @@ def include_image(repr, attachment, field="image"):
if attachment:
repr[field] = {
"type": "Image",
- "href": attachment.download_url_original,
+ "url": attachment.download_url_original,
"mediaType": attachment.mimetype or "image/jpeg",
}
else:
@@ -915,14 +944,14 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer):
if (
instance.attachment_cover
- and instance.attachment_cover.url == attachment_cover["href"]
+ and instance.attachment_cover.url == attachment_cover["url"]
):
# 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"],
+ url=attachment_cover["url"],
actor=instance.attributed_to,
)
return validated_data
@@ -938,7 +967,7 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer):
class ArtistSerializer(MusicEntitySerializer):
- image = LinkSerializer(
+ image = ImageSerializer(
allowed_mimetypes=["image/*"], allow_null=True, required=False
)
updateable_fields = [
@@ -982,7 +1011,7 @@ 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(
+ cover = ImageSerializer(
allowed_mimetypes=["image/*"], allow_null=True, required=False
)
updateable_fields = [
@@ -1045,7 +1074,7 @@ class TrackSerializer(MusicEntitySerializer):
album = AlbumSerializer()
license = serializers.URLField(allow_null=True, required=False)
copyright = serializers.CharField(allow_null=True, required=False)
- image = LinkSerializer(
+ image = ImageSerializer(
allowed_mimetypes=["image/*"], allow_null=True, required=False
)
diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py
index 636a54cf4..dce014f15 100644
--- a/api/funkwhale_api/music/tasks.py
+++ b/api/funkwhale_api/music/tasks.py
@@ -302,7 +302,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"]}
+ try:
+ url = cover["url"]
+ except KeyError:
+ url = cover["href"]
+
+ return {"mimetype": cover["mediaType"], "url": url}
def federation_audio_track_to_metadata(payload, references):
diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py
index 555684e07..d5a781bd6 100644
--- a/api/tests/federation/test_serializers.py
+++ b/api/tests/federation/test_serializers.py
@@ -39,7 +39,7 @@ def test_actor_serializer_from_ap(db):
"icon": {
"type": "Image",
"mediaType": "image/jpeg",
- "href": "https://image.example/image.png",
+ "url": "https://image.example/image.png",
},
}
@@ -65,7 +65,7 @@ def test_actor_serializer_from_ap(db):
assert actor.private_key is None
assert actor.public_key == payload["publicKey"]["publicKeyPem"]
assert actor.domain_id == "test.federation"
- assert actor.attachment_icon.url == payload["icon"]["href"]
+ assert actor.attachment_icon.url == payload["icon"]["url"]
assert actor.attachment_icon.mimetype == payload["icon"]["mediaType"]
@@ -139,7 +139,7 @@ def test_actor_serializer_to_ap(factories):
expected["icon"] = {
"type": "Image",
"mediaType": "image/jpeg",
- "href": utils.full_url(ac.attachment_icon.file.url),
+ "url": utils.full_url(ac.attachment_icon.file.url),
}
serializer = serializers.ActorSerializer(ac)
@@ -595,7 +595,7 @@ def test_activity_pub_artist_serializer_to_ap(factories):
"image": {
"type": "Image",
"mediaType": "image/jpeg",
- "href": utils.full_url(artist.attachment_cover.file.url),
+ "url": utils.full_url(artist.attachment_cover.file.url),
},
"tag": [
{"type": "Hashtag", "name": "#Punk"},
@@ -626,7 +626,7 @@ def test_activity_pub_album_serializer_to_ap(factories):
"image": {
"type": "Image",
"mediaType": "image/jpeg",
- "href": utils.full_url(album.attachment_cover.file.url),
+ "url": utils.full_url(album.attachment_cover.file.url),
},
"musicbrainzId": album.mbid,
"published": album.creation_date.isoformat(),
@@ -661,7 +661,7 @@ def test_activity_pub_artist_serializer_from_ap_update(factories, faker):
"attributedTo": artist.attributed_to.fid,
"mediaType": "text/html",
"content": common_utils.render_html(faker.sentence(), "text/html"),
- "image": {"type": "Image", "mediaType": "image/jpeg", "href": faker.url()},
+ "image": {"type": "Image", "mediaType": "image/jpeg", "url": faker.url()},
"tag": [
{"type": "Hashtag", "name": "#Punk"},
{"type": "Hashtag", "name": "#Rock"},
@@ -677,7 +677,7 @@ def test_activity_pub_artist_serializer_from_ap_update(factories, faker):
assert artist.name == payload["name"]
assert str(artist.mbid) == payload["musicbrainzId"]
- assert artist.attachment_cover.url == payload["image"]["href"]
+ assert artist.attachment_cover.url == payload["image"]["url"]
assert artist.attachment_cover.mimetype == payload["image"]["mediaType"]
assert artist.description.text == payload["content"]
assert artist.description.content_type == "text/html"
@@ -767,7 +767,7 @@ def test_activity_pub_track_serializer_to_ap(factories):
"image": {
"type": "Image",
"mediaType": "image/jpeg",
- "href": utils.full_url(track.attachment_cover.file.url),
+ "url": utils.full_url(track.attachment_cover.file.url),
},
}
serializer = serializers.TrackSerializer(track)
@@ -797,8 +797,8 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
"content": "Hello there",
"attributedTo": track_attributed_to.fid,
"image": {
- "type": "Link",
- "href": "https://cover.image/track.png",
+ "type": "Image",
+ "url": "https://cover.image/track.png",
"mediaType": "image/png",
},
"album": {
@@ -829,8 +829,8 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
"attributedTo": album_artist_attributed_to.fid,
"tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}],
"image": {
- "type": "Link",
- "href": "https://cover.image/album-artist.png",
+ "type": "Image",
+ "url": "https://cover.image/album-artist.png",
"mediaType": "image/png",
},
}
@@ -848,8 +848,8 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
"published": published.isoformat(),
"tag": [{"type": "Hashtag", "name": "ArtistTag"}],
"image": {
- "type": "Link",
- "href": "https://cover.image/artist.png",
+ "type": "Image",
+ "url": "https://cover.image/artist.png",
"mediaType": "image/png",
},
}
@@ -877,7 +877,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
assert str(track.mbid) == data["musicbrainzId"]
assert track.description.text == data["content"]
assert track.description.content_type == "text/html"
- assert track.attachment_cover.url == data["image"]["href"]
+ assert track.attachment_cover.url == data["image"]["url"]
assert track.attachment_cover.mimetype == data["image"]["mediaType"]
assert album.from_activity == activity
@@ -900,7 +900,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
assert artist.attributed_to == artist_attributed_to
assert artist.description.text == data["artists"][0]["content"]
assert artist.description.content_type == data["artists"][0]["mediaType"]
- assert artist.attachment_cover.url == data["artists"][0]["image"]["href"]
+ assert artist.attachment_cover.url == data["artists"][0]["image"]["url"]
assert artist.attachment_cover.mimetype == data["artists"][0]["image"]["mediaType"]
assert album_artist.from_activity == activity
@@ -915,8 +915,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker):
== data["album"]["artists"][0]["mediaType"]
)
assert (
- album_artist.attachment_cover.url
- == data["album"]["artists"][0]["image"]["href"]
+ album_artist.attachment_cover.url == data["album"]["artists"][0]["image"]["url"]
)
assert (
album_artist.attachment_cover.mimetype
@@ -949,7 +948,7 @@ def test_activity_pub_track_serializer_from_ap_update(factories, r_mock, mocker,
"attributedTo": track_attributed_to.fid,
"album": serializers.AlbumSerializer(track.album).data,
"artists": [serializers.ArtistSerializer(track.artist).data],
- "image": {"type": "Link", "mediaType": "image/jpeg", "href": faker.url()},
+ "image": {"type": "Image", "mediaType": "image/jpeg", "url": faker.url()},
"tag": [
{"type": "Hashtag", "name": "#Hello"},
# Ensure we can handle tags without a leading #
@@ -970,7 +969,7 @@ def test_activity_pub_track_serializer_from_ap_update(factories, r_mock, mocker,
assert track.description.content_type == "text/html"
assert track.description.text == "hello there"
assert str(track.mbid) == data["musicbrainzId"]
- assert track.attachment_cover.url == data["image"]["href"]
+ assert track.attachment_cover.url == data["image"]["url"]
assert track.attachment_cover.mimetype == data["image"]["mediaType"]
set_tags.assert_called_once_with(track, *["Hello", "World"])
@@ -1159,7 +1158,7 @@ def test_local_actor_serializer_to_ap(factories):
expected["icon"] = {
"type": "Image",
"mediaType": "image/jpeg",
- "href": utils.full_url(ac.attachment_icon.file.url),
+ "url": utils.full_url(ac.attachment_icon.file.url),
}
serializer = serializers.ActorSerializer(ac)
--
GitLab