From cd361287699da5976facb5541233fb31cb36c3af Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 25 Jan 2019 12:11:34 +0100 Subject: [PATCH] Fix #684: Include shared/public playlists in Subsonic API responses --- api/funkwhale_api/subsonic/authentication.py | 4 +-- api/funkwhale_api/subsonic/views.py | 22 ++++++++------ api/tests/subsonic/test_views.py | 31 ++++++++++++++++---- changes/changelog.d/684.enhancement | 1 + 4 files changed, 41 insertions(+), 17 deletions(-) create mode 100644 changes/changelog.d/684.enhancement diff --git a/api/funkwhale_api/subsonic/authentication.py b/api/funkwhale_api/subsonic/authentication.py index d6edb90cd..2d1e04f17 100644 --- a/api/funkwhale_api/subsonic/authentication.py +++ b/api/funkwhale_api/subsonic/authentication.py @@ -18,7 +18,7 @@ def authenticate(username, password): if password.startswith("enc:"): password = password.replace("enc:", "", 1) password = binascii.unhexlify(password).decode("utf-8") - user = User.objects.get( + user = User.objects.select_related("actor").get( username__iexact=username, is_active=True, subsonic_api_token=password ) except (User.DoesNotExist, binascii.Error): @@ -29,7 +29,7 @@ def authenticate(username, password): def authenticate_salt(username, salt, token): try: - user = User.objects.get( + user = User.objects.select_related("actor").get( username=username, is_active=True, subsonic_api_token__isnull=False ) except User.DoesNotExist: diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 31110dbe6..f7926d1fd 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -11,7 +11,7 @@ from rest_framework.serializers import ValidationError import funkwhale_api from funkwhale_api.activity import record -from funkwhale_api.common import preferences, utils as common_utils +from funkwhale_api.common import fields, preferences, utils as common_utils from funkwhale_api.favorites.models import TrackFavorite from funkwhale_api.music import models as music_models from funkwhale_api.music import utils @@ -68,9 +68,7 @@ def find_object( { "error": { "code": 70, - "message": "{} not found".format( - qs.model.__class__.__name__ - ), + "message": "{} not found".format(qs.model.__name__), } } ) @@ -82,6 +80,14 @@ def find_object( return decorator +def get_playlist_qs(request): + qs = playlists_models.Playlist.objects.filter( + fields.privacy_level_query(request.user) + ) + qs = qs.with_tracks_count().exclude(_tracks_count=0).select_related("user") + return qs.order_by("-creation_date") + + class SubsonicViewSet(viewsets.GenericViewSet): content_negotiation_class = negotiation.SubsonicContentNegociation authentication_classes = [authentication.SubsonicAuthentication] @@ -398,11 +404,9 @@ class SubsonicViewSet(viewsets.GenericViewSet): url_path="getPlaylists", ) def get_playlists(self, request, *args, **kwargs): - playlists = request.user.playlists.with_tracks_count().select_related("user") + qs = get_playlist_qs(request) data = { - "playlists": { - "playlist": [serializers.get_playlist_data(p) for p in playlists] - } + "playlists": {"playlist": [serializers.get_playlist_data(p) for p in qs]} } return response.Response(data) @@ -412,7 +416,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): url_name="get_playlist", url_path="getPlaylist", ) - @find_object(playlists_models.Playlist.objects.with_tracks_count()) + @find_object(lambda request: get_playlist_qs(request)) def get_playlist(self, request, *args, **kwargs): playlist = kwargs.pop("obj") data = {"playlist": serializers.get_playlist_detail_data(playlist)} diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index d18552461..0dbbaf39a 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -387,21 +387,40 @@ def test_search3(f, db, logged_in_api_client, factories): def test_get_playlists(f, db, logged_in_api_client, factories): url = reverse("api:subsonic-get_playlists") assert url.endswith("getPlaylists") is True - playlist = factories["playlists.Playlist"](user=logged_in_api_client.user) + playlist1 = factories["playlists.PlaylistTrack"]( + playlist__user=logged_in_api_client.user + ).playlist + playlist2 = factories["playlists.PlaylistTrack"]( + playlist__privacy_level="everyone" + ).playlist + playlist3 = factories["playlists.PlaylistTrack"]( + playlist__privacy_level="instance" + ).playlist + # private + factories["playlists.PlaylistTrack"](playlist__privacy_level="me") + # no track + factories["playlists.Playlist"](privacy_level="everyone") response = logged_in_api_client.get(url, {"f": f}) - qs = playlist.__class__.objects.with_tracks_count() - assert response.status_code == 200 - assert response.data == { - "playlists": {"playlist": [serializers.get_playlist_data(qs.first())]} + qs = ( + playlist1.__class__.objects.with_tracks_count() + .filter(pk__in=[playlist1.pk, playlist2.pk, playlist3.pk]) + .order_by("-creation_date") + ) + expected = { + "playlists": {"playlist": [serializers.get_playlist_data(p) for p in qs]} } + assert response.status_code == 200 + assert response.data == expected @pytest.mark.parametrize("f", ["json"]) def test_get_playlist(f, db, logged_in_api_client, factories): url = reverse("api:subsonic-get_playlist") assert url.endswith("getPlaylist") is True - playlist = factories["playlists.Playlist"](user=logged_in_api_client.user) + playlist = factories["playlists.PlaylistTrack"]( + playlist__user=logged_in_api_client.user + ).playlist response = logged_in_api_client.get(url, {"f": f, "id": playlist.pk}) qs = playlist.__class__.objects.with_tracks_count() diff --git a/changes/changelog.d/684.enhancement b/changes/changelog.d/684.enhancement new file mode 100644 index 000000000..1cdd0cdd9 --- /dev/null +++ b/changes/changelog.d/684.enhancement @@ -0,0 +1 @@ +Include shared/public playlists in Subsonic API responses (#684)