Fix #1102: Do not include tracks in album API representation

This commit is contained in:
Agate 2020-07-06 10:16:45 +02:00
parent 929077594d
commit 55f4fde0f4
16 changed files with 37 additions and 86 deletions

View File

@ -336,6 +336,7 @@ class ManageBaseArtistSerializer(serializers.ModelSerializer):
class ManageBaseAlbumSerializer(serializers.ModelSerializer): class ManageBaseAlbumSerializer(serializers.ModelSerializer):
cover = music_serializers.cover_field cover = music_serializers.cover_field
domain = serializers.CharField(source="domain_name") domain = serializers.CharField(source="domain_name")
tracks_count = serializers.SerializerMethodField()
class Meta: class Meta:
model = music_models.Album model = music_models.Album
@ -349,8 +350,12 @@ class ManageBaseAlbumSerializer(serializers.ModelSerializer):
"cover", "cover",
"domain", "domain",
"is_local", "is_local",
"tracks_count",
] ]
def get_tracks_count(self, o):
return getattr(o, "_tracks_count", None)
class ManageNestedTrackSerializer(serializers.ModelSerializer): class ManageNestedTrackSerializer(serializers.ModelSerializer):
domain = serializers.CharField(source="domain_name") domain = serializers.CharField(source="domain_name")
@ -428,7 +433,6 @@ class ManageNestedArtistSerializer(ManageBaseArtistSerializer):
class ManageAlbumSerializer( class ManageAlbumSerializer(
music_serializers.OptionalDescriptionMixin, ManageBaseAlbumSerializer music_serializers.OptionalDescriptionMixin, ManageBaseAlbumSerializer
): ):
tracks = ManageNestedTrackSerializer(many=True)
attributed_to = ManageBaseActorSerializer() attributed_to = ManageBaseActorSerializer()
artist = ManageNestedArtistSerializer() artist = ManageNestedArtistSerializer()
tags = serializers.SerializerMethodField() tags = serializers.SerializerMethodField()
@ -437,7 +441,6 @@ class ManageAlbumSerializer(
model = music_models.Album model = music_models.Album
fields = ManageBaseAlbumSerializer.Meta.fields + [ fields = ManageBaseAlbumSerializer.Meta.fields + [
"artist", "artist",
"tracks",
"attributed_to", "attributed_to",
"tags", "tags",
] ]

View File

@ -128,7 +128,7 @@ class ManageAlbumViewSet(
music_models.Album.objects.all() music_models.Album.objects.all()
.order_by("-id") .order_by("-id")
.select_related("attributed_to", "artist", "attachment_cover") .select_related("attributed_to", "artist", "attachment_cover")
.prefetch_related("tracks", music_views.TAG_PREFETCH) .with_tracks_count()
) )
serializer_class = serializers.ManageAlbumSerializer serializer_class = serializers.ManageAlbumSerializer
filterset_class = filters.ManageAlbumFilterSet filterset_class = filters.ManageAlbumFilterSet

View File

@ -319,10 +319,6 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet):
else: else:
return self.exclude(pk__in=matches) return self.exclude(pk__in=matches)
def with_prefetched_tracks_and_playable_uploads(self, actor):
tracks = Track.objects.with_playable_uploads(actor)
return self.prefetch_related(models.Prefetch("tracks", queryset=tracks))
class Album(APIModelMixin): class Album(APIModelMixin):
title = models.CharField(max_length=MAX_LENGTHS["ALBUM_TITLE"]) title = models.CharField(max_length=MAX_LENGTHS["ALBUM_TITLE"])

View File

@ -187,35 +187,12 @@ def serialize_artist_simple(artist):
return data return data
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": DATETIME_FIELD.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(OptionalDescriptionMixin, serializers.Serializer): class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer):
# XXX: remove in 1.0, it's expensive and can work with a filter/api call
tracks = serializers.SerializerMethodField()
artist = serializers.SerializerMethodField() artist = serializers.SerializerMethodField()
cover = cover_field cover = cover_field
is_playable = serializers.SerializerMethodField() is_playable = serializers.SerializerMethodField()
tags = serializers.SerializerMethodField() tags = serializers.SerializerMethodField()
tracks_count = serializers.SerializerMethodField()
attributed_to = serializers.SerializerMethodField() attributed_to = serializers.SerializerMethodField()
id = serializers.IntegerField() id = serializers.IntegerField()
fid = serializers.URLField() fid = serializers.URLField()
@ -232,9 +209,8 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer):
def get_artist(self, o): def get_artist(self, o):
return serialize_artist_simple(o.artist) return serialize_artist_simple(o.artist)
def get_tracks(self, o): def get_tracks_count(self, o):
ordered_tracks = o.tracks.all() return getattr(o, "_tracks_count", None)
return [serialize_album_track(track) for track in ordered_tracks]
def get_is_playable(self, obj): def get_is_playable(self, obj):
try: try:

