Commit 81349e2b authored by Eliot Berriot's avatar Eliot Berriot
Browse files

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

parent 8ed6f830
......@@ -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
def get_listen_url(self, obj):
return obj.listen_url
def get_duration(self, obj):
try:
return obj.duration
except AttributeError:
return None
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 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", [])
......
......@@ -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]
......
......@@ -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(
......
......@@ -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}
)
......
Improved performance of /artists, /albums and /tracks API endpoints by a factor 2 (#865)
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment