diff --git a/CHANGELOG b/CHANGELOG index fbf8d1fbff5a3350570db2b94b43806421dfbdd7..162246f758651520fb1868e7f0fb32a51b840230 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -2,7 +2,15 @@ Changelog ========= -0.2.7 (Unreleased) +0.3 (Unreleased) +------------------ + +- Revamped all import logic, everything is more tested and consistend +- Can now use Acoustid in file imports to automatically grab metadata from musicbrainz +- Brand new file import wizard + + +0.2.7 ------------------ - Shortcuts: can now use the ``f`` shortcut to toggle the currently playing track diff --git a/api/Dockerfile b/api/Dockerfile index 26a4ea3556c3e7bc8b9aa47e02fd890f5ca7650b..ef02669015cc6364522c8785e35956f557ab5d8e 100644 --- a/api/Dockerfile +++ b/api/Dockerfile @@ -6,8 +6,8 @@ ENV PYTHONUNBUFFERED 1 COPY ./requirements.apt /requirements.apt RUN apt-get update -qq && grep "^[^#;]" requirements.apt | xargs apt-get install -y - - +RUN curl -L https://github.com/acoustid/chromaprint/releases/download/v1.4.2/chromaprint-fpcalc-1.4.2-linux-x86_64.tar.gz | tar -xz -C /usr/local/bin --strip 1 +RUN fcalc yolofkjdssdhf COPY ./requirements/base.txt /requirements/base.txt RUN pip install -r /requirements/base.txt COPY ./requirements/production.txt /requirements/production.txt diff --git a/api/config/api_urls.py b/api/config/api_urls.py index a41c7bc0f3eb1b524f15b88f166b93141af2b1d9..d64eeb5fdbb62b5cdfaceaef8740a82cc9d9c2ef 100644 --- a/api/config/api_urls.py +++ b/api/config/api_urls.py @@ -15,6 +15,7 @@ router.register(r'trackfiles', views.TrackFileViewSet, 'trackfiles') router.register(r'artists', views.ArtistViewSet, 'artists') router.register(r'albums', views.AlbumViewSet, 'albums') router.register(r'import-batches', views.ImportBatchViewSet, 'import-batches') +router.register(r'import-jobs', views.ImportJobViewSet, 'import-jobs') router.register(r'submit', views.SubmitViewSet, 'submit') router.register(r'playlists', playlists_views.PlaylistViewSet, 'playlists') router.register( diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 7dffebe94ba9e528006778a1ca924626449e8f15..6f821dfba4de6a3b8af13dd7273c7a3e213cbf2c 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -47,7 +47,6 @@ THIRD_PARTY_APPS = ( 'corsheaders', 'rest_framework', 'rest_framework.authtoken', - 'djcelery', 'taggit', 'cachalot', 'rest_auth', @@ -68,6 +67,7 @@ LOCAL_APPS = ( 'funkwhale_api.playlists', 'funkwhale_api.providers.audiofile', 'funkwhale_api.providers.youtube', + 'funkwhale_api.providers.acoustid', ) # See: https://docs.djangoproject.com/en/dev/ref/settings/#installed-apps @@ -266,14 +266,14 @@ CACHES["default"]["OPTIONS"] = { ########## CELERY INSTALLED_APPS += ('funkwhale_api.taskapp.celery.CeleryConfig',) -BROKER_URL = env( +CELERY_BROKER_URL = env( "CELERY_BROKER_URL", default=env('CACHE_URL', default=CACHE_DEFAULT)) ########## END CELERY # Location of root django.contrib.admin URL, use {% url 'admin:index' %} ADMIN_URL = r'^admin/' # Your common stuff: Below this line define 3rd party library settings -CELERY_DEFAULT_RATE_LIMIT = 1 -CELERYD_TASK_TIME_LIMIT = 300 +CELERY_TASK_DEFAULT_RATE_LIMIT = 1 +CELERY_TASK_TIME_LIMIT = 300 import datetime JWT_AUTH = { 'JWT_ALLOW_REFRESH': True, diff --git a/api/config/settings/local.py b/api/config/settings/local.py index e0d497e793f90a2ab3b5ba7a9d52def7aba0d429..24ad871f75ae59f98f73a91a71fed87611afa942 100644 --- a/api/config/settings/local.py +++ b/api/config/settings/local.py @@ -54,7 +54,7 @@ TEST_RUNNER = 'django.test.runner.DiscoverRunner' ########## CELERY # In development, all tasks will be executed locally by blocking until the task returns -CELERY_ALWAYS_EAGER = False +CELERY_TASK_ALWAYS_EAGER = False ########## END CELERY # Your local stuff: Below this line define 3rd party library settings diff --git a/api/config/settings/test.py b/api/config/settings/test.py index 1056454968cc468ff05c5cf93bcd20c936d24c8e..db7b21415c6583a17472a9b84a61d9793e6cc0fa 100644 --- a/api/config/settings/test.py +++ b/api/config/settings/test.py @@ -23,7 +23,7 @@ CACHES = { } } -BROKER_URL = 'memory://' +CELERY_BROKER_URL = 'memory://' # TESTING # ------------------------------------------------------------------------------ @@ -31,7 +31,7 @@ TEST_RUNNER = 'django.test.runner.DiscoverRunner' ########## CELERY # In development, all tasks will be executed locally by blocking until the task returns -CELERY_ALWAYS_EAGER = True +CELERY_TASK_ALWAYS_EAGER = True ########## END CELERY # Your local stuff: Below this line define 3rd party library settings diff --git a/api/docker/Dockerfile.test b/api/docker/Dockerfile.test index b8539d329de0a199ea76ac3d1790ed8837da4ae9..08b437cf25d136579ab04bf50a484cd8bccb8883 100644 --- a/api/docker/Dockerfile.test +++ b/api/docker/Dockerfile.test @@ -7,6 +7,7 @@ ENV PYTHONDONTWRITEBYTECODE 1 COPY ./requirements.apt /requirements.apt COPY ./install_os_dependencies.sh /install_os_dependencies.sh RUN bash install_os_dependencies.sh install +RUN curl -L https://github.com/acoustid/chromaprint/releases/download/v1.4.2/chromaprint-fpcalc-1.4.2-linux-x86_64.tar.gz | tar -xz -C /usr/local/bin --strip 1 RUN mkdir /requirements COPY ./requirements/base.txt /requirements/base.txt diff --git a/api/funkwhale_api/common/permissions.py b/api/funkwhale_api/common/permissions.py index 6b61d9839d5dd7c14eb4c495b912e35e440cb68c..ecfea4c9a51582031a69c8592d4c2592cb2988e6 100644 --- a/api/funkwhale_api/common/permissions.py +++ b/api/funkwhale_api/common/permissions.py @@ -1,6 +1,6 @@ from django.conf import settings -from rest_framework.permissions import BasePermission +from rest_framework.permissions import BasePermission, DjangoModelPermissions class ConditionalAuthentication(BasePermission): @@ -9,3 +9,14 @@ class ConditionalAuthentication(BasePermission): if settings.API_AUTHENTICATION_REQUIRED: return request.user and request.user.is_authenticated return True + + +class HasModelPermission(DjangoModelPermissions): + """ + Same as DjangoModelPermissions, but we pin the model: + + class MyModelPermission(HasModelPermission): + model = User + """ + def get_required_permissions(self, method, model_cls): + return super().get_required_permissions(method, self.model) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index d776cd9459cd42fdbac62e7168b7262ef2718b0e..303e45228e2d851708d9bb2404dd19e01026918a 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -72,6 +72,14 @@ class ImportJobFactory(factory.django.DjangoModelFactory): model = 'music.ImportJob' +@registry.register(name='music.FileImportJob') +class FileImportJobFactory(ImportJobFactory): + source = 'file://' + mbid = None + audio_file = factory.django.FileField( + from_path=os.path.join(SAMPLES_PATH, 'test.ogg')) + + @registry.register class WorkFactory(factory.django.DjangoModelFactory): mbid = factory.Faker('uuid4') diff --git a/api/funkwhale_api/music/migrations/0016_trackfile_acoustid_track_id.py b/api/funkwhale_api/music/migrations/0016_trackfile_acoustid_track_id.py new file mode 100644 index 0000000000000000000000000000000000000000..21d8ce8ea4dfbade6a5bef39abb36b5cb80f6d15 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0016_trackfile_acoustid_track_id.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0 on 2017-12-26 16:39 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('music', '0015_bind_track_file_to_import_job'), + ] + + operations = [ + migrations.AddField( + model_name='trackfile', + name='acoustid_track_id', + field=models.UUIDField(blank=True, null=True), + ), + ] diff --git a/api/funkwhale_api/music/migrations/0017_auto_20171227_1728.py b/api/funkwhale_api/music/migrations/0017_auto_20171227_1728.py new file mode 100644 index 0000000000000000000000000000000000000000..dfca664370edffbcaecffd764b4f08289e21d4d1 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0017_auto_20171227_1728.py @@ -0,0 +1,28 @@ +# Generated by Django 2.0 on 2017-12-27 17:28 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('music', '0016_trackfile_acoustid_track_id'), + ] + + operations = [ + migrations.AddField( + model_name='importbatch', + name='source', + field=models.CharField(choices=[('api', 'api'), ('shell', 'shell')], default='api', max_length=30), + ), + migrations.AddField( + model_name='importjob', + name='audio_file', + field=models.FileField(blank=True, max_length=255, null=True, upload_to='imports/%Y/%m/%d'), + ), + migrations.AlterField( + model_name='importjob', + name='mbid', + field=models.UUIDField(blank=True, editable=False, null=True), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index b7cf0f25c82d8b6335b1fd30af7d96288f495de0..f919ff0eb3b0b0b6619699b5c65967096bbcec18 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -15,11 +15,9 @@ from django.utils import timezone from taggit.managers import TaggableManager from versatileimagefield.fields import VersatileImageField -from funkwhale_api.taskapp import celery from funkwhale_api import downloader from funkwhale_api import musicbrainz from . import importers -from . import lyrics as lyrics_utils class APIModelMixin(models.Model): @@ -255,14 +253,6 @@ class Lyrics(models.Model): url = models.URLField(unique=True) content = models.TextField(null=True, blank=True) - @celery.app.task(name='Lyrics.fetch_content', filter=celery.task_method) - def fetch_content(self): - html = lyrics_utils._get_html(self.url) - content = lyrics_utils.extract_content(html) - cleaned_content = lyrics_utils.clean_content(content) - self.content = cleaned_content - self.save() - @property def content_rendered(self): return markdown.markdown( @@ -362,6 +352,7 @@ class TrackFile(models.Model): audio_file = models.FileField(upload_to='tracks/%Y/%m/%d', max_length=255) source = models.URLField(null=True, blank=True) duration = models.IntegerField(null=True, blank=True) + acoustid_track_id = models.UUIDField(null=True, blank=True) def download_file(self): # import the track file, since there is not any @@ -393,9 +384,17 @@ class TrackFile(models.Model): class ImportBatch(models.Model): + IMPORT_BATCH_SOURCES = [ + ('api', 'api'), + ('shell', 'shell') + ] + source = models.CharField( + max_length=30, default='api', choices=IMPORT_BATCH_SOURCES) creation_date = models.DateTimeField(default=timezone.now) submitted_by = models.ForeignKey( - 'users.User', related_name='imports', on_delete=models.CASCADE) + 'users.User', + related_name='imports', + on_delete=models.CASCADE) class Meta: ordering = ['-creation_date'] @@ -406,8 +405,11 @@ class ImportBatch(models.Model): @property def status(self): pending = any([job.status == 'pending' for job in self.jobs.all()]) + errored = any([job.status == 'errored' for job in self.jobs.all()]) if pending: return 'pending' + if errored: + return 'errored' return 'finished' class ImportJob(models.Model): @@ -419,36 +421,17 @@ class ImportJob(models.Model): null=True, blank=True, on_delete=models.CASCADE) - source = models.URLField() - mbid = models.UUIDField(editable=False) + source = models.CharField(max_length=500) + mbid = models.UUIDField(editable=False, null=True, blank=True) STATUS_CHOICES = ( ('pending', 'Pending'), - ('finished', 'finished'), + ('finished', 'Finished'), + ('errored', 'Errored'), + ('skipped', 'Skipped'), ) status = models.CharField(choices=STATUS_CHOICES, default='pending', max_length=30) + audio_file = models.FileField( + upload_to='imports/%Y/%m/%d', max_length=255, null=True, blank=True) class Meta: ordering = ('id', ) - @celery.app.task(name='ImportJob.run', filter=celery.task_method) - def run(self, replace=False): - try: - track, created = Track.get_or_create_from_api(mbid=self.mbid) - track_file = None - if replace: - track_file = track.files.first() - elif track.files.count() > 0: - return - - track_file = track_file or TrackFile( - track=track, source=self.source) - track_file.download_file() - track_file.save() - self.status = 'finished' - self.track_file = track_file - self.save() - return track.pk - - except Exception as exc: - if not settings.DEBUG: - raise ImportJob.run.retry(args=[self], exc=exc, countdown=30, max_retries=3) - raise diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index cf9d8749021ca8f5a1c03f297924de2ed1a583ca..9f96dad0e300a64ae0d03eaf36cca177f8f89589 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -113,7 +113,8 @@ class ImportJobSerializer(serializers.ModelSerializer): track_file = TrackFileSerializer(read_only=True) class Meta: model = models.ImportJob - fields = ('id', 'mbid', 'source', 'status', 'track_file') + fields = ('id', 'mbid', 'batch', 'source', 'status', 'track_file', 'audio_file') + read_only_fields = ('status', 'track_file') class ImportBatchSerializer(serializers.ModelSerializer): @@ -121,3 +122,4 @@ class ImportBatchSerializer(serializers.ModelSerializer): class Meta: model = models.ImportBatch fields = ('id', 'jobs', 'status', 'creation_date') + read_only_fields = ('creation_date',) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..fc706c812fcd97145540747cd2fc4a3b11db56b6 --- /dev/null +++ b/api/funkwhale_api/music/tasks.py @@ -0,0 +1,102 @@ +from django.core.files.base import ContentFile + +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 + +from django.conf import settings +from . import models +from . import lyrics as lyrics_utils + + +@celery.app.task(name='acoustid.set_on_track_file') +@celery.require_instance(models.TrackFile, 'track_file') +def set_acoustid_on_track_file(track_file): + client = get_acoustid_client() + result = client.get_best_match(track_file.audio_file.path) + + def update(id): + track_file.acoustid_track_id = id + track_file.save(update_fields=['acoustid_track_id']) + return id + if result: + return update(result['id']) + + +def _do_import(import_job, replace): + 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: + # we try to deduce mbid from acoustid + client = get_acoustid_client() + match = client.get_best_match(import_job.audio_file.path) + if not match: + raise ValueError('Cannot get match') + duration = match['recordings'][0]['duration'] + mbid = match['recordings'][0]['id'] + acoustid_track_id = match['id'] + if mbid: + track, _ = models.Track.get_or_create_from_api(mbid=mbid) + else: + track = import_track_data_from_path(import_job.audio_file.path) + + track_file = None + if replace: + track_file = track.files.first() + elif track.files.count() > 0: + if import_job.audio_file: + import_job.audio_file.delete() + import_job.status = 'skipped' + import_job.save() + return + + track_file = track_file or models.TrackFile( + track=track, source=import_job.source) + track_file.acoustid_track_id = acoustid_track_id + if from_file: + track_file.audio_file = ContentFile(import_job.audio_file.read()) + track_file.audio_file.name = import_job.audio_file.name + track_file.duration = duration + else: + track_file.download_file() + track_file.save() + import_job.status = 'finished' + import_job.track_file = track_file + if import_job.audio_file: + # it's imported on the track, we don't need it anymore + import_job.audio_file.delete() + import_job.save() + return track.pk + + +@celery.app.task(name='ImportJob.run', bind=True) +@celery.require_instance(models.ImportJob, 'import_job') +def import_job_run(self, import_job, replace=False): + def mark_errored(): + import_job.status = 'errored' + import_job.save() + + try: + return _do_import(import_job, replace) + except Exception as exc: + if not settings.DEBUG: + try: + self.retry(exc=exc, countdown=30, max_retries=3) + except: + mark_errored() + raise + mark_errored() + raise + + +@celery.app.task(name='Lyrics.fetch_content') +@celery.require_instance(models.Lyrics, 'lyrics') +def fetch_content(lyrics): + html = lyrics_utils._get_html(lyrics.url) + content = lyrics_utils.extract_content(html) + cleaned_content = lyrics_utils.clean_content(content) + lyrics.content = cleaned_content + lyrics.save(update_fields=['content']) diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 532942e2e651c0262d96b61464b3411473f90fce..dd295ae79f66a2192f9770a24c0b2fb53c5ea49e 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -6,7 +6,7 @@ from django.urls import reverse from django.db import models, transaction from django.db.models.functions import Length from django.conf import settings -from rest_framework import viewsets, views +from rest_framework import viewsets, views, mixins from rest_framework.decorators import detail_route, list_route from rest_framework.response import Response from rest_framework import permissions @@ -15,13 +15,15 @@ from django.contrib.auth.decorators import login_required from django.utils.decorators import method_decorator from funkwhale_api.musicbrainz import api -from funkwhale_api.common.permissions import ConditionalAuthentication +from funkwhale_api.common.permissions import ( + ConditionalAuthentication, HasModelPermission) from taggit.models import Tag from . import models from . import serializers from . import importers from . import filters +from . import tasks from . import utils @@ -70,16 +72,45 @@ class AlbumViewSet(SearchMixin, viewsets.ReadOnlyModelViewSet): ordering_fields = ('creation_date',) -class ImportBatchViewSet(viewsets.ReadOnlyModelViewSet): +class ImportBatchViewSet( + mixins.CreateModelMixin, + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + viewsets.GenericViewSet): queryset = ( models.ImportBatch.objects.all() .prefetch_related('jobs__track_file') .order_by('-creation_date')) serializer_class = serializers.ImportBatchSerializer + permission_classes = (permissions.DjangoModelPermissions, ) def get_queryset(self): return super().get_queryset().filter(submitted_by=self.request.user) + def perform_create(self, serializer): + serializer.save(submitted_by=self.request.user) + + +class ImportJobPermission(HasModelPermission): + # not a typo, perms on import job is proxied to import batch + model = models.ImportBatch + + +class ImportJobViewSet( + mixins.CreateModelMixin, + viewsets.GenericViewSet): + queryset = (models.ImportJob.objects.all()) + serializer_class = serializers.ImportJobSerializer + permission_classes = (ImportJobPermission, ) + + def get_queryset(self): + return super().get_queryset().filter(batch__submitted_by=self.request.user) + + 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) + class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): """ @@ -129,7 +160,8 @@ class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): lyrics = work.fetch_lyrics() try: if not lyrics.content: - lyrics.fetch_content() + tasks.fetch_content(lyrics_id=lyrics.pk) + lyrics.refresh_from_db() except AttributeError: return Response({'error': 'unavailable lyrics'}, status=404) serializer = serializers.LyricsSerializer(lyrics) @@ -244,7 +276,7 @@ class SubmitViewSet(viewsets.ViewSet): pass batch = models.ImportBatch.objects.create(submitted_by=request.user) job = models.ImportJob.objects.create(mbid=request.POST['mbid'], batch=batch, source=request.POST['import_url']) - job.run.delay() + tasks.import_job_run.delay(import_job_id=job.pk) serializer = serializers.ImportBatchSerializer(batch) return Response(serializer.data) @@ -272,7 +304,7 @@ 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']) - job.run.delay() + 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/acoustid/__init__.py b/api/funkwhale_api/providers/acoustid/__init__.py new file mode 100644 index 0000000000000000000000000000000000000000..69fe058b3fa6495de9197efe1d39349c1efb801b --- /dev/null +++ b/api/funkwhale_api/providers/acoustid/__init__.py @@ -0,0 +1,27 @@ +import acoustid + +from dynamic_preferences.registries import global_preferences_registry + + +class Client(object): + def __init__(self, api_key): + self.api_key = api_key + + def match(self, file_path): + return acoustid.match(self.api_key, file_path, parse=False) + + def get_best_match(self, file_path): + results = self.match(file_path=file_path) + MIN_SCORE_FOR_MATCH = 0.8 + try: + rows = results['results'] + except KeyError: + return + for row in rows: + if row['score'] >= MIN_SCORE_FOR_MATCH: + return row + + +def get_acoustid_client(): + manager = global_preferences_registry.manager() + return Client(api_key=manager['providers_acoustid__api_key']) diff --git a/api/funkwhale_api/providers/acoustid/dynamic_preferences_registry.py b/api/funkwhale_api/providers/acoustid/dynamic_preferences_registry.py new file mode 100644 index 0000000000000000000000000000000000000000..da785df4065aff76cf8d1a4e9d19967b0b7bfa9c --- /dev/null +++ b/api/funkwhale_api/providers/acoustid/dynamic_preferences_registry.py @@ -0,0 +1,13 @@ +from dynamic_preferences.types import StringPreference, Section +from dynamic_preferences.registries import global_preferences_registry + +acoustid = Section('providers_acoustid') + + +@global_preferences_registry.register +class APIKey(StringPreference): + section = acoustid + name = 'api_key' + default = '' + verbose_name = 'Acoustid API key' + help_text = 'The API key used to query AcoustID. Get one at https://acoustid.org/new-application.' 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 a4bfe555ed56c3959dc90b598cc52a3b01420faf..4130e93f3805c8e644c542af699c4ad8e7d65146 100644 --- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py +++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py @@ -1,6 +1,10 @@ import glob +import os + +from django.core.files import File from django.core.management.base import BaseCommand, CommandError -from funkwhale_api.providers.audiofile import tasks +from funkwhale_api.music import tasks +from funkwhale_api.users.models import User class Command(BaseCommand): @@ -15,6 +19,11 @@ class Command(BaseCommand): default=False, help='Will match the pattern recursively (including subdirectories)', ) + parser.add_argument( + '--username', + dest='username', + help='The username of the user you want to be bound to the import', + ) parser.add_argument( '--async', action='store_true', @@ -46,6 +55,20 @@ class Command(BaseCommand): if not matching: raise CommandError('No file matching pattern, aborting') + user = None + if options['username']: + try: + user = User.objects.get(username=options['username']) + except User.DoesNotExist: + raise CommandError('Invalid username') + else: + # we bind the import to the first registered superuser + try: + user = User.objects.filter(is_superuser=True).order_by('pk').first() + assert user is not None + except AssertionError: + raise CommandError( + 'No superuser available, please provide a --username') if options['interactive']: message = ( 'Are you sure you want to do this?\n\n' @@ -54,18 +77,35 @@ class Command(BaseCommand): if input(''.join(message)) != 'yes': raise CommandError("Import cancelled.") + batch = self.do_import(matching, 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( + "For details, please refer to import batch #".format(batch.pk)) + + def do_import(self, matching, user, options): message = 'Importing {}...' if options['async']: message = 'Launching import for {}...' + # 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 for path in matching: - self.stdout.write(message.format(path)) + 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)) + + job.save() try: - tasks.from_path(path) + handler(import_job_id=job.pk) except Exception as e: self.stdout.write('Error: {}'.format(e)) - - message = 'Successfully imported {} tracks' - if options['async']: - message = 'Successfully launched import for {} tracks' - self.stdout.write(message.format(len(matching))) + return batch diff --git a/api/funkwhale_api/providers/audiofile/tasks.py b/api/funkwhale_api/providers/audiofile/tasks.py index 9e1b0fb3f8bec4f2f74a2d1a4e643c70d73eb8c2..bc18456c306731eee04e596b53b86c29d1cf87e5 100644 --- a/api/funkwhale_api/providers/audiofile/tasks.py +++ b/api/funkwhale_api/providers/audiofile/tasks.py @@ -1,20 +1,20 @@ +import acoustid import os import datetime from django.core.files import File from funkwhale_api.taskapp import celery +from funkwhale_api.providers.acoustid import get_acoustid_client from funkwhale_api.music import models, metadata -@celery.app.task(name='audiofile.from_path') -def from_path(path): +def import_track_data_from_path(path): data = metadata.Metadata(path) artist = models.Artist.objects.get_or_create( name__iexact=data.get('artist'), defaults={ 'name': data.get('artist'), 'mbid': data.get('musicbrainz_artistid', None), - }, )[0] @@ -39,11 +39,33 @@ def from_path(path): 'mbid': data.get('musicbrainz_recordingid', None), }, )[0] + return track + + +def import_metadata_with_musicbrainz(path): + pass + +@celery.app.task(name='audiofile.from_path') +def from_path(path): + acoustid_track_id = None + try: + client = get_acoustid_client() + result = client.get_best_match(path) + acoustid_track_id = result['id'] + except acoustid.WebServiceError: + track = import_track_data_from_path(path) + except (TypeError, KeyError): + track = import_metadata_without_musicbrainz(path) + else: + track, created = models.Track.get_or_create_from_api( + mbid=result['recordings'][0]['id'] + ) if track.files.count() > 0: raise ValueError('File already exists for track {}'.format(track.pk)) - track_file = models.TrackFile(track=track) + track_file = models.TrackFile( + track=track, acoustid_track_id=acoustid_track_id) track_file.audio_file.save( os.path.basename(path), File(open(path, 'rb')) diff --git a/api/funkwhale_api/taskapp/celery.py b/api/funkwhale_api/taskapp/celery.py index 795262e50de594e2f778c3f942ad915dc8cad034..12e5b24525baf753c56329a3bb12c808ae7f073d 100644 --- a/api/funkwhale_api/taskapp/celery.py +++ b/api/funkwhale_api/taskapp/celery.py @@ -1,10 +1,12 @@ from __future__ import absolute_import import os +import functools + from celery import Celery from django.apps import AppConfig from django.conf import settings -from celery.contrib.methods import task_method + if not settings.configured: # set the default Django settings module for the 'celery' program. @@ -21,12 +23,20 @@ class CeleryConfig(AppConfig): def ready(self): # Using a string here means the worker will not have to # pickle the object when using Windows. - app.config_from_object('django.conf:settings') + app.config_from_object('django.conf:settings', namespace='CELERY') app.autodiscover_tasks(lambda: settings.INSTALLED_APPS, force=True) - - -@app.task(bind=True) -def debug_task(self): - print('Request: {0!r}'.format(self.request)) # pragma: no cover +def require_instance(model_or_qs, parameter_name): + def decorator(function): + @functools.wraps(function) + def inner(*args, **kwargs): + pk = kwargs.pop('_'.join([parameter_name, 'id'])) + try: + instance = model_or_qs.get(pk=pk) + except AttributeError: + instance = model_or_qs.objects.get(pk=pk) + kwargs[parameter_name] = instance + return function(*args, **kwargs) + return inner + return decorator diff --git a/api/install_os_dependencies.sh b/api/install_os_dependencies.sh index aea9dec4554f82bb3eed46a5f7b53aebf2b50bda..91f3f7c3f0aa1321c45c7db65b5a5b09affbbc11 100755 --- a/api/install_os_dependencies.sh +++ b/api/install_os_dependencies.sh @@ -79,4 +79,3 @@ case "$1" in help) usage_message;; *) wrong_command $1;; esac - diff --git a/api/requirements.apt b/api/requirements.apt index 86c7d8b20c99e3cc60c7b06c40f49ac8fc4275d4..e28360b5658fe17d9f2a5afa4d6e28feedf87101 100644 --- a/api/requirements.apt +++ b/api/requirements.apt @@ -7,3 +7,4 @@ libpq-dev postgresql-client libav-tools python3-dev +curl diff --git a/api/requirements/base.txt b/api/requirements/base.txt index 7e56b6cfda24f977288de6ed3b91a5df42f1fde0..cff16d3f1de30ff37810d2c934923bb595d6abdf 100644 --- a/api/requirements/base.txt +++ b/api/requirements/base.txt @@ -24,7 +24,7 @@ django-redis>=4.5,<4.6 redis>=2.10,<2.11 -celery>=3.1,<3.2 +celery>=4.1,<4.2 # Your custom requirements go here @@ -33,7 +33,6 @@ musicbrainzngs==0.6 youtube_dl>=2017.12.14 djangorestframework>=3.7,<3.8 djangorestframework-jwt>=1.11,<1.12 -django-celery>=3.2,<3.3 django-mptt>=0.9,<0.10 google-api-python-client>=1.6,<1.7 arrow>=0.12,<0.13 @@ -57,3 +56,4 @@ git+https://github.com/EliotBerriot/PyMemoize.git@django git+https://github.com/EliotBerriot/django-cachalot.git@django-2 django-dynamic-preferences>=1.5,<1.6 +pyacoustid>=1.1.5,<1.2 diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 37f3dae3afd72f88db2d7f1c2a9c5a40a6f43cec..6c0cffa4e0da24ed87501a692464598563de2480 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -1,6 +1,10 @@ import tempfile import shutil import pytest +from django.core.cache import cache as django_cache +from dynamic_preferences.registries import global_preferences_registry + +from funkwhale_api.taskapp import celery @pytest.fixture(scope="session", autouse=True) @@ -11,12 +15,23 @@ def factories_autodiscover(): factories.registry.autodiscover(app_names) +@pytest.fixture(autouse=True) +def cache(): + yield django_cache + django_cache.clear() + + @pytest.fixture def factories(db): from funkwhale_api import factories yield factories.registry +@pytest.fixture +def preferences(db): + yield global_preferences_registry.manager() + + @pytest.fixture def tmpdir(): d = tempfile.mkdtemp() diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index e29aaf107c922f117e63c2f67c5ca613bce6733b..7a856efd1f21971ec7657417f462d007f053c80a 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -1,4 +1,5 @@ import json +import os import pytest from django.urls import reverse @@ -8,6 +9,8 @@ from funkwhale_api.music import serializers from . import data as api_data +DATA_DIR = os.path.dirname(os.path.abspath(__file__)) + def test_can_submit_youtube_url_for_track_import(mocker, superuser_client): mocker.patch( @@ -34,11 +37,11 @@ def test_can_submit_youtube_url_for_track_import(mocker, superuser_client): assert track.album.title == 'Marsupial Madness' -def test_import_creates_an_import_with_correct_data(superuser_client, settings): +def test_import_creates_an_import_with_correct_data(mocker, superuser_client): + mocker.patch('funkwhale_api.music.tasks.import_job_run') mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' video_id = 'tPEE9ZwTmy0' url = reverse('api:v1:submit-single') - settings.CELERY_ALWAYS_EAGER = False response = superuser_client.post( url, {'import_url': 'https://www.youtube.com/watch?v={0}'.format(video_id), @@ -54,7 +57,8 @@ def test_import_creates_an_import_with_correct_data(superuser_client, settings): assert job.source == 'https://www.youtube.com/watch?v={0}'.format(video_id) -def test_can_import_whole_album(mocker, superuser_client, settings): +def test_can_import_whole_album(mocker, superuser_client): + mocker.patch('funkwhale_api.music.tasks.import_job_run') mocker.patch( 'funkwhale_api.musicbrainz.api.artists.get', return_value=api_data.artists['get']['soad']) @@ -82,7 +86,6 @@ def test_can_import_whole_album(mocker, superuser_client, settings): ] } url = reverse('api:v1:submit-album') - settings.CELERY_ALWAYS_EAGER = False response = superuser_client.post( url, json.dumps(payload), content_type="application/json") @@ -109,7 +112,8 @@ def test_can_import_whole_album(mocker, superuser_client, settings): assert job.source == row['source'] -def test_can_import_whole_artist(mocker, superuser_client, settings): +def test_can_import_whole_artist(mocker, superuser_client): + mocker.patch('funkwhale_api.music.tasks.import_job_run') mocker.patch( 'funkwhale_api.musicbrainz.api.artists.get', return_value=api_data.artists['get']['soad']) @@ -142,7 +146,6 @@ def test_can_import_whole_artist(mocker, superuser_client, settings): ] } url = reverse('api:v1:submit-artist') - settings.CELERY_ALWAYS_EAGER = False response = superuser_client.post( url, json.dumps(payload), content_type="application/json") @@ -189,6 +192,48 @@ def test_user_can_query_api_for_his_own_batches(client, factories): assert results['results'][0]['jobs'][0]['mbid'] == job.mbid +def test_user_can_create_an_empty_batch(client, factories): + user = factories['users.SuperUser']() + url = reverse('api:v1:import-batches-list') + client.login(username=user.username, password='test') + response = client.post(url) + + assert response.status_code == 201 + + batch = user.imports.latest('id') + + assert batch.submitted_by == user + assert batch.source == 'api' + + +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') + user = factories['users.SuperUser']() + batch = factories['music.ImportBatch'](submitted_by=user) + url = reverse('api:v1:import-jobs-list') + client.login(username=user.username, password='test') + with open(path, 'rb') as f: + content = f.read() + f.seek(0) + response = client.post(url, { + 'batch': batch.pk, + 'audio_file': f, + 'source': 'file://' + }, format='multipart') + + assert response.status_code == 201 + + job = batch.jobs.latest('id') + + assert job.status == 'pending' + assert job.source.startswith('file://') + assert 'test.ogg' in job.source + assert job.audio_file.read() == content + + m.assert_called_once_with(import_job_id=job.pk) + + def test_can_search_artist(factories, client): artist1 = factories['music.Artist']() artist2 = factories['music.Artist']() diff --git a/api/tests/music/test_lyrics.py b/api/tests/music/test_lyrics.py index 3670a2e5c1bfb1e24b673c86613fe16073f0d201..d10d113d7741d1e53e64d9af856a4d466bab6d35 100644 --- a/api/tests/music/test_lyrics.py +++ b/api/tests/music/test_lyrics.py @@ -4,6 +4,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 funkwhale_api.music import lyrics as lyrics_utils from .mocking import lyricswiki @@ -18,7 +19,8 @@ def test_works_import_lyrics_if_any(mocker, factories): lyrics = factories['music.Lyrics']( url='http://lyrics.wikia.com/System_Of_A_Down:Chop_Suey!') - lyrics.fetch_content() + tasks.fetch_content(lyrics_id=lyrics.pk) + lyrics.refresh_from_db() self.assertIn( 'Grab a brush and put on a little makeup', lyrics.content, diff --git a/api/tests/music/test_models.py b/api/tests/music/test_models.py index 2ec192517bbf7d060baf37bbcbb5670f1bb2635a..165415465d1431d7294f065efccdb390b87b6b96 100644 --- a/api/tests/music/test_models.py +++ b/api/tests/music/test_models.py @@ -2,6 +2,7 @@ import pytest from funkwhale_api.music import models from funkwhale_api.music import importers +from funkwhale_api.music import tasks def test_can_store_release_group_id_on_album(factories): @@ -44,6 +45,6 @@ def test_import_job_is_bound_to_track_file(factories, mocker): job = factories['music.ImportJob'](mbid=track.mbid) mocker.patch('funkwhale_api.music.models.TrackFile.download_file') - job.run() + tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() assert job.track_file.track == track diff --git a/api/tests/music/test_tasks.py b/api/tests/music/test_tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..873ca18f02e32ee0c0c9b8c489c902bc5ab9f122 --- /dev/null +++ b/api/tests/music/test_tasks.py @@ -0,0 +1,153 @@ +import os +import pytest + +from funkwhale_api.providers.acoustid import get_acoustid_client +from funkwhale_api.music import tasks + +from . import data as api_data + +DATA_DIR = os.path.dirname(os.path.abspath(__file__)) + + +def test_set_acoustid_on_track_file(factories, mocker): + track_file = factories['music.TrackFile'](acoustid_track_id=None) + id = 'e475bf79-c1ce-4441-bed7-1e33f226c0a2' + payload = { + 'results': [ + {'id': id, + 'recordings': [ + {'artists': [ + {'id': '9c6bddde-6228-4d9f-ad0d-03f6fcb19e13', + 'name': 'Binärpilot'}], + 'duration': 268, + 'id': 'f269d497-1cc0-4ae4-a0c4-157ec7d73fcb', + 'title': 'Bend'}], + 'score': 0.860825}], + 'status': 'ok' + } + m = mocker.patch('acoustid.match', return_value=payload) + r = tasks.set_acoustid_on_track_file(track_file_id=track_file.pk) + track_file.refresh_from_db() + + assert str(track_file.acoustid_track_id) == id + assert r == id + m.assert_called_once_with('', track_file.audio_file.path, parse=False) + + +def test_set_acoustid_on_track_file_required_high_score(factories, mocker): + track_file = factories['music.TrackFile'](acoustid_track_id=None) + id = 'e475bf79-c1ce-4441-bed7-1e33f226c0a2' + payload = { + 'results': [{'score': 0.79}], + 'status': 'ok' + } + m = mocker.patch('acoustid.match', return_value=payload) + r = tasks.set_acoustid_on_track_file(track_file_id=track_file.pk) + track_file.refresh_from_db() + + assert track_file.acoustid_track_id is None + + +def test_import_job_can_run_with_file_and_acoustid(factories, mocker): + path = os.path.join(DATA_DIR, 'test.ogg') + mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' + acoustid_payload = { + 'results': [ + {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', + 'recordings': [ + { + 'duration': 268, + 'id': mbid}], + 'score': 0.860825}], + 'status': 'ok' + } + mocker.patch( + 'funkwhale_api.musicbrainz.api.artists.get', + return_value=api_data.artists['get']['adhesive_wombat']) + mocker.patch( + 'funkwhale_api.musicbrainz.api.releases.get', + return_value=api_data.albums['get']['marsupial']) + mocker.patch( + 'funkwhale_api.musicbrainz.api.recordings.search', + return_value=api_data.tracks['search']['8bitadventures']) + mocker.patch('acoustid.match', return_value=acoustid_payload) + + job = factories['music.FileImportJob'](audio_file__path=path) + f = job.audio_file + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + track_file = job.track_file + + with open(path, 'rb') as f: + assert track_file.audio_file.read() == f.read() + assert track_file.duration == 268 + # audio file is deleted from import job once persisted to audio file + assert not job.audio_file + assert job.status == 'finished' + assert job.source == 'file://' + + +def test_import_job_can_be_skipped(factories, mocker): + path = os.path.join(DATA_DIR, 'test.ogg') + mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' + track_file = factories['music.TrackFile'](track__mbid=mbid) + acoustid_payload = { + 'results': [ + {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', + 'recordings': [ + { + 'duration': 268, + 'id': mbid}], + 'score': 0.860825}], + 'status': 'ok' + } + mocker.patch( + 'funkwhale_api.musicbrainz.api.artists.get', + return_value=api_data.artists['get']['adhesive_wombat']) + mocker.patch( + 'funkwhale_api.musicbrainz.api.releases.get', + return_value=api_data.albums['get']['marsupial']) + mocker.patch( + 'funkwhale_api.musicbrainz.api.recordings.search', + return_value=api_data.tracks['search']['8bitadventures']) + mocker.patch('acoustid.match', return_value=acoustid_payload) + + job = factories['music.FileImportJob'](audio_file__path=path) + f = job.audio_file + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + assert job.track_file is None + # audio file is deleted from import job once persisted to audio file + assert not job.audio_file + assert job.status == 'skipped' + + +def test_import_job_can_be_errored(factories, mocker): + path = os.path.join(DATA_DIR, 'test.ogg') + mbid = '9968a9d6-8d92-4051-8f76-674e157b6eed' + track_file = factories['music.TrackFile'](track__mbid=mbid) + acoustid_payload = { + 'results': [ + {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', + 'recordings': [ + { + 'duration': 268, + 'id': mbid}], + 'score': 0.860825}], + 'status': 'ok' + } + class MyException(Exception): + pass + mocker.patch('acoustid.match', side_effect=MyException()) + + job = factories['music.FileImportJob']( + audio_file__path=path, track_file=None) + + with pytest.raises(MyException): + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + assert job.track_file is None + assert job.status == 'errored' diff --git a/api/tests/test_acoustid.py b/api/tests/test_acoustid.py new file mode 100644 index 0000000000000000000000000000000000000000..1f7de9247e226a13971641624b0a436b5718d743 --- /dev/null +++ b/api/tests/test_acoustid.py @@ -0,0 +1,34 @@ +from funkwhale_api.providers.acoustid import get_acoustid_client + + +def test_client_is_configured_with_correct_api_key(preferences): + api_key = 'hello world' + preferences['providers_acoustid__api_key'] = api_key + + client = get_acoustid_client() + assert client.api_key == api_key + + +def test_client_returns_raw_results(db, mocker, preferences): + api_key = 'test' + preferences['providers_acoustid__api_key'] = api_key + payload = { + 'results': [ + {'id': 'e475bf79-c1ce-4441-bed7-1e33f226c0a2', + 'recordings': [ + {'artists': [ + {'id': '9c6bddde-6228-4d9f-ad0d-03f6fcb19e13', + 'name': 'Binärpilot'}], + 'duration': 268, + 'id': 'f269d497-1cc0-4ae4-a0c4-157ec7d73fcb', + 'title': 'Bend'}], + 'score': 0.860825}], + 'status': 'ok' + } + + m = mocker.patch('acoustid.match', return_value=payload) + client = get_acoustid_client() + response = client.match('/tmp/noopfile.mp3') + + assert response == payload + m.assert_called_once_with('test', '/tmp/noopfile.mp3', parse=False) diff --git a/api/tests/test_disk_import.py b/api/tests/test_disk_import.py deleted file mode 100644 index 9aaf399750e3c756c9087d58e6e61931834ab19a..0000000000000000000000000000000000000000 --- a/api/tests/test_disk_import.py +++ /dev/null @@ -1,39 +0,0 @@ -import os -import datetime - -from funkwhale_api.providers.audiofile import tasks - -DATA_DIR = os.path.join( - os.path.dirname(os.path.abspath(__file__)), - 'files' -) - - -def test_can_import_single_audio_file(db, mocker): - metadata = { - 'artist': ['Test artist'], - 'album': ['Test album'], - 'title': ['Test track'], - 'TRACKNUMBER': ['4'], - 'date': ['2012-08-15'], - 'musicbrainz_albumid': ['a766da8b-8336-47aa-a3ee-371cc41ccc75'], - 'musicbrainz_trackid': ['bd21ac48-46d8-4e78-925f-d9cc2a294656'], - 'musicbrainz_artistid': ['013c8e5b-d72a-4cd3-8dee-6c64d6125823'], - } - - m1 = mocker.patch('mutagen.File', return_value=metadata) - m2 = mocker.patch( - 'funkwhale_api.music.metadata.Metadata.get_file_type', - return_value='OggVorbis', - ) - track_file = tasks.from_path(os.path.join(DATA_DIR, 'dummy_file.ogg')) - track = track_file.track - - assert track.title == metadata['title'][0] - assert track.mbid == 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.release_date == datetime.date(2012, 8, 15) - assert track.artist.name == metadata['artist'][0] - assert track.artist.mbid == metadata['musicbrainz_artistid'][0] diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py new file mode 100644 index 0000000000000000000000000000000000000000..e81e9b8141455ffe218dc7be469dcdb73462c718 --- /dev/null +++ b/api/tests/test_import_audio_file.py @@ -0,0 +1,77 @@ +import pytest +import acoustid +import datetime +import os +from django.core.management import call_command +from django.core.management.base import CommandError + +from funkwhale_api.providers.audiofile import tasks + +DATA_DIR = os.path.join( + os.path.dirname(os.path.abspath(__file__)), + 'files' +) + + +def test_can_create_track_from_file_metadata(db, mocker): + mocker.patch('acoustid.match', side_effect=acoustid.WebServiceError('test')) + metadata = { + 'artist': ['Test artist'], + 'album': ['Test album'], + 'title': ['Test track'], + 'TRACKNUMBER': ['4'], + 'date': ['2012-08-15'], + 'musicbrainz_albumid': ['a766da8b-8336-47aa-a3ee-371cc41ccc75'], + 'musicbrainz_trackid': ['bd21ac48-46d8-4e78-925f-d9cc2a294656'], + 'musicbrainz_artistid': ['013c8e5b-d72a-4cd3-8dee-6c64d6125823'], + } + m1 = mocker.patch('mutagen.File', return_value=metadata) + m2 = mocker.patch( + 'funkwhale_api.music.metadata.Metadata.get_file_type', + return_value='OggVorbis', + ) + track = tasks.import_track_data_from_path( + os.path.join(DATA_DIR, 'dummy_file.ogg')) + + assert track.title == metadata['title'][0] + assert track.mbid == 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.release_date == datetime.date(2012, 8, 15) + assert track.artist.name == metadata['artist'][0] + assert track.artist.mbid == 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 + 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_import_files_creates_a_batch_and_job(factories, mocker): + m = mocker.patch('funkwhale_api.music.tasks.import_job_run.delay') + user = factories['users.User'](username='me') + path = os.path.join(DATA_DIR, 'dummy_file.ogg') + call_command( + 'import_files', + path, + username='me', + async=True, + interactive=False) + + batch = user.imports.latest('id') + assert batch.source == 'shell' + assert batch.jobs.count() == 1 + + job = batch.jobs.first() + + assert job.status == 'pending' + with open(path, 'rb') as f: + assert job.audio_file.read() == f.read() + + assert job.source == 'file://' + path + m.assert_called_once_with(import_job_id=job.pk) diff --git a/api/tests/test_tasks.py b/api/tests/test_tasks.py new file mode 100644 index 0000000000000000000000000000000000000000..16088d53ff5e1ceb75307d62bdaa178dc90d3778 --- /dev/null +++ b/api/tests/test_tasks.py @@ -0,0 +1,33 @@ +import pytest + +from funkwhale_api.taskapp import celery + + +class Dummy: + @staticmethod + def noop(instance): + pass + + +def test_require_instance_decorator(factories, mocker): + user = factories['users.User']() + + @celery.require_instance(user.__class__, 'user') + def t(user): + Dummy.noop(user) + + m = mocker.patch.object(Dummy, 'noop') + t(user_id=user.pk) + + m.assert_called_once_with(user) + + +def test_require_instance_decorator_accepts_qs(factories, mocker): + user = factories['users.User'](is_active=False) + qs = user.__class__.objects.filter(is_active=True) + + @celery.require_instance(qs, 'user') + def t(user): + pass + with pytest.raises(user.__class__.DoesNotExist): + t(user_id=user.pk) diff --git a/deploy/nginx.conf b/deploy/nginx.conf index 90b0f000e256db13959c19658c3bbff6d26c7676..cf865a9ea7b3ef84eeb6eda8d6d65f509790eaf5 100644 --- a/deploy/nginx.conf +++ b/deploy/nginx.conf @@ -47,6 +47,8 @@ server { rewrite ^(.+)$ /index.html last; } location /api/ { + # this is needed if you have file import via upload enabled + client_max_body_size 30M; proxy_set_header Host $host; proxy_set_header X-Real-IP $remote_addr; proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for; diff --git a/dev.yml b/dev.yml index 44e38e32671073cbeb4d3c9d697a216c2b99b541..befc4b2434848aca07b1c594f221889a3b227d3d 100644 --- a/dev.yml +++ b/dev.yml @@ -30,7 +30,7 @@ services: links: - postgres - redis - command: python manage.py celery worker + command: celery -A funkwhale_api.taskapp worker -l debug environment: - "DJANGO_ALLOWED_HOSTS=localhost" - "DJANGO_SETTINGS_MODULE=config.settings.local" diff --git a/docker/nginx/conf.dev b/docker/nginx/conf.dev index 9c00fd76fc577c417e489c33a731baba1a2f9980..48436173bcb5ce236bc6a5cce4455467e3794039 100644 --- a/docker/nginx/conf.dev +++ b/docker/nginx/conf.dev @@ -30,7 +30,7 @@ http { server { listen 6001; charset utf-8; - + client_max_body_size 20M; location /_protected/media { internal; alias /protected/media; diff --git a/front/package.json b/front/package.json index 9af43238767258ddb082b17659b6b9d278dd7952..58c22a4082c4d37762c32a3e23e7edf453a7fbca 100644 --- a/front/package.json +++ b/front/package.json @@ -23,6 +23,7 @@ "vue-lazyload": "^1.1.4", "vue-resource": "^1.3.4", "vue-router": "^2.3.1", + "vue-upload-component": "^2.7.4", "vuedraggable": "^2.14.1", "vuex": "^3.0.1", "vuex-persistedstate": "^2.4.2" diff --git a/front/src/components/library/import/BatchDetail.vue b/front/src/components/library/import/BatchDetail.vue index 726ec9c3e29eed7e6e3aa8433e7140d3b2a40cc4..762074c107ff9a2f61fbccb92611041f5d9ecc6f 100644 --- a/front/src/components/library/import/BatchDetail.vue +++ b/front/src/components/library/import/BatchDetail.vue @@ -8,6 +8,7 @@ ['ui', {'active': batch.status === 'pending'}, {'warning': batch.status === 'pending'}, + {'error': batch.status === 'errored'}, {'success': batch.status === 'finished'}, 'progress']"> <div class="bar" :style="progressBarStyle"> @@ -37,7 +38,7 @@ </td> <td> <span - :class="['ui', {'yellow': job.status === 'pending'}, {'green': job.status === 'finished'}, 'label']">{{ job.status }}</span> + :class="['ui', {'yellow': job.status === 'pending'}, {'red': job.status === 'errored'}, {'green': job.status === 'finished'}, 'label']">{{ job.status }}</span> </td> <td> <router-link v-if="job.track_file" :to="{name: 'library.tracks.detail', params: {id: job.track_file.track }}">{{ job.track_file.track }}</router-link> @@ -89,7 +90,7 @@ export default { computed: { progress () { return this.batch.jobs.filter(j => { - return j.status === 'finished' + return j.status !== 'pending' }).length * 100 / this.batch.jobs.length }, progressBarStyle () { diff --git a/front/src/components/library/import/BatchList.vue b/front/src/components/library/import/BatchList.vue index c78f1ea4e19a28195e573880e491baaf81147c7c..f6e7b03e5c06323d5033a1278626efa262d8b123 100644 --- a/front/src/components/library/import/BatchList.vue +++ b/front/src/components/library/import/BatchList.vue @@ -32,7 +32,7 @@ <td>{{ result.jobs.length }}</td> <td> <span - :class="['ui', {'yellow': result.status === 'pending'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }}</span> + :class="['ui', {'yellow': result.status === 'pending'}, {'red': result.status === 'errored'}, {'green': result.status === 'finished'}, 'label']">{{ result.status }}</span> </td> </tr> </tbody> diff --git a/front/src/components/library/import/FileUpload.vue b/front/src/components/library/import/FileUpload.vue new file mode 100644 index 0000000000000000000000000000000000000000..4681d79322727f9341682f5b8586bc0685001676 --- /dev/null +++ b/front/src/components/library/import/FileUpload.vue @@ -0,0 +1,140 @@ +<template> + <div> + <div v-if="batch" class="ui two buttons"> + <file-upload-widget + class="ui icon button" + :post-action="uploadUrl" + :multiple="true" + :size="1024 * 1024 * 30" + :data="uploadData" + :drop="true" + extensions="ogg,mp3" + accept="audio/*" + v-model="files" + name="audio_file" + :thread="3" + @input-filter="inputFilter" + @input-file="inputFile" + ref="upload"> + <i class="upload icon"></i> + Select files to upload... + </file-upload-widget> + <button class="ui icon teal button" v-if="!$refs.upload || !$refs.upload.active" @click.prevent="$refs.upload.active = true"> + <i class="play icon" aria-hidden="true"></i> + Start Upload + </button> + <button type="button" class="ui icon yellow button" v-else @click.prevent="$refs.upload.active = false"> + <i class="pause icon" aria-hidden="true"></i> + Stop Upload + </button> + </div> + <div class="ui hidden divider"></div> + <p> + Once all your files are uploaded, simply head over <router-link :to="{name: 'library.import.batches.detail', params: {id: batch.id }}">import detail page</router-link> to check the import status. + </p> + <table class="ui single line table"> + <thead> + <tr> + <th>File name</th> + <th>Size</th> + <th>Status</th> + </tr> + </thead> + <tbody> + <tr v-for="(file, index) in files" :key="file.id"> + <td>{{ file.name }}</td> + <td>{{ file.size }}</td> + <td> + <span v-if="file.error" class="ui red label"> + {{ file.error }} + </span> + <span v-else-if="file.success" class="ui green label">Success</span> + <span v-else-if="file.active" class="ui yellow label">Uploading...</span> + <template v-else> + <span class="ui label">Pending</span> + <button class="ui tiny basic red icon button" @click.prevent="$refs.upload.remove(file)"><i class="delete icon"></i></button> + </template> + </td> + </tr> + </tbody> + </table> + </div> +</template> + +<script> +import Vue from 'vue' +import logger from '@/logging' +import FileUploadWidget from './FileUploadWidget' +import config from '@/config' + +export default { + components: { + FileUploadWidget + }, + data () { + return { + files: [], + uploadUrl: config.API_URL + 'import-jobs/', + batch: null + } + }, + mounted: function () { + this.createBatch() + }, + methods: { + inputFilter (newFile, oldFile, prevent) { + if (newFile && !oldFile) { + let extension = newFile.name.split('.').pop() + if (['ogg', 'mp3'].indexOf(extension) < 0) { + prevent() + } + } + }, + inputFile (newFile, oldFile) { + if (newFile && !oldFile) { + // add + console.log('add', newFile) + if (!this.batch) { + this.createBatch() + } + } + if (newFile && oldFile) { + // update + console.log('update', newFile) + } + if (!newFile && oldFile) { + // remove + console.log('remove', oldFile) + } + }, + createBatch () { + let self = this + let url = config.API_URL + 'import-batches/' + let resource = Vue.resource(url) + resource.save({}, {}).then((response) => { + self.batch = response.data + }, (response) => { + logger.default.error('error while launching creating batch') + }) + } + }, + computed: { + batchId: function () { + if (this.batch) { + return this.batch.id + } + return null + }, + uploadData: function () { + return { + 'batch': this.batchId, + 'source': 'file://' + } + } + } +} +</script> + +<!-- Add "scoped" attribute to limit CSS to this component only --> +<style scoped> +</style> diff --git a/front/src/components/library/import/FileUploadWidget.vue b/front/src/components/library/import/FileUploadWidget.vue new file mode 100644 index 0000000000000000000000000000000000000000..1de8090c9c91939776818aa22d4e0dd20db9188d --- /dev/null +++ b/front/src/components/library/import/FileUploadWidget.vue @@ -0,0 +1,34 @@ +<script> +import FileUpload from 'vue-upload-component' + +export default { + extends: FileUpload, + methods: { + uploadHtml5 (file) { + let form = new window.FormData() + let value + for (let key in file.data) { + value = file.data[key] + if (value && typeof value === 'object' && typeof value.toString !== 'function') { + if (value instanceof File) { + form.append(key, value, value.name) + } else { + form.append(key, JSON.stringify(value)) + } + } else if (value !== null && value !== undefined) { + form.append(key, value) + } + } + form.append(this.name, file.file, file.file.filename || file.name) + let xhr = new XMLHttpRequest() + xhr.open('POST', file.postAction) + xhr.setRequestHeader('Authorization', this.$store.getters['auth/header']) + return this.uploadXhr(xhr, file, form) + } + } +} +</script> + +<!-- Add "scoped" attribute to limit CSS to this component only --> +<style scoped> +</style> diff --git a/front/src/components/library/import/Main.vue b/front/src/components/library/import/Main.vue index 10f6f352af0717886a0e4dc96b2dabe77d846ab7..66ba8c66144db35f4d4c17dfcada5e4c89fcc2e4 100644 --- a/front/src/components/library/import/Main.vue +++ b/front/src/components/library/import/Main.vue @@ -39,8 +39,8 @@ </div> </div> <div class="field"> - <div class="ui disabled radio checkbox"> - <input type="radio" id="upload" value="upload" v-model="currentSource" disabled> + <div class="ui radio checkbox"> + <input type="radio" id="upload" value="upload" v-model="currentSource"> <label for="upload">File upload</label> </div> </div> @@ -84,8 +84,14 @@ </div> </div> <div v-if="currentStep === 2"> + <file-upload + ref="import" + v-if="currentSource == 'upload'" + ></file-upload> + <component ref="import" + v-if="currentSource == 'external'" :metadata="metadata" :is="importComponent" :backends="backends" @@ -119,6 +125,7 @@ import MetadataSearch from '@/components/metadata/Search' import ReleaseCard from '@/components/metadata/ReleaseCard' import ArtistCard from '@/components/metadata/ArtistCard' import ReleaseImport from './ReleaseImport' +import FileUpload from './FileUpload' import ArtistImport from './ArtistImport' import router from '@/router' @@ -130,7 +137,8 @@ export default { ArtistCard, ReleaseCard, ArtistImport, - ReleaseImport + ReleaseImport, + FileUpload }, props: { mbType: {type: String, required: false}, @@ -142,7 +150,7 @@ export default { currentType: this.mbType || 'artist', currentId: this.mbId, currentStep: 0, - currentSource: this.source || 'external', + currentSource: '', metadata: {}, isImporting: false, importData: {