Merge branch 'develop' into i18n-components

.env.dev

@@ -9,3 +9,4 @@ FUNKWHALE_HOSTNAME=localhost
 FUNKWHALE_PROTOCOL=http
 PYTHONDONTWRITEBYTECODE=true
 WEBPACK_DEVSERVER_PORT=8080
+MUSIC_DIRECTORY_PATH=/music

.gitlab-ci.yml

@@ -70,7 +70,9 @@ build_front:
     - yarn install
     - yarn run i18n-extract
     - yarn run i18n-compile
-    - yarn run build
+    # this is to ensure we don't have any errors in the output,
+    # cf https://code.eliotberriot.com/funkwhale/funkwhale/issues/169
+    - yarn run build | tee /dev/stderr | (! grep -i 'ERROR in')
   cache:
     key: "$CI_PROJECT_ID__front_dependencies"
     paths:

CHANGELOG

@@ -3,6 +3,52 @@ Changelog
 
 .. towncrier
 
+0.10 (Unreleased)
+-----------------
+
+
+In-place import
+^^^^^^^^^^^^^^^
+
+This release includes in-place imports for the CLI import. This means you can
+load gigabytes of music into funkwhale without worrying about about Funkwhale
+copying those music files in its internal storage and eating your disk space.
+
+This new feature is documented <here> and require additional configuration
+to ensure funkwhale and your webserver can serve those files properly.
+
+**Non-docker users:**
+
+Assuming your music is stored in ``/srv/funkwhale/data/music``, add the following
+block to your nginx configuration::
+
+    location /_protected/music {
+        internal;
+        alias   /srv/funkwhale/data/music;
+    }
+
+And the following to your .env file::
+
+    MUSIC_DIRECTORY_PATH=/srv/funkwhale/data/music
+
+**Docker users:**
+
+Assuming your music is stored in ``/srv/funkwhale/data/music``, add the following
+block to your nginx configuration::
+
+    location /_protected/music {
+        internal;
+        alias   /srv/funkwhale/data/music;
+    }
+
+Assuming you have the following volume directive in your ``docker-compose.yml``
+(it's the default): ``/srv/funkwhale/data/music:/music:ro``, then add
+the following to your .env file::
+
+    MUSIC_DIRECTORY_PATH=/music
+    MUSIC_DIRECTORY_SERVE_PATH=/srv/funkwhale/data/music
+
+
 0.9.1 (2018-04-17)
 ------------------
 

api/config/settings/common.py

@@ -441,3 +441,9 @@ EXTERNAL_REQUESTS_VERIFY_SSL = env.bool(
     'EXTERNAL_REQUESTS_VERIFY_SSL',
     default=True
 )
+
+MUSIC_DIRECTORY_PATH = env('MUSIC_DIRECTORY_PATH', default=None)
+# on Docker setup, the music directory may not match the host path,
+# and we need to know it for it to serve stuff properly
+MUSIC_DIRECTORY_SERVE_PATH = env(
+    'MUSIC_DIRECTORY_SERVE_PATH', default=MUSIC_DIRECTORY_PATH)

api/funkwhale_api/music/factories.py

@@ -43,6 +43,7 @@ class TrackFactory(factory.django.DjangoModelFactory):
     artist = factory.SelfAttribute('album.artist')
     position = 1
     tags = ManyToManyFromList('tags')
+
     class Meta:
         model = 'music.Track'
 
@@ -57,6 +58,9 @@ class TrackFileFactory(factory.django.DjangoModelFactory):
         model = 'music.TrackFile'
 
     class Params:
+        in_place = factory.Trait(
+            audio_file=None,
+        )
         federation = factory.Trait(
             audio_file=None,
             library_track=factory.SubFactory(LibraryTrackFactory),
@@ -105,6 +109,10 @@ class ImportJobFactory(factory.django.DjangoModelFactory):
             status='finished',
             track_file=factory.SubFactory(TrackFileFactory),
         )
+        in_place = factory.Trait(
+            status='finished',
+            audio_file=None,
+        )
 
 
 @registry.register(name='music.FileImportJob')

api/funkwhale_api/music/metadata.py

-import mutagen
+from django import forms
 import arrow
+import mutagen
 
 NODEFAULT = object()
 
@@ -50,6 +51,13 @@ def convert_track_number(v):
     except (ValueError, AttributeError, IndexError):
         pass
 
+
+VALIDATION = {
+    'musicbrainz_artistid': forms.UUIDField(),
+    'musicbrainz_albumid': forms.UUIDField(),
+    'musicbrainz_recordingid': forms.UUIDField(),
+}
+
 CONF = {
     'OggVorbis': {
         'getter': lambda f, k: f[k][0],
@@ -146,4 +154,7 @@ class Metadata(object):
         converter = field_conf.get('to_application')
         if converter:
             v = converter(v)
+        field = VALIDATION.get(key)
+        if field:
+            v = field.to_python(v)
         return v

api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py

+# Generated by Django 2.0.3 on 2018-04-19 20:23
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('music', '0024_populate_uuid'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='trackfile',
+            name='source',
+            field=models.URLField(blank=True, max_length=500, null=True),
+        ),
+    ]

api/funkwhale_api/music/models.py

@@ -412,7 +412,7 @@ class TrackFile(models.Model):
     track = models.ForeignKey(
         Track, related_name='files', on_delete=models.CASCADE)
     audio_file = models.FileField(upload_to='tracks/%Y/%m/%d', max_length=255)
-    source = models.URLField(null=True, blank=True)
+    source = models.URLField(null=True, blank=True, max_length=500)
     creation_date = models.DateTimeField(default=timezone.now)
     modification_date = models.DateTimeField(auto_now=True)
     duration = models.IntegerField(null=True, blank=True)
@@ -463,6 +463,26 @@ class TrackFile(models.Model):
             self.mimetype = utils.guess_mimetype(self.audio_file)
         return super().save(**kwargs)
 
+    @property
+    def serve_from_source_path(self):
+        if not self.source or not self.source.startswith('file://'):
+            raise ValueError('Cannot serve this file from source')
+        serve_path = settings.MUSIC_DIRECTORY_SERVE_PATH
+        prefix = settings.MUSIC_DIRECTORY_PATH
+        if not serve_path or not prefix:
+            raise ValueError(
+                'You need to specify MUSIC_DIRECTORY_SERVE_PATH and '
+                'MUSIC_DIRECTORY_PATH to serve in-place imported files'
+            )
+        file_path = self.source.replace('file://', '', 1)
+        parts = os.path.split(file_path.replace(prefix, '', 1))
+        if parts[0] == '/':
+            parts = parts[1:]
+        return os.path.join(
+            serve_path,
+            *parts
+        )
+
 
 IMPORT_STATUS_CHOICES = (
     ('pending', 'Pending'),
@@ -507,6 +527,8 @@ class ImportBatch(models.Model):
     def update_status(self):
         old_status = self.status
         self.status = utils.compute_status(self.jobs.all())
+        if self.status == old_status:
+            return
         self.save(update_fields=['status'])
         if self.status != old_status and self.status == 'finished':
             from . import tasks

api/funkwhale_api/music/tasks.py

@@ -71,7 +71,7 @@ def import_track_from_remote(library_track):
         library_track.title, artist=artist, album=album)
 
 
-def _do_import(import_job, replace, use_acoustid=True):
+def _do_import(import_job, replace=False, use_acoustid=True):
     from_file = bool(import_job.audio_file)
     mbid = import_job.mbid
     acoustid_track_id = None
@@ -93,6 +93,9 @@ def _do_import(import_job, replace, use_acoustid=True):
         track = import_track_data_from_path(import_job.audio_file.path)
     elif import_job.library_track:
         track = import_track_from_remote(import_job.library_track)
+    elif import_job.source.startswith('file://'):
+        track = import_track_data_from_path(
+            import_job.source.replace('file://', '', 1))
     else:
         raise ValueError(
             'Not enough data to process import, '
@@ -123,7 +126,7 @@ def _do_import(import_job, replace, use_acoustid=True):
         else:
             # no downloading, we hotlink
             pass
-    else:
+    elif import_job.audio_file:
         track_file.download_file()
     track_file.save()
     import_job.status = 'finished'
@@ -133,7 +136,7 @@ def _do_import(import_job, replace, use_acoustid=True):
         import_job.audio_file.delete()
     import_job.save()
 
-    return track.pk
+    return track_file
 
 
 @celery.app.task(name='ImportJob.run', bind=True)
@@ -147,7 +150,8 @@ def import_job_run(self, import_job, replace=False, use_acoustid=True):
         import_job.save(update_fields=['status'])
 
     try:
-        return _do_import(import_job, replace, use_acoustid=use_acoustid)
+        tf = _do_import(import_job, replace, use_acoustid=use_acoustid)
+        return tf.pk if tf else None
     except Exception as exc:
         if not settings.DEBUG:
             try:

api/funkwhale_api/music/utils.py

@@ -53,10 +53,11 @@ def guess_mimetype(f):
 
 
 def compute_status(jobs):
-    errored = any([job.status == 'errored' for job in jobs])
+    statuses = jobs.order_by().values_list('status', flat=True).distinct()
+    errored = any([status == 'errored' for status in statuses])
     if errored:
         return 'errored'
-    pending = any([job.status == 'pending' for job in jobs])
+    pending = any([status == 'pending' for status in statuses])
     if pending:
         return 'pending'
     return 'finished'

api/funkwhale_api/music/views.py

@@ -23,13 +23,14 @@ from rest_framework import permissions
 from musicbrainzngs import ResponseError
 
 from funkwhale_api.common import utils as funkwhale_utils
-from funkwhale_api.federation import actors
-from funkwhale_api.requests.models import ImportRequest
-from funkwhale_api.musicbrainz import api
 from funkwhale_api.common.permissions import (
     ConditionalAuthentication, HasModelPermission)
 from taggit.models import Tag
+from funkwhale_api.federation import actors
 from funkwhale_api.federation.authentication import SignatureAuthentication
+from funkwhale_api.federation.models import LibraryTrack
+from funkwhale_api.musicbrainz import api
+from funkwhale_api.requests.models import ImportRequest
 
 from . import filters
 from . import forms
@@ -195,12 +196,13 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
 
     @detail_route(methods=['get'])
     def serve(self, request, *args, **kwargs):
+        queryset = models.TrackFile.objects.select_related(
+            'library_track',
+            'track__album__artist',
+            'track__artist',
+        )
         try:
-            f = models.TrackFile.objects.select_related(
-                'library_track',
-                'track__album__artist',
-                'track__artist',
-            ).get(pk=kwargs['pk'])
+            f = queryset.get(pk=kwargs['pk'])
         except models.TrackFile.DoesNotExist:
             return Response(status=404)
 
@@ -213,14 +215,30 @@ class TrackFileViewSet(viewsets.ReadOnlyModelViewSet):
         if library_track and not audio_file:
             if not library_track.audio_file:
                 # we need to populate from cache
-                library_track.download_audio()
+                with transaction.atomic():
+                    # why the transaction/select_for_update?
+                    # this is because browsers may send multiple requests
+                    # in a short time range, for partial content,
+                    # thus resulting in multiple downloads from the remote
+                    qs = LibraryTrack.objects.select_for_update()
+                    library_track = qs.get(pk=library_track.pk)
+                    library_track.download_audio()
             audio_file = library_track.audio_file
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH,
+                audio_file.url)
             mt = library_track.audio_mimetype
+        elif audio_file:
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH,
+                audio_file.url)
+        elif f.source and f.source.startswith('file://'):
+            file_path = '{}{}'.format(
+                settings.PROTECT_FILES_PATH + '/music',
+                f.serve_from_source_path)
         response = Response()
         filename = f.filename
-        response['X-Accel-Redirect'] = "{}{}".format(
-            settings.PROTECT_FILES_PATH,
-            audio_file.url)
+        response['X-Accel-Redirect'] = file_path
         filename = "filename*=UTF-8''{}".format(
             urllib.parse.quote(filename))
         response["Content-Disposition"] = "attachment; {}".format(filename)

api/funkwhale_api/providers/audiofile/management/commands/import_files.py

 import glob
 import os
 
+from django.conf import settings
 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 models
 from funkwhale_api.music import tasks
 from funkwhale_api.users.models import User
 
@@ -39,7 +39,20 @@ class Command(BaseCommand):
             action='store_true',
             dest='exit_on_failure',
             default=False,
-            help='use this flag to disable error catching',
+            help='Use this flag to disable error catching',
+        )
+        parser.add_argument(
+            '--in-place', '-i',
+            action='store_true',
+            dest='in_place',
+            default=False,
+            help=(
+                'Import files without duplicating them into the media directory.'
+                'For in-place import to work, the music files must be readable'
+                'by the web-server and funkwhale api and celeryworker processes.'
+                'You may want to use this if you have a big music library to '
+                'import and not much disk space available.'
+            )
         )
         parser.add_argument(
             '--no-acoustid',
@@ -54,21 +67,29 @@ class Command(BaseCommand):
         )
 
     def handle(self, *args, **options):
