use a dedicated serializer to handle library creation

api/funkwhale_api/federation/actors.py

@@ -170,7 +170,7 @@ class SystemActor(object):
         if not serializer.is_valid(raise_exception=True):
             return logger.info('Received invalid payload')
 
-        serializer.save()
+        return serializer.save()
 
     def handle_undo_follow(self, ac, sender):
         system_actor = self.get_actor_instance()

api/funkwhale_api/federation/migrations/0004_auto_20180410_1624.py → api/funkwhale_api/federation/migrations/0004_auto_20180410_2025.py

-# Generated by Django 2.0.3 on 2018-04-10 16:24
+# Generated by Django 2.0.3 on 2018-04-10 20:25
 
 from django.db import migrations, models
+import django.db.models.deletion
 
 
 class Migration(migrations.Migration):
@@ -23,6 +24,11 @@ class Migration(migrations.Migration):
             name='approved',
             field=models.NullBooleanField(default=None),
         ),
+        migrations.AddField(
+            model_name='library',
+            name='follow',
+            field=models.OneToOneField(blank=True, null=True, on_delete=django.db.models.deletion.SET_NULL, related_name='library', to='federation.Follow'),
+        ),
         migrations.DeleteModel(
             name='FollowRequest',
         ),

api/funkwhale_api/federation/models.py

@@ -137,6 +137,13 @@ class Library(models.Model):
     # should we automatically import new files from this library?
     autoimport = models.BooleanField()
     tracks_count = models.PositiveIntegerField(null=True, blank=True)
+    follow = models.OneToOneField(
+        Follow,
+        related_name='library',
+        null=True,
+        blank=True,
+        on_delete=models.SET_NULL,
+    )
 
 
 class LibraryTrack(models.Model):

api/funkwhale_api/federation/serializers.py

@@ -8,7 +8,7 @@ from django.db import transaction
 from rest_framework import serializers
 from dynamic_preferences.registries import global_preferences_registry
 
-from funkwhale_api.common.utils import set_query_parameter
+from funkwhale_api.common import utils as funkwhale_utils
 
 from . import activity
 from . import models
@@ -121,6 +121,66 @@ class LibraryActorSerializer(ActorSerializer):
         return validated_data
 
 
+class APILibraryCreateSerializer(serializers.ModelSerializer):
+    actor = serializers.URLField()
+
+    class Meta:
+        model = models.Library
+        fields = [
+            'actor',
+            'autoimport',
+            'federation_enabled',
+            'download_files',
+        ]
+
+    def validate(self, validated_data):
+        from . import actors
+        from . import library
+
+        actor_url = validated_data['actor']
+        actor_data = actors.get_actor_data(actor_url)
+        acs = LibraryActorSerializer(data=actor_data)
+        acs.is_valid(raise_exception=True)
+        try:
+            actor = models.Actor.objects.get(url=actor_url)
+        except models.Actor.DoesNotExist:
+            actor = acs.save()
+        library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance()
+        validated_data['follow'] = models.Follow.objects.get_or_create(
+            actor=library_actor,
+            target=actor,
+        )[0]
+        if validated_data['follow'].approved is None:
+            funkwhale_utils.on_commit(
+                activity.deliver,
+                FollowSerializer(validated_data['follow']).data,
+                on_behalf_of=validated_data['follow'].actor,
+                to=[validated_data['follow'].target.url],
+            )
+
+        library_data = library.get_library_data(
+            acs.validated_data['library_url'])
+        if 'errors' in library_data:
+            raise serializers.ValidationError(str(library_data['errors']))
+        validated_data['library'] = library_data
+        validated_data['actor'] = actor
+        return validated_data
+
+    def create(self, validated_data):
+        library = models.Library.objects.get_or_create(
+            url=validated_data['library']['id'],
+            defaults={
+                'actor': validated_data['actor'],
+                'follow': validated_data['follow'],
+                'tracks_count': validated_data['library']['totalItems'],
+                'federation_enabled': validated_data['federation_enabled'],
+                'autoimport': validated_data['autoimport'],
+                'download_files': validated_data['download_files'],
+            }
+        )[0]
+        return library
+
+
 class FollowSerializer(serializers.Serializer):
     id = serializers.URLField()
     object = serializers.URLField()
