diff --git a/api/funkwhale_api/manage/serializers.py b/api/funkwhale_api/manage/serializers.py index d29433e5669d15e4168833cd58225e00c033b110..d8cab97312b2a4a396363fabc69eb7c2b1019563 100644 --- a/api/funkwhale_api/manage/serializers.py +++ b/api/funkwhale_api/manage/serializers.py @@ -336,6 +336,7 @@ class ManageBaseArtistSerializer(serializers.ModelSerializer): class ManageBaseAlbumSerializer(serializers.ModelSerializer): cover = music_serializers.cover_field domain = serializers.CharField(source="domain_name") + tracks_count = serializers.SerializerMethodField() class Meta: model = music_models.Album @@ -349,8 +350,12 @@ class ManageBaseAlbumSerializer(serializers.ModelSerializer): "cover", "domain", "is_local", + "tracks_count", ] + def get_tracks_count(self, o): + return getattr(o, "_tracks_count", None) + class ManageNestedTrackSerializer(serializers.ModelSerializer): domain = serializers.CharField(source="domain_name") @@ -428,7 +433,6 @@ class ManageNestedArtistSerializer(ManageBaseArtistSerializer): class ManageAlbumSerializer( music_serializers.OptionalDescriptionMixin, ManageBaseAlbumSerializer ): - tracks = ManageNestedTrackSerializer(many=True) attributed_to = ManageBaseActorSerializer() artist = ManageNestedArtistSerializer() tags = serializers.SerializerMethodField() @@ -437,7 +441,6 @@ class ManageAlbumSerializer( model = music_models.Album fields = ManageBaseAlbumSerializer.Meta.fields + [ "artist", - "tracks", "attributed_to", "tags", ] diff --git a/api/funkwhale_api/manage/views.py b/api/funkwhale_api/manage/views.py index adb7128e267334d66a8cc7231168a88c99e7ff96..bc3908daee90e1c37a1b0810a4786e91e77eda71 100644 --- a/api/funkwhale_api/manage/views.py +++ b/api/funkwhale_api/manage/views.py @@ -128,7 +128,7 @@ class ManageAlbumViewSet( music_models.Album.objects.all() .order_by("-id") .select_related("attributed_to", "artist", "attachment_cover") - .prefetch_related("tracks", music_views.TAG_PREFETCH) + .with_tracks_count() ) serializer_class = serializers.ManageAlbumSerializer filterset_class = filters.ManageAlbumFilterSet diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index e020a619b1ac330c81d1fbb38cec71c5d7b2cab7..1d32a01bd976dfbe028966be5b7a254b5adadbb2 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -319,10 +319,6 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): else: 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): title = models.CharField(max_length=MAX_LENGTHS["ALBUM_TITLE"]) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 6abd6e2626bef99b7a41bbd5ff3c9a5d16922b93..7dfef3bcace680ba646ab00849d18a9692249ca7 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -187,35 +187,12 @@ def serialize_artist_simple(artist): 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): - # XXX: remove in 1.0, it's expensive and can work with a filter/api call - tracks = serializers.SerializerMethodField() artist = serializers.SerializerMethodField() cover = cover_field is_playable = serializers.SerializerMethodField() tags = serializers.SerializerMethodField() + tracks_count = serializers.SerializerMethodField() attributed_to = serializers.SerializerMethodField() id = serializers.IntegerField() fid = serializers.URLField() @@ -232,9 +209,8 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): def get_artist(self, o): return serialize_artist_simple(o.artist) - def get_tracks(self, o): - ordered_tracks = o.tracks.all() - return [serialize_album_track(track) for track in ordered_tracks] + def get_tracks_count(self, o): + return getattr(o, "_tracks_count", None) def get_is_playable(self, obj): try: diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index c34d9b175e7babf2c218958091652e062a3055ba..5a4e81da4b1404f601c021e777b3dd76a9aaef7a 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -181,6 +181,7 @@ class AlbumViewSet( queryset = ( models.Album.objects.all() .order_by("-creation_date") + .with_tracks_count() .prefetch_related("artist", "attributed_to", "attachment_cover") ) serializer_class = serializers.AlbumSerializer @@ -217,14 +218,7 @@ class AlbumViewSet( queryset = queryset.exclude(artist__channel=None).filter( artist__attributed_to=self.request.user.actor ) - tracks = ( - 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 - ) + qs = queryset.prefetch_related(TAG_PREFETCH) return qs libraries = action(methods=["get"], detail=True)( diff --git a/api/tests/manage/test_serializers.py b/api/tests/manage/test_serializers.py index c4dbaa45ed1772b5b6952668ef0492bd231c86e7..d6de65a47e61ad2a90971179c4ad55d81edc1fb6 100644 --- a/api/tests/manage/test_serializers.py +++ b/api/tests/manage/test_serializers.py @@ -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): album = factories["music.Album"](attributed=True, with_cover=True) - track = factories["music.Track"](album=album) + setattr(album, "_tracks_count", 42) expected = { "id": album.id, "domain": album.domain_name, @@ -385,11 +385,11 @@ def test_manage_album_serializer(factories, now, to_api_date): "release_date": album.release_date.isoformat(), "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "artist": serializers.ManageNestedArtistSerializer(album.artist).data, - "tracks": [serializers.ManageNestedTrackSerializer(track).data], "attributed_to": serializers.ManageBaseActorSerializer( album.attributed_to ).data, "tags": [], + "tracks_count": 42, } s = serializers.ManageAlbumSerializer(album) diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 5361950e1b0f5117fdf39d658b8f63d176f1a078..9500a2c6a2da4743b8ccf61da48cf96646f4045f 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -115,34 +115,6 @@ def test_artist_with_albums_serializer_channel(factories, to_api_date): 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): upload = factories["music.Upload"]() @@ -200,7 +172,7 @@ def test_album_serializer(factories, to_api_date): track1 = factories["music.Track"]( 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 expected = { "id": album.id, @@ -212,12 +184,14 @@ def test_album_serializer(factories, to_api_date): "is_playable": False, "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "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, "tags": [], "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 diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 5e3c9c953be2d44a55a01d7d4a0c1d757f10f047..2fa20aa1dae8b601de82672083a54310c1f61c35 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -57,7 +57,7 @@ def test_album_list_serializer(api_request, factories, logged_in_api_client): ).track album = track.album 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( qs, many=True, context={"request": request} ) diff --git a/changes/changelog.d/1102.enhancement b/changes/changelog.d/1102.enhancement new file mode 100644 index 0000000000000000000000000000000000000000..6c80b79381a27a4d4c7b956c311bb9f42019b653 --- /dev/null +++ b/changes/changelog.d/1102.enhancement @@ -0,0 +1 @@ +Do not include tracks in album API representation (#1102) \ No newline at end of file diff --git a/changes/notes.rst b/changes/notes.rst index 8d84dc02621ce2db8bd1b62ea5dac3300e0a510a..a95ffd69af493446bafb3b23c3fedb318d3efc3a 100644 --- a/changes/notes.rst +++ b/changes/notes.rst @@ -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, 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>``. \ No newline at end of file diff --git a/docs/api/definitions.yml b/docs/api/definitions.yml index 4903b2c390cf46eb2f0cdc7b40efe5382659b06f..6cc41dccba13ea6a2bf82a20e6ec5b0518e4f029 100644 --- a/docs/api/definitions.yml +++ b/docs/api/definitions.yml @@ -198,10 +198,9 @@ Album: - $ref: "#/BaseAlbum" - type: "object" properties: - tracks: - type: "array" - items: - $ref: "#/AlbumTrack" + tracks_count: + type: "integer" + format: "int64" ArtistAlbum: type: "object" diff --git a/front/src/components/audio/ChannelSerieCard.vue b/front/src/components/audio/ChannelSerieCard.vue index 5a43e02dcc676ea5f5b52c0c0aaf755ec1c35916..2ce1764513823ef3bca1bc805deff43395e39670 100644 --- a/front/src/components/audio/ChannelSerieCard.vue +++ b/front/src/components/audio/ChannelSerieCard.vue @@ -15,8 +15,8 @@ <div class="description"> <translate translate-context="Content/Channel/Paragraph" translate-plural="%{ count } episodes" - :translate-n="serie.tracks.length" - :translate-params="{count: serie.tracks.length}"> + :translate-n="serie.tracks_count" + :translate-params="{count: serie.tracks_count}"> %{ count } episode </translate> </div> diff --git a/front/src/components/audio/album/Card.vue b/front/src/components/audio/album/Card.vue index c619a905374c9f4a8b8d2e9be84daf019278414c..63f418c0f68b910dbafbe4fd81d75256a3bfe958 100644 --- a/front/src/components/audio/album/Card.vue +++ b/front/src/components/audio/album/Card.vue @@ -20,7 +20,7 @@ </div> </div> <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> </div> </div> diff --git a/front/src/components/channels/AlbumSelect.vue b/front/src/components/channels/AlbumSelect.vue index ed4b493e885937a078bedba3426ec004ea73a614..0beafbbc721427130c5a499ba2be909d0ba6ff75 100644 --- a/front/src/components/channels/AlbumSelect.vue +++ b/front/src/components/channels/AlbumSelect.vue @@ -9,7 +9,7 @@ <translate translate-context="*/*/*">None</translate> </option> <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> </select> </div> diff --git a/front/src/components/library/AlbumBase.vue b/front/src/components/library/AlbumBase.vue index 104280a76f240a264853526fad38eb7d8aee4e3e..071b133574ef69a0db0b26a1afdc85addb4b5f42 100644 --- a/front/src/components/library/AlbumBase.vue +++ b/front/src/components/library/AlbumBase.vue @@ -195,7 +195,7 @@ export default { }, computed: { totalTracks () { - return this.object.tracks.length + return this.object.tracks_count }, isChannel () { return this.object.artist.channel diff --git a/front/src/components/manage/library/AlbumsTable.vue b/front/src/components/manage/library/AlbumsTable.vue index af7c82d1afb6a8d91966ea77b7f3c00b767577cc..72f5aee4c5e283faa291f354eea0298814a7060b 100644 --- a/front/src/components/manage/library/AlbumsTable.vue +++ b/front/src/components/manage/library/AlbumsTable.vue @@ -67,7 +67,7 @@ </span> </td> <td> - {{ scope.obj.tracks.length }} + {{ scope.obj.tracks_count }} </td> <td> <human-date v-if="scope.obj.release_date" :date="scope.obj.release_date"></human-date> diff --git a/front/src/components/semantic/Modal.vue b/front/src/components/semantic/Modal.vue index 0a93a053ed3f34652ed9092639f58713e3358e13..5ae134b218639d40caa07af5383741ef649dd657 100644 --- a/front/src/components/semantic/Modal.vue +++ b/front/src/components/semantic/Modal.vue @@ -43,7 +43,6 @@ export default { }.bind(this), onHidden: function () { this.$emit('update:show', false) - this.focusTrap.pause() }.bind(this), onVisible: function () { this.focusTrap.activate() @@ -59,11 +58,15 @@ export default { this.initModal() this.$emit('show') this.control.modal('show') + this.focusTrap.activate() + this.focusTrap.unpause() } else { if (this.control) { this.$emit('hide') this.control.modal('hide') this.control.remove() + this.focusTrap.deactivate() + this.focusTrap.pause() } } }