From 48df30dbd8ba8701aa8f3137303914d396b9aaca Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Mon, 2 Apr 2018 22:12:28 +0200 Subject: [PATCH 01/30] We now persist system accounts to database --- api/funkwhale_api/federation/actors.py | 18 ++++++++++-------- api/tests/federation/test_actors.py | 9 +++++++++ 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 69033f5ca..a4e24912b 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -49,15 +49,17 @@ class SystemActor(object): additional_attributes = {} def get_actor_instance(self): - a = models.Actor( - **self.get_instance_argument( - self.id, - name=self.name, - summary=self.summary, - **self.additional_attributes - ) + args = self.get_instance_argument( + self.id, + name=self.name, + summary=self.summary, + **self.additional_attributes + ) + url = args.pop('url') + a, created = models.Actor.objects.get_or_create( + url=url, + defaults=args, ) - a.pk = self.id return a def get_instance_argument(self, id, name, summary, **kwargs): diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index b3b0f8df0..127b3c15e 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -6,6 +6,7 @@ from django.utils import timezone from rest_framework import exceptions from funkwhale_api.federation import actors +from funkwhale_api.federation import models from funkwhale_api.federation import serializers from funkwhale_api.federation import utils @@ -188,3 +189,11 @@ def test_test_post_outbox_handles_create_note( to=[actor.url], on_behalf_of=actors.SYSTEM_ACTORS['test'].get_actor_instance() ) + + +def test_getting_actor_instance_persists_in_db(db): + test = actors.SYSTEM_ACTORS['test'].get_actor_instance() + from_db = models.Actor.objects.get(url=test.url) + + for f in test._meta.fields: + assert getattr(from_db, f.name) == getattr(test, f.name) From a81c92dbf5041729abcb2963c14938c30a474e38 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Mon, 2 Apr 2018 22:43:59 +0200 Subject: [PATCH 02/30] Additional setting to control wether music library federation needs approval --- api/config/settings/common.py | 4 +++- api/funkwhale_api/federation/actors.py | 5 +++++ api/funkwhale_api/federation/models.py | 8 ++++++++ api/tests/federation/test_actors.py | 25 +++++++++++++++++++++++++ deploy/env.prod.sample | 9 +++++++++ 5 files changed, 50 insertions(+), 1 deletion(-) diff --git a/api/config/settings/common.py b/api/config/settings/common.py index fbe3b7045..6a85a934c 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -30,7 +30,9 @@ FUNKWHALE_HOSTNAME = urlsplit(FUNKWHALE_URL).netloc FEDERATION_ENABLED = env.bool('FEDERATION_ENABLED', default=True) FEDERATION_HOSTNAME = env('FEDERATION_HOSTNAME', default=FUNKWHALE_HOSTNAME) - +FEDERATION_MUSIC_NEEDS_APPROVAL = env.bool( + 'FEDERATION_MUSIC_NEEDS_APPROVAL', default=True +) ALLOWED_HOSTS = env.list('DJANGO_ALLOWED_HOSTS') # APP CONFIGURATION diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index a4e24912b..e29125f5b 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -47,6 +47,7 @@ def get_actor(actor_url): class SystemActor(object): additional_attributes = {} + manually_approves_followers = False def get_actor_instance(self): args = self.get_instance_argument( @@ -113,6 +114,9 @@ class LibraryActor(SystemActor): additional_attributes = { 'manually_approves_followers': True } + @property + def manually_approves_followers(self): + return settings.FEDERATION_MUSIC_NEEDS_APPROVAL class TestActor(SystemActor): @@ -125,6 +129,7 @@ class TestActor(SystemActor): additional_attributes = { 'manually_approves_followers': False } + manually_approves_followers = False def get_outbox(self, data, actor=None): return { diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index d76ad173b..a2cd598f7 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -57,3 +57,11 @@ class Actor(models.Model): setattr(self, field, v.lower()) super().save(**kwargs) + + @property + def is_system(self): + from . import actors + return all([ + settings.FEDERATION_HOSTNAME == self.domain, + self.preferred_username in actors.SYSTEM_ACTORS + ]) diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 127b3c15e..8afb94ad0 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -197,3 +197,28 @@ def test_getting_actor_instance_persists_in_db(db): for f in test._meta.fields: assert getattr(from_db, f.name) == getattr(test, f.name) + + +@pytest.mark.parametrize('username,domain,expected', [ + ('test', 'wrongdomain.com', False), + ('notsystem', '', False), + ('test', '', True), +]) +def test_actor_is_system( + username, domain, expected, nodb_factories, settings): + if not domain: + domain = settings.FEDERATION_HOSTNAME + + actor = nodb_factories['federation.Actor']( + preferred_username=username, + domain=domain, + ) + assert actor.is_system is expected + + +@pytest.mark.parametrize('value', [False, True]) +def test_library_actor_manually_approves_based_on_setting( + value, settings): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = value + library_conf = actors.SYSTEM_ACTORS['library'] + assert library_conf.manually_approves_followers is value diff --git a/deploy/env.prod.sample b/deploy/env.prod.sample index a016b34c7..9e9938500 100644 --- a/deploy/env.prod.sample +++ b/deploy/env.prod.sample @@ -85,3 +85,12 @@ API_AUTHENTICATION_REQUIRED=True # This will help us detect and correct bugs RAVEN_ENABLED=false RAVEN_DSN=https://44332e9fdd3d42879c7d35bf8562c6a4:0062dc16a22b41679cd5765e5342f716@sentry.eliotberriot.com/5 + +# This settings enable/disable federation on the instance level +FEDERATION_ENABLED=True +# This setting decide wether music library is shared automatically +# to followers or if it requires manual approval before. +# FEDERATION_MUSIC_NEEDS_APPROVAL=False +# means anyone can subscribe to your library and import your file, +# use with caution. +FEDERATION_MUSIC_NEEDS_APPROVAL=True From 168c4e7d531e2f42088e4ba4729e280fc81328e8 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 17:36:03 +0200 Subject: [PATCH 03/30] system_conf property on Actor instances --- api/funkwhale_api/federation/models.py | 6 ++++++ api/tests/federation/test_actors.py | 16 ++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index a2cd598f7..35ddce961 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -65,3 +65,9 @@ class Actor(models.Model): settings.FEDERATION_HOSTNAME == self.domain, self.preferred_username in actors.SYSTEM_ACTORS ]) + + @property + def system_conf(self): + from . import actors + if self.is_system: + return actors.SYSTEM_ACTORS[self.preferred_username] diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 8afb94ad0..e72232fc0 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -216,6 +216,22 @@ def test_actor_is_system( assert actor.is_system is expected +@pytest.mark.parametrize('username,domain,expected', [ + ('test', 'wrongdomain.com', None), + ('notsystem', '', None), + ('test', '', actors.SYSTEM_ACTORS['test']), +]) +def test_actor_is_system( + username, domain, expected, nodb_factories, settings): + if not domain: + domain = settings.FEDERATION_HOSTNAME + actor = nodb_factories['federation.Actor']( + preferred_username=username, + domain=domain, + ) + assert actor.system_conf == expected + + @pytest.mark.parametrize('value', [False, True]) def test_library_actor_manually_approves_based_on_setting( value, settings): From 2f6d3ae18002715f0ccead21ac155adb29d2ae2a Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 18:35:08 +0200 Subject: [PATCH 04/30] Ensure unicity on actor username and domain --- .../migrations/0002_auto_20180403_1620.py | 17 +++++++++++++++++ api/funkwhale_api/federation/models.py | 3 +++ 2 files changed, 20 insertions(+) create mode 100644 api/funkwhale_api/federation/migrations/0002_auto_20180403_1620.py diff --git a/api/funkwhale_api/federation/migrations/0002_auto_20180403_1620.py b/api/funkwhale_api/federation/migrations/0002_auto_20180403_1620.py new file mode 100644 index 000000000..2200424d8 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0002_auto_20180403_1620.py @@ -0,0 +1,17 @@ +# Generated by Django 2.0.3 on 2018-04-03 16:20 + +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0001_initial'), + ] + + operations = [ + migrations.AlterUniqueTogether( + name='actor', + unique_together={('domain', 'preferred_username')}, + ), + ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 35ddce961..414bcc50a 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -32,6 +32,9 @@ class Actor(models.Model): default=timezone.now) manually_approves_followers = models.NullBooleanField(default=None) + class Meta: + unique_together = ['domain', 'preferred_username'] + @property def webfinger_subject(self): return '{}@{}'.format( From 6aa6f1d8f869e3821a40dd7a2b061bfa098eb008 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 19:48:50 +0200 Subject: [PATCH 05/30] Test actor can now follow back --- api/funkwhale_api/federation/activity.py | 34 +++++++++ api/funkwhale_api/federation/actors.py | 81 ++++++++++++++++----- api/funkwhale_api/federation/factories.py | 17 +++++ api/funkwhale_api/federation/serializers.py | 4 +- api/tests/federation/test_actors.py | 77 +++++++++++++++++++- 5 files changed, 192 insertions(+), 21 deletions(-) diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 4eeb193b1..3b7648f10 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -83,3 +83,37 @@ def deliver(activity, on_behalf_of, to=[]): ) response.raise_for_status() logger.debug('Remote answered with %s', response.status_code) + + +def get_follow(follow_id, follower, followed): + return { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {} + ], + 'actor': follower.url, + 'id': follower.url + '#follows/{}'.format(follow_id), + 'object': followed.url, + 'type': 'Follow' + } + + +def get_accept_follow(accept_id, accept_actor, follow, follow_actor): + return { + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], + "id": accept_actor.url + '#accepts/follows/{}'.format( + accept_id), + "type": "Accept", + "actor": accept_actor.url, + "object": { + "id": follow['id'], + "type": "Follow", + "actor": follow_actor.url, + "object": accept_actor.url + }, + } diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index e29125f5b..031526f8b 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -1,5 +1,6 @@ import logging import requests +import uuid import xml from django.conf import settings @@ -98,7 +99,7 @@ class SystemActor(object): raise NotImplementedError def post_inbox(self, data, actor=None): - raise NotImplementedError + return self.handle(data, actor=actor) def get_outbox(self, data, actor=None): raise NotImplementedError @@ -106,6 +107,31 @@ class SystemActor(object): def post_outbox(self, data, actor=None): raise NotImplementedError + def handle(self, data, actor=None): + """ + Main entrypoint for handling activities posted to the + actor's inbox + """ + logger.info('Received activity on %s inbox', self.id) + + if actor is None: + raise PermissionDenied('Actor not authenticated') + + serializer = serializers.ActivitySerializer( + data=data, context={'actor': actor}) + serializer.is_valid(raise_exception=True) + + ac = serializer.data + try: + handler = getattr( + self, 'handle_{}'.format(ac['type'].lower())) + except (KeyError, AttributeError): + logger.debug( + 'No handler for activity %s', ac['type']) + return + + return handler(ac, actor) + class LibraryActor(SystemActor): id = 'library' @@ -147,23 +173,6 @@ class TestActor(SystemActor): "orderedItems": [] } - def post_inbox(self, data, actor=None): - if actor is None: - raise PermissionDenied('Actor not authenticated') - - serializer = serializers.ActivitySerializer( - data=data, context={'actor': actor}) - serializer.is_valid(raise_exception=True) - - ac = serializer.validated_data - logger.info('Received activity on %s inbox', self.id) - if ac['type'] == 'Create' and ac['object']['type'] == 'Note': - # we received a toot \o/ - command = self.parse_command(ac['object']['content']) - logger.debug('Parsed command: %s', command) - if command == 'ping': - self.handle_ping(ac, actor) - def parse_command(self, message): """ Remove any links or fancy markup to extract /command from @@ -175,7 +184,16 @@ class TestActor(SystemActor): except IndexError: return - def handle_ping(self, ac, sender): + def handle_create(self, ac, sender): + if ac['object']['type'] != 'Note': + return + + # we received a toot \o/ + command = self.parse_command(ac['object']['content']) + logger.debug('Parsed command: %s', command) + if command != 'ping': + return + now = timezone.now() test_actor = self.get_actor_instance() reply_url = 'https://{}/activities/note/{}'.format( @@ -221,6 +239,31 @@ class TestActor(SystemActor): to=[ac['actor']], on_behalf_of=test_actor) + def handle_follow(self, ac, sender): + # on a follow we: + # 1. send the accept answer + # 2. follow back + test_actor = self.get_actor_instance() + accept_uuid = uuid.uuid4() + accept = activity.get_accept_follow( + accept_id=accept_uuid, + accept_actor=test_actor, + follow=ac, + follow_actor=sender) + activity.deliver( + accept, + to=[ac['actor']], + on_behalf_of=test_actor) + follow_uuid = uuid.uuid4() + follow = activity.get_follow( + follow_id=follow_uuid, + follower=test_actor, + followed=sender) + activity.deliver( + follow, + to=[ac['actor']], + on_behalf_of=test_actor) + SYSTEM_ACTORS = { 'library': LibraryActor(), 'test': TestActor(), diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 88c86f791..6e621c7f3 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -89,3 +89,20 @@ class NoteFactory(factory.Factory): class Meta: model = dict + + +@registry.register(name='federation.Activity') +class ActivityFactory(factory.Factory): + type = 'Create' + id = factory.Faker('url') + published = factory.LazyFunction( + lambda: timezone.now().isoformat() + ) + actor = factory.Faker('url') + object = factory.SubFactory( + NoteFactory, + actor=factory.SelfAttribute('..actor'), + published=factory.SelfAttribute('..published')) + + class Meta: + model = dict diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 2137e8d91..7c35aead3 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -120,7 +120,9 @@ class ActivitySerializer(serializers.Serializer): type = value['type'] except KeyError: raise serializers.ValidationError('Missing object type') - + except TypeError: + # probably a URL + return value try: object_serializer = OBJECT_SERIALIZERS[type] except KeyError: diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index e72232fc0..f0d8f7840 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -1,4 +1,5 @@ import pytest +import uuid from django.urls import reverse from django.utils import timezone @@ -127,7 +128,7 @@ def test_test_post_outbox_validates_actor(nodb_factories): assert msg in exc_info.value -def test_test_post_outbox_handles_create_note( +def test_test_post_inbox_handles_create_note( settings, mocker, factories): deliver = mocker.patch( 'funkwhale_api.federation.activity.deliver') @@ -238,3 +239,77 @@ def test_library_actor_manually_approves_based_on_setting( settings.FEDERATION_MUSIC_NEEDS_APPROVAL = value library_conf = actors.SYSTEM_ACTORS['library'] assert library_conf.manually_approves_followers is value + + +def test_system_actor_handle(mocker, nodb_factories): + handler = mocker.patch( + 'funkwhale_api.federation.actors.TestActor.handle_create') + actor = nodb_factories['federation.Actor']() + activity = nodb_factories['federation.Activity']( + type='Create', actor=actor.url) + serializer = serializers.ActivitySerializer( + data=activity + ) + assert serializer.is_valid() + actors.SYSTEM_ACTORS['test'].handle(activity, actor) + handler.assert_called_once_with(serializer.data, actor) + + +def test_test_actor_handles_follow( + settings, mocker, factories): + deliver = mocker.patch( + 'funkwhale_api.federation.activity.deliver') + actor = factories['federation.Actor']() + now = timezone.now() + mocker.patch('django.utils.timezone.now', return_value=now) + test_actor = actors.SYSTEM_ACTORS['test'].get_actor_instance() + data = { + 'actor': actor.url, + 'type': 'Follow', + 'id': 'http://test.federation/user#follows/267', + 'object': test_actor.url, + } + uid = uuid.uuid4() + mocker.patch('uuid.uuid4', return_value=uid) + expected_accept = { + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], + "id": test_actor.url + '#accepts/follows/{}'.format(uid), + "type": "Accept", + "actor": test_actor.url, + "object": { + "id": data['id'], + "type": "Follow", + "actor": actor.url, + "object": test_actor.url + }, + } + expected_follow = { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {} + ], + 'actor': test_actor.url, + 'id': test_actor.url + '#follows/{}'.format(uid), + 'object': actor.url, + 'type': 'Follow' + } + + actors.SYSTEM_ACTORS['test'].post_inbox(data, actor=actor) + expected_calls = [ + mocker.call( + expected_accept, + to=[actor.url], + on_behalf_of=test_actor, + ), + mocker.call( + expected_follow, + to=[actor.url], + on_behalf_of=test_actor, + ) + ] + deliver.assert_has_calls(expected_calls) From f19418d2c27b1167e3efea06fbeffe1a7b12c4ae Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 21:30:15 +0200 Subject: [PATCH 06/30] Added follow model and factory --- api/funkwhale_api/federation/activity.py | 2 +- api/funkwhale_api/federation/actors.py | 7 +++++ api/funkwhale_api/federation/factories.py | 10 +++++- .../migrations/0003_auto_20180403_1921.py | 31 +++++++++++++++++++ api/funkwhale_api/federation/models.py | 22 +++++++++++++ api/tests/federation/test_actors.py | 4 +++ api/tests/federation/test_models.py | 25 +++++++++++++++ 7 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py create mode 100644 api/tests/federation/test_models.py diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 3b7648f10..5a0974011 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -52,7 +52,7 @@ OBJECT_TYPES = [ 'Relationship', 'Tombstone', 'Video', -] +] + ACTIVITY_TYPES def deliver(activity, on_behalf_of, to=[]): from . import actors diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 031526f8b..22b231e08 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -243,6 +243,7 @@ class TestActor(SystemActor): # on a follow we: # 1. send the accept answer # 2. follow back + # test_actor = self.get_actor_instance() accept_uuid = uuid.uuid4() accept = activity.get_accept_follow( @@ -254,6 +255,12 @@ class TestActor(SystemActor): accept, to=[ac['actor']], on_behalf_of=test_actor) + # we persist the sender in database + sender.save() + models.Follow.objects.get_or_create( + actor=sender, + target=test_actor, + ) follow_uuid = uuid.uuid4() follow = activity.get_follow( follow_id=follow_uuid, diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 6e621c7f3..217b47218 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -53,7 +53,6 @@ class SignedRequestFactory(factory.Factory): @registry.register class ActorFactory(factory.DjangoModelFactory): - public_key = None private_key = None preferred_username = factory.Faker('user_name') @@ -77,6 +76,15 @@ class ActorFactory(factory.DjangoModelFactory): return super()._generate(create, attrs) +@registry.register +class FollowFactory(factory.DjangoModelFactory): + target = factory.SubFactory(ActorFactory) + actor = factory.SubFactory(ActorFactory) + + class Meta: + model = models.Follow + + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): type = 'Note' diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py b/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py new file mode 100644 index 000000000..aadf3257e --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py @@ -0,0 +1,31 @@ +# Generated by Django 2.0.3 on 2018-04-03 19:21 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0002_auto_20180403_1620'), + ] + + operations = [ + migrations.CreateModel( + name='Follow', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('last_modification_date', models.DateTimeField(default=django.utils.timezone.now)), + ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emitted_follows', to='federation.Actor')), + ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follows', to='federation.Actor')), + ], + ), + migrations.AlterUniqueTogether( + name='follow', + unique_together={('actor', 'target')}, + ), + ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 414bcc50a..875268bca 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -1,3 +1,5 @@ +import uuid + from django.conf import settings from django.db import models from django.utils import timezone @@ -74,3 +76,23 @@ class Actor(models.Model): from . import actors if self.is_system: return actors.SYSTEM_ACTORS[self.preferred_username] + + +class Follow(models.Model): + uuid = models.UUIDField(default=uuid.uuid4, unique=True) + actor = models.ForeignKey( + Actor, + related_name='emitted_follows', + on_delete=models.CASCADE, + ) + target = models.ForeignKey( + Actor, + related_name='received_follows', + on_delete=models.CASCADE, + ) + creation_date = models.DateTimeField(default=timezone.now) + last_modification_date = models.DateTimeField( + default=timezone.now) + + class Meta: + unique_together = ['actor', 'target'] diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index f0d8f7840..d50b52ee6 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -313,3 +313,7 @@ def test_test_actor_handles_follow( ) ] deliver.assert_has_calls(expected_calls) + + follow = test_actor.received_follows.first() + assert follow.actor == actor + assert follow.target == test_actor diff --git a/api/tests/federation/test_models.py b/api/tests/federation/test_models.py new file mode 100644 index 000000000..297fe2c58 --- /dev/null +++ b/api/tests/federation/test_models.py @@ -0,0 +1,25 @@ +import pytest + +from django import db + +from funkwhale_api.federation import models + + +def test_cannot_duplicate_actor(factories): + actor = factories['federation.Actor']() + + with pytest.raises(db.IntegrityError): + factories['federation.Actor']( + domain=actor.domain, + preferred_username=actor.preferred_username, + ) + + +def test_cannot_duplicate_follow(factories): + follow = factories['federation.Follow']() + + with pytest.raises(db.IntegrityError): + factories['federation.Follow']( + target=follow.target, + actor=follow.actor, + ) From 657bd4b01ab3fc92afc3bc45b0c2c311a247de0f Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 23:24:51 +0200 Subject: [PATCH 07/30] Follow serializer --- api/funkwhale_api/federation/serializers.py | 34 ++++++++++++++++++--- api/tests/federation/test_serializers.py | 19 ++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 7c35aead3..075e253da 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -11,6 +11,12 @@ from . import models from . import utils +AP_CONTEXT = [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {}, +] + class ActorSerializer(serializers.ModelSerializer): # left maps to activitypub fields, right to our internal models id = serializers.URLField(source='url') @@ -43,11 +49,7 @@ class ActorSerializer(serializers.ModelSerializer): def to_representation(self, instance): ret = super().to_representation(instance) - ret['@context'] = [ - 'https://www.w3.org/ns/activitystreams', - 'https://w3id.org/security/v1', - {}, - ] + ret['@context'] = AP_CONTEXT if instance.public_key: ret['publicKey'] = { 'owner': instance.url, @@ -87,6 +89,28 @@ class ActorSerializer(serializers.ModelSerializer): return value[:500] +class FollowSerializer(serializers.ModelSerializer): + # left maps to activitypub fields, right to our internal models + id = serializers.URLField(source='get_federation_url') + object = serializers.URLField(source='target.url') + actor = serializers.URLField(source='actor.url') + type = serializers.CharField(source='ap_type') + + class Meta: + model = models.Actor + fields = [ + 'id', + 'object', + 'actor', + 'type' + ] + + def to_representation(self, instance): + ret = super().to_representation(instance) + ret['@context'] = AP_CONTEXT + return ret + + class ActorWebfingerSerializer(serializers.ModelSerializer): class Meta: model = models.Actor diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index efa92b16a..77c14531c 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -144,3 +144,22 @@ def test_webfinger_serializer(): serializer = serializers.ActorWebfingerSerializer(actor) assert serializer.data == expected + + +def test_follow_serializer_to_ap(factories): + follow = factories['federation.Follow'](local=True) + serializer = serializers.FollowSerializer(follow) + + expected = { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {}, + ], + 'id': follow.get_federation_url(), + 'type': 'Follow', + 'actor': follow.actor.url, + 'object': follow.target.url, + } + + assert serializer.data == expected From 81e7f03f7757f31cba0803cc089d555f56318a90 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 23:25:22 +0200 Subject: [PATCH 08/30] Now persist actors in database during auth --- api/funkwhale_api/federation/actors.py | 2 -- api/funkwhale_api/federation/authentication.py | 6 +++++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 22b231e08..d70ce23e5 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -255,8 +255,6 @@ class TestActor(SystemActor): accept, to=[ac['actor']], on_behalf_of=test_actor) - # we persist the sender in database - sender.save() models.Follow.objects.get_or_create( actor=sender, target=test_actor, diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py index e199ef134..f2926bb30 100644 --- a/api/funkwhale_api/federation/authentication.py +++ b/api/funkwhale_api/federation/authentication.py @@ -7,6 +7,7 @@ from rest_framework import exceptions from . import actors from . import keys +from . import models from . import serializers from . import signing from . import utils @@ -42,7 +43,10 @@ class SignatureAuthentication(authentication.BaseAuthentication): except cryptography.exceptions.InvalidSignature: raise exceptions.AuthenticationFailed('Invalid signature') - return serializer.build() + try: + return models.Actor.objects.get(url=actor_data['id']) + except models.Actor.DoesNotExist: + return serializer.save() def authenticate(self, request): setattr(request, 'actor', None) From 3ad1fe17d5109a04f622c94b6c64dea1e248d521 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Tue, 3 Apr 2018 23:25:44 +0200 Subject: [PATCH 09/30] Test bot can now unfollow --- api/funkwhale_api/federation/actors.py | 36 +++++++++++++++- api/funkwhale_api/federation/factories.py | 12 ++++++ api/funkwhale_api/federation/models.py | 7 +++ api/tests/federation/test_actors.py | 47 ++++++++++++++++----- api/tests/federation/test_authentication.py | 4 +- api/tests/federation/test_models.py | 7 +++ 6 files changed, 100 insertions(+), 13 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index d70ce23e5..89c621edc 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -130,7 +130,7 @@ class SystemActor(object): 'No handler for activity %s', ac['type']) return - return handler(ac, actor) + return handler(data, actor) class LibraryActor(SystemActor): @@ -269,6 +269,40 @@ class TestActor(SystemActor): to=[ac['actor']], on_behalf_of=test_actor) + def handle_undo(self, ac, sender): + if ac['object']['type'] != 'Follow': + return + + if ac['object']['actor'] != sender.url: + # not the same actor, permission issue + return + + test_actor = self.get_actor_instance() + models.Follow.objects.filter( + actor=sender, + target=test_actor, + ).delete() + # we also unfollow the sender, if possible + try: + follow = models.Follow.objects.get( + target=sender, + actor=test_actor, + ) + except models.Follow.DoesNotExist: + return + undo = { + '@context': serializers.AP_CONTEXT, + 'type': 'Undo', + 'id': follow.get_federation_url() + '/undo', + 'actor': test_actor.url, + 'object': serializers.FollowSerializer(follow).data, + } + follow.delete() + activity.deliver( + undo, + to=[sender.url], + on_behalf_of=test_actor) + SYSTEM_ACTORS = { 'library': LibraryActor(), 'test': TestActor(), diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 217b47218..16abf80dc 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -3,6 +3,7 @@ import requests import requests_http_signature from django.utils import timezone +from django.conf import settings from funkwhale_api.factories import registry @@ -65,6 +66,12 @@ class ActorFactory(factory.DjangoModelFactory): class Meta: model = models.Actor + class Params: + local = factory.Trait( + domain=factory.LazyAttribute( + lambda o: settings.FEDERATION_HOSTNAME) + ) + @classmethod def _generate(cls, create, attrs): has_public = attrs.get('public_key') is not None @@ -84,6 +91,11 @@ class FollowFactory(factory.DjangoModelFactory): class Meta: model = models.Follow + class Params: + local = factory.Trait( + actor=factory.SubFactory(ActorFactory, local=True) + ) + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 875268bca..a228a3803 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -14,6 +14,8 @@ TYPE_CHOICES = [ class Actor(models.Model): + ap_type = 'Actor' + url = models.URLField(unique=True, max_length=500, db_index=True) outbox_url = models.URLField(max_length=500) inbox_url = models.URLField(max_length=500) @@ -79,6 +81,8 @@ class Actor(models.Model): class Follow(models.Model): + ap_type = 'Follow' + uuid = models.UUIDField(default=uuid.uuid4, unique=True) actor = models.ForeignKey( Actor, @@ -96,3 +100,6 @@ class Follow(models.Model): class Meta: unique_together = ['actor', 'target'] + + def get_federation_url(self): + return '{}#follows/{}'.format(self.actor.url, self.uuid) diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index d50b52ee6..c1b9d8a23 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -169,11 +169,7 @@ def test_test_post_inbox_handles_create_note( }] ) expected_activity = { - '@context': [ - 'https://www.w3.org/ns/activitystreams', - 'https://w3id.org/security/v1', - {} - ], + '@context': serializers.AP_CONTEXT, 'actor': test_actor.url, 'id': 'https://{}/activities/note/{}/activity'.format( settings.FEDERATION_HOSTNAME, now.timestamp() @@ -288,11 +284,7 @@ def test_test_actor_handles_follow( }, } expected_follow = { - '@context': [ - 'https://www.w3.org/ns/activitystreams', - 'https://w3id.org/security/v1', - {} - ], + '@context': serializers.AP_CONTEXT, 'actor': test_actor.url, 'id': test_actor.url + '#follows/{}'.format(uid), 'object': actor.url, @@ -317,3 +309,38 @@ def test_test_actor_handles_follow( follow = test_actor.received_follows.first() assert follow.actor == actor assert follow.target == test_actor + + +def test_test_actor_handles_undo_follow( + settings, mocker, factories): + deliver = mocker.patch( + 'funkwhale_api.federation.activity.deliver') + test_actor = actors.SYSTEM_ACTORS['test'].get_actor_instance() + follow = factories['federation.Follow'](target=test_actor) + reverse_follow = factories['federation.Follow']( + actor=test_actor, target=follow.actor) + follow_serializer = serializers.FollowSerializer(follow) + reverse_follow_serializer = serializers.FollowSerializer( + reverse_follow) + undo = { + '@context': serializers.AP_CONTEXT, + 'type': 'Undo', + 'id': follow_serializer.data['id'] + '/undo', + 'actor': follow.actor.url, + 'object': follow_serializer.data, + } + expected_undo = { + '@context': serializers.AP_CONTEXT, + 'type': 'Undo', + 'id': reverse_follow_serializer.data['id'] + '/undo', + 'actor': reverse_follow.actor.url, + 'object': reverse_follow_serializer.data, + } + + actors.SYSTEM_ACTORS['test'].post_inbox(undo, actor=follow.actor) + deliver.assert_called_once_with( + expected_undo, + to=[follow.actor.url], + on_behalf_of=test_actor,) + + assert models.Follow.objects.count() == 0 diff --git a/api/tests/federation/test_authentication.py b/api/tests/federation/test_authentication.py index 1837b3950..c6a97a07a 100644 --- a/api/tests/federation/test_authentication.py +++ b/api/tests/federation/test_authentication.py @@ -3,7 +3,7 @@ from funkwhale_api.federation import keys from funkwhale_api.federation import signing -def test_authenticate(nodb_factories, mocker, api_request): +def test_authenticate(factories, mocker, api_request): private, public = keys.get_key_pair() actor_url = 'https://test.federation/actor' mocker.patch( @@ -18,7 +18,7 @@ def test_authenticate(nodb_factories, mocker, api_request): 'id': actor_url + '#main-key', } }) - signed_request = nodb_factories['federation.SignedRequest']( + signed_request = factories['federation.SignedRequest']( auth__key=private, auth__key_id=actor_url + '#main-key', auth__headers=[ diff --git a/api/tests/federation/test_models.py b/api/tests/federation/test_models.py index 297fe2c58..18daf8788 100644 --- a/api/tests/federation/test_models.py +++ b/api/tests/federation/test_models.py @@ -23,3 +23,10 @@ def test_cannot_duplicate_follow(factories): target=follow.target, actor=follow.actor, ) + +def test_follow_federation_url(factories): + follow = factories['federation.Follow'](local=True) + expected = '{}#follows/{}'.format( + follow.actor.url, follow.uuid) + + assert follow.get_federation_url() == expected From b833a11fb657c41755f18002aa4daca50c43ccd6 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 4 Apr 2018 19:38:28 +0200 Subject: [PATCH 10/30] FollowRequest model --- .../migrations/0004_followrequest.py | 28 +++++++++++++++++++ api/funkwhale_api/federation/models.py | 18 ++++++++++++ 2 files changed, 46 insertions(+) create mode 100644 api/funkwhale_api/federation/migrations/0004_followrequest.py diff --git a/api/funkwhale_api/federation/migrations/0004_followrequest.py b/api/funkwhale_api/federation/migrations/0004_followrequest.py new file mode 100644 index 000000000..6ede72747 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0004_followrequest.py @@ -0,0 +1,28 @@ +# Generated by Django 2.0.3 on 2018-04-04 17:11 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0003_auto_20180403_1921'), + ] + + operations = [ + migrations.CreateModel( + name='FollowRequest', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('last_modification_date', models.DateTimeField(default=django.utils.timezone.now)), + ('approved', models.NullBooleanField(default=None)), + ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emmited_follow_requests', to='federation.Actor')), + ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follow_requests', to='federation.Actor')), + ], + ), + ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index a228a3803..50ff9d319 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -103,3 +103,21 @@ class Follow(models.Model): def get_federation_url(self): return '{}#follows/{}'.format(self.actor.url, self.uuid) + + +class FollowRequest(models.Model): + uuid = models.UUIDField(default=uuid.uuid4, unique=True) + actor = models.ForeignKey( + Actor, + related_name='emmited_follow_requests', + on_delete=models.CASCADE, + ) + target = models.ForeignKey( + Actor, + related_name='received_follow_requests', + on_delete=models.CASCADE, + ) + creation_date = models.DateTimeField(default=timezone.now) + last_modification_date = models.DateTimeField( + default=timezone.now) + approved = models.NullBooleanField(default=None) From d8f86c4fce52b6b610ac72e22bf658ecc1cbba3c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 4 Apr 2018 19:38:55 +0200 Subject: [PATCH 11/30] Factorized follow logic between system actors, Library can now accept follows --- api/funkwhale_api/federation/activity.py | 19 +++++ api/funkwhale_api/federation/actors.py | 61 ++++++++-------- api/tests/federation/test_activity.py | 42 +++++++++++ api/tests/federation/test_actors.py | 89 ++++++++++++++---------- 4 files changed, 145 insertions(+), 66 deletions(-) diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 5a0974011..1b03d19f8 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -2,7 +2,9 @@ import logging import json import requests import requests_http_signature +import uuid +from . import models from . import signing logger = logging.getLogger(__name__) @@ -117,3 +119,20 @@ def get_accept_follow(accept_id, accept_actor, follow, follow_actor): "object": accept_actor.url }, } + + +def accept_follow(target, follow, actor): + accept_uuid = uuid.uuid4() + accept = get_accept_follow( + accept_id=accept_uuid, + accept_actor=target, + follow=follow, + follow_actor=actor) + deliver( + accept, + to=[actor.url], + on_behalf_of=target) + return models.Follow.objects.get_or_create( + actor=actor, + target=target, + ) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 89c621edc..8871b1013 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -132,6 +132,20 @@ class SystemActor(object): return handler(data, actor) + def handle_follow(self, ac, sender): + system_actor = self.get_actor_instance() + if self.manually_approves_followers: + fr, created = models.FollowRequest.objects.get_or_create( + actor=sender, + target=system_actor, + approved=None, + ) + return fr + + return activity.accept_follow( + system_actor, ac, sender + ) + class LibraryActor(SystemActor): id = 'library' @@ -140,6 +154,7 @@ class LibraryActor(SystemActor): additional_attributes = { 'manually_approves_followers': True } + @property def manually_approves_followers(self): return settings.FEDERATION_MUSIC_NEEDS_APPROVAL @@ -159,18 +174,18 @@ class TestActor(SystemActor): def get_outbox(self, data, actor=None): return { - "@context": [ - "https://www.w3.org/ns/activitystreams", - "https://w3id.org/security/v1", - {} - ], - "id": utils.full_url( + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], + "id": utils.full_url( reverse( 'federation:instance-actors-outbox', kwargs={'actor': self.id})), - "type": "OrderedCollection", - "totalItems": 0, - "orderedItems": [] + "type": "OrderedCollection", + "totalItems": 0, + "orderedItems": [] } def parse_command(self, message): @@ -204,10 +219,10 @@ class TestActor(SystemActor): ) reply_activity = { "@context": [ - "https://www.w3.org/ns/activitystreams", - "https://w3id.org/security/v1", - {} - ], + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], 'type': 'Create', 'actor': test_actor.url, 'id': '{}/activity'.format(reply_url), @@ -240,25 +255,9 @@ class TestActor(SystemActor): on_behalf_of=test_actor) def handle_follow(self, ac, sender): - # on a follow we: - # 1. send the accept answer - # 2. follow back - # + super().handle_follow(ac, sender) + # also, we follow back test_actor = self.get_actor_instance() - accept_uuid = uuid.uuid4() - accept = activity.get_accept_follow( - accept_id=accept_uuid, - accept_actor=test_actor, - follow=ac, - follow_actor=sender) - activity.deliver( - accept, - to=[ac['actor']], - on_behalf_of=test_actor) - models.Follow.objects.get_or_create( - actor=sender, - target=test_actor, - ) follow_uuid = uuid.uuid4() follow = activity.get_follow( follow_id=follow_uuid, diff --git a/api/tests/federation/test_activity.py b/api/tests/federation/test_activity.py index a6e1d28aa..09c5e3bf7 100644 --- a/api/tests/federation/test_activity.py +++ b/api/tests/federation/test_activity.py @@ -1,5 +1,8 @@ +import uuid + from funkwhale_api.federation import activity + def test_deliver(nodb_factories, r_mock, mocker): to = nodb_factories['federation.Actor']() mocker.patch( @@ -30,3 +33,42 @@ def test_deliver(nodb_factories, r_mock, mocker): assert r_mock.call_count == 1 assert request.url == to.inbox_url assert request.headers['content-type'] == 'application/activity+json' + + +def test_accept_follow(mocker, factories): + deliver = mocker.patch( + 'funkwhale_api.federation.activity.deliver') + actor = factories['federation.Actor']() + target = factories['federation.Actor'](local=True) + follow = { + 'actor': actor.url, + 'type': 'Follow', + 'id': 'http://test.federation/user#follows/267', + 'object': target.url, + } + uid = uuid.uuid4() + mocker.patch('uuid.uuid4', return_value=uid) + expected_accept = { + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], + "id": target.url + '#accepts/follows/{}'.format(uid), + "type": "Accept", + "actor": target.url, + "object": { + "id": follow['id'], + "type": "Follow", + "actor": actor.url, + "object": target.url + }, + } + activity.accept_follow( + target, follow, actor + ) + deliver.assert_called_once_with( + expected_accept, to=[actor.url], on_behalf_of=target + ) + follow_instance = actor.emitted_follows.first() + assert follow_instance.target == target diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index c1b9d8a23..5ade9cdc8 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -93,18 +93,18 @@ def test_get_test(settings, preferences): def test_test_get_outbox(): expected = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - "https://w3id.org/security/v1", - {} - ], - "id": utils.full_url( + "@context": [ + "https://www.w3.org/ns/activitystreams", + "https://w3id.org/security/v1", + {} + ], + "id": utils.full_url( reverse( 'federation:instance-actors-outbox', kwargs={'actor': 'test'})), - "type": "OrderedCollection", - "totalItems": 0, - "orderedItems": [] + "type": "OrderedCollection", + "totalItems": 0, + "orderedItems": [] } data = actors.SYSTEM_ACTORS['test'].get_outbox({}, actor=None) @@ -248,7 +248,7 @@ def test_system_actor_handle(mocker, nodb_factories): ) assert serializer.is_valid() actors.SYSTEM_ACTORS['test'].handle(activity, actor) - handler.assert_called_once_with(serializer.data, actor) + handler.assert_called_once_with(activity, actor) def test_test_actor_handles_follow( @@ -258,6 +258,8 @@ def test_test_actor_handles_follow( actor = factories['federation.Actor']() now = timezone.now() mocker.patch('django.utils.timezone.now', return_value=now) + accept_follow = mocker.patch( + 'funkwhale_api.federation.activity.accept_follow') test_actor = actors.SYSTEM_ACTORS['test'].get_actor_instance() data = { 'actor': actor.url, @@ -267,22 +269,6 @@ def test_test_actor_handles_follow( } uid = uuid.uuid4() mocker.patch('uuid.uuid4', return_value=uid) - expected_accept = { - "@context": [ - "https://www.w3.org/ns/activitystreams", - "https://w3id.org/security/v1", - {} - ], - "id": test_actor.url + '#accepts/follows/{}'.format(uid), - "type": "Accept", - "actor": test_actor.url, - "object": { - "id": data['id'], - "type": "Follow", - "actor": actor.url, - "object": test_actor.url - }, - } expected_follow = { '@context': serializers.AP_CONTEXT, 'actor': test_actor.url, @@ -292,12 +278,10 @@ def test_test_actor_handles_follow( } actors.SYSTEM_ACTORS['test'].post_inbox(data, actor=actor) + accept_follow.assert_called_once_with( + test_actor, data, actor + ) expected_calls = [ - mocker.call( - expected_accept, - to=[actor.url], - on_behalf_of=test_actor, - ), mocker.call( expected_follow, to=[actor.url], @@ -306,10 +290,6 @@ def test_test_actor_handles_follow( ] deliver.assert_has_calls(expected_calls) - follow = test_actor.received_follows.first() - assert follow.actor == actor - assert follow.target == test_actor - def test_test_actor_handles_undo_follow( settings, mocker, factories): @@ -344,3 +324,42 @@ def test_test_actor_handles_undo_follow( on_behalf_of=test_actor,) assert models.Follow.objects.count() == 0 + + +def test_library_actor_handles_follow_manual_approval( + settings, mocker, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + actor = factories['federation.Actor']() + now = timezone.now() + mocker.patch('django.utils.timezone.now', return_value=now) + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + data = { + 'actor': actor.url, + 'type': 'Follow', + 'id': 'http://test.federation/user#follows/267', + 'object': library_actor.url, + } + + library_actor.system_conf.post_inbox(data, actor=actor) + fr = library_actor.received_follow_requests.first() + + assert library_actor.received_follow_requests.count() == 1 + assert fr.target == library_actor + assert fr.actor == actor + assert fr.approved is None + + +def test_library_actor_handles_follow_auto_approval( + settings, mocker, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + actor = factories['federation.Actor']() + accept_follow = mocker.patch( + 'funkwhale_api.federation.activity.accept_follow') + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + data = { + 'actor': actor.url, + 'type': 'Follow', + 'id': 'http://test.federation/user#follows/267', + 'object': library_actor.url, + } + library_actor.system_conf.post_inbox(data, actor=actor) From cb9309c29882ebce5b618d0a242f39978a3be286 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 4 Apr 2018 22:40:57 +0200 Subject: [PATCH 12/30] Factorized undo follow --- api/funkwhale_api/federation/activity.py | 14 +++++++ api/funkwhale_api/federation/actors.py | 50 +++++++++++++----------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index 1b03d19f8..b253955c8 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -101,6 +101,20 @@ def get_follow(follow_id, follower, followed): } +def get_undo(id, actor, object): + return { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {} + ], + 'type': 'Undo', + 'id': id + '/undo', + 'actor': actor.url, + 'object': object, + } + + def get_accept_follow(accept_id, accept_actor, follow, follow_actor): return { "@context": [ diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 8871b1013..0da78fdbe 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -146,6 +146,23 @@ class SystemActor(object): system_actor, ac, sender ) + def handle_undo_follow(self, ac, sender): + actor = self.get_actor_instance() + models.Follow.objects.filter( + actor=sender, + target=actor, + ).delete() + + def handle_undo(self, ac, sender): + if ac['object']['type'] != 'Follow': + return + + if ac['object']['actor'] != sender.url: + # not the same actor, permission issue + return + + self.handle_undo_follow(ac, sender) + class LibraryActor(SystemActor): id = 'library' @@ -268,39 +285,28 @@ class TestActor(SystemActor): to=[ac['actor']], on_behalf_of=test_actor) - def handle_undo(self, ac, sender): - if ac['object']['type'] != 'Follow': - return - - if ac['object']['actor'] != sender.url: - # not the same actor, permission issue - return - - test_actor = self.get_actor_instance() - models.Follow.objects.filter( - actor=sender, - target=test_actor, - ).delete() + def handle_undo_follow(self, ac, sender): + super().handle_undo_follow(ac, sender) + actor = self.get_actor_instance() # we also unfollow the sender, if possible try: follow = models.Follow.objects.get( target=sender, - actor=test_actor, + actor=actor, ) except models.Follow.DoesNotExist: return - undo = { - '@context': serializers.AP_CONTEXT, - 'type': 'Undo', - 'id': follow.get_federation_url() + '/undo', - 'actor': test_actor.url, - 'object': serializers.FollowSerializer(follow).data, - } + undo = activity.get_undo( + id=follow.get_federation_url(), + actor=actor, + object=serializers.FollowSerializer(follow).data, + ) follow.delete() activity.deliver( undo, to=[sender.url], - on_behalf_of=test_actor) + on_behalf_of=actor) + SYSTEM_ACTORS = { 'library': LibraryActor(), From e0dcb87f15579cf0018dc40936fdc5352b2648a9 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 4 Apr 2018 23:12:41 +0200 Subject: [PATCH 13/30] Follow request approve/refuse logic --- api/funkwhale_api/federation/factories.py | 9 +++++ api/funkwhale_api/federation/models.py | 36 ++++++++++++++++++ api/tests/federation/test_actors.py | 7 +++- api/tests/federation/test_models.py | 46 +++++++++++++++++++++++ 4 files changed, 97 insertions(+), 1 deletion(-) diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 16abf80dc..277b9ce0c 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -97,6 +97,15 @@ class FollowFactory(factory.DjangoModelFactory): ) +@registry.register +class FollowRequestFactory(factory.DjangoModelFactory): + target = factory.SubFactory(ActorFactory) + actor = factory.SubFactory(ActorFactory) + + class Meta: + model = models.FollowRequest + + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): type = 'Note' diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 50ff9d319..833b5d8f3 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -35,6 +35,13 @@ class Actor(models.Model): last_fetch_date = models.DateTimeField( default=timezone.now) manually_approves_followers = models.NullBooleanField(default=None) + followers = models.ManyToManyField( + to='self', + symmetrical=False, + through='Follow', + through_fields=('target', 'actor'), + related_name='following', + ) class Meta: unique_together = ['domain', 'preferred_username'] @@ -65,6 +72,10 @@ class Actor(models.Model): super().save(**kwargs) + @property + def is_local(self): + return self.domain == settings.FEDERATION_HOSTNAME + @property def is_system(self): from . import actors @@ -121,3 +132,28 @@ class FollowRequest(models.Model): last_modification_date = models.DateTimeField( default=timezone.now) approved = models.NullBooleanField(default=None) + + def approve(self): + from . import activity + from . import serializers + self.approved = True + self.save(update_fields=['approved']) + Follow.objects.get_or_create( + target=self.target, + actor=self.actor + ) + if self.target.is_local: + follow = { + '@context': serializers.AP_CONTEXT, + 'actor': self.actor.url, + 'id': self.actor.url + '#follows/{}'.format(uuid.uuid4()), + 'object': self.target.url, + 'type': 'Follow' + } + activity.accept_follow( + self.target, follow, self.actor + ) + + def refuse(self): + self.approved = False + self.save(update_fields=['approved']) diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 5ade9cdc8..be24a5360 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -351,7 +351,7 @@ def test_library_actor_handles_follow_manual_approval( def test_library_actor_handles_follow_auto_approval( settings, mocker, factories): - settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False actor = factories['federation.Actor']() accept_follow = mocker.patch( 'funkwhale_api.federation.activity.accept_follow') @@ -363,3 +363,8 @@ def test_library_actor_handles_follow_auto_approval( 'object': library_actor.url, } library_actor.system_conf.post_inbox(data, actor=actor) + + assert library_actor.received_follow_requests.count() == 0 + accept_follow.assert_called_once_with( + library_actor, data, actor + ) diff --git a/api/tests/federation/test_models.py b/api/tests/federation/test_models.py index 18daf8788..86e4f4a84 100644 --- a/api/tests/federation/test_models.py +++ b/api/tests/federation/test_models.py @@ -1,8 +1,10 @@ import pytest +import uuid from django import db from funkwhale_api.federation import models +from funkwhale_api.federation import serializers def test_cannot_duplicate_actor(factories): @@ -30,3 +32,47 @@ def test_follow_federation_url(factories): follow.actor.url, follow.uuid) assert follow.get_federation_url() == expected + + +def test_follow_request_approve(mocker, factories): + uid = uuid.uuid4() + mocker.patch('uuid.uuid4', return_value=uid) + accept_follow = mocker.patch( + 'funkwhale_api.federation.activity.accept_follow') + fr = factories['federation.FollowRequest'](target__local=True) + fr.approve() + + follow = { + '@context': serializers.AP_CONTEXT, + 'actor': fr.actor.url, + 'id': fr.actor.url + '#follows/{}'.format(uid), + 'object': fr.target.url, + 'type': 'Follow' + } + + assert fr.approved is True + assert list(fr.target.followers.all()) == [fr.actor] + accept_follow.assert_called_once_with( + fr.target, follow, fr.actor + ) + + +def test_follow_request_approve_non_local(mocker, factories): + uid = uuid.uuid4() + mocker.patch('uuid.uuid4', return_value=uid) + accept_follow = mocker.patch( + 'funkwhale_api.federation.activity.accept_follow') + fr = factories['federation.FollowRequest']() + fr.approve() + + assert fr.approved is True + assert list(fr.target.followers.all()) == [fr.actor] + accept_follow.assert_not_called() + + +def test_follow_request_refused(mocker, factories): + fr = factories['federation.FollowRequest']() + fr.refuse() + + assert fr.approved is False + assert fr.target.followers.count() == 0 From 4d6e894b6291684b97a42d0d864020137f800715 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 5 Apr 2018 23:22:28 +0200 Subject: [PATCH 14/30] AudioCollection to import job and track file creation --- api/funkwhale_api/music/serializers.py | 81 ++++++++++++++++++++ api/funkwhale_api/music/tasks.py | 48 +++++++++++- api/tests/music/test_import.py | 101 +++++++++++++++++++++++++ api/tests/music/test_serializers.py | 32 ++++++++ 4 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 api/tests/music/test_serializers.py diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 48419bbe4..9f0b7af5c 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -1,3 +1,4 @@ +from django.db import transaction from rest_framework import serializers from taggit.models import Tag @@ -150,3 +151,83 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer): def get_type(self, obj): return 'Audio' + + +class AudioMetadataSerializer(serializers.Serializer): + artist = serializers.CharField(required=False) + release = serializers.CharField(required=False) + recording = serializers.CharField(required=False) + + +class AudioSerializer(serializers.Serializer): + type = serializers.CharField() + id = serializers.URLField() + url = serializers.JSONField() + metadata = AudioMetadataSerializer() + + def validate_type(self, v): + if v != 'Audio': + raise serializers.ValidationError('Invalid type for audio') + return v + + 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/'): + 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/'): + raise serializers.ValidationError('Invalid mediaType') + + return v + + def create(self, validated_data, batch): + metadata = validated_data['metadata'].copy() + metadata['mediaType'] = validated_data['url']['mediaType'] + return models.ImportJob.objects.create( + batch=batch, + source=validated_data['url']['href'], + federation_source=validated_data['id'], + metadata=metadata, + ) + + +class AudioCollectionImportSerializer(serializers.Serializer): + id = serializers.URLField() + items = serializers.ListField( + child=AudioSerializer(), + min_length=1, + ) + + @transaction.atomic + def create(self, validated_data): + batch = models.ImportBatch.objects.create( + federation_actor=self.context['sender'], + federation_source=validated_data['id'], + source='federation', + ) + for i in validated_data['items']: + s = AudioSerializer(data=i) + job = s.create(i, batch) + return batch diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index bf7a847d0..e4214d990 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -25,6 +25,44 @@ def set_acoustid_on_track_file(track_file): return update(result['id']) +def get_mbid(url, type): + prefix = 'https://musicbrainz.org/{}/'.format(type) + if url.startswith(prefix): + return url.replace(prefix, '') + + +def import_track_from_metadata(metadata): + raw_track = metadata['recording'] + if isinstance(raw_track, str): + track_mbid = get_mbid(raw_track, 'recording') + return models.Track.get_or_create_from_api(mbid=track_mbid) + + raw_album = metadata['release'] + if isinstance(raw_album, str): + album_mbid = get_mbid(raw_album, 'release') + album = models.Album.get_or_create_from_api(mbid=album_mbid) + return models.Track.get_or_create_from_title( + raw_track['title'], artist=album.artist, album=album) + + raw_artist = metadata['artist'] + if isinstance(raw_artist, str): + artist_mbid = get_mbid(raw_artist, 'artist') + artist = models.Artist.get_or_create_from_api(mbid=artist_mbid) + album = models.Album.get_or_create_from_title( + raw_album['title'], artist=artist) + return models.Track.get_or_create_from_title( + raw_track['title'], artist=artist, album=album) + + # worst case scenario, we have absolutely no way to link to a + # musicbrainz resource, we rely on the name/titles + artist = models.Artist.get_or_create_from_name( + raw_artist['name']) + album = models.Album.get_or_create_from_title( + raw_album['title'], artist=artist) + return models.Track.get_or_create_from_title( + raw_track['title'], artist=artist, album=album) + + def _do_import(import_job, replace, use_acoustid=True): from_file = bool(import_job.audio_file) mbid = import_job.mbid @@ -43,9 +81,14 @@ def _do_import(import_job, replace, use_acoustid=True): acoustid_track_id = match['id'] if mbid: track, _ = models.Track.get_or_create_from_api(mbid=mbid) - else: + elif import_job.audio_file: track = import_track_data_from_path(import_job.audio_file.path) + else: + # probably federation, we use metadata stored on the job itself + if not import_job.metadata: + raise ValueError('We cannot import without at least metadatas') + track = import_track_from_metadata(import_job.metadata) track_file = None if replace: track_file = track.files.first() @@ -63,6 +106,9 @@ def _do_import(import_job, replace, use_acoustid=True): track_file.audio_file = ContentFile(import_job.audio_file.read()) track_file.audio_file.name = import_job.audio_file.name track_file.duration = duration + elif import_job.federation_source: + # no downloading, we hotlink + pass else: track_file.download_file() track_file.save() diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 0f709e81f..e9ad9d0f5 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -1,7 +1,10 @@ import json +import pytest from django.urls import reverse +from funkwhale_api.music import tasks + def test_create_import_can_bind_to_request( artists, albums, mocker, factories, superuser_api_client): @@ -33,3 +36,101 @@ def test_create_import_can_bind_to_request( batch = request.import_batches.latest('id') assert batch.import_request == request + + +@pytest.mark.parametrize('url,type,expected', [ + ('https://musicbrainz.org/artist/test', 'artist', 'test'), + ('https://musicbrainz.org/release/test', 'release', 'test'), + ('https://musicbrainz.org/recording/test', 'recording', 'test'), + ('https://musicbrainz.org/recording/test', 'artist', None), +]) +def test_get_mbid(url, type, expected): + assert tasks.get_mbid(url, type) == expected + + +def test_import_job_from_federation_no_musicbrainz(factories): + job = factories['music.ImportJob']( + federation=True, + metadata__artist={'name': 'Hello'}, + metadata__release={'title': 'World'}, + metadata__recording={'title': 'Ping'}, + mbid=None, + ) + + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + tf = job.track_file + assert tf.track.title == 'Ping' + assert tf.track.artist.name == 'Hello' + assert tf.track.album.title == 'World' + + +def test_import_job_from_federation_musicbrainz_recording(factories, mocker): + t = factories['music.Track']() + track_from_api = mocker.patch( + 'funkwhale_api.music.models.Track.get_or_create_from_api', + return_value=t) + job = factories['music.ImportJob']( + federation=True, + metadata__artist={'name': 'Hello'}, + metadata__release={'title': 'World'}, + mbid=None, + ) + + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + tf = job.track_file + assert tf.track == t + track_from_api.assert_called_once_with( + mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) + + +def test_import_job_from_federation_musicbrainz_release(factories, mocker): + a = factories['music.Album']() + album_from_api = mocker.patch( + 'funkwhale_api.music.models.Album.get_or_create_from_api', + return_value=a) + job = factories['music.ImportJob']( + federation=True, + metadata__artist={'name': 'Hello'}, + metadata__recording={'title': 'Ping'}, + mbid=None, + ) + + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + tf = job.track_file + assert tf.track.title == 'Ping' + assert tf.track.artist == a.artist + assert tf.track.album == a + + album_from_api.assert_called_once_with( + mbid=tasks.get_mbid(job.metadata['release'], 'release')) + + +def test_import_job_from_federation_musicbrainz_artist(factories, mocker): + a = factories['music.Artist']() + artist_from_api = mocker.patch( + 'funkwhale_api.music.models.Artist.get_or_create_from_api', + return_value=a) + job = factories['music.ImportJob']( + federation=True, + metadata__release={'title': 'World'}, + metadata__recording={'title': 'Ping'}, + mbid=None, + ) + + tasks.import_job_run(import_job_id=job.pk) + job.refresh_from_db() + + tf = job.track_file + assert tf.track.title == 'Ping' + assert tf.track.artist == a + assert tf.track.album.artist == a + assert tf.track.album.title == 'World' + + artist_from_api.assert_called_once_with( + mbid=tasks.get_mbid(job.metadata['artist'], 'artist')) diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py new file mode 100644 index 000000000..556ac4c0e --- /dev/null +++ b/api/tests/music/test_serializers.py @@ -0,0 +1,32 @@ +from funkwhale_api.music import serializers + + +def test_activity_pub_audio_collection_serializer(factories): + sender = factories['federation.Actor']() + + collection = { + 'id': 'https://batch.import', + 'type': 'Collection', + 'totalItems': 2, + 'items': factories['federation.Audio'].create_batch(size=2) + } + + serializer = serializers.AudioCollectionImportSerializer( + data=collection, context={'sender': sender}) + + assert serializer.is_valid(raise_exception=True) + + batch = serializer.save() + jobs = list(batch.jobs.all()) + + assert batch.source == 'federation' + assert batch.federation_source == collection['id'] + assert batch.federation_actor == sender + assert len(jobs) == 2 + + for i, a in enumerate(collection['items']): + job = jobs[i] + assert job.federation_source == a['id'] + assert job.source == a['url']['href'] + a['metadata']['mediaType'] = a['url']['mediaType'] + assert job.metadata == a['metadata'] From 363acca53db4ea164e18892b603de4724bf05648 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 5 Apr 2018 23:26:41 +0200 Subject: [PATCH 15/30] AudioCollection to import job and track file creation --- .../migrations/0005_actor_followers.py | 18 +++++++ .../migrations/0023_auto_20180405_1830.py | 47 +++++++++++++++++++ api/funkwhale_api/music/models.py | 47 ++++++++++++++++++- api/tests/music/test_import.py | 4 ++ 4 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 api/funkwhale_api/federation/migrations/0005_actor_followers.py create mode 100644 api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py diff --git a/api/funkwhale_api/federation/migrations/0005_actor_followers.py b/api/funkwhale_api/federation/migrations/0005_actor_followers.py new file mode 100644 index 000000000..94a1c75ac --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0005_actor_followers.py @@ -0,0 +1,18 @@ +# Generated by Django 2.0.3 on 2018-04-05 16:35 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0004_followrequest'), + ] + + operations = [ + migrations.AddField( + model_name='actor', + name='followers', + field=models.ManyToManyField(related_name='following', through='federation.Follow', to='federation.Actor'), + ), + ] diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py b/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py new file mode 100644 index 000000000..3cef1f42e --- /dev/null +++ b/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py @@ -0,0 +1,47 @@ +# Generated by Django 2.0.3 on 2018-04-05 18:30 + +from django.conf import settings +import django.contrib.postgres.fields.jsonb +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0005_actor_followers'), + ('music', '0022_importbatch_import_request'), + ] + + operations = [ + migrations.AddField( + model_name='importbatch', + name='federation_actor', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Actor'), + ), + migrations.AddField( + model_name='importbatch', + name='federation_source', + field=models.URLField(blank=True, null=True), + ), + migrations.AddField( + model_name='importjob', + name='federation_source', + field=models.URLField(blank=True, null=True), + ), + migrations.AddField( + model_name='importjob', + name='metadata', + field=django.contrib.postgres.fields.jsonb.JSONField(default={}), + ), + migrations.AlterField( + model_name='importbatch', + name='source', + field=models.CharField(choices=[('api', 'api'), ('shell', 'shell'), ('federation', 'federation')], default='api', max_length=30), + ), + migrations.AlterField( + model_name='importbatch', + name='submitted_by', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='imports', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 7138dcdd6..cff162972 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -8,6 +8,7 @@ import markdown from django.conf import settings from django.db import models +from django.contrib.postgres.fields import JSONField from django.core.files.base import ContentFile from django.core.files import File from django.db.models.signals import post_save @@ -65,6 +66,7 @@ class APIModelMixin(models.Model): pass return cleaned_data + class Artist(APIModelMixin): name = models.CharField(max_length=255) @@ -90,10 +92,19 @@ class Artist(APIModelMixin): t.append(tag) return set(t) + @classmethod + def get_or_create_from_name(cls, name, **kwargs): + kwargs.update({'name': name}) + return cls.objects.get_or_create( + name__iexact=name, + defaults=kwargs)[0] + + def import_artist(v): a = Artist.get_or_create_from_api(mbid=v[0]['artist']['id'])[0] return a + def parse_date(v): if len(v) == 4: return datetime.date(int(v), 1, 1) @@ -108,6 +119,7 @@ def import_tracks(instance, cleaned_data, raw_data): track_cleaned_data['position'] = int(track_data['position']) track = importers.load(Track, track_cleaned_data, track_data, Track.import_hooks) + class Album(APIModelMixin): title = models.CharField(max_length=255) artist = models.ForeignKey( @@ -170,6 +182,14 @@ class Album(APIModelMixin): t.append(tag) return set(t) + @classmethod + def get_or_create_from_title(cls, title, **kwargs): + kwargs.update({'title': title}) + return cls.objects.get_or_create( + title__iexact=title, + defaults=kwargs)[0] + + def import_tags(instance, cleaned_data, raw_data): MINIMUM_COUNT = 2 tags_to_add = [] @@ -182,6 +202,7 @@ def import_tags(instance, cleaned_data, raw_data): tags_to_add.append(tag_data['name']) instance.tags.add(*tags_to_add) + def import_album(v): a = Album.get_or_create_from_api(mbid=v[0]['id'])[0] return a @@ -328,7 +349,7 @@ class Track(APIModelMixin): def save(self, **kwargs): try: self.artist - except Artist.DoesNotExist: + except Artist.DoesNotExist: self.artist = self.album.artist super().save(**kwargs) @@ -366,6 +387,13 @@ class Track(APIModelMixin): self.mbid) return settings.FUNKWHALE_URL + '/tracks/{}'.format(self.pk) + @classmethod + def get_or_create_from_title(cls, title, **kwargs): + kwargs.update({'title': title}) + return cls.objects.get_or_create( + title__iexact=title, + defaults=kwargs)[0] + class TrackFile(models.Model): track = models.ForeignKey( @@ -420,7 +448,8 @@ IMPORT_STATUS_CHOICES = ( class ImportBatch(models.Model): IMPORT_BATCH_SOURCES = [ ('api', 'api'), - ('shell', 'shell') + ('shell', 'shell'), + ('federation', 'federation'), ] source = models.CharField( max_length=30, default='api', choices=IMPORT_BATCH_SOURCES) @@ -428,6 +457,8 @@ class ImportBatch(models.Model): submitted_by = models.ForeignKey( 'users.User', related_name='imports', + null=True, + blank=True, on_delete=models.CASCADE) status = models.CharField( choices=IMPORT_STATUS_CHOICES, default='pending', max_length=30) @@ -437,6 +468,16 @@ class ImportBatch(models.Model): null=True, blank=True, on_delete=models.CASCADE) + + federation_source = models.URLField(null=True, blank=True) + federation_actor = models.ForeignKey( + 'federation.Actor', + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name='import_batches', + ) + class Meta: ordering = ['-creation_date'] @@ -464,6 +505,8 @@ class ImportJob(models.Model): choices=IMPORT_STATUS_CHOICES, default='pending', max_length=30) audio_file = models.FileField( upload_to='imports/%Y/%m/%d', max_length=255, null=True, blank=True) + federation_source = models.URLField(null=True, blank=True) + metadata = JSONField(default={}) class Meta: ordering = ('id', ) diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index e9ad9d0f5..87e1899d6 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -61,6 +61,7 @@ def test_import_job_from_federation_no_musicbrainz(factories): job.refresh_from_db() tf = job.track_file + assert tf.source == job.source assert tf.track.title == 'Ping' assert tf.track.artist.name == 'Hello' assert tf.track.album.title == 'World' @@ -82,6 +83,7 @@ def test_import_job_from_federation_musicbrainz_recording(factories, mocker): job.refresh_from_db() tf = job.track_file + assert tf.source == job.source assert tf.track == t track_from_api.assert_called_once_with( mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) @@ -103,6 +105,7 @@ def test_import_job_from_federation_musicbrainz_release(factories, mocker): job.refresh_from_db() tf = job.track_file + assert tf.source == job.source assert tf.track.title == 'Ping' assert tf.track.artist == a.artist assert tf.track.album == a @@ -127,6 +130,7 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): job.refresh_from_db() tf = job.track_file + assert tf.source == job.source assert tf.track.title == 'Ping' assert tf.track.artist == a assert tf.track.album.artist == a From feab0f98ba009ef3eeeef2175fb32a020b3c8084 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 5 Apr 2018 23:27:03 +0200 Subject: [PATCH 16/30] Spaces > Tabs --- api/tests/federation/test_serializers.py | 88 ++++++++++++------------ 1 file changed, 44 insertions(+), 44 deletions(-) diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 77c14531c..6d027ec91 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -7,25 +7,25 @@ from funkwhale_api.federation import serializers def test_actor_serializer_from_ap(db): payload = { - 'id': 'https://test.federation/user', - 'type': 'Person', - 'following': 'https://test.federation/user/following', - 'followers': 'https://test.federation/user/followers', - 'inbox': 'https://test.federation/user/inbox', - 'outbox': 'https://test.federation/user/outbox', - 'preferredUsername': 'user', - 'name': 'Real User', - 'summary': 'Hello world', - 'url': 'https://test.federation/@user', - 'manuallyApprovesFollowers': False, - 'publicKey': { - 'id': 'https://test.federation/user#main-key', - 'owner': 'https://test.federation/user', - 'publicKeyPem': 'yolo' - }, - 'endpoints': { - 'sharedInbox': 'https://test.federation/inbox' - }, + 'id': 'https://test.federation/user', + 'type': 'Person', + 'following': 'https://test.federation/user/following', + 'followers': 'https://test.federation/user/followers', + 'inbox': 'https://test.federation/user/inbox', + 'outbox': 'https://test.federation/user/outbox', + 'preferredUsername': 'user', + 'name': 'Real User', + 'summary': 'Hello world', + 'url': 'https://test.federation/@user', + 'manuallyApprovesFollowers': False, + 'publicKey': { + 'id': 'https://test.federation/user#main-key', + 'owner': 'https://test.federation/user', + 'publicKeyPem': 'yolo' + }, + 'endpoints': { + 'sharedInbox': 'https://test.federation/inbox' + }, } serializer = serializers.ActorSerializer(data=payload) @@ -50,13 +50,13 @@ def test_actor_serializer_from_ap(db): def test_actor_serializer_only_mandatory_field_from_ap(db): payload = { - 'id': 'https://test.federation/user', - 'type': 'Person', - 'following': 'https://test.federation/user/following', - 'followers': 'https://test.federation/user/followers', - 'inbox': 'https://test.federation/user/inbox', - 'outbox': 'https://test.federation/user/outbox', - 'preferredUsername': 'user', + 'id': 'https://test.federation/user', + 'type': 'Person', + 'following': 'https://test.federation/user/following', + 'followers': 'https://test.federation/user/followers', + 'inbox': 'https://test.federation/user/inbox', + 'outbox': 'https://test.federation/user/outbox', + 'preferredUsername': 'user', } serializer = serializers.ActorSerializer(data=payload) @@ -82,24 +82,24 @@ def test_actor_serializer_to_ap(): 'https://w3id.org/security/v1', {}, ], - 'id': 'https://test.federation/user', - 'type': 'Person', - 'following': 'https://test.federation/user/following', - 'followers': 'https://test.federation/user/followers', - 'inbox': 'https://test.federation/user/inbox', - 'outbox': 'https://test.federation/user/outbox', - 'preferredUsername': 'user', - 'name': 'Real User', - 'summary': 'Hello world', - 'manuallyApprovesFollowers': False, - 'publicKey': { - 'id': 'https://test.federation/user#main-key', - 'owner': 'https://test.federation/user', - 'publicKeyPem': 'yolo' - }, - 'endpoints': { - 'sharedInbox': 'https://test.federation/inbox' - }, + 'id': 'https://test.federation/user', + 'type': 'Person', + 'following': 'https://test.federation/user/following', + 'followers': 'https://test.federation/user/followers', + 'inbox': 'https://test.federation/user/inbox', + 'outbox': 'https://test.federation/user/outbox', + 'preferredUsername': 'user', + 'name': 'Real User', + 'summary': 'Hello world', + 'manuallyApprovesFollowers': False, + 'publicKey': { + 'id': 'https://test.federation/user#main-key', + 'owner': 'https://test.federation/user', + 'publicKeyPem': 'yolo' + }, + 'endpoints': { + 'sharedInbox': 'https://test.federation/inbox' + }, } ac = models.Actor( url=expected['id'], From 87daa81762d0b8fd7c4a34671a8f6173a7921e5e Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Thu, 5 Apr 2018 23:27:19 +0200 Subject: [PATCH 17/30] More factories --- api/funkwhale_api/federation/factories.py | 47 +++++++++++++++++++++++ api/funkwhale_api/music/factories.py | 18 +++++++++ 2 files changed, 65 insertions(+) diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 277b9ce0c..63d40aff8 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -1,6 +1,7 @@ import factory import requests import requests_http_signature +import uuid from django.utils import timezone from django.conf import settings @@ -52,6 +53,21 @@ class SignedRequestFactory(factory.Factory): self.headers.update(default_headers) +@registry.register(name='federation.Link') +class LinkFactory(factory.Factory): + type = 'Link' + href = factory.Faker('url') + mediaType = 'text/html' + + class Meta: + model = dict + + class Params: + audio = factory.Trait( + mediaType=factory.Iterator(['audio/mp3', 'audio/ogg']) + ) + + @registry.register class ActorFactory(factory.DjangoModelFactory): public_key = None @@ -135,3 +151,34 @@ class ActivityFactory(factory.Factory): class Meta: model = dict + + +@registry.register(name='federation.AudioMetadata') +class AudioMetadataFactory(factory.Factory): + recording = factory.LazyAttribute( + lambda o: 'https://musicbrainz.org/recording/{}'.format(uuid.uuid4()) + ) + artist = factory.LazyAttribute( + lambda o: 'https://musicbrainz.org/artist/{}'.format(uuid.uuid4()) + ) + release = factory.LazyAttribute( + lambda o: 'https://musicbrainz.org/release/{}'.format(uuid.uuid4()) + ) + + class Meta: + model = dict + + +@registry.register(name='federation.Audio') +class AudioFactory(factory.Factory): + type = 'Audio' + id = factory.Faker('url') + published = factory.LazyFunction( + lambda: timezone.now().isoformat() + ) + actor = factory.Faker('url') + url = factory.SubFactory(LinkFactory, audio=True) + metadata = factory.SubFactory(AudioMetadataFactory) + + class Meta: + model = dict diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 303e45228..27387ca9f 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -2,6 +2,10 @@ import factory import os from funkwhale_api.factories import registry, ManyToManyFromList +from funkwhale_api.federation.factories import ( + AudioMetadataFactory, + ActorFactory, +) from funkwhale_api.users.factories import UserFactory SAMPLES_PATH = os.path.join( @@ -61,6 +65,13 @@ class ImportBatchFactory(factory.django.DjangoModelFactory): class Meta: model = 'music.ImportBatch' + class Params: + federation = factory.Trait( + submitted_by=None, + federation_actor=factory.SubFactory(ActorFactory), + source='federation', + ) + @registry.register class ImportJobFactory(factory.django.DjangoModelFactory): @@ -71,6 +82,13 @@ class ImportJobFactory(factory.django.DjangoModelFactory): class Meta: model = 'music.ImportJob' + class Params: + federation = factory.Trait( + batch=factory.SubFactory(ImportBatchFactory, federation=True), + federation_source=factory.Faker('url'), + metadata=factory.SubFactory(AudioMetadataFactory), + ) + @registry.register(name='music.FileImportJob') class FileImportJobFactory(ImportJobFactory): From 679adfe156dcf784b0865ca58a0709127f42a235 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 13:17:26 +0200 Subject: [PATCH 18/30] See #126: Added uuid field to all music models --- .../migrations/0024_auto_20180406_1115.py | 54 +++++++++++++++++++ api/funkwhale_api/music/models.py | 11 ++++ 2 files changed, 65 insertions(+) create mode 100644 api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py diff --git a/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py b/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py new file mode 100644 index 000000000..8b655a642 --- /dev/null +++ b/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py @@ -0,0 +1,54 @@ +# Generated by Django 2.0.3 on 2018-04-06 11:15 + +from django.db import migrations, models +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('music', '0023_auto_20180405_1830'), + ] + + operations = [ + migrations.AddField( + model_name='album', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='artist', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='importbatch', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='importjob', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='lyrics', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='track', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='trackfile', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='work', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index cff162972..cf48667cc 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -5,6 +5,7 @@ import datetime import tempfile import shutil import markdown +import uuid from django.conf import settings from django.db import models @@ -27,6 +28,8 @@ from . import utils class APIModelMixin(models.Model): mbid = models.UUIDField(unique=True, db_index=True, null=True, blank=True) + uuid = models.UUIDField( + unique=True, db_index=True, default=uuid.uuid4) api_includes = [] creation_date = models.DateTimeField(default=timezone.now) import_hooks = [] @@ -269,6 +272,8 @@ class Work(APIModelMixin): class Lyrics(models.Model): + uuid = models.UUIDField( + unique=True, db_index=True, default=uuid.uuid4) work = models.ForeignKey( Work, related_name='lyrics', @@ -396,6 +401,8 @@ class Track(APIModelMixin): class TrackFile(models.Model): + uuid = models.UUIDField( + unique=True, db_index=True, default=uuid.uuid4) track = models.ForeignKey( Track, related_name='files', on_delete=models.CASCADE) audio_file = models.FileField(upload_to='tracks/%Y/%m/%d', max_length=255) @@ -446,6 +453,8 @@ IMPORT_STATUS_CHOICES = ( ) class ImportBatch(models.Model): + uuid = models.UUIDField( + unique=True, db_index=True, default=uuid.uuid4) IMPORT_BATCH_SOURCES = [ ('api', 'api'), ('shell', 'shell'), @@ -490,6 +499,8 @@ class ImportBatch(models.Model): class ImportJob(models.Model): + uuid = models.UUIDField( + unique=True, db_index=True, default=uuid.uuid4) batch = models.ForeignKey( ImportBatch, related_name='jobs', on_delete=models.CASCADE) track_file = models.ForeignKey( From 80206761a311893b96ecf0fb4e2ae2f5adf7a50f Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 14:26:39 +0200 Subject: [PATCH 19/30] Easy and resusable Audio and AudioCollection serializer --- api/funkwhale_api/music/models.py | 12 +++++ api/funkwhale_api/music/serializers.py | 46 ++++++++++++++++++ api/tests/music/test_serializers.py | 64 +++++++++++++++++++++++++- 3 files changed, 121 insertions(+), 1 deletion(-) diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index cf48667cc..5d8035f35 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -22,6 +22,7 @@ from versatileimagefield.fields import VersatileImageField from funkwhale_api import downloader from funkwhale_api import musicbrainz +from funkwhale_api.federation import utils as federation_utils from . import importers from . import utils @@ -69,6 +70,12 @@ class APIModelMixin(models.Model): pass return cleaned_data + @property + def musicbrainz_url(self): + if self.mbid: + return 'https://musicbrainz.org/{}/{}'.format( + self.musicbrainz_model, self.mbid) + class Artist(APIModelMixin): name = models.CharField(max_length=255) @@ -426,6 +433,11 @@ class TrackFile(models.Model): shutil.rmtree(tmp_dir) return self.audio_file + def get_federation_url(self): + return federation_utils.full_url( + '/federation/music/file/{}'.format(self.uuid) + ) + @property def path(self): if settings.PROTECT_AUDIO_FILES: diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 9f0b7af5c..5cd2f2cc2 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -3,6 +3,8 @@ from rest_framework import serializers from taggit.models import Tag from funkwhale_api.activity import serializers as activity_serializers +from funkwhale_api.federation.serializers import AP_CONTEXT +from funkwhale_api.federation import utils as federation_utils from . import models @@ -212,6 +214,29 @@ class AudioSerializer(serializers.Serializer): metadata=metadata, ) + def to_representation(self, instance): + d = { + 'type': 'Audio', + 'id': instance.get_federation_url(), + 'name': instance.track.full_name, + 'metadata': { + 'artist': instance.track.artist.musicbrainz_url, + 'release': instance.track.album.musicbrainz_url, + 'track': instance.track.musicbrainz_url, + }, + 'url': { + 'href': federation_utils.full_url(instance.path), + 'type': 'Link', + 'mediaType': instance.mimetype + }, + 'attributedTo': [ + self.context['actor'].url + ] + } + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d + class AudioCollectionImportSerializer(serializers.Serializer): id = serializers.URLField() @@ -231,3 +256,24 @@ class AudioCollectionImportSerializer(serializers.Serializer): s = AudioSerializer(data=i) job = s.create(i, batch) return batch + + def to_representation(self, instance): + d = { + 'id': instance['id'], + 'actor': instance['actor'].url, + 'totalItems': len(instance['items']), + 'type': 'Collection', + 'items': [ + AudioSerializer( + i, + context={ + 'actor': instance['actor'], + 'include_ap_context': False + } + ).data + for i in instance['items'] + ] + } + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 556ac4c0e..1270ae765 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -1,7 +1,10 @@ +from funkwhale_api.federation import actors +from funkwhale_api.federation import utils as federation_utils +from funkwhale_api.federation.serializers import AP_CONTEXT from funkwhale_api.music import serializers -def test_activity_pub_audio_collection_serializer(factories): +def test_activity_pub_audio_collection_serializer_to_import(factories): sender = factories['federation.Actor']() collection = { @@ -30,3 +33,62 @@ def test_activity_pub_audio_collection_serializer(factories): assert job.source == a['url']['href'] a['metadata']['mediaType'] = a['url']['mediaType'] assert job.metadata == a['metadata'] + + +def test_activity_pub_audio_serializer_to_ap(factories): + tf = factories['music.TrackFile'](mimetype='audio/mp3') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': AP_CONTEXT, + 'type': 'Audio', + 'id': tf.get_federation_url(), + 'name': tf.track.full_name, + 'metadata': { + 'artist': tf.track.artist.musicbrainz_url, + 'release': tf.track.album.musicbrainz_url, + 'track': tf.track.musicbrainz_url, + }, + 'url': { + 'href': federation_utils.full_url(tf.path), + 'type': 'Link', + 'mediaType': 'audio/mp3' + }, + 'attributedTo': [ + library.url + ] + } + + serializer = serializers.AudioSerializer(tf, context={'actor': library}) + + assert serializer.data == expected + + +def test_activity_pub_audio_collection_serializer_to_ap(factories): + tf1 = factories['music.TrackFile'](mimetype='audio/mp3') + tf2 = factories['music.TrackFile'](mimetype='audio/ogg') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': AP_CONTEXT, + 'id': 'https://test.id', + 'actor': library.url, + 'totalItems': 2, + 'type': 'Collection', + 'items': [ + serializers.AudioSerializer( + tf1, context={'actor': library, 'include_ap_context': False} + ).data, + serializers.AudioSerializer( + tf2, context={'actor': library, 'include_ap_context': False} + ).data, + ] + } + + collection = { + 'id': expected['id'], + 'actor': library, + 'items': [tf1, tf2], + } + serializer = serializers.AudioCollectionImportSerializer( + collection, context={'actor': library, 'id': 'https://test.id'}) + + assert serializer.data == expected From 04d710e690a0ef4fc1cbc18a2af95d9cd3a7349b Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 14:45:06 +0200 Subject: [PATCH 20/30] Library can now receive import info from followed instances --- api/funkwhale_api/federation/activity.py | 2 + api/funkwhale_api/federation/actors.py | 29 ++++++++++++ api/tests/federation/test_actors.py | 57 ++++++++++++++++++++++++ 3 files changed, 88 insertions(+) diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index b253955c8..db71bd4fb 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -44,10 +44,12 @@ ACTIVITY_TYPES = [ OBJECT_TYPES = [ 'Article', 'Audio', + 'Collection', 'Document', 'Event', 'Image', 'Note', + 'OrderedCollection', 'Page', 'Place', 'Profile', diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 0da78fdbe..fa1b56282 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -176,6 +176,35 @@ class LibraryActor(SystemActor): def manually_approves_followers(self): return settings.FEDERATION_MUSIC_NEEDS_APPROVAL + def handle_create(self, ac, sender): + from funkwhale_api.music.serializers import ( + AudioCollectionImportSerializer) + + library = self.get_actor_instance() + if not library.following.filter(url=sender.url).exists(): + logger.info( + 'Skipping import, we\'re not following %s', sender.url) + return + + if ac['object']['type'] != 'Collection': + return + + if ac['object']['totalItems'] <= 0: + return + + items = ac['object']['items'] + + serializer = AudioCollectionImportSerializer( + data=ac['object'], + context={'sender': sender}) + + if not serializer.is_valid(): + logger.error( + 'Cannot import audio collection: %s', serializer.errors) + return + + serializer.save() + class TestActor(SystemActor): id = 'test' diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index be24a5360..7a5e0d31b 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -10,6 +10,7 @@ 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 def test_actor_fetching(r_mock): @@ -368,3 +369,59 @@ def test_library_actor_handles_follow_auto_approval( accept_follow.assert_called_once_with( library_actor, data, actor ) + + +def test_library_actor_handle_create_audio_not_following(mocker, factories): + # when we receive inbox create audio, we should not do anything + # if we're not actually following the sender + mocked_create = mocker.patch( + 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + ) + actor = factories['federation.Actor']() + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + data = { + 'actor': 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=actor) + + mocked_create.assert_not_called() + music_models.ImportBatch.objects.count() == 0 + + +def test_library_actor_handle_create_audio(mocker, factories): + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + follow = factories['federation.Follow'](actor=library_actor) + + data = { + 'actor': follow.target.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=follow.target) + + batch = follow.target.import_batches.latest('id') + + assert batch.federation_source == data['object']['id'] + assert batch.federation_actor == follow.target + assert batch.jobs.count() == 2 + + jobs = list(batch.jobs.order_by('id')) + for i, a in enumerate(data['object']['items']): + job = jobs[i] + assert job.federation_source == a['id'] + assert job.source == a['url']['href'] From 8db832f03bb8b3430d19b03ecb97e488d17d92b9 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 15:20:53 +0200 Subject: [PATCH 21/30] Now store source AP track file on track_file --- .../migrations/0003_auto_20180403_1921.py | 31 ------ .../migrations/0003_auto_20180406_1319.py | 48 ++++++++ .../migrations/0004_followrequest.py | 28 ----- .../migrations/0005_actor_followers.py | 18 --- api/funkwhale_api/federation/models.py | 8 +- .../migrations/0023_auto_20180405_1830.py | 47 -------- .../migrations/0023_auto_20180406_1319.py | 104 ++++++++++++++++++ .../migrations/0024_auto_20180406_1115.py | 54 --------- api/funkwhale_api/music/models.py | 6 + api/funkwhale_api/music/tasks.py | 1 + api/tests/music/test_import.py | 4 + 11 files changed, 167 insertions(+), 182 deletions(-) delete mode 100644 api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py create mode 100644 api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py delete mode 100644 api/funkwhale_api/federation/migrations/0004_followrequest.py delete mode 100644 api/funkwhale_api/federation/migrations/0005_actor_followers.py delete mode 100644 api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py create mode 100644 api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py delete mode 100644 api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py b/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py deleted file mode 100644 index aadf3257e..000000000 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180403_1921.py +++ /dev/null @@ -1,31 +0,0 @@ -# Generated by Django 2.0.3 on 2018-04-03 19:21 - -from django.db import migrations, models -import django.db.models.deletion -import django.utils.timezone -import uuid - - -class Migration(migrations.Migration): - - dependencies = [ - ('federation', '0002_auto_20180403_1620'), - ] - - operations = [ - migrations.CreateModel( - name='Follow', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), - ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), - ('last_modification_date', models.DateTimeField(default=django.utils.timezone.now)), - ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emitted_follows', to='federation.Actor')), - ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follows', to='federation.Actor')), - ], - ), - migrations.AlterUniqueTogether( - name='follow', - unique_together={('actor', 'target')}, - ), - ] diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py b/api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py new file mode 100644 index 000000000..cc653b1aa --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py @@ -0,0 +1,48 @@ +# Generated by Django 2.0.3 on 2018-04-06 13:19 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0002_auto_20180403_1620'), + ] + + operations = [ + migrations.CreateModel( + name='Follow', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emitted_follows', to='federation.Actor')), + ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follows', to='federation.Actor')), + ], + ), + migrations.CreateModel( + name='FollowRequest', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('approved', models.NullBooleanField(default=None)), + ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emmited_follow_requests', to='federation.Actor')), + ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follow_requests', to='federation.Actor')), + ], + ), + migrations.AddField( + model_name='actor', + name='followers', + field=models.ManyToManyField(related_name='following', through='federation.Follow', to='federation.Actor'), + ), + migrations.AlterUniqueTogether( + name='follow', + unique_together={('actor', 'target')}, + ), + ] diff --git a/api/funkwhale_api/federation/migrations/0004_followrequest.py b/api/funkwhale_api/federation/migrations/0004_followrequest.py deleted file mode 100644 index 6ede72747..000000000 --- a/api/funkwhale_api/federation/migrations/0004_followrequest.py +++ /dev/null @@ -1,28 +0,0 @@ -# Generated by Django 2.0.3 on 2018-04-04 17:11 - -from django.db import migrations, models -import django.db.models.deletion -import django.utils.timezone -import uuid - - -class Migration(migrations.Migration): - - dependencies = [ - ('federation', '0003_auto_20180403_1921'), - ] - - operations = [ - migrations.CreateModel( - name='FollowRequest', - fields=[ - ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), - ('uuid', models.UUIDField(default=uuid.uuid4, unique=True)), - ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), - ('last_modification_date', models.DateTimeField(default=django.utils.timezone.now)), - ('approved', models.NullBooleanField(default=None)), - ('actor', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='emmited_follow_requests', to='federation.Actor')), - ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follow_requests', to='federation.Actor')), - ], - ), - ] diff --git a/api/funkwhale_api/federation/migrations/0005_actor_followers.py b/api/funkwhale_api/federation/migrations/0005_actor_followers.py deleted file mode 100644 index 94a1c75ac..000000000 --- a/api/funkwhale_api/federation/migrations/0005_actor_followers.py +++ /dev/null @@ -1,18 +0,0 @@ -# Generated by Django 2.0.3 on 2018-04-05 16:35 - -from django.db import migrations, models - - -class Migration(migrations.Migration): - - dependencies = [ - ('federation', '0004_followrequest'), - ] - - operations = [ - migrations.AddField( - model_name='actor', - name='followers', - field=models.ManyToManyField(related_name='following', through='federation.Follow', to='federation.Actor'), - ), - ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 833b5d8f3..4bf597001 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -106,8 +106,8 @@ class Follow(models.Model): on_delete=models.CASCADE, ) creation_date = models.DateTimeField(default=timezone.now) - last_modification_date = models.DateTimeField( - default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) class Meta: unique_together = ['actor', 'target'] @@ -129,8 +129,8 @@ class FollowRequest(models.Model): on_delete=models.CASCADE, ) creation_date = models.DateTimeField(default=timezone.now) - last_modification_date = models.DateTimeField( - default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) approved = models.NullBooleanField(default=None) def approve(self): diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py b/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py deleted file mode 100644 index 3cef1f42e..000000000 --- a/api/funkwhale_api/music/migrations/0023_auto_20180405_1830.py +++ /dev/null @@ -1,47 +0,0 @@ -# Generated by Django 2.0.3 on 2018-04-05 18:30 - -from django.conf import settings -import django.contrib.postgres.fields.jsonb -from django.db import migrations, models -import django.db.models.deletion - - -class Migration(migrations.Migration): - - dependencies = [ - ('federation', '0005_actor_followers'), - ('music', '0022_importbatch_import_request'), - ] - - operations = [ - migrations.AddField( - model_name='importbatch', - name='federation_actor', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Actor'), - ), - migrations.AddField( - model_name='importbatch', - name='federation_source', - field=models.URLField(blank=True, null=True), - ), - migrations.AddField( - model_name='importjob', - name='federation_source', - field=models.URLField(blank=True, null=True), - ), - migrations.AddField( - model_name='importjob', - name='metadata', - field=django.contrib.postgres.fields.jsonb.JSONField(default={}), - ), - migrations.AlterField( - model_name='importbatch', - name='source', - field=models.CharField(choices=[('api', 'api'), ('shell', 'shell'), ('federation', 'federation')], default='api', max_length=30), - ), - migrations.AlterField( - model_name='importbatch', - name='submitted_by', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='imports', to=settings.AUTH_USER_MODEL), - ), - ] diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py b/api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py new file mode 100644 index 000000000..c51a7b9fa --- /dev/null +++ b/api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py @@ -0,0 +1,104 @@ +# Generated by Django 2.0.3 on 2018-04-06 13:19 + +from django.conf import settings +import django.contrib.postgres.fields.jsonb +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import uuid + + +class Migration(migrations.Migration): + + dependencies = [ + ('federation', '0003_auto_20180406_1319'), + ('music', '0022_importbatch_import_request'), + ] + + operations = [ + migrations.AddField( + model_name='album', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='artist', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='importbatch', + name='federation_actor', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Actor'), + ), + migrations.AddField( + model_name='importbatch', + name='federation_source', + field=models.URLField(blank=True, null=True), + ), + migrations.AddField( + model_name='importbatch', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='importjob', + name='federation_source', + field=models.URLField(blank=True, null=True), + ), + migrations.AddField( + model_name='importjob', + name='metadata', + field=django.contrib.postgres.fields.jsonb.JSONField(default={}), + ), + migrations.AddField( + model_name='importjob', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='lyrics', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='track', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='trackfile', + name='creation_date', + field=models.DateTimeField(default=django.utils.timezone.now), + ), + migrations.AddField( + model_name='trackfile', + name='federation_source', + field=models.URLField(blank=True, null=True), + ), + migrations.AddField( + model_name='trackfile', + name='modification_date', + field=models.DateTimeField(auto_now=True), + ), + migrations.AddField( + model_name='trackfile', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AddField( + model_name='work', + name='uuid', + field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), + ), + migrations.AlterField( + model_name='importbatch', + name='source', + field=models.CharField(choices=[('api', 'api'), ('shell', 'shell'), ('federation', 'federation')], default='api', max_length=30), + ), + migrations.AlterField( + model_name='importbatch', + name='submitted_by', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='imports', to=settings.AUTH_USER_MODEL), + ), + ] diff --git a/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py b/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py deleted file mode 100644 index 8b655a642..000000000 --- a/api/funkwhale_api/music/migrations/0024_auto_20180406_1115.py +++ /dev/null @@ -1,54 +0,0 @@ -# Generated by Django 2.0.3 on 2018-04-06 11:15 - -from django.db import migrations, models -import uuid - - -class Migration(migrations.Migration): - - dependencies = [ - ('music', '0023_auto_20180405_1830'), - ] - - operations = [ - migrations.AddField( - model_name='album', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='artist', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='importbatch', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='importjob', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='lyrics', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='track', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='trackfile', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - migrations.AddField( - model_name='work', - name='uuid', - field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), - ), - ] diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 5d8035f35..efffb12d9 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -414,6 +414,12 @@ class TrackFile(models.Model): 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) + creation_date = models.DateTimeField(default=timezone.now) + modification_date = models.DateTimeField(auto_now=True) + + # points to the URL of the original trackfile ActivityPub Object + federation_source = models.URLField(null=True, blank=True) + duration = models.IntegerField(null=True, blank=True) acoustid_track_id = models.UUIDField(null=True, blank=True) mimetype = models.CharField(null=True, blank=True, max_length=200) diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index e4214d990..4f85613eb 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -102,6 +102,7 @@ def _do_import(import_job, replace, use_acoustid=True): track_file = track_file or models.TrackFile( track=track, source=import_job.source) track_file.acoustid_track_id = acoustid_track_id + track_file.federation_source = import_job.federation_source if from_file: track_file.audio_file = ContentFile(import_job.audio_file.read()) track_file.audio_file.name = import_job.audio_file.name diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 87e1899d6..98174891f 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -62,6 +62,7 @@ def test_import_job_from_federation_no_musicbrainz(factories): tf = job.track_file assert tf.source == job.source + assert tf.federation_source == job.federation_source assert tf.track.title == 'Ping' assert tf.track.artist.name == 'Hello' assert tf.track.album.title == 'World' @@ -84,6 +85,7 @@ def test_import_job_from_federation_musicbrainz_recording(factories, mocker): tf = job.track_file assert tf.source == job.source + assert tf.federation_source == job.federation_source assert tf.track == t track_from_api.assert_called_once_with( mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) @@ -105,6 +107,7 @@ def test_import_job_from_federation_musicbrainz_release(factories, mocker): job.refresh_from_db() tf = job.track_file + assert tf.federation_source == job.federation_source assert tf.source == job.source assert tf.track.title == 'Ping' assert tf.track.artist == a.artist @@ -131,6 +134,7 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): tf = job.track_file assert tf.source == job.source + assert tf.federation_source == job.federation_source assert tf.track.title == 'Ping' assert tf.track.artist == a assert tf.track.album.artist == a From b75872866c20f198663c73f7896f2debbe7936ac Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 17:57:50 +0200 Subject: [PATCH 22/30] Util function to manipulate url params --- api/funkwhale_api/common/utils.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/api/funkwhale_api/common/utils.py b/api/funkwhale_api/common/utils.py index c9d450e6a..2d7641bf5 100644 --- a/api/funkwhale_api/common/utils.py +++ b/api/funkwhale_api/common/utils.py @@ -1,3 +1,4 @@ +from urllib.parse import urlencode, parse_qs, urlsplit, urlunsplit import os import shutil @@ -25,3 +26,20 @@ def on_commit(f, *args, **kwargs): return transaction.on_commit( lambda: f(*args, **kwargs) ) + + +def set_query_parameter(url, **kwargs): + """Given a URL, set or replace a query parameter and return the + modified URL. + + >>> set_query_parameter('http://example.com?foo=bar&biz=baz', 'foo', 'stuff') + 'http://example.com?foo=stuff&biz=baz' + """ + scheme, netloc, path, query_string, fragment = urlsplit(url) + query_params = parse_qs(query_string) + + for param_name, param_value in kwargs.items(): + query_params[param_name] = [param_value] + new_query_string = urlencode(query_params, doseq=True) + + return urlunsplit((scheme, netloc, path, new_query_string, fragment)) From 4ce9f9bf08a59397645943df9fd8744b2c1ae5ce Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 17:58:16 +0200 Subject: [PATCH 23/30] Dedicated permission to access library data via activity pub --- api/funkwhale_api/federation/permissions.py | 19 +++++++++ api/tests/federation/test_permissions.py | 45 +++++++++++++++++++++ 2 files changed, 64 insertions(+) create mode 100644 api/funkwhale_api/federation/permissions.py create mode 100644 api/tests/federation/test_permissions.py diff --git a/api/funkwhale_api/federation/permissions.py b/api/funkwhale_api/federation/permissions.py new file mode 100644 index 000000000..370328eaa --- /dev/null +++ b/api/funkwhale_api/federation/permissions.py @@ -0,0 +1,19 @@ +from django.conf import settings + +from rest_framework.permissions import BasePermission + +from . import actors + + +class LibraryFollower(BasePermission): + + def has_permission(self, request, view): + if not settings.FEDERATION_MUSIC_NEEDS_APPROVAL: + return True + + actor = getattr(request, 'actor', None) + if actor is None: + return False + + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + return library.followers.filter(url=actor.url).exists() diff --git a/api/tests/federation/test_permissions.py b/api/tests/federation/test_permissions.py new file mode 100644 index 000000000..1a6977542 --- /dev/null +++ b/api/tests/federation/test_permissions.py @@ -0,0 +1,45 @@ +from rest_framework.views import APIView + +from funkwhale_api.federation import actors +from funkwhale_api.federation import permissions + + +def test_library_follower( + factories, api_request, anonymous_user, settings): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + view = APIView.as_view() + permission = permissions.LibraryFollower() + request = api_request.get('/') + setattr(request, 'user', anonymous_user) + check = permission.has_permission(request, view) + + assert check is False + + +def test_library_follower_actor_non_follower( + factories, api_request, anonymous_user, settings): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + actor = factories['federation.Actor']() + view = APIView.as_view() + permission = permissions.LibraryFollower() + request = api_request.get('/') + setattr(request, 'user', anonymous_user) + setattr(request, 'actor', actor) + check = permission.has_permission(request, view) + + assert check is False + + +def test_library_follower_actor_follower( + factories, api_request, anonymous_user, settings): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + follow = factories['federation.Follow'](target=library) + view = APIView.as_view() + permission = permissions.LibraryFollower() + request = api_request.get('/') + setattr(request, 'user', anonymous_user) + setattr(request, 'actor', follow.actor) + check = permission.has_permission(request, view) + + assert check is True From 393110a7f04b920bfdb882600ddfaae1298116b5 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 17:58:43 +0200 Subject: [PATCH 24/30] Serializers for paginated collections --- api/config/settings/common.py | 3 + api/funkwhale_api/federation/serializers.py | 66 +++++++++++++++++++ api/tests/federation/test_serializers.py | 72 +++++++++++++++++++++ 3 files changed, 141 insertions(+) diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 6a85a934c..e45f6c256 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -30,6 +30,9 @@ FUNKWHALE_HOSTNAME = urlsplit(FUNKWHALE_URL).netloc FEDERATION_ENABLED = env.bool('FEDERATION_ENABLED', default=True) FEDERATION_HOSTNAME = env('FEDERATION_HOSTNAME', default=FUNKWHALE_HOSTNAME) +FEDERATION_COLLECTION_PAGE_SIZE = env.int( + 'FEDERATION_COLLECTION_PAGE_SIZE', default=50 +) FEDERATION_MUSIC_NEEDS_APPROVAL = env.bool( 'FEDERATION_MUSIC_NEEDS_APPROVAL', default=True ) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 075e253da..05e9c7c8e 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -2,10 +2,13 @@ import urllib.parse from django.urls import reverse from django.conf import settings +from django.core.paginator import Paginator from rest_framework import serializers from dynamic_preferences.registries import global_preferences_registry +from funkwhale_api.common.utils import set_query_parameter + from . import activity from . import models from . import utils @@ -199,3 +202,66 @@ OBJECT_SERIALIZERS = { t: ObjectSerializer for t in activity.OBJECT_TYPES } + + +class PaginatedCollectionSerializer(serializers.Serializer): + + def to_representation(self, conf): + paginator = Paginator( + conf['items'], + conf.get('page_size', 20) + ) + first = set_query_parameter(conf['id'], page=1) + current = first + last = set_query_parameter(conf['id'], page=paginator.num_pages) + d = { + 'id': conf['id'], + 'actor': conf['actor'].url, + 'totalItems': paginator.count, + 'type': 'Collection', + 'current': current, + 'first': first, + 'last': last, + } + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d + + +class CollectionPageSerializer(serializers.Serializer): + + def to_representation(self, conf): + page = conf['page'] + first = set_query_parameter(conf['id'], page=1) + last = set_query_parameter(conf['id'], page=page.paginator.num_pages) + id = set_query_parameter(conf['id'], page=page.number) + d = { + 'id': id, + 'partOf': conf['id'], + 'actor': conf['actor'].url, + 'totalItems': page.paginator.count, + 'type': 'CollectionPage', + 'first': first, + 'last': last, + 'items': [ + conf['item_serializer']( + i, + context={ + 'actor': conf['actor'], + 'include_ap_context': False} + ).data + for i in page.object_list + ] + } + + if page.has_previous(): + d['prev'] = set_query_parameter( + conf['id'], page=page.previous_page_number()) + + if page.has_previous(): + d['next'] = set_query_parameter( + conf['id'], page=page.next_page_number()) + + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 6d027ec91..1e580040e 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -1,8 +1,10 @@ from django.urls import reverse +from django.core.paginator import Paginator from funkwhale_api.federation import keys from funkwhale_api.federation import models from funkwhale_api.federation import serializers +from funkwhale_api.music.serializers import AudioSerializer def test_actor_serializer_from_ap(db): @@ -163,3 +165,73 @@ def test_follow_serializer_to_ap(factories): } assert serializer.data == expected + + +def test_paginated_collection_serializer(factories): + tfs = factories['music.TrackFile'].create_batch(size=5) + actor = factories['federation.Actor'](local=True) + + conf = { + 'id': 'https://test.federation/test', + 'items': tfs, + 'item_serializer': AudioSerializer, + 'actor': actor, + 'page_size': 2, + } + expected = { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {}, + ], + 'type': 'Collection', + 'id': conf['id'], + 'actor': actor.url, + 'totalItems': len(tfs), + 'current': conf['id'] + '?page=1', + 'last': conf['id'] + '?page=3', + 'first': conf['id'] + '?page=1', + } + + serializer = serializers.PaginatedCollectionSerializer(conf) + + assert serializer.data == expected + + +def test_collection_page_serializer(factories): + tfs = factories['music.TrackFile'].create_batch(size=5) + actor = factories['federation.Actor'](local=True) + + conf = { + 'id': 'https://test.federation/test', + 'item_serializer': AudioSerializer, + 'actor': actor, + 'page': Paginator(tfs, 2).page(2), + } + expected = { + '@context': [ + 'https://www.w3.org/ns/activitystreams', + 'https://w3id.org/security/v1', + {}, + ], + 'type': 'CollectionPage', + 'id': conf['id'] + '?page=2', + 'actor': actor.url, + 'totalItems': len(tfs), + 'partOf': conf['id'], + 'prev': conf['id'] + '?page=1', + 'next': conf['id'] + '?page=3', + 'first': conf['id'] + '?page=1', + 'last': conf['id'] + '?page=3', + 'items': [ + conf['item_serializer']( + i, + context={'actor': actor, 'include_ap_context': False} + ).data + for i in conf['page'].object_list + ] + } + + serializer = serializers.CollectionPageSerializer(conf) + + assert serializer.data == expected From a03f0ffea54432b6c2c5c36c9df7c3098dee3b8c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 17:59:06 +0200 Subject: [PATCH 25/30] We now have a library browsable via activitypub --- api/funkwhale_api/federation/actors.py | 16 +++++ api/funkwhale_api/federation/urls.py | 13 +++- api/funkwhale_api/federation/views.py | 55 +++++++++++++++- api/tests/federation/test_views.py | 90 +++++++++++++++++++++++++- 4 files changed, 169 insertions(+), 5 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index fa1b56282..6f782ced4 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -50,6 +50,11 @@ class SystemActor(object): additional_attributes = {} manually_approves_followers = False + def serialize(self): + actor = self.get_actor_instance() + serializer = serializers.ActorSerializer() + return serializer.data + def get_actor_instance(self): args = self.get_instance_argument( self.id, @@ -172,6 +177,17 @@ class LibraryActor(SystemActor): 'manually_approves_followers': True } + def serialize(self): + data = super().serialize() + urls = data.setdefault('url', []) + urls.append({ + 'type': 'Link', + 'mediaType': 'application/activity+json', + 'name': 'library', + 'href': utils.full_url(reverse('federation:music:files-list')) + }) + return data + @property def manually_approves_followers(self): return settings.FEDERATION_MUSIC_NEEDS_APPROVAL diff --git a/api/funkwhale_api/federation/urls.py b/api/funkwhale_api/federation/urls.py index f2c6f4c78..e899869a4 100644 --- a/api/funkwhale_api/federation/urls.py +++ b/api/funkwhale_api/federation/urls.py @@ -1,8 +1,10 @@ -from rest_framework import routers +from django.conf.urls import include, url +from rest_framework import routers from . import views router = routers.SimpleRouter(trailing_slash=False) +music_router = routers.SimpleRouter(trailing_slash=False) router.register( r'federation/instance/actors', views.InstanceActorViewSet, @@ -12,4 +14,11 @@ router.register( views.WellKnownViewSet, 'well-known') -urlpatterns = router.urls +music_router.register( + r'federation/files', + views.MusicFilesViewSet, + 'files', +) +urlpatterns = router.urls + [ + url('music/', include((music_router.urls, 'music'), namespace='music')) +] diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 2e3feb8d0..390a371bc 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -1,16 +1,23 @@ from django import forms from django.conf import settings +from django.core import paginator from django.http import HttpResponse +from django.urls import reverse from rest_framework import viewsets from rest_framework import views from rest_framework import response from rest_framework.decorators import list_route, detail_route +from funkwhale_api.music.models import TrackFile +from funkwhale_api.music.serializers import AudioSerializer + from . import actors from . import authentication +from . import permissions from . import renderers from . import serializers +from . import utils from . import webfinger @@ -38,8 +45,8 @@ class InstanceActorViewSet(FederationMixin, viewsets.GenericViewSet): def retrieve(self, request, *args, **kwargs): system_actor = self.get_object() actor = system_actor.get_actor_instance() - serializer = serializers.ActorSerializer(actor) - return response.Response(serializer.data, status=200) + data = actor.system_conf.serialize() + return response.Response(data, status=200) @detail_route(methods=['get', 'post']) def inbox(self, request, *args, **kwargs): @@ -101,3 +108,47 @@ class WellKnownViewSet(FederationMixin, viewsets.GenericViewSet): username, hostname = clean_result actor = actors.SYSTEM_ACTORS[username].get_actor_instance() return serializers.ActorWebfingerSerializer(actor).data + + +class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): + authentication_classes = [ + authentication.SignatureAuthentication] + permission_classes = [permissions.LibraryFollower] + renderer_classes = [renderers.ActivityPubRenderer] + + 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') + if page is None: + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page_size': settings.FEDERATION_COLLECTION_PAGE_SIZE, + 'items': qs, + 'item_serializer': AudioSerializer, + 'actor': library, + } + serializer = serializers.PaginatedCollectionSerializer(conf) + data = serializer.data + else: + try: + page_number = int(page) + except: + return response.Response( + {'page': ['Invalid page number']}, status=400) + p = paginator.Paginator( + qs, settings.FEDERATION_COLLECTION_PAGE_SIZE) + try: + page = p.page(page_number) + except paginator.EmptyPage: + return response.Response(status=404) + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page': page, + 'item_serializer': AudioSerializer, + 'actor': library, + } + serializer = serializers.CollectionPageSerializer(conf) + data = serializer.data + + return response.Response(data) diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index 0d2ac882f..6f05a16f9 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -1,11 +1,13 @@ from django.urls import reverse +from django.core.paginator import Paginator import pytest from funkwhale_api.federation import actors from funkwhale_api.federation import serializers +from funkwhale_api.federation import utils from funkwhale_api.federation import webfinger - +from funkwhale_api.music.serializers import AudioSerializer @pytest.mark.parametrize('system_actor', actors.SYSTEM_ACTORS.keys()) @@ -62,3 +64,89 @@ def test_wellknown_webfinger_system( assert response.status_code == 200 assert response['Content-Type'] == 'application/jrd+json' assert response.data == serializer.data + + +def test_audio_file_list_requires_authenticated_actor( + db, settings, api_client): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = True + url = reverse('federation:music:files-list') + response = api_client.get(url) + + assert response.status_code == 403 + + +def test_audio_file_list_actor_no_page( + db, settings, api_client, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + settings.FEDERATION_COLLECTION_PAGE_SIZE = 2 + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + tfs = factories['music.TrackFile'].create_batch(size=5) + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page_size': 2, + 'items': list(reversed(tfs)), # we order by -creation_date + 'item_serializer': AudioSerializer, + 'actor': library + } + expected = serializers.PaginatedCollectionSerializer(conf).data + url = reverse('federation:music:files-list') + response = api_client.get(url) + + assert response.status_code == 200 + assert response.data == expected + + +def test_audio_file_list_actor_page( + db, settings, api_client, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + settings.FEDERATION_COLLECTION_PAGE_SIZE = 2 + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + tfs = factories['music.TrackFile'].create_batch(size=5) + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page': Paginator(list(reversed(tfs)), 2).page(2), + 'item_serializer': AudioSerializer, + 'actor': library + } + expected = serializers.CollectionPageSerializer(conf).data + url = reverse('federation:music:files-list') + response = api_client.get(url, data={'page': 2}) + + assert response.status_code == 200 + assert response.data == expected + + +def test_audio_file_list_actor_page_error( + db, settings, api_client, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + url = reverse('federation:music:files-list') + response = api_client.get(url, data={'page': 'nope'}) + + assert response.status_code == 400 + + +def test_audio_file_list_actor_page_error_too_far( + db, settings, api_client, factories): + settings.FEDERATION_MUSIC_NEEDS_APPROVAL = False + url = reverse('federation:music:files-list') + response = api_client.get(url, data={'page': 5000}) + + assert response.status_code == 404 + + +def test_library_actor_includes_library_link(db, settings, api_client): + actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + url = reverse( + 'federation:instance-actors-detail', + kwargs={'actor': 'library'}) + response = api_client.get(url) + expected_links = [ + { + 'type': 'Link', + 'name': 'library', + 'mediaType': 'application/activity+json', + 'href': utils.full_url(reverse('federation:music:files-list')) + } + ] + assert response.status_code == 200 + assert response.data['url'] == expected_links From f273faf9de56e0d265d2093eef847451c25f3d64 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Fri, 6 Apr 2018 18:49:29 +0200 Subject: [PATCH 26/30] Added Library model to have more granular federation management --- api/funkwhale_api/federation/actors.py | 12 +++-- api/funkwhale_api/federation/factories.py | 11 +++++ ...406_1319.py => 0003_auto_20180406_1621.py} | 17 ++++++- api/funkwhale_api/federation/models.py | 18 ++++++++ api/funkwhale_api/music/factories.py | 6 ++- ...406_1319.py => 0023_auto_20180406_1621.py} | 31 +++++++------ api/funkwhale_api/music/models.py | 22 +++++---- api/funkwhale_api/music/serializers.py | 6 +-- api/funkwhale_api/music/tasks.py | 12 +++-- api/tests/federation/test_actors.py | 46 +++++++++++++++---- api/tests/federation/test_models.py | 7 +++ api/tests/federation/test_views.py | 2 + api/tests/music/test_import.py | 12 +++-- api/tests/music/test_serializers.py | 10 ++-- 14 files changed, 159 insertions(+), 53 deletions(-) rename api/funkwhale_api/federation/migrations/{0003_auto_20180406_1319.py => 0003_auto_20180406_1621.py} (69%) rename api/funkwhale_api/music/migrations/{0023_auto_20180406_1319.py => 0023_auto_20180406_1621.py} (86%) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 6f782ced4..205c3486d 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -52,7 +52,7 @@ class SystemActor(object): def serialize(self): actor = self.get_actor_instance() - serializer = serializers.ActorSerializer() + serializer = serializers.ActorSerializer(actor) return serializer.data def get_actor_instance(self): @@ -196,8 +196,12 @@ class LibraryActor(SystemActor): from funkwhale_api.music.serializers import ( AudioCollectionImportSerializer) - library = self.get_actor_instance() - if not library.following.filter(url=sender.url).exists(): + try: + remote_library = models.Library.objects.get( + actor=sender, + federation_enabled=True, + ) + except models.Library.DoesNotExist: logger.info( 'Skipping import, we\'re not following %s', sender.url) return @@ -212,7 +216,7 @@ class LibraryActor(SystemActor): serializer = AudioCollectionImportSerializer( data=ac['object'], - context={'sender': sender}) + context={'library': remote_library}) if not serializer.is_valid(): logger.error( diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index 63d40aff8..fc8932396 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -122,6 +122,17 @@ class FollowRequestFactory(factory.DjangoModelFactory): model = models.FollowRequest +@registry.register +class LibraryFactory(factory.DjangoModelFactory): + actor = factory.SubFactory(ActorFactory) + url = factory.Faker('url') + federation_enabled = True + download_files = False + + class Meta: + model = models.Library + + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): type = 'Note' diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py b/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py similarity index 69% rename from api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py rename to api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py index cc653b1aa..f8771752e 100644 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1319.py +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.3 on 2018-04-06 13:19 +# Generated by Django 2.0.3 on 2018-04-06 16:21 from django.db import migrations, models import django.db.models.deletion @@ -36,6 +36,21 @@ class Migration(migrations.Migration): ('target', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='received_follow_requests', to='federation.Actor')), ], ), + migrations.CreateModel( + name='Library', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('fetched_date', models.DateTimeField(blank=True, null=True)), + ('uuid', models.UUIDField(default=uuid.uuid4)), + ('url', models.URLField()), + ('federation_enabled', models.BooleanField()), + ('download_files', models.BooleanField()), + ('files_count', models.PositiveIntegerField(blank=True, null=True)), + ('actor', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='library', to='federation.Actor')), + ], + ), migrations.AddField( model_name='actor', name='followers', diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 4bf597001..8a90bdb76 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -157,3 +157,21 @@ class FollowRequest(models.Model): def refuse(self): self.approved = False self.save(update_fields=['approved']) + + +class Library(models.Model): + creation_date = models.DateTimeField(default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) + fetched_date = models.DateTimeField(null=True, blank=True) + actor = models.OneToOneField( + Actor, + on_delete=models.CASCADE, + related_name='library') + uuid = models.UUIDField(default=uuid.uuid4) + url = models.URLField() + # use this flag to disable federation with a library + federation_enabled = models.BooleanField() + # should we mirror files locally or hotlink them? + download_files = models.BooleanField() + files_count = models.PositiveIntegerField(null=True, blank=True) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 27387ca9f..8da56b2a9 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -5,6 +5,7 @@ from funkwhale_api.factories import registry, ManyToManyFromList from funkwhale_api.federation.factories import ( AudioMetadataFactory, ActorFactory, + LibraryFactory, ) from funkwhale_api.users.factories import UserFactory @@ -68,7 +69,8 @@ class ImportBatchFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( submitted_by=None, - federation_actor=factory.SubFactory(ActorFactory), + source_library=factory.SubFactory(LibraryFactory), + source_library_url=factory.Faker('url'), source='federation', ) @@ -85,7 +87,7 @@ class ImportJobFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( batch=factory.SubFactory(ImportBatchFactory, federation=True), - federation_source=factory.Faker('url'), + source_library_url=factory.Faker('url'), metadata=factory.SubFactory(AudioMetadataFactory), ) diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py b/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py similarity index 86% rename from api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py rename to api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py index c51a7b9fa..2bc8085a0 100644 --- a/api/funkwhale_api/music/migrations/0023_auto_20180406_1319.py +++ b/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.3 on 2018-04-06 13:19 +# Generated by Django 2.0.3 on 2018-04-06 16:21 from django.conf import settings import django.contrib.postgres.fields.jsonb @@ -11,7 +11,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ - ('federation', '0003_auto_20180406_1319'), + ('federation', '0003_auto_20180406_1621'), ('music', '0022_importbatch_import_request'), ] @@ -28,12 +28,12 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='importbatch', - name='federation_actor', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Actor'), + name='source_library', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Library'), ), migrations.AddField( model_name='importbatch', - name='federation_source', + name='source_library_url', field=models.URLField(blank=True, null=True), ), migrations.AddField( @@ -43,13 +43,13 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='importjob', - name='federation_source', - field=models.URLField(blank=True, null=True), + name='metadata', + field=django.contrib.postgres.fields.jsonb.JSONField(default={}), ), migrations.AddField( model_name='importjob', - name='metadata', - field=django.contrib.postgres.fields.jsonb.JSONField(default={}), + name='source_library_url', + field=models.URLField(blank=True, null=True), ), migrations.AddField( model_name='importjob', @@ -73,13 +73,18 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='trackfile', - name='federation_source', - field=models.URLField(blank=True, null=True), + name='modification_date', + field=models.DateTimeField(auto_now=True), ), migrations.AddField( model_name='trackfile', - name='modification_date', - field=models.DateTimeField(auto_now=True), + name='source_library', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='track_files', to='federation.Library'), + ), + migrations.AddField( + model_name='trackfile', + name='source_library_url', + field=models.URLField(blank=True, null=True), ), migrations.AddField( model_name='trackfile', diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index efffb12d9..4c0b6a098 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -417,8 +417,14 @@ class TrackFile(models.Model): creation_date = models.DateTimeField(default=timezone.now) modification_date = models.DateTimeField(auto_now=True) + source_library = models.ForeignKey( + 'federation.Library', + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name='track_files') # points to the URL of the original trackfile ActivityPub Object - federation_source = models.URLField(null=True, blank=True) + source_library_url = models.URLField(null=True, blank=True) duration = models.IntegerField(null=True, blank=True) acoustid_track_id = models.UUIDField(null=True, blank=True) @@ -470,6 +476,7 @@ IMPORT_STATUS_CHOICES = ( ('skipped', 'Skipped'), ) + class ImportBatch(models.Model): uuid = models.UUIDField( unique=True, db_index=True, default=uuid.uuid4) @@ -496,14 +503,13 @@ class ImportBatch(models.Model): blank=True, on_delete=models.CASCADE) - federation_source = models.URLField(null=True, blank=True) - federation_actor = models.ForeignKey( - 'federation.Actor', - on_delete=models.SET_NULL, + source_library = models.ForeignKey( + 'federation.Library', null=True, blank=True, - related_name='import_batches', - ) + on_delete=models.SET_NULL, + related_name='import_batches') + source_library_url = models.URLField(null=True, blank=True) class Meta: ordering = ['-creation_date'] @@ -534,7 +540,7 @@ class ImportJob(models.Model): choices=IMPORT_STATUS_CHOICES, default='pending', max_length=30) audio_file = models.FileField( upload_to='imports/%Y/%m/%d', max_length=255, null=True, blank=True) - federation_source = models.URLField(null=True, blank=True) + source_library_url = models.URLField(null=True, blank=True) metadata = JSONField(default={}) class Meta: diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 5cd2f2cc2..78b2f7c04 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -210,7 +210,7 @@ class AudioSerializer(serializers.Serializer): return models.ImportJob.objects.create( batch=batch, source=validated_data['url']['href'], - federation_source=validated_data['id'], + source_library_url=validated_data['id'], metadata=metadata, ) @@ -248,8 +248,8 @@ class AudioCollectionImportSerializer(serializers.Serializer): @transaction.atomic def create(self, validated_data): batch = models.ImportBatch.objects.create( - federation_actor=self.context['sender'], - federation_source=validated_data['id'], + source_library=self.context['library'], + source_library_url=validated_data['id'], source='federation', ) for i in validated_data['items']: diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 4f85613eb..67b538c1e 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -102,14 +102,18 @@ def _do_import(import_job, replace, use_acoustid=True): track_file = track_file or models.TrackFile( track=track, source=import_job.source) track_file.acoustid_track_id = acoustid_track_id - track_file.federation_source = import_job.federation_source + track_file.source_library = import_job.batch.source_library + track_file.source_library_url = import_job.source_library_url 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 - elif import_job.federation_source: - # no downloading, we hotlink - pass + elif import_job.source_library_url: + if track_file.source_library.download_files: + raise NotImplementedError() + else: + # no downloading, we hotlink + pass else: track_file.download_file() track_file.save() diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 7a5e0d31b..6f5e88663 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -371,9 +371,9 @@ def test_library_actor_handles_follow_auto_approval( ) -def test_library_actor_handle_create_audio_not_following(mocker, factories): +def test_library_actor_handle_create_audio_no_library(mocker, factories): # when we receive inbox create audio, we should not do anything - # if we're not actually following the sender + # if we don't have a configured library matching the sender mocked_create = mocker.patch( 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' ) @@ -396,12 +396,40 @@ def test_library_actor_handle_create_audio_not_following(mocker, factories): music_models.ImportBatch.objects.count() == 0 +def test_library_actor_handle_create_audio_no_library_enabled( + mocker, factories): + # when we receive inbox create audio, we should not do anything + # if we don't have an enabled library + mocked_create = mocker.patch( + 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + ) + disabled_library = factories['federation.Library']( + federation_enabled=False) + actor = disabled_library.actor + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + data = { + 'actor': 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=actor) + + mocked_create.assert_not_called() + music_models.ImportBatch.objects.count() == 0 + + def test_library_actor_handle_create_audio(mocker, factories): library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() - follow = factories['federation.Follow'](actor=library_actor) + remote_library = factories['federation.Library']() data = { - 'actor': follow.target.url, + 'actor': remote_library.actor.url, 'type': 'Create', 'id': 'http://test.federation/audio/create', 'object': { @@ -412,16 +440,16 @@ def test_library_actor_handle_create_audio(mocker, factories): }, } - library_actor.system_conf.post_inbox(data, actor=follow.target) + library_actor.system_conf.post_inbox(data, actor=remote_library.actor) - batch = follow.target.import_batches.latest('id') + batch = remote_library.import_batches.latest('id') - assert batch.federation_source == data['object']['id'] - assert batch.federation_actor == follow.target + assert batch.source_library_url == data['object']['id'] + assert batch.source_library == remote_library assert batch.jobs.count() == 2 jobs = list(batch.jobs.order_by('id')) for i, a in enumerate(data['object']['items']): job = jobs[i] - assert job.federation_source == a['id'] + assert job.source_library_url == a['id'] assert job.source == a['url']['href'] diff --git a/api/tests/federation/test_models.py b/api/tests/federation/test_models.py index 86e4f4a84..b17b6eb65 100644 --- a/api/tests/federation/test_models.py +++ b/api/tests/federation/test_models.py @@ -26,6 +26,7 @@ def test_cannot_duplicate_follow(factories): actor=follow.actor, ) + def test_follow_federation_url(factories): follow = factories['federation.Follow'](local=True) expected = '{}#follows/{}'.format( @@ -76,3 +77,9 @@ def test_follow_request_refused(mocker, factories): assert fr.approved is False assert fr.target.followers.count() == 0 + + +def test_library_model_unique_per_actor(factories): + library = factories['federation.Library']() + with pytest.raises(db.IntegrityError): + factories['federation.Library'](actor=library.actor) diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index 6f05a16f9..c7e1fc012 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -19,6 +19,8 @@ def test_instance_actors(system_actor, db, settings, api_client): response = api_client.get(url) serializer = serializers.ActorSerializer(actor) + if system_actor == 'library': + response.data.pop('url') assert response.status_code == 200 assert response.data == serializer.data diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 98174891f..7e3ebcc19 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -62,7 +62,8 @@ def test_import_job_from_federation_no_musicbrainz(factories): tf = job.track_file assert tf.source == job.source - assert tf.federation_source == job.federation_source + assert tf.source_library == job.batch.source_library + assert tf.source_library_url == job.source_library_url assert tf.track.title == 'Ping' assert tf.track.artist.name == 'Hello' assert tf.track.album.title == 'World' @@ -85,7 +86,8 @@ def test_import_job_from_federation_musicbrainz_recording(factories, mocker): tf = job.track_file assert tf.source == job.source - assert tf.federation_source == job.federation_source + assert tf.source_library == job.batch.source_library + assert tf.source_library_url == job.source_library_url assert tf.track == t track_from_api.assert_called_once_with( mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) @@ -107,7 +109,8 @@ def test_import_job_from_federation_musicbrainz_release(factories, mocker): job.refresh_from_db() tf = job.track_file - assert tf.federation_source == job.federation_source + assert tf.source_library == job.batch.source_library + assert tf.source_library_url == job.source_library_url assert tf.source == job.source assert tf.track.title == 'Ping' assert tf.track.artist == a.artist @@ -134,7 +137,8 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): tf = job.track_file assert tf.source == job.source - assert tf.federation_source == job.federation_source + assert tf.source_library == job.batch.source_library + assert tf.source_library_url == job.source_library_url assert tf.track.title == 'Ping' assert tf.track.artist == a assert tf.track.album.artist == a diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py index 1270ae765..a1f5999e7 100644 --- a/api/tests/music/test_serializers.py +++ b/api/tests/music/test_serializers.py @@ -5,7 +5,7 @@ from funkwhale_api.music import serializers def test_activity_pub_audio_collection_serializer_to_import(factories): - sender = factories['federation.Actor']() + remote_library = factories['federation.Library']() collection = { 'id': 'https://batch.import', @@ -15,7 +15,7 @@ def test_activity_pub_audio_collection_serializer_to_import(factories): } serializer = serializers.AudioCollectionImportSerializer( - data=collection, context={'sender': sender}) + data=collection, context={'library': remote_library}) assert serializer.is_valid(raise_exception=True) @@ -23,13 +23,13 @@ def test_activity_pub_audio_collection_serializer_to_import(factories): jobs = list(batch.jobs.all()) assert batch.source == 'federation' - assert batch.federation_source == collection['id'] - assert batch.federation_actor == sender + assert batch.source_library_url == collection['id'] + assert batch.source_library == remote_library assert len(jobs) == 2 for i, a in enumerate(collection['items']): job = jobs[i] - assert job.federation_source == a['id'] + assert job.source_library_url == a['id'] assert job.source == a['url']['href'] a['metadata']['mediaType'] = a['url']['mediaType'] assert job.metadata == a['metadata'] From b29ca4479729153201dbbaff755c84523330db74 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 7 Apr 2018 11:29:40 +0200 Subject: [PATCH 27/30] Now store remote library tracks in a dedicated model, this is much simpler --- api/funkwhale_api/federation/actors.py | 35 +++-- api/funkwhale_api/federation/factories.py | 64 +++++++- ...406_1621.py => 0003_auto_20180407_0852.py} | 26 +++- api/funkwhale_api/federation/models.py | 30 +++- api/funkwhale_api/federation/serializers.py | 147 ++++++++++++++++++ api/funkwhale_api/federation/urls.py | 4 +- api/funkwhale_api/federation/views.py | 24 +-- api/funkwhale_api/music/factories.py | 10 +- ...406_1621.py => 0023_auto_20180407_0852.py} | 34 +--- api/funkwhale_api/music/models.py | 29 +--- api/funkwhale_api/music/serializers.py | 124 --------------- api/funkwhale_api/music/tasks.py | 68 ++++---- api/tests/federation/test_actors.py | 35 +++-- api/tests/federation/test_serializers.py | 146 ++++++++++++++++- api/tests/federation/test_views.py | 5 +- api/tests/music/test_import.py | 74 +++++---- api/tests/music/test_serializers.py | 94 ----------- 17 files changed, 555 insertions(+), 394 deletions(-) rename api/funkwhale_api/federation/migrations/{0003_auto_20180406_1621.py => 0003_auto_20180407_0852.py} (65%) rename api/funkwhale_api/music/migrations/{0023_auto_20180406_1621.py => 0023_auto_20180407_0852.py} (68%) delete mode 100644 api/tests/music/test_serializers.py diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index 205c3486d..ffbafd8b7 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -4,6 +4,7 @@ import uuid import xml from django.conf import settings +from django.db import transaction from django.urls import reverse from django.utils import timezone @@ -192,10 +193,8 @@ class LibraryActor(SystemActor): def manually_approves_followers(self): return settings.FEDERATION_MUSIC_NEEDS_APPROVAL + @transaction.atomic def handle_create(self, ac, sender): - from funkwhale_api.music.serializers import ( - AudioCollectionImportSerializer) - try: remote_library = models.Library.objects.get( actor=sender, @@ -212,18 +211,28 @@ class LibraryActor(SystemActor): if ac['object']['totalItems'] <= 0: return - items = ac['object']['items'] - - serializer = AudioCollectionImportSerializer( - data=ac['object'], - context={'library': remote_library}) - - if not serializer.is_valid(): - logger.error( - 'Cannot import audio collection: %s', serializer.errors) + try: + items = ac['object']['items'] + except KeyError: + logger.warning('No items in collection!') return - serializer.save() + item_serializers = [ + serializers.AudioSerializer( + data=i, context={'library': remote_library}) + for i in items + ] + + valid_serializers = [] + for s in item_serializers: + if s.is_valid(): + valid_serializers.append(s) + else: + logger.debug( + 'Skipping invalid item %s, %s', s.initial_data, s.errors) + + for s in valid_serializers: + s.save() class TestActor(SystemActor): diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index fc8932396..b3ac72039 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -128,11 +128,73 @@ class LibraryFactory(factory.DjangoModelFactory): url = factory.Faker('url') federation_enabled = True download_files = False + autoimport = False class Meta: model = models.Library +class ArtistMetadataFactory(factory.Factory): + name = factory.Faker('name') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +class ReleaseMetadataFactory(factory.Factory): + title = factory.Faker('sentence') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +class RecordingMetadataFactory(factory.Factory): + title = factory.Faker('sentence') + + class Meta: + model = dict + + class Params: + musicbrainz = factory.Trait( + musicbrainz_id=factory.Faker('uuid4') + ) + + +@registry.register(name='federation.LibraryTrackMetadata') +class LibraryTrackMetadataFactory(factory.Factory): + artist = factory.SubFactory(ArtistMetadataFactory) + recording = factory.SubFactory(RecordingMetadataFactory) + release = factory.SubFactory(ReleaseMetadataFactory) + + class Meta: + model = dict + + +@registry.register +class LibraryTrackFactory(factory.DjangoModelFactory): + library = factory.SubFactory(LibraryFactory) + url = factory.Faker('url') + title = factory.Faker('sentence') + artist_name = factory.Faker('sentence') + album_title = factory.Faker('sentence') + audio_url = factory.Faker('url') + audio_mimetype = 'audio/ogg' + metadata = factory.SubFactory(LibraryTrackMetadataFactory) + + class Meta: + model = models.LibraryTrack + + @registry.register(name='federation.Note') class NoteFactory(factory.Factory): type = 'Note' @@ -189,7 +251,7 @@ class AudioFactory(factory.Factory): ) actor = factory.Faker('url') url = factory.SubFactory(LinkFactory, audio=True) - metadata = factory.SubFactory(AudioMetadataFactory) + metadata = factory.SubFactory(LibraryTrackMetadataFactory) class Meta: model = dict diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py b/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py similarity index 65% rename from api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py rename to api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py index f8771752e..afc7ea083 100644 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180406_1621.py +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py @@ -1,5 +1,6 @@ -# Generated by Django 2.0.3 on 2018-04-06 16:21 +# Generated by Django 2.0.3 on 2018-04-07 08:52 +import django.contrib.postgres.fields.jsonb from django.db import migrations, models import django.db.models.deletion import django.utils.timezone @@ -9,6 +10,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ + ('music', '0022_importbatch_import_request'), ('federation', '0002_auto_20180403_1620'), ] @@ -47,10 +49,30 @@ class Migration(migrations.Migration): ('url', models.URLField()), ('federation_enabled', models.BooleanField()), ('download_files', models.BooleanField()), - ('files_count', models.PositiveIntegerField(blank=True, null=True)), + ('autoimport', models.BooleanField()), + ('tracks_count', models.PositiveIntegerField(blank=True, null=True)), ('actor', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='library', to='federation.Actor')), ], ), + migrations.CreateModel( + name='LibraryTrack', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('url', models.URLField(unique=True)), + ('audio_url', models.URLField()), + ('audio_mimetype', models.CharField(max_length=200)), + ('creation_date', models.DateTimeField(default=django.utils.timezone.now)), + ('modification_date', models.DateTimeField(auto_now=True)), + ('fetched_date', models.DateTimeField(blank=True, null=True)), + ('published_date', models.DateTimeField(blank=True, null=True)), + ('artist_name', models.CharField(max_length=500)), + ('album_title', models.CharField(max_length=500)), + ('title', models.CharField(max_length=500)), + ('metadata', django.contrib.postgres.fields.jsonb.JSONField(default={}, max_length=10000)), + ('library', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tracks', to='federation.Library')), + ('local_track_file', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='library_track', to='music.TrackFile')), + ], + ), migrations.AddField( model_name='actor', name='followers', diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 8a90bdb76..91f2ea973 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -1,6 +1,7 @@ import uuid from django.conf import settings +from django.contrib.postgres.fields import JSONField from django.db import models from django.utils import timezone @@ -170,8 +171,35 @@ class Library(models.Model): related_name='library') uuid = models.UUIDField(default=uuid.uuid4) url = models.URLField() + # use this flag to disable federation with a library federation_enabled = models.BooleanField() # should we mirror files locally or hotlink them? download_files = models.BooleanField() - files_count = models.PositiveIntegerField(null=True, blank=True) + # should we automatically import new files from this library? + autoimport = models.BooleanField() + tracks_count = models.PositiveIntegerField(null=True, blank=True) + + +class LibraryTrack(models.Model): + url = models.URLField(unique=True) + audio_url = models.URLField() + audio_mimetype = models.CharField(max_length=200) + creation_date = models.DateTimeField(default=timezone.now) + modification_date = models.DateTimeField( + auto_now=True) + fetched_date = models.DateTimeField(null=True, blank=True) + published_date = models.DateTimeField(null=True, blank=True) + library = models.ForeignKey( + Library, related_name='tracks', on_delete=models.CASCADE) + local_track_file = models.OneToOneField( + 'music.TrackFile', + related_name='library_track', + on_delete=models.CASCADE, + null=True, + blank=True, + ) + artist_name = models.CharField(max_length=500) + album_title = models.CharField(max_length=500) + title = models.CharField(max_length=500) + metadata = JSONField(default={}, max_length=10000) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 05e9c7c8e..17541c50f 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -3,6 +3,7 @@ import urllib.parse from django.urls import reverse from django.conf import settings from django.core.paginator import Paginator +from django.db import transaction from rest_framework import serializers from dynamic_preferences.registries import global_preferences_registry @@ -265,3 +266,149 @@ class CollectionPageSerializer(serializers.Serializer): if self.context.get('include_ap_context', True): d['@context'] = AP_CONTEXT return d + + +class ArtistMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + name = serializers.CharField() + + +class ReleaseMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + title = serializers.CharField() + + +class RecordingMetadataSerializer(serializers.Serializer): + musicbrainz_id = serializers.UUIDField(required=False) + title = serializers.CharField() + + +class AudioMetadataSerializer(serializers.Serializer): + artist = ArtistMetadataSerializer() + release = ReleaseMetadataSerializer() + recording = RecordingMetadataSerializer() + + +class AudioSerializer(serializers.Serializer): + type = serializers.CharField() + id = serializers.URLField() + url = serializers.JSONField() + published = serializers.DateTimeField() + updated = serializers.DateTimeField(required=False) + metadata = AudioMetadataSerializer() + + def validate_type(self, v): + if v != 'Audio': + raise serializers.ValidationError('Invalid type for audio') + return v + + 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/'): + 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/'): + raise serializers.ValidationError('Invalid mediaType') + + return v + + def create(self, validated_data): + defaults = { + 'audio_mimetype': validated_data['url']['mediaType'], + 'audio_url': validated_data['url']['href'], + 'metadata': validated_data['metadata'], + 'artist_name': validated_data['metadata']['artist']['name'], + 'album_title': validated_data['metadata']['release']['title'], + 'title': validated_data['metadata']['recording']['title'], + 'published_date': validated_data['published'], + 'modification_date': validated_data.get('updated'), + } + return models.LibraryTrack.objects.get_or_create( + library=self.context['library'], + url=validated_data['id'], + defaults=defaults + )[0] + + def to_representation(self, instance): + track = instance.track + album = instance.track.album + artist = instance.track.artist + + d = { + 'type': 'Audio', + 'id': instance.get_federation_url(), + 'name': instance.track.full_name, + 'published': instance.creation_date.isoformat(), + 'updated': instance.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'musicbrainz_id': str(artist.mbid) if artist.mbid else None, + 'name': artist.name, + }, + 'release': { + 'musicbrainz_id': str(album.mbid) if album.mbid else None, + 'title': album.title, + }, + 'recording': { + 'musicbrainz_id': str(track.mbid) if track.mbid else None, + 'title': track.title, + }, + }, + 'url': { + 'href': utils.full_url(instance.path), + 'type': 'Link', + 'mediaType': instance.mimetype + }, + 'attributedTo': [ + self.context['actor'].url + ] + } + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d + + +class CollectionSerializer(serializers.Serializer): + + def to_representation(self, conf): + d = { + 'id': conf['id'], + 'actor': conf['actor'].url, + 'totalItems': len(conf['items']), + 'type': 'Collection', + 'items': [ + conf['item_serializer']( + i, + context={ + 'actor': conf['actor'], + 'include_ap_context': False} + ).data + for i in conf['items'] + ] + } + + if self.context.get('include_ap_context', True): + d['@context'] = AP_CONTEXT + return d diff --git a/api/funkwhale_api/federation/urls.py b/api/funkwhale_api/federation/urls.py index e899869a4..2c24b5257 100644 --- a/api/funkwhale_api/federation/urls.py +++ b/api/funkwhale_api/federation/urls.py @@ -15,10 +15,10 @@ router.register( 'well-known') music_router.register( - r'federation/files', + r'files', views.MusicFilesViewSet, 'files', ) urlpatterns = router.urls + [ - url('music/', include((music_router.urls, 'music'), namespace='music')) + url('federation/music/', include((music_router.urls, 'music'), namespace='music')) ] diff --git a/api/funkwhale_api/federation/views.py b/api/funkwhale_api/federation/views.py index 390a371bc..35d8a75a5 100644 --- a/api/funkwhale_api/federation/views.py +++ b/api/funkwhale_api/federation/views.py @@ -10,7 +10,6 @@ from rest_framework import response from rest_framework.decorators import list_route, detail_route from funkwhale_api.music.models import TrackFile -from funkwhale_api.music.serializers import AudioSerializer from . import actors from . import authentication @@ -119,13 +118,16 @@ 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') + qs = TrackFile.objects.order_by('-creation_date').select_related( + 'track__artist', + 'track__album__artist' + ) if page is None: conf = { 'id': utils.full_url(reverse('federation:music:files-list')), 'page_size': settings.FEDERATION_COLLECTION_PAGE_SIZE, 'items': qs, - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library, } serializer = serializers.PaginatedCollectionSerializer(conf) @@ -140,15 +142,15 @@ class MusicFilesViewSet(FederationMixin, viewsets.GenericViewSet): qs, settings.FEDERATION_COLLECTION_PAGE_SIZE) try: page = p.page(page_number) + conf = { + 'id': utils.full_url(reverse('federation:music:files-list')), + 'page': page, + 'item_serializer': serializers.AudioSerializer, + 'actor': library, + } + serializer = serializers.CollectionPageSerializer(conf) + data = serializer.data except paginator.EmptyPage: return response.Response(status=404) - conf = { - 'id': utils.full_url(reverse('federation:music:files-list')), - 'page': page, - 'item_serializer': AudioSerializer, - 'actor': library, - } - serializer = serializers.CollectionPageSerializer(conf) - data = serializer.data return response.Response(data) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 8da56b2a9..83aad432a 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -3,9 +3,7 @@ import os from funkwhale_api.factories import registry, ManyToManyFromList from funkwhale_api.federation.factories import ( - AudioMetadataFactory, - ActorFactory, - LibraryFactory, + LibraryTrackFactory, ) from funkwhale_api.users.factories import UserFactory @@ -69,8 +67,6 @@ class ImportBatchFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( submitted_by=None, - source_library=factory.SubFactory(LibraryFactory), - source_library_url=factory.Faker('url'), source='federation', ) @@ -86,9 +82,9 @@ class ImportJobFactory(factory.django.DjangoModelFactory): class Params: federation = factory.Trait( + mbid=None, + library_track=factory.SubFactory(LibraryTrackFactory), batch=factory.SubFactory(ImportBatchFactory, federation=True), - source_library_url=factory.Faker('url'), - metadata=factory.SubFactory(AudioMetadataFactory), ) diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py b/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py similarity index 68% rename from api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py rename to api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py index 2bc8085a0..b1bbeacef 100644 --- a/api/funkwhale_api/music/migrations/0023_auto_20180406_1621.py +++ b/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py @@ -1,7 +1,6 @@ -# Generated by Django 2.0.3 on 2018-04-06 16:21 +# Generated by Django 2.0.3 on 2018-04-07 08:52 from django.conf import settings -import django.contrib.postgres.fields.jsonb from django.db import migrations, models import django.db.models.deletion import django.utils.timezone @@ -11,7 +10,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ - ('federation', '0003_auto_20180406_1621'), + ('federation', '0003_auto_20180407_0852'), ('music', '0022_importbatch_import_request'), ] @@ -26,16 +25,6 @@ class Migration(migrations.Migration): name='uuid', field=models.UUIDField(db_index=True, default=uuid.uuid4, unique=True), ), - migrations.AddField( - model_name='importbatch', - name='source_library', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_batches', to='federation.Library'), - ), - migrations.AddField( - model_name='importbatch', - name='source_library_url', - field=models.URLField(blank=True, null=True), - ), migrations.AddField( model_name='importbatch', name='uuid', @@ -43,13 +32,8 @@ class Migration(migrations.Migration): ), migrations.AddField( model_name='importjob', - name='metadata', - field=django.contrib.postgres.fields.jsonb.JSONField(default={}), - ), - migrations.AddField( - model_name='importjob', - name='source_library_url', - field=models.URLField(blank=True, null=True), + name='library_track', + field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='import_jobs', to='federation.LibraryTrack'), ), migrations.AddField( model_name='importjob', @@ -76,16 +60,6 @@ class Migration(migrations.Migration): name='modification_date', field=models.DateTimeField(auto_now=True), ), - migrations.AddField( - model_name='trackfile', - name='source_library', - field=models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='track_files', to='federation.Library'), - ), - migrations.AddField( - model_name='trackfile', - name='source_library_url', - field=models.URLField(blank=True, null=True), - ), migrations.AddField( model_name='trackfile', name='uuid', diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index 4c0b6a098..bcf691bcd 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -9,7 +9,6 @@ import uuid from django.conf import settings from django.db import models -from django.contrib.postgres.fields import JSONField from django.core.files.base import ContentFile from django.core.files import File from django.db.models.signals import post_save @@ -416,16 +415,6 @@ class TrackFile(models.Model): source = models.URLField(null=True, blank=True) creation_date = models.DateTimeField(default=timezone.now) modification_date = models.DateTimeField(auto_now=True) - - source_library = models.ForeignKey( - 'federation.Library', - null=True, - blank=True, - on_delete=models.SET_NULL, - related_name='track_files') - # points to the URL of the original trackfile ActivityPub Object - source_library_url = models.URLField(null=True, blank=True) - duration = models.IntegerField(null=True, blank=True) acoustid_track_id = models.UUIDField(null=True, blank=True) mimetype = models.CharField(null=True, blank=True, max_length=200) @@ -503,14 +492,6 @@ class ImportBatch(models.Model): blank=True, on_delete=models.CASCADE) - source_library = models.ForeignKey( - 'federation.Library', - null=True, - blank=True, - on_delete=models.SET_NULL, - related_name='import_batches') - source_library_url = models.URLField(null=True, blank=True) - class Meta: ordering = ['-creation_date'] @@ -540,8 +521,14 @@ class ImportJob(models.Model): choices=IMPORT_STATUS_CHOICES, default='pending', max_length=30) audio_file = models.FileField( upload_to='imports/%Y/%m/%d', max_length=255, null=True, blank=True) - source_library_url = models.URLField(null=True, blank=True) - metadata = JSONField(default={}) + + library_track = models.ForeignKey( + 'federation.LibraryTrack', + related_name='import_jobs', + on_delete=models.SET_NULL, + null=True, + blank=True + ) class Meta: ordering = ('id', ) diff --git a/api/funkwhale_api/music/serializers.py b/api/funkwhale_api/music/serializers.py index 78b2f7c04..42795dbea 100644 --- a/api/funkwhale_api/music/serializers.py +++ b/api/funkwhale_api/music/serializers.py @@ -153,127 +153,3 @@ class TrackActivitySerializer(activity_serializers.ModelSerializer): def get_type(self, obj): return 'Audio' - - -class AudioMetadataSerializer(serializers.Serializer): - artist = serializers.CharField(required=False) - release = serializers.CharField(required=False) - recording = serializers.CharField(required=False) - - -class AudioSerializer(serializers.Serializer): - type = serializers.CharField() - id = serializers.URLField() - url = serializers.JSONField() - metadata = AudioMetadataSerializer() - - def validate_type(self, v): - if v != 'Audio': - raise serializers.ValidationError('Invalid type for audio') - return v - - 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/'): - 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/'): - raise serializers.ValidationError('Invalid mediaType') - - return v - - def create(self, validated_data, batch): - metadata = validated_data['metadata'].copy() - metadata['mediaType'] = validated_data['url']['mediaType'] - return models.ImportJob.objects.create( - batch=batch, - source=validated_data['url']['href'], - source_library_url=validated_data['id'], - metadata=metadata, - ) - - def to_representation(self, instance): - d = { - 'type': 'Audio', - 'id': instance.get_federation_url(), - 'name': instance.track.full_name, - 'metadata': { - 'artist': instance.track.artist.musicbrainz_url, - 'release': instance.track.album.musicbrainz_url, - 'track': instance.track.musicbrainz_url, - }, - 'url': { - 'href': federation_utils.full_url(instance.path), - 'type': 'Link', - 'mediaType': instance.mimetype - }, - 'attributedTo': [ - self.context['actor'].url - ] - } - if self.context.get('include_ap_context', True): - d['@context'] = AP_CONTEXT - return d - - -class AudioCollectionImportSerializer(serializers.Serializer): - id = serializers.URLField() - items = serializers.ListField( - child=AudioSerializer(), - min_length=1, - ) - - @transaction.atomic - def create(self, validated_data): - batch = models.ImportBatch.objects.create( - source_library=self.context['library'], - source_library_url=validated_data['id'], - source='federation', - ) - for i in validated_data['items']: - s = AudioSerializer(data=i) - job = s.create(i, batch) - return batch - - def to_representation(self, instance): - d = { - 'id': instance['id'], - 'actor': instance['actor'].url, - 'totalItems': len(instance['items']), - 'type': 'Collection', - 'items': [ - AudioSerializer( - i, - context={ - 'actor': instance['actor'], - 'include_ap_context': False - } - ).data - for i in instance['items'] - ] - } - if self.context.get('include_ap_context', True): - d['@context'] = AP_CONTEXT - return d diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index 67b538c1e..c58eb7136 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -25,42 +25,46 @@ def set_acoustid_on_track_file(track_file): return update(result['id']) -def get_mbid(url, type): - prefix = 'https://musicbrainz.org/{}/'.format(type) - if url.startswith(prefix): - return url.replace(prefix, '') - - -def import_track_from_metadata(metadata): - raw_track = metadata['recording'] - if isinstance(raw_track, str): - track_mbid = get_mbid(raw_track, 'recording') +def import_track_from_remote(library_track): + metadata = library_track.metadata + try: + track_mbid = metadata['recording']['musicbrainz_id'] + assert track_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: return models.Track.get_or_create_from_api(mbid=track_mbid) - raw_album = metadata['release'] - if isinstance(raw_album, str): - album_mbid = get_mbid(raw_album, 'release') + try: + album_mbid = metadata['release']['musicbrainz_id'] + assert album_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: album = models.Album.get_or_create_from_api(mbid=album_mbid) return models.Track.get_or_create_from_title( - raw_track['title'], artist=album.artist, album=album) + library_track.title, artist=album.artist, album=album) - raw_artist = metadata['artist'] - if isinstance(raw_artist, str): - artist_mbid = get_mbid(raw_artist, 'artist') + try: + artist_mbid = metadata['artist']['musicbrainz_id'] + assert artist_mbid # for null/empty values + except (KeyError, AssertionError): + pass + else: artist = models.Artist.get_or_create_from_api(mbid=artist_mbid) album = models.Album.get_or_create_from_title( - raw_album['title'], artist=artist) + library_track.album_title, artist=artist) return models.Track.get_or_create_from_title( - raw_track['title'], artist=artist, album=album) + library_track.title, artist=artist, album=album) # worst case scenario, we have absolutely no way to link to a # musicbrainz resource, we rely on the name/titles artist = models.Artist.get_or_create_from_name( - raw_artist['name']) + library_track.artist_name) album = models.Album.get_or_create_from_title( - raw_album['title'], artist=artist) + library_track.album_title, artist=artist) return models.Track.get_or_create_from_title( - raw_track['title'], artist=artist, album=album) + library_track.title, artist=artist, album=album) def _do_import(import_job, replace, use_acoustid=True): @@ -83,12 +87,13 @@ def _do_import(import_job, replace, use_acoustid=True): track, _ = models.Track.get_or_create_from_api(mbid=mbid) elif import_job.audio_file: 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) else: - # probably federation, we use metadata stored on the job itself - if not import_job.metadata: - raise ValueError('We cannot import without at least metadatas') + raise ValueError( + 'Not enough data to process import, ' + 'add a mbid, an audio file or a library track') - track = import_track_from_metadata(import_job.metadata) track_file = None if replace: track_file = track.files.first() @@ -102,14 +107,13 @@ def _do_import(import_job, replace, use_acoustid=True): track_file = track_file or models.TrackFile( track=track, source=import_job.source) track_file.acoustid_track_id = acoustid_track_id - track_file.source_library = import_job.batch.source_library - track_file.source_library_url = import_job.source_library_url 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 - elif import_job.source_library_url: - if track_file.source_library.download_files: + elif import_job.library_track: + track_file.mimetype = import_job.library_track.audio_mimetype + if import_job.library_track.library.download_files: raise NotImplementedError() else: # no downloading, we hotlink @@ -117,6 +121,10 @@ def _do_import(import_job, replace, use_acoustid=True): else: track_file.download_file() track_file.save() + if import_job.library_track: + import_job.library_track.local_track_file = track_file + import_job.library_track.save( + update_fields=['modification_date', 'local_track_file']) import_job.status = 'finished' import_job.track_file = track_file if import_job.audio_file: diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 6f5e88663..107047b56 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -1,3 +1,4 @@ +import arrow import pytest import uuid @@ -10,7 +11,6 @@ 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 def test_actor_fetching(r_mock): @@ -375,7 +375,7 @@ def test_library_actor_handle_create_audio_no_library(mocker, factories): # when we receive inbox create audio, we should not do anything # if we don't have a configured library matching the sender mocked_create = mocker.patch( - 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + 'funkwhale_api.federation.serializers.AudioSerializer.create' ) actor = factories['federation.Actor']() library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() @@ -393,7 +393,7 @@ def test_library_actor_handle_create_audio_no_library(mocker, factories): library_actor.system_conf.post_inbox(data, actor=actor) mocked_create.assert_not_called() - music_models.ImportBatch.objects.count() == 0 + models.LibraryTrack.objects.count() == 0 def test_library_actor_handle_create_audio_no_library_enabled( @@ -401,7 +401,7 @@ def test_library_actor_handle_create_audio_no_library_enabled( # when we receive inbox create audio, we should not do anything # if we don't have an enabled library mocked_create = mocker.patch( - 'funkwhale_api.music.serializers.AudioCollectionImportSerializer.create' + 'funkwhale_api.federation.serializers.AudioSerializer.create' ) disabled_library = factories['federation.Library']( federation_enabled=False) @@ -421,12 +421,14 @@ def test_library_actor_handle_create_audio_no_library_enabled( library_actor.system_conf.post_inbox(data, actor=actor) mocked_create.assert_not_called() - music_models.ImportBatch.objects.count() == 0 + models.LibraryTrack.objects.count() == 0 def test_library_actor_handle_create_audio(mocker, factories): library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() - remote_library = factories['federation.Library']() + remote_library = factories['federation.Library']( + federation_enabled=True + ) data = { 'actor': remote_library.actor.url, @@ -442,14 +444,19 @@ def test_library_actor_handle_create_audio(mocker, factories): library_actor.system_conf.post_inbox(data, actor=remote_library.actor) - batch = remote_library.import_batches.latest('id') + lts = list(remote_library.tracks.order_by('id')) - assert batch.source_library_url == data['object']['id'] - assert batch.source_library == remote_library - assert batch.jobs.count() == 2 + assert len(lts) == 2 - jobs = list(batch.jobs.order_by('id')) for i, a in enumerate(data['object']['items']): - job = jobs[i] - assert job.source_library_url == a['id'] - assert job.source == a['url']['href'] + 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']) diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index 1e580040e..45778ed48 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -1,10 +1,13 @@ +import arrow + from django.urls import reverse from django.core.paginator import Paginator +from funkwhale_api.federation import actors from funkwhale_api.federation import keys from funkwhale_api.federation import models from funkwhale_api.federation import serializers -from funkwhale_api.music.serializers import AudioSerializer +from funkwhale_api.federation import utils def test_actor_serializer_from_ap(db): @@ -174,7 +177,7 @@ def test_paginated_collection_serializer(factories): conf = { 'id': 'https://test.federation/test', 'items': tfs, - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': actor, 'page_size': 2, } @@ -204,7 +207,7 @@ def test_collection_page_serializer(factories): conf = { 'id': 'https://test.federation/test', - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': actor, 'page': Paginator(tfs, 2).page(2), } @@ -235,3 +238,140 @@ def test_collection_page_serializer(factories): serializer = serializers.CollectionPageSerializer(conf) assert serializer.data == expected + + +def test_activity_pub_audio_serializer_to_library_track(factories): + remote_library = factories['federation.Library']() + audio = factories['federation.Audio']() + serializer = serializers.AudioSerializer( + data=audio, context={'library': remote_library}) + + assert serializer.is_valid(raise_exception=True) + + lt = serializer.save() + + assert lt.pk is not None + assert lt.url == audio['id'] + assert lt.library == remote_library + assert lt.audio_url == audio['url']['href'] + assert lt.audio_mimetype == audio['url']['mediaType'] + assert lt.metadata == audio['metadata'] + assert lt.title == audio['metadata']['recording']['title'] + assert lt.artist_name == audio['metadata']['artist']['name'] + assert lt.album_title == audio['metadata']['release']['title'] + assert lt.published_date == arrow.get(audio['published']) + + +def test_activity_pub_audio_serializer_to_ap(factories): + tf = factories['music.TrackFile'](mimetype='audio/mp3') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'type': 'Audio', + 'id': tf.get_federation_url(), + 'name': tf.track.full_name, + 'published': tf.creation_date.isoformat(), + 'updated': tf.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'musicbrainz_id': tf.track.artist.mbid, + 'name': tf.track.artist.name, + }, + 'release': { + 'musicbrainz_id': tf.track.album.mbid, + 'title': tf.track.album.title, + }, + 'recording': { + 'musicbrainz_id': tf.track.mbid, + 'title': tf.track.title, + }, + }, + 'url': { + 'href': utils.full_url(tf.path), + 'type': 'Link', + 'mediaType': 'audio/mp3' + }, + 'attributedTo': [ + library.url + ] + } + + serializer = serializers.AudioSerializer(tf, context={'actor': library}) + + assert serializer.data == expected + + +def test_activity_pub_audio_serializer_to_ap_no_mbid(factories): + tf = factories['music.TrackFile']( + mimetype='audio/mp3', + track__mbid=None, + track__album__mbid=None, + track__album__artist__mbid=None, + ) + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'type': 'Audio', + 'id': tf.get_federation_url(), + 'name': tf.track.full_name, + 'published': tf.creation_date.isoformat(), + 'updated': tf.modification_date.isoformat(), + 'metadata': { + 'artist': { + 'name': tf.track.artist.name, + 'musicbrainz_id': None, + }, + 'release': { + 'title': tf.track.album.title, + 'musicbrainz_id': None, + }, + 'recording': { + 'title': tf.track.title, + 'musicbrainz_id': None, + }, + }, + 'url': { + 'href': utils.full_url(tf.path), + 'type': 'Link', + 'mediaType': 'audio/mp3' + }, + 'attributedTo': [ + library.url + ] + } + + serializer = serializers.AudioSerializer(tf, context={'actor': library}) + + assert serializer.data == expected + + +def test_collection_serializer_to_ap(factories): + tf1 = factories['music.TrackFile'](mimetype='audio/mp3') + tf2 = factories['music.TrackFile'](mimetype='audio/ogg') + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + expected = { + '@context': serializers.AP_CONTEXT, + 'id': 'https://test.id', + 'actor': library.url, + 'totalItems': 2, + 'type': 'Collection', + 'items': [ + serializers.AudioSerializer( + tf1, context={'actor': library, 'include_ap_context': False} + ).data, + serializers.AudioSerializer( + tf2, context={'actor': library, 'include_ap_context': False} + ).data, + ] + } + + collection = { + 'id': expected['id'], + 'actor': library, + 'items': [tf1, tf2], + 'item_serializer': serializers.AudioSerializer + } + serializer = serializers.CollectionSerializer( + collection, context={'actor': library, 'id': 'https://test.id'}) + + assert serializer.data == expected diff --git a/api/tests/federation/test_views.py b/api/tests/federation/test_views.py index c7e1fc012..c26810dad 100644 --- a/api/tests/federation/test_views.py +++ b/api/tests/federation/test_views.py @@ -7,7 +7,6 @@ from funkwhale_api.federation import actors from funkwhale_api.federation import serializers from funkwhale_api.federation import utils from funkwhale_api.federation import webfinger -from funkwhale_api.music.serializers import AudioSerializer @pytest.mark.parametrize('system_actor', actors.SYSTEM_ACTORS.keys()) @@ -87,7 +86,7 @@ def test_audio_file_list_actor_no_page( 'id': utils.full_url(reverse('federation:music:files-list')), 'page_size': 2, 'items': list(reversed(tfs)), # we order by -creation_date - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library } expected = serializers.PaginatedCollectionSerializer(conf).data @@ -107,7 +106,7 @@ def test_audio_file_list_actor_page( conf = { 'id': utils.full_url(reverse('federation:music:files-list')), 'page': Paginator(list(reversed(tfs)), 2).page(2), - 'item_serializer': AudioSerializer, + 'item_serializer': serializers.AudioSerializer, 'actor': library } expected = serializers.CollectionPageSerializer(conf).data diff --git a/api/tests/music/test_import.py b/api/tests/music/test_import.py index 7e3ebcc19..a15f027ba 100644 --- a/api/tests/music/test_import.py +++ b/api/tests/music/test_import.py @@ -38,32 +38,23 @@ def test_create_import_can_bind_to_request( assert batch.import_request == request -@pytest.mark.parametrize('url,type,expected', [ - ('https://musicbrainz.org/artist/test', 'artist', 'test'), - ('https://musicbrainz.org/release/test', 'release', 'test'), - ('https://musicbrainz.org/recording/test', 'recording', 'test'), - ('https://musicbrainz.org/recording/test', 'artist', None), -]) -def test_get_mbid(url, type, expected): - assert tasks.get_mbid(url, type) == expected - - def test_import_job_from_federation_no_musicbrainz(factories): + lt = factories['federation.LibraryTrack']( + artist_name='Hello', + album_title='World', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__release={'title': 'World'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track.title == 'Ping' assert tf.track.artist.name == 'Hello' assert tf.track.album.title == 'World' @@ -74,23 +65,25 @@ def test_import_job_from_federation_musicbrainz_recording(factories, mocker): track_from_api = mocker.patch( 'funkwhale_api.music.models.Track.get_or_create_from_api', return_value=t) + lt = factories['federation.LibraryTrack']( + metadata__recording__musicbrainz=True, + artist_name='Hello', + album_title='World', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__release={'title': 'World'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track == t track_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['recording'], 'recording')) + mbid=lt.metadata['recording']['musicbrainz_id']) def test_import_job_from_federation_musicbrainz_release(factories, mocker): @@ -98,26 +91,28 @@ def test_import_job_from_federation_musicbrainz_release(factories, mocker): album_from_api = mocker.patch( 'funkwhale_api.music.models.Album.get_or_create_from_api', return_value=a) + lt = factories['federation.LibraryTrack']( + metadata__release__musicbrainz=True, + artist_name='Hello', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__artist={'name': 'Hello'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url - assert tf.source == job.source + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track assert tf.track.title == 'Ping' assert tf.track.artist == a.artist assert tf.track.album == a album_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['release'], 'release')) + mbid=lt.metadata['release']['musicbrainz_id']) def test_import_job_from_federation_musicbrainz_artist(factories, mocker): @@ -125,24 +120,27 @@ def test_import_job_from_federation_musicbrainz_artist(factories, mocker): artist_from_api = mocker.patch( 'funkwhale_api.music.models.Artist.get_or_create_from_api', return_value=a) + lt = factories['federation.LibraryTrack']( + metadata__artist__musicbrainz=True, + album_title='World', + title='Ping', + ) job = factories['music.ImportJob']( federation=True, - metadata__release={'title': 'World'}, - metadata__recording={'title': 'Ping'}, - mbid=None, + library_track=lt, ) tasks.import_job_run(import_job_id=job.pk) job.refresh_from_db() tf = job.track_file - assert tf.source == job.source - assert tf.source_library == job.batch.source_library - assert tf.source_library_url == job.source_library_url + assert tf.mimetype == lt.audio_mimetype + assert tf.library_track == job.library_track + assert tf.track.title == 'Ping' assert tf.track.artist == a assert tf.track.album.artist == a assert tf.track.album.title == 'World' artist_from_api.assert_called_once_with( - mbid=tasks.get_mbid(job.metadata['artist'], 'artist')) + mbid=lt.metadata['artist']['musicbrainz_id']) diff --git a/api/tests/music/test_serializers.py b/api/tests/music/test_serializers.py deleted file mode 100644 index a1f5999e7..000000000 --- a/api/tests/music/test_serializers.py +++ /dev/null @@ -1,94 +0,0 @@ -from funkwhale_api.federation import actors -from funkwhale_api.federation import utils as federation_utils -from funkwhale_api.federation.serializers import AP_CONTEXT -from funkwhale_api.music import serializers - - -def test_activity_pub_audio_collection_serializer_to_import(factories): - remote_library = factories['federation.Library']() - - collection = { - 'id': 'https://batch.import', - 'type': 'Collection', - 'totalItems': 2, - 'items': factories['federation.Audio'].create_batch(size=2) - } - - serializer = serializers.AudioCollectionImportSerializer( - data=collection, context={'library': remote_library}) - - assert serializer.is_valid(raise_exception=True) - - batch = serializer.save() - jobs = list(batch.jobs.all()) - - assert batch.source == 'federation' - assert batch.source_library_url == collection['id'] - assert batch.source_library == remote_library - assert len(jobs) == 2 - - for i, a in enumerate(collection['items']): - job = jobs[i] - assert job.source_library_url == a['id'] - assert job.source == a['url']['href'] - a['metadata']['mediaType'] = a['url']['mediaType'] - assert job.metadata == a['metadata'] - - -def test_activity_pub_audio_serializer_to_ap(factories): - tf = factories['music.TrackFile'](mimetype='audio/mp3') - library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - expected = { - '@context': AP_CONTEXT, - 'type': 'Audio', - 'id': tf.get_federation_url(), - 'name': tf.track.full_name, - 'metadata': { - 'artist': tf.track.artist.musicbrainz_url, - 'release': tf.track.album.musicbrainz_url, - 'track': tf.track.musicbrainz_url, - }, - 'url': { - 'href': federation_utils.full_url(tf.path), - 'type': 'Link', - 'mediaType': 'audio/mp3' - }, - 'attributedTo': [ - library.url - ] - } - - serializer = serializers.AudioSerializer(tf, context={'actor': library}) - - assert serializer.data == expected - - -def test_activity_pub_audio_collection_serializer_to_ap(factories): - tf1 = factories['music.TrackFile'](mimetype='audio/mp3') - tf2 = factories['music.TrackFile'](mimetype='audio/ogg') - library = actors.SYSTEM_ACTORS['library'].get_actor_instance() - expected = { - '@context': AP_CONTEXT, - 'id': 'https://test.id', - 'actor': library.url, - 'totalItems': 2, - 'type': 'Collection', - 'items': [ - serializers.AudioSerializer( - tf1, context={'actor': library, 'include_ap_context': False} - ).data, - serializers.AudioSerializer( - tf2, context={'actor': library, 'include_ap_context': False} - ).data, - ] - } - - collection = { - 'id': expected['id'], - 'actor': library, - 'items': [tf1, tf2], - } - serializer = serializers.AudioCollectionImportSerializer( - collection, context={'actor': library, 'id': 'https://test.id'}) - - assert serializer.data == expected From 9612b1bace4b59cafeed1304971395ffbc1ececb Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 7 Apr 2018 15:34:35 +0200 Subject: [PATCH 28/30] Can now serve track from remote library --- api/funkwhale_api/federation/activity.py | 15 +--- api/funkwhale_api/federation/actors.py | 32 ++++++--- .../federation/authentication.py | 2 + .../federation/management/__init__.py | 0 .../management/commands/__init__.py | 0 .../management/commands/generate_keys.py | 53 -------------- ...407_0852.py => 0003_auto_20180407_1010.py} | 4 +- api/funkwhale_api/federation/models.py | 7 -- api/funkwhale_api/federation/signing.py | 15 ++++ api/funkwhale_api/music/factories.py | 12 ++++ ...407_0852.py => 0023_auto_20180407_1010.py} | 9 ++- api/funkwhale_api/music/models.py | 14 ++-- api/funkwhale_api/music/permissions.py | 23 +++++++ api/funkwhale_api/music/tasks.py | 5 +- api/funkwhale_api/music/utils.py | 7 ++ api/funkwhale_api/music/views.py | 69 +++++++++++++++---- api/tests/conftest.py | 9 +++ api/tests/federation/conftest.py | 10 --- api/tests/federation/test_actors.py | 16 +++-- api/tests/federation/test_commands.py | 14 ---- api/tests/music/test_permissions.py | 56 +++++++++++++++ api/tests/music/test_views.py | 40 +++++++++++ 22 files changed, 272 insertions(+), 140 deletions(-) delete mode 100644 api/funkwhale_api/federation/management/__init__.py delete mode 100644 api/funkwhale_api/federation/management/commands/__init__.py delete mode 100644 api/funkwhale_api/federation/management/commands/generate_keys.py rename api/funkwhale_api/federation/migrations/{0003_auto_20180407_0852.py => 0003_auto_20180407_1010.py} (94%) rename api/funkwhale_api/music/migrations/{0023_auto_20180407_0852.py => 0023_auto_20180407_1010.py} (88%) create mode 100644 api/funkwhale_api/music/permissions.py delete mode 100644 api/tests/federation/conftest.py delete mode 100644 api/tests/federation/test_commands.py create mode 100644 api/tests/music/test_permissions.py diff --git a/api/funkwhale_api/federation/activity.py b/api/funkwhale_api/federation/activity.py index db71bd4fb..7502bd739 100644 --- a/api/funkwhale_api/federation/activity.py +++ b/api/funkwhale_api/federation/activity.py @@ -58,21 +58,12 @@ OBJECT_TYPES = [ 'Video', ] + ACTIVITY_TYPES + def deliver(activity, on_behalf_of, to=[]): from . import actors logger.info('Preparing activity delivery to %s', to) - auth = requests_http_signature.HTTPSignatureAuth( - use_auth_header=False, - headers=[ - '(request-target)', - 'user-agent', - 'host', - 'date', - 'content-type',], - algorithm='rsa-sha256', - key=on_behalf_of.private_key.encode('utf-8'), - key_id=on_behalf_of.private_key_id, - ) + auth = signing.get_auth( + on_behalf_of.private_key, on_behalf_of.private_key_id) for url in to: recipient_actor = actors.get_actor(url) logger.debug('delivering to %s', recipient_actor.inbox_url) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index ffbafd8b7..f640c9b12 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -13,8 +13,10 @@ from rest_framework.exceptions import PermissionDenied from dynamic_preferences.registries import global_preferences_registry from . import activity +from . import keys from . import models from . import serializers +from . import signing from . import utils logger = logging.getLogger(__name__) @@ -51,24 +53,37 @@ class SystemActor(object): additional_attributes = {} manually_approves_followers = False + def get_request_auth(self): + actor = self.get_actor_instance() + return signing.get_auth( + actor.private_key, actor.private_key_id) + def serialize(self): actor = self.get_actor_instance() serializer = serializers.ActorSerializer(actor) return serializer.data def get_actor_instance(self): + try: + return models.Actor.objects.get(url=self.get_actor_url()) + except models.Actor.DoesNotExist: + pass + private, public = keys.get_key_pair() args = self.get_instance_argument( self.id, name=self.name, summary=self.summary, **self.additional_attributes ) - url = args.pop('url') - a, created = models.Actor.objects.get_or_create( - url=url, - defaults=args, - ) - return a + args['private_key'] = private.decode('utf-8') + args['public_key'] = public.decode('utf-8') + return models.Actor.objects.create(**args) + + def get_actor_url(self): + return utils.full_url( + reverse( + 'federation:instance-actors-detail', + kwargs={'actor': self.id})) def get_instance_argument(self, id, name, summary, **kwargs): preferences = global_preferences_registry.manager() @@ -78,10 +93,7 @@ class SystemActor(object): 'type': 'Person', 'name': name.format(host=settings.FEDERATION_HOSTNAME), 'manually_approves_followers': True, - 'url': utils.full_url( - reverse( - 'federation:instance-actors-detail', - kwargs={'actor': id})), + 'url': self.get_actor_url(), 'shared_inbox_url': utils.full_url( reverse( 'federation:instance-actors-inbox', diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py index f2926bb30..7f8ad6653 100644 --- a/api/funkwhale_api/federation/authentication.py +++ b/api/funkwhale_api/federation/authentication.py @@ -51,6 +51,8 @@ class SignatureAuthentication(authentication.BaseAuthentication): def authenticate(self, request): setattr(request, 'actor', None) actor = self.authenticate_actor(request) + if not actor: + return user = AnonymousUser() setattr(request, 'actor', actor) return (user, None) diff --git a/api/funkwhale_api/federation/management/__init__.py b/api/funkwhale_api/federation/management/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/api/funkwhale_api/federation/management/commands/__init__.py b/api/funkwhale_api/federation/management/commands/__init__.py deleted file mode 100644 index e69de29bb..000000000 diff --git a/api/funkwhale_api/federation/management/commands/generate_keys.py b/api/funkwhale_api/federation/management/commands/generate_keys.py deleted file mode 100644 index eafe9aae3..000000000 --- a/api/funkwhale_api/federation/management/commands/generate_keys.py +++ /dev/null @@ -1,53 +0,0 @@ -from django.core.management.base import BaseCommand, CommandError -from django.db import transaction - -from dynamic_preferences.registries import global_preferences_registry - -from funkwhale_api.federation import keys - - -class Command(BaseCommand): - help = ( - 'Generate a public/private key pair for your instance,' - ' for federation purposes. If a key pair already exists, does nothing.' - ) - - def add_arguments(self, parser): - parser.add_argument( - '--replace', - action='store_true', - dest='replace', - default=False, - help='Replace existing key pair, if any', - ) - parser.add_argument( - '--noinput', '--no-input', action='store_false', dest='interactive', - help="Do NOT prompt the user for input of any kind.", - ) - - @transaction.atomic - def handle(self, *args, **options): - preferences = global_preferences_registry.manager() - existing_public = preferences['federation__public_key'] - existing_private = preferences['federation__public_key'] - - if existing_public or existing_private and not options['replace']: - raise CommandError( - 'Keys are already present! ' - 'Replace them with --replace if you know what you are doing.') - - if options['interactive']: - message = ( - 'Are you sure you want to do this?\n\n' - "Type 'yes' to continue, or 'no' to cancel: " - ) - if input(''.join(message)) != 'yes': - raise CommandError("Operation cancelled.") - private, public = keys.get_key_pair() - preferences['federation__public_key'] = public.decode('utf-8') - preferences['federation__private_key'] = private.decode('utf-8') - - self.stdout.write( - 'Your new key pair was generated.' - 'Your public key is now:\n\n{}'.format(public.decode('utf-8')) - ) diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py b/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py similarity index 94% rename from api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py rename to api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py index afc7ea083..38ac7cb4f 100644 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180407_0852.py +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.3 on 2018-04-07 08:52 +# Generated by Django 2.0.3 on 2018-04-07 10:10 import django.contrib.postgres.fields.jsonb from django.db import migrations, models @@ -10,7 +10,6 @@ import uuid class Migration(migrations.Migration): dependencies = [ - ('music', '0022_importbatch_import_request'), ('federation', '0002_auto_20180403_1620'), ] @@ -70,7 +69,6 @@ class Migration(migrations.Migration): ('title', models.CharField(max_length=500)), ('metadata', django.contrib.postgres.fields.jsonb.JSONField(default={}, max_length=10000)), ('library', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='tracks', to='federation.Library')), - ('local_track_file', models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='library_track', to='music.TrackFile')), ], ), migrations.AddField( diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 91f2ea973..bf1e5d830 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -192,13 +192,6 @@ class LibraryTrack(models.Model): published_date = models.DateTimeField(null=True, blank=True) library = models.ForeignKey( Library, related_name='tracks', on_delete=models.CASCADE) - local_track_file = models.OneToOneField( - 'music.TrackFile', - related_name='library_track', - on_delete=models.CASCADE, - null=True, - blank=True, - ) artist_name = models.CharField(max_length=500) album_title = models.CharField(max_length=500) title = models.CharField(max_length=500) diff --git a/api/funkwhale_api/federation/signing.py b/api/funkwhale_api/federation/signing.py index 7e4d2aa5a..8d984d3ff 100644 --- a/api/funkwhale_api/federation/signing.py +++ b/api/funkwhale_api/federation/signing.py @@ -53,3 +53,18 @@ def verify_django(django_request, public_key): request.headers[h] = str(v) prepared_request = request.prepare() return verify(request, public_key) + + +def get_auth(private_key, private_key_id): + return requests_http_signature.HTTPSignatureAuth( + use_auth_header=False, + headers=[ + '(request-target)', + 'user-agent', + 'host', + 'date', + 'content-type'], + algorithm='rsa-sha256', + key=private_key.encode('utf-8'), + key_id=private_key_id, + ) diff --git a/api/funkwhale_api/music/factories.py b/api/funkwhale_api/music/factories.py index 83aad432a..2bf1960ca 100644 --- a/api/funkwhale_api/music/factories.py +++ b/api/funkwhale_api/music/factories.py @@ -56,6 +56,18 @@ class TrackFileFactory(factory.django.DjangoModelFactory): class Meta: model = 'music.TrackFile' + class Params: + federation = factory.Trait( + audio_file=None, + library_track=factory.SubFactory(LibraryTrackFactory), + mimetype=factory.LazyAttribute( + lambda o: o.library_track.audio_mimetype + ), + source=factory.LazyAttribute( + lambda o: o.library_track.audio_url + ), + ) + @registry.register class ImportBatchFactory(factory.django.DjangoModelFactory): diff --git a/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py b/api/funkwhale_api/music/migrations/0023_auto_20180407_1010.py similarity index 88% rename from api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py rename to api/funkwhale_api/music/migrations/0023_auto_20180407_1010.py index b1bbeacef..0539d90f6 100644 --- a/api/funkwhale_api/music/migrations/0023_auto_20180407_0852.py +++ b/api/funkwhale_api/music/migrations/0023_auto_20180407_1010.py @@ -1,4 +1,4 @@ -# Generated by Django 2.0.3 on 2018-04-07 08:52 +# Generated by Django 2.0.3 on 2018-04-07 10:10 from django.conf import settings from django.db import migrations, models @@ -10,7 +10,7 @@ import uuid class Migration(migrations.Migration): dependencies = [ - ('federation', '0003_auto_20180407_0852'), + ('federation', '0003_auto_20180407_1010'), ('music', '0022_importbatch_import_request'), ] @@ -55,6 +55,11 @@ class Migration(migrations.Migration): name='creation_date', field=models.DateTimeField(default=django.utils.timezone.now), ), + migrations.AddField( + model_name='trackfile', + name='library_track', + field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='local_track_file', to='federation.LibraryTrack'), + ), migrations.AddField( model_name='trackfile', name='modification_date', diff --git a/api/funkwhale_api/music/models.py b/api/funkwhale_api/music/models.py index bcf691bcd..beec551a5 100644 --- a/api/funkwhale_api/music/models.py +++ b/api/funkwhale_api/music/models.py @@ -419,6 +419,14 @@ class TrackFile(models.Model): acoustid_track_id = models.UUIDField(null=True, blank=True) mimetype = models.CharField(null=True, blank=True, max_length=200) + library_track = models.OneToOneField( + 'federation.LibraryTrack', + related_name='local_track_file', + on_delete=models.CASCADE, + null=True, + blank=True, + ) + def download_file(self): # import the track file, since there is not any # we create a tmp dir for the download @@ -441,10 +449,8 @@ class TrackFile(models.Model): @property def path(self): - if settings.PROTECT_AUDIO_FILES: - return reverse( - 'api:v1:trackfiles-serve', kwargs={'pk': self.pk}) - return self.audio_file.url + return reverse( + 'api:v1:trackfiles-serve', kwargs={'pk': self.pk}) @property def filename(self): diff --git a/api/funkwhale_api/music/permissions.py b/api/funkwhale_api/music/permissions.py new file mode 100644 index 000000000..a8e62f1e7 --- /dev/null +++ b/api/funkwhale_api/music/permissions.py @@ -0,0 +1,23 @@ +from django.conf import settings + +from rest_framework.permissions import BasePermission + +from funkwhale_api.federation import actors + + +class Listen(BasePermission): + + def has_permission(self, request, view): + if not settings.PROTECT_AUDIO_FILES: + return True + + user = getattr(request, 'user', None) + if user and user.is_authenticated: + return True + + actor = getattr(request, 'actor', None) + if actor is None: + return False + + library = actors.SYSTEM_ACTORS['library'].get_actor_instance() + return library.followers.filter(url=actor.url).exists() diff --git a/api/funkwhale_api/music/tasks.py b/api/funkwhale_api/music/tasks.py index c58eb7136..012b72cd2 100644 --- a/api/funkwhale_api/music/tasks.py +++ b/api/funkwhale_api/music/tasks.py @@ -112,6 +112,7 @@ def _do_import(import_job, replace, use_acoustid=True): track_file.audio_file.name = import_job.audio_file.name track_file.duration = duration elif import_job.library_track: + track_file.library_track = import_job.library_track track_file.mimetype = import_job.library_track.audio_mimetype if import_job.library_track.library.download_files: raise NotImplementedError() @@ -121,10 +122,6 @@ def _do_import(import_job, replace, use_acoustid=True): else: track_file.download_file() track_file.save() - if import_job.library_track: - import_job.library_track.local_track_file = track_file - import_job.library_track.save( - update_fields=['modification_date', 'local_track_file']) import_job.status = 'finished' import_job.track_file = track_file if import_job.audio_file: diff --git a/api/funkwhale_api/music/utils.py b/api/funkwhale_api/music/utils.py index df659cb80..af0e59ab4 100644 --- a/api/funkwhale_api/music/utils.py +++ b/api/funkwhale_api/music/utils.py @@ -60,3 +60,10 @@ def compute_status(jobs): if pending: return 'pending' return 'finished' + + +def get_ext_from_type(mimetype): + mapping = { + 'audio/ogg': 'ogg', + 'audio/mpeg': 'mp3', + } diff --git a/api/funkwhale_api/music/views.py b/api/funkwhale_api/music/views.py index 5ac3143f9..5f8fc1736 100644 --- a/api/funkwhale_api/music/views.py +++ b/api/funkwhale_api/music/views.py @@ -1,36 +1,42 @@ import ffmpeg import os import json +import requests import subprocess import unicodedata import urllib -from django.urls import reverse +from django.contrib.auth.decorators import login_required +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.conf import settings from django.http import StreamingHttpResponse +from django.urls import reverse +from django.utils.decorators import method_decorator 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 settings as rest_settings from rest_framework import permissions from musicbrainzngs import ResponseError -from django.contrib.auth.decorators import login_required -from django.utils.decorators import method_decorator 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.authentication import SignatureAuthentication -from . import forms -from . import models -from . import serializers -from . import importers from . import filters +from . import forms +from . import importers +from . import models +from . import permissions as music_permissions +from . import serializers from . import tasks from . import utils @@ -45,6 +51,7 @@ class SearchMixin(object): serializer = self.serializer_class(queryset, many=True) return Response(serializer.data) + class TagViewSetMixin(object): def get_queryset(self): @@ -179,22 +186,54 @@ class TrackViewSet(TagViewSetMixin, SearchMixin, viewsets.ReadOnlyModelViewSet): class TrackFileViewSet(viewsets.ReadOnlyModelViewSet): queryset = (models.TrackFile.objects.all().order_by('-id')) serializer_class = serializers.TrackFileSerializer - permission_classes = [ConditionalAuthentication] + authentication_classes = rest_settings.api_settings.DEFAULT_AUTHENTICATION_CLASSES + [ + SignatureAuthentication + ] + permission_classes = [music_permissions.Listen] @detail_route(methods=['get']) def serve(self, request, *args, **kwargs): try: - f = models.TrackFile.objects.get(pk=kwargs['pk']) + f = models.TrackFile.objects.select_related( + 'library_track', + 'track__album__artist', + 'track__artist', + ).get(pk=kwargs['pk']) except models.TrackFile.DoesNotExist: return Response(status=404) - response = Response() + mt = f.mimetype + try: + library_track = f.library_track + except ObjectDoesNotExist: + library_track = None + if library_track and not f.audio_file: + # we proxy the response to the remote library + # since we did not mirror the file locally + mt = library_track.audio_mimetype + file_extension = utils.get_ext_from_type(mt) + filename = '{}.{}'.format(f.track.full_name, file_extension) + auth = actors.SYSTEM_ACTORS['library'].get_request_auth() + remote_response = requests.get( + library_track.audio_url, + auth=auth, + stream=True, + headers={ + 'Content-Type': 'application/activity+json' + }) + response = StreamingHttpResponse(remote_response.iter_content()) + else: + response = Response() + filename = f.filename + response['X-Accel-Redirect'] = "{}{}".format( + settings.PROTECT_FILES_PATH, + f.audio_file.url) filename = "filename*=UTF-8''{}".format( - urllib.parse.quote(f.filename)) + urllib.parse.quote(filename)) response["Content-Disposition"] = "attachment; {}".format(filename) - response['X-Accel-Redirect'] = "{}{}".format( - settings.PROTECT_FILES_PATH, - f.audio_file.url) + if mt: + response["Content-Type"] = mt + return response @list_route(methods=['get']) diff --git a/api/tests/conftest.py b/api/tests/conftest.py index d5bb56565..4f1ee8962 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -162,3 +162,12 @@ def media_root(settings): def r_mock(): with requests_mock.mock() as m: yield m + + +@pytest.fixture +def authenticated_actor(factories, mocker): + actor = factories['federation.Actor']() + mocker.patch( + 'funkwhale_api.federation.authentication.SignatureAuthentication.authenticate_actor', + return_value=actor) + yield actor diff --git a/api/tests/federation/conftest.py b/api/tests/federation/conftest.py deleted file mode 100644 index c5831914b..000000000 --- a/api/tests/federation/conftest.py +++ /dev/null @@ -1,10 +0,0 @@ -import pytest - - -@pytest.fixture -def authenticated_actor(nodb_factories, mocker): - actor = nodb_factories['federation.Actor']() - mocker.patch( - 'funkwhale_api.federation.authentication.SignatureAuthentication.authenticate_actor', - return_value=actor) - yield actor diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py index 107047b56..090d9b03f 100644 --- a/api/tests/federation/test_actors.py +++ b/api/tests/federation/test_actors.py @@ -26,14 +26,17 @@ def test_actor_fetching(r_mock): assert r == payload -def test_get_library(settings, preferences): - preferences['federation__public_key'] = 'public_key' +def test_get_library(db, settings, mocker): + get_key_pair = mocker.patch( + 'funkwhale_api.federation.keys.get_key_pair', + return_value=(b'private', b'public')) expected = { 'preferred_username': 'library', 'domain': settings.FEDERATION_HOSTNAME, 'type': 'Person', 'name': '{}\'s library'.format(settings.FEDERATION_HOSTNAME), 'manually_approves_followers': True, + 'public_key': 'public', 'url': utils.full_url( reverse( 'federation:instance-actors-detail', @@ -50,7 +53,6 @@ def test_get_library(settings, preferences): reverse( 'federation:instance-actors-outbox', kwargs={'actor': 'library'})), - 'public_key': 'public_key', 'summary': 'Bot account to federate with {}\'s library'.format( settings.FEDERATION_HOSTNAME), } @@ -59,14 +61,17 @@ def test_get_library(settings, preferences): assert getattr(actor, key) == value -def test_get_test(settings, preferences): - preferences['federation__public_key'] = 'public_key' +def test_get_test(db, mocker, settings): + get_key_pair = mocker.patch( + 'funkwhale_api.federation.keys.get_key_pair', + return_value=(b'private', b'public')) expected = { 'preferred_username': 'test', 'domain': settings.FEDERATION_HOSTNAME, 'type': 'Person', 'name': '{}\'s test account'.format(settings.FEDERATION_HOSTNAME), 'manually_approves_followers': False, + 'public_key': 'public', 'url': utils.full_url( reverse( 'federation:instance-actors-detail', @@ -83,7 +88,6 @@ def test_get_test(settings, preferences): reverse( 'federation:instance-actors-outbox', kwargs={'actor': 'test'})), - 'public_key': 'public_key', 'summary': 'Bot account to test federation with {}. Send me /ping and I\'ll answer you.'.format( settings.FEDERATION_HOSTNAME), } diff --git a/api/tests/federation/test_commands.py b/api/tests/federation/test_commands.py deleted file mode 100644 index 7c5333068..000000000 --- a/api/tests/federation/test_commands.py +++ /dev/null @@ -1,14 +0,0 @@ -from django.core.management import call_command - - -def test_generate_instance_key_pair(preferences, mocker): - mocker.patch( - 'funkwhale_api.federation.keys.get_key_pair', - return_value=(b'private', b'public')) - assert preferences['federation__public_key'] == '' - assert preferences['federation__private_key'] == '' - - call_command('generate_keys', interactive=False) - - assert preferences['federation__private_key'] == 'private' - assert preferences['federation__public_key'] == 'public' diff --git a/api/tests/music/test_permissions.py b/api/tests/music/test_permissions.py new file mode 100644 index 000000000..6cce85e08 --- /dev/null +++ b/api/tests/music/test_permissions.py @@ -0,0 +1,56 @@ +from rest_framework.views import APIView + +from funkwhale_api.federation import actors +from funkwhale_api.music import permissions + + +def test_list_permission_no_protect(anonymous_user, api_request, settings): + settings.PROTECT_AUDIO_FILES = False + view = APIView.as_view() + permission = permissions.Listen() + request = api_request.get('/') + assert permission.has_permission(request, view) is True + + +def test_list_permission_protect_anonymous( + anonymous_user, api_request, settings): + settings.PROTECT_AUDIO_FILES = True + view = APIView.as_view() + permission = permissions.Listen() + request = api_request.get('/') + assert permission.has_permission(request, view) is False + + +def test_list_permission_protect_authenticated( + factories, api_request, settings): + settings.PROTECT_AUDIO_FILES = True + user = factories['users.User']() + view = APIView.as_view() + permission = permissions.Listen() + request = api_request.get('/') + setattr(request, 'user', user) + assert permission.has_permission(request, view) is True + + +def test_list_permission_protect_not_following_actor( + factories, api_request, settings): + settings.PROTECT_AUDIO_FILES = True + actor = factories['federation.Actor']() + view = APIView.as_view() + permission = permissions.Listen() + request = api_request.get('/') + setattr(request, 'actor', actor) + assert permission.has_permission(request, view) is False + + +def test_list_permission_protect_following_actor( + factories, api_request, settings): + settings.PROTECT_AUDIO_FILES = True + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + follow = factories['federation.Follow'](target=library_actor) + view = APIView.as_view() + permission = permissions.Listen() + request = api_request.get('/') + setattr(request, 'actor', follow.actor) + + assert permission.has_permission(request, view) is True diff --git a/api/tests/music/test_views.py b/api/tests/music/test_views.py index 295604616..468ea77e3 100644 --- a/api/tests/music/test_views.py +++ b/api/tests/music/test_views.py @@ -1,6 +1,8 @@ +import io import pytest from funkwhale_api.music import views +from funkwhale_api.federation import actors @pytest.mark.parametrize('param,expected', [ @@ -43,3 +45,41 @@ def test_album_view_filter_listenable( queryset = view.filter_queryset(view.get_queryset()) assert list(queryset) == expected + + +def test_can_serve_track_file_as_remote_library( + factories, authenticated_actor, settings, api_client): + settings.PROTECT_AUDIO_FILES = True + library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance() + follow = factories['federation.Follow']( + actor=authenticated_actor, target=library_actor) + + track_file = factories['music.TrackFile']() + response = api_client.get(track_file.path) + + assert response.status_code == 200 + assert response['X-Accel-Redirect'] == "{}{}".format( + settings.PROTECT_FILES_PATH, + track_file.audio_file.url) + + +def test_can_serve_track_file_as_remote_library_deny_not_following( + factories, authenticated_actor, settings, api_client): + settings.PROTECT_AUDIO_FILES = True + track_file = factories['music.TrackFile']() + response = api_client.get(track_file.path) + + assert response.status_code == 403 + + +def test_can_proxy_remote_track( + factories, settings, api_client, r_mock): + settings.PROTECT_AUDIO_FILES = False + track_file = factories['music.TrackFile'](federation=True) + + r_mock.get(track_file.library_track.audio_url, body=io.StringIO('test')) + response = api_client.get(track_file.path) + + assert response.status_code == 200 + assert list(response.streaming_content) == [b't', b'e', b's', b't'] + assert response['Content-Type'] == track_file.library_track.audio_mimetype From e10a82060dcf22332328ceef972da9f16fe7ab91 Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 7 Apr 2018 15:39:17 +0200 Subject: [PATCH 29/30] Ensure we delete existing actors to reset private and public keys --- .../federation/migrations/0003_auto_20180407_1010.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py b/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py index 38ac7cb4f..12e3d73fe 100644 --- a/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py +++ b/api/funkwhale_api/federation/migrations/0003_auto_20180407_1010.py @@ -7,6 +7,16 @@ import django.utils.timezone import uuid +def delete_system_actors(apps, schema_editor): + """Revert site domain and name to default.""" + Actor = apps.get_model("federation", "Actor") + Actor.objects.filter(preferred_username__in=['test', 'library']).delete() + + +def backward(apps, schema_editor): + pass + + class Migration(migrations.Migration): dependencies = [ @@ -14,6 +24,7 @@ class Migration(migrations.Migration): ] operations = [ + migrations.RunPython(delete_system_actors, backward), migrations.CreateModel( name='Follow', fields=[ From bf70fa1f537f38774d7e97d1ab53a16d7459bbeb Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Sat, 7 Apr 2018 15:48:47 +0200 Subject: [PATCH 30/30] Removed now useless private and public keys preferences --- api/funkwhale_api/federation/actors.py | 3 -- .../dynamic_preferences_registry.py | 28 ------------------- 2 files changed, 31 deletions(-) diff --git a/api/funkwhale_api/federation/actors.py b/api/funkwhale_api/federation/actors.py index f640c9b12..54d78d9ff 100644 --- a/api/funkwhale_api/federation/actors.py +++ b/api/funkwhale_api/federation/actors.py @@ -86,7 +86,6 @@ class SystemActor(object): kwargs={'actor': self.id})) def get_instance_argument(self, id, name, summary, **kwargs): - preferences = global_preferences_registry.manager() p = { 'preferred_username': id, 'domain': settings.FEDERATION_HOSTNAME, @@ -106,8 +105,6 @@ class SystemActor(object): reverse( 'federation:instance-actors-outbox', kwargs={'actor': id})), - 'public_key': preferences['federation__public_key'], - 'private_key': preferences['federation__private_key'], 'summary': summary.format(host=settings.FEDERATION_HOSTNAME) } p.update(kwargs) diff --git a/api/funkwhale_api/federation/dynamic_preferences_registry.py b/api/funkwhale_api/federation/dynamic_preferences_registry.py index 83d0285be..c7cb015a8 100644 --- a/api/funkwhale_api/federation/dynamic_preferences_registry.py +++ b/api/funkwhale_api/federation/dynamic_preferences_registry.py @@ -4,31 +4,3 @@ from dynamic_preferences import types from dynamic_preferences.registries import global_preferences_registry federation = types.Section('federation') - - -@global_preferences_registry.register -class FederationPrivateKey(types.StringPreference): - show_in_api = False - section = federation - name = 'private_key' - default = '' - help_text = ( - 'Instance private key, used for signing federation HTTP requests' - ) - verbose_name = ( - 'Instance private key (keep it secret, do not change it)' - ) - - -@global_preferences_registry.register -class FederationPublicKey(types.StringPreference): - show_in_api = False - section = federation - name = 'public_key' - default = '' - help_text = ( - 'Instance public key, used for signing federation HTTP requests' - ) - verbose_name = ( - 'Instance public key (do not change it)' - )