diff --git a/CHANGELOG b/CHANGELOG index fbf8d1fbff5a3350570db2b94b43806421dfbdd7..162246f758651520fb1868e7f0fb32a51b840230 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 a41c7bc0f3eb1b524f15b88f166b93141af2b1d9..d64eeb5fdbb62b5cdfaceaef8740a82cc9d9c2ef 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 6b61d9839d5dd7c14eb4c495b912e35e440cb68c..ecfea4c9a51582031a69c8592d4c2592cb2988e6 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 fdb88c3367d2136b3f5fe476bc4b6f09b6e38228..f919ff0eb3b0b0b6619699b5c65967096bbcec18 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 cf9d8749021ca8f5a1c03f297924de2ed1a583ca..9f96dad0e300a64ae0d03eaf36cca177f8f89589 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 009f00680f1fa76283e89263b3a89d36093c9e17..fc706c812fcd97145540747cd2fc4a3b11db56b6 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 d7152ff221f70c4b9b18446844ac719cc48da57d..dd295ae79f66a2192f9770a24c0b2fb53c5ea49e 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 74dd84625186f1d64cbfa975679c5e402e6d23a4..7a856efd1f21971ec7657417f462d007f053c80a 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 90b0f000e256db13959c19658c3bbff6d26c7676..cf865a9ea7b3ef84eeb6eda8d6d65f509790eaf5 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 9c00fd76fc577c417e489c33a731baba1a2f9980..48436173bcb5ce236bc6a5cce4455467e3794039 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 9af43238767258ddb082b17659b6b9d278dd7952..58c22a4082c4d37762c32a3e23e7edf453a7fbca 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 726ec9c3e29eed7e6e3aa8433e7140d3b2a40cc4..762074c107ff9a2f61fbccb92611041f5d9ecc6f 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']"> <div class="bar" :style="progressBarStyle"> @@ -37,7 +38,7 @@ </td> <td> <span - :class="['ui', {'yellow': job.status === 'pending'}, {'green': job.status === 'finished'}, 'label']">{{ job.status }}</span> + :class="['ui', {'yellow': job.status === 'pending'}, {'red': job.status === 'errored'}, {'green': job.status === 'finished'}, 'label']">{{ job.status }}</span> </td> <td> <router-link v-if="job.track_file" :to="{name: 'library.tracks.detail', params: {id: job.track_file.track }}">{{ job.track_file.track }}</router-link> @@ -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 c78f1ea4e19a28195e573880e491baaf81147c7c..f6e7b03e5c06323d5033a1278626efa262d8b123 100644 --- a/front/src/components/library/import/BatchList.vue +++ b/front/src/components/library/import/BatchList.vue @@ -32,7 +32,7 @@ <td>{{ result.jobs.length }}</td> <td> <span - :class="['ui', {'yellow': result.status === 'pending'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }}</span> + :class="['ui', {'yellow': result.status === 'pending'}, {'red': result.status === 'errored'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }}</span> </td> </tr> </tbody> diff --git a/front/src/components/library/import/FileUpload.vue b/front/src/components/library/import/FileUpload.vue new file mode 100644 index 0000000000000000000000000000000000000000..4681d79322727f9341682f5b8586bc0685001676 --- /dev/null +++ b/front/src/components/library/import/FileUpload.vue @@ -0,0 +1,140 @@ +<template> + <div> + <div v-if="batch" class="ui two buttons"> + <file-upload-widget + class="ui icon button" + :post-action="uploadUrl" + :multiple="true" + :size="1024 * 1024 * 30" + :data="uploadData" + :drop="true" + extensions="ogg,mp3" + accept="audio/*" + v-model="files" + name="audio_file" + :thread="3" + @input-filter="inputFilter" + @input-file="inputFile" + ref="upload"> + <i class="upload icon"></i> + Select files to upload... + </file-upload-widget> + <button class="ui icon teal button" v-if="!$refs.upload || !$refs.upload.active" @click.prevent="$refs.upload.active = true"> + <i class="play icon" aria-hidden="true"></i> + Start Upload + </button> + <button type="button" class="ui icon yellow button" v-else @click.prevent="$refs.upload.active = false"> + <i class="pause icon" aria-hidden="true"></i> + Stop Upload + </button> + </div> + <div class="ui hidden divider"></div> + <p> + Once all your files are uploaded, simply head over <router-link :to="{name: 'library.import.batches.detail', params: {id: batch.id }}">import detail page</router-link> to check the import status. + </p> + <table class="ui single line table"> + <thead> + <tr> + <th>File name</th> + <th>Size</th> + <th>Status</th> + </tr> + </thead> + <tbody> + <tr v-for="(file, index) in files" :key="file.id"> + <td>{{ file.name }}</td> + <td>{{ file.size }}</td> + <td> + <span v-if="file.error" class="ui red label"> + {{ file.error }} + </span> + <span v-else-if="file.success" class="ui green label">Success</span> + <span v-else-if="file.active" class="ui yellow label">Uploading...</span> + <template v-else> + <span class="ui label">Pending</span> + <button class="ui tiny basic red icon button" @click.prevent="$refs.upload.remove(file)"><i class="delete icon"></i></button> + </template> + </td> + </tr> + </tbody> + </table> + </div> +</template> + +<script> +import Vue from 'vue' +import logger from '@/logging' +import FileUploadWidget from './FileUploadWidget' +import config from '@/config' + +export default { + components: { + FileUploadWidget + }, + data () { + return { + files: [], + uploadUrl: config.API_URL + 'import-jobs/', + batch: null + } + }, + mounted: function () { + this.createBatch() + }, + methods: { + inputFilter (newFile, oldFile, prevent) { + if (newFile && !oldFile) { + let extension = newFile.name.split('.').pop() + if (['ogg', 'mp3'].indexOf(extension) < 0) { + prevent() + } + } + }, + inputFile (newFile, oldFile) { + if (newFile && !oldFile) { + // add + console.log('add', newFile) + if (!this.batch) { + this.createBatch() + } + } + if (newFile && oldFile) { + // update + console.log('update', newFile) + } + if (!newFile && oldFile) { + // remove + console.log('remove', oldFile) + } + }, + createBatch () { + let self = this + let url = config.API_URL + 'import-batches/' + let resource = Vue.resource(url) + resource.save({}, {}).then((response) => { + self.batch = response.data + }, (response) => { + logger.default.error('error while launching creating batch') + }) + } + }, + computed: { + batchId: function () { + if (this.batch) { + return this.batch.id + } + return null + }, + uploadData: function () { + return { + 'batch': this.batchId, + 'source': 'file://' + } + } + } +} +</script> + +<!-- Add "scoped" attribute to limit CSS to this component only --> +<style scoped> +</style> diff --git a/front/src/components/library/import/FileUploadWidget.vue b/front/src/components/library/import/FileUploadWidget.vue new file mode 100644 index 0000000000000000000000000000000000000000..1de8090c9c91939776818aa22d4e0dd20db9188d --- /dev/null +++ b/front/src/components/library/import/FileUploadWidget.vue @@ -0,0 +1,34 @@ +<script> +import FileUpload from 'vue-upload-component' + +export default { + extends: FileUpload, + methods: { + uploadHtml5 (file) { + let form = new window.FormData() + let value + for (let key in file.data) { + value = file.data[key] + if (value && typeof value === 'object' && typeof value.toString !== 'function') { + if (value instanceof File) { + form.append(key, value, value.name) + } else { + form.append(key, JSON.stringify(value)) + } + } else if (value !== null && value !== undefined) { + form.append(key, value) + } + } + form.append(this.name, file.file, file.file.filename || file.name) + let xhr = new XMLHttpRequest() + xhr.open('POST', file.postAction) + xhr.setRequestHeader('Authorization', this.$store.getters['auth/header']) + return this.uploadXhr(xhr, file, form) + } + } +} +</script> + +<!-- Add "scoped" attribute to limit CSS to this component only --> +<style scoped> +</style> diff --git a/front/src/components/library/import/Main.vue b/front/src/components/library/import/Main.vue index 10f6f352af0717886a0e4dc96b2dabe77d846ab7..66ba8c66144db35f4d4c17dfcada5e4c89fcc2e4 100644 --- a/front/src/components/library/import/Main.vue +++ b/front/src/components/library/import/Main.vue @@ -39,8 +39,8 @@ </div> </div> <div class="field"> - <div class="ui disabled radio checkbox"> - <input type="radio" id="upload" value="upload" v-model="currentSource" disabled> + <div class="ui radio checkbox"> + <input type="radio" id="upload" value="upload" v-model="currentSource"> <label for="upload">File upload</label> </div> </div> @@ -84,8 +84,14 @@ </div> </div> <div v-if="currentStep === 2"> + <file-upload + ref="import" + v-if="currentSource == 'upload'" + ></file-upload> + <component ref="import" + v-if="currentSource == 'external'" :metadata="metadata" :is="importComponent" :backends="backends" @@ -119,6 +125,7 @@ import MetadataSearch from '@/components/metadata/Search' import ReleaseCard from '@/components/metadata/ReleaseCard' import ArtistCard from '@/components/metadata/ArtistCard' import ReleaseImport from './ReleaseImport' +import FileUpload from './FileUpload' import ArtistImport from './ArtistImport' import router from '@/router' @@ -130,7 +137,8 @@ export default { ArtistCard, ReleaseCard, ArtistImport, - ReleaseImport + ReleaseImport, + FileUpload }, props: { mbType: {type: String, required: false}, @@ -142,7 +150,7 @@ export default { currentType: this.mbType || 'artist', currentId: this.mbId, currentStep: 0, - currentSource: this.source || 'external', + currentSource: '', metadata: {}, isImporting: false, importData: {