From f31874edf59016f13bf8bace6515d7f152598bd9 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Mon, 16 Apr 2018 21:59:13 +0200 Subject: [PATCH] Implemented followers notification on import and autoimport --- api/funkwhale_api/federation/actors.py | 29 ++++++- api/funkwhale_api/federation/models.py | 5 ++ api/funkwhale_api/federation/serializers.py | 10 ++- api/funkwhale_api/music/factories.py | 7 ++ api/funkwhale_api/music/models.py | 9 ++ api/funkwhale_api/music/tasks.py | 46 +++++++++++ api/tests/federation/test_actors.py | 61 ++++++++++++++ api/tests/music/test_import.py | 87 ++++++++++++++++++++ front/build/dev-server.js | 2 +- front/src/views/federation/LibraryDetail.vue | 2 +- 10 files changed, 253 insertions(+), 5 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 27a418c7d..380bb23c0 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -12,6 +12,9 @@ from rest_framework.exceptions import PermissionDenied from dynamic_preferences.registries import global_preferences_registry from funkwhale_api.common import session +from funkwhale_api.common import utils as funkwhale_utils +from funkwhale_api.music import models as music_models +from funkwhale_api.music import tasks as music_tasks from . import activity from . import keys @@ -243,7 +246,7 @@ class LibraryActor(SystemActor): data=i, context={'library': remote_library}) for i in items ] - + now = timezone.now() valid_serializers = [] for s in item_serializers: if s.is_valid(): @@ -252,8 +255,30 @@ class LibraryActor(SystemActor): logger.debug( 'Skipping invalid item %s, %s', s.initial_data, s.errors) + lts = [] for s in valid_serializers: - s.save() + lts.append(s.save()) + + if remote_library.autoimport: + batch = music_models.ImportBatch.objects.create( + source='federation', + ) + for lt in lts: + if lt.creation_date < now: + # track was already in the library, we do not trigger + # an import + continue + 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, + ) class TestActor(SystemActor): diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 066c5847b..d91a00c8b 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -97,6 +97,11 @@ class Actor(models.Model): if self.is_system: return actors.SYSTEM_ACTORS[self.preferred_username] + def get_approved_followers(self): + follows = self.received_follows.filter(approved=True) + return self.followers.filter( + pk__in=follows.values_list('actor', flat=True)) + class Follow(models.Model): ap_type = 'Follow' diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 4964106d8..b56dd3f44 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -493,7 +493,7 @@ class ActorWebfingerSerializer(serializers.Serializer): class ActivitySerializer(serializers.Serializer): actor = serializers.URLField() - id = serializers.URLField() + id = serializers.URLField(required=False) type = serializers.ChoiceField( choices=[(c, c) for c in activity.ACTIVITY_TYPES]) object = serializers.JSONField() @@ -525,6 +525,14 @@ class ActivitySerializer(serializers.Serializer): ) return value + def to_representation(self, conf): + d = {} + d.update(conf) + + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d + class ObjectSerializer(serializers.Serializer): id = serializers.URLField() diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 2bf1960ca..ea7ff64df 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -81,6 +81,9 @@ class ImportBatchFactory(factory.django.DjangoModelFactory): submitted_by=None, source='federation', ) + finished = factory.Trait( + status='finished', + ) @registry.register @@ -98,6 +101,10 @@ class ImportJobFactory(factory.django.DjangoModelFactory): library_track=factory.SubFactory(LibraryTrackFactory), batch=factory.SubFactory(ImportBatchFactory, federation=True), ) + finished = factory.Trait( + status='finished', + track_file=factory.SubFactory(TrackFileFactory), + ) @registry.register(name='music.FileImportJob') diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index beec551a5..4ec3ff427 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -505,8 +505,17 @@ class ImportBatch(models.Model): return str(self.pk) def update_status(self): + old_status = self.status self.status = utils.compute_status(self.jobs.all()) self.save(update_fields=['status']) + if self.status != old_status and self.status == 'finished': + from . import tasks + tasks.import_batch_notify_followers.delay(import_batch_id=self.pk) + + def get_federation_url(self): + return federation_utils.full_url( + '/federation/music/import/batch/{}'.format(self.uuid) + ) class ImportJob(models.Model): diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 012b72cd2..bc5ab94f0 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -2,6 +2,10 @@ from django.core.files.base import ContentFile from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.federation import activity +from funkwhale_api.federation import actors +from funkwhale_api.federation import models as federation_models +from funkwhale_api.federation import serializers as federation_serializers 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 @@ -128,6 +132,7 @@ def _do_import(import_job, replace, use_acoustid=True): # it's imported on the track, we don't need it anymore import_job.audio_file.delete() import_job.save() + return track.pk @@ -162,3 +167,44 @@ def fetch_content(lyrics): cleaned_content = lyrics_utils.clean_content(content) lyrics.content = cleaned_content lyrics.save(update_fields=['content']) + + +@celery.app.task(name='music.import_batch_notify_followers') +@celery.require_instance( + models.ImportBatch.objects.filter(status='finished'), 'import_batch') +def import_batch_notify_followers(import_batch): + if not settings.FEDERATION_ENABLED: + return + + if import_batch.source == 'federation': + return + + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + followers = library_actor.get_approved_followers() + jobs = import_batch.jobs.filter( + status='finished', + library_track__isnull=True, + track_file__isnull=False, + ).select_related( + 'track_file__track__artist', + 'track_file__track__album__artist', + ) + track_files = [job.track_file for job in jobs] + collection = federation_serializers.CollectionSerializer({ + 'actor': library_actor, + 'id': import_batch.get_federation_url(), + 'items': track_files, + 'item_serializer': federation_serializers.AudioSerializer + }).data + for f in followers: + create = federation_serializers.ActivitySerializer( + { + 'type': 'Create', + 'id': collection['id'], + 'object': collection, + 'actor': library_actor.url, + 'to': [f.url], + } + ).data + + activity.deliver(create, on_behalf_of=library_actor, to=[f.url]) diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index fe70cc6e5..7281147a1 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -12,6 +12,8 @@ from funkwhale_api.federation import actors from funkwhale_api.federation import models from funkwhale_api.federation import serializers from funkwhale_api.federation import utils +from funkwhale_api.music import models as music_models +from funkwhale_api.music import tasks as music_tasks def test_actor_fetching(r_mock): @@ -465,3 +467,62 @@ def test_library_actor_handle_create_audio(mocker, factories): assert lt.artist_name == a['metadata']['artist']['name'] assert lt.album_title == a['metadata']['release']['title'] assert lt.published_date == arrow.get(a['published']) + + +def test_library_actor_handle_create_audio_autoimport(mocker, factories): + mocked_import = mocker.patch( + 'funkwhale_api.common.utils.on_commit') + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + remote_library = factories['federation.Library']( + federation_enabled=True, + autoimport=True, + ) + + data = { + 'actor': remote_library.actor.url, + 'type': 'Create', + 'id': 'http://test.federation/audio/create', + 'object': { + 'id': 'https://batch.import', + 'type': 'Collection', + 'totalItems': 2, + 'items': factories['federation.Audio'].create_batch(size=2) + }, + } + + library_actor.system_conf.post_inbox(data, actor=remote_library.actor) + + lts = list(remote_library.tracks.order_by('id')) + + assert len(lts) == 2 + + for i, a in enumerate(data['object']['items']): + lt = lts[i] + assert lt.pk is not None + assert lt.url == a['id'] + assert lt.library == remote_library + assert lt.audio_url == a['url']['href'] + assert lt.audio_mimetype == a['url']['mediaType'] + assert lt.metadata == a['metadata'] + assert lt.title == a['metadata']['recording']['title'] + assert lt.artist_name == a['metadata']['artist']['name'] + assert lt.album_title == a['metadata']['release']['title'] + assert lt.published_date == arrow.get(a['published']) + + batch = music_models.ImportBatch.objects.latest('id') + + assert batch.jobs.count() == len(lts) + assert batch.source == 'federation' + assert batch.submitted_by is None + + for i, job in enumerate(batch.jobs.order_by('id')): + lt = lts[i] + assert job.library_track == lt + assert job.mbid == lt.mbid + assert job.source == lt.url + + mocked_import.assert_any_call( + music_tasks.import_job_run.delay, + import_job_id=job.pk, + use_acoustid=False, + ) diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index a15f027ba..2f22ed69a 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -3,6 +3,8 @@ import pytest from django.urls import reverse +from funkwhale_api.federation import actors +from funkwhale_api.federation import serializers as federation_serializers from funkwhale_api.music import tasks @@ -144,3 +146,88 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): artist_from_api.assert_called_once_with( mbid=lt.metadata['artist']['musicbrainz_id']) + + +def test_import_job_run_triggers_notifies_followers( + factories, mocker, tmpfile): + mocker.patch( + 'funkwhale_api.downloader.download', + return_value={'audio_file_path': tmpfile.name}) + mocked_notify = mocker.patch( + 'funkwhale_api.music.tasks.import_batch_notify_followers.delay') + batch = factories['music.ImportBatch']() + job = factories['music.ImportJob']( + finished=True, batch=batch) + track = factories['music.Track'](mbid=job.mbid) + + batch.update_status() + batch.refresh_from_db() + + assert batch.status == 'finished' + + mocked_notify.assert_called_once_with(import_batch_id=batch.pk) + + +def test_import_batch_notifies_followers_skip_on_disabled_federation( + settings, factories, mocker): + mocked_deliver = mocker.patch('funkwhale_api.federation.activity.deliver') + batch = factories['music.ImportBatch'](finished=True) + settings.FEDERATION_ENABLED = False + tasks.import_batch_notify_followers(import_batch_id=batch.pk) + + mocked_deliver.assert_not_called() + + +def test_import_batch_notifies_followers_skip_on_federation_import( + factories, mocker): + mocked_deliver = mocker.patch('funkwhale_api.federation.activity.deliver') + batch = factories['music.ImportBatch'](finished=True, federation=True) + tasks.import_batch_notify_followers(import_batch_id=batch.pk) + + mocked_deliver.assert_not_called() + + +def test_import_batch_notifies_followers( + factories, mocker): + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + + f1 = factories['federation.Follow'](approved=True, target=library_actor) + f2 = factories['federation.Follow'](approved=False, target=library_actor) + f3 = factories['federation.Follow']() + + mocked_deliver = mocker.patch('funkwhale_api.federation.activity.deliver') + batch = factories['music.ImportBatch']() + job1 = factories['music.ImportJob']( + finished=True, batch=batch) + job2 = factories['music.ImportJob']( + finished=True, federation=True, batch=batch) + job3 = factories['music.ImportJob']( + status='pending', batch=batch) + + batch.status = 'finished' + batch.save() + tasks.import_batch_notify_followers(import_batch_id=batch.pk) + + # only f1 match the requirements to be notified + # and only job1 is a non federated track with finished import + expected = { + '@context': federation_serializers.AP_CONTEXT, + 'actor': library_actor.url, + 'type': 'Create', + 'id': batch.get_federation_url(), + 'to': [f1.actor.url], + 'object': federation_serializers.CollectionSerializer( + { + 'id': batch.get_federation_url(), + 'items': [job1.track_file], + 'actor': library_actor, + 'item_serializer': federation_serializers.AudioSerializer + } + ).data + } + + mocked_deliver.assert_called_once_with( + expected, + on_behalf_of=library_actor, + to=[f1.actor.url] + ) diff --git a/front/build/dev-server.js b/front/build/dev-server.js index 14d21919c..f9c389e72 100644 --- a/front/build/dev-server.js +++ b/front/build/dev-server.js @@ -14,7 +14,7 @@ var webpackConfig = process.env.NODE_ENV === 'testing' ? require('./webpack.prod.conf') : require('./webpack.dev.conf') -// require('./i18n') +require('./i18n') // default port where dev server listens for incoming traffic var port = process.env.PORT || config.dev.port diff --git a/front/src/views/federation/LibraryDetail.vue b/front/src/views/federation/LibraryDetail.vue index c64ca2cf2..086ffd143 100644 --- a/front/src/views/federation/LibraryDetail.vue +++ b/front/src/views/federation/LibraryDetail.vue @@ -46,7 +46,6 @@ -