diff --git a/.env.dev b/.env.dev index 3f904078c..c09262509 100644 --- a/.env.dev +++ b/.env.dev @@ -9,3 +9,4 @@ FUNKWHALE_HOSTNAME=localhost FUNKWHALE_PROTOCOL=http PYTHONDONTWRITEBYTECODE=true WEBPACK_DEVSERVER_PORT=8080 +MUSIC_DIRECTORY_PATH=/music diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 217047794..6a0f4b9d8 100644 --- a/.gitlab-ci.yml +++ b/.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: @@ -81,9 +83,9 @@ build_front: paths: - front/dist/ only: - - tags - - master - - develop + - tags@funkwhale/funkwhale + - master@funkwhale/funkwhale + - develop@funkwhale/funkwhale tags: - docker @@ -100,7 +102,7 @@ pages: paths: - public only: - - develop + - develop@funkwhale/funkwhale tags: - docker @@ -114,7 +116,7 @@ docker_develop: - docker build -t $IMAGE . - docker push $IMAGE only: - - develop + - develop@funkwhale/funkwhale tags: - dind @@ -128,9 +130,9 @@ build_api: - api script: echo Done! only: - - tags - - master - - develop + - tags@funkwhale/funkwhale + - master@funkwhale/funkwhale + - develop@funkwhale/funkwhale docker_release: @@ -144,6 +146,6 @@ docker_release: - docker push $IMAGE - docker push $IMAGE_LATEST only: - - tags + - tags@funkwhale/funkwhale tags: - dind diff --git a/CHANGELOG b/CHANGELOG index b230b1556..c56d58836 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -3,6 +3,88 @@ Changelog .. towncrier +0.10 (2018-04-23) +----------------- + +Features: + +- Can now import files in-place from the CLI importer (#155) + + +Enhancements: + +- Avoid downloading audio files multiple times from remote libraries (#163) +- Better file import performance and error handling (#144) +- Import job and batch API and front-end have been improved with better + performance, pagination and additional filters (#171) +- Increased max_length on TrackFile.source, this will help when importing files + with a really long path (#142) +- Player is back in Queue tab (#150) + + +Bugfixes: + +- Fail graciously when AP representation includes a null_value for mediaType +- Fix sidebar tabs not showing under small resolution under Chrome (#173) +- Fixed broken login due to badly configured Axios (#172) +- Fixed broken playlist modal after login (#155) +- Fixed queue reorder or track deletion restarting currently playing track + (#151) +- Radio will now append new track if you delete the last track in queue (#145) +- Reset all sensitive front-end data on logout (#124) +- Typos/not showing text due to i18n work (#175) + + +Documentation: + +- Better documentation for hardware requirements and memory usage (#165) + + +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:: + + # this is the path in the container + MUSIC_DIRECTORY_PATH=/music + # this is the path on the host + MUSIC_DIRECTORY_SERVE_PATH=/srv/funkwhale/data/music + + 0.9.1 (2018-04-17) ------------------ diff --git a/api/config/settings/common.py b/api/config/settings/common.py index a972ec7ef..de1d653cb 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -390,6 +390,12 @@ REST_FRAMEWORK = { ATOMIC_REQUESTS = False USE_X_FORWARDED_HOST = True USE_X_FORWARDED_PORT = True + +# Wether we should use Apache, Nginx (or other) headers when serving audio files +# Default to Nginx +REVERSE_PROXY_TYPE = env('REVERSE_PROXY_TYPE', default='nginx') +assert REVERSE_PROXY_TYPE in ['apache2', 'nginx'], 'Unsupported REVERSE_PROXY_TYPE' + # Wether we should check user permission before serving audio files (meaning # return an obfuscated url) # This require a special configuration on the reverse proxy side @@ -441,3 +447,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) diff --git a/api/funkwhale_api/__init__.py b/api/funkwhale_api/__init__.py index f3a544e46..596926919 100644 --- a/api/funkwhale_api/__init__.py +++ b/api/funkwhale_api/__init__.py @@ -1,3 +1,3 @@ # -*- coding: utf-8 -*- -__version__ = '0.9.1' +__version__ = '0.10' __version_info__ = tuple([int(num) if num.isdigit() else num for num in __version__.replace('-', '.', 1).split('.')]) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 735a101b4..00bb7d45b 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -708,23 +708,7 @@ class AudioSerializer(serializers.Serializer): except (KeyError, TypeError): raise serializers.ValidationError('Missing mediaType') - if not media_type.startswith('audio/'): - raise serializers.ValidationError('Invalid mediaType') - - return url - - def validate_url(self, v): - try: - url = v['href'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing href') - - try: - media_type = v['mediaType'] - except (KeyError, TypeError): - raise serializers.ValidationError('Missing mediaType') - - if not media_type.startswith('audio/'): + if not media_type or not media_type.startswith('audio/'): raise serializers.ValidationError('Invalid mediaType') return v diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index ea7ff64df..bc0c74a2d 100644 --- a/api/funkwhale_api/music/factories.py +++ b/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') diff --git a/api/funkwhale_api/music/filters.py b/api/funkwhale_api/music/filters.py index fbea3735a..752422e75 100644 --- a/api/funkwhale_api/music/filters.py +++ b/api/funkwhale_api/music/filters.py @@ -2,6 +2,7 @@ from django.db.models import Count from django_filters import rest_framework as filters +from funkwhale_api.common import fields from . import models @@ -28,6 +29,39 @@ class ArtistFilter(ListenableMixin): } +class ImportBatchFilter(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'submitted_by__username', + 'source', + ]) + + class Meta: + model = models.ImportBatch + fields = { + 'status': ['exact'], + 'source': ['exact'], + 'submitted_by': ['exact'], + } + + +class ImportJobFilter(filters.FilterSet): + q = fields.SearchFilter(search_fields=[ + 'batch__submitted_by__username', + 'source', + ]) + + class Meta: + model = models.ImportJob + fields = { + 'batch': ['exact'], + 'batch__status': ['exact'], + 'batch__source': ['exact'], + 'batch__submitted_by': ['exact'], + 'status': ['exact'], + 'source': ['exact'], + } + + class AlbumFilter(ListenableMixin): listenable = filters.BooleanFilter(name='_', method='filter_listenable') diff --git a/api/funkwhale_api/music/metadata.py b/api/funkwhale_api/music/metadata.py index 3748d5573..a20069783 100644 --- a/api/funkwhale_api/music/metadata.py +++ b/api/funkwhale_api/music/metadata.py @@ -1,5 +1,6 @@ -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 diff --git a/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py b/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py new file mode 100644 index 000000000..6b0230d50 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0025_auto_20180419_2023.py @@ -0,0 +1,18 @@ +# 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), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 4ec3ff427..98fc1965b 100644 --- a/api/funkwhale_api/music/models.py +++ b/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 diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index b5f69eb1d..b9ecfc50d 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -6,6 +6,7 @@ from funkwhale_api.activity import serializers as activity_serializers from funkwhale_api.federation import utils as federation_utils from funkwhale_api.federation.models import LibraryTrack from funkwhale_api.federation.serializers import AP_CONTEXT +from funkwhale_api.users.serializers import UserBasicSerializer from . import models @@ -90,6 +91,7 @@ class TrackSerializerNested(LyricsMixin): files = TrackFileSerializer(many=True, read_only=True) album = SimpleAlbumSerializer(read_only=True) tags = TagSerializer(many=True, read_only=True) + class Meta: model = models.Track fields = ('id', 'mbid', 'title', 'artist', 'files', 'album', 'tags', 'lyrics') @@ -108,6 +110,7 @@ class AlbumSerializerNested(serializers.ModelSerializer): class ArtistSerializerNested(serializers.ModelSerializer): albums = AlbumSerializerNested(many=True, read_only=True) tags = TagSerializer(many=True, read_only=True) + class Meta: model = models.Artist fields = ('id', 'mbid', 'name', 'albums', 'tags') @@ -121,18 +124,43 @@ class LyricsSerializer(serializers.ModelSerializer): class ImportJobSerializer(serializers.ModelSerializer): track_file = TrackFileSerializer(read_only=True) + class Meta: model = models.ImportJob - fields = ('id', 'mbid', 'batch', 'source', 'status', 'track_file', 'audio_file') + fields = ( + 'id', + 'mbid', + 'batch', + 'source', + 'status', + 'track_file', + 'audio_file') read_only_fields = ('status', 'track_file') class ImportBatchSerializer(serializers.ModelSerializer): - jobs = ImportJobSerializer(many=True, read_only=True) + submitted_by = UserBasicSerializer(read_only=True) + class Meta: model = models.ImportBatch - fields = ('id', 'jobs', 'status', 'creation_date', 'import_request') - read_only_fields = ('creation_date',) + fields = ( + 'id', + 'submitted_by', + 'source', + 'status', + 'creation_date', + 'import_request') + read_only_fields = ( + 'creation_date', 'submitted_by', 'source') + + def to_representation(self, instance): + repr = super().to_representation(instance) + try: + repr['job_count'] = instance.job_count + except AttributeError: + # Queryset was not annotated + pass + return repr class TrackActivitySerializer(activity_serializers.ModelSerializer): diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index bc5ab94f0..f2244d785 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/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,8 @@ def _do_import(import_job, replace, use_acoustid=True): else: # no downloading, we hotlink pass - else: + elif not import_job.audio_file and not import_job.source.startswith('file://'): + # not an implace import, and we have a source, so let's download it track_file.download_file() track_file.save() import_job.status = 'finished' @@ -133,7 +137,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 +151,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: diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py index af0e59ab4..7a851f7cc 100644 --- a/api/funkwhale_api/music/utils.py +++ b/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' diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index e8ace1b3a..af063da46 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -11,6 +11,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.conf import settings from django.db import models, transaction from django.db.models.functions import Length +from django.db.models import Count from django.http import StreamingHttpResponse from django.urls import reverse from django.utils.decorators import method_decorator @@ -23,13 +24,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 @@ -98,14 +100,14 @@ class ImportBatchViewSet( mixins.RetrieveModelMixin, viewsets.GenericViewSet): queryset = ( - models.ImportBatch.objects.all() - .prefetch_related('jobs__track_file') - .order_by('-creation_date')) + models.ImportBatch.objects + .select_related() + .order_by('-creation_date') + .annotate(job_count=Count('jobs')) + ) serializer_class = serializers.ImportBatchSerializer permission_classes = (permissions.DjangoModelPermissions, ) - - def get_queryset(self): - return super().get_queryset().filter(submitted_by=self.request.user) + filter_class = filters.ImportBatchFilter def perform_create(self, serializer): serializer.save(submitted_by=self.request.user) @@ -118,13 +120,30 @@ class ImportJobPermission(HasModelPermission): class ImportJobViewSet( mixins.CreateModelMixin, + mixins.ListModelMixin, viewsets.GenericViewSet): - queryset = (models.ImportJob.objects.all()) + queryset = (models.ImportJob.objects.all().select_related()) serializer_class = serializers.ImportJobSerializer permission_classes = (ImportJobPermission, ) + filter_class = filters.ImportJobFilter - def get_queryset(self): - return super().get_queryset().filter(batch__submitted_by=self.request.user) + @list_route(methods=['get']) + def stats(self, request, *args, **kwargs): + qs = models.ImportJob.objects.all() + filterset = filters.ImportJobFilter(request.GET, queryset=qs) + qs = filterset.qs + qs = qs.values('status').order_by('status') + qs = qs.annotate(status_count=Count('status')) + + data = {} + for row in qs: + data[row['status']] = row['status_count'] + + for s, _ in models.IMPORT_STATUS_CHOICES: + data.setdefault(s, 0) + + data['count'] = sum([v for v in data.values()]) + return Response(data) def perform_create(self, serializer): source = 'file://' + serializer.validated_data['audio_file'].name @@ -135,7 +154,8 @@ class ImportJobViewSet( ) -class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): +class TrackViewSet( + TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): """ A simple ViewSet for viewing and editing accounts. """ @@ -185,6 +205,25 @@ class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): return Response(serializer.data) +def get_file_path(audio_file): + t = settings.REVERSE_PROXY_TYPE + if t == 'nginx': + # we have to use the internal locations + try: + path = audio_file.url + except AttributeError: + # a path was given + path = '/music' + audio_file + return settings.PROTECT_FILES_PATH + path + if t == 'apache2': + try: + path = audio_file.path + except AttributeError: + # a path was given + path = audio_file + return path + + class TrackFileViewSet(viewsets.ReadOnlyModelViewSet): queryset = (models.TrackFile.objects.all().order_by('-id')) serializer_class = serializers.TrackFileSerializer @@ -195,12 +234,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 +253,29 @@ 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 = get_file_path(audio_file) mt = library_track.audio_mimetype + elif audio_file: + file_path = get_file_path(audio_file) + elif f.source and f.source.startswith('file://'): + file_path = get_file_path(f.serve_from_source_path) response = Response() filename = f.filename - response['X-Accel-Redirect'] = "{}{}".format( - settings.PROTECT_FILES_PATH, - audio_file.url) + mapping = { + 'nginx': 'X-Accel-Redirect', + 'apache2': 'X-Sendfile', + } + file_header = mapping[settings.REVERSE_PROXY_TYPE] + response[file_header] = file_path filename = "filename*=UTF-8''{}".format( urllib.parse.quote(filename)) response["Content-Disposition"] = "attachment; {}".format(filename) 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 dbc01289f..a2757c692 100644 --- a/api/funkwhale_api/providers/audiofile/management/commands/import_files.py +++ b/api/funkwhale_api/providers/audiofile/management/commands/import_files.py @@ -1,11 +1,11 @@ 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']) diff --git a/api/funkwhale_api/providers/audiofile/tasks.py b/api/funkwhale_api/providers/audiofile/tasks.py index bc18456c3..40114c877 100644 --- a/api/funkwhale_api/providers/audiofile/tasks.py +++ b/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 diff --git a/api/requirements/base.txt b/api/requirements/base.txt index b66e297a9..ac0586566 100644 --- a/api/requirements/base.txt +++ b/api/requirements/base.txt @@ -33,6 +33,7 @@ musicbrainzngs==0.6 youtube_dl>=2017.12.14 djangorestframework>=3.7,<3.8 djangorestframework-jwt>=1.11,<1.12 +oauth2client<4 google-api-python-client>=1.6,<1.7 arrow>=0.12,<0.13 persisting-theory>=0.2,<0.3 diff --git a/api/tests/music/test_api.py b/api/tests/music/test_api.py index 606720e13..cc6fe644b 100644 --- a/api/tests/music/test_api.py +++ b/api/tests/music/test_api.py @@ -181,30 +181,6 @@ def test_can_import_whole_artist( assert job.source == row['source'] -def test_user_can_query_api_for_his_own_batches( - superuser_api_client, factories): - factories['music.ImportJob']() - job = factories['music.ImportJob']( - batch__submitted_by=superuser_api_client.user) - url = reverse('api:v1:import-batches-list') - - response = superuser_api_client.get(url) - results = response.data - assert results['count'] == 1 - assert results['results'][0]['jobs'][0]['mbid'] == job.mbid - - -def test_user_cannnot_access_other_batches( - superuser_api_client, factories): - factories['music.ImportJob']() - job = factories['music.ImportJob']() - url = reverse('api:v1:import-batches-list') - - response = superuser_api_client.get(url) - results = response.data - assert results['count'] == 0 - - def test_user_can_create_an_empty_batch(superuser_api_client, factories): url = reverse('api:v1:import-batches-list') response = superuser_api_client.post(url) diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 2f22ed69a..65e0242fb 100644 --- a/api/tests/music/test_import.py +++ b/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' diff --git a/api/tests/music/test_metadata.py b/api/tests/music/test_metadata.py index 5df2dbcf1..342bc99b8 100644 --- a/api/tests/music/test_metadata.py +++ b/api/tests/music/test_metadata.py @@ -1,6 +1,7 @@ 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') diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 81f34fbe9..5d7589af0 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -76,6 +76,31 @@ def test_can_serve_track_file_as_remote_library_deny_not_following( assert response.status_code == 403 +def test_serve_file_apache(factories, api_client, settings): + settings.PROTECT_AUDIO_FILES = False + settings.REVERSE_PROXY_TYPE = 'apache2' + tf = factories['music.TrackFile']() + response = api_client.get(tf.path) + + assert response.status_code == 200 + assert response['X-Sendfile'] == tf.audio_file.path + + +def test_serve_file_apache_in_place(factories, api_client, settings): + settings.PROTECT_AUDIO_FILES = False + settings.REVERSE_PROXY_TYPE = 'apache2' + settings.MUSIC_DIRECTORY_PATH = '/music' + settings.MUSIC_DIRECTORY_SERVE_PATH = '/host/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-Sendfile'] == '/host/music/test.ogg' + + def test_can_proxy_remote_track( factories, settings, api_client, r_mock): settings.PROTECT_AUDIO_FILES = False @@ -93,6 +118,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) @@ -109,3 +153,46 @@ def test_can_create_import_from_federation_tracks( assert batch.jobs.count() == 5 for i, job in enumerate(batch.jobs.all()): assert job.library_track == lts[i] + + +def test_can_list_import_jobs(factories, superuser_api_client): + job = factories['music.ImportJob']() + url = reverse('api:v1:import-jobs-list') + response = superuser_api_client.get(url) + + assert response.status_code == 200 + assert response.data['results'][0]['id'] == job.pk + + +def test_import_job_stats(factories, superuser_api_client): + job1 = factories['music.ImportJob'](status='pending') + job2 = factories['music.ImportJob'](status='errored') + + url = reverse('api:v1:import-jobs-stats') + response = superuser_api_client.get(url) + expected = { + 'errored': 1, + 'pending': 1, + 'finished': 0, + 'skipped': 0, + 'count': 2, + } + assert response.status_code == 200 + assert response.data == expected + + +def test_import_job_stats_filter(factories, superuser_api_client): + job1 = factories['music.ImportJob'](status='pending') + job2 = factories['music.ImportJob'](status='errored') + + url = reverse('api:v1:import-jobs-stats') + response = superuser_api_client.get(url, {'batch': job1.batch.pk}) + expected = { + 'errored': 0, + 'pending': 1, + 'finished': 0, + 'skipped': 0, + 'count': 1, + } + assert response.status_code == 200 + assert response.data == expected diff --git a/api/tests/test_import_audio_file.py b/api/tests/test_import_audio_file.py index 67263e66d..8217ffa0b 100644 --- a/api/tests/test_import_audio_file.py +++ b/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=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_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=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( import_job_id=job.pk, use_acoustid=False) diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index cc4f357ca..3bcd2f6d5 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -20,6 +20,14 @@ services: restart: unless-stopped image: funkwhale/funkwhale:${FUNKWHALE_VERSION:-latest} env_file: .env + # Celery workers handle background tasks (such file imports or federation + # messaging). The more processes a worker gets, the more tasks + # can be processed in parallel. However, more processes also means + # a bigger memory footprint. + # By default, a worker will span a number of process equal to your number + # of CPUs. You can adjust this, by explicitly setting the --concurrency + # flag: + # celery -A funkwhale_api.taskapp worker -l INFO --concurrency=4 command: celery -A funkwhale_api.taskapp worker -l INFO links: - postgres diff --git a/deploy/env.prod.sample b/deploy/env.prod.sample index 9e9938500..54f2e1ef0 100644 --- a/deploy/env.prod.sample +++ b/deploy/env.prod.sample @@ -1,17 +1,22 @@ +# If you have any doubts about what a setting does, +# check https://docs.funkwhale.audio/configuration.html#configuration-reference + # If you're tweaking this file from the template, ensure you edit at least the # following variables: # - DJANGO_SECRET_KEY # - DJANGO_ALLOWED_HOSTS # - FUNKWHALE_URL - -# Additionaly, on non-docker setup **only**, you'll also have to tweak/uncomment those variables: +# On non-docker setup **only**, you'll also have to tweak/uncomment those variables: # - DATABASE_URL # - CACHE_URL # - STATIC_ROOT # - MEDIA_ROOT # # You **don't** need to update those variables on pure docker setups. - +# +# Additional options you may want to check: +# - MUSIC_DIRECTORY_PATH and MUSIC_DIRECTORY_SERVE_PATH if you plan to use +# in-place import # Docker only # ----------- @@ -19,7 +24,9 @@ # (it will be interpolated in docker-compose file) # You can comment or ignore this if you're not using docker FUNKWHALE_VERSION=latest +MUSIC_DIRECTORY_PATH=/music +# End of Docker-only configuration # General configuration # --------------------- @@ -34,6 +41,11 @@ FUNKWHALE_API_PORT=5000 # your instance FUNKWHALE_URL=https://yourdomain.funwhale +# Depending on the reverse proxy used in front of your funkwhale instance, +# the API will use different kind of headers to serve audio files +# Allowed values: nginx, apache2 +REVERSE_PROXY_TYPE=nginx + # API/Django configuration # Database configuration @@ -94,3 +106,9 @@ FEDERATION_ENABLED=True # means anyone can subscribe to your library and import your file, # use with caution. FEDERATION_MUSIC_NEEDS_APPROVAL=True + +# In-place import settings +# You can safely leave those settings uncommented if you don't plan to use +# in place imports. +# MUSIC_DIRECTORY_PATH= +# MUSIC_DIRECTORY_SERVE_PATH= diff --git a/deploy/funkwhale-worker.service b/deploy/funkwhale-worker.service index cb3c88307..4df60b5e9 100644 --- a/deploy/funkwhale-worker.service +++ b/deploy/funkwhale-worker.service @@ -8,6 +8,14 @@ User=funkwhale # adapt this depending on the path of your funkwhale installation WorkingDirectory=/srv/funkwhale/api EnvironmentFile=/srv/funkwhale/config/.env +# Celery workers handle background tasks (such file imports or federation +# messaging). The more processes a worker gets, the more tasks +# can be processed in parallel. However, more processes also means +# a bigger memory footprint. +# By default, a worker will span a number of process equal to your number +# of CPUs. You can adjust this, by explicitly setting the --concurrency +# flag: +# celery -A funkwhale_api.taskapp worker -l INFO --concurrency=4 ExecStart=/srv/funkwhale/virtualenv/bin/celery -A funkwhale_api.taskapp worker -l INFO [Install] diff --git a/deploy/nginx.conf b/deploy/nginx.conf index 1c304b493..b3a4c6aaf 100644 --- a/deploy/nginx.conf +++ b/deploy/nginx.conf @@ -84,6 +84,14 @@ server { alias /srv/funkwhale/data/media; } + location /_protected/music { + # this is an internal location that is used to serve + # audio files once correct permission / authentication + # has been checked on API side + internal; + alias /srv/funkwhale/data/music; + } + # Transcoding logic and caching location = /transcode-auth { include /etc/nginx/funkwhale_proxy.conf; diff --git a/dev.yml b/dev.yml index 2df7b44e6..264fc9534 100644 --- a/dev.yml +++ b/dev.yml @@ -65,7 +65,7 @@ services: - "CACHE_URL=redis://redis:6379/0" volumes: - ./api:/app - - ./data/music:/music + - "${MUSIC_DIRECTORY-./data/music}:/music:ro" networks: - internal api: @@ -78,7 +78,7 @@ services: command: python /app/manage.py runserver 0.0.0.0:12081 volumes: - ./api:/app - - ./data/music:/music + - "${MUSIC_DIRECTORY-./data/music}:/music:ro" environment: - "FUNKWHALE_HOSTNAME=${FUNKWHALE_HOSTNAME-localhost}" - "FUNKWHALE_HOSTNAME_SUFFIX=funkwhale.test" @@ -107,6 +107,7 @@ services: volumes: - ./docker/nginx/conf.dev:/etc/nginx/nginx.conf - ./docker/nginx/entrypoint.sh:/entrypoint.sh:ro + - "${MUSIC_DIRECTORY-./data/music}:/music:ro" - ./deploy/funkwhale_proxy.conf:/etc/nginx/funkwhale_proxy.conf.template:ro - ./api/funkwhale_api/media:/protected/media ports: diff --git a/docker/nginx/conf.dev b/docker/nginx/conf.dev index e832a5ae3..38c3de6c7 100644 --- a/docker/nginx/conf.dev +++ b/docker/nginx/conf.dev @@ -42,6 +42,10 @@ http { internal; alias /protected/media; } + location /_protected/music { + internal; + alias /music; + } location = /transcode-auth { # needed so we can authenticate transcode requests, but still # cache the result diff --git a/docs/configuration.rst b/docs/configuration.rst index 5883a2d17..c0de76f56 100644 --- a/docs/configuration.rst +++ b/docs/configuration.rst @@ -33,3 +33,44 @@ The URL should be ``/api/admin/dynamic_preferences/globalpreferencemodel/`` (pre If you plan to use acoustid and external imports (e.g. with the youtube backends), you should edit the corresponding settings in this interface. + +Configuration reference +----------------------- + +.. _setting-MUSIC_DIRECTORY_PATH: + +``MUSIC_DIRECTORY_PATH`` +^^^^^^^^^^^^^^^^^^^^^^^^ + +Default: ``None`` + +The path on your server where Funwkhale can import files using :ref:`in-place import +`. It must be readable by the webserver and funkwhale +api and worker processes. + +On docker installations, we recommend you use the default of ``/music`` +for this value. For non-docker installation, you can use any absolute path. +``/srv/funkwhale/data/music`` is a safe choice if you don't know what to use. + +.. note:: This path should not include any trailing slash + +.. _setting-MUSIC_DIRECTORY_SERVE_PATH: + +``MUSIC_DIRECTORY_SERVE_PATH`` +^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + +Default: :ref:`setting-MUSIC_DIRECTORY_PATH` + +When using Docker, the value of :ref:`MUSIC_DIRECTORY_PATH` in your containers +may differ from the real path on your host. Assuming you have the following directive +in your :file:`docker-compose.yml` file:: + + volumes: + - /srv/funkwhale/data/music:/music:ro + +Then, the value of :ref:`setting-MUSIC_DIRECTORY_SERVE_PATH` should be +``/srv/funkwhale/data``. This must be readable by the webserver. + +On non-docker setup, you don't need to configure this setting. + +.. note:: This path should not include any trailing slash diff --git a/docs/importing-music.rst b/docs/importing-music.rst index f09eea7b1..97dd13854 100644 --- a/docs/importing-music.rst +++ b/docs/importing-music.rst @@ -22,8 +22,15 @@ to the ``/music`` directory on the container: docker-compose run --rm api python manage.py import_files "/music/**/*.ogg" --recursive --noinput -For the best results, we recommand tagging your music collection through -`Picard `_ in order to have the best quality metadata. +The import command supports several options, and you can check the help to +get details:: + + docker-compose run --rm api python manage.py import_files --help + +.. note:: + + For the best results, we recommand tagging your music collection through + `Picard `_ in order to have the best quality metadata. .. note:: @@ -39,18 +46,39 @@ For the best results, we recommand tagging your music collection through At the moment, only OGG/Vorbis and MP3 files with ID3 tags are supported -.. note:: - The --recursive flag will work only on Python 3.5+, which is the default - version When using Docker or Debian 9. If you use an older version of Python, - remove the --recursive flag and use more explicit import patterns instead:: +.. _in-place-import: - # this will only import ogg files at the second level - "/srv/funkwhale/data/music/*/*.ogg" - # this will only import ogg files in the fiven directory - "/srv/funkwhale/data/music/System-of-a-down/*.ogg" +In-place import +^^^^^^^^^^^^^^^ +By default, the CLI-importer will copy imported files to Funkwhale's internal +storage. This means importing a 1Gb library will result in the same amount +of space being used by Funkwhale. +While this behaviour has some benefits (easier backups and configuration), +it's not always the best choice, especially if you have a huge library +to import and don't want to double your disk usage. + +The CLI importer supports an additional ``--in-place`` option that triggers the +following behaviour during import: + +1. Imported files are not store in funkwhale anymore +2. Instead, Funkwhale will store the file path and use it to serve the music + +Because those files are not managed by Funkwhale, we offer additional +configuration options to ensure the webserver can serve them properly: + +- :ref:`setting-MUSIC_DIRECTORY_PATH` +- :ref:`setting-MUSIC_DIRECTORY_SERVE_PATH` + +.. warning:: + + While in-place import is faster and less disk-space-hungry, it's also + more fragile: if, for some reason, you move or rename the source files, + Funkwhale will not be able to serve those files anymore. + + Thus, be especially careful when you manipulate the source files. Getting demo tracks ^^^^^^^^^^^^^^^^^^^ diff --git a/docs/installation/index.rst b/docs/installation/index.rst index 2e62c71ec..776c22424 100644 --- a/docs/installation/index.rst +++ b/docs/installation/index.rst @@ -12,9 +12,48 @@ The project relies on the following components and services to work: - A celery worker to run asynchronouse tasks (such as music import) - A celery scheduler to run recurrent tasks + +Hardware requirements +--------------------- + +Funkwhale is not especially CPU hungry, unless you're relying heavily +on the transcoding feature (which is basic and unoptimized at the moment). + +On a dockerized instance with 2 CPUs and a few active users, the memory footprint is around ~500Mb:: + + CONTAINER MEM USAGE + funkwhale_api_1 202.1 MiB + funkwhale_celerybeat_1 96.52 MiB + funkwhale_celeryworker_1 168.7 MiB + funkwhale_postgres_1 22.73 MiB + funkwhale_redis_1 1.496 MiB + +Thus, Funkwhale should run fine on commodity hardware, small hosting boxes and +Raspberry Pi. We lack real-world exemples of such deployments, so don't hesitate +do give us your feedback (either positive or negative). + +Software requirements +--------------------- + +Software requirements will vary depending of your installation method. For +Docker-based installations, the only requirement will be an Nginx reverse-proxy +that will expose your instance to the outside world. + +If you plan to install your Funkwhale instance without Docker, most of the +dependencies should be available in your distribution's repositories. + +.. note:: + + Funkwhale works only with Pyhon >= 3.5, as we need support for async/await. + Older versions of Python are not supported. + + Available installation methods ------------------------------- +Docker is the recommended and easiest way to setup your Funkwhale instance. +We also maintain an installation guide for Debian 9. + .. toctree:: :maxdepth: 1 @@ -67,3 +106,24 @@ Then, download our sample virtualhost file and proxy conf: curl -L -o /etc/nginx/sites-enabled/funkwhale.conf "https://code.eliotberriot.com/funkwhale/funkwhale/raw/|version|/deploy/nginx.conf" Ensure static assets and proxy pass match your configuration, and check the configuration is valid with ``nginx -t``. If everything is fine, you can restart your nginx server with ``service nginx restart``. + +About internal locations +~~~~~~~~~~~~~~~~~~~~~~~~ + +Music (and other static) files are never served by the app itself, but by the reverse +proxy. This is needed because a webserver is way more efficient at serving +files than a Python process. + +However, we do want to ensure users have the right to access music files, and +it can't be done at the proxy's level. To tackle this issue, `we use +nginx's internal directive `_. + +When the API receives a request on its music serving endpoint, it will check +that the user making the request can access the file. Then, it will return an empty +response with a ``X-Accel-Redirect`` header. This header will contain the path +to the file to serve to the user, and will be picked by nginx, but never sent +back to the client. + +Using this technique, we can ensure music files are covered by the authentication +and permission policy of your instance, while keeping as much as performance +as possible. diff --git a/front/src/components/About.vue b/front/src/components/About.vue index 09a5ee24c..524191250 100644 --- a/front/src/components/About.vue +++ b/front/src/components/About.vue @@ -3,15 +3,16 @@

- - + +

- Unfortunately, owners of this instance did not yet take the time to complete this page.

+ {{ $t('Unfortunately, owners of this instance did not yet take the time to complete this page.') }} +

diff --git a/front/src/components/Home.vue b/front/src/components/Home.vue index ce1307ff0..03f4513e6 100644 --- a/front/src/components/Home.vue +++ b/front/src/components/Home.vue @@ -3,15 +3,15 @@

- Welcome on Funkwhale + {{ $t('Welcome on Funkwhale') }}

-

We think listening music should be simple.

+

{{ $t('We think listening music should be simple.') }}

- Learn more about this instance + {{ $t('Learn more about this instance') }} - Get me to the library + {{ $t('Get me to the library') }}
@@ -22,9 +22,9 @@

- Why funkwhale? + {{ $t('Why funkwhale?') }}

-

That's simple: we loved Grooveshark and we want to build something even better.

+

{{ $t('That\'s simple: we loved Grooveshark and we want to build something even better.') }}

@@ -35,26 +35,26 @@

- Unlimited music + {{ $t('Unlimited music') }}

-

Funkwhale is designed to make it easy to listen to music you like, or to discover new artists.

+

{{ $t('Funkwhale is designed to make it easy to listen to music you like, or to discover new artists.') }}

- Click once, listen for hours using built-in radios + {{ $t('Click once, listen for hours using built-in radios') }}
- Keep a track of your favorite songs + {{ $t('Keep a track of your favorite songs') }}
- Playlists? We got them + {{ $t('Playlists? We got them') }}
@@ -62,26 +62,28 @@

- Clean library + {{ $t('Clean library') }}

-

Funkwhale takes care of handling your music.

+

{{ $t('Funkwhale takes care of handling your music') }}.

- Import music from various platforms, such as YouTube or SoundCloud + {{ $t('Import music from various platforms, such as YouTube or SoundCloud') }}
- Get quality metadata about your music thanks to MusicBrainz + + {{ $t('MusicBrainz') }} +
- Covers, lyrics, our goal is to have them all ;) + {{ $t('Covers, lyrics, our goal is to have them all ;)') }}
@@ -89,20 +91,20 @@

