diff --git a/api/funkwhale_api/favorites/serializers.py b/api/funkwhale_api/favorites/serializers.py index 276b0f6bde6d19120998a21f58f44a5dd3ffbd01..bb4538b2d91c709c86072f0d75d5808e98bbec57 100644 --- a/api/funkwhale_api/favorites/serializers.py +++ b/api/funkwhale_api/favorites/serializers.py @@ -3,7 +3,6 @@ from django.conf import settings from rest_framework import serializers from funkwhale_api.activity import serializers as activity_serializers -from funkwhale_api.music.serializers import TrackSerializerNested from funkwhale_api.music.serializers import TrackActivitySerializer from funkwhale_api.users.serializers import UserActivitySerializer @@ -35,7 +34,6 @@ class TrackFavoriteActivitySerializer(activity_serializers.ModelSerializer): class UserTrackFavoriteSerializer(serializers.ModelSerializer): - # track = TrackSerializerNested(read_only=True) class Meta: model = models.TrackFavorite fields = ('id', 'track', 'creation_date') diff --git a/api/funkwhale_api/history/serializers.py b/api/funkwhale_api/history/serializers.py index f7333f24349777a6a41b15e4de0d6a282502aacd..572787ae0031cb5be38c80014b7484f286de26e3 100644 --- a/api/funkwhale_api/history/serializers.py +++ b/api/funkwhale_api/history/serializers.py @@ -1,7 +1,6 @@ from rest_framework import serializers from funkwhale_api.activity import serializers as activity_serializers -from funkwhale_api.music.serializers import TrackSerializerNested from funkwhale_api.music.serializers import TrackActivitySerializer from funkwhale_api.users.serializers import UserActivitySerializer diff --git a/api/funkwhale_api/history/views.py b/api/funkwhale_api/history/views.py index bea96a4187792421bf6827717eb1032e9dcaacf3..3da8b2a38bed467ffe5fb83f7ea4c8d1d000c406 100644 --- a/api/funkwhale_api/history/views.py +++ b/api/funkwhale_api/history/views.py @@ -6,7 +6,6 @@ from rest_framework.decorators import detail_route from funkwhale_api.activity import record from funkwhale_api.common.permissions import ConditionalAuthentication -from funkwhale_api.music.serializers import TrackSerializerNested from . import models from . import serializers diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 1259cc3c12406a7848649d829dfed7e8999f4539..51ccec90542affbc408f8019ee6fed0ec31c563b 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -80,6 +80,12 @@ class ArtistQuerySet(models.QuerySet): def with_albums_count(self): return self.annotate(_albums_count=models.Count('albums')) + def with_albums(self): + return self.prefetch_related( + models.Prefetch( + 'albums', queryset=Album.objects.with_tracks_count()) + ) + class Artist(APIModelMixin): name = models.CharField(max_length=255) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index d9d48496e487395be4ff0516b64f43b1074f37c1..04264692797131d01cdd813c5fa7cd455c0d4d18 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -13,24 +13,38 @@ from . import models from . import tasks -class TagSerializer(serializers.ModelSerializer): - class Meta: - model = Tag - fields = ('id', 'name', 'slug') - +class ArtistAlbumSerializer(serializers.ModelSerializer): + tracks_count = serializers.SerializerMethodField() -class SimpleArtistSerializer(serializers.ModelSerializer): class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'creation_date') + model = models.Album + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'release_date', + 'cover', + 'creation_date', + 'tracks_count', + ) + def get_tracks_count(self, o): + return o._tracks_count -class ArtistSerializer(serializers.ModelSerializer): - tags = TagSerializer(many=True, read_only=True) + +class ArtistWithAlbumsSerializer(serializers.ModelSerializer): + albums = ArtistAlbumSerializer(many=True, read_only=True) class Meta: model = models.Artist - fields = ('id', 'mbid', 'name', 'tags', 'creation_date') + fields = ( + 'id', + 'mbid', + 'name', + 'creation_date', + 'albums', + ) class TrackFileSerializer(serializers.ModelSerializer): @@ -62,71 +76,107 @@ class TrackFileSerializer(serializers.ModelSerializer): return url -class SimpleAlbumSerializer(serializers.ModelSerializer): +class AlbumTrackSerializer(serializers.ModelSerializer): + files = TrackFileSerializer(many=True, read_only=True) class Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'release_date', 'cover') + model = models.Track + fields = ( + 'id', + 'mbid', + 'title', + 'album', + 'artist', + 'creation_date', + 'files', + 'position', + ) + + +class ArtistSimpleSerializer(serializers.ModelSerializer): + class Meta: + model = models.Artist + fields = ( + 'id', + 'mbid', + 'name', + 'creation_date', + ) class AlbumSerializer(serializers.ModelSerializer): - tags = TagSerializer(many=True, read_only=True) + tracks = serializers.SerializerMethodField() + artist = ArtistSimpleSerializer(read_only=True) + class Meta: model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'release_date', 'tags') - + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'tracks', + 'release_date', + 'cover', + 'creation_date', + ) -class LyricsMixin(serializers.ModelSerializer): - lyrics = serializers.SerializerMethodField() + def get_tracks(self, o): + ordered_tracks = sorted(o.tracks.all(), key=lambda v: v.position) + return AlbumTrackSerializer(ordered_tracks, many=True).data - def get_lyrics(self, obj): - return obj.get_lyrics_url() +class TrackAlbumSerializer(serializers.ModelSerializer): + artist = ArtistSimpleSerializer(read_only=True) -class TrackSerializer(LyricsMixin): - files = TrackFileSerializer(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) class Meta: - model = models.Track + model = models.Album fields = ( 'id', 'mbid', 'title', 'artist', - 'files', - 'tags', - 'position', - 'lyrics') + 'release_date', + 'cover', + 'creation_date', + ) -class TrackSerializerNested(LyricsMixin): - artist = ArtistSerializer() +class TrackSerializer(serializers.ModelSerializer): files = TrackFileSerializer(many=True, read_only=True) - album = SimpleAlbumSerializer(read_only=True) - tags = TagSerializer(many=True, read_only=True) + artist = ArtistSimpleSerializer(read_only=True) + album = TrackAlbumSerializer(read_only=True) + lyrics = serializers.SerializerMethodField() class Meta: model = models.Track - fields = ('id', 'mbid', 'title', 'artist', 'files', 'album', 'tags', 'lyrics') + fields = ( + 'id', + 'mbid', + 'title', + 'album', + 'artist', + 'creation_date', + 'files', + 'position', + 'lyrics', + ) + def get_lyrics(self, obj): + return obj.get_lyrics_url() -class AlbumSerializerNested(serializers.ModelSerializer): - tracks = TrackSerializer(many=True, read_only=True) - artist = SimpleArtistSerializer() - tags = TagSerializer(many=True, read_only=True) +class TagSerializer(serializers.ModelSerializer): class Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'artist', 'release_date', 'tracks', 'tags') + model = Tag + fields = ('id', 'name', 'slug') -class ArtistSerializerNested(serializers.ModelSerializer): - albums = AlbumSerializerNested(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) +class SimpleAlbumSerializer(serializers.ModelSerializer): class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'albums', 'tags') + model = models.Album + fields = ('id', 'mbid', 'title', 'release_date', 'cover') class LyricsSerializer(serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index e71d3555e67a21012ee34d79e2fd56b28354fcf8..3a03a9227a5a4d18e11aafb27d87d5740d54738f 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -68,13 +68,8 @@ class TagViewSetMixin(object): class ArtistViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): - queryset = ( - models.Artist.objects.all() - .prefetch_related( - 'albums__tracks__files', - 'albums__tracks__artist', - 'albums__tracks__tags')) - serializer_class = serializers.ArtistSerializerNested + queryset = models.Artist.objects.with_albums() + serializer_class = serializers.ArtistWithAlbumsSerializer permission_classes = [ConditionalAuthentication] search_fields = ['name__unaccent'] filter_class = filters.ArtistFilter @@ -88,7 +83,7 @@ class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): .select_related() .prefetch_related('tracks__tags', 'tracks__files')) - serializer_class = serializers.AlbumSerializerNested + serializer_class = serializers.AlbumSerializer permission_classes = [ConditionalAuthentication] search_fields = ['title__unaccent'] ordering_fields = ('creation_date',) @@ -166,7 +161,7 @@ class TrackViewSet( A simple ViewSet for viewing and editing accounts. """ queryset = (models.Track.objects.all().for_nested_serialization()) - serializer_class = serializers.TrackSerializerNested + serializer_class = serializers.TrackSerializer permission_classes = [ConditionalAuthentication] search_fields = ['title', 'artist__name'] ordering_fields = ( @@ -370,10 +365,10 @@ class Search(views.APIView): def get(self, request, *args, **kwargs): query = request.GET['query'] results = { - 'tags': serializers.TagSerializer(self.get_tags(query), many=True).data, - 'artists': serializers.ArtistSerializerNested(self.get_artists(query), many=True).data, - 'tracks': serializers.TrackSerializerNested(self.get_tracks(query), many=True).data, - 'albums': serializers.AlbumSerializerNested(self.get_albums(query), many=True).data, + # 'tags': serializers.TagSerializer(self.get_tags(query), many=True).data, + 'artists': serializers.ArtistWithAlbumsSerializer(self.get_artists(query), many=True).data, + 'tracks': serializers.TrackSerializer(self.get_tracks(query), many=True).data, + 'albums': serializers.AlbumSerializer(self.get_albums(query), many=True).data, } return Response(results, status=200) @@ -387,14 +382,10 @@ class Search(views.APIView): return ( models.Track.objects.all() .filter(query_obj) - .select_related('album__artist') - .prefetch_related( - 'tags', - 'artist__albums__tracks__tags', - 'files') + .select_related('artist', 'album__artist') + .prefetch_related('files') )[:self.max_results] - def get_albums(self, query): search_fields = [ 'mbid', @@ -406,27 +397,19 @@ class Search(views.APIView): .filter(query_obj) .select_related() .prefetch_related( - 'tracks__tags', 'tracks__files', - ) + ) )[:self.max_results] - def get_artists(self, query): search_fields = ['mbid', 'name__unaccent'] query_obj = utils.get_query(query, search_fields) return ( models.Artist.objects.all() .filter(query_obj) - .select_related() - .prefetch_related( - 'albums__tracks__tags', - 'albums__tracks__files', - ) - + .with_albums() )[:self.max_results] - def get_tags(self, query): search_fields = ['slug', 'name__unaccent'] query_obj = utils.get_query(query, search_fields) diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index fcb2a412d49b6501eed5d880c5a93f14d87985d8..3f01fd68942b01b0f109e8f32a0eeca29c990585 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -5,13 +5,13 @@ from taggit.models import Tag from funkwhale_api.common import preferences from funkwhale_api.music.models import Track -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.users.serializers import UserBasicSerializer from . import models class PlaylistTrackSerializer(serializers.ModelSerializer): - track = TrackSerializerNested() + track = TrackSerializer() class Meta: model = models.PlaylistTrack diff --git a/api/funkwhale_api/radios/serializers.py b/api/funkwhale_api/radios/serializers.py index 195b382c99a4b32872b34923f958e1c8b682ac94..8c59f87156888d66a8c0f329ae91b436eb09b506 100644 --- a/api/funkwhale_api/radios/serializers.py +++ b/api/funkwhale_api/radios/serializers.py @@ -1,6 +1,6 @@ from rest_framework import serializers -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.users.serializers import UserBasicSerializer from . import filters @@ -46,7 +46,7 @@ class RadioSessionTrackSerializerCreate(serializers.ModelSerializer): class RadioSessionTrackSerializer(serializers.ModelSerializer): - track = TrackSerializerNested() + track = TrackSerializer() class Meta: model = models.RadioSessionTrack diff --git a/api/funkwhale_api/radios/views.py b/api/funkwhale_api/radios/views.py index 37c07c5e4cdf7c30799aa8b3af12fd94d1150274..ca510b82c431beb89f08a7e173eb12ab3fd2abaf 100644 --- a/api/funkwhale_api/radios/views.py +++ b/api/funkwhale_api/radios/views.py @@ -7,7 +7,7 @@ from rest_framework import status from rest_framework.response import Response from rest_framework.decorators import detail_route, list_route -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.common.permissions import ConditionalAuthentication from . import models @@ -49,7 +49,7 @@ class RadioViewSet( page = self.paginate_queryset(tracks) if page is not None: - serializer = TrackSerializerNested(page, many=True) + serializer = TrackSerializer(page, many=True) return self.get_paginated_response(serializer.data) @list_route(methods=['get']) @@ -72,7 +72,7 @@ class RadioViewSet( results = filters.test(f) if results['candidates']['sample']: qs = results['candidates']['sample'].for_nested_serialization() - results['candidates']['sample'] = TrackSerializerNested( + results['candidates']['sample'] = TrackSerializer( qs, many=True).data data['filters'].append(results) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index b7a7d071ab6e8e548b7026b84ff815c0f2e6e43a..d6b1c0204e906b7fe073c4794adccdc00430928c 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,3 +1,4 @@ +import datetime import factory import pytest import requests_mock @@ -10,6 +11,7 @@ from django.test import client from dynamic_preferences.registries import global_preferences_registry +from rest_framework import fields as rest_fields from rest_framework.test import APIClient from rest_framework.test import APIRequestFactory @@ -233,3 +235,16 @@ def assert_user_permission(): assert HasUserPermission in view.permission_classes assert set(view.required_permissions) == set(permissions) return inner + + +@pytest.fixture +def to_api_date(): + def inner(value): + if isinstance(value, datetime.datetime): + f = rest_fields.DateTimeField() + return f.to_representation(value) + if isinstance(value, datetime.date): + f = rest_fields.DateField() + return f.to_representation(value) + raise ValueError('Invalid value: {}'.format(value)) + return inner diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 53ee29f3e909a9c4a78022d0669fdc3710412513..7aa20e6262dd42f204054cf58b4d1f69cd2795a5 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -223,41 +223,6 @@ def test_user_can_create_import_job_with_file( import_job_id=job.pk) -def test_can_search_artist(factories, logged_in_client): - artist1 = factories['music.Artist']() - artist2 = factories['music.Artist']() - expected = [serializers.ArtistSerializerNested(artist1).data] - url = reverse('api:v1:artists-search') - response = logged_in_client.get(url, {'query': artist1.name}) - assert response.data == expected - - -def test_can_search_artist_by_name_start(factories, logged_in_client): - artist1 = factories['music.Artist'](name='alpha') - artist2 = factories['music.Artist'](name='beta') - expected = { - 'next': None, - 'previous': None, - 'count': 1, - 'results': [serializers.ArtistSerializerNested(artist1).data] - } - url = reverse('api:v1:artists-list') - response = logged_in_client.get(url, {'name__startswith': 'a'}) - - assert expected == response.data - - -def test_can_search_tracks(factories, logged_in_client): - track1 = factories['music.Track'](title="test track 1") - track2 = factories['music.Track']() - query = 'test track 1' - expected = [serializers.TrackSerializerNested(track1).data] - url = reverse('api:v1:tracks-search') - response = logged_in_client.get(url, {'query': query}) - - assert expected == response.data - - @pytest.mark.parametrize('route,method', [ ('api:v1:tags-list', 'get'), ('api:v1:tracks-list', 'get'), diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py new file mode 100644 index 0000000000000000000000000000000000000000..fa22ceceeb79199b40ca71297fb4bc0f849dfc7e --- /dev/null +++ b/api/tests/music/test_serializers.py @@ -0,0 +1,121 @@ +from funkwhale_api.music import serializers + + +def test_artist_album_serializer(factories, to_api_date): + track = factories['music.Track']() + album = track.album + album = album.__class__.objects.with_tracks_count().get(pk=album.pk) + expected = { + 'id': album.id, + 'mbid': str(album.mbid), + 'title': album.title, + 'artist': album.artist.id, + 'creation_date': to_api_date(album.creation_date), + 'tracks_count': 1, + 'cover': album.cover.url, + 'release_date': to_api_date(album.release_date), + } + serializer = serializers.ArtistAlbumSerializer(album) + + assert serializer.data == expected + + +def test_artist_with_albums_serializer(factories, to_api_date): + track = factories['music.Track']() + artist = track.artist + artist = artist.__class__.objects.with_albums().get(pk=artist.pk) + album = list(artist.albums.all())[0] + + expected = { + 'id': artist.id, + 'mbid': str(artist.mbid), + 'name': artist.name, + 'creation_date': to_api_date(artist.creation_date), + 'albums': [ + serializers.ArtistAlbumSerializer(album).data + ] + } + serializer = serializers.ArtistWithAlbumsSerializer(artist) + assert serializer.data == expected + + +def test_album_track_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + track = tf.track + + expected = { + 'id': track.id, + 'artist': track.artist.id, + 'album': track.album.id, + 'mbid': str(track.mbid), + 'title': track.title, + 'position': track.position, + 'creation_date': to_api_date(track.creation_date), + 'files': [ + serializers.TrackFileSerializer(tf).data + ] + } + serializer = serializers.AlbumTrackSerializer(track) + assert serializer.data == expected + + +def test_track_file_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + + expected = { + 'id': tf.id, + 'path': tf.path, + 'source': tf.source, + 'filename': tf.filename, + 'mimetype': tf.mimetype, + 'track': tf.track.pk, + 'duration': tf.duration, + 'mimetype': tf.mimetype, + 'bitrate': tf.bitrate, + 'size': tf.size, + } + serializer = serializers.TrackFileSerializer(tf) + assert serializer.data == expected + + +def test_album_serializer(factories, to_api_date): + track1 = factories['music.Track'](position=2) + track2 = factories['music.Track'](position=1, album=track1.album) + album = track1.album + expected = { + 'id': album.id, + 'mbid': str(album.mbid), + 'title': album.title, + 'artist': serializers.ArtistSimpleSerializer(album.artist).data, + 'creation_date': to_api_date(album.creation_date), + 'cover': album.cover.url, + 'release_date': to_api_date(album.release_date), + 'tracks': serializers.AlbumTrackSerializer( + [track2, track1], + many=True + ).data + } + serializer = serializers.AlbumSerializer(album) + + assert serializer.data == expected + + +def test_track_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + track = tf.track + + expected = { + 'id': track.id, + 'artist': serializers.ArtistSimpleSerializer(track.artist).data, + 'album': serializers.TrackAlbumSerializer(track.album).data, + 'mbid': str(track.mbid), + 'title': track.title, + 'position': track.position, + 'creation_date': to_api_date(track.creation_date), + 'lyrics': track.get_lyrics_url(), + 'files': [ + serializers.TrackFileSerializer(tf).data + ] + } + serializer = serializers.TrackSerializer(track) + assert serializer.data == expected diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 030fc3a73eeeabaf0507d5bdbc9949e3d5c4bff5..38366442f309a4b80eb25f7dfeda517b608ab778 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -4,6 +4,7 @@ import pytest from django.urls import reverse from django.utils import timezone +from funkwhale_api.music import serializers from funkwhale_api.music import views from funkwhale_api.federation import actors @@ -16,6 +17,65 @@ def test_permissions(assert_user_permission, view, permissions): assert_user_permission(view, permissions) +def test_artist_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + artist = track.artist + request = api_request.get('/') + qs = artist.__class__.objects.with_albums() + serializer = serializers.ArtistWithAlbumsSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:artists-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + +def test_album_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + album = track.album + request = api_request.get('/') + qs = album.__class__.objects.all() + serializer = serializers.AlbumSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:albums-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + +def test_track_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + request = api_request.get('/') + qs = track.__class__.objects.all() + serializer = serializers.TrackSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:tracks-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + @pytest.mark.parametrize('param,expected', [ ('true', 'full'), ('false', 'empty'), diff --git a/api/tests/radios/test_api.py b/api/tests/radios/test_api.py index 25c099014e91edea53a00850af44ba967480a7bd..66bf6052d539ef1745c45c277385b099a18c51d3 100644 --- a/api/tests/radios/test_api.py +++ b/api/tests/radios/test_api.py @@ -3,7 +3,7 @@ import pytest from django.urls import reverse -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.radios import filters from funkwhale_api.radios import serializers @@ -43,7 +43,7 @@ def test_can_validate_config(logged_in_client, factories): expected = { 'count': candidates.count(), - 'sample': TrackSerializerNested(candidates, many=True).data + 'sample': TrackSerializer(candidates, many=True).data } assert payload['filters'][0]['candidates'] == expected assert payload['filters'][0]['errors'] == []