refactor: Adds duration annotation to album's queryset

MR !1408
environments/review-front-fix-rjvmfb/deployments/8579
Marcos Peña 2021-11-26 07:35:12 +00:00 zatwierdzone przez JuniorJPDJ
rodzic 4df883678b
commit 274bdd1d3e
7 zmienionych plików z 111 dodań i 43 usunięć

Wyświetl plik

@ -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"])

Wyświetl plik

@ -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()

Wyświetl plik

@ -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:

Wyświetl plik

@ -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,
},

Wyświetl plik

@ -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

Wyświetl plik

@ -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),

Wyświetl plik

@ -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)