diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 4325a1697..a34c927d5 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -5,6 +5,9 @@ import tempfile import urllib.parse import uuid +from django.db.models.expressions import OuterRef, Subquery +from django.db.models.query_utils import Q + import arrow import pydub from django.conf import settings @@ -319,6 +322,19 @@ class AlbumQuerySet(common_models.LocalFromFidQuerySet, models.QuerySet): else: return self.exclude(pk__in=matches) + def with_duration(self): + # takes one upload per track + subquery = Subquery( + Upload.objects.filter(track_id=OuterRef("tracks")) + .order_by("id") + .values("id")[:1] + ) + return self.annotate( + duration=models.Sum( + "tracks__uploads__duration", filter=Q(tracks__uploads=subquery), + ) + ) + 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 6e2c0fe2c..b207b68fc 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -201,6 +201,7 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): release_date = serializers.DateField() creation_date = serializers.DateTimeField() is_local = serializers.BooleanField() + duration = serializers.SerializerMethodField(read_only=True) get_attributed_to = serialize_attributed_to @@ -222,6 +223,13 @@ class AlbumSerializer(OptionalDescriptionMixin, serializers.Serializer): tagged_items = getattr(obj, "_prefetched_tagged_items", []) return [ti.tag.name for ti in tagged_items] + def get_duration(self, obj): + try: + return obj.duration + except AttributeError: + # no annotation? + return 0 + class TrackAlbumSerializer(serializers.ModelSerializer): artist = serializers.SerializerMethodField() diff --git a/api/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index 0048e448d..2b3bae2e1 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -1,8 +1,6 @@ import collections from django.db.models import Count, functions -from django.db.models import Sum -from django.db.models.expressions import OuterRef, Subquery from rest_framework import serializers from funkwhale_api.history import models as history_models @@ -143,22 +141,13 @@ def get_track_data(album, track, upload): def get_album2_data(album): - # takes one upload per track - subquery = Subquery( - music_models.Upload.objects.filter(track_id=OuterRef("id")) - .order_by("id") - .values("id")[:1] - ) payload = { "id": album.id, "artistId": album.artist.id, "name": album.title, "artist": album.artist.name, "created": to_subsonic_date(album.creation_date), - "duration": album.tracks.filter(uploads__in=subquery).aggregate( - d=Sum("uploads__duration") - )["d"] - or 0, + "duration": album.duration, "playCount": album.tracks.aggregate(l=Count("listenings"))["l"] or 0, } if album.attachment_cover_id: diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index f03fe2654..bb576c3ce 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -265,7 +265,8 @@ class SubsonicViewSet(viewsets.GenericViewSet): detail=False, methods=["get", "post"], url_name="get_album", url_path="getAlbum" ) @find_object( - music_models.Album.objects.select_related("artist"), filter_playable=True + music_models.Album.objects.with_duration().select_related("artist"), + filter_playable=True, ) def get_album(self, request, *args, **kwargs): album = kwargs.pop("obj") @@ -443,6 +444,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): ) ) .with_tracks_count() + .with_duration() .order_by("artist__name") ) data = request.GET or request.POST @@ -533,9 +535,9 @@ class SubsonicViewSet(viewsets.GenericViewSet): "subsonic": "album", "search_fields": ["title"], "queryset": ( - music_models.Album.objects.with_tracks_count().select_related( - "artist" - ) + music_models.Album.objects.with_duration() + .with_tracks_count() + .select_related("artist") ), "serializer": serializers.get_album_list2_data, }, diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index a0746d3b8..24dd99c26 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -182,6 +182,7 @@ def test_album_serializer(factories, to_api_date): "artist": serializers.serialize_artist_simple(album.artist), "creation_date": to_api_date(album.creation_date), "is_playable": False, + "duration": 0, "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "release_date": to_api_date(album.release_date), "tracks_count": 2, @@ -214,6 +215,7 @@ def test_track_album_serializer(factories, to_api_date): "cover": common_serializers.AttachmentSerializer(album.attachment_cover).data, "release_date": to_api_date(album.release_date), "tracks_count": 2, + "duration": 0, "is_local": album.is_local, "tags": [], "attributed_to": federation_serializers.APIActorSerializer(actor).data, @@ -605,3 +607,44 @@ def test_sort_uploads_for_listen(factories): remote_upload_with_local_version, ] assert serializers.sort_uploads_for_listen(unsorted) == expected + + +def test_album_serializer_includes_duration(tmpfile, factories): + album = factories["music.Album"]() + event = { + "path": tmpfile.name, + } + library = factories["music.Library"]() + track1 = factories["music.Track"](album=album) + track2 = factories["music.Track"](album=album) + factories["music.Upload"]( + source="file://{}".format(event["path"]), + track=track1, + checksum="old", + library=library, + import_status="finished", + audio_file=None, + duration=21, + ) + factories["music.Upload"]( + source="file://{}".format(event["path"]), + track=track1, + checksum="old", + library=library, + import_status="finished", + audio_file=None, + duration=21, + ) + factories["music.Upload"]( + source="file://{}".format(event["path"]), + track=track2, + checksum="old", + library=library, + import_status="finished", + audio_file=None, + duration=21, + ) + qs = album.__class__.objects.with_duration() + + serializer = serializers.AlbumSerializer(qs.get()) + assert serializer.data["duration"] == 42 diff --git a/api/tests/subsonic/test_serializers.py b/api/tests/subsonic/test_serializers.py index 75d1de895..d6d9458f6 100644 --- a/api/tests/subsonic/test_serializers.py +++ b/api/tests/subsonic/test_serializers.py @@ -1,7 +1,6 @@ import datetime -from django.db.models.aggregates import Count, Sum -from django.db.models.expressions import OuterRef, Subquery +from django.db.models.aggregates import Count import pytest from funkwhale_api.music import models as music_models @@ -175,12 +174,6 @@ def test_get_album_serializer(factories): track = factories["music.Track"](album=album, disc_number=42) upload = factories["music.Upload"](track=track, bitrate=42000, duration=43, size=44) tagged_item = factories["tags.TaggedItem"](content_object=album, tag__name="foo") - # takes one upload per track - subquery = Subquery( - music_models.Upload.objects.filter(track_id=OuterRef("id")) - .order_by("id") - .values("id")[:1] - ) expected = { "id": album.pk, "artistId": artist.pk, @@ -191,10 +184,7 @@ def test_get_album_serializer(factories): "year": album.release_date.year, "coverArt": "al-{}".format(album.id), "genre": tagged_item.tag.name, - "duration": album.tracks.filter(uploads__in=subquery).aggregate( - d=Sum("uploads__duration") - )["d"] - or 0, + "duration": 43, "playCount": album.tracks.aggregate(l=Count("listenings"))["l"] or 0, "song": [ { @@ -221,7 +211,9 @@ def test_get_album_serializer(factories): ], } - assert serializers.GetAlbumSerializer(album).data == expected + qs = album.__class__.objects.with_duration() + + assert serializers.GetAlbumSerializer(qs.first()).data == expected def test_starred_tracks2_serializer(factories): @@ -237,10 +229,10 @@ def test_starred_tracks2_serializer(factories): def test_get_album_list2_serializer(factories): - album1 = factories["music.Album"]() - album2 = factories["music.Album"]() + album1 = factories["music.Album"]().__class__.objects.with_duration().first() + album2 = factories["music.Album"]().__class__.objects.with_duration().last() - qs = music_models.Album.objects.with_tracks_count().order_by("pk") + qs = music_models.Album.objects.with_tracks_count().with_duration().order_by("pk") expected = [ serializers.get_album2_data(album1), serializers.get_album2_data(album2), diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index 5f28b1fea..473120075 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -182,7 +182,11 @@ def test_get_album( url = reverse("api:subsonic-get_album") assert url.endswith("getAlbum") is True artist = factories["music.Artist"]() - album = factories["music.Album"](artist=artist) + album = ( + factories["music.Album"](artist=artist) + .__class__.objects.with_duration() + .first() + ) factories["music.Track"].create_batch(size=3, album=album, playable=True) playable_by = mocker.spy(music_models.AlbumQuerySet, "playable_by") expected = {"album": serializers.GetAlbumSerializer(album).data} @@ -192,7 +196,7 @@ def test_get_album( assert response.data == expected playable_by.assert_called_once_with( - music_models.Album.objects.select_related("artist"), None + music_models.Album.objects.with_duration().select_related("artist"), None ) @@ -422,8 +426,12 @@ def test_get_album_list2( ): url = reverse("api:subsonic-get_album_list2") assert url.endswith("getAlbumList2") is True - album1 = factories["music.Album"](playable=True) - album2 = factories["music.Album"](playable=True) + album1 = factories["music.Album"](playable=True).__class__.objects.with_duration()[ + 0 + ] + album2 = factories["music.Album"](playable=True).__class__.objects.with_duration()[ + 1 + ] factories["music.Album"]() playable_by = mocker.spy(music_models.AlbumQuerySet, "playable_by") response = logged_in_api_client.get(url, {"f": f, "type": "newest"}) @@ -439,8 +447,12 @@ def test_get_album_list2_recent(db, logged_in_api_client, factories): url = reverse("api:subsonic-get_album_list2") assert url.endswith("getAlbumList2") is True factories["music.Album"](playable=True, release_date=None) - album2 = factories["music.Album"](playable=True) - album3 = factories["music.Album"](playable=True) + album2 = factories["music.Album"](playable=True).__class__.objects.with_duration()[ + 1 + ] + album3 = factories["music.Album"](playable=True).__class__.objects.with_duration()[ + 2 + ] response = logged_in_api_client.get(url, {"f": "json", "type": "recent"}) assert response.status_code == 200 @@ -454,7 +466,11 @@ def test_get_album_list2_recent(db, logged_in_api_client, factories): def test_get_album_list2_pagination(f, db, logged_in_api_client, factories): url = reverse("api:subsonic-get_album_list2") assert url.endswith("getAlbumList2") is True - album1 = factories["music.Album"](playable=True) + album1 = ( + factories["music.Album"](playable=True) + .__class__.objects.with_duration() + .first() + ) factories["music.Album"](playable=True) response = logged_in_api_client.get( url, {"f": f, "type": "newest", "size": 1, "offset": 1} @@ -472,10 +488,10 @@ def test_get_album_list2_by_genre(f, db, logged_in_api_client, factories): assert url.endswith("getAlbumList2") is True album1 = factories["music.Album"]( artist__name="Artist1", playable=True, set_tags=["Rock"] - ) + ).__class__.objects.with_duration()[0] album2 = factories["music.Album"]( artist__name="Artist2", playable=True, artist__set_tags=["Rock"] - ) + ).__class__.objects.with_duration()[1] factories["music.Album"](playable=True, set_tags=["Pop"]) response = logged_in_api_client.get( url, {"f": f, "type": "byGenre", "size": 5, "offset": 0, "genre": "rock"} @@ -500,7 +516,7 @@ def test_get_album_list2_by_year(params, expected, db, logged_in_api_client, fac albums = [ factories["music.Album"]( playable=True, release_date=datetime.date(1900 + i, 1, 1) - ) + ).__class__.objects.with_duration()[i] for i in range(5) ] url = reverse("api:subsonic-get_album_list2") @@ -562,7 +578,9 @@ def test_search3(f, db, logged_in_api_client, factories): artist = factories["music.Artist"](name="testvalue", playable=True) factories["music.Artist"](name="nope") factories["music.Artist"](name="nope2", playable=True) - album = factories["music.Album"](title="testvalue", playable=True) + album = factories["music.Album"]( + title="testvalue", playable=True + ).__class__.objects.with_duration()[2] factories["music.Album"](title="nope") factories["music.Album"](title="nope2", playable=True) track = factories["music.Track"](title="testvalue", playable=True)