Merge branch '138-import-unicode' into 'develop'

Resolve "Importer chokes on files with bad unicode characters"

Closes #138 and #120

See merge request funkwhale/funkwhale!108

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 = (

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)

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__

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(
+                self.stdout.write(message.format(path))
-                    import_handler,
+                self.import_file(path, batch, import_handler, options)
-                    import_job_id=job.pk,
-                    use_acoustid=not options['no_acoustid'])
             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'])

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')

changes/changelog.d/120.bugfix

@@ -0,0 +1 @@
+Better error handling during file import (#120)

changes/changelog.d/138.bugfix

@@ -0,0 +1 @@
+Better handling of utf-8 filenames during file import (#138)