diff --git a/api/funkwhale_api/favorites/serializers.py b/api/funkwhale_api/favorites/serializers.py index 276b0f6bde6d19120998a21f58f44a5dd3ffbd01..bb4538b2d91c709c86072f0d75d5808e98bbec57 100644 --- a/api/funkwhale_api/favorites/serializers.py +++ b/api/funkwhale_api/favorites/serializers.py @@ -3,7 +3,6 @@ from django.conf import settings from rest_framework import serializers from funkwhale_api.activity import serializers as activity_serializers -from funkwhale_api.music.serializers import TrackSerializerNested from funkwhale_api.music.serializers import TrackActivitySerializer from funkwhale_api.users.serializers import UserActivitySerializer @@ -35,7 +34,6 @@ class TrackFavoriteActivitySerializer(activity_serializers.ModelSerializer): class UserTrackFavoriteSerializer(serializers.ModelSerializer): - # track = TrackSerializerNested(read_only=True) class Meta: model = models.TrackFavorite fields = ('id', 'track', 'creation_date') diff --git a/api/funkwhale_api/history/serializers.py b/api/funkwhale_api/history/serializers.py index f7333f24349777a6a41b15e4de0d6a282502aacd..572787ae0031cb5be38c80014b7484f286de26e3 100644 --- a/api/funkwhale_api/history/serializers.py +++ b/api/funkwhale_api/history/serializers.py @@ -1,7 +1,6 @@ from rest_framework import serializers from funkwhale_api.activity import serializers as activity_serializers -from funkwhale_api.music.serializers import TrackSerializerNested from funkwhale_api.music.serializers import TrackActivitySerializer from funkwhale_api.users.serializers import UserActivitySerializer diff --git a/api/funkwhale_api/history/views.py b/api/funkwhale_api/history/views.py index bea96a4187792421bf6827717eb1032e9dcaacf3..3da8b2a38bed467ffe5fb83f7ea4c8d1d000c406 100644 --- a/api/funkwhale_api/history/views.py +++ b/api/funkwhale_api/history/views.py @@ -6,7 +6,6 @@ from rest_framework.decorators import detail_route from funkwhale_api.activity import record from funkwhale_api.common.permissions import ConditionalAuthentication -from funkwhale_api.music.serializers import TrackSerializerNested from . import models from . import serializers diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index 6da9cca63636fd51c562f066c790c473093cffe3..dc7aafc219644ee87c70d0d3e495c9f4b79be04b 100644 --- a/api/funkwhale_api/music/filters.py +++ b/api/funkwhale_api/music/filters.py @@ -32,6 +32,33 @@ class ArtistFilter(ListenableMixin): } +class TrackFilter(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'title', + 'album__title', + 'artist__name', + ]) + listenable = filters.BooleanFilter(name='_', method='filter_listenable') + + class Meta: + model = models.Track + fields = { + 'title': ['exact', 'iexact', 'startswith', 'icontains'], + 'listenable': ['exact'], + 'artist': ['exact'], + 'album': ['exact'], + } + + def filter_listenable(self, queryset, name, value): + queryset = queryset.annotate( + files_count=Count('files') + ) + if value: + return queryset.filter(files_count__gt=0) + else: + return queryset.filter(files_count=0) + + class ImportBatchFilter(filters.FilterSet): q = fields.SearchFilter(search_fields=[ 'submitted_by__username', @@ -67,7 +94,12 @@ class ImportJobFilter(filters.FilterSet): class AlbumFilter(ListenableMixin): listenable = filters.BooleanFilter(name='_', method='filter_listenable') + q = fields.SearchFilter(search_fields=[ + 'title', + 'artist__name' + 'source', + ]) class Meta: model = models.Album - fields = ['listenable'] + fields = ['listenable', 'q', 'artist'] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 1259cc3c12406a7848649d829dfed7e8999f4539..51ece7d7b3f98b1888139a22c74a5066c1bd7862 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -80,6 +80,12 @@ class ArtistQuerySet(models.QuerySet): def with_albums_count(self): return self.annotate(_albums_count=models.Count('albums')) + def with_albums(self): + return self.prefetch_related( + models.Prefetch( + 'albums', queryset=Album.objects.with_tracks_count()) + ) + class Artist(APIModelMixin): name = models.CharField(max_length=255) @@ -313,11 +319,8 @@ class Lyrics(models.Model): class TrackQuerySet(models.QuerySet): def for_nested_serialization(self): return (self.select_related() - .select_related('album__artist') - .prefetch_related( - 'tags', - 'files', - 'artist__albums__tracks__tags')) + .select_related('album__artist', 'artist') + .prefetch_related('files')) class Track(APIModelMixin): diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index d9d48496e487395be4ff0516b64f43b1074f37c1..c77983a404cf89e9e9f4779ecc3e19773136fda0 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -13,24 +13,38 @@ from . import models from . import tasks -class TagSerializer(serializers.ModelSerializer): - class Meta: - model = Tag - fields = ('id', 'name', 'slug') - +class ArtistAlbumSerializer(serializers.ModelSerializer): + tracks_count = serializers.SerializerMethodField() -class SimpleArtistSerializer(serializers.ModelSerializer): class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'creation_date') + model = models.Album + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'release_date', + 'cover', + 'creation_date', + 'tracks_count', + ) + def get_tracks_count(self, o): + return o._tracks_count -class ArtistSerializer(serializers.ModelSerializer): - tags = TagSerializer(many=True, read_only=True) + +class ArtistWithAlbumsSerializer(serializers.ModelSerializer): + albums = ArtistAlbumSerializer(many=True, read_only=True) class Meta: model = models.Artist - fields = ('id', 'mbid', 'name', 'tags', 'creation_date') + fields = ( + 'id', + 'mbid', + 'name', + 'creation_date', + 'albums', + ) class TrackFileSerializer(serializers.ModelSerializer): @@ -62,71 +76,110 @@ class TrackFileSerializer(serializers.ModelSerializer): return url -class SimpleAlbumSerializer(serializers.ModelSerializer): +class AlbumTrackSerializer(serializers.ModelSerializer): + files = TrackFileSerializer(many=True, read_only=True) class Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'release_date', 'cover') + model = models.Track + fields = ( + 'id', + 'mbid', + 'title', + 'album', + 'artist', + 'creation_date', + 'files', + 'position', + ) + + +class ArtistSimpleSerializer(serializers.ModelSerializer): + class Meta: + model = models.Artist + fields = ( + 'id', + 'mbid', + 'name', + 'creation_date', + ) class AlbumSerializer(serializers.ModelSerializer): - tags = TagSerializer(many=True, read_only=True) + tracks = serializers.SerializerMethodField() + artist = ArtistSimpleSerializer(read_only=True) + class Meta: model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'release_date', 'tags') - + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'tracks', + 'release_date', + 'cover', + 'creation_date', + ) -class LyricsMixin(serializers.ModelSerializer): - lyrics = serializers.SerializerMethodField() + def get_tracks(self, o): + ordered_tracks = sorted( + o.tracks.all(), + key=lambda v: (v.position, v.title) if v.position else (99999, v.title) + ) + return AlbumTrackSerializer(ordered_tracks, many=True).data - def get_lyrics(self, obj): - return obj.get_lyrics_url() +class TrackAlbumSerializer(serializers.ModelSerializer): + artist = ArtistSimpleSerializer(read_only=True) -class TrackSerializer(LyricsMixin): - files = TrackFileSerializer(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) class Meta: - model = models.Track + model = models.Album fields = ( 'id', 'mbid', 'title', 'artist', - 'files', - 'tags', - 'position', - 'lyrics') + 'release_date', + 'cover', + 'creation_date', + ) -class TrackSerializerNested(LyricsMixin): - artist = ArtistSerializer() +class TrackSerializer(serializers.ModelSerializer): files = TrackFileSerializer(many=True, read_only=True) - album = SimpleAlbumSerializer(read_only=True) - tags = TagSerializer(many=True, read_only=True) + artist = ArtistSimpleSerializer(read_only=True) + album = TrackAlbumSerializer(read_only=True) + lyrics = serializers.SerializerMethodField() class Meta: model = models.Track - fields = ('id', 'mbid', 'title', 'artist', 'files', 'album', 'tags', 'lyrics') + fields = ( + 'id', + 'mbid', + 'title', + 'album', + 'artist', + 'creation_date', + 'files', + 'position', + 'lyrics', + ) + def get_lyrics(self, obj): + return obj.get_lyrics_url() -class AlbumSerializerNested(serializers.ModelSerializer): - tracks = TrackSerializer(many=True, read_only=True) - artist = SimpleArtistSerializer() - tags = TagSerializer(many=True, read_only=True) +class TagSerializer(serializers.ModelSerializer): class Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'artist', 'release_date', 'tracks', 'tags') + model = Tag + fields = ('id', 'name', 'slug') -class ArtistSerializerNested(serializers.ModelSerializer): - albums = AlbumSerializerNested(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) +class SimpleAlbumSerializer(serializers.ModelSerializer): class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'albums', 'tags') + model = models.Album + fields = ('id', 'mbid', 'title', 'release_date', 'cover') class LyricsSerializer(serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index e71d3555e67a21012ee34d79e2fd56b28354fcf8..24a9cbbcd0c863f5b5351380d1be5c0c5b0836b0 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -46,17 +46,6 @@ from . import utils logger = logging.getLogger(__name__) -class SearchMixin(object): - search_fields = [] - - @list_route(methods=['get']) - def search(self, request, *args, **kwargs): - query = utils.get_query(request.GET['query'], self.search_fields) - queryset = self.get_queryset().filter(query) - serializer = self.serializer_class(queryset, many=True) - return Response(serializer.data) - - class TagViewSetMixin(object): def get_queryset(self): @@ -67,31 +56,25 @@ class TagViewSetMixin(object): return queryset -class ArtistViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): - queryset = ( - models.Artist.objects.all() - .prefetch_related( - 'albums__tracks__files', - 'albums__tracks__artist', - 'albums__tracks__tags')) - serializer_class = serializers.ArtistSerializerNested +class ArtistViewSet(viewsets.ReadOnlyModelViewSet): + queryset = models.Artist.objects.with_albums() + serializer_class = serializers.ArtistWithAlbumsSerializer permission_classes = [ConditionalAuthentication] - search_fields = ['name__unaccent'] filter_class = filters.ArtistFilter ordering_fields = ('id', 'name', 'creation_date') -class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): +class AlbumViewSet(viewsets.ReadOnlyModelViewSet): queryset = ( models.Album.objects.all() - .order_by('-creation_date') + .order_by('artist', 'release_date') .select_related() - .prefetch_related('tracks__tags', - 'tracks__files')) - serializer_class = serializers.AlbumSerializerNested + .prefetch_related( + 'tracks__artist', + 'tracks__files')) + serializer_class = serializers.AlbumSerializer permission_classes = [ConditionalAuthentication] - search_fields = ['title__unaccent'] - ordering_fields = ('creation_date',) + ordering_fields = ('creation_date', 'release_date', 'title') filter_class = filters.AlbumFilter @@ -160,19 +143,20 @@ class ImportJobViewSet( ) -class TrackViewSet( - TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): +class TrackViewSet(TagViewSetMixin, viewsets.ReadOnlyModelViewSet): """ A simple ViewSet for viewing and editing accounts. """ queryset = (models.Track.objects.all().for_nested_serialization()) - serializer_class = serializers.TrackSerializerNested + serializer_class = serializers.TrackSerializer permission_classes = [ConditionalAuthentication] - search_fields = ['title', 'artist__name'] + filter_class = filters.TrackFilter ordering_fields = ( 'creation_date', 'title__unaccent', 'album__title__unaccent', + 'album__release_date', + 'position', 'artist__name__unaccent', ) @@ -370,10 +354,10 @@ class Search(views.APIView): def get(self, request, *args, **kwargs): query = request.GET['query'] results = { - 'tags': serializers.TagSerializer(self.get_tags(query), many=True).data, - 'artists': serializers.ArtistSerializerNested(self.get_artists(query), many=True).data, - 'tracks': serializers.TrackSerializerNested(self.get_tracks(query), many=True).data, - 'albums': serializers.AlbumSerializerNested(self.get_albums(query), many=True).data, + # 'tags': serializers.TagSerializer(self.get_tags(query), many=True).data, + 'artists': serializers.ArtistWithAlbumsSerializer(self.get_artists(query), many=True).data, + 'tracks': serializers.TrackSerializer(self.get_tracks(query), many=True).data, + 'albums': serializers.AlbumSerializer(self.get_albums(query), many=True).data, } return Response(results, status=200) @@ -387,14 +371,10 @@ class Search(views.APIView): return ( models.Track.objects.all() .filter(query_obj) - .select_related('album__artist') - .prefetch_related( - 'tags', - 'artist__albums__tracks__tags', - 'files') + .select_related('artist', 'album__artist') + .prefetch_related('files') )[:self.max_results] - def get_albums(self, query): search_fields = [ 'mbid', @@ -406,27 +386,19 @@ class Search(views.APIView): .filter(query_obj) .select_related() .prefetch_related( - 'tracks__tags', 'tracks__files', - ) + ) )[:self.max_results] - def get_artists(self, query): search_fields = ['mbid', 'name__unaccent'] query_obj = utils.get_query(query, search_fields) return ( models.Artist.objects.all() .filter(query_obj) - .select_related() - .prefetch_related( - 'albums__tracks__tags', - 'albums__tracks__files', - ) - + .with_albums() )[:self.max_results] - def get_tags(self, query): search_fields = ['slug', 'name__unaccent'] query_obj = utils.get_query(query, search_fields) diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index fcb2a412d49b6501eed5d880c5a93f14d87985d8..3f01fd68942b01b0f109e8f32a0eeca29c990585 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -5,13 +5,13 @@ from taggit.models import Tag from funkwhale_api.common import preferences from funkwhale_api.music.models import Track -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.users.serializers import UserBasicSerializer from . import models class PlaylistTrackSerializer(serializers.ModelSerializer): - track = TrackSerializerNested() + track = TrackSerializer() class Meta: model = models.PlaylistTrack diff --git a/api/funkwhale_api/radios/filters.py b/api/funkwhale_api/radios/filters.py index 344a4dabff3fb29b1d0fc7d5b8f4bda480c786ae..d0d338d663a62008ce6f1ccf03e3e4f8ed679b37 100644 --- a/api/funkwhale_api/radios/filters.py +++ b/api/funkwhale_api/radios/filters.py @@ -144,8 +144,8 @@ class ArtistFilter(RadioFilter): 'name': 'ids', 'type': 'list', 'subtype': 'number', - 'autocomplete': reverse_lazy('api:v1:artists-search'), - 'autocomplete_qs': 'query={query}', + 'autocomplete': reverse_lazy('api:v1:artists-list'), + 'autocomplete_qs': 'q={query}', 'autocomplete_fields': {'name': 'name', 'value': 'id'}, 'label': 'Artist', 'placeholder': 'Select artists' diff --git a/api/funkwhale_api/radios/serializers.py b/api/funkwhale_api/radios/serializers.py index 195b382c99a4b32872b34923f958e1c8b682ac94..8c59f87156888d66a8c0f329ae91b436eb09b506 100644 --- a/api/funkwhale_api/radios/serializers.py +++ b/api/funkwhale_api/radios/serializers.py @@ -1,6 +1,6 @@ from rest_framework import serializers -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.users.serializers import UserBasicSerializer from . import filters @@ -46,7 +46,7 @@ class RadioSessionTrackSerializerCreate(serializers.ModelSerializer): class RadioSessionTrackSerializer(serializers.ModelSerializer): - track = TrackSerializerNested() + track = TrackSerializer() class Meta: model = models.RadioSessionTrack diff --git a/api/funkwhale_api/radios/views.py b/api/funkwhale_api/radios/views.py index 37c07c5e4cdf7c30799aa8b3af12fd94d1150274..ca510b82c431beb89f08a7e173eb12ab3fd2abaf 100644 --- a/api/funkwhale_api/radios/views.py +++ b/api/funkwhale_api/radios/views.py @@ -7,7 +7,7 @@ from rest_framework import status from rest_framework.response import Response from rest_framework.decorators import detail_route, list_route -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.common.permissions import ConditionalAuthentication from . import models @@ -49,7 +49,7 @@ class RadioViewSet( page = self.paginate_queryset(tracks) if page is not None: - serializer = TrackSerializerNested(page, many=True) + serializer = TrackSerializer(page, many=True) return self.get_paginated_response(serializer.data) @list_route(methods=['get']) @@ -72,7 +72,7 @@ class RadioViewSet( results = filters.test(f) if results['candidates']['sample']: qs = results['candidates']['sample'].for_nested_serialization() - results['candidates']['sample'] = TrackSerializerNested( + results['candidates']['sample'] = TrackSerializer( qs, many=True).data data['filters'].append(results) diff --git a/api/funkwhale_api/requests/views.py b/api/funkwhale_api/requests/views.py index 395fac66cff33e3a01fdcc33a56adfcea0aa0c7e..6553f3316fb3cbd50f040202d0bb36a38a7fceb6 100644 --- a/api/funkwhale_api/requests/views.py +++ b/api/funkwhale_api/requests/views.py @@ -3,15 +3,12 @@ from rest_framework import status from rest_framework.response import Response from rest_framework.decorators import detail_route -from funkwhale_api.music.views import SearchMixin - from . import filters from . import models from . import serializers class ImportRequestViewSet( - SearchMixin, mixins.CreateModelMixin, mixins.RetrieveModelMixin, mixins.ListModelMixin, @@ -22,7 +19,6 @@ class ImportRequestViewSet( models.ImportRequest.objects.all() .select_related() .order_by('-creation_date')) - search_fields = ['artist_name', 'album_name', 'comment'] filter_class = filters.ImportRequestFilter ordering_fields = ('id', 'artist_name', 'creation_date', 'status') diff --git a/api/tests/conftest.py b/api/tests/conftest.py index b7a7d071ab6e8e548b7026b84ff815c0f2e6e43a..d6b1c0204e906b7fe073c4794adccdc00430928c 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,3 +1,4 @@ +import datetime import factory import pytest import requests_mock @@ -10,6 +11,7 @@ from django.test import client from dynamic_preferences.registries import global_preferences_registry +from rest_framework import fields as rest_fields from rest_framework.test import APIClient from rest_framework.test import APIRequestFactory @@ -233,3 +235,16 @@ def assert_user_permission(): assert HasUserPermission in view.permission_classes assert set(view.required_permissions) == set(permissions) return inner + + +@pytest.fixture +def to_api_date(): + def inner(value): + if isinstance(value, datetime.datetime): + f = rest_fields.DateTimeField() + return f.to_representation(value) + if isinstance(value, datetime.date): + f = rest_fields.DateField() + return f.to_representation(value) + raise ValueError('Invalid value: {}'.format(value)) + return inner diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 53ee29f3e909a9c4a78022d0669fdc3710412513..7aa20e6262dd42f204054cf58b4d1f69cd2795a5 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -223,41 +223,6 @@ def test_user_can_create_import_job_with_file( import_job_id=job.pk) -def test_can_search_artist(factories, logged_in_client): - artist1 = factories['music.Artist']() - artist2 = factories['music.Artist']() - expected = [serializers.ArtistSerializerNested(artist1).data] - url = reverse('api:v1:artists-search') - response = logged_in_client.get(url, {'query': artist1.name}) - assert response.data == expected - - -def test_can_search_artist_by_name_start(factories, logged_in_client): - artist1 = factories['music.Artist'](name='alpha') - artist2 = factories['music.Artist'](name='beta') - expected = { - 'next': None, - 'previous': None, - 'count': 1, - 'results': [serializers.ArtistSerializerNested(artist1).data] - } - url = reverse('api:v1:artists-list') - response = logged_in_client.get(url, {'name__startswith': 'a'}) - - assert expected == response.data - - -def test_can_search_tracks(factories, logged_in_client): - track1 = factories['music.Track'](title="test track 1") - track2 = factories['music.Track']() - query = 'test track 1' - expected = [serializers.TrackSerializerNested(track1).data] - url = reverse('api:v1:tracks-search') - response = logged_in_client.get(url, {'query': query}) - - assert expected == response.data - - @pytest.mark.parametrize('route,method', [ ('api:v1:tags-list', 'get'), ('api:v1:tracks-list', 'get'), diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py new file mode 100644 index 0000000000000000000000000000000000000000..fa22ceceeb79199b40ca71297fb4bc0f849dfc7e --- /dev/null +++ b/api/tests/music/test_serializers.py @@ -0,0 +1,121 @@ +from funkwhale_api.music import serializers + + +def test_artist_album_serializer(factories, to_api_date): + track = factories['music.Track']() + album = track.album + album = album.__class__.objects.with_tracks_count().get(pk=album.pk) + expected = { + 'id': album.id, + 'mbid': str(album.mbid), + 'title': album.title, + 'artist': album.artist.id, + 'creation_date': to_api_date(album.creation_date), + 'tracks_count': 1, + 'cover': album.cover.url, + 'release_date': to_api_date(album.release_date), + } + serializer = serializers.ArtistAlbumSerializer(album) + + assert serializer.data == expected + + +def test_artist_with_albums_serializer(factories, to_api_date): + track = factories['music.Track']() + artist = track.artist + artist = artist.__class__.objects.with_albums().get(pk=artist.pk) + album = list(artist.albums.all())[0] + + expected = { + 'id': artist.id, + 'mbid': str(artist.mbid), + 'name': artist.name, + 'creation_date': to_api_date(artist.creation_date), + 'albums': [ + serializers.ArtistAlbumSerializer(album).data + ] + } + serializer = serializers.ArtistWithAlbumsSerializer(artist) + assert serializer.data == expected + + +def test_album_track_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + track = tf.track + + expected = { + 'id': track.id, + 'artist': track.artist.id, + 'album': track.album.id, + 'mbid': str(track.mbid), + 'title': track.title, + 'position': track.position, + 'creation_date': to_api_date(track.creation_date), + 'files': [ + serializers.TrackFileSerializer(tf).data + ] + } + serializer = serializers.AlbumTrackSerializer(track) + assert serializer.data == expected + + +def test_track_file_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + + expected = { + 'id': tf.id, + 'path': tf.path, + 'source': tf.source, + 'filename': tf.filename, + 'mimetype': tf.mimetype, + 'track': tf.track.pk, + 'duration': tf.duration, + 'mimetype': tf.mimetype, + 'bitrate': tf.bitrate, + 'size': tf.size, + } + serializer = serializers.TrackFileSerializer(tf) + assert serializer.data == expected + + +def test_album_serializer(factories, to_api_date): + track1 = factories['music.Track'](position=2) + track2 = factories['music.Track'](position=1, album=track1.album) + album = track1.album + expected = { + 'id': album.id, + 'mbid': str(album.mbid), + 'title': album.title, + 'artist': serializers.ArtistSimpleSerializer(album.artist).data, + 'creation_date': to_api_date(album.creation_date), + 'cover': album.cover.url, + 'release_date': to_api_date(album.release_date), + 'tracks': serializers.AlbumTrackSerializer( + [track2, track1], + many=True + ).data + } + serializer = serializers.AlbumSerializer(album) + + assert serializer.data == expected + + +def test_track_serializer(factories, to_api_date): + tf = factories['music.TrackFile']() + track = tf.track + + expected = { + 'id': track.id, + 'artist': serializers.ArtistSimpleSerializer(track.artist).data, + 'album': serializers.TrackAlbumSerializer(track.album).data, + 'mbid': str(track.mbid), + 'title': track.title, + 'position': track.position, + 'creation_date': to_api_date(track.creation_date), + 'lyrics': track.get_lyrics_url(), + 'files': [ + serializers.TrackFileSerializer(tf).data + ] + } + serializer = serializers.TrackSerializer(track) + assert serializer.data == expected diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 030fc3a73eeeabaf0507d5bdbc9949e3d5c4bff5..38366442f309a4b80eb25f7dfeda517b608ab778 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -4,6 +4,7 @@ import pytest from django.urls import reverse from django.utils import timezone +from funkwhale_api.music import serializers from funkwhale_api.music import views from funkwhale_api.federation import actors @@ -16,6 +17,65 @@ def test_permissions(assert_user_permission, view, permissions): assert_user_permission(view, permissions) +def test_artist_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + artist = track.artist + request = api_request.get('/') + qs = artist.__class__.objects.with_albums() + serializer = serializers.ArtistWithAlbumsSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:artists-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + +def test_album_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + album = track.album + request = api_request.get('/') + qs = album.__class__.objects.all() + serializer = serializers.AlbumSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:albums-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + +def test_track_list_serializer(api_request, factories, logged_in_api_client): + track = factories['music.Track']() + request = api_request.get('/') + qs = track.__class__.objects.all() + serializer = serializers.TrackSerializer( + qs, many=True, context={'request': request}) + expected = { + 'count': 1, + 'next': None, + 'previous': None, + 'results': serializer.data + } + url = reverse('api:v1:tracks-list') + response = logged_in_api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + @pytest.mark.parametrize('param,expected', [ ('true', 'full'), ('false', 'empty'), diff --git a/api/tests/radios/test_api.py b/api/tests/radios/test_api.py index 25c099014e91edea53a00850af44ba967480a7bd..66bf6052d539ef1745c45c277385b099a18c51d3 100644 --- a/api/tests/radios/test_api.py +++ b/api/tests/radios/test_api.py @@ -3,7 +3,7 @@ import pytest from django.urls import reverse -from funkwhale_api.music.serializers import TrackSerializerNested +from funkwhale_api.music.serializers import TrackSerializer from funkwhale_api.radios import filters from funkwhale_api.radios import serializers @@ -43,7 +43,7 @@ def test_can_validate_config(logged_in_client, factories): expected = { 'count': candidates.count(), - 'sample': TrackSerializerNested(candidates, many=True).data + 'sample': TrackSerializer(candidates, many=True).data } assert payload['filters'][0]['candidates'] == expected assert payload['filters'][0]['errors'] == [] diff --git a/changes/changelog.d/224.enhancement b/changes/changelog.d/224.enhancement new file mode 100644 index 0000000000000000000000000000000000000000..43c2e88c01e77279a87de4c71b91f0ba25bc56e9 --- /dev/null +++ b/changes/changelog.d/224.enhancement @@ -0,0 +1,30 @@ +Retructured music API to increase performance and remove useless endpoints (#224) + +Music API changes +^^^^^^^^^^^^^^^^^ + +This release includes an API break. Even though the API is advertised +as unstable, and not documented, here is a brief explanation of the change in +case you are using the API in a client or in a script. Summary of the changes: + +- ``/api/v1/artists`` does not includes a list of tracks anymore. It was to heavy + to return all of this data all the time. You can get all tracks for an + artist using ``/api/v1/tracks?artist=artist_id`` +- Additionally, ``/api/v1/tracks`` now support an ``album`` filter to filter + tracks matching an album +- ``/api/v1/artists/search``, ``/api/v1/albums/search`` and ``/api/v1/tracks/search`` + endpoints are removed. Use ``/api/v1/{artists|albums|tracks}/?q=yourquery`` + instead. It's also more powerful, since you can combine search with other + filters and ordering options. +- ``/api/v1/requests/import-requests/search`` endpoint is removed as well. + Use ``/api/v1/requests/import-requests/?q=yourquery`` + instead. It's also more powerful, since you can combine search with other + filters and ordering options. + +Of course, the front-end was updated to work with the new API, so this should +not impact end-users in any way, apart from slight performance gains. + +.. note:: + + The API is still not stable and may evolve again in the future. API freeze + will come at a later point. diff --git a/changes/changelog.d/226.bugfix b/changes/changelog.d/226.bugfix new file mode 100644 index 0000000000000000000000000000000000000000..18d448c23dfe2beb987eb33ebad4d7d9ac32b28f --- /dev/null +++ b/changes/changelog.d/226.bugfix @@ -0,0 +1 @@ +Empty save button in radio builder (#226) diff --git a/dev.yml b/dev.yml index e85ce3b91f59345982e1443a6637ef2fbb726c86..5dccfeca3be8123ea8babfe430c11124cc955c8b 100644 --- a/dev.yml +++ b/dev.yml @@ -130,7 +130,7 @@ services: ports: - '8002:8080' volumes: - - "./api/docs/swagger.yml:/usr/share/nginx/html/swagger.yml" + - "./docs/swagger.yml:/usr/share/nginx/html/swagger.yml" networks: internal: diff --git a/docs/swagger.yml b/docs/swagger.yml index 7735a8f20ab18053e771951846e9f16911b2d408..71c74e442634ac013efd175c26759e4fdb434623 100644 --- a/docs/swagger.yml +++ b/docs/swagger.yml @@ -78,7 +78,7 @@ paths: results: type: "array" items: - $ref: "#/definitions/ArtistNested" + $ref: "#/definitions/ArtistWithAlbums" properties: resultsCount: @@ -106,7 +106,7 @@ definitions: creation_date: type: "string" format: "date-time" - ArtistNested: + ArtistWithAlbums: type: "object" allOf: - $ref: "#/definitions/Artist" @@ -115,7 +115,7 @@ definitions: albums: type: "array" items: - $ref: "#/definitions/AlbumNested" + $ref: "#/definitions/ArtistAlbum" Album: type: "object" @@ -143,16 +143,16 @@ definitions: format: "date" example: "2001-01-01" - AlbumNested: + ArtistAlbum: type: "object" allOf: - $ref: "#/definitions/Album" - type: "object" properties: - tracks: - type: "array" - items: - $ref: "#/definitions/Track" + tracks_count: + type: "integer" + format: "int64" + example: 16 Track: type: "object" diff --git a/front/src/components/audio/PlayButton.vue b/front/src/components/audio/PlayButton.vue index efa59a29d5c65fe30b88aedc5f453b8e9a250a61..6fc7699690f53ab66e998d320655327f51d21cc9 100644 --- a/front/src/components/audio/PlayButton.vue +++ b/front/src/components/audio/PlayButton.vue @@ -21,7 +21,6 @@ <script> import axios from 'axios' -import logger from '@/logging' import jQuery from 'jquery' export default { @@ -30,18 +29,15 @@ export default { tracks: {type: Array, required: false}, track: {type: Object, required: false}, playlist: {type: Object, required: false}, - discrete: {type: Boolean, default: false} + discrete: {type: Boolean, default: false}, + artist: {type: Number, required: false}, + album: {type: Number, required: false} }, data () { return { isLoading: false } }, - created () { - if (!this.playlist && !this.track && !this.tracks) { - logger.default.error('You have to provide either a track playlist or tracks property') - } - }, mounted () { jQuery(this.$el).find('.ui.dropdown').dropdown() }, @@ -62,6 +58,10 @@ export default { return this.tracks.length > 0 } else if (this.playlist) { return true + } else if (this.artist) { + return true + } else if (this.album) { + return true } return false } @@ -81,6 +81,20 @@ export default { return plt.track })) }) + } else if (self.artist) { + let params = { + params: {'artist': self.artist, 'ordering': 'album__release_date,position'} + } + axios.get('tracks', params).then((response) => { + resolve(response.data.results) + }) + } else if (self.album) { + let params = { + params: {'album': self.album, 'ordering': 'position'} + } + axios.get('tracks', params).then((response) => { + resolve(response.data.results) + }) } }) return getTracks.then((tracks) => { diff --git a/front/src/components/audio/artist/Card.vue b/front/src/components/audio/artist/Card.vue index 3ad6fb1c60263ac2d350350b3318eecec72885d0..a46506791e083eb5cc750c8f84918a6bc1fb2318 100644 --- a/front/src/components/audio/artist/Card.vue +++ b/front/src/components/audio/artist/Card.vue @@ -18,10 +18,10 @@ <router-link class="discrete link":to="{name: 'library.albums.detail', params: {id: album.id }}"> <strong>{{ album.title }}</strong> </router-link><br /> - {{ album.tracks.length }} tracks + {{ album.tracks_count }} tracks </td> <td> - <play-button class="right floated basic icon" :discrete="true" :tracks="album.tracks"></play-button> + <play-button class="right floated basic icon" :discrete="true" :album="album.id"></play-button> </td> </tr> </tbody> @@ -45,7 +45,7 @@ {{ artist.albums.length }} </i18next> </span> - <play-button class="mini basic orange right floated" :tracks="allTracks"> + <play-button class="mini basic orange right floated" :artist="artist.id"> <i18next path="Play all"/> </play-button> </div> @@ -74,15 +74,6 @@ export default { return this.artist.albums } return this.artist.albums.slice(0, this.initialAlbums) - }, - allTracks () { - let tracks = [] - this.artist.albums.forEach(album => { - album.tracks.forEach(track => { - tracks.push(track) - }) - }) - return tracks } } } diff --git a/front/src/components/library/Artist.vue b/front/src/components/library/Artist.vue index e16cb6587431d0d7494cae1ef0e92bf5c96fe189..7d0a41d8988055316ecb4773103e129e970f92c7 100644 --- a/front/src/components/library/Artist.vue +++ b/front/src/components/library/Artist.vue @@ -10,7 +10,7 @@ <i class="circular inverted users violet icon"></i> <div class="content"> {{ artist.name }} - <div class="sub header"> + <div class="sub header" v-if="albums"> {{ $t('{% track_count %} tracks in {% album_count %} albums', {track_count: totalTracks, album_count: albums.length})}} </div> </div> @@ -18,7 +18,7 @@ <div class="ui hidden divider"></div> <radio-button type="artist" :object-id="artist.id"></radio-button> </button> - <play-button class="orange" :tracks="allTracks"><i18next path="Play all albums"/></play-button> + <play-button class="orange" :artist="artist.id"><i18next path="Play all albums"/></play-button> <a :href="wikipediaUrl" target="_blank" class="ui button"> <i class="wikipedia icon"></i> @@ -30,10 +30,13 @@ </a> </div> </div> - <div class="ui vertical stripe segment"> + <div v-if="isLoadingAlbums" class="ui vertical stripe segment"> + <div :class="['ui', 'centered', 'active', 'inline', 'loader']"></div> + </div> + <div v-else-if="albums" class="ui vertical stripe segment"> <h2><i18next path="Albums by this artist"/></h2> <div class="ui stackable doubling three column grid"> - <div class="column" :key="album.id" v-for="album in sortedAlbums"> + <div class="column" :key="album.id" v-for="album in albums"> <album-card :mode="'rich'" class="fluid" :album="album"></album-card> </div> </div> @@ -43,7 +46,6 @@ </template> <script> -import _ from 'lodash' import axios from 'axios' import logger from '@/logging' import backend from '@/audio/backend' @@ -63,6 +65,7 @@ export default { data () { return { isLoading: true, + isLoadingAlbums: true, artist: null, albums: null } @@ -78,18 +81,19 @@ export default { logger.default.debug('Fetching artist "' + this.id + '"') axios.get(url).then((response) => { self.artist = response.data - self.albums = JSON.parse(JSON.stringify(self.artist.albums)).map((album) => { - return backend.Album.clean(album) - }) self.isLoading = false + self.isLoadingAlbums = true + axios.get('albums/', {params: {artist: this.id, ordering: '-release_date'}}).then((response) => { + self.albums = JSON.parse(JSON.stringify(response.data.results)).map((album) => { + return backend.Album.clean(album) + }) + + self.isLoadingAlbums = false + }) }) } }, computed: { - sortedAlbums () { - let a = this.albums || [] - return _.orderBy(a, ['release_date'], ['asc']) - }, totalTracks () { return this.albums.map((album) => { return album.tracks.length diff --git a/front/src/components/library/Artists.vue b/front/src/components/library/Artists.vue index 0811f0aa79083da03b5c12b74795ef9ddefb7c8f..aafa3a160e6798729bb0d7652d0c1f2b7304db94 100644 --- a/front/src/components/library/Artists.vue +++ b/front/src/components/library/Artists.vue @@ -69,7 +69,6 @@ import axios from 'axios' import _ from 'lodash' import $ from 'jquery' -import backend from '@/audio/backend' import logger from '@/logging' import OrderingMixin from '@/components/mixins/Ordering' @@ -135,13 +134,6 @@ export default { logger.default.debug('Fetching artists') axios.get(url, {params: params}).then((response) => { self.result = response.data - self.result.results.map((artist) => { - var albums = JSON.parse(JSON.stringify(artist.albums)).map((album) => { - return backend.Album.clean(album) - }) - artist.albums = albums - return artist - }) self.isLoading = false }) }, 500), diff --git a/front/src/components/library/Home.vue b/front/src/components/library/Home.vue index e69ecea805c8fda726d3c02e50e46dc93d881c41..2be693fab2ed7a2a58934b36945f21ec8ca4e41d 100644 --- a/front/src/components/library/Home.vue +++ b/front/src/components/library/Home.vue @@ -30,7 +30,6 @@ <script> import axios from 'axios' import Search from '@/components/audio/Search' -import backend from '@/audio/backend' import logger from '@/logging' import ArtistCard from '@/components/audio/artist/Card' import RadioCard from '@/components/radios/Card' @@ -66,13 +65,6 @@ export default { logger.default.time('Loading latest artists') axios.get(url, {params: params}).then((response) => { self.artists = response.data.results - self.artists.map((artist) => { - var albums = JSON.parse(JSON.stringify(artist.albums)).map((album) => { - return backend.Album.clean(album) - }) - artist.albums = albums - return artist - }) logger.default.timeEnd('Loading latest artists') self.isLoadingArtists = false }) diff --git a/front/src/components/library/radios/Builder.vue b/front/src/components/library/radios/Builder.vue index 85642b0a1235a61b859934df1fd46e5a2943dfc2..6c81f532f2a9478ca7a66ecfbeb255e103ba2c09 100644 --- a/front/src/components/library/radios/Builder.vue +++ b/front/src/components/library/radios/Builder.vue @@ -14,7 +14,7 @@ <input id="public" type="checkbox" v-model="isPublic" /> <i18next tag="label" for="public" path="Display publicly"/> </div> - <button :disabled="!canSave" @click="save" class="ui green button"><i18ext path="Save"/></button> + <button :disabled="!canSave" @click="save" class="ui green button"><i18next path="Save"/></button> <radio-button v-if="id" type="custom" :custom-radio-id="id"></radio-button> </div> </div> diff --git a/front/src/components/library/radios/Filter.vue b/front/src/components/library/radios/Filter.vue index c7f981ede934084606b07d068be5f88a9fdfa9c9..b27c36077c113b801bf842f84ad868cfecc87542 100644 --- a/front/src/components/library/radios/Filter.vue +++ b/front/src/components/library/radios/Filter.vue @@ -123,7 +123,7 @@ export default { if (settings.fields.remoteValues) { return initialResponse } - return {results: initialResponse} + return {results: initialResponse.results} } } }