diff --git a/.env.dev b/.env.dev index e117dbe56..7037b1dbd 100644 --- a/.env.dev +++ b/.env.dev @@ -11,3 +11,4 @@ WEBPACK_DEVSERVER_PORT=8080 MUSIC_DIRECTORY_PATH=/music BROWSABLE_API_ENABLED=True CACHEOPS_ENABLED=False +FORWARDED_PROTO=http diff --git a/CHANGELOG b/CHANGELOG index c53714488..283bcfcfb 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -10,6 +10,210 @@ This changelog is viewable on the web at https://docs.funkwhale.audio/changelog. .. towncrier +0.14 (2018-06-02) +----------------- + +Upgrade instructions are available at + https://docs.funkwhale.audio/upgrading.html + +Features: + +- Admins can now configure default permissions that will be granted to all + registered users (#236) +- Files management interface for users with "library" permission (#223) +- New action table component for quick and efficient batch actions (#228) This + is implemented on the federated tracks pages, but will be included in other + pages as well depending on the feedback. + + +Enhancements: + +- Added a new "upload" permission that allows user to launch import and view + their own imports (#230) +- Added Support for OggTheora in import. +- Autoremove media files on model instance deletion (#241) +- Can now import a whole remote library at once thanks to new Action Table + component (#164) +- Can now use album covers from flac/mp3 metadata and separate file in track + directory (#219) +- Implemented getCovertArt in Subsonic API to serve album covers (#258) +- Implemented scrobble endpoint of subsonic API, listenings are now tracked + correctly from third party apps that use this endpoint (#260) +- Retructured music API to increase performance and remove useless endpoints + (#224) + + +Bugfixes: + +- Consistent constraints/checks for URL size (#207) +- Display proper total number of tracks on radio detail (#225) +- Do not crash on flac import if musicbrainz tags are missing (#214) +- Empty save button in radio builder (#226) +- Ensure anonymous users can use the app if the instance is configured + accordingly (#229) +- Ensure inactive users cannot get auth tokens (#218) This was already the case + bug we missed some checks +- File-upload import now supports Flac files (#213) +- File-upload importer should now work properly, assuming files are tagged + (#106) +- Fixed a few broken translations strings (#227) +- Fixed broken ordering in front-end lists (#179) +- Fixed ignored page_size paremeter on artist and favorites list (#240) +- Read ID3Tag Tracknumber from TRCK (#220) +- We now fetch album covers regardless of the import methods (#231) + +Documentation: + +- Added missing subsonic configuration block in deployment vhost files (#249) +- Moved upgrade doc under install doc in TOC (#251) + + +Other: + +- Removed acoustid support, as the integration was buggy and error-prone (#106) + + +Files management interface +^^^^^^^^^^^^^^^^^^^^^^^^^^ + +This is the first bit of an ongoing work that will span several releases, to +bring more powerful library management features to Funkwhale. This iteration +includes a basic file management interface where users with the "library" +permission can list and search available files, order them using +various criterias (size, bitrate, duration...) and delete them. + +New "upload" permission +^^^^^^^^^^^^^^^^^^^^^^^ + +This new permission is helpful if you want to give upload/import rights +to some users, but don't want them to be able to manage the library as a whole: +although there are no controls yet for managing library in the fron-end, +subsequent release will introduce management interfaces for artists, files, +etc. + +Because of that, users with the "library" permission will have much more power, +and will also be able to remove content from the platform. On the other hand, +users with the "upload" permission will only have the ability to add new +content. + +Also, this release also includes a new feature called "default permissions": +those are permissions that are granted to every users on the platform. +On public/open instances, this will play well with the "upload" permission +since everyone will be able to contribute to the instance library without +an admin giving the permission to every single user. + +Smarter album cover importer +^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +In earlier versions, covers where only imported when launching a YouTube import. +Starting from this release, covers will be imported regardless of the import mode +(file upload, youtube-dl, CLI, in-place...). Funkwhale will look for covers +in the following order: + +1. In the imported file itself (FLAC/MP3 only) +2. In a cover.jpg or cover.png in the file directory +3. By fetching cover art from Musibrainz, assuming the file is tagged correctly + +This will only work for newly imported tracks and albums though. In the future, +we may offer an option to refetch album covers from the interface, but in the +meantime, you can use the following snippet: + +.. code-block:: python + + # Store this in /tmp/update_albums.py + from funkwhale_api.music.models import Album, TrackFile + from funkwhale_api.music.tasks import update_album_cover + + albums_without_covers = Album.objects.filter(cover='') + total = albums_without_covers.count() + print('Found {} albums without cover'.format(total)) + for i, album in enumerate(albums_without_covers.iterator()): + print('[{}/{}] Fetching cover for {}...'.format(i+1, total, album.title)) + f = TrackFile.objects.filter(track__album=album).filter(source__startswith='file://').first() + update_album_cover(album, track_file=f) + +Then launch it:: + + # docker setups + cat /tmp/update_albums.py | docker-compose run --rm api python manage.py shell -i python + + # non-docker setups + source /srv/funkwhale/load_env + source /srv/funkwhale/virtualenv/bin/activate + cat /tmp/update_albums.py | python manage.py shell -i python + + # cleanup + rm /tmp/update_albums.py + +.. note:: + + Depending on your number of albums, the previous snippet may take some time + to execute. You can interrupt it at any time using ctrl-c and relaunch it later, + as it's idempotent. + +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. + +Flac files imports via upload +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +You have nothing to do to benefit from this, however, since Flac files +tend to be a lot bigger than other files, you may want to increase the +``client_max_body_size`` value in your Nginx configuration if you plan +to upload flac files. + +Missing subsonic configuration bloc in vhost files +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Because of a missing bloc in the sample Nginx and Apache configurations, +instances that were deployed after the 0.13 release are likely to be unable +to answer to Subsonic clients (the missing bits were properly documented +in the changelog). + +Ensure you have the following snippets in your Nginx or Apache configuration +if you plan to use the Subsonic API. + +Nginx:: + + location /rest/ { + include /etc/nginx/funkwhale_proxy.conf; + proxy_pass http://funkwhale-api/api/subsonic/rest/; + } + +Apache2:: + + + ProxyPass ${funkwhale-api}/api/subsonic/rest + ProxyPassReverse ${funkwhale-api}/api/subsonic/rest + + + 0.13 (2018-05-19) ----------------- diff --git a/api/config/api_urls.py b/api/config/api_urls.py index e75781d14..98b863a93 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -38,6 +38,10 @@ v1_patterns += [ include( ('funkwhale_api.instance.urls', 'instance'), namespace='instance')), + url(r'^manage/', + include( + ('funkwhale_api.manage.urls', 'manage'), + namespace='manage')), url(r'^federation/', include( ('funkwhale_api.federation.api_urls', 'federation'), diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 59aa93117..6ab2a8303 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -97,6 +97,7 @@ THIRD_PARTY_APPS = ( 'dynamic_preferences', 'django_filters', 'cacheops', + 'django_cleanup', ) @@ -302,6 +303,9 @@ ROOT_URLCONF = 'config.urls' WSGI_APPLICATION = 'config.wsgi.application' ASGI_APPLICATION = "config.routing.application" +# This ensures that Django will be able to detect a secure connection +SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https') + # AUTHENTICATION CONFIGURATION # ------------------------------------------------------------------------------ AUTHENTICATION_BACKENDS = ( @@ -433,12 +437,6 @@ USE_X_FORWARDED_PORT = True REVERSE_PROXY_TYPE = env('REVERSE_PROXY_TYPE', default='nginx') assert REVERSE_PROXY_TYPE in ['apache2', 'nginx'], 'Unsupported REVERSE_PROXY_TYPE' -# Wether we should check user permission before serving audio files (meaning -# return an obfuscated url) -# This require a special configuration on the reverse proxy side -# See https://wellfire.co/learn/nginx-django-x-accel-redirects/ for example -PROTECT_AUDIO_FILES = env.bool('PROTECT_AUDIO_FILES', default=True) - # Which path will be used to process the internal redirection # **DO NOT** put a slash at the end PROTECT_FILES_PATH = env('PROTECT_FILES_PATH', default='/_protected') diff --git a/api/config/settings/local.py b/api/config/settings/local.py index 592600629..df14945cc 100644 --- a/api/config/settings/local.py +++ b/api/config/settings/local.py @@ -76,3 +76,4 @@ LOGGING = { }, }, } +CSRF_TRUSTED_ORIGINS = [o for o in ALLOWED_HOSTS] diff --git a/api/config/settings/production.py b/api/config/settings/production.py index 2866e9103..39be40dc3 100644 --- a/api/config/settings/production.py +++ b/api/config/settings/production.py @@ -22,10 +22,6 @@ from .common import * # noqa # Raises ImproperlyConfigured exception if DJANGO_SECRET_KEY not in os.environ SECRET_KEY = env("DJANGO_SECRET_KEY") -# This ensures that Django will be able to detect a secure connection -# properly on Heroku. -SECURE_PROXY_SSL_HEADER = ('HTTP_X_FORWARDED_PROTO', 'https') - # django-secure # ------------------------------------------------------------------------------ # INSTALLED_APPS += ("djangosecure", ) diff --git a/api/funkwhale_api/__init__.py b/api/funkwhale_api/__init__.py index b0d7cc517..0896aba8a 100644 --- a/api/funkwhale_api/__init__.py +++ b/api/funkwhale_api/__init__.py @@ -1,3 +1,3 @@ # -*- coding: utf-8 -*- -__version__ = '0.13' +__version__ = '0.14' __version_info__ = tuple([int(num) if num.isdigit() else num for num in __version__.replace('-', '.', 1).split('.')]) diff --git a/api/funkwhale_api/common/pagination.py b/api/funkwhale_api/common/pagination.py index 224c470dc..20efcb741 100644 --- a/api/funkwhale_api/common/pagination.py +++ b/api/funkwhale_api/common/pagination.py @@ -3,4 +3,4 @@ from rest_framework.pagination import PageNumberPagination class FunkwhalePagination(PageNumberPagination): page_size_query_param = 'page_size' - max_page_size = 25 + max_page_size = 50 diff --git a/api/funkwhale_api/common/preferences.py b/api/funkwhale_api/common/preferences.py index e6eb8beda..a2d3f04b7 100644 --- a/api/funkwhale_api/common/preferences.py +++ b/api/funkwhale_api/common/preferences.py @@ -1,4 +1,8 @@ from django.conf import settings +from django import forms + +from dynamic_preferences import serializers +from dynamic_preferences import types from dynamic_preferences.registries import global_preferences_registry @@ -10,3 +14,38 @@ class DefaultFromSettingMixin(object): def get(pref): manager = global_preferences_registry.manager() return manager[pref] + + +class StringListSerializer(serializers.BaseSerializer): + separator = ',' + sort = True + + @classmethod + def to_db(cls, value, **kwargs): + if not value: + return + + if type(value) not in [list, tuple]: + raise cls.exception( + "Cannot serialize, value {} is not a list or a tuple".format( + value)) + + if cls.sort: + value = sorted(value) + return cls.separator.join(value) + + @classmethod + def to_python(cls, value, **kwargs): + if not value: + return [] + return value.split(',') + + +class StringListPreference(types.BasePreferenceType): + serializer = StringListSerializer + field_class = forms.MultipleChoiceField + + def get_api_additional_data(self): + d = super(StringListPreference, self).get_api_additional_data() + d['choices'] = self.get('choices') + return d diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py new file mode 100644 index 000000000..a995cc360 --- /dev/null +++ b/api/funkwhale_api/common/serializers.py @@ -0,0 +1,83 @@ +from rest_framework import serializers + + +class ActionSerializer(serializers.Serializer): + """ + A special serializer that can operate on a list of objects + and apply actions on it. + """ + + action = serializers.CharField(required=True) + objects = serializers.JSONField(required=True) + filters = serializers.DictField(required=False) + actions = None + filterset_class = None + # those are actions identifier where we don't want to allow the "all" + # selector because it's to dangerous. Like object deletion. + dangerous_actions = [] + + def __init__(self, *args, **kwargs): + self.queryset = kwargs.pop('queryset') + if self.actions is None: + raise ValueError( + 'You must declare a list of actions on ' + 'the serializer class') + + for action in self.actions: + handler_name = 'handle_{}'.format(action) + assert hasattr(self, handler_name), ( + '{} miss a {} method'.format( + self.__class__.__name__, handler_name) + ) + super().__init__(self, *args, **kwargs) + + def validate_action(self, value): + if value not in self.actions: + raise serializers.ValidationError( + '{} is not a valid action. Pick one of {}.'.format( + value, ', '.join(self.actions) + ) + ) + return value + + def validate_objects(self, value): + qs = None + if value == 'all': + return self.queryset.all().order_by('id') + if type(value) in [list, tuple]: + return self.queryset.filter(pk__in=value).order_by('id') + + raise serializers.ValidationError( + '{} is not a valid value for objects. You must provide either a ' + 'list of identifiers or the string "all".'.format(value)) + + def validate(self, data): + dangerous = data['action'] in self.dangerous_actions + if dangerous and self.initial_data['objects'] == 'all': + raise serializers.ValidationError( + 'This action is to dangerous to be applied to all objects') + if self.filterset_class and 'filters' in data: + qs_filterset = self.filterset_class( + data['filters'], queryset=data['objects']) + try: + assert qs_filterset.form.is_valid() + except (AssertionError, TypeError): + raise serializers.ValidationError('Invalid filters') + data['objects'] = qs_filterset.qs + + data['count'] = data['objects'].count() + if data['count'] < 1: + raise serializers.ValidationError( + 'No object matching your request') + return data + + def save(self): + handler_name = 'handle_{}'.format(self.validated_data['action']) + handler = getattr(self, handler_name) + result = handler(self.validated_data['objects']) + payload = { + 'updated': self.validated_data['count'], + 'action': self.validated_data['action'], + 'result': result, + } + return payload diff --git a/api/funkwhale_api/favorites/serializers.py b/api/funkwhale_api/favorites/serializers.py index 276b0f6bd..bb4538b2d 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/favorites/views.py b/api/funkwhale_api/favorites/views.py index d874c9e1e..cd2aa3b61 100644 --- a/api/funkwhale_api/favorites/views.py +++ b/api/funkwhale_api/favorites/views.py @@ -12,12 +12,6 @@ from . import models from . import serializers -class CustomLimitPagination(pagination.PageNumberPagination): - page_size = 100 - page_size_query_param = 'page_size' - max_page_size = 100 - - class TrackFavoriteViewSet(mixins.CreateModelMixin, mixins.DestroyModelMixin, mixins.ListModelMixin, @@ -26,7 +20,6 @@ class TrackFavoriteViewSet(mixins.CreateModelMixin, serializer_class = serializers.UserTrackFavoriteSerializer queryset = (models.TrackFavorite.objects.all()) permission_classes = [ConditionalAuthentication] - pagination_class = CustomLimitPagination def create(self, request, *args, **kwargs): serializer = self.get_serializer(data=request.data) diff --git a/api/funkwhale_api/federation/filters.py b/api/funkwhale_api/federation/filters.py index 7a388ff12..1d93f68b9 100644 --- a/api/funkwhale_api/federation/filters.py +++ b/api/funkwhale_api/federation/filters.py @@ -24,7 +24,7 @@ class LibraryFilter(django_filters.FilterSet): class LibraryTrackFilter(django_filters.FilterSet): library = django_filters.CharFilter('library__uuid') - imported = django_filters.CharFilter(method='filter_imported') + status = django_filters.CharFilter(method='filter_status') q = fields.SearchFilter(search_fields=[ 'artist_name', 'title', @@ -32,11 +32,15 @@ class LibraryTrackFilter(django_filters.FilterSet): 'library__actor__domain', ]) - def filter_imported(self, queryset, field_name, value): - if value.lower() in ['true', '1', 'yes']: - queryset = queryset.filter(local_track_file__isnull=False) - elif value.lower() in ['false', '0', 'no']: - queryset = queryset.filter(local_track_file__isnull=True) + def filter_status(self, queryset, field_name, value): + if value == 'imported': + return queryset.filter(local_track_file__isnull=False) + elif value == 'not_imported': + return queryset.filter( + local_track_file__isnull=True + ).exclude(import_jobs__status='pending') + elif value == 'import_pending': + return queryset.filter(import_jobs__status='pending') return queryset class Meta: diff --git a/api/funkwhale_api/federation/migrations/0006_auto_20180521_1702.py b/api/funkwhale_api/federation/migrations/0006_auto_20180521_1702.py new file mode 100644 index 000000000..7dcf85670 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0006_auto_20180521_1702.py @@ -0,0 +1,28 @@ +# Generated by Django 2.0.4 on 2018-05-21 17:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0005_auto_20180413_1723'), + ] + + operations = [ + migrations.AlterField( + model_name='library', + name='url', + field=models.URLField(max_length=500), + ), + migrations.AlterField( + model_name='librarytrack', + name='audio_url', + field=models.URLField(max_length=500), + ), + migrations.AlterField( + model_name='librarytrack', + name='url', + field=models.URLField(max_length=500, unique=True), + ), + ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 69d0ea925..8b4f28475 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -139,7 +139,7 @@ class Library(models.Model): on_delete=models.CASCADE, related_name='library') uuid = models.UUIDField(default=uuid.uuid4) - url = models.URLField() + url = models.URLField(max_length=500) # use this flag to disable federation with a library federation_enabled = models.BooleanField() @@ -166,8 +166,8 @@ def get_file_path(instance, filename): class LibraryTrack(models.Model): - url = models.URLField(unique=True) - audio_url = models.URLField() + url = models.URLField(unique=True, max_length=500) + audio_url = models.URLField(max_length=500) audio_mimetype = models.CharField(max_length=200) audio_file = models.FileField( upload_to=get_file_path, diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 8d3dd6379..6ffffaa9a 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -10,8 +10,11 @@ from rest_framework import serializers from dynamic_preferences.registries import global_preferences_registry from funkwhale_api.common import utils as funkwhale_utils - +from funkwhale_api.common import serializers as common_serializers +from funkwhale_api.music import models as music_models +from funkwhale_api.music import tasks as music_tasks from . import activity +from . import filters from . import models from . import utils @@ -26,16 +29,16 @@ logger = logging.getLogger(__name__) class ActorSerializer(serializers.Serializer): - id = serializers.URLField() - outbox = serializers.URLField() - inbox = serializers.URLField() + id = serializers.URLField(max_length=500) + outbox = serializers.URLField(max_length=500) + inbox = serializers.URLField(max_length=500) type = serializers.ChoiceField(choices=models.TYPE_CHOICES) preferredUsername = serializers.CharField() manuallyApprovesFollowers = serializers.NullBooleanField(required=False) name = serializers.CharField(required=False, max_length=200) summary = serializers.CharField(max_length=None, required=False) - followers = serializers.URLField(required=False, allow_null=True) - following = serializers.URLField(required=False, allow_null=True) + followers = serializers.URLField(max_length=500, required=False, allow_null=True) + following = serializers.URLField(max_length=500, required=False, allow_null=True) publicKey = serializers.JSONField(required=False) def to_representation(self, instance): @@ -224,7 +227,7 @@ class APILibraryFollowUpdateSerializer(serializers.Serializer): class APILibraryCreateSerializer(serializers.ModelSerializer): - actor = serializers.URLField() + actor = serializers.URLField(max_length=500) federation_enabled = serializers.BooleanField() uuid = serializers.UUIDField(read_only=True) @@ -293,6 +296,7 @@ class APILibraryCreateSerializer(serializers.ModelSerializer): class APILibraryTrackSerializer(serializers.ModelSerializer): library = APILibrarySerializer() + status = serializers.SerializerMethodField() class Meta: model = models.LibraryTrack @@ -311,13 +315,25 @@ class APILibraryTrackSerializer(serializers.ModelSerializer): 'title', 'library', 'local_track_file', + 'status', ] + def get_status(self, o): + try: + if o.local_track_file is not None: + return 'imported' + except music_models.TrackFile.DoesNotExist: + pass + for job in o.import_jobs.all(): + if job.status == 'pending': + return 'import_pending' + return 'not_imported' + class FollowSerializer(serializers.Serializer): - id = serializers.URLField() - object = serializers.URLField() - actor = serializers.URLField() + id = serializers.URLField(max_length=500) + object = serializers.URLField(max_length=500) + actor = serializers.URLField(max_length=500) type = serializers.ChoiceField(choices=['Follow']) def validate_object(self, v): @@ -374,8 +390,8 @@ class APIFollowSerializer(serializers.ModelSerializer): class AcceptFollowSerializer(serializers.Serializer): - id = serializers.URLField() - actor = serializers.URLField() + id = serializers.URLField(max_length=500) + actor = serializers.URLField(max_length=500) object = FollowSerializer() type = serializers.ChoiceField(choices=['Accept']) @@ -417,8 +433,8 @@ class AcceptFollowSerializer(serializers.Serializer): class UndoFollowSerializer(serializers.Serializer): - id = serializers.URLField() - actor = serializers.URLField() + id = serializers.URLField(max_length=500) + actor = serializers.URLField(max_length=500) object = FollowSerializer() type = serializers.ChoiceField(choices=['Undo']) @@ -459,9 +475,9 @@ class UndoFollowSerializer(serializers.Serializer): class ActorWebfingerSerializer(serializers.Serializer): subject = serializers.CharField() - aliases = serializers.ListField(child=serializers.URLField()) + aliases = serializers.ListField(child=serializers.URLField(max_length=500)) links = serializers.ListField() - actor_url = serializers.URLField(required=False) + actor_url = serializers.URLField(max_length=500, required=False) def validate(self, validated_data): validated_data['actor_url'] = None @@ -496,8 +512,8 @@ class ActorWebfingerSerializer(serializers.Serializer): class ActivitySerializer(serializers.Serializer): - actor = serializers.URLField() - id = serializers.URLField(required=False) + actor = serializers.URLField(max_length=500) + id = serializers.URLField(max_length=500, required=False) type = serializers.ChoiceField( choices=[(c, c) for c in activity.ACTIVITY_TYPES]) object = serializers.JSONField() @@ -539,8 +555,8 @@ class ActivitySerializer(serializers.Serializer): class ObjectSerializer(serializers.Serializer): - id = serializers.URLField() - url = serializers.URLField(required=False, allow_null=True) + id = serializers.URLField(max_length=500) + url = serializers.URLField(max_length=500, required=False, allow_null=True) type = serializers.ChoiceField( choices=[(c, c) for c in activity.OBJECT_TYPES]) content = serializers.CharField( @@ -554,16 +570,16 @@ class ObjectSerializer(serializers.Serializer): updated = serializers.DateTimeField( required=False, allow_null=True) to = serializers.ListField( - child=serializers.URLField(), + child=serializers.URLField(max_length=500), required=False, allow_null=True) cc = serializers.ListField( - child=serializers.URLField(), + child=serializers.URLField(max_length=500), required=False, allow_null=True) bto = serializers.ListField( - child=serializers.URLField(), + child=serializers.URLField(max_length=500), required=False, allow_null=True) bcc = serializers.ListField( - child=serializers.URLField(), + child=serializers.URLField(max_length=500), required=False, allow_null=True) OBJECT_SERIALIZERS = { @@ -575,10 +591,10 @@ OBJECT_SERIALIZERS = { class PaginatedCollectionSerializer(serializers.Serializer): type = serializers.ChoiceField(choices=['Collection']) totalItems = serializers.IntegerField(min_value=0) - actor = serializers.URLField() - id = serializers.URLField() - first = serializers.URLField() - last = serializers.URLField() + actor = serializers.URLField(max_length=500) + id = serializers.URLField(max_length=500) + first = serializers.URLField(max_length=500) + last = serializers.URLField(max_length=500) def to_representation(self, conf): paginator = Paginator( @@ -607,13 +623,13 @@ class CollectionPageSerializer(serializers.Serializer): type = serializers.ChoiceField(choices=['CollectionPage']) totalItems = serializers.IntegerField(min_value=0) items = serializers.ListField() - actor = serializers.URLField() - id = serializers.URLField() - first = serializers.URLField() - last = serializers.URLField() - next = serializers.URLField(required=False) - prev = serializers.URLField(required=False) - partOf = serializers.URLField() + actor = serializers.URLField(max_length=500) + id = serializers.URLField(max_length=500) + first = serializers.URLField(max_length=500) + last = serializers.URLField(max_length=500) + next = serializers.URLField(max_length=500, required=False) + prev = serializers.URLField(max_length=500, required=False) + partOf = serializers.URLField(max_length=500) def validate_items(self, v): item_serializer = self.context.get('item_serializer') @@ -698,7 +714,7 @@ class AudioMetadataSerializer(serializers.Serializer): class AudioSerializer(serializers.Serializer): type = serializers.CharField() - id = serializers.URLField() + id = serializers.URLField(max_length=500) url = serializers.JSONField() published = serializers.DateTimeField() updated = serializers.DateTimeField(required=False) @@ -806,3 +822,29 @@ class CollectionSerializer(serializers.Serializer): if self.context.get('include_ap_context', True): d['@context'] = AP_CONTEXT return d + + +class LibraryTrackActionSerializer(common_serializers.ActionSerializer): + actions = ['import'] + filterset_class = filters.LibraryTrackFilter + + @transaction.atomic + def handle_import(self, objects): + batch = music_models.ImportBatch.objects.create( + source='federation', + submitted_by=self.context['submitted_by'] + ) + jobs = [] + for lt in objects: + job = music_models.ImportJob( + batch=batch, + library_track=lt, + mbid=lt.mbid, + source=lt.url, + ) + jobs.append(job) + + music_models.ImportJob.objects.bulk_create(jobs) + music_tasks.import_batch_run.delay(import_batch_id=batch.pk) + + return {'batch': {'id': batch.pk}} diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 06a2cd040..1350ec731 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -15,7 +15,7 @@ from rest_framework.serializers import ValidationError from funkwhale_api.common import preferences from funkwhale_api.common import utils as funkwhale_utils -from funkwhale_api.music.models import TrackFile +from funkwhale_api.music import models as music_models from funkwhale_api.users.permissions import HasUserPermission from . import activity @@ -148,7 +148,9 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): def list(self, request, *args, **kwargs): page = request.GET.get('page') library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - qs = TrackFile.objects.order_by('-creation_date').select_related( + qs = music_models.TrackFile.objects.order_by( + '-creation_date' + ).select_related( 'track__artist', 'track__album__artist' ).filter(library_track__isnull=True) @@ -294,7 +296,7 @@ class LibraryTrackViewSet( 'library__actor', 'library__follow', 'local_track_file', - ) + ).prefetch_related('import_jobs') filter_class = filters.LibraryTrackFilter serializer_class = serializers.APILibraryTrackSerializer ordering_fields = ( @@ -307,3 +309,16 @@ class LibraryTrackViewSet( 'fetched_date', 'published_date', ) + + @list_route(methods=['post']) + def action(self, request, *args, **kwargs): + queryset = models.LibraryTrack.objects.filter( + local_track_file__isnull=True) + serializer = serializers.LibraryTrackActionSerializer( + request.data, + queryset=queryset, + context={'submitted_by': request.user} + ) + serializer.is_valid(raise_exception=True) + result = serializer.save() + return response.Response(result, status=200) diff --git a/api/funkwhale_api/history/serializers.py b/api/funkwhale_api/history/serializers.py index f7333f243..572787ae0 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 bea96a418..3da8b2a38 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/manage/__init__.py b/api/funkwhale_api/manage/__init__.py new file mode 100644 index 000000000..03e091e5c --- /dev/null +++ b/api/funkwhale_api/manage/__init__.py @@ -0,0 +1,3 @@ +""" +App that includes all views/serializers and stuff for management API +""" diff --git a/api/funkwhale_api/manage/filters.py b/api/funkwhale_api/manage/filters.py new file mode 100644 index 000000000..9853b7a61 --- /dev/null +++ b/api/funkwhale_api/manage/filters.py @@ -0,0 +1,25 @@ +from django.db.models import Count + +from django_filters import rest_framework as filters + +from funkwhale_api.common import fields +from funkwhale_api.music import models as music_models + + +class ManageTrackFileFilterSet(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'track__title', + 'track__album__title', + 'track__artist__name', + 'source', + ]) + + class Meta: + model = music_models.TrackFile + fields = [ + 'q', + 'track__album', + 'track__artist', + 'track', + 'library_track' + ] diff --git a/api/funkwhale_api/manage/serializers.py b/api/funkwhale_api/manage/serializers.py new file mode 100644 index 000000000..02300ec06 --- /dev/null +++ b/api/funkwhale_api/manage/serializers.py @@ -0,0 +1,82 @@ +from django.db import transaction +from rest_framework import serializers + +from funkwhale_api.common import serializers as common_serializers +from funkwhale_api.music import models as music_models + +from . import filters + + +class ManageTrackFileArtistSerializer(serializers.ModelSerializer): + class Meta: + model = music_models.Artist + fields = [ + 'id', + 'mbid', + 'creation_date', + 'name', + ] + + +class ManageTrackFileAlbumSerializer(serializers.ModelSerializer): + artist = ManageTrackFileArtistSerializer() + + class Meta: + model = music_models.Album + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'release_date', + 'cover', + 'creation_date', + ) + + +class ManageTrackFileTrackSerializer(serializers.ModelSerializer): + artist = ManageTrackFileArtistSerializer() + album = ManageTrackFileAlbumSerializer() + + class Meta: + model = music_models.Track + fields = ( + 'id', + 'mbid', + 'title', + 'album', + 'artist', + 'creation_date', + 'position', + ) + + +class ManageTrackFileSerializer(serializers.ModelSerializer): + track = ManageTrackFileTrackSerializer() + + class Meta: + model = music_models.TrackFile + fields = ( + 'id', + 'path', + 'source', + 'filename', + 'mimetype', + 'track', + 'duration', + 'mimetype', + 'bitrate', + 'size', + 'path', + 'library_track', + ) + + +class ManageTrackFileActionSerializer(common_serializers.ActionSerializer): + actions = ['delete'] + dangerous_actions = ['delete'] + filterset_class = filters.ManageTrackFileFilterSet + + @transaction.atomic + def handle_delete(self, objects): + return objects.delete() diff --git a/api/funkwhale_api/manage/urls.py b/api/funkwhale_api/manage/urls.py new file mode 100644 index 000000000..c434581ec --- /dev/null +++ b/api/funkwhale_api/manage/urls.py @@ -0,0 +1,11 @@ +from django.conf.urls import include, url +from . import views + +from rest_framework import routers +library_router = routers.SimpleRouter() +library_router.register(r'track-files', views.ManageTrackFileViewSet, 'track-files') + +urlpatterns = [ + url(r'^library/', + include((library_router.urls, 'instance'), namespace='library')), +] diff --git a/api/funkwhale_api/manage/views.py b/api/funkwhale_api/manage/views.py new file mode 100644 index 000000000..74059caa1 --- /dev/null +++ b/api/funkwhale_api/manage/views.py @@ -0,0 +1,49 @@ +from rest_framework import mixins +from rest_framework import response +from rest_framework import viewsets +from rest_framework.decorators import list_route + +from funkwhale_api.music import models as music_models +from funkwhale_api.users.permissions import HasUserPermission + +from . import filters +from . import serializers + + +class ManageTrackFileViewSet( + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + mixins.DestroyModelMixin, + viewsets.GenericViewSet): + queryset = ( + music_models.TrackFile.objects.all() + .select_related( + 'track__artist', + 'track__album__artist', + 'library_track') + .order_by('-id') + ) + serializer_class = serializers.ManageTrackFileSerializer + filter_class = filters.ManageTrackFileFilterSet + permission_classes = (HasUserPermission,) + required_permissions = ['library'] + ordering_fields = [ + 'accessed_date', + 'modification_date', + 'creation_date', + 'track__artist__name', + 'bitrate', + 'size', + 'duration', + ] + + @list_route(methods=['post']) + def action(self, request, *args, **kwargs): + queryset = self.get_queryset() + serializer = serializers.ManageTrackFileActionSerializer( + request.data, + queryset=queryset, + ) + serializer.is_valid(raise_exception=True) + result = serializer.save() + return response.Response(result, status=200) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 412e2f798..11423f5b0 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -117,6 +117,11 @@ class ImportJobFactory(factory.django.DjangoModelFactory): status='finished', audio_file=None, ) + with_audio_file = factory.Trait( + status='finished', + audio_file=factory.django.FileField( + from_path=os.path.join(SAMPLES_PATH, 'test.ogg')), + ) @registry.register(name='music.FileImportJob') diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index 6da9cca63..dc7aafc21 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/metadata.py b/api/funkwhale_api/music/metadata.py index 494256711..3637b1c8c 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/api/funkwhale_api/music/metadata.py @@ -9,7 +9,13 @@ class TagNotFound(KeyError): pass +class UnsupportedTag(KeyError): + pass + + def get_id3_tag(f, k): + if k == 'pictures': + return f.tags.getall('APIC') # First we try to grab the standard key try: return f.tags[k].text[0] @@ -28,13 +34,39 @@ def get_id3_tag(f, k): raise TagNotFound(k) +def clean_id3_pictures(apic): + pictures = [] + for p in list(apic): + pictures.append({ + 'mimetype': p.mime, + 'content': p.data, + 'description': p.desc, + 'type': p.type.real, + }) + return pictures + + def get_flac_tag(f, k): + if k == 'pictures': + return f.pictures try: - return f.get(k)[0] + return f.get(k, [])[0] except (KeyError, IndexError): raise TagNotFound(k) +def clean_flac_pictures(apic): + pictures = [] + for p in list(apic): + pictures.append({ + 'mimetype': p.mime, + 'content': p.data, + 'description': p.desc, + 'type': p.type.real, + }) + return pictures + + def get_mp3_recording_id(f, k): try: return [ @@ -73,35 +105,51 @@ CONF = { 'field': 'TRACKNUMBER', 'to_application': convert_track_number }, - 'title': { - 'field': 'title' - }, - 'artist': { - 'field': 'artist' - }, - 'album': { - 'field': 'album' - }, + 'title': {}, + 'artist': {}, + 'album': {}, 'date': { 'field': 'date', 'to_application': lambda v: arrow.get(v).date() }, - 'musicbrainz_albumid': { - 'field': 'musicbrainz_albumid' - }, - 'musicbrainz_artistid': { - 'field': 'musicbrainz_artistid' - }, + 'musicbrainz_albumid': {}, + 'musicbrainz_artistid': {}, 'musicbrainz_recordingid': { 'field': 'musicbrainz_trackid' }, } }, - 'MP3': { - 'getter': get_id3_tag, + 'OggTheora': { + 'getter': lambda f, k: f[k][0], 'fields': { 'track_number': { - 'field': 'TPOS', + 'field': 'TRACKNUMBER', + 'to_application': convert_track_number + }, + 'title': {}, + 'artist': {}, + 'album': {}, + 'date': { + 'field': 'date', + 'to_application': lambda v: arrow.get(v).date() + }, + 'musicbrainz_albumid': { + 'field': 'MusicBrainz Album Id' + }, + 'musicbrainz_artistid': { + 'field': 'MusicBrainz Artist Id' + }, + 'musicbrainz_recordingid': { + 'field': 'MusicBrainz Track Id' + }, + } + }, + 'MP3': { + 'getter': get_id3_tag, + 'clean_pictures': clean_id3_pictures, + 'fields': { + 'track_number': { + 'field': 'TRCK', 'to_application': convert_track_number }, 'title': { @@ -127,37 +175,31 @@ CONF = { 'field': 'UFID', 'getter': get_mp3_recording_id, }, + 'pictures': {}, } }, 'FLAC': { 'getter': get_flac_tag, + 'clean_pictures': clean_flac_pictures, 'fields': { 'track_number': { 'field': 'tracknumber', 'to_application': convert_track_number }, - 'title': { - 'field': 'title' - }, - 'artist': { - 'field': 'artist' - }, - 'album': { - 'field': 'album' - }, + 'title': {}, + 'artist': {}, + 'album': {}, 'date': { 'field': 'date', 'to_application': lambda v: arrow.get(str(v)).date() }, - 'musicbrainz_albumid': { - 'field': 'musicbrainz_albumid' - }, - 'musicbrainz_artistid': { - 'field': 'musicbrainz_artistid' - }, + 'musicbrainz_albumid': {}, + 'musicbrainz_artistid': {}, 'musicbrainz_recordingid': { 'field': 'musicbrainz_trackid' }, + 'test': {}, + 'pictures': {}, } }, } @@ -179,8 +221,12 @@ class Metadata(object): return f.__class__.__name__ def get(self, key, default=NODEFAULT): - field_conf = self._conf['fields'][key] - real_key = field_conf['field'] + try: + field_conf = self._conf['fields'][key] + except KeyError: + raise UnsupportedTag( + '{} is not supported for this file format'.format(key)) + real_key = field_conf.get('field', key) try: getter = field_conf.get('getter', self._conf['getter']) v = getter(self._file, real_key) @@ -196,3 +242,16 @@ class Metadata(object): if field: v = field.to_python(v) return v + + def get_picture(self, picture_type='cover_front'): + ptype = getattr(mutagen.id3.PictureType, picture_type.upper()) + try: + pictures = self.get('pictures') + except (UnsupportedTag, TagNotFound): + return + + cleaner = self._conf.get('clean_pictures', lambda v: v) + pictures = cleaner(pictures) + for p in pictures: + if p['type'] == ptype: + return p diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 1259cc3c1..0ba4d22c3 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -23,6 +23,7 @@ from funkwhale_api import downloader from funkwhale_api import musicbrainz from funkwhale_api.federation import utils as federation_utils from . import importers +from . import metadata from . import utils @@ -80,6 +81,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) @@ -186,10 +193,20 @@ class Album(APIModelMixin): } objects = AlbumQuerySet.as_manager() - def get_image(self): - image_data = musicbrainz.api.images.get_front(str(self.mbid)) - f = ContentFile(image_data) - self.cover.save('{0}.jpg'.format(self.mbid), f) + def get_image(self, data=None): + if data: + f = ContentFile(data['content']) + extensions = { + 'image/jpeg': 'jpg', + 'image/png': 'png', + 'image/gif': 'gif', + } + extension = extensions.get(data['mimetype'], 'jpg') + self.cover.save('{}.{}'.format(self.uuid, extension), f) + else: + image_data = musicbrainz.api.images.get_front(str(self.mbid)) + f = ContentFile(image_data) + self.cover.save('{0}.jpg'.format(self.mbid), f) return self.cover.file def __str__(self): @@ -313,11 +330,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): @@ -519,6 +533,12 @@ class TrackFile(models.Model): self.mimetype = utils.guess_mimetype(self.audio_file) return super().save(**kwargs) + def get_metadata(self): + audio_file = self.get_audio_file() + if not audio_file: + return + return metadata.Metadata(audio_file) + IMPORT_STATUS_CHOICES = ( ('pending', 'Pending'), diff --git a/api/funkwhale_api/music/permissions.py b/api/funkwhale_api/music/permissions.py index 77f95c477..d31e1c5d5 100644 --- a/api/funkwhale_api/music/permissions.py +++ b/api/funkwhale_api/music/permissions.py @@ -10,9 +10,6 @@ from funkwhale_api.federation import models class Listen(BasePermission): def has_permission(self, request, view): - if not settings.PROTECT_AUDIO_FILES: - return True - if not preferences.get('common__api_authentication_required'): return True diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index d9d48496e..b72bb8c4a 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 ArtistAlbumSerializer(serializers.ModelSerializer): + tracks_count = serializers.SerializerMethodField() + class Meta: - model = Tag - fields = ('id', 'name', 'slug') + 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 SimpleArtistSerializer(serializers.ModelSerializer): - class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'creation_date') - - -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 Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'release_date', 'cover') - - -class AlbumSerializer(serializers.ModelSerializer): - tags = TagSerializer(many=True, read_only=True) - class Meta: - model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'release_date', 'tags') - - -class LyricsMixin(serializers.ModelSerializer): - lyrics = serializers.SerializerMethodField() - - def get_lyrics(self, obj): - return obj.get_lyrics_url() - - -class TrackSerializer(LyricsMixin): +class AlbumTrackSerializer(serializers.ModelSerializer): files = TrackFileSerializer(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) + class Meta: model = models.Track fields = ( 'id', 'mbid', 'title', + 'album', 'artist', + 'creation_date', 'files', - 'tags', 'position', - 'lyrics') + ) -class TrackSerializerNested(LyricsMixin): - artist = ArtistSerializer() - files = TrackFileSerializer(many=True, read_only=True) - album = SimpleAlbumSerializer(read_only=True) - tags = TagSerializer(many=True, read_only=True) - +class ArtistSimpleSerializer(serializers.ModelSerializer): class Meta: - model = models.Track - fields = ('id', 'mbid', 'title', 'artist', 'files', 'album', 'tags', 'lyrics') + model = models.Artist + fields = ( + 'id', + 'mbid', + 'name', + 'creation_date', + ) -class AlbumSerializerNested(serializers.ModelSerializer): - tracks = TrackSerializer(many=True, read_only=True) - artist = SimpleArtistSerializer() - tags = TagSerializer(many=True, read_only=True) +class AlbumSerializer(serializers.ModelSerializer): + tracks = serializers.SerializerMethodField() + artist = ArtistSimpleSerializer(read_only=True) class Meta: model = models.Album - fields = ('id', 'mbid', 'title', 'cover', 'artist', 'release_date', 'tracks', 'tags') + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'tracks', + 'release_date', + 'cover', + 'creation_date', + ) + + 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 -class ArtistSerializerNested(serializers.ModelSerializer): - albums = AlbumSerializerNested(many=True, read_only=True) - tags = TagSerializer(many=True, read_only=True) +class TrackAlbumSerializer(serializers.ModelSerializer): + artist = ArtistSimpleSerializer(read_only=True) class Meta: - model = models.Artist - fields = ('id', 'mbid', 'name', 'albums', 'tags') + model = models.Album + fields = ( + 'id', + 'mbid', + 'title', + 'artist', + 'release_date', + 'cover', + 'creation_date', + ) + + +class TrackSerializer(serializers.ModelSerializer): + files = TrackFileSerializer(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', + 'album', + 'artist', + 'creation_date', + 'files', + 'position', + 'lyrics', + ) + + def get_lyrics(self, obj): + return obj.get_lyrics_url() + + +class TagSerializer(serializers.ModelSerializer): + class Meta: + model = Tag + fields = ('id', 'name', 'slug') + + +class SimpleAlbumSerializer(serializers.ModelSerializer): + + class Meta: + model = models.Album + fields = ('id', 'mbid', 'title', 'release_date', 'cover') class LyricsSerializer(serializers.ModelSerializer): @@ -197,28 +250,6 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer): return 'Audio' -class SubmitFederationTracksSerializer(serializers.Serializer): - library_tracks = serializers.PrimaryKeyRelatedField( - many=True, - queryset=LibraryTrack.objects.filter(local_track_file__isnull=True), - ) - - @transaction.atomic - def save(self, **kwargs): - batch = models.ImportBatch.objects.create( - source='federation', - **kwargs - ) - for lt in self.validated_data['library_tracks']: - models.ImportJob.objects.create( - batch=batch, - library_track=lt, - mbid=lt.mbid, - source=lt.url, - ) - return batch - - class ImportJobRunSerializer(serializers.Serializer): jobs = serializers.PrimaryKeyRelatedField( many=True, diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 34345e47b..218e374e8 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -1,7 +1,10 @@ +import logging import os from django.core.files.base import ContentFile +from musicbrainzngs import ResponseError + from funkwhale_api.common import preferences from funkwhale_api.federation import activity from funkwhale_api.federation import actors @@ -9,13 +12,15 @@ from funkwhale_api.federation import models as federation_models from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.taskapp import celery from funkwhale_api.providers.acoustid import get_acoustid_client -from funkwhale_api.providers.audiofile.tasks import import_track_data_from_path +from funkwhale_api.providers.audiofile import tasks as audiofile_tasks from django.conf import settings from . import models from . import lyrics as lyrics_utils from . import utils as music_utils +logger = logging.getLogger(__name__) + @celery.app.task(name='acoustid.set_on_track_file') @celery.require_instance(models.TrackFile, 'track_file') @@ -73,13 +78,16 @@ def import_track_from_remote(library_track): library_track.title, artist=artist, album=album)[0] -def _do_import(import_job, replace=False, use_acoustid=True): +def _do_import(import_job, replace=False, use_acoustid=False): + logger.info('[Import Job %s] starting job', import_job.pk) from_file = bool(import_job.audio_file) mbid = import_job.mbid acoustid_track_id = None duration = None track = None - use_acoustid = use_acoustid and preferences.get('providers_acoustid__api_key') + # use_acoustid = use_acoustid and preferences.get('providers_acoustid__api_key') + # Acoustid is not reliable, we disable it for now. + use_acoustid = False if not mbid and use_acoustid and from_file: # we try to deduce mbid from acoustid client = get_acoustid_client() @@ -89,14 +97,32 @@ def _do_import(import_job, replace=False, use_acoustid=True): mbid = match['recordings'][0]['id'] acoustid_track_id = match['id'] if mbid: + logger.info( + '[Import Job %s] importing track from musicbrainz recording %s', + import_job.pk, + str(mbid)) track, _ = models.Track.get_or_create_from_api(mbid=mbid) elif import_job.audio_file: - track = import_track_data_from_path(import_job.audio_file.path) + logger.info( + '[Import Job %s] importing track from uploaded track data at %s', + import_job.pk, + import_job.audio_file.path) + track = audiofile_tasks.import_track_data_from_path( + import_job.audio_file.path) elif import_job.library_track: + logger.info( + '[Import Job %s] importing track from federated library track %s', + import_job.pk, + import_job.library_track.pk) track = import_track_from_remote(import_job.library_track) elif import_job.source.startswith('file://'): - track = import_track_data_from_path( - import_job.source.replace('file://', '', 1)) + tf_path = import_job.source.replace('file://', '', 1) + logger.info( + '[Import Job %s] importing track from local track data at %s', + import_job.pk, + tf_path) + track = audiofile_tasks.import_track_data_from_path( + tf_path) else: raise ValueError( 'Not enough data to process import, ' @@ -104,8 +130,13 @@ def _do_import(import_job, replace=False, use_acoustid=True): track_file = None if replace: + logger.info( + '[Import Job %s] replacing existing audio file', import_job.pk) track_file = track.files.first() elif track.files.count() > 0: + logger.info( + '[Import Job %s] skipping, we already have a file for this track', + import_job.pk) if import_job.audio_file: import_job.audio_file.delete() import_job.status = 'skipped' @@ -129,6 +160,9 @@ def _do_import(import_job, replace=False, use_acoustid=True): pass elif not import_job.audio_file and not import_job.source.startswith('file://'): # not an implace import, and we have a source, so let's download it + logger.info( + '[Import Job %s] downloading audio file from remote', + import_job.pk) track_file.download_file() elif not import_job.audio_file and import_job.source.startswith('file://'): # in place import, we set mimetype from extension @@ -136,23 +170,96 @@ def _do_import(import_job, replace=False, use_acoustid=True): track_file.mimetype = music_utils.get_type_from_ext(ext) track_file.set_audio_data() track_file.save() + # if no cover is set on track album, we try to update it as well: + if not track.album.cover: + logger.info( + '[Import Job %s] retrieving album cover', + import_job.pk) + update_album_cover(track.album, track_file) import_job.status = 'finished' import_job.track_file = track_file if import_job.audio_file: # it's imported on the track, we don't need it anymore import_job.audio_file.delete() import_job.save() - + logger.info( + '[Import Job %s] job finished', + import_job.pk) return track_file +def update_album_cover(album, track_file, replace=False): + if album.cover and not replace: + return + + if track_file: + # maybe the file has a cover embedded? + try: + metadata = track_file.get_metadata() + except FileNotFoundError: + metadata = None + if metadata: + cover = metadata.get_picture('cover_front') + if cover: + # best case scenario, cover is embedded in the track + logger.info( + '[Album %s] Using cover embedded in file', + album.pk) + return album.get_image(data=cover) + if track_file.source and track_file.source.startswith('file://'): + # let's look for a cover in the same directory + path = os.path.dirname(track_file.source.replace('file://', '', 1)) + logger.info( + '[Album %s] scanning covers from %s', + album.pk, + path) + cover = get_cover_from_fs(path) + if cover: + return album.get_image(data=cover) + if not album.mbid: + return + try: + logger.info( + '[Album %s] Fetching cover from musicbrainz release %s', + album.pk, + str(album.mbid)) + return album.get_image() + except ResponseError as exc: + logger.warning( + '[Album %s] cannot fetch cover from musicbrainz: %s', + album.pk, + str(exc)) + + +IMAGE_TYPES = [ + ('jpg', 'image/jpeg'), + ('png', 'image/png'), +] + +def get_cover_from_fs(dir_path): + if os.path.exists(dir_path): + for e, m in IMAGE_TYPES: + cover_path = os.path.join(dir_path, 'cover.{}'.format(e)) + if not os.path.exists(cover_path): + logger.debug('Cover %s does not exists', cover_path) + continue + with open(cover_path, 'rb') as c: + logger.info('Found cover at %s', cover_path) + return { + 'mimetype': m, + 'content': c.read(), + } + + + @celery.app.task(name='ImportJob.run', bind=True) @celery.require_instance( models.ImportJob.objects.filter( status__in=['pending', 'errored']), 'import_job') -def import_job_run(self, import_job, replace=False, use_acoustid=True): - def mark_errored(): +def import_job_run(self, import_job, replace=False, use_acoustid=False): + def mark_errored(exc): + logger.error('[Import Job %s] Error during import: %s', str(exc)) import_job.status = 'errored' import_job.save(update_fields=['status']) @@ -164,12 +271,19 @@ def import_job_run(self, import_job, replace=False, use_acoustid=True): try: self.retry(exc=exc, countdown=30, max_retries=3) except: - mark_errored() + mark_errored(exc) raise - mark_errored() + mark_errored(exc) raise +@celery.app.task(name='ImportBatch.run') +@celery.require_instance(models.ImportBatch, 'import_batch') +def import_batch_run(import_batch): + for job_id in import_batch.jobs.order_by('id').values_list('id', flat=True): + import_job_run.delay(import_job_id=job_id) + + @celery.app.task(name='Lyrics.fetch_content') @celery.require_instance(models.Lyrics, 'lyrics') def fetch_content(lyrics): diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index e71d3555e..2f5b75a97 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 @@ -108,12 +91,21 @@ class ImportBatchViewSet( ) serializer_class = serializers.ImportBatchSerializer permission_classes = (HasUserPermission,) - required_permissions = ['library'] + required_permissions = ['library', 'upload'] + permission_operator = 'or' filter_class = filters.ImportBatchFilter def perform_create(self, serializer): serializer.save(submitted_by=self.request.user) + def get_queryset(self): + qs = super().get_queryset() + # if user do not have library permission, we limit to their + # own jobs + if not self.request.user.has_permissions('library'): + qs = qs.filter(submitted_by=self.request.user) + return qs + class ImportJobViewSet( mixins.CreateModelMixin, @@ -122,11 +114,22 @@ class ImportJobViewSet( queryset = (models.ImportJob.objects.all().select_related()) serializer_class = serializers.ImportJobSerializer permission_classes = (HasUserPermission,) - required_permissions = ['library'] + required_permissions = ['library', 'upload'] + permission_operator = 'or' filter_class = filters.ImportJobFilter + def get_queryset(self): + qs = super().get_queryset() + # if user do not have library permission, we limit to their + # own jobs + if not self.request.user.has_permissions('library'): + qs = qs.filter(batch__submitted_by=self.request.user) + return qs + @list_route(methods=['get']) def stats(self, request, *args, **kwargs): + if not request.user.has_permissions('library'): + return Response(status=403) qs = models.ImportJob.objects.all() filterset = filters.ImportJobFilter(request.GET, queryset=qs) qs = filterset.qs @@ -160,20 +163,21 @@ 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', - 'artist__name__unaccent', + 'title', + 'album__title', + 'album__release_date', + 'position', + 'artist__name', ) def get_queryset(self): @@ -238,8 +242,8 @@ def get_file_path(audio_file): 'You need to specify MUSIC_DIRECTORY_SERVE_PATH and ' 'MUSIC_DIRECTORY_PATH to serve in-place imported files' ) - path = audio_file.replace(prefix, serve_path, 1).encode('utf-8') - return path + path = audio_file.replace(prefix, serve_path, 1) + return path.encode('utf-8') def handle_serve(track_file): @@ -370,10 +374,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 +391,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 +406,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) @@ -477,22 +469,6 @@ class SubmitViewSet(viewsets.ViewSet): data, request, batch=None, import_request=import_request) return Response(import_data) - @list_route(methods=['post']) - @transaction.non_atomic_requests - def federation(self, request, *args, **kwargs): - serializer = serializers.SubmitFederationTracksSerializer( - data=request.data) - serializer.is_valid(raise_exception=True) - batch = serializer.save(submitted_by=request.user) - for job in batch.jobs.all(): - funkwhale_utils.on_commit( - tasks.import_job_run.delay, - import_job_id=job.pk, - use_acoustid=False, - ) - - return Response({'id': batch.id}, status=201) - @transaction.atomic def _import_album(self, data, request, batch=None, import_request=None): # we import the whole album here to prevent race conditions that occurs diff --git a/api/funkwhale_api/playlists/serializers.py b/api/funkwhale_api/playlists/serializers.py index fcb2a412d..3f01fd689 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/providers/audiofile/management/commands/import_files.py b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py index a2757c692..70ff90ffa 100644 --- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py +++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py @@ -54,13 +54,6 @@ class Command(BaseCommand): 'import and not much disk space available.' ) ) - parser.add_argument( - '--no-acoustid', - action='store_true', - dest='no_acoustid', - default=False, - help='Use this flag to completely bypass acoustid completely', - ) parser.add_argument( '--noinput', '--no-input', action='store_false', dest='interactive', help="Do NOT prompt the user for input of any kind.", @@ -118,7 +111,6 @@ class Command(BaseCommand): len(filtered['new']))) self.stdout.write('Selected options: {}'.format(', '.join([ - 'no acoustid' if options['no_acoustid'] else 'use acoustid', 'in place' if options['in_place'] else 'copy music files', ]))) if len(filtered['new']) == 0: @@ -201,4 +193,4 @@ class Command(BaseCommand): job.save() import_handler( import_job_id=job.pk, - use_acoustid=not options['no_acoustid']) + use_acoustid=False) diff --git a/api/funkwhale_api/radios/filters.py b/api/funkwhale_api/radios/filters.py index 344a4dabf..d0d338d66 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 195b382c9..8c59f8715 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 37c07c5e4..ca510b82c 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 395fac66c..6553f3316 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/funkwhale_api/subsonic/serializers.py b/api/funkwhale_api/subsonic/serializers.py index 6709930f5..97cdbcfc6 100644 --- a/api/funkwhale_api/subsonic/serializers.py +++ b/api/funkwhale_api/subsonic/serializers.py @@ -4,6 +4,7 @@ from django.db.models import functions, Count from rest_framework import serializers +from funkwhale_api.history import models as history_models from funkwhale_api.music import models as music_models @@ -57,8 +58,10 @@ class GetArtistSerializer(serializers.Serializer): 'name': album.title, 'artist': artist.name, 'created': album.creation_date, - 'songCount': len(album.tracks.all()) + 'songCount': len(album.tracks.all()), } + if album.cover: + album_data['coverArt'] = 'al-{}'.format(album.id) if album.release_date: album_data['year'] = album.release_date.year payload['album'].append(album_data) @@ -81,6 +84,8 @@ def get_track_data(album, track, tf): 'artistId': album.artist.pk, 'type': 'music', } + if track.album.cover: + data['coverArt'] = 'al-{}'.format(track.album.id) if tf.bitrate: data['bitrate'] = int(tf.bitrate/1000) if tf.size: @@ -98,6 +103,9 @@ def get_album2_data(album): 'artist': album.artist.name, 'created': album.creation_date, } + if album.cover: + payload['coverArt'] = 'al-{}'.format(album.id) + try: payload['songCount'] = album._tracks_count except AttributeError: @@ -221,3 +229,18 @@ def get_music_directory_data(artist): td['size'] = tf.size data['child'].append(td) return data + + +class ScrobbleSerializer(serializers.Serializer): + submission = serializers.BooleanField(default=True, required=False) + id = serializers.PrimaryKeyRelatedField( + queryset=music_models.Track.objects.annotate( + files_count=Count('files') + ).filter(files_count__gt=0) + ) + + def create(self, data): + return history_models.Listening.objects.create( + user=self.context['user'], + track=data['id'], + ) diff --git a/api/funkwhale_api/subsonic/views.py b/api/funkwhale_api/subsonic/views.py index 2692a3dda..cc75b5279 100644 --- a/api/funkwhale_api/subsonic/views.py +++ b/api/funkwhale_api/subsonic/views.py @@ -1,5 +1,6 @@ import datetime +from django.conf import settings from django.utils import timezone from rest_framework import exceptions @@ -10,6 +11,7 @@ from rest_framework import viewsets from rest_framework.decorators import list_route from rest_framework.serializers import ValidationError +from funkwhale_api.activity import record from funkwhale_api.common import preferences from funkwhale_api.favorites.models import TrackFavorite from funkwhale_api.music import models as music_models @@ -459,7 +461,7 @@ class SubsonicViewSet(viewsets.GenericViewSet): 'code': 10, 'message': 'Playlist ID or name must be specified.' } - }, data) + }) playlist = request.user.playlists.create( name=name @@ -503,3 +505,71 @@ class SubsonicViewSet(viewsets.GenericViewSet): } } return response.Response(data) + + @list_route( + methods=['get', 'post'], + url_name='get_cover_art', + url_path='getCoverArt') + def get_cover_art(self, request, *args, **kwargs): + data = request.GET or request.POST + id = data.get('id', '') + if not id: + return response.Response({ + 'error': { + 'code': 10, + 'message': 'cover art ID must be specified.' + } + }) + + if id.startswith('al-'): + try: + album_id = int(id.replace('al-', '')) + album = music_models.Album.objects.exclude( + cover__isnull=True + ).exclude(cover='').get(pk=album_id) + except (TypeError, ValueError, music_models.Album.DoesNotExist): + return response.Response({ + 'error': { + 'code': 70, + 'message': 'cover art not found.' + } + }) + cover = album.cover + else: + return response.Response({ + 'error': { + 'code': 70, + 'message': 'cover art not found.' + } + }) + + mapping = { + 'nginx': 'X-Accel-Redirect', + 'apache2': 'X-Sendfile', + } + path = music_views.get_file_path(cover) + file_header = mapping[settings.REVERSE_PROXY_TYPE] + # let the proxy set the content-type + r = response.Response({}, content_type='') + r[file_header] = path + return r + + @list_route( + methods=['get', 'post'], + url_name='scrobble', + url_path='scrobble') + def scrobble(self, request, *args, **kwargs): + data = request.GET or request.POST + serializer = serializers.ScrobbleSerializer( + data=data, context={'user': request.user}) + if not serializer.is_valid(): + return response.Response({ + 'error': { + 'code': 0, + 'message': 'Invalid payload' + } + }) + if serializer.validated_data['submission']: + l = serializer.save() + record.send(l) + return response.Response({}) diff --git a/api/funkwhale_api/users/admin.py b/api/funkwhale_api/users/admin.py index 7e9062a13..cb74abf0e 100644 --- a/api/funkwhale_api/users/admin.py +++ b/api/funkwhale_api/users/admin.py @@ -62,6 +62,7 @@ class UserAdmin(AuthUserAdmin): 'is_active', 'is_staff', 'is_superuser', + 'permission_upload', 'permission_library', 'permission_settings', 'permission_federation')}), diff --git a/api/funkwhale_api/users/dynamic_preferences_registry.py b/api/funkwhale_api/users/dynamic_preferences_registry.py index 4f7360530..7108360b9 100644 --- a/api/funkwhale_api/users/dynamic_preferences_registry.py +++ b/api/funkwhale_api/users/dynamic_preferences_registry.py @@ -1,6 +1,10 @@ from dynamic_preferences import types from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.common import preferences as common_preferences + +from . import models + users = types.Section('users') @@ -14,3 +18,23 @@ class RegistrationEnabled(types.BooleanPreference): help_text = ( 'When enabled, new users will be able to register on this instance.' ) + + +@global_preferences_registry.register +class DefaultPermissions(common_preferences.StringListPreference): + show_in_api = True + section = users + name = 'default_permissions' + default = [] + verbose_name = 'Default permissions' + help_text = ( + 'A list of default preferences to give to all registered users.' + ) + choices = [ + (k, c['label']) + for k, c in models.PERMISSIONS_CONFIGURATION.items() + ] + field_kwargs = { + 'choices': choices, + 'required': False, + } diff --git a/api/funkwhale_api/users/migrations/0007_auto_20180524_2009.py b/api/funkwhale_api/users/migrations/0007_auto_20180524_2009.py new file mode 100644 index 000000000..e3d582c53 --- /dev/null +++ b/api/funkwhale_api/users/migrations/0007_auto_20180524_2009.py @@ -0,0 +1,33 @@ +# Generated by Django 2.0.4 on 2018-05-24 20:09 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0006_auto_20180517_2324'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='permission_upload', + field=models.BooleanField(default=False, verbose_name='Upload new content to the library'), + ), + migrations.AlterField( + model_name='user', + name='permission_federation', + field=models.BooleanField(default=False, help_text='Follow other instances, accept/deny library follow requests...', verbose_name='Manage library federation'), + ), + migrations.AlterField( + model_name='user', + name='permission_library', + field=models.BooleanField(default=False, help_text='Manage library', verbose_name='Manage library'), + ), + migrations.AlterField( + model_name='user', + name='permission_settings', + field=models.BooleanField(default=False, verbose_name='Manage instance-level settings'), + ), + ] diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index c16cd62b3..fcf78d047 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -13,17 +13,33 @@ from django.utils.encoding import python_2_unicode_compatible from django.utils.translation import ugettext_lazy as _ from funkwhale_api.common import fields +from funkwhale_api.common import preferences def get_token(): return binascii.b2a_hex(os.urandom(15)).decode('utf-8') -PERMISSIONS = [ - 'federation', - 'library', - 'settings', -] +PERMISSIONS_CONFIGURATION = { + 'federation': { + 'label': 'Manage library federation', + 'help_text': 'Follow other instances, accept/deny library follow requests...', + }, + 'library': { + 'label': 'Manage library', + 'help_text': 'Manage library, delete files, tracks, artists, albums...', + }, + 'settings': { + 'label': 'Manage instance-level settings', + 'help_text': '', + }, + 'upload': { + 'label': 'Upload new content to the library', + 'help_text': '', + }, +} + +PERMISSIONS = sorted(PERMISSIONS_CONFIGURATION.keys()) @python_2_unicode_compatible @@ -47,30 +63,43 @@ class User(AbstractUser): # permissions permission_federation = models.BooleanField( - 'Manage library federation', - help_text='Follow other instances, accept/deny library follow requests...', + PERMISSIONS_CONFIGURATION['federation']['label'], + help_text=PERMISSIONS_CONFIGURATION['federation']['help_text'], default=False) permission_library = models.BooleanField( - 'Manage library', - help_text='Import new content, manage existing content', + PERMISSIONS_CONFIGURATION['library']['label'], + help_text=PERMISSIONS_CONFIGURATION['library']['help_text'], default=False) permission_settings = models.BooleanField( - 'Manage instance-level settings', + PERMISSIONS_CONFIGURATION['settings']['label'], + help_text=PERMISSIONS_CONFIGURATION['settings']['help_text'], + default=False) + permission_upload = models.BooleanField( + PERMISSIONS_CONFIGURATION['upload']['label'], + help_text=PERMISSIONS_CONFIGURATION['upload']['help_text'], default=False) def __str__(self): return self.username def get_permissions(self): + defaults = preferences.get('users__default_permissions') perms = {} for p in PERMISSIONS: - v = self.is_superuser or getattr(self, 'permission_{}'.format(p)) + v = ( + self.is_superuser or + getattr(self, 'permission_{}'.format(p)) or + p in defaults + ) perms[p] = v return perms - def has_permissions(self, *perms): + def has_permissions(self, *perms, operator='and'): + if operator not in ['and', 'or']: + raise ValueError('Invalid operator {}'.format(operator)) permissions = self.get_permissions() - return all([permissions[p] for p in perms]) + checker = all if operator == 'and' else any + return checker([permissions[p] for p in perms]) def get_absolute_url(self): return reverse('users:detail', kwargs={'username': self.username}) diff --git a/api/funkwhale_api/users/permissions.py b/api/funkwhale_api/users/permissions.py index 2ff49ff3f..146bc5e1c 100644 --- a/api/funkwhale_api/users/permissions.py +++ b/api/funkwhale_api/users/permissions.py @@ -16,4 +16,6 @@ class HasUserPermission(BasePermission): return False if request.user.is_anonymous: return False - return request.user.has_permissions(*view.required_permissions) + operator = getattr(view, 'permission_operator', 'and') + return request.user.has_permissions( + *view.required_permissions, operator=operator) diff --git a/api/requirements/base.txt b/api/requirements/base.txt index ac0586566..13c0efdbc 100644 --- a/api/requirements/base.txt +++ b/api/requirements/base.txt @@ -10,7 +10,7 @@ Pillow>=4.3,<4.4 # For user registration, either via email or social # Well-built with regular release cycles! -django-allauth>=0.34,<0.35 +django-allauth>=0.36,<0.37 # Python-PostgreSQL Database Adapter @@ -65,3 +65,4 @@ cryptography>=2,<3 # requests-http-signature==0.0.3 # clone until the branch is merged and released upstream git+https://github.com/EliotBerriot/requests-http-signature.git@signature-header-support +django-cleanup==2.1.0 diff --git a/api/tests/common/test_preferences.py b/api/tests/common/test_preferences.py new file mode 100644 index 000000000..475610a93 --- /dev/null +++ b/api/tests/common/test_preferences.py @@ -0,0 +1,44 @@ +import pytest + +from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.common import preferences as common_preferences + + +@pytest.fixture +def string_list_pref(preferences): + + @global_preferences_registry.register + class P(common_preferences.StringListPreference): + default = ['hello'] + section = 'test' + name = 'string_list' + yield + del global_preferences_registry['test']['string_list'] + + +@pytest.mark.parametrize('input,output', [ + (['a', 'b', 'c'], 'a,b,c'), + (['a', 'c', 'b'], 'a,b,c'), + (('a', 'c', 'b'), 'a,b,c'), + ([], None), +]) +def test_string_list_serializer_to_db(input, output): + s = common_preferences.StringListSerializer.to_db(input) == output + + +@pytest.mark.parametrize('input,output', [ + ('a,b,c', ['a', 'b', 'c'], ), + (None, []), + ('', []), +]) +def test_string_list_serializer_to_python(input, output): + s = common_preferences.StringListSerializer.to_python(input) == output + + +def test_string_list_pref_default(string_list_pref, preferences): + assert preferences['test__string_list'] == ['hello'] + + +def test_string_list_pref_set(string_list_pref, preferences): + preferences['test__string_list'] = ['world', 'hello'] + assert preferences['test__string_list'] == ['hello', 'world'] diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py new file mode 100644 index 000000000..f0f5fb7e6 --- /dev/null +++ b/api/tests/common/test_serializers.py @@ -0,0 +1,136 @@ +import django_filters + +from funkwhale_api.common import serializers +from funkwhale_api.users import models + + +class TestActionFilterSet(django_filters.FilterSet): + class Meta: + model = models.User + fields = ['is_active'] + + +class TestSerializer(serializers.ActionSerializer): + actions = ['test'] + filterset_class = TestActionFilterSet + + def handle_test(self, objects): + return {'hello': 'world'} + + +class TestDangerousSerializer(serializers.ActionSerializer): + actions = ['test', 'test_dangerous'] + dangerous_actions = ['test_dangerous'] + + def handle_test(self, objects): + pass + + def handle_test_dangerous(self, objects): + pass + + +def test_action_serializer_validates_action(): + data = {'objects': 'all', 'action': 'nope'} + serializer = TestSerializer(data, queryset=models.User.objects.none()) + + assert serializer.is_valid() is False + assert 'action' in serializer.errors + + +def test_action_serializer_validates_objects(): + data = {'objects': 'nope', 'action': 'test'} + serializer = TestSerializer(data, queryset=models.User.objects.none()) + + assert serializer.is_valid() is False + assert 'objects' in serializer.errors + + +def test_action_serializers_objects_clean_ids(factories): + user1 = factories['users.User']() + user2 = factories['users.User']() + + data = {'objects': [user1.pk], 'action': 'test'} + serializer = TestSerializer(data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is True + assert list(serializer.validated_data['objects']) == [user1] + + +def test_action_serializers_objects_clean_all(factories): + user1 = factories['users.User']() + user2 = factories['users.User']() + + data = {'objects': 'all', 'action': 'test'} + serializer = TestSerializer(data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is True + assert list(serializer.validated_data['objects']) == [user1, user2] + + +def test_action_serializers_save(factories, mocker): + handler = mocker.spy(TestSerializer, 'handle_test') + user1 = factories['users.User']() + user2 = factories['users.User']() + + data = {'objects': 'all', 'action': 'test'} + serializer = TestSerializer(data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is True + result = serializer.save() + assert result == { + 'updated': 2, + 'action': 'test', + 'result': {'hello': 'world'}, + } + handler.assert_called_once() + + +def test_action_serializers_filterset(factories): + user1 = factories['users.User'](is_active=False) + user2 = factories['users.User'](is_active=True) + + data = { + 'objects': 'all', + 'action': 'test', + 'filters': {'is_active': True}, + } + serializer = TestSerializer(data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is True + assert list(serializer.validated_data['objects']) == [user2] + + +def test_action_serializers_validates_at_least_one_object(): + data = { + 'objects': 'all', + 'action': 'test', + } + serializer = TestSerializer(data, queryset=models.User.objects.none()) + + assert serializer.is_valid() is False + assert 'non_field_errors' in serializer.errors + + +def test_dangerous_actions_refuses_all(factories): + factories['users.User']() + data = { + 'objects': 'all', + 'action': 'test_dangerous', + } + serializer = TestDangerousSerializer( + data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is False + assert 'non_field_errors' in serializer.errors + + +def test_dangerous_actions_refuses_not_listed(factories): + factories['users.User']() + data = { + 'objects': 'all', + 'action': 'test', + } + serializer = TestDangerousSerializer( + data, queryset=models.User.objects.all()) + + assert serializer.is_valid() is True diff --git a/api/tests/conftest.py b/api/tests/conftest.py index b7a7d071a..7caff2009 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 @@ -229,7 +231,21 @@ def authenticated_actor(factories, mocker): @pytest.fixture def assert_user_permission(): - def inner(view, permissions): + def inner(view, permissions, operator='and'): assert HasUserPermission in view.permission_classes + assert getattr(view, 'permission_operator', 'and') == operator 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/federation/test_serializers.py b/api/tests/federation/test_serializers.py index f298c61f5..fcf2ba1b6 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -699,3 +699,26 @@ def test_api_library_create_serializer_save(factories, r_mock): assert library.tracks_count == 10 assert library.actor == actor assert library.follow == follow + + +def test_tapi_library_track_serializer_not_imported(factories): + lt = factories['federation.LibraryTrack']() + serializer = serializers.APILibraryTrackSerializer(lt) + + assert serializer.get_status(lt) == 'not_imported' + + +def test_tapi_library_track_serializer_imported(factories): + tf = factories['music.TrackFile'](federation=True) + lt = tf.library_track + serializer = serializers.APILibraryTrackSerializer(lt) + + assert serializer.get_status(lt) == 'imported' + + +def test_tapi_library_track_serializer_import_pending(factories): + job = factories['music.ImportJob'](federation=True, status='pending') + lt = job.library_track + serializer = serializers.APILibraryTrackSerializer(lt) + + assert serializer.get_status(lt) == 'import_pending' diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index 10237ed9f..04a419aed 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -418,3 +418,39 @@ def test_can_filter_pending_follows(factories, superuser_api_client): assert response.status_code == 200 assert len(response.data['results']) == 0 + + +def test_library_track_action_import( + factories, superuser_api_client, mocker): + lt1 = factories['federation.LibraryTrack']() + lt2 = factories['federation.LibraryTrack'](library=lt1.library) + lt3 = factories['federation.LibraryTrack']() + lt4 = factories['federation.LibraryTrack'](library=lt3.library) + mocked_run = mocker.patch( + 'funkwhale_api.music.tasks.import_batch_run.delay') + + payload = { + 'objects': 'all', + 'action': 'import', + 'filters': { + 'library': lt1.library.uuid + } + } + url = reverse('api:v1:federation:library-tracks-action') + response = superuser_api_client.post(url, payload, format='json') + batch = superuser_api_client.user.imports.latest('id') + expected = { + 'updated': 2, + 'action': 'import', + 'result': { + 'batch': {'id': batch.pk} + } + } + + imported_lts = [lt1, lt2] + assert response.status_code == 200 + assert response.data == expected + assert batch.jobs.count() == 2 + for i, job in enumerate(batch.jobs.all()): + assert job.library_track == imported_lts[i] + mocked_run.assert_called_once_with(import_batch_id=batch.pk) diff --git a/api/tests/manage/__init__.py b/api/tests/manage/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/api/tests/manage/test_serializers.py b/api/tests/manage/test_serializers.py new file mode 100644 index 000000000..45167722c --- /dev/null +++ b/api/tests/manage/test_serializers.py @@ -0,0 +1,10 @@ +from funkwhale_api.manage import serializers + + +def test_manage_track_file_action_delete(factories): + tfs = factories['music.TrackFile'](size=5) + s = serializers.ManageTrackFileActionSerializer(queryset=None) + + s.handle_delete(tfs.__class__.objects.all()) + + assert tfs.__class__.objects.count() == 0 diff --git a/api/tests/manage/test_views.py b/api/tests/manage/test_views.py new file mode 100644 index 000000000..db2e0980a --- /dev/null +++ b/api/tests/manage/test_views.py @@ -0,0 +1,26 @@ +import pytest + +from django.urls import reverse + +from funkwhale_api.manage import serializers +from funkwhale_api.manage import views + + +@pytest.mark.parametrize('view,permissions,operator', [ + (views.ManageTrackFileViewSet, ['library'], 'and'), +]) +def test_permissions(assert_user_permission, view, permissions, operator): + assert_user_permission(view, permissions, operator) + + +def test_track_file_view(factories, superuser_api_client): + tfs = factories['music.TrackFile'].create_batch(size=5) + qs = tfs[0].__class__.objects.order_by('-creation_date') + url = reverse('api:v1:manage:library:track-files-list') + + response = superuser_api_client.get(url, {'sort': '-creation_date'}) + expected = serializers.ManageTrackFileSerializer( + qs, many=True, context={'request': response.wsgi_request}).data + + assert response.data['count'] == len(tfs) + assert response.data['results'] == expected diff --git a/api/tests/music/cover.jpg b/api/tests/music/cover.jpg new file mode 100644 index 000000000..71911bf48 Binary files /dev/null and b/api/tests/music/cover.jpg differ diff --git a/api/tests/music/cover.png b/api/tests/music/cover.png new file mode 100644 index 000000000..32f56c8ac Binary files /dev/null and b/api/tests/music/cover.png differ diff --git a/api/tests/music/sample.flac b/api/tests/music/sample.flac index 6eff1c06e..fe3ec6e4a 100644 Binary files a/api/tests/music/sample.flac and b/api/tests/music/sample.flac differ diff --git a/api/tests/music/test.mp3 b/api/tests/music/test.mp3 index 35a6e5fce..8502de71b 100644 Binary files a/api/tests/music/test.mp3 and b/api/tests/music/test.mp3 differ diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 53ee29f3e..7aa20e626 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_metadata.py b/api/tests/music/test_metadata.py index 3f1ea9177..326f18324 100644 --- a/api/tests/music/test_metadata.py +++ b/api/tests/music/test_metadata.py @@ -24,13 +24,29 @@ def test_can_get_metadata_from_ogg_file(field, value): assert data.get(field) == value +@pytest.mark.parametrize('field,value', [ + ('title', 'Drei Kreuze (dass wir hier sind)'), + ('artist', 'Die Toten Hosen'), + ('album', 'Ballast der Republik'), + ('date', datetime.date(2012, 5, 4)), + ('track_number', 1), + ('musicbrainz_albumid', uuid.UUID('1f0441ad-e609-446d-b355-809c445773cf')), + ('musicbrainz_recordingid', uuid.UUID('124d0150-8627-46bc-bc14-789a3bc960c8')), + ('musicbrainz_artistid', uuid.UUID('c3bc80a6-1f4a-4e17-8cf0-6b1efe8302f1')), +]) +def test_can_get_metadata_from_ogg_theora_file(field, value): + path = os.path.join(DATA_DIR, 'test_theora.ogg') + data = metadata.Metadata(path) + + assert data.get(field) == value + @pytest.mark.parametrize('field,value', [ ('title', 'Bend'), - ('artist', 'Binärpilot'), + ('artist', 'Bindrpilot'), ('album', 'You Can\'t Stop Da Funk'), ('date', datetime.date(2006, 2, 7)), - ('track_number', 1), + ('track_number', 2), ('musicbrainz_albumid', uuid.UUID('ce40cdb1-a562-4fd8-a269-9269f98d4124')), ('musicbrainz_recordingid', uuid.UUID('f269d497-1cc0-4ae4-a0c4-157ec7d73fcb')), ('musicbrainz_artistid', uuid.UUID('9c6bddde-6228-4d9f-ad0d-03f6fcb19e13')), @@ -42,6 +58,20 @@ def test_can_get_metadata_from_id3_mp3_file(field, value): assert data.get(field) == value +@pytest.mark.parametrize('name', ['test.mp3', 'sample.flac']) +def test_can_get_pictures(name): + path = os.path.join(DATA_DIR, name) + data = metadata.Metadata(path) + + pictures = data.get('pictures') + assert len(pictures) == 1 + cover_data = data.get_picture('cover_front') + assert cover_data['mimetype'].startswith('image/') + assert len(cover_data['content']) > 0 + assert type(cover_data['content']) == bytes + assert type(cover_data['description']) == str + + @pytest.mark.parametrize('field,value', [ ('title', '999,999'), ('artist', 'Nine Inch Nails'), @@ -57,3 +87,11 @@ def test_can_get_metadata_from_flac_file(field, value): data = metadata.Metadata(path) assert data.get(field) == value + + +def test_can_get_metadata_from_flac_file_not_crash_if_empty(): + path = os.path.join(DATA_DIR, 'sample.flac') + data = metadata.Metadata(path) + + with pytest.raises(metadata.TagNotFound): + data.get('test') diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py index e926d07fa..feb68ea33 100644 --- a/api/tests/music/test_models.py +++ b/api/tests/music/test_models.py @@ -110,3 +110,11 @@ def test_track_get_file_size_in_place(factories): in_place=True, source='file://{}'.format(path)) assert tf.get_file_size() == 297745 + + +def test_album_get_image_content(factories): + album = factories['music.Album']() + album.get_image(data={'content': b'test', 'mimetype':'image/jpeg'}) + album.refresh_from_db() + + assert album.cover.read() == b'test' diff --git a/api/tests/music/test_permissions.py b/api/tests/music/test_permissions.py index a5f0c4109..825d1731d 100644 --- a/api/tests/music/test_permissions.py +++ b/api/tests/music/test_permissions.py @@ -4,26 +4,17 @@ from funkwhale_api.federation import actors from funkwhale_api.music import permissions -def test_list_permission_no_protect(anonymous_user, api_request, settings): - settings.PROTECT_AUDIO_FILES = False +def test_list_permission_no_protect(preferences, anonymous_user, api_request): + preferences['common__api_authentication_required'] = False view = APIView.as_view() permission = permissions.Listen() request = api_request.get('/') assert permission.has_permission(request, view) is True -def test_list_permission_protect_anonymous( - db, anonymous_user, api_request, settings): - settings.PROTECT_AUDIO_FILES = True - view = APIView.as_view() - permission = permissions.Listen() - request = api_request.get('/') - assert permission.has_permission(request, view) is False - - def test_list_permission_protect_authenticated( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True user = factories['users.User']() view = APIView.as_view() permission = permissions.Listen() @@ -33,8 +24,8 @@ def test_list_permission_protect_authenticated( def test_list_permission_protect_not_following_actor( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True actor = factories['federation.Actor']() view = APIView.as_view() permission = permissions.Listen() @@ -44,8 +35,8 @@ def test_list_permission_protect_not_following_actor( def test_list_permission_protect_following_actor( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=True, target=library_actor) @@ -58,8 +49,8 @@ def test_list_permission_protect_following_actor( def test_list_permission_protect_following_actor_not_approved( - factories, api_request, settings): - settings.PROTECT_AUDIO_FILES = True + factories, api_request, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=False, target=library_actor) diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py new file mode 100644 index 000000000..fa22cecee --- /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_tasks.py b/api/tests/music/test_tasks.py index c5839432b..77245e204 100644 --- a/api/tests/music/test_tasks.py +++ b/api/tests/music/test_tasks.py @@ -47,6 +47,16 @@ def test_set_acoustid_on_track_file_required_high_score(factories, mocker): assert track_file.acoustid_track_id is None +def test_import_batch_run(factories, mocker): + job = factories['music.ImportJob']() + mocked_job_run = mocker.patch( + 'funkwhale_api.music.tasks.import_job_run.delay') + tasks.import_batch_run(import_batch_id=job.batch.pk) + + mocked_job_run.assert_called_once_with(import_job_id=job.pk) + + +@pytest.mark.skip('Acoustid is disabled') def test_import_job_can_run_with_file_and_acoustid( artists, albums, tracks, preferences, factories, mocker): preferences['providers_acoustid__api_key'] = 'test' @@ -105,7 +115,7 @@ def test_run_import_skipping_accoustid(factories, mocker): def test__do_import_skipping_accoustid(factories, mocker): t = factories['music.Track']() m = mocker.patch( - 'funkwhale_api.music.tasks.import_track_data_from_path', + 'funkwhale_api.providers.audiofile.tasks.import_track_data_from_path', return_value=t) path = os.path.join(DATA_DIR, 'test.ogg') job = factories['music.FileImportJob']( @@ -121,7 +131,7 @@ def test__do_import_skipping_accoustid_if_no_key( preferences['providers_acoustid__api_key'] = '' t = factories['music.Track']() m = mocker.patch( - 'funkwhale_api.music.tasks.import_track_data_from_path', + 'funkwhale_api.providers.audiofile.tasks.import_track_data_from_path', return_value=t) path = os.path.join(DATA_DIR, 'test.ogg') job = factories['music.FileImportJob']( @@ -132,32 +142,14 @@ def test__do_import_skipping_accoustid_if_no_key( m.assert_called_once_with(p) -def test_import_job_can_be_skipped( - artists, albums, tracks, factories, mocker, preferences): - preferences['providers_acoustid__api_key'] = 'test' +def test_import_job_skip_if_already_exists( + artists, albums, tracks, factories, mocker): path = os.path.join(DATA_DIR, 'test.ogg') mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' track_file = factories['music.TrackFile'](track__mbid=mbid) - acoustid_payload = { - 'results': [ - {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', - 'recordings': [ - { - 'duration': 268, - 'id': mbid}], - 'score': 0.860825}], - 'status': 'ok' - } mocker.patch( - 'funkwhale_api.musicbrainz.api.artists.get', - return_value=artists['get']['adhesive_wombat']) - mocker.patch( - 'funkwhale_api.musicbrainz.api.releases.get', - return_value=albums['get']['marsupial']) - mocker.patch( - 'funkwhale_api.musicbrainz.api.recordings.search', - return_value=tracks['search']['8bitadventures']) - mocker.patch('acoustid.match', return_value=acoustid_payload) + 'funkwhale_api.providers.audiofile.tasks.import_track_data_from_path', + return_value=track_file.track) job = factories['music.FileImportJob'](audio_file__path=path) f = job.audio_file @@ -171,30 +163,94 @@ def test_import_job_can_be_skipped( def test_import_job_can_be_errored(factories, mocker, preferences): - preferences['providers_acoustid__api_key'] = 'test' path = os.path.join(DATA_DIR, 'test.ogg') mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' track_file = factories['music.TrackFile'](track__mbid=mbid) - acoustid_payload = { - 'results': [ - {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', - 'recordings': [ - { - 'duration': 268, - 'id': mbid}], - 'score': 0.860825}], - 'status': 'ok' - } + class MyException(Exception): pass - mocker.patch('acoustid.match', side_effect=MyException()) + + mocker.patch( + 'funkwhale_api.music.tasks._do_import', + side_effect=MyException()) job = factories['music.FileImportJob']( audio_file__path=path, track_file=None) with pytest.raises(MyException): tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() assert job.track_file is None assert job.status == 'errored' + + +def test__do_import_calls_update_album_cover_if_no_cover(factories, mocker): + path = os.path.join(DATA_DIR, 'test.ogg') + album = factories['music.Album'](cover='') + track = factories['music.Track'](album=album) + + mocker.patch( + 'funkwhale_api.providers.audiofile.tasks.import_track_data_from_path', + return_value=track) + + mocked_update = mocker.patch( + 'funkwhale_api.music.tasks.update_album_cover') + + job = factories['music.FileImportJob']( + audio_file__path=path, track_file=None) + + tasks.import_job_run(import_job_id=job.pk) + + mocked_update.assert_called_once_with(album, track.files.first()) + + +def test_update_album_cover_mbid(factories, mocker): + album = factories['music.Album'](cover='') + + mocked_get = mocker.patch('funkwhale_api.music.models.Album.get_image') + tasks.update_album_cover(album=album, track_file=None) + + mocked_get.assert_called_once_with() + + +def test_update_album_cover_file_data(factories, mocker): + path = os.path.join(DATA_DIR, 'test.mp3') + album = factories['music.Album'](cover='', mbid=None) + tf = factories['music.TrackFile'](track__album=album) + + mocked_get = mocker.patch('funkwhale_api.music.models.Album.get_image') + mocker.patch( + 'funkwhale_api.music.metadata.Metadata.get_picture', + return_value={'hello': 'world'}) + tasks.update_album_cover(album=album, track_file=tf) + md = data = tf.get_metadata() + mocked_get.assert_called_once_with( + data={'hello': 'world'}) + + +@pytest.mark.parametrize('ext,mimetype', [ + ('jpg', 'image/jpeg'), + ('png', 'image/png'), +]) +def test_update_album_cover_file_cover_separate_file( + ext, mimetype, factories, mocker): + mocker.patch('funkwhale_api.music.tasks.IMAGE_TYPES', [(ext, mimetype)]) + path = os.path.join(DATA_DIR, 'test.mp3') + image_path = os.path.join(DATA_DIR, 'cover.{}'.format(ext)) + with open(image_path, 'rb') as f: + image_content = f.read() + album = factories['music.Album'](cover='', mbid=None) + tf = factories['music.TrackFile']( + track__album=album, + source='file://' + image_path) + + mocked_get = mocker.patch('funkwhale_api.music.models.Album.get_image') + mocker.patch( + 'funkwhale_api.music.metadata.Metadata.get_picture', + return_value=None) + tasks.update_album_cover(album=album, track_file=tf) + md = data = tf.get_metadata() + mocked_get.assert_called_once_with( + data={'mimetype': mimetype, 'content': image_content}) diff --git a/api/tests/music/test_theora.ogg b/api/tests/music/test_theora.ogg new file mode 100644 index 000000000..3aa711738 Binary files /dev/null and b/api/tests/music/test_theora.ogg differ diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 030fc3a73..91fef13f2 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -4,16 +4,76 @@ 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 -@pytest.mark.parametrize('view,permissions', [ - (views.ImportBatchViewSet, ['library']), - (views.ImportJobViewSet, ['library']), +@pytest.mark.parametrize('view,permissions,operator', [ + (views.ImportBatchViewSet, ['library', 'upload'], 'or'), + (views.ImportJobViewSet, ['library', 'upload'], 'or'), ]) -def test_permissions(assert_user_permission, view, permissions): - assert_user_permission(view, permissions) +def test_permissions(assert_user_permission, view, permissions, operator): + assert_user_permission(view, permissions, operator) + + +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', [ @@ -59,8 +119,8 @@ def test_album_view_filter_listenable( def test_can_serve_track_file_as_remote_library( - factories, authenticated_actor, settings, api_client): - settings.PROTECT_AUDIO_FILES = True + factories, authenticated_actor, api_client, settings, preferences): + preferences['common__api_authentication_required'] = True library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() follow = factories['federation.Follow']( approved=True, @@ -77,8 +137,8 @@ def test_can_serve_track_file_as_remote_library( def test_can_serve_track_file_as_remote_library_deny_not_following( - factories, authenticated_actor, settings, api_client): - settings.PROTECT_AUDIO_FILES = True + factories, authenticated_actor, settings, api_client, preferences): + preferences['common__api_authentication_required'] = True track_file = factories['music.TrackFile']() response = api_client.get(track_file.path) @@ -92,12 +152,18 @@ def test_can_serve_track_file_as_remote_library_deny_not_following( ('nginx', '/app/music', '/_protected/music/hello/world.mp3'), ]) def test_serve_file_in_place( - proxy, serve_path, expected, factories, api_client, settings): + proxy, + serve_path, + expected, + factories, + api_client, + preferences, + settings): headers = { 'apache2': 'X-Sendfile', 'nginx': 'X-Accel-Redirect', } - settings.PROTECT_AUDIO_FILES = False + preferences['common__api_authentication_required'] = False settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy settings.MUSIC_DIRECTORY_PATH = '/app/music' @@ -119,8 +185,14 @@ def test_serve_file_in_place( ('nginx', '/app/music', '/_protected/music/hello/worldéà.mp3'), ]) def test_serve_file_in_place_utf8( - proxy, serve_path, expected, factories, api_client, settings): - settings.PROTECT_AUDIO_FILES = False + proxy, + serve_path, + expected, + factories, + api_client, + settings, + preferences): + preferences['common__api_authentication_required'] = False settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy settings.MUSIC_DIRECTORY_PATH = '/app/music' @@ -138,12 +210,18 @@ def test_serve_file_in_place_utf8( ('nginx', '/app/music', '/_protected/media/tracks/hello/world.mp3'), ]) def test_serve_file_media( - proxy, serve_path, expected, factories, api_client, settings): + proxy, + serve_path, + expected, + factories, + api_client, + settings, + preferences): headers = { 'apache2': 'X-Sendfile', 'nginx': 'X-Accel-Redirect', } - settings.PROTECT_AUDIO_FILES = False + preferences['common__api_authentication_required'] = False settings.MEDIA_ROOT = '/host/media' settings.PROTECT_FILE_PATH = '/_protected/music' settings.REVERSE_PROXY_TYPE = proxy @@ -160,8 +238,8 @@ def test_serve_file_media( def test_can_proxy_remote_track( - factories, settings, api_client, r_mock): - settings.PROTECT_AUDIO_FILES = False + factories, settings, api_client, r_mock, preferences): + preferences['common__api_authentication_required'] = False track_file = factories['music.TrackFile'](federation=True) r_mock.get(track_file.library_track.audio_url, body=io.BytesIO(b'test')) @@ -176,8 +254,9 @@ def test_can_proxy_remote_track( assert library_track.audio_file.read() == b'test' -def test_serve_updates_access_date(factories, settings, api_client): - settings.PROTECT_AUDIO_FILES = False +def test_serve_updates_access_date( + factories, settings, api_client, preferences): + preferences['common__api_authentication_required'] = False track_file = factories['music.TrackFile']() now = timezone.now() assert track_file.accessed_date is None @@ -189,24 +268,6 @@ def test_serve_updates_access_date(factories, settings, api_client): assert track_file.accessed_date > now -def test_can_create_import_from_federation_tracks( - factories, superuser_api_client, mocker): - lts = factories['federation.LibraryTrack'].create_batch(size=5) - mocker.patch('funkwhale_api.music.tasks.import_job_run') - - payload = { - 'library_tracks': [l.pk for l in lts] - } - url = reverse('api:v1:submit-federation') - response = superuser_api_client.post(url, payload) - - assert response.status_code == 201 - batch = superuser_api_client.user.imports.latest('id') - assert batch.jobs.count() == 5 - for i, job in enumerate(batch.jobs.all()): - assert job.library_track == lts[i] - - def test_can_list_import_jobs(factories, superuser_api_client): job = factories['music.ImportJob']() url = reverse('api:v1:import-jobs-list') @@ -309,3 +370,27 @@ def test_import_batch_and_job_run_via_api( run.assert_any_call(import_job_id=job1.pk) run.assert_any_call(import_job_id=job2.pk) + + +def test_import_job_viewset_get_queryset_upload_filters_user( + factories, logged_in_api_client): + logged_in_api_client.user.permission_upload = True + logged_in_api_client.user.save() + + job = factories['music.ImportJob']() + url = reverse('api:v1:import-jobs-list') + response = logged_in_api_client.get(url) + + assert response.data['count'] == 0 + + +def test_import_batch_viewset_get_queryset_upload_filters_user( + factories, logged_in_api_client): + logged_in_api_client.user.permission_upload = True + logged_in_api_client.user.save() + + job = factories['music.ImportBatch']() + url = reverse('api:v1:import-batches-list') + response = logged_in_api_client.get(url) + + assert response.data['count'] == 0 diff --git a/api/tests/radios/test_api.py b/api/tests/radios/test_api.py index 25c099014..66bf6052d 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/api/tests/subsonic/test_authentication.py b/api/tests/subsonic/test_authentication.py index 724513523..656f8c44d 100644 --- a/api/tests/subsonic/test_authentication.py +++ b/api/tests/subsonic/test_authentication.py @@ -1,4 +1,7 @@ import binascii +import pytest + +from rest_framework import exceptions from funkwhale_api.subsonic import authentication @@ -54,3 +57,19 @@ def test_auth_with_password_cleartext(api_request, factories): u, _ = authenticator.authenticate(request) assert user == u + + +def test_auth_with_inactive_users(api_request, factories): + salt = 'salt' + user = factories['users.User'](is_active=False) + user.subsonic_api_token = 'password' + user.save() + token = authentication.get_token(salt, 'password') + request = api_request.get('/', { + 'u': user.username, + 'p': 'password', + }) + + authenticator = authentication.SubsonicAuthentication() + with pytest.raises(exceptions.AuthenticationFailed): + authenticator.authenticate(request) diff --git a/api/tests/subsonic/test_serializers.py b/api/tests/subsonic/test_serializers.py index ad9f739a1..6b9ec232d 100644 --- a/api/tests/subsonic/test_serializers.py +++ b/api/tests/subsonic/test_serializers.py @@ -60,6 +60,7 @@ def test_get_artist_serializer(factories): 'album': [ { 'id': album.pk, + 'coverArt': 'al-{}'.format(album.id), 'artistId': artist.pk, 'name': album.title, 'artist': artist.name, @@ -88,11 +89,13 @@ def test_get_album_serializer(factories): 'songCount': 1, 'created': album.creation_date, 'year': album.release_date.year, + 'coverArt': 'al-{}'.format(album.id), 'song': [ { 'id': track.pk, 'isDir': 'false', 'title': track.title, + 'coverArt': 'al-{}'.format(album.id), 'album': album.title, 'artist': artist.name, 'track': track.position, @@ -211,3 +214,22 @@ def test_directory_serializer_artist(factories): } data = serializers.get_music_directory_data(artist) assert data == expected + + +def test_scrobble_serializer(factories): + tf = factories['music.TrackFile']() + track = tf.track + user = factories['users.User']() + payload = { + 'id': track.pk, + 'submission': True, + } + serializer = serializers.ScrobbleSerializer( + data=payload, context={'user': user}) + + assert serializer.is_valid(raise_exception=True) + + listening = serializer.save() + + assert listening.user == user + assert listening.track == track diff --git a/api/tests/subsonic/test_views.py b/api/tests/subsonic/test_views.py index bd445e070..52e410e52 100644 --- a/api/tests/subsonic/test_views.py +++ b/api/tests/subsonic/test_views.py @@ -391,3 +391,30 @@ def test_get_indexes(f, db, logged_in_api_client, factories): assert response.status_code == 200 assert response.data == expected + + +def test_get_cover_art_album(factories, logged_in_api_client): + url = reverse('api:subsonic-get-cover-art') + assert url.endswith('getCoverArt') is True + album = factories['music.Album']() + response = logged_in_api_client.get(url, {'id': 'al-{}'.format(album.pk)}) + + assert response.status_code == 200 + assert response['Content-Type'] == '' + assert response['X-Accel-Redirect'] == music_views.get_file_path( + album.cover + ).decode('utf-8') + + +def test_scrobble(factories, logged_in_api_client): + tf = factories['music.TrackFile']() + track = tf.track + url = reverse('api:subsonic-scrobble') + assert url.endswith('scrobble') is True + response = logged_in_api_client.get( + url, {'id': track.pk, 'submission': True}) + + assert response.status_code == 200 + + l = logged_in_api_client.user.listenings.latest('id') + assert l.track == track diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index 8217ffa0b..de8186075 100644 --- a/api/tests/test_import_audio_file.py +++ b/api/tests/test_import_audio_file.py @@ -1,5 +1,4 @@ import pytest -import acoustid import datetime import os import uuid @@ -17,8 +16,6 @@ DATA_DIR = os.path.join( def test_can_create_track_from_file_metadata(db, mocker): - mocker.patch( - 'acoustid.match', side_effect=acoustid.WebServiceError('test')) metadata = { 'artist': ['Test artist'], 'album': ['Test album'], @@ -94,24 +91,6 @@ def test_import_files_creates_a_batch_and_job(factories, mocker): assert job.audio_file.read() == f.read() assert job.source == 'file://' + path - m.assert_called_once_with( - import_job_id=job.pk, - use_acoustid=True) - - -def test_import_files_skip_acoustid(factories, mocker): - m = mocker.patch('funkwhale_api.music.tasks.import_job_run') - user = factories['users.User'](username='me') - path = os.path.join(DATA_DIR, 'dummy_file.ogg') - call_command( - 'import_files', - path, - username='me', - async=False, - no_acoustid=True, - interactive=False) - batch = user.imports.latest('id') - job = batch.jobs.first() m.assert_called_once_with( import_job_id=job.pk, use_acoustid=False) @@ -128,7 +107,6 @@ def test_import_files_skip_if_path_already_imported(factories, mocker): path, username='me', async=False, - no_acoustid=True, interactive=False) assert user.imports.count() == 0 @@ -142,7 +120,6 @@ def test_import_files_works_with_utf8_file_name(factories, mocker): path, username='me', async=False, - no_acoustid=True, interactive=False) batch = user.imports.latest('id') job = batch.jobs.first() @@ -162,7 +139,6 @@ def test_import_files_in_place(factories, mocker, settings): username='me', async=False, in_place=True, - no_acoustid=True, interactive=False) batch = user.imports.latest('id') job = batch.jobs.first() diff --git a/api/tests/users/test_models.py b/api/tests/users/test_models.py index 49199e0a7..42123b5e8 100644 --- a/api/tests/users/test_models.py +++ b/api/tests/users/test_models.py @@ -41,12 +41,34 @@ def test_get_permissions_regular(factories): assert perms[p] is False +def test_get_permissions_default(factories, preferences): + preferences['users__default_permissions'] = ['upload', 'federation'] + user = factories['users.User']() + + perms = user.get_permissions() + assert perms['upload'] is True + assert perms['federation'] is True + assert perms['library'] is False + assert perms['settings'] is False + + @pytest.mark.parametrize('args,perms,expected', [ ({'is_superuser': True}, ['federation', 'library'], True), ({'is_superuser': False}, ['federation'], False), ({'permission_library': True}, ['library'], True), ({'permission_library': True}, ['library', 'federation'], False), ]) -def test_has_permissions(args, perms, expected, factories): +def test_has_permissions_and(args, perms, expected, factories): user = factories['users.User'](**args) - assert user.has_permissions(*perms) is expected + assert user.has_permissions(*perms, operator='and') is expected + + +@pytest.mark.parametrize('args,perms,expected', [ + ({'is_superuser': True}, ['federation', 'library'], True), + ({'is_superuser': False}, ['federation'], False), + ({'permission_library': True}, ['library', 'federation'], True), + ({'permission_library': True}, ['federation'], False), +]) +def test_has_permissions_or(args, perms, expected, factories): + user = factories['users.User'](**args) + assert user.has_permissions(*perms, operator='or') is expected diff --git a/api/tests/users/test_permissions.py b/api/tests/users/test_permissions.py index 1564c761d..518ccd1c8 100644 --- a/api/tests/users/test_permissions.py +++ b/api/tests/users/test_permissions.py @@ -39,7 +39,7 @@ def test_has_user_permission_logged_in_single(value, factories, api_request): (False, False, False), (True, True, True), ]) -def test_has_user_permission_logged_in_single( +def test_has_user_permission_logged_in_multiple_and( federation, library, expected, factories, api_request): user = factories['users.User']( permission_federation=federation, @@ -48,9 +48,35 @@ def test_has_user_permission_logged_in_single( class View(APIView): required_permissions = ['federation', 'library'] + permission_operator = 'and' view = View() permission = permissions.HasUserPermission() request = api_request.get('/') setattr(request, 'user', user) result = permission.has_permission(request, view) assert result == user.has_permissions('federation', 'library') == expected + + +@pytest.mark.parametrize('federation,library,expected', [ + (True, False, True), + (False, True, True), + (False, False, False), + (True, True, True), +]) +def test_has_user_permission_logged_in_multiple_or( + federation, library, expected, factories, api_request): + user = factories['users.User']( + permission_federation=federation, + permission_library=library, + ) + + class View(APIView): + required_permissions = ['federation', 'library'] + permission_operator = 'or' + view = View() + permission = permissions.HasUserPermission() + request = api_request.get('/') + setattr(request, 'user', user) + result = permission.has_permission(request, view) + assert result == user.has_permissions( + 'federation', 'library', operator='or') == expected diff --git a/api/tests/users/test_views.py b/api/tests/users/test_views.py index 1bbf8b9a2..6418889ce 100644 --- a/api/tests/users/test_views.py +++ b/api/tests/users/test_views.py @@ -7,7 +7,7 @@ from django.urls import reverse from funkwhale_api.users.models import User -def test_can_create_user_via_api(preferences, client, db): +def test_can_create_user_via_api(preferences, api_client, db): url = reverse('rest_register') data = { 'username': 'test1', @@ -16,14 +16,14 @@ def test_can_create_user_via_api(preferences, client, db): 'password2': 'testtest', } preferences['users__registration_enabled'] = True - response = client.post(url, data) + response = api_client.post(url, data) assert response.status_code == 201 u = User.objects.get(email='test1@test.com') assert u.username == 'test1' -def test_can_restrict_usernames(settings, preferences, db, client): +def test_can_restrict_usernames(settings, preferences, db, api_client): url = reverse('rest_register') preferences['users__registration_enabled'] = True settings.USERNAME_BLACKLIST = ['funkwhale'] @@ -34,13 +34,13 @@ def test_can_restrict_usernames(settings, preferences, db, client): 'password2': 'testtest', } - response = client.post(url, data) + response = api_client.post(url, data) assert response.status_code == 400 assert 'username' in response.data -def test_can_disable_registration_view(preferences, client, db): +def test_can_disable_registration_view(preferences, api_client, db): url = reverse('rest_register') data = { 'username': 'test1', @@ -49,7 +49,7 @@ def test_can_disable_registration_view(preferences, client, db): 'password2': 'testtest', } preferences['users__registration_enabled'] = False - response = client.post(url, data) + response = api_client.post(url, data) assert response.status_code == 403 @@ -73,7 +73,7 @@ def test_can_fetch_data_from_api(api_client, factories): assert response.data['permissions'] == user.get_permissions() -def test_can_get_token_via_api(client, factories): +def test_can_get_token_via_api(api_client, factories): user = factories['users.User']() url = reverse('api:v1:token') payload = { @@ -81,12 +81,24 @@ def test_can_get_token_via_api(client, factories): 'password': 'test' } - response = client.post(url, payload) + response = api_client.post(url, payload) assert response.status_code == 200 - assert '"token":' in response.content.decode('utf-8') + assert 'token' in response.data -def test_can_refresh_token_via_api(client, factories): +def test_can_get_token_via_api_inactive(api_client, factories): + user = factories['users.User'](is_active=False) + url = reverse('api:v1:token') + payload = { + 'username': user.username, + 'password': 'test' + } + + response = api_client.post(url, payload) + assert response.status_code == 400 + + +def test_can_refresh_token_via_api(api_client, factories, mocker): # first, we get a token user = factories['users.User']() url = reverse('api:v1:token') @@ -95,21 +107,19 @@ def test_can_refresh_token_via_api(client, factories): 'password': 'test' } - response = client.post(url, payload) + response = api_client.post(url, payload) assert response.status_code == 200 - token = json.loads(response.content.decode('utf-8'))['token'] + token = response.data['token'] url = reverse('api:v1:token_refresh') - response = client.post(url,{'token': token}) + response = api_client.post(url, {'token': token}) assert response.status_code == 200 - assert '"token":' in response.content.decode('utf-8') - # a different token should be returned - assert token in response.content.decode('utf-8') + assert 'token' in response.data -def test_changing_password_updates_secret_key(logged_in_client): - user = logged_in_client.user +def test_changing_password_updates_secret_key(logged_in_api_client): + user = logged_in_api_client.user password = user.password secret_key = user.secret_key payload = { @@ -119,7 +129,7 @@ def test_changing_password_updates_secret_key(logged_in_client): } url = reverse('change_password') - response = logged_in_client.post(url, payload) + response = logged_in_api_client.post(url, payload) user.refresh_from_db() diff --git a/demo/setup.sh b/demo/setup.sh index b96f517b3..e33bdf290 100644 --- a/demo/setup.sh +++ b/demo/setup.sh @@ -23,8 +23,6 @@ echo "DJANGO_SECRET_KEY=demo" >> .env echo "DJANGO_ALLOWED_HOSTS=demo.funkwhale.audio" >> .env echo "FUNKWHALE_VERSION=$version" >> .env echo "FUNKWHALE_API_PORT=5001" >> .env -echo "FEDERATION_MUSIC_NEEDS_APPROVAL=False" >>.env -echo "PROTECT_AUDIO_FILES=False" >> .env /usr/local/bin/docker-compose pull /usr/local/bin/docker-compose up -d postgres redis sleep 5 diff --git a/deploy/apache.conf b/deploy/apache.conf index 5bfcbce04..5b74efecd 100644 --- a/deploy/apache.conf +++ b/deploy/apache.conf @@ -84,6 +84,12 @@ Define MUSIC_DIRECTORY_PATH /srv/funkwhale/data/music ProxyPassReverse ${funkwhale-api}/federation + # You can comment this if you don't plan to use the Subsonic API + + ProxyPass ${funkwhale-api}/api/subsonic/rest + ProxyPassReverse ${funkwhale-api}/api/subsonic/rest + + ProxyPass ${funkwhale-api}/.well-known/ ProxyPassReverse ${funkwhale-api}/.well-known/ diff --git a/deploy/nginx.conf b/deploy/nginx.conf index 7d344408b..5314d9017 100644 --- a/deploy/nginx.conf +++ b/deploy/nginx.conf @@ -67,6 +67,12 @@ server { proxy_pass http://funkwhale-api/federation/; } + # You can comment this if you do not plan to use the Subsonic API + location /rest/ { + include /etc/nginx/funkwhale_proxy.conf; + proxy_pass http://funkwhale-api/api/subsonic/rest/; + } + location /.well-known/ { include /etc/nginx/funkwhale_proxy.conf; proxy_pass http://funkwhale-api/.well-known/; diff --git a/dev.yml b/dev.yml index e85ce3b91..5dccfeca3 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/docker/nginx/conf.dev b/docker/nginx/conf.dev index ab6714e60..673edd1a4 100644 --- a/docker/nginx/conf.dev +++ b/docker/nginx/conf.dev @@ -36,7 +36,7 @@ http { server { listen 6001; charset utf-8; - client_max_body_size 20M; + client_max_body_size 30M; include /etc/nginx/funkwhale_proxy.conf; location /_protected/media { internal; diff --git a/docker/nginx/entrypoint.sh b/docker/nginx/entrypoint.sh index 14e072a7e..ea6a3322d 100755 --- a/docker/nginx/entrypoint.sh +++ b/docker/nginx/entrypoint.sh @@ -12,6 +12,7 @@ cp /etc/nginx/funkwhale_proxy.conf{.template,} sed -i "s/X-Forwarded-Host \$host:\$server_port/X-Forwarded-Host ${FUNKWHALE_HOSTNAME}:${FORWARDED_PORT}/" /etc/nginx/funkwhale_proxy.conf sed -i "s/proxy_set_header Host \$host/proxy_set_header Host ${FUNKWHALE_HOSTNAME}/" /etc/nginx/funkwhale_proxy.conf sed -i "s/proxy_set_header X-Forwarded-Port \$server_port/proxy_set_header X-Forwarded-Port ${FORWARDED_PORT}/" /etc/nginx/funkwhale_proxy.conf +sed -i "s/proxy_set_header X-Forwarded-Proto \$scheme/proxy_set_header X-Forwarded-Proto ${FORWARDED_PROTO}/" /etc/nginx/funkwhale_proxy.conf cat /etc/nginx/funkwhale_proxy.conf nginx -g "daemon off;" diff --git a/docs/importing-music.rst b/docs/importing-music.rst index 97dd13854..b190dff36 100644 --- a/docs/importing-music.rst +++ b/docs/importing-music.rst @@ -6,7 +6,8 @@ From music directory on the server You can import music files in funkwhale assuming they are located on the server and readable by the funkwhale application. Your music files should contain at -least an ``artist``, ``album`` and ``title`` tags. +least an ``artist``, ``album`` and ``title`` tags, but we recommend you tag +it extensively using a proper tool, such as Beets or Musicbrainz Picard. You can import those tracks as follows, assuming they are located in ``/srv/funkwhale/data/music``: @@ -32,11 +33,6 @@ get details:: For the best results, we recommand tagging your music collection through `Picard `_ in order to have the best quality metadata. -.. note:: - - Autotagging using acoustid is experimental now and can yield unexpected - result. You can disable acoustid by passing the --no-acoustid flag. - .. note:: This command is idempotent, meaning you can run it multiple times on the same @@ -44,7 +40,7 @@ get details:: .. note:: - At the moment, only OGG/Vorbis and MP3 files with ID3 tags are supported + At the moment, only Flac, OGG/Vorbis and MP3 files with ID3 tags are supported .. _in-place-import: @@ -80,6 +76,15 @@ configuration options to ensure the webserver can serve them properly: Thus, be especially careful when you manipulate the source files. +Album covers +^^^^^^^^^^^^ + +Whenever possible, Funkwhale will import album cover, with the following precedence: + +1. It will use the cover embedded in the audio files themeselves, if any (Flac/MP3 only) +2. It will use a cover.jpg or a cover.png file from the imported track directory, if any +3. It will fectch cover art from musicbrainz, assuming the file is tagged correctly + Getting demo tracks ^^^^^^^^^^^^^^^^^^^ diff --git a/docs/index.rst b/docs/index.rst index 01f76d3cc..f177a4240 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -14,11 +14,11 @@ Funkwhale is a self-hosted, modern free and open-source music server, heavily in users/index features installation/index + upgrading configuration importing-music federation api - upgrading third-party contributing changelog diff --git a/docs/swagger.yml b/docs/swagger.yml index 7735a8f20..71c74e442 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/package.json b/front/package.json index 8844e8bee..3dec9c257 100644 --- a/front/package.json +++ b/front/package.json @@ -33,7 +33,7 @@ "raven-js": "^3.22.3", "semantic-ui-css": "^2.2.10", "showdown": "^1.8.6", - "vue": "^2.3.3", + "vue": "^2.5.16", "vue-lazyload": "^1.1.4", "vue-masonry": "^0.10.16", "vue-router": "^2.3.1", diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue index 9f3134c2a..72c55847f 100644 --- a/front/src/components/Sidebar.vue +++ b/front/src/components/Sidebar.vue @@ -68,6 +68,18 @@ :title="$t('Pending import requests')"> {{ notifications.importRequests }} + + {{ $t('Library') }} + + + {{ $t('Import music') }} + { return e diff --git a/front/src/components/admin/SettingsGroup.vue b/front/src/components/admin/SettingsGroup.vue index 255f04488..f6d57c239 100644 --- a/front/src/components/admin/SettingsGroup.vue +++ b/front/src/components/admin/SettingsGroup.vue @@ -50,6 +50,13 @@