- Easy to use + {{ $t('Easy to use') }}

-

Funkwhale is dead simple to use.

+

{{ $t('Funkwhale is dead simple to use.') }}

- No add-ons, no plugins : you only need a web library + {{ $t('No add-ons, no plugins : you only need a web library') }}
- Access your music from a clean interface that focus on what really matters + {{ $t('Access your music from a clean interface that focus on what really matters') }}
@@ -110,26 +112,26 @@

- Your music, your way + {{ $t('Your music, your way') }}

-

Funkwhale is free and gives you control on your music.

+

{{ $t('Funkwhale is free and gives you control on your music.') }}

- The plaform is free and open-source, you can install it and modify it without worries + {{ $t('The plaform is free and open-source, you can install it and modify it without worries') }}
- We do not track you or bother you with ads + {{ $t('We do not track you or bother you with ads') }}
- You can invite friends and family to your instance so they can enjoy your music + {{ $t('You can invite friends and family to your instance so they can enjoy your music') }}
diff --git a/front/src/components/PageNotFound.vue b/front/src/components/PageNotFound.vue index 25e6f86fd..b4d2250ca 100644 --- a/front/src/components/PageNotFound.vue +++ b/front/src/components/PageNotFound.vue @@ -5,13 +5,13 @@

- Whale Page not found! + {{ $t('Whale') }} {{ $t('Page not found!') }}