-        # self.stdout.write(self.style.SUCCESS('Successfully closed poll "%s"' % poll_id))
-
-        # Recursive is supported only on Python 3.5+, so we pass the option
-        # only if it's True to avoid breaking on older versions of Python
         glob_kwargs = {}
         if options['recursive']:
             glob_kwargs['recursive'] = True
         try:
-            matching = glob.glob(options['path'], **glob_kwargs)
+            matching = sorted(glob.glob(options['path'], **glob_kwargs))
         except TypeError:
             raise Exception('You need Python 3.5 to use the --recursive flag')
 
-        self.stdout.write('This will import {} files matching this pattern: {}'.format(
-            len(matching), options['path']))
-
+        if options['in_place']:
+            self.stdout.write(
+                'Checking imported paths against settings.MUSIC_DIRECTORY_PATH')
+            p = settings.MUSIC_DIRECTORY_PATH
+            if not p:
+                raise CommandError(
+                    'Importing in-place requires setting the '
+                    'MUSIC_DIRECTORY_PATH variable')
+            for m in matching:
+                if not m.startswith(p):
+                    raise CommandError(
+                        'Importing in-place only works if importing'
+                        'from {} (MUSIC_DIRECTORY_PATH), as this directory'
+                        'needs to be accessible by the webserver.'
+                        'Culprit: {}'.format(p, m))
         if not matching:
             raise CommandError('No file matching pattern, aborting')
 
