From f1cf250e2959e2fe7a333994fd10fe1817795e3b Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sun, 25 Mar 2018 15:40:37 +0200 Subject: [PATCH 1/2] Fix #138: Better handling of utf-8 filenames during file import --- api/config/settings/common.py | 1 + api/funkwhale_api/common/storage.py | 12 ++++++++++++ api/tests/files/utf8-éà◌.ogg | 0 api/tests/test_import_audio_file.py | 24 ++++++++++++++++++++++++ changes/changelog.d/138.bugfix | 1 + 5 files changed, 38 insertions(+) create mode 100644 api/funkwhale_api/common/storage.py create mode 100644 api/tests/files/utf8-éà◌.ogg create mode 100644 changes/changelog.d/138.bugfix diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 077566d1c..f8bd77252 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 000000000..658ce795a --- /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/tests/files/utf8-éà◌.ogg b/api/tests/files/utf8-éà◌.ogg new file mode 100644 index 000000000..e69de29bb diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index 4f3de27db..67263e66d 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/138.bugfix b/changes/changelog.d/138.bugfix new file mode 100644 index 000000000..2a8f7aeb0 --- /dev/null +++ b/changes/changelog.d/138.bugfix @@ -0,0 +1 @@ +Better handling of utf-8 filenames during file import (#138) From e99d757b5741f44c4fed2dfc1bcaafc6c243295f Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sun, 25 Mar 2018 15:44:48 +0200 Subject: [PATCH 2/2] Fix #120: Better error handling during file import --- api/funkwhale_api/music/metadata.py | 8 +++- .../management/commands/import_files.py | 42 ++++++++++++------- changes/changelog.d/120.bugfix | 1 + 3 files changed, 36 insertions(+), 15 deletions(-) create mode 100644 changes/changelog.d/120.bugfix diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index 31d13d495..3748d5573 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 2fa5e464c..dbc01289f 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/changes/changelog.d/120.bugfix b/changes/changelog.d/120.bugfix new file mode 100644 index 000000000..b7d9ef066 --- /dev/null +++ b/changes/changelog.d/120.bugfix @@ -0,0 +1 @@ +Better error handling during file import (#120)