-

We're sorry, the page you asked for does not exists.

-

Requested URL: {{ path }}

+

{{ $t('We\'re sorry, the page you asked for does not exists.') }}

+ {{ path }} - Go to home page + {{ $t('Go to home page') }}
diff --git a/front/src/components/Sidebar.vue b/front/src/components/Sidebar.vue index 96047ab98..fb4074d80 100644 --- a/front/src/components/Sidebar.vue +++ b/front/src/components/Sidebar.vue @@ -18,12 +18,12 @@ @@ -31,37 +31,35 @@
- -
- Do you want to restore your previous queue? + {{ $t('Do you want to restore your previous queue?') }}
-

{{ queue.previousQueue.tracks.length }} tracks

+

{{ $t('{%count%} tracks', { count: queue.previousQueue.tracks.length }) }}

-
Yes
-
No
+
{{ $t('Yes') }}
+
{{ $t('No') }}
@@ -90,17 +88,17 @@
-
- You have a radio playing + {{ $t('You have a radio playing') }}
-

New tracks will be appended here automatically.

-
Stop radio
+

{{ $t('New tracks will be appended here automatically.') }}

+
{{ $t('Stop radio') }}
+
@@ -143,8 +141,9 @@ export default { ...mapActions({ cleanTrack: 'queue/cleanTrack' }), - reorder: function (oldValue, newValue) { - this.$store.commit('queue/reorder', {oldValue, newValue}) + reorder: function (event) { + this.$store.commit('queue/reorder', { + oldIndex: event.oldIndex, newIndex: event.newIndex}) }, scrollToCurrent () { let current = $(this.$el).find('[data-tab="queue"] .active')[0] @@ -159,7 +158,6 @@ export default { // for half the height of the containers display area var scrollBack = (container.scrollHeight - container.scrollTop <= container.clientHeight) ? 0 : container.clientHeight / 2 container.scrollTop = container.scrollTop - scrollBack - console.log(container.scrollHeight - container.scrollTop, container.clientHeight) } }, watch: { @@ -239,9 +237,6 @@ $sidebar-color: #3D3E3F; flex-direction: column; overflow-y: auto; justify-content: space-between; - @include media(">tablet") { - height: 0px; - } @include media("
- - - + + +
{{ event.object.name }} diff --git a/front/src/components/activity/Listen.vue b/front/src/components/activity/Listen.vue index d207c280d..bfa3ca164 100644 --- a/front/src/components/activity/Listen.vue +++ b/front/src/components/activity/Listen.vue @@ -5,16 +5,16 @@
- - - + + + +
{{ event.object.name }} - {{ event.object.album }} - {{ event.object.artist }} + {{ event.object.album }}{{ event.object.artist }} {{ event.object.artist }} diff --git a/front/src/components/audio/Player.vue b/front/src/components/audio/Player.vue index ad90a5995..c475ec684 100644 --- a/front/src/components/audio/Player.vue +++ b/front/src/components/audio/Player.vue @@ -4,7 +4,7 @@ { + self.$emit('next') + }) + }, + previous () { + let self = this + this.$store.dispatch('queue/previous').then(() => { + self.$emit('previous') + }) + }, touchProgress (e) { let time let target = this.$refs.progress diff --git a/front/src/components/audio/Track.vue b/front/src/components/audio/Track.vue index 68dd34459..08a055f5c 100644 --- a/front/src/components/audio/Track.vue +++ b/front/src/components/audio/Track.vue @@ -73,7 +73,10 @@ export default { }, methods: { errored: function () { - this.$store.dispatch('player/trackErrored') + let self = this + setTimeout( + () => { self.$store.dispatch('player/trackErrored') } + , 1000) }, sourceErrored: function () { this.sourceErrors += 1 @@ -83,9 +86,15 @@ export default { } }, updateDuration: function (e) { + if (!this.$refs.audio) { + return + } this.$store.commit('player/duration', this.$refs.audio.duration) }, loaded: function () { + if (!this.$refs.audio) { + return + } this.$refs.audio.volume = this.volume this.$store.commit('player/resetErrorCount') if (this.isCurrent) { diff --git a/front/src/components/library/Artist.vue b/front/src/components/library/Artist.vue index 5c17ac6af..e16cb6587 100644 --- a/front/src/components/library/Artist.vue +++ b/front/src/components/library/Artist.vue @@ -11,10 +11,7 @@
{{ artist.name }}
- - {{ totalTracks }} - {{ albums.length }} - + {{ $t('{% track_count %} tracks in {% album_count %} albums', {track_count: totalTracks, album_count: albums.length})}}
diff --git a/front/src/components/library/import/BatchDetail.vue b/front/src/components/library/import/BatchDetail.vue index c7894fcc0..b73c8cf82 100644 --- a/front/src/components/library/import/BatchDetail.vue +++ b/front/src/components/library/import/BatchDetail.vue @@ -4,31 +4,80 @@
-
-
-
+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
+ {{ $t('Import batch') }} + + #{{ batch.id }} +
+ {{ $t('Launch date') }} + + +
+ {{ $t('Submitted by') }} + + +
{{ $t('Pending') }}{{ stats.pending }}
{{ $t('Skipped') }}{{ stats.skipped }}
{{ $t('Errored') }}{{ stats.errored }}
{{ $t('Finished') }}{{ stats.finished }}/{{ stats.count}}
+
+
+
+ + +
+
+ + +
-
Importing {{ batch.jobs.length }} tracks...
-
Imported {{ batch.jobs.length }} tracks!
- +
- - - - - + + + + + - + + + + + + + + +
{{ $t('Job ID') }}{{ $t('Recording MusicBrainz ID') }}{{ $t('Source') }}{{ $t('Status') }}{{ $t('Track') }}
{{ job.id }} {{ job.mbid }} @@ -45,29 +94,64 @@
+ + + {{ $t('Showing results {%start%}-{%end%} on {%total%}', {start: ((jobFilters.page-1) * jobFilters.paginateBy) + 1 , end: ((jobFilters.page-1) * jobFilters.paginateBy) + jobResult.results.length, total: jobResult.count})}} + +
-