@@ -86,6 +107,24 @@ class Command(BaseCommand):
             except AssertionError:
                 raise CommandError(
                     'No superuser available, please provide a --username')
+
+        filtered = self.filter_matching(matching, options)
+        self.stdout.write('Import summary:')
+        self.stdout.write('- {} files found matching this pattern: {}'.format(
+            len(matching), options['path']))
+        self.stdout.write('- {} files already found in database'.format(
+            len(filtered['skipped'])))
+        self.stdout.write('- {} new files'.format(
+            len(filtered['new'])))
+
+        self.stdout.write('Selected options: {}'.format(', '.join([
+            'no acoustid' if options['no_acoustid'] else 'use acoustid',
+            'in place' if options['in_place'] else 'copy music files',
+        ])))
+        if len(filtered['new']) == 0:
+            self.stdout.write('Nothing new to import, exiting')
+            return
+
         if options['interactive']:
             message = (
                 'Are you sure you want to do this?\n\n'
@@ -94,27 +133,52 @@ class Command(BaseCommand):
             if input(''.join(message)) != 'yes':
                 raise CommandError("Import cancelled.")
 
-        batch = self.do_import(matching, user=user, options=options)
+        batch, errors = self.do_import(
+            filtered['new'], user=user, options=options)
         message = 'Successfully imported {} tracks'
         if options['async']:
             message = 'Successfully launched import for {} tracks'
-        self.stdout.write(message.format(len(matching)))
+
+        self.stdout.write(message.format(len(filtered['new'])))
+        if len(errors) > 0:
+            self.stderr.write(
+                '{} tracks could not be imported:'.format(len(errors)))
+
+            for path, error in errors:
+                self.stderr.write('- {}: {}'.format(path, error))
         self.stdout.write(
             "For details, please refer to import batch #{}".format(batch.pk))
 
-    @transaction.atomic
-    def do_import(self, matching, user, options):
-        message = 'Importing {}...'
+    def filter_matching(self, matching, options):
+        sources = ['file://{}'.format(p) for p in matching]
+        # we skip reimport for path that are already found
+        # as a TrackFile.source
+        existing = models.TrackFile.objects.filter(source__in=sources)
+        existing = existing.values_list('source', flat=True)
+        existing = set([p.replace('file://', '', 1) for p in existing])
+        skipped = set(matching) & existing
+        result = {
+            'initial': matching,
+            'skipped': list(sorted(skipped)),
+            'new': list(sorted(set(matching) - skipped)),
+        }
+        return result
+
+    def do_import(self, paths, user, options):
+        message = '{i}/{total} Importing {path}...'
         if options['async']:
-            message = 'Launching import for {}...'
+            message = '{i}/{total} Launching import for {path}...'
 
         # we create an import batch binded to the user
-        batch = user.imports.create(source='shell')
         async = options['async']
         import_handler = tasks.import_job_run.delay if async else tasks.import_job_run
-        for path in matching:
+        batch = user.imports.create(source='shell')
+        total = len(paths)
+        errors = []
+        for i, path in list(enumerate(paths)):
             try:
-                self.stdout.write(message.format(path))
+                self.stdout.write(
+                    message.format(path=path, i=i+1, total=len(paths)))
                 self.import_file(path, batch, import_handler, options)
             except Exception as e:
                 if options['exit_on_failure']:
@@ -122,18 +186,19 @@ class Command(BaseCommand):
                 m = 'Error while importing {}: {} {}'.format(
                     path, e.__class__.__name__, e)
                 self.stderr.write(m)
-        return batch
+                errors.append((path, '{} {}'.format(e.__class__.__name__, e)))
+        return batch, errors
 
     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))
