Merge branch 'performance-optimization' into 'develop'

Fix #865: Performance optimization on /artists, /albums and /tracks endpoints

Closes #865

See merge request funkwhale/funkwhale!902

api/funkwhale_api/music/serializers.py

@@ -104,58 +104,50 @@ class ArtistWithAlbumsSerializer(serializers.ModelSerializer):
     get_attributed_to = serialize_attributed_to
 
     def get_tracks_count(self, o):
-        return getattr(o, "_tracks_count", None)
-
-
-class ArtistSimpleSerializer(serializers.ModelSerializer):
-    class Meta:
-        model = models.Artist
-        fields = ("id", "fid", "mbid", "name", "creation_date", "is_local")
-
-
-class AlbumTrackSerializer(serializers.ModelSerializer):
-    artist = ArtistSimpleSerializer(read_only=True)
-    uploads = serializers.SerializerMethodField()
-    listen_url = serializers.SerializerMethodField()
-    duration = serializers.SerializerMethodField()
-
-    class Meta:
-        model = models.Track
-        fields = (
-            "id",
-            "fid",
-            "mbid",
-            "title",
-            "album",
-            "artist",
-            "creation_date",
-            "position",
-            "disc_number",
-            "uploads",
-            "listen_url",
-            "duration",
-            "copyright",
-            "license",
-            "is_local",
-        )
-
-    def get_uploads(self, obj):
-        uploads = getattr(obj, "playable_uploads", [])
-        return TrackUploadSerializer(uploads, many=True).data
+        tracks = getattr(o, "_prefetched_tracks", None)
+        return len(tracks) if tracks else None
+
+
+def serialize_artist_simple(artist):
+    return {
+        "id": artist.id,
+        "fid": artist.fid,
+        "mbid": str(artist.mbid),
+        "name": artist.name,
+        "creation_date": serializers.DateTimeField().to_representation(
+            artist.creation_date
+        ),
+        "is_local": artist.is_local,
+    }
 
-    def get_listen_url(self, obj):
-        return obj.listen_url
 
-    def get_duration(self, obj):
-        try:
-            return obj.duration
-        except AttributeError:
-            return None
+def serialize_album_track(track):
+    return {
+        "id": track.id,
+        "fid": track.fid,
+        "mbid": str(track.mbid),
+        "title": track.title,
+        "artist": serialize_artist_simple(track.artist),
+        "album": track.album_id,
+        "creation_date": serializers.DateTimeField().to_representation(
+            track.creation_date
+        ),
+        "position": track.position,
+        "disc_number": track.disc_number,
+        "uploads": [
+            serialize_upload(u) for u in getattr(track, "playable_uploads", [])
+        ],
+        "listen_url": track.listen_url,
+        "duration": getattr(track, "duration", None),
+        "copyright": track.copyright,
+        "license": track.license_id,
+        "is_local": track.is_local,
+    }
 
 
 class AlbumSerializer(serializers.ModelSerializer):
     tracks = serializers.SerializerMethodField()
-    artist = ArtistSimpleSerializer(read_only=True)
+    artist = serializers.SerializerMethodField()
     cover = cover_field
     is_playable = serializers.SerializerMethodField()
     tags = serializers.SerializerMethodField()
@@ -181,9 +173,12 @@ class AlbumSerializer(serializers.ModelSerializer):
 
     get_attributed_to = serialize_attributed_to
 
+    def get_artist(self, o):
+        return serialize_artist_simple(o.artist)
+
     def get_tracks(self, o):
         ordered_tracks = o.tracks.all()
-        return AlbumTrackSerializer(ordered_tracks, many=True).data
+        return [serialize_album_track(track) for track in ordered_tracks]
 
     def get_is_playable(self, obj):
         try:
@@ -199,7 +194,7 @@ class AlbumSerializer(serializers.ModelSerializer):
 
 
 class TrackAlbumSerializer(serializers.ModelSerializer):
-    artist = ArtistSimpleSerializer(read_only=True)
+    artist = serializers.SerializerMethodField()
     cover = cover_field
 
     class Meta:
@@ -216,23 +211,24 @@ class TrackAlbumSerializer(serializers.ModelSerializer):
             "is_local",
         )
 
+    def get_artist(self, o):
+        return serialize_artist_simple(o.artist)
 
-class TrackUploadSerializer(serializers.ModelSerializer):
-    class Meta:
-        model = models.Upload
-        fields = (
-            "uuid",
-            "listen_url",
-            "size",
-            "duration",
-            "bitrate",
-            "mimetype",
-            "extension",
-        )
+
+def serialize_upload(upload):
+    return {
+        "uuid": str(upload.uuid),
+        "listen_url": upload.listen_url,
+        "size": upload.size,
+        "duration": upload.duration,
+        "bitrate": upload.bitrate,
+        "mimetype": upload.mimetype,
+        "extension": upload.extension,
+    }
 
 
 class TrackSerializer(serializers.ModelSerializer):
-    artist = ArtistSimpleSerializer(read_only=True)
+    artist = serializers.SerializerMethodField()
     album = TrackAlbumSerializer(read_only=True)
     uploads = serializers.SerializerMethodField()
     listen_url = serializers.SerializerMethodField()
@@ -262,12 +258,14 @@ class TrackSerializer(serializers.ModelSerializer):
 
     get_attributed_to = serialize_attributed_to
 
+    def get_artist(self, o):
+        return serialize_artist_simple(o.artist)
+
     def get_listen_url(self, obj):
         return obj.listen_url
 
     def get_uploads(self, obj):
