diff --git a/api/funkwhale_api/favorites/views.py b/api/funkwhale_api/favorites/views.py index 7e30c28a6a7283e0c57d049961e617fb60aec3c4..5f4e1cd42e34393ecf8545c62278a2097cc56deb 100644 --- a/api/funkwhale_api/favorites/views.py +++ b/api/funkwhale_api/favorites/views.py @@ -51,7 +51,7 @@ class TrackFavoriteViewSet( queryset = queryset.filter( fields.privacy_level_query(self.request.user, "user__privacy_level") ) - tracks = Track.objects.annotate_playable_by_actor( + tracks = Track.objects.with_playable_uploads( music_utils.get_actor_from_request(self.request) ).select_related("artist", "album__artist") queryset = queryset.prefetch_related(Prefetch("track", queryset=tracks)) diff --git a/api/funkwhale_api/history/views.py b/api/funkwhale_api/history/views.py index ec3b4f3c080fabe174a340e3326b7365668f9c9e..56c30af36511e8cf124645dfcd00f835e3a04287 100644 --- a/api/funkwhale_api/history/views.py +++ b/api/funkwhale_api/history/views.py @@ -41,7 +41,7 @@ class ListeningViewSet( queryset = queryset.filter( fields.privacy_level_query(self.request.user, "user__privacy_level") ) - tracks = Track.objects.annotate_playable_by_actor( + tracks = Track.objects.with_playable_uploads( music_utils.get_actor_from_request(self.request) ).select_related("artist", "album__artist") return queryset.prefetch_related(Prefetch("track", queryset=tracks)) diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index c99bfe2c1913da7ec633a044fcc1f9234347bbb7..43a95047f757514936bfd96207869faaa2c16b2e 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -124,8 +124,8 @@ class ArtistQuerySet(models.QuerySet): def annotate_playable_by_actor(self, actor): tracks = ( - Track.objects.playable_by(actor) - .filter(artist=models.OuterRef("id")) + Upload.objects.playable_by(actor) + .filter(track__artist=models.OuterRef("id")) .order_by("id") .values("id")[:1] ) @@ -192,8 +192,8 @@ class AlbumQuerySet(models.QuerySet): def annotate_playable_by_actor(self, actor): tracks = ( - Track.objects.playable_by(actor) - .filter(album=models.OuterRef("id")) + Upload.objects.playable_by(actor) + .filter(track__album=models.OuterRef("id")) .order_by("id") .values("id")[:1] ) @@ -207,6 +207,15 @@ class AlbumQuerySet(models.QuerySet): else: return self.exclude(tracks__in=tracks).distinct() + 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=255) @@ -403,18 +412,14 @@ class TrackQuerySet(models.QuerySet): else: return self.exclude(uploads__in=files).distinct() - def annotate_duration(self): - first_upload = Upload.objects.filter(track=models.OuterRef("pk")).order_by("pk") - return self.annotate( - duration=models.Subquery(first_upload.values("duration")[:1]) - ) - - def annotate_file_data(self): - first_upload = Upload.objects.filter(track=models.OuterRef("pk")).order_by("pk") - return self.annotate( - bitrate=models.Subquery(first_upload.values("bitrate")[:1]), - size=models.Subquery(first_upload.values("size")[:1]), - mimetype=models.Subquery(first_upload.values("mimetype")[:1]), + def with_playable_uploads(self, actor): + uploads = Upload.objects.playable_by(actor).select_related('track') + return self.prefetch_related( + models.Prefetch( + 'uploads', + queryset=uploads, + to_attr='playable_uploads' + ) ) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index a23fc1daadf6d7bfbeb4b7d9d625475fcd6a1ad1..5d1dab00696fef6614c73371a4419c42cfbf2b6e 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -59,7 +59,7 @@ class ArtistSimpleSerializer(serializers.ModelSerializer): class AlbumTrackSerializer(serializers.ModelSerializer): artist = ArtistSimpleSerializer(read_only=True) - is_playable = serializers.SerializerMethodField() + uploads = serializers.SerializerMethodField() listen_url = serializers.SerializerMethodField() duration = serializers.SerializerMethodField() @@ -73,16 +73,14 @@ class AlbumTrackSerializer(serializers.ModelSerializer): "artist", "creation_date", "position", - "is_playable", + "uploads", "listen_url", "duration", ) - def get_is_playable(self, obj): - try: - return bool(obj.is_playable_by_actor) - except AttributeError: - return None + 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 @@ -123,7 +121,9 @@ class AlbumSerializer(serializers.ModelSerializer): def get_is_playable(self, obj): try: - return any([bool(t.is_playable_by_actor) for t in obj.tracks.all()]) + return any( + [bool(getattr(t, "playable_uploads", [])) for t in obj.tracks.all()] + ) except AttributeError: return None @@ -145,16 +145,26 @@ class TrackAlbumSerializer(serializers.ModelSerializer): ) +class TrackUploadSerializer(serializers.ModelSerializer): + class Meta: + model = models.Upload + fields = ( + "uuid", + "listen_url", + "size", + "duration", + "bitrate", + "mimetype", + "extension", + ) + + class TrackSerializer(serializers.ModelSerializer): artist = ArtistSimpleSerializer(read_only=True) album = TrackAlbumSerializer(read_only=True) lyrics = serializers.SerializerMethodField() - is_playable = serializers.SerializerMethodField() + uploads = serializers.SerializerMethodField() listen_url = serializers.SerializerMethodField() - duration = serializers.SerializerMethodField() - bitrate = serializers.SerializerMethodField() - size = serializers.SerializerMethodField() - mimetype = serializers.SerializerMethodField() class Meta: model = models.Track @@ -167,12 +177,8 @@ class TrackSerializer(serializers.ModelSerializer): "creation_date", "position", "lyrics", - "is_playable", + "uploads", "listen_url", - "duration", - "bitrate", - "size", - "mimetype", ) def get_lyrics(self, obj): @@ -181,35 +187,9 @@ class TrackSerializer(serializers.ModelSerializer): def get_listen_url(self, obj): return obj.listen_url - def get_is_playable(self, obj): - try: - return bool(obj.is_playable_by_actor) - except AttributeError: - return None - - def get_duration(self, obj): - try: - return obj.duration - except AttributeError: - return None - - def get_bitrate(self, obj): - try: - return obj.bitrate - except AttributeError: - return None - - def get_size(self, obj): - try: - return obj.size - except AttributeError: - return None - - def get_mimetype(self, obj): - try: - return obj.mimetype - except AttributeError: - return None + def get_uploads(self, obj): + uploads = getattr(obj, "playable_uploads", []) + return TrackUploadSerializer(uploads, many=True).data class LibraryForOwnerSerializer(serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py index f146dc344a6898e279d0fd195d88cc0afba89f6e..2c1210cf78c7cddeaba1968f0557d4f48c048302 100644 --- a/api/funkwhale_api/music/utils.py +++ b/api/funkwhale_api/music/utils.py @@ -5,7 +5,6 @@ import mutagen import pydub from funkwhale_api.common.search import normalize_query, get_query # noqa -from funkwhale_api.common import utils def guess_mimetype(f): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index c5cfeb25cf16a7c0b1566ad2ca65d98bd781a503..744fd43dd83a48ae236a81a9f7e278cc61758f61 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -93,17 +93,9 @@ class AlbumViewSet(viewsets.ReadOnlyModelViewSet): def get_queryset(self): queryset = super().get_queryset() - tracks = models.Track.objects.annotate_playable_by_actor( + tracks = models.Track.objects.select_related("artist").with_playable_uploads( utils.get_actor_from_request(self.request) - ).select_related("artist") - if ( - hasattr(self, "kwargs") - and self.kwargs - and self.request.method.lower() == "get" - ): - # we are detailing a single album, so we can add the overhead - # to fetch additional data - tracks = tracks.annotate_duration() + ) qs = queryset.prefetch_related(Prefetch("tracks", queryset=tracks)) return qs.distinct() @@ -194,18 +186,10 @@ class TrackViewSet(TagViewSetMixin, viewsets.ReadOnlyModelViewSet): if user.is_authenticated and filter_favorites == "true": queryset = queryset.filter(track_favorites__user=user) - queryset = queryset.annotate_playable_by_actor( + queryset = queryset.with_playable_uploads( utils.get_actor_from_request(self.request) - ).annotate_duration() - if ( - hasattr(self, "kwargs") - and self.kwargs - and self.request.method.lower() == "get" - ): - # we are detailing a single track, so we can add the overhead - # to fetch additional data - queryset = queryset.annotate_file_data() - return queryset.distinct() + ) + return queryset @detail_route(methods=["get"]) @transaction.non_atomic_requests diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index e1895137d3630ec9970862ce3c85bbfc0472a0ee..1d33388015a80c618628b4923311cab49f15298c 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -38,15 +38,14 @@ class PlaylistQuerySet(models.QuerySet): ) return self.prefetch_related(plt_prefetch) - def annotate_playable_by_actor(self, actor): - plts = ( - PlaylistTrack.objects.playable_by(actor) - .filter(playlist=models.OuterRef("id")) - .order_by("id") - .values("id")[:1] + def with_playable_plts(self, actor): + return self.prefetch_related( + models.Prefetch( + "playlist_tracks", + queryset=PlaylistTrack.objects.playable_by(actor), + to_attr="playable_plts", + ) ) - subquery = models.Subquery(plts) - return self.annotate(is_playable_by_actor=subquery) def playable_by(self, actor, include=True): plts = PlaylistTrack.objects.playable_by(actor, include) @@ -148,7 +147,7 @@ class Playlist(models.Model): class PlaylistTrackQuerySet(models.QuerySet): def for_nested_serialization(self, actor=None): - tracks = music_models.Track.objects.annotate_playable_by_actor(actor) + tracks = music_models.Track.objects.with_playable_uploads(actor) tracks = tracks.select_related("artist", "album__artist") return self.prefetch_related( models.Prefetch("track", queryset=tracks, to_attr="_prefetched_track") @@ -156,8 +155,8 @@ class PlaylistTrackQuerySet(models.QuerySet): def annotate_playable_by_actor(self, actor): tracks = ( - music_models.Track.objects.playable_by(actor) - .filter(pk=models.OuterRef("track")) + music_models.Upload.objects.playable_by(actor) + .filter(track__pk=models.OuterRef("track")) .order_by("id") .values("id")[:1] ) diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index c1ca84e15be2708b6b328f582eadedd3aed08ce1..b64996640259c03a6394c1353bea641afb31dcf5 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -93,7 +93,7 @@ class PlaylistSerializer(serializers.ModelSerializer): def get_is_playable(self, obj): try: - return bool(obj.is_playable_by_actor) + return bool(obj.playable_plts) except AttributeError: return None diff --git a/api/funkwhale_api/playlists/views.py b/api/funkwhale_api/playlists/views.py index 4934b92a019529542702796d4de71d40144885f8..6ff49173c9413bd9c82f7575dd5d1533b1d63489 100644 --- a/api/funkwhale_api/playlists/views.py +++ b/api/funkwhale_api/playlists/views.py @@ -78,7 +78,7 @@ class PlaylistViewSet( def get_queryset(self): return self.queryset.filter( fields.privacy_level_query(self.request.user) - ).annotate_playable_by_actor(music_utils.get_actor_from_request(self.request)) + ).with_playable_plts(music_utils.get_actor_from_request(self.request)) def perform_create(self, serializer): return serializer.save( diff --git a/api/tests/favorites/test_favorites.py b/api/tests/favorites/test_favorites.py index 6ac244c69fac164b49171b3d0f8dba20f13aa02d..0b99c93409a7ca1eb542fe2607d29a3b54c1293c 100644 --- a/api/tests/favorites/test_favorites.py +++ b/api/tests/favorites/test_favorites.py @@ -35,7 +35,6 @@ def test_user_can_get_his_favorites(api_request, factories, logged_in_client, cl "creation_date": favorite.creation_date.isoformat().replace("+00:00", "Z"), } ] - expected[0]["track"]["is_playable"] = False assert response.status_code == 200 assert response.data["results"] == expected diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py index d045a04c258ce61ee3c42c065e6768581a2e167d..874c35afa53a7e1a0a56cda2d3b5c5e91a07f3b7 100644 --- a/api/tests/music/test_models.py +++ b/api/tests/music/test_models.py @@ -464,24 +464,6 @@ def test_library_queryset_with_follows(factories): assert l2._follows == [follow] -def test_annotate_duration(factories): - tf = factories["music.Upload"](duration=32) - - track = models.Track.objects.annotate_duration().get(pk=tf.track.pk) - - assert track.duration == 32 - - -def test_annotate_file_data(factories): - tf = factories["music.Upload"](size=42, bitrate=55, mimetype="audio/ogg") - - track = models.Track.objects.annotate_file_data().get(pk=tf.track.pk) - - assert track.size == 42 - assert track.bitrate == 55 - assert track.mimetype == "audio/ogg" - - @pytest.mark.parametrize( "model,factory_args,namespace", [ diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 330371834d628713ea9cd2fc1cbfe772f19a65a7..3bd13a599150ee8a32af3e00932bd58454853df9 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -48,6 +48,7 @@ def test_artist_with_albums_serializer(factories, to_api_date): def test_album_track_serializer(factories, to_api_date): upload = factories["music.Upload"]() track = upload.track + setattr(track, "playable_uploads", [upload]) expected = { "id": track.id, @@ -56,7 +57,7 @@ def test_album_track_serializer(factories, to_api_date): "mbid": str(track.mbid), "title": track.title, "position": track.position, - "is_playable": None, + "uploads": [serializers.TrackUploadSerializer(upload).data], "creation_date": to_api_date(track.creation_date), "listen_url": track.listen_url, "duration": None, @@ -127,7 +128,7 @@ def test_album_serializer(factories, to_api_date): "title": album.title, "artist": serializers.ArtistSimpleSerializer(album.artist).data, "creation_date": to_api_date(album.creation_date), - "is_playable": None, + "is_playable": False, "cover": { "original": album.cover.url, "square_crop": album.cover.crop["400x400"].url, @@ -145,7 +146,7 @@ def test_album_serializer(factories, to_api_date): def test_track_serializer(factories, to_api_date): upload = factories["music.Upload"]() track = upload.track - + setattr(track, "playable_uploads", [upload]) expected = { "id": track.id, "artist": serializers.ArtistSimpleSerializer(track.artist).data, @@ -153,14 +154,10 @@ def test_track_serializer(factories, to_api_date): "mbid": str(track.mbid), "title": track.title, "position": track.position, - "is_playable": None, + "uploads": [serializers.TrackUploadSerializer(upload).data], "creation_date": to_api_date(track.creation_date), "lyrics": track.get_lyrics_url(), "listen_url": track.listen_url, - "duration": None, - "size": None, - "bitrate": None, - "mimetype": None, } serializer = serializers.TrackSerializer(track) assert serializer.data == expected @@ -260,3 +257,20 @@ def test_manage_upload_action_relaunch_import(factories, mocker): finished.refresh_from_db() assert finished.import_status == "finished" assert m.call_count == 3 + + +def test_track_upload_serializer(factories): + upload = factories["music.Upload"]() + + expected = { + "listen_url": upload.listen_url, + "uuid": str(upload.uuid), + "size": upload.size, + "bitrate": upload.bitrate, + "mimetype": upload.mimetype, + "extension": upload.extension, + "duration": upload.duration, + } + + serializer = serializers.TrackUploadSerializer(upload) + assert serializer.data == expected diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index 10d9fba7517dcaba213f8b4b40c8cb340a49fa4a..4e03d7906c674a34f9947d71f0684261ad4cad33 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -549,11 +549,11 @@ def test_scan_page_trigger_next_page_scan_skip_if_same(mocker, factories, r_mock def test_clean_transcoding_cache(preferences, now, factories): - preferences['music__transcoding_cache_duration'] = 60 - u1 = factories['music.UploadVersion']( + preferences["music__transcoding_cache_duration"] = 60 + u1 = factories["music.UploadVersion"]( accessed_date=now - datetime.timedelta(minutes=61) ) - u2 = factories['music.UploadVersion']( + u2 = factories["music.UploadVersion"]( accessed_date=now - datetime.timedelta(minutes=59) ) @@ -562,4 +562,4 @@ def test_clean_transcoding_cache(preferences, now, factories): u2.refresh_from_db() with pytest.raises(u1.__class__.DoesNotExist): - u1.refresh_from_db() \ No newline at end of file + u1.refresh_from_db() diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index d4ce1e2bd3c5be562d446ef2874b9905616d5aeb..615a592438990b5084679a8e8053974e3c6bc3f7 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -39,13 +39,11 @@ def test_album_list_serializer(api_request, factories, logged_in_api_client): ).track album = track.album request = api_request.get("/") - qs = album.__class__.objects.all() + qs = album.__class__.objects.with_prefetched_tracks_and_playable_uploads(None) serializer = serializers.AlbumSerializer( qs, many=True, context={"request": request} ) expected = {"count": 1, "next": None, "previous": None, "results": serializer.data} - expected["results"][0]["is_playable"] = True - expected["results"][0]["tracks"][0]["is_playable"] = True url = reverse("api:v1:albums-list") response = logged_in_api_client.get(url) @@ -58,12 +56,11 @@ def test_track_list_serializer(api_request, factories, logged_in_api_client): library__privacy_level="everyone", import_status="finished" ).track request = api_request.get("/") - qs = track.__class__.objects.all() + qs = track.__class__.objects.with_playable_uploads(None) serializer = serializers.TrackSerializer( qs, many=True, context={"request": request} ) expected = {"count": 1, "next": None, "previous": None, "results": serializer.data} - expected["results"][0]["is_playable"] = True url = reverse("api:v1:tracks-list") response = logged_in_api_client.get(url) diff --git a/api/tests/playlists/test_models.py b/api/tests/playlists/test_models.py index 46c14d11c7f34ab7088840a60e5e57eb726b2a95..b90f525184b2f4863d8d9f7ab07314ac1e1a1712 100644 --- a/api/tests/playlists/test_models.py +++ b/api/tests/playlists/test_models.py @@ -150,10 +150,6 @@ def test_playlist_playable_by_anonymous(privacy_level, expected, factories): factories["music.Upload"]( track=track, library__privacy_level=privacy_level, import_status="finished" ) - queryset = playlist.__class__.objects.playable_by(None).annotate_playable_by_actor( - None - ) + queryset = playlist.__class__.objects.playable_by(None).with_playable_plts(None) match = playlist in list(queryset) assert match is expected - if expected: - assert bool(queryset.first().is_playable_by_actor) is expected diff --git a/api/tests/playlists/test_views.py b/api/tests/playlists/test_views.py index 1256347f3e379322020838e7504263703fbe267a..1c2b0f19eea43fce59dcf0ed166badd8e5603265 100644 --- a/api/tests/playlists/test_views.py +++ b/api/tests/playlists/test_views.py @@ -145,7 +145,7 @@ def test_can_list_tracks_from_playlist(level, factories, logged_in_api_client): url = reverse("api:v1:playlists-tracks", kwargs={"pk": plt.playlist.pk}) response = logged_in_api_client.get(url) serialized_plt = serializers.PlaylistTrackSerializer(plt).data - serialized_plt["track"]["is_playable"] = False + assert response.data["count"] == 1 assert response.data["results"][0] == serialized_plt