+        if not options['in_place']:
+            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,
+            job.save()
+        import_handler(
             import_job_id=job.pk,
             use_acoustid=not options['no_acoustid'])

api/funkwhale_api/providers/audiofile/tasks.py

@@ -2,12 +2,14 @@ import acoustid
 import os
 import datetime
 from django.core.files import File
+from django.db import transaction
 
 from funkwhale_api.taskapp import celery
 from funkwhale_api.providers.acoustid import get_acoustid_client
 from funkwhale_api.music import models, metadata
 
 
+@transaction.atomic
 def import_track_data_from_path(path):
     data = metadata.Metadata(path)
     artist = models.Artist.objects.get_or_create(
@@ -45,6 +47,7 @@ def import_track_data_from_path(path):
 def import_metadata_with_musicbrainz(path):
     pass
 
+
 @celery.app.task(name='audiofile.from_path')
 def from_path(path):
     acoustid_track_id = None

api/tests/music/test_import.py

@@ -231,3 +231,15 @@ def test_import_batch_notifies_followers(
         on_behalf_of=library_actor,
         to=[f1.actor.url]
     )
+
+
+def test__do_import_in_place_mbid(factories, tmpfile):
+    path = '/test.ogg'
+    job = factories['music.ImportJob'](
+        in_place=True, source='file:///test.ogg')
+
+    track = factories['music.Track'](mbid=job.mbid)
+    tf = tasks._do_import(job, use_acoustid=False)
+
+    assert bool(tf.audio_file) is False
+    assert tf.source == 'file:///test.ogg'

api/tests/music/test_metadata.py

 import datetime
 import os
 import pytest
+import uuid
 
 from funkwhale_api.music import metadata
 
@@ -13,9 +14,9 @@ DATA_DIR = os.path.dirname(os.path.abspath(__file__))
     ('album', 'Peer Gynt Suite no. 1, op. 46'),
     ('date', datetime.date(2012, 8, 15)),
     ('track_number', 1),
-    ('musicbrainz_albumid', 'a766da8b-8336-47aa-a3ee-371cc41ccc75'),
-    ('musicbrainz_recordingid', 'bd21ac48-46d8-4e78-925f-d9cc2a294656'),
-    ('musicbrainz_artistid', '013c8e5b-d72a-4cd3-8dee-6c64d6125823'),
+    ('musicbrainz_albumid', uuid.UUID('a766da8b-8336-47aa-a3ee-371cc41ccc75')),
+    ('musicbrainz_recordingid', uuid.UUID('bd21ac48-46d8-4e78-925f-d9cc2a294656')),
+    ('musicbrainz_artistid', uuid.UUID('013c8e5b-d72a-4cd3-8dee-6c64d6125823')),
 ])
 def test_can_get_metadata_from_ogg_file(field, value):
     path = os.path.join(DATA_DIR, 'test.ogg')
