See #224: less clutered music API, better serializers

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')

api/funkwhale_api/history/serializers.py

 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
 

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

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)

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):

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)

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

api/funkwhale_api/radios/serializers.py

 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

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)
 

api/tests/conftest.py

+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

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'),

api/tests/music/test_serializers.py

+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

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'),

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'] == []