From ba4b6f6ba609ad9db5043d623e2198e6e9d0161b Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 23 May 2018 21:50:23 +0200 Subject: [PATCH] See #228: now use our new action logic for library track import --- api/funkwhale_api/common/serializers.py | 24 +++++++------ api/funkwhale_api/federation/serializers.py | 31 ++++++++++++++++- api/funkwhale_api/federation/views.py | 19 +++++++++-- api/funkwhale_api/music/serializers.py | 22 ------------ api/funkwhale_api/music/views.py | 16 --------- api/tests/common/test_serializers.py | 11 ++++++ api/tests/federation/test_views.py | 34 +++++++++++++++++++ api/tests/music/test_views.py | 18 ---------- .../federation/LibraryTrackTable.vue | 7 ++-- 9 files changed, 109 insertions(+), 73 deletions(-) diff --git a/api/funkwhale_api/common/serializers.py b/api/funkwhale_api/common/serializers.py index 7e214d7db..62d9c567e 100644 --- a/api/funkwhale_api/common/serializers.py +++ b/api/funkwhale_api/common/serializers.py @@ -49,17 +49,19 @@ class ActionSerializer(serializers.Serializer): 'list of identifiers or the string "all".'.format(value)) def validate(self, data): - if not self.filterset_class or 'filters' not in data: - # no additional filters to apply, we just skip - return data + if self.filterset_class and 'filters' in data: + qs_filterset = self.filterset_class( + data['filters'], queryset=data['objects']) + try: + assert qs_filterset.form.is_valid() + except (AssertionError, TypeError): + raise serializers.ValidationError('Invalid filters') + data['objects'] = qs_filterset.qs - qs_filterset = self.filterset_class( - data['filters'], queryset=data['objects']) - try: - assert qs_filterset.form.is_valid() - except (AssertionError, TypeError): - raise serializers.ValidationError('Invalid filters') - data['objects'] = qs_filterset.qs + data['count'] = data['objects'].count() + if data['count'] < 1: + raise serializers.ValidationError( + 'No object matching your request') return data def save(self): @@ -67,7 +69,7 @@ class ActionSerializer(serializers.Serializer): handler = getattr(self, handler_name) result = handler(self.validated_data['objects']) payload = { - 'updated': self.validated_data['objects'].count(), + 'updated': self.validated_data['count'], 'action': self.validated_data['action'], 'result': result, } diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 51561e222..77cb25096 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -10,8 +10,11 @@ from rest_framework import serializers from dynamic_preferences.registries import global_preferences_registry from funkwhale_api.common import utils as funkwhale_utils - +from funkwhale_api.common import serializers as common_serializers +from funkwhale_api.music import models as music_models +from funkwhale_api.music import tasks as music_tasks from . import activity +from . import filters from . import models from . import utils @@ -806,3 +809,29 @@ class CollectionSerializer(serializers.Serializer): if self.context.get('include_ap_context', True): d['@context'] = AP_CONTEXT return d + + +class LibraryTrackActionSerializer(common_serializers.ActionSerializer): + actions = ['import'] + filterset_class = filters.LibraryTrackFilter + + @transaction.atomic + def handle_import(self, objects): + batch = music_models.ImportBatch.objects.create( + source='federation', + submitted_by=self.context['submitted_by'] + ) + for lt in objects: + job = music_models.ImportJob.objects.create( + batch=batch, + library_track=lt, + mbid=lt.mbid, + source=lt.url, + ) + funkwhale_utils.on_commit( + music_tasks.import_job_run.delay, + import_job_id=job.pk, + use_acoustid=False, + ) + + return {'batch': {'id': batch.pk}} diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 06a2cd040..01bc02fdf 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -15,7 +15,7 @@ from rest_framework.serializers import ValidationError from funkwhale_api.common import preferences from funkwhale_api.common import utils as funkwhale_utils -from funkwhale_api.music.models import TrackFile +from funkwhale_api.music import models as music_models from funkwhale_api.users.permissions import HasUserPermission from . import activity @@ -148,7 +148,9 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): def list(self, request, *args, **kwargs): page = request.GET.get('page') library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - qs = TrackFile.objects.order_by('-creation_date').select_related( + qs = music_models.TrackFile.objects.order_by( + '-creation_date' + ).select_related( 'track__artist', 'track__album__artist' ).filter(library_track__isnull=True) @@ -307,3 +309,16 @@ class LibraryTrackViewSet( 'fetched_date', 'published_date', ) + + @list_route(methods=['post']) + def action(self, request, *args, **kwargs): + queryset = models.LibraryTrack.objects.filter( + local_track_file__isnull=True) + serializer = serializers.LibraryTrackActionSerializer( + request.data, + queryset=queryset, + context={'submitted_by': request.user} + ) + serializer.is_valid(raise_exception=True) + result = serializer.save() + return response.Response(result, status=200) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index c77983a40..b72bb8c4a 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -250,28 +250,6 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer): return 'Audio' -class SubmitFederationTracksSerializer(serializers.Serializer): - library_tracks = serializers.PrimaryKeyRelatedField( - many=True, - queryset=LibraryTrack.objects.filter(local_track_file__isnull=True), - ) - - @transaction.atomic - def save(self, **kwargs): - batch = models.ImportBatch.objects.create( - source='federation', - **kwargs - ) - for lt in self.validated_data['library_tracks']: - models.ImportJob.objects.create( - batch=batch, - library_track=lt, - mbid=lt.mbid, - source=lt.url, - ) - return batch - - class ImportJobRunSerializer(serializers.Serializer): jobs = serializers.PrimaryKeyRelatedField( many=True, diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 24a9cbbcd..b42ad1c35 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -449,22 +449,6 @@ class SubmitViewSet(viewsets.ViewSet): data, request, batch=None, import_request=import_request) return Response(import_data) - @list_route(methods=['post']) - @transaction.non_atomic_requests - def federation(self, request, *args, **kwargs): - serializer = serializers.SubmitFederationTracksSerializer( - data=request.data) - serializer.is_valid(raise_exception=True) - batch = serializer.save(submitted_by=request.user) - for job in batch.jobs.all(): - funkwhale_utils.on_commit( - tasks.import_job_run.delay, - import_job_id=job.pk, - use_acoustid=False, - ) - - return Response({'id': batch.id}, status=201) - @transaction.atomic def _import_album(self, data, request, batch=None, import_request=None): # we import the whole album here to prevent race conditions that occurs diff --git a/api/tests/common/test_serializers.py b/api/tests/common/test_serializers.py index 075e957f6..563676556 100644 --- a/api/tests/common/test_serializers.py +++ b/api/tests/common/test_serializers.py @@ -87,3 +87,14 @@ def test_action_serializers_filterset(factories): assert serializer.is_valid() is True assert list(serializer.validated_data['objects']) == [user2] + + +def test_action_serializers_validates_at_least_one_object(): + data = { + 'objects': 'all', + 'action': 'test', + } + serializer = TestSerializer(data, queryset=models.User.objects.none()) + + assert serializer.is_valid() is False + assert 'non_field_errors' in serializer.errors diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index 10237ed9f..1b0de5754 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -418,3 +418,37 @@ def test_can_filter_pending_follows(factories, superuser_api_client): assert response.status_code == 200 assert len(response.data['results']) == 0 + + +def test_library_track_action_import( + factories, superuser_api_client, mocker): + lt1 = factories['federation.LibraryTrack']() + lt2 = factories['federation.LibraryTrack'](library=lt1.library) + lt3 = factories['federation.LibraryTrack']() + lt4 = factories['federation.LibraryTrack'](library=lt3.library) + mocker.patch('funkwhale_api.music.tasks.import_job_run') + + payload = { + 'objects': 'all', + 'action': 'import', + 'filters': { + 'library': lt1.library.uuid + } + } + url = reverse('api:v1:federation:library-tracks-action') + response = superuser_api_client.post(url, payload, format='json') + batch = superuser_api_client.user.imports.latest('id') + expected = { + 'updated': 2, + 'action': 'import', + 'result': { + 'batch': {'id': batch.pk} + } + } + + imported_lts = [lt1, lt2] + assert response.status_code == 200 + assert response.data == expected + assert batch.jobs.count() == 2 + for i, job in enumerate(batch.jobs.all()): + assert job.library_track == imported_lts[i] diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 38366442f..9328ba329 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -249,24 +249,6 @@ def test_serve_updates_access_date(factories, settings, api_client): assert track_file.accessed_date > now -def test_can_create_import_from_federation_tracks( - factories, superuser_api_client, mocker): - lts = factories['federation.LibraryTrack'].create_batch(size=5) - mocker.patch('funkwhale_api.music.tasks.import_job_run') - - payload = { - 'library_tracks': [l.pk for l in lts] - } - url = reverse('api:v1:submit-federation') - response = superuser_api_client.post(url, payload) - - assert response.status_code == 201 - batch = superuser_api_client.user.imports.latest('id') - 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') diff --git a/front/src/components/federation/LibraryTrackTable.vue b/front/src/components/federation/LibraryTrackTable.vue index d8ee48bf2..ad8b801a6 100644 --- a/front/src/components/federation/LibraryTrackTable.vue +++ b/front/src/components/federation/LibraryTrackTable.vue @@ -157,10 +157,11 @@ export default { let self = this self.isImporting = true let payload = { - library_tracks: this.checked + objects: this.checked, + action: 'import' } - axios.post('/submit/federation/', payload).then((response) => { - self.importBatch = response.data + axios.post('/federation/library-tracks/action/', payload).then((response) => { + self.importBatch = response.data.result.batch self.isImporting = false self.fetchData() }, error => {