diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 000289e51..077566d1c 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -57,7 +57,6 @@ THIRD_PARTY_APPS = ( 'taggit', 'rest_auth', 'rest_auth.registration', - 'mptt', 'dynamic_preferences', 'django_filters', 'cacheops', @@ -383,3 +382,6 @@ CACHEOPS = { # Custom Admin URL, use {% url 'admin:index' %} ADMIN_URL = env('DJANGO_ADMIN_URL', default='^api/admin/') CSRF_USE_SESSIONS = True + +# Playlist settings +PLAYLISTS_MAX_TRACKS = env.int('PLAYLISTS_MAX_TRACKS', default=250) diff --git a/api/funkwhale_api/common/fields.py b/api/funkwhale_api/common/fields.py new file mode 100644 index 000000000..ef9f840dc --- /dev/null +++ b/api/funkwhale_api/common/fields.py @@ -0,0 +1,27 @@ +from django.db import models + + +PRIVACY_LEVEL_CHOICES = [ + ('me', 'Only me'), + ('followers', 'Me and my followers'), + ('instance', 'Everyone on my instance, and my followers'), + ('everyone', 'Everyone, including people on other instances'), +] + + +def get_privacy_field(): + return models.CharField( + max_length=30, choices=PRIVACY_LEVEL_CHOICES, default='instance') + + +def privacy_level_query(user, lookup_field='privacy_level'): + if user.is_anonymous: + return models.Q(**{ + lookup_field: 'everyone', + }) + + return models.Q(**{ + '{}__in'.format(lookup_field): [ + 'me', 'followers', 'instance', 'everyone' + ] + }) diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py index ecfea4c9a..c99c275c1 100644 --- a/api/funkwhale_api/common/permissions.py +++ b/api/funkwhale_api/common/permissions.py @@ -1,4 +1,7 @@ +import operator + from django.conf import settings +from django.http import Http404 from rest_framework.permissions import BasePermission, DjangoModelPermissions @@ -20,3 +23,39 @@ class HasModelPermission(DjangoModelPermissions): """ def get_required_permissions(self, method, model_cls): return super().get_required_permissions(method, self.model) + + +class OwnerPermission(BasePermission): + """ + Ensure the request user is the owner of the object. + + Usage: + + class MyView(APIView): + model = MyModel + permission_classes = [OwnerPermission] + owner_field = 'owner' + owner_checks = ['read', 'write'] + """ + perms_map = { + 'GET': 'read', + 'OPTIONS': 'read', + 'HEAD': 'read', + 'POST': 'write', + 'PUT': 'write', + 'PATCH': 'write', + 'DELETE': 'write', + } + + def has_object_permission(self, request, view, obj): + method_check = self.perms_map[request.method] + owner_checks = getattr(view, 'owner_checks', ['read', 'write']) + if method_check not in owner_checks: + # check not enabled + return True + + owner_field = getattr(view, 'owner_field', 'user') + owner = operator.attrgetter(owner_field)(obj) + if owner != request.user: + raise Http404 + return True diff --git a/api/funkwhale_api/playlists/admin.py b/api/funkwhale_api/playlists/admin.py index b337154c9..68e447f38 100644 --- a/api/funkwhale_api/playlists/admin.py +++ b/api/funkwhale_api/playlists/admin.py @@ -5,13 +5,13 @@ from . import models @admin.register(models.Playlist) class PlaylistAdmin(admin.ModelAdmin): - list_display = ['name', 'user', 'is_public', 'creation_date'] + list_display = ['name', 'user', 'privacy_level', 'creation_date'] search_fields = ['name', ] list_select_related = True @admin.register(models.PlaylistTrack) class PlaylistTrackAdmin(admin.ModelAdmin): - list_display = ['playlist', 'track', 'position', ] + list_display = ['playlist', 'track', 'index'] search_fields = ['track__name', 'playlist__name'] list_select_related = True diff --git a/api/funkwhale_api/playlists/factories.py b/api/funkwhale_api/playlists/factories.py index 19e4770cf..cddea6002 100644 --- a/api/funkwhale_api/playlists/factories.py +++ b/api/funkwhale_api/playlists/factories.py @@ -1,6 +1,7 @@ import factory from funkwhale_api.factories import registry +from funkwhale_api.music.factories import TrackFactory from funkwhale_api.users.factories import UserFactory @@ -11,3 +12,12 @@ class PlaylistFactory(factory.django.DjangoModelFactory): class Meta: model = 'playlists.Playlist' + + +@registry.register +class PlaylistTrackFactory(factory.django.DjangoModelFactory): + playlist = factory.SubFactory(PlaylistFactory) + track = factory.SubFactory(TrackFactory) + + class Meta: + model = 'playlists.PlaylistTrack' diff --git a/api/funkwhale_api/playlists/filters.py b/api/funkwhale_api/playlists/filters.py new file mode 100644 index 000000000..bc4941510 --- /dev/null +++ b/api/funkwhale_api/playlists/filters.py @@ -0,0 +1,22 @@ +from django_filters import rest_framework as filters + +from funkwhale_api.music import utils + +from . import models + + + +class PlaylistFilter(filters.FilterSet): + q = filters.CharFilter(name='_', method='filter_q') + + class Meta: + model = models.Playlist + fields = { + 'user': ['exact'], + 'name': ['exact', 'icontains'], + 'q': 'exact', + } + + def filter_q(self, queryset, name, value): + query = utils.get_query(value, ['name', 'user__username']) + return queryset.filter(query) diff --git a/api/funkwhale_api/playlists/migrations/0001_initial.py b/api/funkwhale_api/playlists/migrations/0001_initial.py index bc97d8122..987b2f9cf 100644 --- a/api/funkwhale_api/playlists/migrations/0001_initial.py +++ b/api/funkwhale_api/playlists/migrations/0001_initial.py @@ -4,7 +4,6 @@ from __future__ import unicode_literals from django.db import migrations, models from django.conf import settings import django.utils.timezone -import mptt.fields class Migration(migrations.Migration): @@ -34,7 +33,7 @@ class Migration(migrations.Migration): ('tree_id', models.PositiveIntegerField(db_index=True, editable=False)), ('position', models.PositiveIntegerField(db_index=True, editable=False)), ('playlist', models.ForeignKey(to='playlists.Playlist', related_name='playlist_tracks', on_delete=models.CASCADE)), - ('previous', mptt.fields.TreeOneToOneField(null=True, to='playlists.PlaylistTrack', related_name='next', blank=True, on_delete=models.CASCADE)), + ('previous', models.OneToOneField(null=True, to='playlists.PlaylistTrack', related_name='next', blank=True, on_delete=models.CASCADE)), ('track', models.ForeignKey(to='music.Track', related_name='playlist_tracks', on_delete=models.CASCADE)), ], options={ diff --git a/api/funkwhale_api/playlists/migrations/0002_auto_20180316_2217.py b/api/funkwhale_api/playlists/migrations/0002_auto_20180316_2217.py new file mode 100644 index 000000000..23d0a8eab --- /dev/null +++ b/api/funkwhale_api/playlists/migrations/0002_auto_20180316_2217.py @@ -0,0 +1,22 @@ +# Generated by Django 2.0.3 on 2018-03-16 22:17 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('playlists', '0001_initial'), + ] + + operations = [ + migrations.RemoveField( + model_name='playlist', + name='is_public', + ), + migrations.AddField( + model_name='playlist', + name='privacy_level', + field=models.CharField(choices=[('me', 'Only me'), ('followers', 'Me and my followers'), ('instance', 'Everyone on my instance, and my followers'), ('everyone', 'Everyone, including people on other instances')], default='instance', max_length=30), + ), + ] diff --git a/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py b/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py new file mode 100644 index 000000000..0284f8f2c --- /dev/null +++ b/api/funkwhale_api/playlists/migrations/0003_auto_20180319_1214.py @@ -0,0 +1,52 @@ +# Generated by Django 2.0.3 on 2018-03-19 12:14 + +from django.db import migrations, models +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [ + ('playlists', '0002_auto_20180316_2217'), + ] + + operations = [ + migrations.AlterModelOptions( + name='playlisttrack', + options={'ordering': ('-playlist', 'index')}, + ), + migrations.AddField( + model_name='playlisttrack', + name='creation_date', + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name='playlisttrack', + name='index', + field=models.PositiveIntegerField(null=True), + ), + migrations.RemoveField( + model_name='playlisttrack', + name='lft', + ), + migrations.RemoveField( + model_name='playlisttrack', + name='position', + ), + migrations.RemoveField( + model_name='playlisttrack', + name='previous', + ), + migrations.RemoveField( + model_name='playlisttrack', + name='rght', + ), + migrations.RemoveField( + model_name='playlisttrack', + name='tree_id', + ), + migrations.AlterUniqueTogether( + name='playlisttrack', + unique_together={('playlist', 'index')}, + ), + ] diff --git a/api/funkwhale_api/playlists/migrations/0004_auto_20180320_1713.py b/api/funkwhale_api/playlists/migrations/0004_auto_20180320_1713.py new file mode 100644 index 000000000..415b53612 --- /dev/null +++ b/api/funkwhale_api/playlists/migrations/0004_auto_20180320_1713.py @@ -0,0 +1,27 @@ +# Generated by Django 2.0.3 on 2018-03-20 17:13 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('playlists', '0003_auto_20180319_1214'), + ] + + operations = [ + migrations.AddField( + model_name='playlist', + name='modification_date', + field=models.DateTimeField(auto_now=True), + ), + migrations.AlterField( + model_name='playlisttrack', + name='index', + field=models.PositiveIntegerField(blank=True, null=True), + ), + migrations.AlterUniqueTogether( + name='playlisttrack', + unique_together=set(), + ), + ] diff --git a/api/funkwhale_api/playlists/models.py b/api/funkwhale_api/playlists/models.py index e89dce81c..6bb8fe178 100644 --- a/api/funkwhale_api/playlists/models.py +++ b/api/funkwhale_api/playlists/models.py @@ -1,43 +1,130 @@ +from django.conf import settings from django.db import models +from django.db import transaction from django.utils import timezone -from mptt.models import MPTTModel, TreeOneToOneField +from rest_framework import exceptions + +from funkwhale_api.common import fields class Playlist(models.Model): name = models.CharField(max_length=50) - is_public = models.BooleanField(default=False) user = models.ForeignKey( 'users.User', related_name="playlists", on_delete=models.CASCADE) creation_date = models.DateTimeField(default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) + privacy_level = fields.get_privacy_field() def __str__(self): return self.name - def add_track(self, track, previous=None): - plt = PlaylistTrack(previous=previous, track=track, playlist=self) - plt.save() + @transaction.atomic + def insert(self, plt, index=None): + """ + Given a PlaylistTrack, insert it at the correct index in the playlist, + and update other tracks index if necessary. + """ + old_index = plt.index + move = old_index is not None + if index is not None and index == old_index: + # moving at same position, just skip + return index - return plt + existing = self.playlist_tracks.select_for_update() + if move: + existing = existing.exclude(pk=plt.pk) + total = existing.filter(index__isnull=False).count() + + if index is None: + # we simply increment the last track index by 1 + index = total + + if index > total: + raise exceptions.ValidationError('Index is not continuous') + + if index < 0: + raise exceptions.ValidationError('Index must be zero or positive') + + if move: + # we remove the index temporarily, to avoid integrity errors + plt.index = None + plt.save(update_fields=['index']) + if index > old_index: + # new index is higher than current, we decrement previous tracks + to_update = existing.filter( + index__gt=old_index, index__lte=index) + to_update.update(index=models.F('index') - 1) + if index < old_index: + # new index is lower than current, we increment next tracks + to_update = existing.filter(index__lt=old_index, index__gte=index) + to_update.update(index=models.F('index') + 1) + else: + to_update = existing.filter(index__gte=index) + to_update.update(index=models.F('index') + 1) + + plt.index = index + plt.save(update_fields=['index']) + self.save(update_fields=['modification_date']) + return index + + @transaction.atomic + def remove(self, index): + existing = self.playlist_tracks.select_for_update() + self.save(update_fields=['modification_date']) + to_update = existing.filter(index__gt=index) + return to_update.update(index=models.F('index') - 1) + + @transaction.atomic + def insert_many(self, tracks): + existing = self.playlist_tracks.select_for_update() + now = timezone.now() + total = existing.filter(index__isnull=False).count() + if existing.count() + len(tracks) > settings.PLAYLISTS_MAX_TRACKS: + raise exceptions.ValidationError( + 'Playlist would reach the maximum of {} tracks'.format( + settings.PLAYLISTS_MAX_TRACKS)) + self.save(update_fields=['modification_date']) + start = total + plts = [ + PlaylistTrack( + creation_date=now, playlist=self, track=track, index=start+i) + for i, track in enumerate(tracks) + ] + return PlaylistTrack.objects.bulk_create(plts) + +class PlaylistTrackQuerySet(models.QuerySet): + def for_nested_serialization(self): + return (self.select_related() + .select_related('track__album__artist') + .prefetch_related( + 'track__tags', + 'track__files', + 'track__artist__albums__tracks__tags')) -class PlaylistTrack(MPTTModel): +class PlaylistTrack(models.Model): track = models.ForeignKey( 'music.Track', related_name='playlist_tracks', on_delete=models.CASCADE) - previous = TreeOneToOneField( - 'self', - blank=True, - null=True, - related_name='next', - on_delete=models.CASCADE) + index = models.PositiveIntegerField(null=True, blank=True) playlist = models.ForeignKey( Playlist, related_name='playlist_tracks', on_delete=models.CASCADE) + creation_date = models.DateTimeField(default=timezone.now) - class MPTTMeta: - level_attr = 'position' - parent_attr = 'previous' + objects = PlaylistTrackQuerySet.as_manager() class Meta: - ordering = ('-playlist', 'position') + ordering = ('-playlist', 'index') + unique_together = ('playlist', 'index') + + def delete(self, *args, **kwargs): + playlist = self.playlist + index = self.index + update_indexes = kwargs.pop('update_indexes', False) + r = super().delete(*args, **kwargs) + if index is not None and update_indexes: + playlist.remove(index) + return r diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index 7f889d53e..6caf9aa4a 100644 --- a/api/funkwhale_api/playlists/serializers.py +++ b/api/funkwhale_api/playlists/serializers.py @@ -1,8 +1,11 @@ +from django.conf import settings +from django.db import transaction from rest_framework import serializers from taggit.models import Tag +from funkwhale_api.music.models import Track from funkwhale_api.music.serializers import TrackSerializerNested - +from funkwhale_api.users.serializers import UserBasicSerializer from . import models @@ -11,20 +14,81 @@ class PlaylistTrackSerializer(serializers.ModelSerializer): class Meta: model = models.PlaylistTrack - fields = ('id', 'track', 'playlist', 'position') + fields = ('id', 'track', 'playlist', 'index', 'creation_date') -class PlaylistTrackCreateSerializer(serializers.ModelSerializer): +class PlaylistTrackWriteSerializer(serializers.ModelSerializer): + index = serializers.IntegerField( + required=False, min_value=0, allow_null=True) class Meta: model = models.PlaylistTrack - fields = ('id', 'track', 'playlist', 'position') + fields = ('id', 'track', 'playlist', 'index') + + def validate_playlist(self, value): + if self.context.get('request'): + # validate proper ownership on the playlist + if self.context['request'].user != value.user: + raise serializers.ValidationError( + 'You do not have the permission to edit this playlist') + existing = value.playlist_tracks.count() + if existing >= settings.PLAYLISTS_MAX_TRACKS: + raise serializers.ValidationError( + 'Playlist has reached the maximum of {} tracks'.format( + settings.PLAYLISTS_MAX_TRACKS)) + return value + + @transaction.atomic + def create(self, validated_data): + index = validated_data.pop('index', None) + instance = super().create(validated_data) + instance.playlist.insert(instance, index) + return instance + + @transaction.atomic + def update(self, instance, validated_data): + update_index = 'index' in validated_data + index = validated_data.pop('index', None) + super().update(instance, validated_data) + if update_index: + instance.playlist.insert(instance, index) + return instance + + def get_unique_together_validators(self): + """ + We explicitely disable unique together validation here + because it collides with our internal logic + """ + return [] class PlaylistSerializer(serializers.ModelSerializer): - playlist_tracks = PlaylistTrackSerializer(many=True, read_only=True) + tracks_count = serializers.SerializerMethodField(read_only=True) + user = UserBasicSerializer(read_only=True) class Meta: model = models.Playlist - fields = ('id', 'name', 'is_public', 'creation_date', 'playlist_tracks') - read_only_fields = ['id', 'playlist_tracks', 'creation_date'] + fields = ( + 'id', + 'name', + 'tracks_count', + 'user', + 'modification_date', + 'creation_date', + 'privacy_level',) + read_only_fields = [ + 'id', + 'modification_date', + 'creation_date',] + + def get_tracks_count(self, obj): + try: + return obj.tracks_count + except AttributeError: + # no annotation? + return obj.playlist_tracks.count() + + +class PlaylistAddManySerializer(serializers.Serializer): + tracks = serializers.PrimaryKeyRelatedField( + many=True, queryset=Track.objects.for_nested_serialization()) diff --git a/api/funkwhale_api/playlists/views.py b/api/funkwhale_api/playlists/views.py index 1a88d231e..683f90388 100644 --- a/api/funkwhale_api/playlists/views.py +++ b/api/funkwhale_api/playlists/views.py @@ -1,58 +1,123 @@ +from django.db.models import Count +from django.db import transaction + +from rest_framework import exceptions from rest_framework import generics, mixins, viewsets from rest_framework import status +from rest_framework.decorators import detail_route from rest_framework.response import Response +from rest_framework.permissions import IsAuthenticatedOrReadOnly +from funkwhale_api.common import permissions +from funkwhale_api.common import fields from funkwhale_api.music.models import Track -from funkwhale_api.common.permissions import ConditionalAuthentication +from . import filters from . import models from . import serializers - class PlaylistViewSet( mixins.RetrieveModelMixin, mixins.CreateModelMixin, + mixins.UpdateModelMixin, mixins.DestroyModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet): serializer_class = serializers.PlaylistSerializer - queryset = (models.Playlist.objects.all()) - permission_classes = [ConditionalAuthentication] + queryset = ( + models.Playlist.objects.all().select_related('user') + .annotate(tracks_count=Count('playlist_tracks')) + ) + permission_classes = [ + permissions.ConditionalAuthentication, + permissions.OwnerPermission, + IsAuthenticatedOrReadOnly, + ] + owner_checks = ['write'] + filter_class = filters.PlaylistFilter + ordering_fields = ('id', 'name', 'creation_date', 'modification_date') - def create(self, request, *args, **kwargs): - serializer = self.get_serializer(data=request.data) + @detail_route(methods=['get']) + def tracks(self, request, *args, **kwargs): + playlist = self.get_object() + plts = playlist.playlist_tracks.all().for_nested_serialization() + serializer = serializers.PlaylistTrackSerializer(plts, many=True) + data = { + 'count': len(plts), + 'results': serializer.data + } + return Response(data, status=200) + + @detail_route(methods=['post']) + @transaction.atomic + def add(self, request, *args, **kwargs): + playlist = self.get_object() + serializer = serializers.PlaylistAddManySerializer(data=request.data) serializer.is_valid(raise_exception=True) - instance = self.perform_create(serializer) - serializer = self.get_serializer(instance=instance) - headers = self.get_success_headers(serializer.data) - return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + try: + plts = playlist.insert_many(serializer.validated_data['tracks']) + except exceptions.ValidationError as e: + payload = {'playlist': e.detail} + return Response(payload, status=400) + ids = [p.id for p in plts] + plts = models.PlaylistTrack.objects.filter( + pk__in=ids).order_by('index').for_nested_serialization() + serializer = serializers.PlaylistTrackSerializer(plts, many=True) + data = { + 'count': len(plts), + 'results': serializer.data + } + return Response(data, status=201) + + @detail_route(methods=['delete']) + @transaction.atomic + def clear(self, request, *args, **kwargs): + playlist = self.get_object() + playlist.playlist_tracks.all().delete() + playlist.save(update_fields=['modification_date']) + return Response(status=204) def get_queryset(self): - return self.queryset.filter(user=self.request.user) + return self.queryset.filter( + fields.privacy_level_query(self.request.user)) def perform_create(self, serializer): - return serializer.save(user=self.request.user) + return serializer.save( + user=self.request.user, + privacy_level=serializer.validated_data.get( + 'privacy_level', self.request.user.privacy_level) + ) class PlaylistTrackViewSet( + mixins.RetrieveModelMixin, mixins.CreateModelMixin, + mixins.UpdateModelMixin, mixins.DestroyModelMixin, mixins.ListModelMixin, viewsets.GenericViewSet): serializer_class = serializers.PlaylistTrackSerializer - queryset = (models.PlaylistTrack.objects.all()) - permission_classes = [ConditionalAuthentication] + queryset = (models.PlaylistTrack.objects.all().for_nested_serialization()) + permission_classes = [ + permissions.ConditionalAuthentication, + permissions.OwnerPermission, + IsAuthenticatedOrReadOnly, + ] + owner_field = 'playlist.user' + owner_checks = ['write'] - def create(self, request, *args, **kwargs): - serializer = serializers.PlaylistTrackCreateSerializer( - data=request.data) - serializer.is_valid(raise_exception=True) - instance = self.perform_create(serializer) - serializer = self.get_serializer(instance=instance) - headers = self.get_success_headers(serializer.data) - return Response(serializer.data, status=status.HTTP_201_CREATED, headers=headers) + def get_serializer_class(self): + if self.request.method in ['PUT', 'PATCH', 'DELETE', 'POST']: + return serializers.PlaylistTrackWriteSerializer + return self.serializer_class def get_queryset(self): - return self.queryset.filter(playlist__user=self.request.user) + return self.queryset.filter( + fields.privacy_level_query( + self.request.user, + lookup_field='playlist__privacy_level')) + + def perform_destroy(self, instance): + instance.delete(update_indexes=True) diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index a5478656b..9516c108f 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -10,15 +10,9 @@ from django.db import models from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ +from funkwhale_api.common import fields -PRIVACY_LEVEL_CHOICES = [ - ('me', 'Only me'), - ('followers', 'Me and my followers'), - ('instance', 'Everyone on my instance, and my followers'), - ('everyone', 'Everyone, including people on other instances'), -] - @python_2_unicode_compatible class User(AbstractUser): @@ -39,8 +33,8 @@ class User(AbstractUser): }, } - privacy_level = models.CharField( - max_length=30, choices=PRIVACY_LEVEL_CHOICES, default='instance') + privacy_level = fields.get_privacy_field() + def __str__(self): return self.username diff --git a/api/requirements/base.txt b/api/requirements/base.txt index bfa1a47db..efcc4eea4 100644 --- a/api/requirements/base.txt +++ b/api/requirements/base.txt @@ -33,7 +33,6 @@ musicbrainzngs==0.6 youtube_dl>=2017.12.14 djangorestframework>=3.7,<3.8 djangorestframework-jwt>=1.11,<1.12 -django-mptt>=0.9,<0.10 google-api-python-client>=1.6,<1.7 arrow>=0.12,<0.13 persisting-theory>=0.2,<0.3 diff --git a/api/setup.cfg b/api/setup.cfg index 954b4d196..34daa8c68 100644 --- a/api/setup.cfg +++ b/api/setup.cfg @@ -6,7 +6,7 @@ exclude = .tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules max-line-length = 120 exclude=.tox,.git,*/migrations/*,*/static/CACHE/*,docs,node_modules -[pytest] +[tool:pytest] DJANGO_SETTINGS_MODULE=config.settings.test python_files = tests.py test_*.py *_tests.py testpaths = tests diff --git a/api/tests/common/test_fields.py b/api/tests/common/test_fields.py new file mode 100644 index 000000000..7c63431a3 --- /dev/null +++ b/api/tests/common/test_fields.py @@ -0,0 +1,17 @@ +import pytest + +from django.contrib.auth.models import AnonymousUser +from django.db.models import Q + +from funkwhale_api.common import fields +from funkwhale_api.users.factories import UserFactory + + +@pytest.mark.parametrize('user,expected', [ + (AnonymousUser(), Q(privacy_level='everyone')), + (UserFactory.build(pk=1), + Q(privacy_level__in=['me', 'followers', 'instance', 'everyone'])), +]) +def test_privacy_level_query(user,expected): + query = fields.privacy_level_query(user) + assert query == expected diff --git a/api/tests/common/test_permissions.py b/api/tests/common/test_permissions.py new file mode 100644 index 000000000..b5c5160f8 --- /dev/null +++ b/api/tests/common/test_permissions.py @@ -0,0 +1,42 @@ +import pytest + +from rest_framework.views import APIView + +from django.contrib.auth.models import AnonymousUser +from django.http import Http404 + +from funkwhale_api.common import permissions + + +def test_owner_permission_owner_field_ok(nodb_factories, api_request): + playlist = nodb_factories['playlists.Playlist']() + view = APIView.as_view() + permission = permissions.OwnerPermission() + request = api_request.get('/') + setattr(request, 'user', playlist.user) + check = permission.has_object_permission(request, view, playlist) + + assert check is True + + +def test_owner_permission_owner_field_not_ok(nodb_factories, api_request): + playlist = nodb_factories['playlists.Playlist']() + view = APIView.as_view() + permission = permissions.OwnerPermission() + request = api_request.get('/') + setattr(request, 'user', AnonymousUser()) + + with pytest.raises(Http404): + permission.has_object_permission(request, view, playlist) + + +def test_owner_permission_read_only(nodb_factories, api_request): + playlist = nodb_factories['playlists.Playlist']() + view = APIView.as_view() + setattr(view, 'owner_checks', ['write']) + permission = permissions.OwnerPermission() + request = api_request.get('/') + setattr(request, 'user', AnonymousUser()) + check = permission.has_object_permission(request, view, playlist) + + assert check is True diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 4ff1a8ee7..62bc5ada6 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,8 +1,11 @@ +import factory import tempfile import shutil import pytest + from django.core.cache import cache as django_cache from dynamic_preferences.registries import global_preferences_registry + from rest_framework.test import APIClient from rest_framework.test import APIRequestFactory @@ -27,6 +30,16 @@ def cache(): @pytest.fixture def factories(db): from funkwhale_api import factories + for v in factories.registry.values(): + v._meta.strategy = factory.CREATE_STRATEGY + yield factories.registry + + +@pytest.fixture +def nodb_factories(): + from funkwhale_api import factories + for v in factories.registry.values(): + v._meta.strategy = factory.BUILD_STRATEGY yield factories.registry diff --git a/api/tests/playlists/test_models.py b/api/tests/playlists/test_models.py new file mode 100644 index 000000000..c9def4dab --- /dev/null +++ b/api/tests/playlists/test_models.py @@ -0,0 +1,126 @@ +import pytest + +from rest_framework import exceptions + + +def test_can_insert_plt(factories): + plt = factories['playlists.PlaylistTrack']() + modification_date = plt.playlist.modification_date + + assert plt.index is None + + plt.playlist.insert(plt) + plt.refresh_from_db() + + assert plt.index == 0 + assert plt.playlist.modification_date > modification_date + + +def test_insert_use_last_idx_by_default(factories): + playlist = factories['playlists.Playlist']() + plts = factories['playlists.PlaylistTrack'].create_batch( + size=3, playlist=playlist) + + for i, plt in enumerate(plts): + index = playlist.insert(plt) + plt.refresh_from_db() + + assert index == i + assert plt.index == i + +def test_can_insert_at_index(factories): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist) + playlist.insert(first) + new_first = factories['playlists.PlaylistTrack'](playlist=playlist) + index = playlist.insert(new_first, index=0) + first.refresh_from_db() + new_first.refresh_from_db() + + assert index == 0 + assert first.index == 1 + assert new_first.index == 0 + + +def test_can_insert_and_move(factories): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + second = factories['playlists.PlaylistTrack'](playlist=playlist, index=1) + third = factories['playlists.PlaylistTrack'](playlist=playlist, index=2) + + playlist.insert(second, index=0) + + first.refresh_from_db() + second.refresh_from_db() + third.refresh_from_db() + + assert third.index == 2 + assert second.index == 0 + assert first.index == 1 + + +def test_can_insert_and_move_last_to_0(factories): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + second = factories['playlists.PlaylistTrack'](playlist=playlist, index=1) + third = factories['playlists.PlaylistTrack'](playlist=playlist, index=2) + + playlist.insert(third, index=0) + + first.refresh_from_db() + second.refresh_from_db() + third.refresh_from_db() + + assert third.index == 0 + assert first.index == 1 + assert second.index == 2 + + +def test_cannot_insert_at_wrong_index(factories): + plt = factories['playlists.PlaylistTrack']() + new = factories['playlists.PlaylistTrack'](playlist=plt.playlist) + with pytest.raises(exceptions.ValidationError): + plt.playlist.insert(new, 2) + + +def test_cannot_insert_at_negative_index(factories): + plt = factories['playlists.PlaylistTrack']() + new = factories['playlists.PlaylistTrack'](playlist=plt.playlist) + with pytest.raises(exceptions.ValidationError): + plt.playlist.insert(new, -1) + + +def test_remove_update_indexes(factories): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + second = factories['playlists.PlaylistTrack'](playlist=playlist, index=1) + third = factories['playlists.PlaylistTrack'](playlist=playlist, index=2) + + second.delete(update_indexes=True) + + first.refresh_from_db() + third.refresh_from_db() + + assert first.index == 0 + assert third.index == 1 + + +def test_can_insert_many(factories): + playlist = factories['playlists.Playlist']() + existing = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + tracks = factories['music.Track'].create_batch(size=3) + plts = playlist.insert_many(tracks) + for i, plt in enumerate(plts): + assert plt.index == i + 1 + assert plt.track == tracks[i] + assert plt.playlist == playlist + + +def test_insert_many_honor_max_tracks(factories, settings): + settings.PLAYLISTS_MAX_TRACKS = 4 + playlist = factories['playlists.Playlist']() + plts = factories['playlists.PlaylistTrack'].create_batch( + size=2, playlist=playlist) + track = factories['music.Track']() + with pytest.raises(exceptions.ValidationError): + playlist.insert_many([track, track, track]) diff --git a/api/tests/playlists/test_serializers.py b/api/tests/playlists/test_serializers.py new file mode 100644 index 000000000..8e30919e6 --- /dev/null +++ b/api/tests/playlists/test_serializers.py @@ -0,0 +1,74 @@ +from funkwhale_api.playlists import models +from funkwhale_api.playlists import serializers + + +def test_cannot_max_500_tracks_per_playlist(factories, settings): + settings.PLAYLISTS_MAX_TRACKS = 2 + playlist = factories['playlists.Playlist']() + plts = factories['playlists.PlaylistTrack'].create_batch( + size=2, playlist=playlist) + track = factories['music.Track']() + serializer = serializers.PlaylistTrackWriteSerializer(data={ + 'playlist': playlist.pk, + 'track': track.pk, + }) + + assert serializer.is_valid() is False + assert 'playlist' in serializer.errors + + +def test_create_insert_is_called_when_index_is_None(factories, mocker): + insert = mocker.spy(models.Playlist, 'insert') + playlist = factories['playlists.Playlist']() + track = factories['music.Track']() + serializer = serializers.PlaylistTrackWriteSerializer(data={ + 'playlist': playlist.pk, + 'track': track.pk, + 'index': None, + }) + assert serializer.is_valid() is True + + plt = serializer.save() + insert.assert_called_once_with(playlist, plt, None) + assert plt.index == 0 + + +def test_create_insert_is_called_when_index_is_provided(factories, mocker): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + insert = mocker.spy(models.Playlist, 'insert') + factories['playlists.Playlist']() + track = factories['music.Track']() + serializer = serializers.PlaylistTrackWriteSerializer(data={ + 'playlist': playlist.pk, + 'track': track.pk, + 'index': 0, + }) + assert serializer.is_valid() is True + + plt = serializer.save() + first.refresh_from_db() + insert.assert_called_once_with(playlist, plt, 0) + assert plt.index == 0 + assert first.index == 1 + + +def test_update_insert_is_called_when_index_is_provided(factories, mocker): + playlist = factories['playlists.Playlist']() + first = factories['playlists.PlaylistTrack'](playlist=playlist, index=0) + second = factories['playlists.PlaylistTrack'](playlist=playlist, index=1) + insert = mocker.spy(models.Playlist, 'insert') + factories['playlists.Playlist']() + track = factories['music.Track']() + serializer = serializers.PlaylistTrackWriteSerializer(second, data={ + 'playlist': playlist.pk, + 'track': second.track.pk, + 'index': 0, + }) + assert serializer.is_valid() is True + + plt = serializer.save() + first.refresh_from_db() + insert.assert_called_once_with(playlist, plt, 0) + assert plt.index == 0 + assert first.index == 1 diff --git a/api/tests/playlists/test_views.py b/api/tests/playlists/test_views.py new file mode 100644 index 000000000..5bf834888 --- /dev/null +++ b/api/tests/playlists/test_views.py @@ -0,0 +1,197 @@ +import json +import pytest + +from django.urls import reverse +from django.core.exceptions import ValidationError +from django.utils import timezone + +from funkwhale_api.playlists import models +from funkwhale_api.playlists import serializers + + +def test_can_create_playlist_via_api(logged_in_api_client): + url = reverse('api:v1:playlists-list') + data = { + 'name': 'test', + 'privacy_level': 'everyone' + } + + response = logged_in_api_client.post(url, data) + + playlist = logged_in_api_client.user.playlists.latest('id') + assert playlist.name == 'test' + assert playlist.privacy_level == 'everyone' + + +def test_serializer_includes_tracks_count(factories, logged_in_api_client): + playlist = factories['playlists.Playlist']() + plt = factories['playlists.PlaylistTrack'](playlist=playlist) + + url = reverse('api:v1:playlists-detail', kwargs={'pk': playlist.pk}) + response = logged_in_api_client.get(url) + + assert response.data['tracks_count'] == 1 + + +def test_playlist_inherits_user_privacy(logged_in_api_client): + url = reverse('api:v1:playlists-list') + user = logged_in_api_client.user + user.privacy_level = 'me' + user.save() + + data = { + 'name': 'test', + } + + response = logged_in_api_client.post(url, data) + playlist = user.playlists.latest('id') + assert playlist.privacy_level == user.privacy_level + + +def test_can_add_playlist_track_via_api(factories, logged_in_api_client): + tracks = factories['music.Track'].create_batch(5) + playlist = factories['playlists.Playlist'](user=logged_in_api_client.user) + url = reverse('api:v1:playlist-tracks-list') + data = { + 'playlist': playlist.pk, + 'track': tracks[0].pk + } + + response = logged_in_api_client.post(url, data) + assert response.status_code == 201 + plts = logged_in_api_client.user.playlists.latest('id').playlist_tracks.all() + assert plts.first().track == tracks[0] + + +@pytest.mark.parametrize('name,method', [ + ('api:v1:playlist-tracks-list', 'post'), + ('api:v1:playlists-list', 'post'), +]) +def test_url_requires_login(name, method, factories, api_client): + url = reverse(name) + + response = getattr(api_client, method)(url, {}) + + assert response.status_code == 401 + + +def test_only_can_add_track_on_own_playlist_via_api( + factories, logged_in_api_client): + track = factories['music.Track']() + playlist = factories['playlists.Playlist']() + url = reverse('api:v1:playlist-tracks-list') + data = { + 'playlist': playlist.pk, + 'track': track.pk + } + + response = logged_in_api_client.post(url, data) + assert response.status_code == 400 + assert playlist.playlist_tracks.count() == 0 + + +def test_deleting_plt_updates_indexes( + mocker, factories, logged_in_api_client): + remove = mocker.spy(models.Playlist, 'remove') + track = factories['music.Track']() + plt = factories['playlists.PlaylistTrack']( + index=0, + playlist__user=logged_in_api_client.user) + url = reverse('api:v1:playlist-tracks-detail', kwargs={'pk': plt.pk}) + + response = logged_in_api_client.delete(url) + + assert response.status_code == 204 + remove.assert_called_once_with(plt.playlist, 0) + + +@pytest.mark.parametrize('level', ['instance', 'me', 'followers']) +def test_playlist_privacy_respected_in_list_anon(level, factories, api_client): + factories['playlists.Playlist'](privacy_level=level) + url = reverse('api:v1:playlists-list') + response = api_client.get(url) + + assert response.data['count'] == 0 + + +@pytest.mark.parametrize('method', ['PUT', 'PATCH', 'DELETE']) +def test_only_owner_can_edit_playlist(method, factories, api_client): + playlist = factories['playlists.Playlist']() + url = reverse('api:v1:playlists-detail', kwargs={'pk': playlist.pk}) + response = api_client.get(url) + + assert response.status_code == 404 + + +@pytest.mark.parametrize('method', ['PUT', 'PATCH', 'DELETE']) +def test_only_owner_can_edit_playlist_track(method, factories, api_client): + plt = factories['playlists.PlaylistTrack']() + url = reverse('api:v1:playlist-tracks-detail', kwargs={'pk': plt.pk}) + response = api_client.get(url) + + assert response.status_code == 404 + + +@pytest.mark.parametrize('level', ['instance', 'me', 'followers']) +def test_playlist_track_privacy_respected_in_list_anon( + level, factories, api_client): + factories['playlists.PlaylistTrack'](playlist__privacy_level=level) + url = reverse('api:v1:playlist-tracks-list') + response = api_client.get(url) + + assert response.data['count'] == 0 + + +@pytest.mark.parametrize('level', ['instance', 'me', 'followers']) +def test_can_list_tracks_from_playlist( + level, factories, logged_in_api_client): + plt = factories['playlists.PlaylistTrack']( + playlist__user=logged_in_api_client.user) + url = reverse('api:v1:playlists-tracks', kwargs={'pk': plt.playlist.pk}) + response = logged_in_api_client.get(url) + serialized_plt = serializers.PlaylistTrackSerializer(plt).data + + assert response.data['count'] == 1 + assert response.data['results'][0] == serialized_plt + + +def test_can_add_multiple_tracks_at_once_via_api( + factories, mocker, logged_in_api_client): + playlist = factories['playlists.Playlist'](user=logged_in_api_client.user) + tracks = factories['music.Track'].create_batch(size=5) + track_ids = [t.id for t in tracks] + mocker.spy(playlist, 'insert_many') + url = reverse('api:v1:playlists-add', kwargs={'pk': playlist.pk}) + response = logged_in_api_client.post(url, {'tracks': track_ids}) + + assert response.status_code == 201 + assert playlist.playlist_tracks.count() == len(track_ids) + + for plt in playlist.playlist_tracks.order_by('index'): + assert response.data['results'][plt.index]['id'] == plt.id + assert plt.track == tracks[plt.index] + + +def test_can_clear_playlist_from_api( + factories, mocker, logged_in_api_client): + playlist = factories['playlists.Playlist'](user=logged_in_api_client.user) + plts = factories['playlists.PlaylistTrack'].create_batch( + size=5, playlist=playlist) + url = reverse('api:v1:playlists-clear', kwargs={'pk': playlist.pk}) + response = logged_in_api_client.delete(url) + + assert response.status_code == 204 + assert playlist.playlist_tracks.count() == 0 + + +def test_update_playlist_from_api( + factories, mocker, logged_in_api_client): + playlist = factories['playlists.Playlist'](user=logged_in_api_client.user) + plts = factories['playlists.PlaylistTrack'].create_batch( + size=5, playlist=playlist) + url = reverse('api:v1:playlists-detail', kwargs={'pk': playlist.pk}) + response = logged_in_api_client.patch(url, {'name': 'test'}) + playlist.refresh_from_db() + + assert response.status_code == 200 + assert response.data['user']['username'] == playlist.user.username diff --git a/api/tests/test_playlists.py b/api/tests/test_playlists.py deleted file mode 100644 index f496a64cb..000000000 --- a/api/tests/test_playlists.py +++ /dev/null @@ -1,54 +0,0 @@ -import json -from django.urls import reverse -from django.core.exceptions import ValidationError -from django.utils import timezone - -from funkwhale_api.playlists import models -from funkwhale_api.playlists.serializers import PlaylistSerializer - - - -def test_can_create_playlist(factories): - tracks = factories['music.Track'].create_batch(5) - playlist = factories['playlists.Playlist']() - - previous = None - for track in tracks: - previous = playlist.add_track(track, previous=previous) - - playlist_tracks = list(playlist.playlist_tracks.all()) - - previous = None - for idx, track in enumerate(tracks): - plt = playlist_tracks[idx] - assert plt.position == idx - assert plt.track == track - if previous: - assert playlist_tracks[idx + 1] == previous - assert plt.playlist == playlist - - -def test_can_create_playlist_via_api(logged_in_client): - url = reverse('api:v1:playlists-list') - data = { - 'name': 'test', - } - - response = logged_in_client.post(url, data) - - playlist = logged_in_client.user.playlists.latest('id') - assert playlist.name == 'test' - - -def test_can_add_playlist_track_via_api(factories, logged_in_client): - tracks = factories['music.Track'].create_batch(5) - playlist = factories['playlists.Playlist'](user=logged_in_client.user) - url = reverse('api:v1:playlist-tracks-list') - data = { - 'playlist': playlist.pk, - 'track': tracks[0].pk - } - - response = logged_in_client.post(url, data) - plts = logged_in_client.user.playlists.latest('id').playlist_tracks.all() - assert plts.first().track == tracks[0] diff --git a/changes/changelog.d/3.feature b/changes/changelog.d/3.feature new file mode 100644 index 000000000..e2b962c44 --- /dev/null +++ b/changes/changelog.d/3.feature @@ -0,0 +1 @@ +Playlists are here \o/ :tada: (#3, #93, #94) diff --git a/front/src/App.vue b/front/src/App.vue index 347f19e30..d15eebdba 100644 --- a/front/src/App.vue +++ b/front/src/App.vue @@ -29,6 +29,8 @@ v-if="$store.state.instance.settings.raven.front_enabled.value" :dsn="$store.state.instance.settings.raven.front_dsn.value"> + + @@ -39,11 +41,14 @@ import logger from '@/logging' import Sidebar from '@/components/Sidebar' import Raven from '@/components/Raven' +import PlaylistModal from '@/components/playlists/PlaylistModal' + export default { name: 'app', components: { Sidebar, - Raven + Raven, + PlaylistModal }, created () { this.$store.dispatch('instance/fetchSettings') diff --git a/front/src/components/Pagination.vue b/front/src/components/Pagination.vue index 57ae4235d..47cf5183a 100644 --- a/front/src/components/Pagination.vue +++ b/front/src/components/Pagination.vue @@ -64,7 +64,6 @@ export default { } } }) - console.log(final) return final }, maxPage: function () { diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue index 179ffedd7..f225313b6 100644 --- a/front/src/components/Sidebar.vue +++ b/front/src/components/Sidebar.vue @@ -36,6 +36,12 @@ Login Browse library Favorites + + Playlists + Activity diff --git a/front/src/components/audio/PlayButton.vue b/front/src/components/audio/PlayButton.vue index 679f8b795..f2a389862 100644 --- a/front/src/components/audio/PlayButton.vue +++ b/front/src/components/audio/PlayButton.vue @@ -3,11 +3,11 @@ -