-        uploads = getattr(obj, "playable_uploads", [])
-        return TrackUploadSerializer(uploads, many=True).data
+        return [serialize_upload(u) for u in getattr(obj, "playable_uploads", [])]
 
     def get_tags(self, obj):
         tagged_items = getattr(obj, "_prefetched_tagged_items", [])

api/funkwhale_api/music/views.py

@@ -96,8 +96,14 @@ def refetch_obj(obj, queryset):
 class ArtistViewSet(common_views.SkipFilterForGetObject, viewsets.ReadOnlyModelViewSet):
     queryset = (
         models.Artist.objects.all()
-        .select_related("attributed_to")
-        .annotate(_tracks_count=Count("tracks"))
+        .prefetch_related("attributed_to")
+        .prefetch_related(
+            Prefetch(
+                "tracks",
+                queryset=models.Track.objects.all(),
+                to_attr="_prefetched_tracks",
+            )
+        )
     )
     serializer_class = serializers.ArtistWithAlbumsSerializer
     permission_classes = [oauth_permissions.ScopePermission]

api/tests/music/test_serializers.py

@@ -62,7 +62,7 @@ def test_artist_with_albums_serializer(factories, to_api_date):
     artist = track.artist
     artist = artist.__class__.objects.with_albums().get(pk=artist.pk)
     album = list(artist.albums.all())[0]
-    setattr(artist, "_tracks_count", 42)
+    setattr(artist, "_prefetched_tracks", range(42))
     expected = {
         "id": artist.id,
         "fid": artist.fid,
@@ -89,13 +89,13 @@ def test_album_track_serializer(factories, to_api_date):
     expected = {
         "id": track.id,
         "fid": track.fid,
-        "artist": serializers.ArtistSimpleSerializer(track.artist).data,
+        "artist": serializers.serialize_artist_simple(track.artist),
         "album": track.album.id,
         "mbid": str(track.mbid),
         "title": track.title,
         "position": track.position,
         "disc_number": track.disc_number,
-        "uploads": [serializers.TrackUploadSerializer(upload).data],
+        "uploads": [serializers.serialize_upload(upload)],
         "creation_date": to_api_date(track.creation_date),
         "listen_url": track.listen_url,
         "duration": None,
@@ -103,8 +103,8 @@ def test_album_track_serializer(factories, to_api_date):
         "copyright": track.copyright,
         "is_local": track.is_local,
     }
-    serializer = serializers.AlbumTrackSerializer(track)
-    assert serializer.data == expected
+    data = serializers.serialize_album_track(track)
+    assert data == expected
 
 
 def test_upload_serializer(factories, to_api_date):
@@ -169,7 +169,7 @@ def test_album_serializer(factories, to_api_date):
         "fid": album.fid,
         "mbid": str(album.mbid),
         "title": album.title,
-        "artist": serializers.ArtistSimpleSerializer(album.artist).data,
+        "artist": serializers.serialize_artist_simple(album.artist),
         "creation_date": to_api_date(album.creation_date),
         "is_playable": False,
         "cover": {
@@ -179,7 +179,7 @@ def test_album_serializer(factories, to_api_date):
             "small_square_crop": album.cover.crop["50x50"].url,
         },
         "release_date": to_api_date(album.release_date),
-        "tracks": serializers.AlbumTrackSerializer([track2, track1], many=True).data,
+        "tracks": [serializers.serialize_album_track(t) for t in [track2, track1]],
         "is_local": album.is_local,
         "tags": [],
         "attributed_to": federation_serializers.APIActorSerializer(actor).data,
@@ -202,13 +202,13 @@ def test_track_serializer(factories, to_api_date):
     expected = {
         "id": track.id,
         "fid": track.fid,
-        "artist": serializers.ArtistSimpleSerializer(track.artist).data,
+        "artist": serializers.serialize_artist_simple(track.artist),
         "album": serializers.TrackAlbumSerializer(track.album).data,
         "mbid": str(track.mbid),
         "title": track.title,
         "position": track.position,
         "disc_number": track.disc_number,
-        "uploads": [serializers.TrackUploadSerializer(upload).data],
+        "uploads": [serializers.serialize_upload(upload)],
         "creation_date": to_api_date(track.creation_date),
         "listen_url": track.listen_url,
         "license": upload.track.license.code,
@@ -317,7 +317,7 @@ def test_manage_upload_action_relaunch_import(factories, mocker):
     assert m.call_count == 3
 
 
-def test_track_upload_serializer(factories):
+def test_serialize_upload(factories):
     upload = factories["music.Upload"]()
 
     expected = {
@@ -330,8 +330,8 @@ def test_track_upload_serializer(factories):
         "duration": upload.duration,
     }
 
-    serializer = serializers.TrackUploadSerializer(upload)
-    assert serializer.data == expected
+    data = serializers.serialize_upload(upload)
+    assert data == expected
 
 
 @pytest.mark.parametrize(

api/tests/music/test_views.py

@@ -6,7 +6,7 @@ import urllib.parse
 import uuid
 
 import pytest
-from django.db.models import Count
+from django.db.models import Prefetch
 from django.urls import reverse
 from django.utils import timezone
 
@@ -28,7 +28,9 @@ def test_artist_list_serializer(api_request, factories, logged_in_api_client):
     ).track
     artist = track.artist
     request = api_request.get("/")
-    qs = artist.__class__.objects.with_albums().annotate(_tracks_count=Count("tracks"))
+    qs = artist.__class__.objects.with_albums().prefetch_related(
+        Prefetch("tracks", to_attr="_prefetched_tracks")
+    )
     serializer = serializers.ArtistWithAlbumsSerializer(
         qs, many=True, context={"request": request}
     )

changes/changelog.d/865.bugfix

+Improved performance of /artists, /albums and /tracks API endpoints by a factor 2 (#865)