From f273faf9de56e0d265d2093eef847451c25f3d64 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Fri, 6 Apr 2018 18:49:29 +0200
Subject: [PATCH] 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 6f782ced..205c3486 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 63d40aff..fc893239 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 cc653b1a..f8771752 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 4bf59700..8a90bdb7 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 27387ca9..8da56b2a 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 c51a7b9f..2bc8085a 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 efffb12d..4c0b6a09 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 5cd2f2cc..78b2f7c0 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 4f85613e..67b538c1 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 7a5e0d31..6f5e8866 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 86e4f4a8..b17b6eb6 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 6f05a16f..c7e1fc01 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 98174891..7e3ebcc1 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 1270ae76..a1f5999e 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']
--
GitLab