@@ -30,9 +31,9 @@ def test_can_get_metadata_from_ogg_file(field, value):
     ('album', 'You Can\'t Stop Da Funk'),
     ('date', datetime.date(2006, 2, 7)),
     ('track_number', 1),
-    ('musicbrainz_albumid', 'ce40cdb1-a562-4fd8-a269-9269f98d4124'),
-    ('musicbrainz_recordingid', 'f269d497-1cc0-4ae4-a0c4-157ec7d73fcb'),
-    ('musicbrainz_artistid', '9c6bddde-6228-4d9f-ad0d-03f6fcb19e13'),
+    ('musicbrainz_albumid', uuid.UUID('ce40cdb1-a562-4fd8-a269-9269f98d4124')),
+    ('musicbrainz_recordingid', uuid.UUID('f269d497-1cc0-4ae4-a0c4-157ec7d73fcb')),
+    ('musicbrainz_artistid', uuid.UUID('9c6bddde-6228-4d9f-ad0d-03f6fcb19e13')),
 ])
 def test_can_get_metadata_from_id3_mp3_file(field, value):
     path = os.path.join(DATA_DIR, 'test.mp3')

api/tests/music/test_views.py

@@ -93,6 +93,25 @@ def test_can_proxy_remote_track(
     assert library_track.audio_file.read() == b'test'
 
 
+def test_can_serve_in_place_imported_file(
+        factories, settings, api_client, r_mock):
+    settings.PROTECT_AUDIO_FILES = False
+    settings.MUSIC_DIRECTORY_SERVE_PATH = '/host/music'
+    settings.MUSIC_DIRECTORY_PATH = '/music'
+    settings.MUSIC_DIRECTORY_PATH = '/music'
+    track_file = factories['music.TrackFile'](
+        in_place=True,
+        source='file:///music/test.ogg')
+
+    response = api_client.get(track_file.path)
+
+    assert response.status_code == 200
+    assert response['X-Accel-Redirect'] == '{}{}'.format(
+        settings.PROTECT_FILES_PATH,
+        '/music/host/music/test.ogg'
+    )
+
+
 def test_can_create_import_from_federation_tracks(
         factories, superuser_api_client, mocker):
     lts = factories['federation.LibraryTrack'].create_batch(size=5)

api/tests/test_import_audio_file.py

@@ -2,6 +2,8 @@ import pytest
 import acoustid
 import datetime
 import os
+import uuid
+
 from django.core.management import call_command
 from django.core.management.base import CommandError
 
@@ -15,7 +17,8 @@ DATA_DIR = os.path.join(
 
 
 def test_can_create_track_from_file_metadata(db, mocker):
-    mocker.patch('acoustid.match', side_effect=acoustid.WebServiceError('test'))
+    mocker.patch(
+        'acoustid.match', side_effect=acoustid.WebServiceError('test'))
     metadata = {
         'artist': ['Test artist'],
         'album': ['Test album'],
@@ -35,33 +38,49 @@ def test_can_create_track_from_file_metadata(db, mocker):
         os.path.join(DATA_DIR, 'dummy_file.ogg'))
 
     assert track.title == metadata['title'][0]
-    assert track.mbid == metadata['musicbrainz_trackid'][0]
+    assert track.mbid == uuid.UUID(metadata['musicbrainz_trackid'][0])
     assert track.position == 4
     assert track.album.title == metadata['album'][0]
-    assert track.album.mbid == metadata['musicbrainz_albumid'][0]
+    assert track.album.mbid == uuid.UUID(metadata['musicbrainz_albumid'][0])
     assert track.album.release_date == datetime.date(2012, 8, 15)
     assert track.artist.name == metadata['artist'][0]
-    assert track.artist.mbid == metadata['musicbrainz_artistid'][0]
+    assert track.artist.mbid == uuid.UUID(metadata['musicbrainz_artistid'][0])
 
 
 def test_management_command_requires_a_valid_username(factories, mocker):
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     user = factories['users.User'](username='me')
-    mocker.patch('funkwhale_api.providers.audiofile.management.commands.import_files.Command.do_import')  # NOQA
+    mocker.patch(
+        'funkwhale_api.providers.audiofile.management.commands.import_files.Command.do_import',  # noqa
+        return_value=(mocker.MagicMock(), []))
     with pytest.raises(CommandError):
         call_command('import_files', path, username='not_me', interactive=False)
     call_command('import_files', path, username='me', interactive=False)
 
 
+def test_in_place_import_only_from_music_dir(factories, settings):
+    user = factories['users.User'](username='me')
+    settings.MUSIC_DIRECTORY_PATH = '/nope'
+    path = os.path.join(DATA_DIR, 'dummy_file.ogg')
+    with pytest.raises(CommandError):
+        call_command(
+            'import_files',
+            path,
+            in_place=True,
+            username='me',
+            interactive=False
+        )
+
+
 def test_import_files_creates_a_batch_and_job(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
         interactive=False)
 
     batch = user.imports.latest('id')
@@ -76,45 +95,79 @@ 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,
         use_acoustid=True)
 
 
 def test_import_files_skip_acoustid(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'dummy_file.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
         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_import_files_skip_if_path_already_imported(factories, mocker):
+    user = factories['users.User'](username='me')
+    path = os.path.join(DATA_DIR, 'dummy_file.ogg')
+    existing = factories['music.TrackFile'](
+        source='file://{}'.format(path))
+
+    call_command(
+        'import_files',
+        path,
+        username='me',
+        async=False,
+        no_acoustid=True,
+        interactive=False)
+    assert user.imports.count() == 0
+
+
 def test_import_files_works_with_utf8_file_name(factories, mocker):
-    m = mocker.patch('funkwhale_api.common.utils.on_commit')
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
+    user = factories['users.User'](username='me')
+    path = os.path.join(DATA_DIR, 'utf8-éà◌.ogg')
+    call_command(
+        'import_files',
+        path,
+        username='me',
+        async=False,
+        no_acoustid=True,
+        interactive=False)
+    batch = user.imports.latest('id')
+    job = batch.jobs.first()
+    m.assert_called_once_with(
+        import_job_id=job.pk,
+        use_acoustid=False)
+
+
+def test_import_files_in_place(factories, mocker, settings):
+    settings.MUSIC_DIRECTORY_PATH = DATA_DIR
+    m = mocker.patch('funkwhale_api.music.tasks.import_job_run')
     user = factories['users.User'](username='me')
     path = os.path.join(DATA_DIR, 'utf8-éà◌.ogg')
     call_command(
         'import_files',
         path,
         username='me',
-        async=True,
+        async=False,
+        in_place=True,
         no_acoustid=True,
         interactive=False)
     batch = user.imports.latest('id')
     job = batch.jobs.first()
+    assert bool(job.audio_file) is False
     m.assert_called_once_with(
-        music_tasks.import_job_run.delay,
         import_job_id=job.pk,
         use_acoustid=False)
 

changes/changelog.d/124.bugfix

+Reset all sensitive front-end data on logout (#124)

changes/changelog.d/142.enhancement

+Increased max_length on TrackFile.source, this will help when importing files with a really long path (#142)

changes/changelog.d/144.enhancement

+Better file import performance and error handling (#144)