{{ setting.help_text }}

+ - + @@ -30,10 +30,13 @@ -
+
+
+
+

-
+
@@ -43,7 +46,6 @@ diff --git a/front/src/components/library/Radios.vue b/front/src/components/library/Radios.vue index 9fcadf0a6..794e3a13b 100644 --- a/front/src/components/library/Radios.vue +++ b/front/src/components/library/Radios.vue @@ -23,7 +23,7 @@
@@ -99,7 +99,7 @@ export default { page: parseInt(this.defaultPage), query: this.defaultQuery, paginateBy: parseInt(this.defaultPaginateBy || 12), - orderingDirection: defaultOrdering.direction, + orderingDirection: defaultOrdering.direction || '+', ordering: defaultOrdering.field, orderingOptions: [ ['creation_date', 'Creation date'], diff --git a/front/src/components/library/Track.vue b/front/src/components/library/Track.vue index 155a1245a..24acca75b 100644 --- a/front/src/components/library/Track.vue +++ b/front/src/components/library/Track.vue @@ -14,8 +14,7 @@ {{ track.album.title }} - - + {{ track.artist.name }} diff --git a/front/src/components/library/import/FileUpload.vue b/front/src/components/library/import/FileUpload.vue index 9a4b820e3..7aa8adac0 100644 --- a/front/src/components/library/import/FileUpload.vue +++ b/front/src/components/library/import/FileUpload.vue @@ -1,6 +1,10 @@