From 1c8f055490bcbe769efa6a717e4463aea9eed47c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 27 Dec 2017 23:32:02 +0100 Subject: [PATCH] Brand new file importer --- CHANGELOG | 10 +- api/config/api_urls.py | 1 + api/funkwhale_api/common/permissions.py | 13 +- api/funkwhale_api/music/models.py | 5 +- api/funkwhale_api/music/serializers.py | 4 +- api/funkwhale_api/music/tasks.py | 1 + api/funkwhale_api/music/views.py | 36 ++++- api/tests/music/test_api.py | 45 ++++++ deploy/nginx.conf | 2 + docker/nginx/conf.dev | 2 +- front/package.json | 1 + .../components/library/import/BatchDetail.vue | 5 +- .../components/library/import/BatchList.vue | 2 +- .../components/library/import/FileUpload.vue | 140 ++++++++++++++++++ .../library/import/FileUploadWidget.vue | 34 +++++ front/src/components/library/import/Main.vue | 16 +- 16 files changed, 302 insertions(+), 15 deletions(-) create mode 100644 front/src/components/library/import/FileUpload.vue create mode 100644 front/src/components/library/import/FileUploadWidget.vue diff --git a/CHANGELOG b/CHANGELOG index fbf8d1fbf..162246f75 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,15 @@ Changelog ========= -0.2.7 (Unreleased) +0.3 (Unreleased) +------------------ + +- Revamped all import logic, everything is more tested and consistend +- Can now use Acoustid in file imports to automatically grab metadata from musicbrainz +- Brand new file import wizard + + +0.2.7 ------------------ - Shortcuts: can now use the ``f`` shortcut to toggle the currently playing track diff --git a/api/config/api_urls.py b/api/config/api_urls.py index a41c7bc0f..d64eeb5fd 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -15,6 +15,7 @@ router.register(r'trackfiles', views.TrackFileViewSet, 'trackfiles') router.register(r'artists', views.ArtistViewSet, 'artists') router.register(r'albums', views.AlbumViewSet, 'albums') router.register(r'import-batches', views.ImportBatchViewSet, 'import-batches') +router.register(r'import-jobs', views.ImportJobViewSet, 'import-jobs') router.register(r'submit', views.SubmitViewSet, 'submit') router.register(r'playlists', playlists_views.PlaylistViewSet, 'playlists') router.register( diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py index 6b61d9839..ecfea4c9a 100644 --- a/api/funkwhale_api/common/permissions.py +++ b/api/funkwhale_api/common/permissions.py @@ -1,6 +1,6 @@ from django.conf import settings -from rest_framework.permissions import BasePermission +from rest_framework.permissions import BasePermission, DjangoModelPermissions class ConditionalAuthentication(BasePermission): @@ -9,3 +9,14 @@ class ConditionalAuthentication(BasePermission): if settings.API_AUTHENTICATION_REQUIRED: return request.user and request.user.is_authenticated return True + + +class HasModelPermission(DjangoModelPermissions): + """ + Same as DjangoModelPermissions, but we pin the model: + + class MyModelPermission(HasModelPermission): + model = User + """ + def get_required_permissions(self, method, model_cls): + return super().get_required_permissions(method, self.model) diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index fdb88c336..f919ff0eb 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -405,8 +405,11 @@ class ImportBatch(models.Model): @property def status(self): pending = any([job.status == 'pending' for job in self.jobs.all()]) + errored = any([job.status == 'errored' for job in self.jobs.all()]) if pending: return 'pending' + if errored: + return 'errored' return 'finished' class ImportJob(models.Model): @@ -418,7 +421,7 @@ class ImportJob(models.Model): null=True, blank=True, on_delete=models.CASCADE) - source = models.URLField() + source = models.CharField(max_length=500) mbid = models.UUIDField(editable=False, null=True, blank=True) STATUS_CHOICES = ( ('pending', 'Pending'), diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index cf9d87490..9f96dad0e 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -113,7 +113,8 @@ class ImportJobSerializer(serializers.ModelSerializer): track_file = TrackFileSerializer(read_only=True) class Meta: model = models.ImportJob - fields = ('id', 'mbid', 'source', 'status', 'track_file') + fields = ('id', 'mbid', 'batch', 'source', 'status', 'track_file', 'audio_file') + read_only_fields = ('status', 'track_file') class ImportBatchSerializer(serializers.ModelSerializer): @@ -121,3 +122,4 @@ class ImportBatchSerializer(serializers.ModelSerializer): class Meta: model = models.ImportBatch fields = ('id', 'jobs', 'status', 'creation_date') + read_only_fields = ('creation_date',) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 009f00680..fc706c812 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -2,6 +2,7 @@ from django.core.files.base import ContentFile 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 django.conf import settings from . import models diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index d7152ff22..dd295ae79 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -6,7 +6,7 @@ from django.urls import reverse from django.db import models, transaction from django.db.models.functions import Length from django.conf import settings -from rest_framework import viewsets, views +from rest_framework import viewsets, views, mixins from rest_framework.decorators import detail_route, list_route from rest_framework.response import Response from rest_framework import permissions @@ -15,7 +15,8 @@ from django.contrib.auth.decorators import login_required from django.utils.decorators import method_decorator from funkwhale_api.musicbrainz import api -from funkwhale_api.common.permissions import ConditionalAuthentication +from funkwhale_api.common.permissions import ( + ConditionalAuthentication, HasModelPermission) from taggit.models import Tag from . import models @@ -71,16 +72,45 @@ class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): ordering_fields = ('creation_date',) -class ImportBatchViewSet(viewsets.ReadOnlyModelViewSet): +class ImportBatchViewSet( + mixins.CreateModelMixin, + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + viewsets.GenericViewSet): queryset = ( models.ImportBatch.objects.all() .prefetch_related('jobs__track_file') .order_by('-creation_date')) serializer_class = serializers.ImportBatchSerializer + permission_classes = (permissions.DjangoModelPermissions, ) def get_queryset(self): return super().get_queryset().filter(submitted_by=self.request.user) + def perform_create(self, serializer): + serializer.save(submitted_by=self.request.user) + + +class ImportJobPermission(HasModelPermission): + # not a typo, perms on import job is proxied to import batch + model = models.ImportBatch + + +class ImportJobViewSet( + mixins.CreateModelMixin, + viewsets.GenericViewSet): + queryset = (models.ImportJob.objects.all()) + serializer_class = serializers.ImportJobSerializer + permission_classes = (ImportJobPermission, ) + + def get_queryset(self): + return super().get_queryset().filter(batch__submitted_by=self.request.user) + + def perform_create(self, serializer): + source = 'file://' + serializer.validated_data['audio_file'].name + serializer.save(source=source) + tasks.import_job_run.delay(import_job_id=serializer.instance.pk) + class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): """ diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 74dd84625..7a856efd1 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -1,4 +1,5 @@ import json +import os import pytest from django.urls import reverse @@ -8,6 +9,8 @@ from funkwhale_api.music import serializers from . import data as api_data +DATA_DIR = os.path.dirname(os.path.abspath(__file__)) + def test_can_submit_youtube_url_for_track_import(mocker, superuser_client): mocker.patch( @@ -189,6 +192,48 @@ def test_user_can_query_api_for_his_own_batches(client, factories): assert results['results'][0]['jobs'][0]['mbid'] == job.mbid +def test_user_can_create_an_empty_batch(client, factories): + user = factories['users.SuperUser']() + url = reverse('api:v1:import-batches-list') + client.login(username=user.username, password='test') + response = client.post(url) + + assert response.status_code == 201 + + batch = user.imports.latest('id') + + assert batch.submitted_by == user + assert batch.source == 'api' + + +def test_user_can_create_import_job_with_file(client, factories, mocker): + path = os.path.join(DATA_DIR, 'test.ogg') + m = mocker.patch('funkwhale_api.music.tasks.import_job_run.delay') + user = factories['users.SuperUser']() + batch = factories['music.ImportBatch'](submitted_by=user) + url = reverse('api:v1:import-jobs-list') + client.login(username=user.username, password='test') + with open(path, 'rb') as f: + content = f.read() + f.seek(0) + response = client.post(url, { + 'batch': batch.pk, + 'audio_file': f, + 'source': 'file://' + }, format='multipart') + + assert response.status_code == 201 + + job = batch.jobs.latest('id') + + assert job.status == 'pending' + assert job.source.startswith('file://') + assert 'test.ogg' in job.source + assert job.audio_file.read() == content + + m.assert_called_once_with(import_job_id=job.pk) + + def test_can_search_artist(factories, client): artist1 = factories['music.Artist']() artist2 = factories['music.Artist']() diff --git a/deploy/nginx.conf b/deploy/nginx.conf index 90b0f000e..cf865a9ea 100644 --- a/deploy/nginx.conf +++ b/deploy/nginx.conf @@ -47,6 +47,8 @@ server { rewrite ^(.+)$ /index.html last; } location /api/ { + # this is needed if you have file import via upload enabled + client_max_body_size 30M; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/docker/nginx/conf.dev b/docker/nginx/conf.dev index 9c00fd76f..48436173b 100644 --- a/docker/nginx/conf.dev +++ b/docker/nginx/conf.dev @@ -30,7 +30,7 @@ http { server { listen 6001; charset utf-8; - + client_max_body_size 20M; location /_protected/media { internal; alias /protected/media; diff --git a/front/package.json b/front/package.json index 9af432387..58c22a408 100644 --- a/front/package.json +++ b/front/package.json @@ -23,6 +23,7 @@ "vue-lazyload": "^1.1.4", "vue-resource": "^1.3.4", "vue-router": "^2.3.1", + "vue-upload-component": "^2.7.4", "vuedraggable": "^2.14.1", "vuex": "^3.0.1", "vuex-persistedstate": "^2.4.2" diff --git a/front/src/components/library/import/BatchDetail.vue b/front/src/components/library/import/BatchDetail.vue index 726ec9c3e..762074c10 100644 --- a/front/src/components/library/import/BatchDetail.vue +++ b/front/src/components/library/import/BatchDetail.vue @@ -8,6 +8,7 @@ ['ui', {'active': batch.status === 'pending'}, {'warning': batch.status === 'pending'}, + {'error': batch.status === 'errored'}, {'success': batch.status === 'finished'}, 'progress']">
@@ -37,7 +38,7 @@ {{ job.status }} + :class="['ui', {'yellow': job.status === 'pending'}, {'red': job.status === 'errored'}, {'green': job.status === 'finished'}, 'label']">{{ job.status }} {{ job.track_file.track }} @@ -89,7 +90,7 @@ export default { computed: { progress () { return this.batch.jobs.filter(j => { - return j.status === 'finished' + return j.status !== 'pending' }).length * 100 / this.batch.jobs.length }, progressBarStyle () { diff --git a/front/src/components/library/import/BatchList.vue b/front/src/components/library/import/BatchList.vue index c78f1ea4e..f6e7b03e5 100644 --- a/front/src/components/library/import/BatchList.vue +++ b/front/src/components/library/import/BatchList.vue @@ -32,7 +32,7 @@ {{ result.jobs.length }} {{ result.status }} + :class="['ui', {'yellow': result.status === 'pending'}, {'red': result.status === 'errored'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }} diff --git a/front/src/components/library/import/FileUpload.vue b/front/src/components/library/import/FileUpload.vue new file mode 100644 index 000000000..4681d7932 --- /dev/null +++ b/front/src/components/library/import/FileUpload.vue @@ -0,0 +1,140 @@ + + + + + + diff --git a/front/src/components/library/import/FileUploadWidget.vue b/front/src/components/library/import/FileUploadWidget.vue new file mode 100644 index 000000000..1de8090c9 --- /dev/null +++ b/front/src/components/library/import/FileUploadWidget.vue @@ -0,0 +1,34 @@ + + + + diff --git a/front/src/components/library/import/Main.vue b/front/src/components/library/import/Main.vue index 10f6f352a..66ba8c661 100644 --- a/front/src/components/library/import/Main.vue +++ b/front/src/components/library/import/Main.vue @@ -39,8 +39,8 @@
-
- +
+
@@ -84,8 +84,14 @@
+ +