From 27f0826195c52bc6e81f11b1684824f0d878f405 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Mon, 22 Jul 2019 09:39:40 +0200 Subject: [PATCH] See #432: expose and federate tags on artists and albums --- api/funkwhale_api/federation/serializers.py | 25 ++++---- api/funkwhale_api/music/tasks.py | 64 ++++++++++++--------- api/tests/federation/test_serializers.py | 20 ++++++- api/tests/music/test_tasks.py | 12 +++- 4 files changed, 78 insertions(+), 43 deletions(-) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index ed40017cc..86e991cea 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -779,6 +779,7 @@ MUSIC_ENTITY_JSONLD_MAPPING = { "published": jsonld.first_val(contexts.AS.published), "musicbrainzId": jsonld.first_val(contexts.FW.musicbrainzId), "attributedTo": jsonld.first_id(contexts.AS.attributedTo), + "tags": jsonld.raw(contexts.AS.tag), } @@ -803,6 +804,9 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer): name = serializers.CharField(max_length=1000) attributedTo = serializers.URLField(max_length=500, allow_null=True, required=False) updateable_fields = [] + tags = serializers.ListField( + child=TagSerializer(), min_length=0, required=False, allow_null=True + ) def update(self, instance, validated_data): attributed_to_fid = validated_data.get("attributedTo") @@ -818,6 +822,12 @@ class MusicEntitySerializer(jsonld.JsonLdSerializer): tags_models.set_tags(instance, *tags) return instance + 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)) + ] + class ArtistSerializer(MusicEntitySerializer): updateable_fields = [ @@ -840,6 +850,7 @@ class ArtistSerializer(MusicEntitySerializer): "attributedTo": instance.attributed_to.fid if instance.attributed_to else None, + "tag": self.get_tags_repr(instance), } if self.context.get("include_ap_context", self.parent is None): @@ -889,6 +900,7 @@ class AlbumSerializer(MusicEntitySerializer): "attributedTo": instance.attributed_to.fid if instance.attributed_to else None, + "tag": self.get_tags_repr(instance), } if instance.cover: d["cover"] = { @@ -909,9 +921,6 @@ class TrackSerializer(MusicEntitySerializer): album = AlbumSerializer() license = serializers.URLField(allow_null=True, required=False) copyright = serializers.CharField(allow_null=True, required=False) - tags = serializers.ListField( - child=TagSerializer(), min_length=0, required=False, allow_null=True - ) updateable_fields = [ ("name", "title"), @@ -934,7 +943,6 @@ class TrackSerializer(MusicEntitySerializer): "disc": jsonld.first_val(contexts.FW.disc), "license": jsonld.first_id(contexts.FW.license), "position": jsonld.first_val(contexts.FW.position), - "tags": jsonld.raw(contexts.AS.tag), }, ) @@ -962,12 +970,7 @@ class TrackSerializer(MusicEntitySerializer): "attributedTo": instance.attributed_to.fid if instance.attributed_to else None, - "tag": [ - {"type": "Hashtag", "name": "#{}".format(tag)} - for tag in sorted( - instance.tagged_items.values_list("tag__name", flat=True) - ) - ], + "tag": self.get_tags_repr(instance), } if self.context.get("include_ap_context", self.parent is None): @@ -977,7 +980,6 @@ class TrackSerializer(MusicEntitySerializer): def create(self, validated_data): from funkwhale_api.music import tasks as music_tasks - tags = [t["name"] for t in validated_data.get("tags", []) or []] references = {} actors_to_fetch = set() actors_to_fetch.add( @@ -1012,7 +1014,6 @@ class TrackSerializer(MusicEntitySerializer): metadata = music_tasks.federation_audio_track_to_metadata( validated_data, references ) - metadata["tags"] = tags from_activity = self.context.get("activity") if from_activity: diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 7372f82c5..b8009fda3 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -298,6 +298,7 @@ def federation_audio_track_to_metadata(payload, references): if payload["album"].get("musicbrainzId") else None, "release_date": payload["album"].get("released"), + "tags": [t["name"] for t in payload["album"].get("tags", []) or []], "artists": [ { "fid": a["id"], @@ -305,6 +306,7 @@ def federation_audio_track_to_metadata(payload, references): "fdate": a["published"], "attributed_to": references.get(a.get("attributedTo")), "mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None, + "tags": [t["name"] for t in a.get("tags", []) or []], } for a in payload["album"]["artists"] ], @@ -316,12 +318,14 @@ def federation_audio_track_to_metadata(payload, references): "fdate": a["published"], "attributed_to": references.get(a.get("attributedTo")), "mbid": str(a["musicbrainzId"]) if a.get("musicbrainzId") else None, + "tags": [t["name"] for t in a.get("tags", []) or []], } for a in payload["artists"] ], # federation "fid": payload["id"], "fdate": payload["published"], + "tags": [t["name"] for t in payload.get("tags", []) or []], } cover = payload["album"].get("cover") if cover: @@ -438,10 +442,10 @@ def _get_track(data, attributed_to=None): # get / create artist and album artist artists = getter(data, "artists", default=[]) - artist = artists[0] - artist_mbid = artist.get("mbid", None) - artist_fid = artist.get("fid", None) - artist_name = artist["name"] + artist_data = artists[0] + artist_mbid = artist_data.get("mbid", None) + artist_fid = artist_data.get("fid", None) + artist_name = artist_data["name"] if artist_mbid: query = Q(mbid=artist_mbid) @@ -454,24 +458,26 @@ def _get_track(data, attributed_to=None): "mbid": artist_mbid, "fid": artist_fid, "from_activity_id": from_activity_id, - "attributed_to": artist.get("attributed_to", attributed_to), + "attributed_to": artist_data.get("attributed_to", attributed_to), } - if artist.get("fdate"): - defaults["creation_date"] = artist.get("fdate") + if artist_data.get("fdate"): + defaults["creation_date"] = artist_data.get("fdate") - artist = get_best_candidate_or_create( + artist, created = get_best_candidate_or_create( models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"] - )[0] + ) + if created: + tags_models.add_tags(artist, *artist_data.get("tags", [])) album_artists = getter(data, "album", "artists", default=artists) or artists - album_artist = album_artists[0] - album_artist_name = album_artist.get("name") + album_artist_data = album_artists[0] + album_artist_name = album_artist_data.get("name") if album_artist_name == artist_name: album_artist = artist else: query = Q(name__iexact=album_artist_name) - album_artist_mbid = album_artist.get("mbid", None) - album_artist_fid = album_artist.get("fid", None) + album_artist_mbid = album_artist_data.get("mbid", None) + album_artist_fid = album_artist_data.get("fid", None) if album_artist_mbid: query |= Q(mbid=album_artist_mbid) if album_artist_fid: @@ -481,19 +487,21 @@ def _get_track(data, attributed_to=None): "mbid": album_artist_mbid, "fid": album_artist_fid, "from_activity_id": from_activity_id, - "attributed_to": album_artist.get("attributed_to", attributed_to), + "attributed_to": album_artist_data.get("attributed_to", attributed_to), } - if album_artist.get("fdate"): - defaults["creation_date"] = album_artist.get("fdate") + if album_artist_data.get("fdate"): + defaults["creation_date"] = album_artist_data.get("fdate") - album_artist = get_best_candidate_or_create( + album_artist, created = get_best_candidate_or_create( models.Artist, query, defaults=defaults, sort_fields=["mbid", "fid"] - )[0] + ) + if created: + tags_models.add_tags(album_artist, *album_artist_data.get("tags", [])) # get / create album - album = data["album"] - album_title = album["title"] - album_fid = album.get("fid", None) + album_data = data["album"] + album_title = album_data["title"] + album_fid = album_data.get("fid", None) if album_mbid: query = Q(mbid=album_mbid) @@ -506,17 +514,19 @@ def _get_track(data, attributed_to=None): "title": album_title, "artist": album_artist, "mbid": album_mbid, - "release_date": album.get("release_date"), + "release_date": album_data.get("release_date"), "fid": album_fid, "from_activity_id": from_activity_id, - "attributed_to": album.get("attributed_to", attributed_to), + "attributed_to": album_data.get("attributed_to", attributed_to), } - if album.get("fdate"): - defaults["creation_date"] = album.get("fdate") + if album_data.get("fdate"): + defaults["creation_date"] = album_data.get("fdate") - album = get_best_candidate_or_create( + album, created = get_best_candidate_or_create( models.Album, query, defaults=defaults, sort_fields=["mbid", "fid"] - )[0] + ) + if created: + tags_models.add_tags(album, *album_data.get("tags", [])) # get / create track track_title = data["title"] diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 140f31528..a0b773fb4 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -559,7 +559,7 @@ def test_music_library_serializer_from_private(factories, mocker): def test_activity_pub_artist_serializer_to_ap(factories): - artist = factories["music.Artist"](attributed=True) + artist = factories["music.Artist"](attributed=True, set_tags=["Punk", "Rock"]) expected = { "@context": jsonld.get_default_context(), "type": "Artist", @@ -568,6 +568,10 @@ def test_activity_pub_artist_serializer_to_ap(factories): "musicbrainzId": artist.mbid, "published": artist.creation_date.isoformat(), "attributedTo": artist.attributed_to.fid, + "tag": [ + {"type": "Hashtag", "name": "#Punk"}, + {"type": "Hashtag", "name": "#Rock"}, + ], } serializer = serializers.ArtistSerializer(artist) @@ -575,7 +579,7 @@ def test_activity_pub_artist_serializer_to_ap(factories): def test_activity_pub_album_serializer_to_ap(factories): - album = factories["music.Album"](attributed=True) + album = factories["music.Album"](attributed=True, set_tags=["Punk", "Rock"]) expected = { "@context": jsonld.get_default_context(), @@ -596,6 +600,10 @@ def test_activity_pub_album_serializer_to_ap(factories): ).data ], "attributedTo": album.attributed_to.fid, + "tag": [ + {"type": "Hashtag", "name": "#Punk"}, + {"type": "Hashtag", "name": "#Rock"}, + ], } serializer = serializers.AlbumSerializer(album) @@ -673,6 +681,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): "href": "https://cover.image/test.png", "mediaType": "image/png", }, + "tag": [{"type": "Hashtag", "name": "AlbumTag"}], "artists": [ { "type": "Artist", @@ -681,6 +690,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): "musicbrainzId": str(uuid.uuid4()), "published": published.isoformat(), "attributedTo": album_artist_attributed_to.fid, + "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], } ], }, @@ -692,6 +702,7 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): "musicbrainzId": str(uuid.uuid4()), "attributedTo": artist_attributed_to.fid, "published": published.isoformat(), + "tag": [{"type": "Hashtag", "name": "ArtistTag"}], } ], "tag": [ @@ -741,7 +752,10 @@ def test_activity_pub_track_serializer_from_ap(factories, r_mock, mocker): assert album_artist.creation_date == published assert album_artist.attributed_to == album_artist_attributed_to - add_tags.assert_called_once_with(track, *["Hello", "World"]) + add_tags.assert_any_call(track, *["Hello", "World"]) + add_tags.assert_any_call(album, *["AlbumTag"]) + add_tags.assert_any_call(album_artist, *["AlbumArtistTag"]) + add_tags.assert_any_call(artist, *["ArtistTag"]) def test_activity_pub_track_serializer_from_ap_update(factories, r_mock, mocker): diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index 08beaa94e..51ea71635 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -46,7 +46,9 @@ def test_can_create_track_from_file_metadata_no_mbid(db, mocker): assert track.artist.mbid is None assert track.artist.attributed_to is None match_license.assert_called_once_with(metadata["license"], metadata["copyright"]) - add_tags.assert_called_once_with(track, *metadata["tags"]) + add_tags.assert_any_call(track, *metadata["tags"]) + add_tags.assert_any_call(track.artist, *[]) + add_tags.assert_any_call(track.album, *[]) def test_can_create_track_from_file_metadata_attributed_to(factories, mocker): @@ -558,6 +560,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "license": "http://creativecommons.org/licenses/by-sa/4.0/", "copyright": "2018 Someone", "attributedTo": "http://track.attributed", + "tag": [{"type": "Hashtag", "name": "TrackTag"}], "album": { "published": published.isoformat(), "type": "Album", @@ -565,6 +568,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "name": "Purple album", "musicbrainzId": str(uuid.uuid4()), "released": released.isoformat(), + "tag": [{"type": "Hashtag", "name": "AlbumTag"}], "attributedTo": "http://album.attributed", "artists": [ { @@ -574,6 +578,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "name": "John Smith", "musicbrainzId": str(uuid.uuid4()), "attributedTo": "http://album-artist.attributed", + "tag": [{"type": "Hashtag", "name": "AlbumArtistTag"}], } ], "cover": { @@ -590,6 +595,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "name": "Bob Smith", "musicbrainzId": str(uuid.uuid4()), "attributedTo": "http://artist.attributed", + "tag": [{"type": "Hashtag", "name": "ArtistTag"}], } ], } @@ -605,6 +611,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "fdate": serializer.validated_data["published"], "fid": payload["id"], "attributed_to": references["http://track.attributed"], + "tags": ["TrackTag"], "album": { "title": payload["album"]["name"], "attributed_to": references["http://album.attributed"], @@ -612,6 +619,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "mbid": payload["album"]["musicbrainzId"], "fid": payload["album"]["id"], "fdate": serializer.validated_data["album"]["published"], + "tags": ["AlbumTag"], "artists": [ { "name": a["name"], @@ -621,6 +629,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "fdate": serializer.validated_data["album"]["artists"][i][ "published" ], + "tags": ["AlbumArtistTag"], } for i, a in enumerate(payload["album"]["artists"]) ], @@ -634,6 +643,7 @@ def test_federation_audio_track_to_metadata(now, mocker): "fid": a["id"], "fdate": serializer.validated_data["artists"][i]["published"], "attributed_to": references["http://artist.attributed"], + "tags": ["ArtistTag"], } for i, a in enumerate(payload["artists"]) ],