Commit b5226367 authored by Eliot Berriot's avatar Eliot Berriot 💬

Fixed broken import because of missing transaction

parent 1e2ab3ee
......@@ -5,6 +5,7 @@ Changelog
----------------
- Always use username in sidebar (#89)
- Fixed broken import because of missing transaction
0.5.2 (2018-02-26)
......
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)
)
......@@ -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'
......
......@@ -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
......
......@@ -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
......@@ -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):
......
......@@ -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)
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment