Merge branch '111-skip-acoustid' into 'develop'

Resolve "Allow skip accoustid entirely on music import (CLI)"

Closes #111

See merge request funkwhale/funkwhale!79

.gitignore

@@ -84,3 +84,5 @@ front/test/unit/coverage
 front/test/e2e/reports
 front/selenium-debug.log
 docs/_build
+
+data/

api/funkwhale_api/music/tasks.py

 from django.core.files.base import ContentFile
 
+from dynamic_preferences.registries import global_preferences_registry
+
 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
@@ -23,13 +25,15 @@ def set_acoustid_on_track_file(track_file):
         return update(result['id'])
 
 
-def _do_import(import_job, replace):
+def _do_import(import_job, replace, use_acoustid=True):
     from_file = bool(import_job.audio_file)
     mbid = import_job.mbid
     acoustid_track_id = None
     duration = None
     track = None
-    if not mbid and from_file:
+    manager = global_preferences_registry.manager()
+    use_acoustid = use_acoustid and manager['providers_acoustid__api_key']
+    if not mbid and use_acoustid and from_file:
         # we try to deduce mbid from acoustid
         client = get_acoustid_client()
         match = client.get_best_match(import_job.audio_file.path)
@@ -76,13 +80,13 @@ def _do_import(import_job, replace):
     models.ImportJob.objects.filter(
         status__in=['pending', 'errored']),
     'import_job')
-def import_job_run(self, import_job, replace=False):
+def import_job_run(self, import_job, replace=False, use_acoustid=True):
     def mark_errored():
         import_job.status = 'errored'
-        import_job.save()
+        import_job.save(update_fields=['status'])
 
     try:
-        return _do_import(import_job, replace)
+        return _do_import(import_job, replace, use_acoustid=use_acoustid)
     except Exception as exc:
         if not settings.DEBUG:
             try:

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(
+            '--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.",
@@ -108,7 +115,10 @@ class Command(BaseCommand):
 
             job.save()
             try:
-                utils.on_commit(import_handler, import_job_id=job.pk)
+                utils.on_commit(
+                    import_handler,
+                    import_job_id=job.pk,
+                    use_acoustid=not options['no_acoustid'])
             except Exception as e:
                 self.stdout.write('Error: {}'.format(e))
 

api/tests/music/test_tasks.py

@@ -9,7 +9,8 @@ from . import data as api_data
 DATA_DIR = os.path.dirname(os.path.abspath(__file__))
 
 
-def test_set_acoustid_on_track_file(factories, mocker):
+def test_set_acoustid_on_track_file(factories, mocker, preferences):
+    preferences['providers_acoustid__api_key'] = 'test'
     track_file = factories['music.TrackFile'](acoustid_track_id=None)
     id = 'e475bf79-c1ce-4441-bed7-1e33f226c0a2'
     payload = {
@@ -31,7 +32,7 @@ def test_set_acoustid_on_track_file(factories, mocker):
 
     assert str(track_file.acoustid_track_id) == id
     assert r == id
-    m.assert_called_once_with('', track_file.audio_file.path, parse=False)
+    m.assert_called_once_with('test', track_file.audio_file.path, parse=False)
 
 
 def test_set_acoustid_on_track_file_required_high_score(factories, mocker):
@@ -48,7 +49,9 @@ def test_set_acoustid_on_track_file_required_high_score(factories, mocker):
     assert track_file.acoustid_track_id is None
 
 
-def test_import_job_can_run_with_file_and_acoustid(factories, mocker):
+def test_import_job_can_run_with_file_and_acoustid(
+        preferences, factories, mocker):
+    preferences['providers_acoustid__api_key'] = 'test'
     path = os.path.join(DATA_DIR, 'test.ogg')
     mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed'
     acoustid_payload = {
@@ -88,7 +91,46 @@ def test_import_job_can_run_with_file_and_acoustid(factories, mocker):
     assert job.source == 'file://'
 
 
-def test_import_job_can_be_skipped(factories, mocker):
+def test_run_import_skipping_accoustid(factories, mocker):
+    m = mocker.patch('funkwhale_api.music.tasks._do_import')
+    path = os.path.join(DATA_DIR, 'test.ogg')
+    job = factories['music.FileImportJob'](audio_file__path=path)
+    tasks.import_job_run(import_job_id=job.pk, use_acoustid=False)
+    m.assert_called_once_with(job, False, use_acoustid=False)
+
+
+def test__do_import_skipping_accoustid(factories, mocker):
+    t = factories['music.Track']()
+    m = mocker.patch(
+        'funkwhale_api.music.tasks.import_track_data_from_path',
+        return_value=t)
+    path = os.path.join(DATA_DIR, 'test.ogg')
+    job = factories['music.FileImportJob'](
+        mbid=None,
+        audio_file__path=path)
+    p = job.audio_file.path
+    tasks._do_import(job, replace=False, use_acoustid=False)
+    m.assert_called_once_with(p)
+
+
+def test__do_import_skipping_accoustid_if_no_key(
+        factories, mocker, preferences):
+    preferences['providers_acoustid__api_key'] = ''
+    t = factories['music.Track']()
+    m = mocker.patch(
+        'funkwhale_api.music.tasks.import_track_data_from_path',
+        return_value=t)
+    path = os.path.join(DATA_DIR, 'test.ogg')
+    job = factories['music.FileImportJob'](
+        mbid=None,
+        audio_file__path=path)
+    p = job.audio_file.path
+    tasks._do_import(job, replace=False, use_acoustid=False)
+    m.assert_called_once_with(p)
+
+
+def test_import_job_can_be_skipped(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)
@@ -124,7 +166,8 @@ def test_import_job_can_be_skipped(factories, mocker):
     assert job.status == 'skipped'
 
 
-def test_import_job_can_be_errored(factories, mocker):
+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)

api/tests/test_import_audio_file.py

@@ -54,7 +54,7 @@ def test_management_command_requires_a_valid_username(factories, mocker):
 
 
 def test_import_files_creates_a_batch_and_job(factories, mocker):
-    m = m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.common.utils.on_commit')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     call_command(
@@ -77,4 +77,24 @@ def test_import_files_creates_a_batch_and_job(factories, mocker):
     assert job.source == 'file://' + path
     m.assert_called_once_with(
         music_tasks.import_job_run.delay,
-        import_job_id=job.pk)
+        import_job_id=job.pk,
+        use_acoustid=True)
+
+
+def test_import_files_skip_acoustid(factories, mocker):
+    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    user = factories['users.User'](username='me')
+    path = os.path.join(DATA_DIR, 'dummy_file.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)

changes/changelog.d/111.feature

+Can now skip acoustid on file import with the --no-acoustid flag (#111)

docs/importing-music.rst

@@ -25,6 +25,10 @@ to the ``/music`` directory on the container:
 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::