@@ -163,6 +223,20 @@ class FollowSerializer(serializers.Serializer):
         return ret
 
 
+class APIFollowSerializer(serializers.ModelSerializer):
+    class Meta:
+        model = models.Follow
+        fields = [
+            'uuid',
+            'id',
+            'approved',
+            'creation_date',
+            'modification_date',
+            'actor',
+            'target',
+        ]
+
+
 class AcceptFollowSerializer(serializers.Serializer):
     id = serializers.URLField()
     actor = serializers.URLField()
@@ -244,7 +318,7 @@ class UndoFollowSerializer(serializers.Serializer):
         }
 
     def save(self):
-        self.validated_data['follow'].delete()
+        return self.validated_data['follow'].delete()
 
 
 class ActorWebfingerSerializer(serializers.Serializer):
@@ -365,9 +439,10 @@ class PaginatedCollectionSerializer(serializers.Serializer):
             conf['items'],
             conf.get('page_size', 20)
         )
-        first = set_query_parameter(conf['id'], page=1)
+        first = funkwhale_utils.set_query_parameter(conf['id'], page=1)
         current = first
-        last = set_query_parameter(conf['id'], page=paginator.num_pages)
+        last = funkwhale_utils.set_query_parameter(
+            conf['id'], page=paginator.num_pages)
         d = {
             'id': conf['id'],
             'actor': conf['actor'].url,
@@ -394,9 +469,12 @@ 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)
+        first = funkwhale_utils.set_query_parameter(
+            conf['id'], page=1)
+        last = funkwhale_utils.set_query_parameter(
+            conf['id'], page=page.paginator.num_pages)
+        id = funkwhale_utils.set_query_parameter(
+            conf['id'], page=page.number)
         d = {
             'id': id,
             'partOf': conf['id'],
@@ -417,11 +495,11 @@ class CollectionPageSerializer(serializers.Serializer):
         }
 
         if page.has_previous():
