diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 2c72865f63fa3e8d108d5ed1677a386ca7f9fc0d..83b7e0255ea2726a7de3af6420485cef9078cf98 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -231,6 +231,7 @@ STATIC_ROOT = env("STATIC_ROOT", default=str(ROOT_DIR('staticfiles'))) # See: https://docs.djangoproject.com/en/dev/ref/settings/#static-url STATIC_URL = env("STATIC_URL", default='/staticfiles/') +DEFAULT_FILE_STORAGE = 'funkwhale_api.common.storage.ASCIIFileSystemStorage' # See: https://docs.djangoproject.com/en/dev/ref/contrib/staticfiles/#std:setting-STATICFILES_DIRS STATICFILES_DIRS = ( diff --git a/api/funkwhale_api/common/storage.py b/api/funkwhale_api/common/storage.py new file mode 100644 index 0000000000000000000000000000000000000000..658ce795a4bad7290b1aa8d07766207d2da5a2b3 --- /dev/null +++ b/api/funkwhale_api/common/storage.py @@ -0,0 +1,12 @@ +import unicodedata + +from django.core.files.storage import FileSystemStorage + + +class ASCIIFileSystemStorage(FileSystemStorage): + """ + Convert unicode characters in name to ASCII characters. + """ + def get_valid_name(self, name): + name = unicodedata.normalize('NFKD', name).encode('ascii', 'ignore') + return super().get_valid_name(name) diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index 31d13d4957f6882b647b99388f334a1b03f092bb..3748d55730faf8188d56828bda3afd67dd219393 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/api/funkwhale_api/music/metadata.py @@ -121,7 +121,13 @@ class Metadata(object): def __init__(self, path): self._file = mutagen.File(path) - self._conf = CONF[self.get_file_type(self._file)] + if self._file is None: + raise ValueError('Cannot parse metadata from {}'.format(path)) + ft = self.get_file_type(self._file) + try: + self._conf = CONF[ft] + except KeyError: + raise ValueError('Unsupported format {}'.format(ft)) def get_file_type(self, f): return f.__class__.__name__ 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 2fa5e464cb6c650c3250f68888b0dfe599b25546..dbc01289f1206c18a15a691d2fb8025695155b17 100644 --- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py +++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py @@ -34,6 +34,13 @@ class Command(BaseCommand): default=False, help='Will launch celery tasks for each file to import instead of doing it synchronously and block the CLI', ) + parser.add_argument( + '--exit', '-x', + action='store_true', + dest='exit_on_failure', + default=False, + help='use this flag to disable error catching', + ) parser.add_argument( '--no-acoustid', action='store_true', @@ -106,20 +113,27 @@ class Command(BaseCommand): async = options['async'] import_handler = tasks.import_job_run.delay if async else tasks.import_job_run for path in matching: - job = batch.jobs.create( - source='file://' + path, - ) - name = os.path.basename(path) - with open(path, 'rb') as f: - job.audio_file.save(name, File(f)) - - job.save() try: - utils.on_commit( - import_handler, - import_job_id=job.pk, - use_acoustid=not options['no_acoustid']) + self.stdout.write(message.format(path)) + self.import_file(path, batch, import_handler, options) except Exception as e: - self.stdout.write('Error: {}'.format(e)) - + if options['exit_on_failure']: + raise + m = 'Error while importing {}: {} {}'.format( + path, e.__class__.__name__, e) + self.stderr.write(m) return batch + + def import_file(self, path, batch, import_handler, options): + job = batch.jobs.create( + source='file://' + path, + ) + name = os.path.basename(path) + with open(path, 'rb') as f: + job.audio_file.save(name, File(f)) + + job.save() + utils.on_commit( + import_handler, + import_job_id=job.pk, + use_acoustid=not options['no_acoustid']) diff --git "a/api/tests/files/utf8-\303\251\303\240\342\227\214.ogg" "b/api/tests/files/utf8-\303\251\303\240\342\227\214.ogg" new file mode 100644 index 0000000000000000000000000000000000000000..e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index 4f3de27db47df47250c69a844e4f15a37f092c21..67263e66d0c56309bb21f344274277b7d4464165 100644 --- a/api/tests/test_import_audio_file.py +++ b/api/tests/test_import_audio_file.py @@ -98,3 +98,27 @@ def test_import_files_skip_acoustid(factories, mocker): music_tasks.import_job_run.delay, import_job_id=job.pk, use_acoustid=False) + + +def test_import_files_works_with_utf8_file_name(factories, mocker): + m = mocker.patch('funkwhale_api.common.utils.on_commit') + user = factories['users.User'](username='me') + path = os.path.join(DATA_DIR, 'utf8-éà◌.ogg') + call_command( + 'import_files', + path, + username='me', + async=True, + no_acoustid=True, + interactive=False) + batch = user.imports.latest('id') + job = batch.jobs.first() + m.assert_called_once_with( + music_tasks.import_job_run.delay, + import_job_id=job.pk, + use_acoustid=False) + + +def test_storage_rename_utf_8_files(factories): + tf = factories['music.TrackFile'](audio_file__filename='été.ogg') + assert tf.audio_file.name.endswith('ete.ogg') diff --git a/changes/changelog.d/120.bugfix b/changes/changelog.d/120.bugfix new file mode 100644 index 0000000000000000000000000000000000000000..b7d9ef0662ff962d51ddecf81519b5a56cb3d380 --- /dev/null +++ b/changes/changelog.d/120.bugfix @@ -0,0 +1 @@ +Better error handling during file import (#120) diff --git a/changes/changelog.d/138.bugfix b/changes/changelog.d/138.bugfix new file mode 100644 index 0000000000000000000000000000000000000000..2a8f7aeb0d886fc8150d3504f612d6b2cfa05147 --- /dev/null +++ b/changes/changelog.d/138.bugfix @@ -0,0 +1 @@ +Better handling of utf-8 filenames during file import (#138)