diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 9aac55515f1cab143a02e58e3870d4203439aad9..bf7a847d0c45a2aeabd7fb189ae017a655e4e966 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -1,5 +1,7 @@ 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: 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 17a199473de9a8f0ec82da1d44b5ae74a481f31c..e7ca023313d8edfe1f0b0ccb4d3675d28a778193 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( + '--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.", @@ -109,7 +116,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)) diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py index 873ca18f02e32ee0c0c9b8c489c902bc5ab9f122..5ecf9b9e46310c5f3779fa4ced90daa0809c5aff 100644 --- a/api/tests/music/test_tasks.py +++ b/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) diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index 2c254afd959414d7ae3dbccdf5840cc42ad50956..4f3de27db47df47250c69a844e4f15a37f092c21 100644 --- a/api/tests/test_import_audio_file.py +++ b/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) diff --git a/changes/changelog.d/111.feature b/changes/changelog.d/111.feature new file mode 100644 index 0000000000000000000000000000000000000000..73fc5df3ada96c095d834bcf8ea66da283bb23c7 --- /dev/null +++ b/changes/changelog.d/111.feature @@ -0,0 +1 @@ +Can now skip acoustid on file import with the --no-acoustid flag (#111) diff --git a/docs/importing-music.rst b/docs/importing-music.rst index 3fa9e999008e36d9276b2c94137f109ee397c840..92ff6bfb503208319ecef08347fdf72cf8d7709f 100644 --- a/docs/importing-music.rst +++ b/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::