diff --git a/CHANGELOG b/CHANGELOG index 561c3f7e8..7c1215ee4 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,7 @@ Changelog ---------------- - Always use username in sidebar (#89) +- Fixed broken import because of missing transaction 0.5.2 (2018-02-26) diff --git a/api/funkwhale_api/common/utils.py b/api/funkwhale_api/common/utils.py index 838c15c00..c9d450e6a 100644 --- a/api/funkwhale_api/common/utils.py +++ b/api/funkwhale_api/common/utils.py @@ -1,6 +1,8 @@ import os import shutil +from django.db import transaction + def rename_file(instance, field_name, new_name, allow_missing_file=False): field = getattr(instance, field_name) @@ -17,3 +19,9 @@ def rename_file(instance, field_name, new_name, allow_missing_file=False): field.name = os.path.join(initial_path, new_name_with_extension) instance.save() return new_name_with_extension + + +def on_commit(f, *args, **kwargs): + return transaction.on_commit( + lambda: f(*args, **kwargs) + ) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index fc706c812..cb4a737c9 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -73,7 +73,10 @@ def _do_import(import_job, replace): @celery.app.task(name='ImportJob.run', bind=True) -@celery.require_instance(models.ImportJob, 'import_job') +@celery.require_instance( + models.ImportJob.objects.filter( + status__in=['pending', 'errored']), + 'import_job') def import_job_run(self, import_job, replace=False): def mark_errored(): import_job.status = 'errored' diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index bf9d39b1d..ac76667da 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -19,6 +19,7 @@ from musicbrainzngs import ResponseError from django.contrib.auth.decorators import login_required from django.utils.decorators import method_decorator +from funkwhale_api.common import utils as funkwhale_utils from funkwhale_api.requests.models import ImportRequest from funkwhale_api.musicbrainz import api from funkwhale_api.common.permissions import ( @@ -116,7 +117,10 @@ class ImportJobViewSet( def perform_create(self, serializer): source = 'file://' + serializer.validated_data['audio_file'].name serializer.save(source=source) - tasks.import_job_run.delay(import_job_id=serializer.instance.pk) + funkwhale_utils.on_commit( + tasks.import_job_run.delay, + import_job_id=serializer.instance.pk + ) class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): @@ -336,6 +340,7 @@ class SubmitViewSet(viewsets.ViewSet): data, request, batch=None, import_request=import_request) return Response(import_data) + @transaction.atomic def _import_album(self, data, request, batch=None, import_request=None): # we import the whole album here to prevent race conditions that occurs # when using get_or_create_from_api in tasks @@ -355,7 +360,11 @@ class SubmitViewSet(viewsets.ViewSet): models.TrackFile.objects.get(track__mbid=row['mbid']) except models.TrackFile.DoesNotExist: job = models.ImportJob.objects.create(mbid=row['mbid'], batch=batch, source=row['source']) - tasks.import_job_run.delay(import_job_id=job.pk) + funkwhale_utils.on_commit( + tasks.import_job_run.delay, + import_job_id=job.pk + ) + serializer = serializers.ImportBatchSerializer(batch) return serializer.data, batch 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 4130e93f3..17a199473 100644 --- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py +++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py @@ -3,6 +3,9 @@ import os from django.core.files import File from django.core.management.base import BaseCommand, CommandError +from django.db import transaction + +from funkwhale_api.common import utils from funkwhale_api.music import tasks from funkwhale_api.users.models import User @@ -86,6 +89,7 @@ class Command(BaseCommand): self.stdout.write( "For details, please refer to import batch #".format(batch.pk)) + @transaction.atomic def do_import(self, matching, user, options): message = 'Importing {}...' if options['async']: @@ -94,7 +98,7 @@ class Command(BaseCommand): # we create an import batch binded to the user batch = user.imports.create(source='shell') async = options['async'] - handler = tasks.import_job_run.delay if async else tasks.import_job_run + 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, @@ -105,7 +109,8 @@ class Command(BaseCommand): job.save() try: - handler(import_job_id=job.pk) + utils.on_commit(import_handler, import_job_id=job.pk) except Exception as e: self.stdout.write('Error: {}'.format(e)) + return batch diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 7a856efd1..8196d3c09 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -6,6 +6,7 @@ from django.urls import reverse from funkwhale_api.music import models from funkwhale_api.musicbrainz import api from funkwhale_api.music import serializers +from funkwhale_api.music import tasks from . import data as api_data @@ -208,7 +209,7 @@ def test_user_can_create_an_empty_batch(client, factories): def test_user_can_create_import_job_with_file(client, factories, mocker): path = os.path.join(DATA_DIR, 'test.ogg') - m = mocker.patch('funkwhale_api.music.tasks.import_job_run.delay') + m = mocker.patch('funkwhale_api.common.utils.on_commit') user = factories['users.SuperUser']() batch = factories['music.ImportBatch'](submitted_by=user) url = reverse('api:v1:import-jobs-list') @@ -231,7 +232,9 @@ def test_user_can_create_import_job_with_file(client, factories, mocker): assert 'test.ogg' in job.source assert job.audio_file.read() == content - m.assert_called_once_with(import_job_id=job.pk) + m.assert_called_once_with( + tasks.import_job_run.delay, + import_job_id=job.pk) def test_can_search_artist(factories, client): diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index e81e9b814..2c254afd9 100644 --- a/api/tests/test_import_audio_file.py +++ b/api/tests/test_import_audio_file.py @@ -6,6 +6,7 @@ from django.core.management import call_command from django.core.management.base import CommandError from funkwhale_api.providers.audiofile import tasks +from funkwhale_api.music import tasks as music_tasks DATA_DIR = os.path.join( os.path.dirname(os.path.abspath(__file__)), @@ -53,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 = mocker.patch('funkwhale_api.music.tasks.import_job_run.delay') + m = 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( @@ -74,4 +75,6 @@ def test_import_files_creates_a_batch_and_job(factories, mocker): assert job.audio_file.read() == f.read() assert job.source == 'file://' + path - m.assert_called_once_with(import_job_id=job.pk) + m.assert_called_once_with( + music_tasks.import_job_run.delay, + import_job_id=job.pk)