-            d['prev'] = set_query_parameter(
+            d['prev'] = funkwhale_utils.set_query_parameter(
                 conf['id'], page=page.previous_page_number())
 
         if page.has_next():
-            d['next'] = set_query_parameter(
+            d['next'] = funkwhale_utils.set_query_parameter(
                 conf['id'], page=page.next_page_number())
 
         if self.context.get('include_ap_context', True):

api/funkwhale_api/federation/views.py

@@ -179,26 +179,7 @@ class LibraryViewSet(viewsets.GenericViewSet):
 
     @transaction.atomic
     def create(self, request, *args, **kwargs):
-        try:
-            actor_url = request.data['actor_url']
-        except KeyError:
-            raise ValidationError('Missing actor_url')
-
-        try:
-            actor = actors.get_actor(actor_url)
-            library_data = library.get_library_data(actor.url)
-        except Exception as e:
-            raise ValidationError('Error while fetching actor and library')
-
-        library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance()
-        follow, created = models.Follow.objects.get_or_create(
-            actor=library_actor,
-            target=actor,
-        )
-        serializer = serializers.FollowSerializer(follow)
-        activity.deliver(
-            serializer.data,
-            on_behalf_of=library_actor,
-            to=[actor.url]
-        )
-        return response.Response({}, status=201)
+        serializer = serializers.APILibraryCreateSerializer(data=request.data)
+        serializer.is_valid(raise_exception=True)
+        library = serializer.save()
+        return response.Response(serializer.data, status=201)

api/tests/federation/test_serializers.py

@@ -619,3 +619,46 @@ def test_collection_serializer_to_ap(factories):
         collection, context={'actor': library, 'id': 'https://test.id'})
 
     assert serializer.data == expected
+
+
+def test_api_library_create_serializer_save(factories, r_mock):
+    library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance()
+    actor = factories['federation.Actor']()
+    follow = factories['federation.Follow'](
+        target=actor,
+        actor=library_actor,
+    )
+    actor_data = serializers.ActorSerializer(actor).data
+    actor_data['url'] = [{
+        'href': 'https://test.library',
+        'name': 'library',
+        'type': 'Link',
+    }]
+    library_conf = {
+        'id': 'https://test.library',
+        'items': range(10),
+        'actor': actor,
+        'page_size': 5,
+    }
+    library_data = serializers.PaginatedCollectionSerializer(library_conf).data
+    r_mock.get(actor.url, json=actor_data)
+    r_mock.get('https://test.library', json=library_data)
+    data = {
+        'actor': actor.url,
+        'autoimport': False,
+        'federation_enabled': True,
+        'download_files': False,
+    }
+
+    serializer = serializers.APILibraryCreateSerializer(data=data)
+    assert serializer.is_valid(raise_exception=True) is True
+    library = serializer.save()
+    follow = models.Follow.objects.get(
+        target=actor, actor=library_actor, approved=None)
+
+    assert library.autoimport is data['autoimport']
+    assert library.federation_enabled is data['federation_enabled']
+    assert library.download_files is data['download_files']
+    assert library.tracks_count == 10
+    assert library.actor == actor
+    assert library.follow == follow

api/tests/federation/test_views.py

@@ -4,6 +4,7 @@ from django.core.paginator import Paginator
 import pytest
 
 from funkwhale_api.federation import actors
+from funkwhale_api.federation import activity
 from funkwhale_api.federation import models
 from funkwhale_api.federation import serializers
 from funkwhale_api.federation import utils
@@ -182,22 +183,37 @@ def test_can_scan_library(superuser_api_client, mocker):
     scan.assert_called_once_with('test@test.library')
 
 
-def test_follow_library_manually(superuser_api_client, mocker, factories):
+def test_follow_library(superuser_api_client, mocker, factories, r_mock):
     library_actor = actors.SYSTEM_ACTORS['library'].get_actor_instance()
-    actor = factories['federation.Actor'](manually_approves_followers=True)
+    actor = factories['federation.Actor']()
     follow = {'test': 'follow'}
-    deliver = mocker.patch(
-        'funkwhale_api.federation.activity.deliver')
-    actor_get = mocker.patch(
-        'funkwhale_api.federation.actors.get_actor',
-        return_value=actor)
-    library_get = mocker.patch(
-        'funkwhale_api.federation.library.get_library_data',
-        return_value={})
+    on_commit = mocker.patch(
+        'funkwhale_api.common.utils.on_commit')
+    actor_data = serializers.ActorSerializer(actor).data
+    actor_data['url'] = [{
+        'href': 'https://test.library',
+        'name': 'library',
+        'type': 'Link',
+    }]
+    library_conf = {
+        'id': 'https://test.library',
+        'items': range(10),
+        'actor': actor,
+        'page_size': 5,
+    }
+    library_data = serializers.PaginatedCollectionSerializer(library_conf).data
+    r_mock.get(actor.url, json=actor_data)
+    r_mock.get('https://test.library', json=library_data)
+    data = {
+        'actor': actor.url,
+        'autoimport': False,
+        'federation_enabled': True,
+        'download_files': False,
+    }
 
     url = reverse('api:v1:federation:libraries-list')
     response = superuser_api_client.post(
-        url, {'actor_url': actor.url})
+        url, data)
 
     assert response.status_code == 201
 
@@ -206,8 +222,13 @@ def test_follow_library_manually(superuser_api_client, mocker, factories):
         target=actor,
         approved=None,
     )
+    library = follow.library
+
+    assert response.data == serializers.APILibraryCreateSerializer(
+        library).data
 
-    deliver.assert_called_once_with(
+    on_commit.assert_called_once_with(
+        activity.deliver,
         serializers.FollowSerializer(follow).data,
         on_behalf_of=library_actor,
         to=[actor.url]