View File

@ -181,6 +181,7 @@ class AlbumViewSet(
queryset = ( queryset = (
models.Album.objects.all() models.Album.objects.all()
.order_by("-creation_date") .order_by("-creation_date")
.with_tracks_count()
.prefetch_related("artist", "attributed_to", "attachment_cover") .prefetch_related("artist", "attributed_to", "attachment_cover")
) )
serializer_class = serializers.AlbumSerializer serializer_class = serializers.AlbumSerializer
@ -217,14 +218,7 @@ class AlbumViewSet(
queryset = queryset.exclude(artist__channel=None).filter( queryset = queryset.exclude(artist__channel=None).filter(
artist__attributed_to=self.request.user.actor artist__attributed_to=self.request.user.actor
) )
tracks = ( qs = queryset.prefetch_related(TAG_PREFETCH)
models.Track.objects.prefetch_related("artist")
.with_playable_uploads(utils.get_actor_from_request(self.request))
.order_for_album()
)
qs = queryset.prefetch_related(
Prefetch("tracks", queryset=tracks), TAG_PREFETCH
)
return qs return qs
libraries = action(methods=["get"], detail=True)( libraries = action(methods=["get"], detail=True)(

View File

@ -373,7 +373,7 @@ def test_manage_nested_artist_serializer(factories, now, to_api_date):
def test_manage_album_serializer(factories, now, to_api_date): def test_manage_album_serializer(factories, now, to_api_date):
album = factories["music.Album"](attributed=True, with_cover=True) album = factories["music.Album"](attributed=True, with_cover=True)
track = factories["music.Track"](album=album) setattr(album, "_tracks_count", 42)
expected = { expected = {
"id": album.id, "id": album.id,
"domain": album.domain_name, "domain": album.domain_name,
@ -385,11 +385,11 @@ def test_manage_album_serializer(factories, now, to_api_date):
"release_date": album.release_date.isoformat(), "release_date": album.release_date.isoformat(),
"cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data,
"artist": serializers.ManageNestedArtistSerializer(album.artist).data, "artist": serializers.ManageNestedArtistSerializer(album.artist).data,
"tracks": [serializers.ManageNestedTrackSerializer(track).data],
"attributed_to": serializers.ManageBaseActorSerializer( "attributed_to": serializers.ManageBaseActorSerializer(
album.attributed_to album.attributed_to
).data, ).data,
"tags": [], "tags": [],
"tracks_count": 42,
} }
s = serializers.ManageAlbumSerializer(album) s = serializers.ManageAlbumSerializer(album)

View File

@ -115,34 +115,6 @@ def test_artist_with_albums_serializer_channel(factories, to_api_date):
assert serializer.data == expected assert serializer.data == expected
def test_album_track_serializer(factories, to_api_date):
upload = factories["music.Upload"](
track__license="cc-by-4.0", track__copyright="test", track__disc_number=2
)
track = upload.track
setattr(track, "playable_uploads", [upload])
expected = {
"id": track.id,
"fid": track.fid,
"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.serialize_upload(upload)],
"creation_date": to_api_date(track.creation_date),
"listen_url": track.listen_url,
"duration": None,
"license": track.license.code,
"copyright": track.copyright,
"is_local": track.is_local,
}
data = serializers.serialize_album_track(track)
assert data == expected
def test_upload_serializer(factories, to_api_date): def test_upload_serializer(factories, to_api_date):
upload = factories["music.Upload"]() upload = factories["music.Upload"]()
@ -200,7 +172,7 @@ def test_album_serializer(factories, to_api_date):
track1 = factories["music.Track"]( track1 = factories["music.Track"](
position=2, album__attributed_to=actor, album__with_cover=True position=2, album__attributed_to=actor, album__with_cover=True
) )
track2 = factories["music.Track"](position=1, album=track1.album) factories["music.Track"](position=1, album=track1.album)
album = track1.album album = track1.album
expected = { expected = {
"id": album.id, "id": album.id,
@ -212,12 +184,14 @@ def test_album_serializer(factories, to_api_date):
"is_playable": False, "is_playable": False,
"cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data,
"release_date": to_api_date(album.release_date), "release_date": to_api_date(album.release_date),
"tracks": [serializers.serialize_album_track(t) for t in [track2, track1]], "tracks_count": 2,
"is_local": album.is_local, "is_local": album.is_local,
"tags": [], "tags": [],
"attributed_to": federation_serializers.APIActorSerializer(actor).data, "attributed_to": federation_serializers.APIActorSerializer(actor).data,
} }
serializer = serializers.AlbumSerializer(album) serializer = serializers.AlbumSerializer(
album.__class__.objects.with_tracks_count().get(pk=album.pk)
)
assert serializer.data == expected assert serializer.data == expected

View File

@ -57,7 +57,7 @@ def test_album_list_serializer(api_request, factories, logged_in_api_client):
).track ).track
album = track.album album = track.album
request = api_request.get("/") request = api_request.get("/")
qs = album.__class__.objects.with_prefetched_tracks_and_playable_uploads(None) qs = album.__class__.objects.with_tracks_count()
serializer = serializers.AlbumSerializer( serializer = serializers.AlbumSerializer(
qs, many=True, context={"request": request} qs, many=True, context={"request": request}
) )

View File

@ -0,0 +1 @@
Do not include tracks in album API representation (#1102)

View File

@ -31,3 +31,11 @@ Now, it returns all the accessible libraries (including ones from other users an
If you are consuming the API via a third-party client and need to retrieve your libraries, If you are consuming the API via a third-party client and need to retrieve your libraries,
use the ``scope`` parameter, like this: ``GET /api/v1/libraries?scope=me`` use the ``scope`` parameter, like this: ``GET /api/v1/libraries?scope=me``
API breaking change in ``/api/v1/albums``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
To increase performance, querying ``/api/v1/albums`` doesn't return album tracks anymore. This caused
some performance issues, especially as some albums and series have dozens or even hundreds of tracks.
If you want to retrieve tracks for an album, you can query ``/api/v1/tracks/?album=<albumid>``.

View File

@ -198,10 +198,9 @@ Album:
- $ref: "#/BaseAlbum" - $ref: "#/BaseAlbum"
- type: "object" - type: "object"
properties: properties:
tracks: tracks_count:
type: "array" type: "integer"
items: format: "int64"
$ref: "#/AlbumTrack"
ArtistAlbum: ArtistAlbum:
type: "object" type: "object"

View File

@ -15,8 +15,8 @@
<div class="description"> <div class="description">
<translate translate-context="Content/Channel/Paragraph" <translate translate-context="Content/Channel/Paragraph"
translate-plural="%{ count } episodes" translate-plural="%{ count } episodes"
:translate-n="serie.tracks.length" :translate-n="serie.tracks_count"
:translate-params="{count: serie.tracks.length}"> :translate-params="{count: serie.tracks_count}">
%{ count } episode %{ count } episode
</translate> </translate>
</div> </div>

View File

@ -20,7 +20,7 @@
</div> </div>
</div> </div>
<div class="extra content"> <div class="extra content">
<translate translate-context="*/*/*" :translate-params="{count: album.tracks.length}" :translate-n="album.tracks.length" translate-plural="%{ count } tracks">%{ count } track</translate> <translate translate-context="*/*/*" :translate-params="{count: album.tracks_count}" :translate-n="album.tracks_count" translate-plural="%{ count } tracks">%{ count } track</translate>
<play-button class="right floated basic icon" :dropdown-only="true" :is-playable="album.is_playable" :dropdown-icon-classes="['ellipsis', 'horizontal', 'large really discrete']" :album="album"></play-button> <play-button class="right floated basic icon" :dropdown-only="true" :is-playable="album.is_playable" :dropdown-icon-classes="['ellipsis', 'horizontal', 'large really discrete']" :album="album"></play-button>
</div> </div>
</div> </div>

View File

@ -9,7 +9,7 @@
<translate translate-context="*/*/*">None</translate> <translate translate-context="*/*/*">None</translate>
</option> </option>
<option v-for="album in albums" :key="album.id" :value="album.id"> <option v-for="album in albums" :key="album.id" :value="album.id">
{{ album.title }} (<translate translate-context="*/*/*" :translate-params="{count: album.tracks.length}" :translate-n="album.tracks.length" translate-plural="%{ count } tracks">%{ count } track</translate>) {{ album.title }} (<translate translate-context="*/*/*" :translate-params="{count: album.tracks_count}" :translate-n="album.tracks_count" translate-plural="%{ count } tracks">%{ count } track</translate>)
</option> </option>
</select> </select>
</div> </div>

View File

@ -195,7 +195,7 @@ export default {
}, },
computed: { computed: {
totalTracks () { totalTracks () {
return this.object.tracks.length return this.object.tracks_count
}, },
isChannel () { isChannel () {
return this.object.artist.channel return this.object.artist.channel

View File

@ -67,7 +67,7 @@
</span> </span>
</td> </td>
<td> <td>
{{ scope.obj.tracks.length }} {{ scope.obj.tracks_count }}
</td> </td>
<td> <td>
<human-date v-if="scope.obj.release_date" :date="scope.obj.release_date"></human-date> <human-date v-if="scope.obj.release_date" :date="scope.obj.release_date"></human-date>