Merge branch '106-web-upload' into 'develop'
Resolve "File import is broken" Closes #106 and #213 See merge request funkwhale/funkwhale!204
This commit is contained in:
Eliot Berriot
commit
7db01394ec
|
|
@ -9,7 +9,7 @@ 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
|
||||
|
|
@ -73,13 +73,15 @@ 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):
|
||||
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()
|
||||
|
|
@ -91,11 +93,12 @@ def _do_import(import_job, replace=False, use_acoustid=True):
|
|||
if 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)
|
||||
track = audiofile_tasks.import_track_data_from_path(
|
||||
import_job.audio_file.path)
|
||||
elif import_job.library_track:
|
||||
track = import_track_from_remote(import_job.library_track)
|
||||
elif import_job.source.startswith('file://'):
|
||||
track = import_track_data_from_path(
|
||||
track = audiofile_tasks.import_track_data_from_path(
|
||||
import_job.source.replace('file://', '', 1))
|
||||
else:
|
||||
raise ValueError(
|
||||
|
|
@ -151,7 +154,7 @@ def _do_import(import_job, replace=False, use_acoustid=True):
|
|||
models.ImportJob.objects.filter(
|
||||
status__in=['pending', 'errored']),
|
||||
'import_job')
|
||||
def import_job_run(self, import_job, replace=False, use_acoustid=True):
|
||||
def import_job_run(self, import_job, replace=False, use_acoustid=False):
|
||||
def mark_errored():
|
||||
import_job.status = 'errored'
|
||||
import_job.save(update_fields=['status'])
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
|
|
@ -105,7 +105,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 +121,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 +132,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,29 +153,23 @@ 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
|
||||
|
|
|
|||
|
|
@ -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()
|
||||
|
|
|
|||
|
|
@ -0,0 +1 @@
|
|||
File-upload importer should now work properly, assuming files are tagged (#106)
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
File-upload import now supports Flac files (#213)
|
||||
|
||||
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.
|
||||
|
|
@ -0,0 +1 @@
|
|||
Removed acoustid support, as the integration was buggy and error-prone (#106)
|
||||
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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 <http://picard.musicbrainz.org/>`_ 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:
|
||||
|
|
|
|||
|
|
@ -1,6 +1,10 @@
|
|||
<template>
|
||||
<div>
|
||||
<div v-if="batch" class="ui container">
|
||||
<div class="ui message">
|
||||
{{ $t('Ensure your music files are properly tagged before uploading them.') }}
|
||||
<a href="http://picard.musicbrainz.org/" target='_blank'>{{ $t('We recommend using Picard for that purpose.') }}</a>
|
||||
</div>
|
||||
<file-upload-widget
|
||||
:class="['ui', 'icon', 'left', 'floated', 'button']"
|
||||
:post-action="uploadUrl"
|
||||
|
|
@ -8,7 +12,7 @@
|
|||
:size="1024 * 1024 * 30"
|
||||
:data="uploadData"
|
||||
:drop="true"
|
||||
extensions="ogg,mp3"
|
||||
extensions="ogg,mp3,flac"
|
||||
accept="audio/*"
|
||||
v-model="files"
|
||||
name="audio_file"
|
||||
|
|
@ -21,7 +25,7 @@
|
|||
</file-upload-widget>
|
||||
<button
|
||||
:class="['ui', 'right', 'floated', 'icon', {disabled: files.length === 0}, 'button']"
|
||||
v-if="!$refs.upload || !$refs.upload.active" @click.prevent="$refs.upload.active = true">
|
||||
v-if="!$refs.upload || !$refs.upload.active" @click.prevent="startUpload()">
|
||||
<i class="play icon" aria-hidden="true"></i>
|
||||
<i18next path="Start Upload"/>
|
||||
</button>
|
||||
|
|
@ -88,7 +92,7 @@ export default {
|
|||
inputFilter (newFile, oldFile, prevent) {
|
||||
if (newFile && !oldFile) {
|
||||
let extension = newFile.name.split('.').pop()
|
||||
if (['ogg', 'mp3'].indexOf(extension) < 0) {
|
||||
if (['ogg', 'mp3', 'flac'].indexOf(extension) < 0) {
|
||||
prevent()
|
||||
}
|
||||
}
|
||||
|
|
@ -114,6 +118,10 @@ export default {
|
|||
}, (response) => {
|
||||
logger.default.error('error while launching creating batch')
|
||||
})
|
||||
},
|
||||
startUpload () {
|
||||
this.$emit('batch-created', this.batch)
|
||||
this.$refs.upload.active = true
|
||||
}
|
||||
},
|
||||
computed: {
|
||||
|
|
|
|||
|
|
@ -24,16 +24,25 @@
|
|||
<div class="ui hidden divider"></div>
|
||||
<div class="ui centered buttons">
|
||||
<button @click="currentStep -= 1" :disabled="currentStep === 0" class="ui icon button"><i class="left arrow icon"></i><i18next path="Previous step"/></button>
|
||||
<button @click="currentStep += 1" v-if="currentStep < 2" class="ui icon button"><i18next path="Next step"/><i class="right arrow icon"></i></button>
|
||||
<button @click="nextStep()" v-if="currentStep < 2" class="ui icon button"><i18next path="Next step"/><i class="right arrow icon"></i></button>
|
||||
<button
|
||||
@click="$refs.import.launchImport()"
|
||||
v-if="currentStep === 2"
|
||||
v-if="currentStep === 2 && currentSource != 'upload'"
|
||||
:class="['ui', 'positive', 'icon', {'loading': isImporting}, 'button']"
|
||||
:disabled="isImporting || importData.count === 0"
|
||||
>
|
||||
<i18next path="Import {%0%} tracks">{{ importData.count }}</i18next>
|
||||
<i class="check icon"></i>
|
||||
</button>
|
||||
<button
|
||||
v-else-if="currentStep === 2 && currentSource === 'upload'"
|
||||
@click="$router.push({name: 'library.import.batches.detail', params: {id: importBatch.id}})"
|
||||
:class="['ui', 'positive', 'icon', {'disabled': !importBatch}, 'button']"
|
||||
:disabled="!importBatch"
|
||||
>
|
||||
{{ $t('Finish import' )}}
|
||||
<i class="check icon"></i>
|
||||
</button>
|
||||
</div>
|
||||
<div class="ui hidden divider"></div>
|
||||
<div class="ui attached segment">
|
||||
|
|
@ -100,6 +109,7 @@
|
|||
<div v-if="currentStep === 2">
|
||||
<file-upload
|
||||
ref="import"
|
||||
@batch-created="updateBatch"
|
||||
v-if="currentSource == 'upload'"
|
||||
></file-upload>
|
||||
|
||||
|
|
@ -165,6 +175,7 @@ export default {
|
|||
currentSource: this.source,
|
||||
metadata: {},
|
||||
isImporting: false,
|
||||
importBatch: null,
|
||||
importData: {
|
||||
tracks: []
|
||||
},
|
||||
|
|
@ -214,11 +225,22 @@ export default {
|
|||
updateId (newValue) {
|
||||
this.currentId = newValue
|
||||
},
|
||||
updateBatch (batch) {
|
||||
this.importBatch = batch
|
||||
},
|
||||
fetchRequest (id) {
|
||||
let self = this
|
||||
axios.get(`requests/import-requests/${id}`).then((response) => {
|
||||
self.currentRequest = response.data
|
||||
})
|
||||
},
|
||||
nextStep () {
|
||||
if (this.currentStep === 0 && this.currentSource === 'upload') {
|
||||
// we skip metadata directly
|
||||
this.currentStep += 2
|
||||
} else {
|
||||
this.currentStep += 1
|
||||
}
|
||||
}
|
||||
},
|
||||
computed: {
|
||||
|
|
|
|||
|
|
@ -93,8 +93,7 @@ export default {
|
|||
label: this.$t('Imports'),
|
||||
id: 'imports',
|
||||
settings: [
|
||||
'providers_youtube__api_key',
|
||||
'providers_acoustid__api_key'
|
||||
'providers_youtube__api_key'
|
||||
]
|
||||
},
|
||||
{
|
||||
|
|
|
|||
Loading…
Reference in New Issue