From 6dcde77b1e33e10bfb6c30d80a39820fce32e352 Mon Sep 17 00:00:00 2001
From: Eliot Berriot <contact@eliotberriot.com>
Date: Sat, 28 Apr 2018 00:25:47 +0200
Subject: [PATCH] Avoid fetching Actor object on every request authentication
---
api/config/settings/common.py | 4 +++
api/funkwhale_api/federation/actors.py | 12 ++++++-
.../federation/authentication.py | 17 +++------
api/funkwhale_api/federation/serializers.py | 7 ++--
api/tests/federation/test_actors.py | 36 +++++++++++++++++++
changes/changelog.d/actor-fetch.enhancement | 1 +
.../src/components/federation/LibraryCard.vue | 2 ++
front/src/views/federation/LibraryDetail.vue | 1 +
8 files changed, 63 insertions(+), 17 deletions(-)
create mode 100644 changes/changelog.d/actor-fetch.enhancement
diff --git a/api/config/settings/common.py b/api/config/settings/common.py
index f1a383c5..2b9580b0 100644
--- a/api/config/settings/common.py
+++ b/api/config/settings/common.py
@@ -56,6 +56,10 @@ FEDERATION_COLLECTION_PAGE_SIZE = env.int(
FEDERATION_MUSIC_NEEDS_APPROVAL = env.bool(
'FEDERATION_MUSIC_NEEDS_APPROVAL', default=True
)
+# how much minutes to wait before refetching actor data
+# when authenticating
+FEDERATION_ACTOR_FETCH_DELAY = env.int(
+ 'FEDERATION_ACTOR_FETCH_DELAY', default=60 * 12)
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 380bb23c..a0e23d12 100644
--- a/api/funkwhale_api/federation/actors.py
+++ b/api/funkwhale_api/federation/actors.py
@@ -1,3 +1,4 @@
+import datetime
import logging
import uuid
import xml
@@ -49,11 +50,20 @@ def get_actor_data(actor_url):
def get_actor(actor_url):
+ try:
+ actor = models.Actor.objects.get(url=actor_url)
+ except models.Actor.DoesNotExist:
+ actor = None
+ fetch_delta = datetime.timedelta(
+ minutes=settings.FEDERATION_ACTOR_FETCH_DELAY)
+ if actor and actor.last_fetch_date > timezone.now() - fetch_delta:
+ # cache is hot, we can return as is
+ return actor
data = get_actor_data(actor_url)
serializer = serializers.ActorSerializer(data=data)
serializer.is_valid(raise_exception=True)
- return serializer.build()
+ return serializer.save(last_fetch_date=timezone.now())
class SystemActor(object):
diff --git a/api/funkwhale_api/federation/authentication.py b/api/funkwhale_api/federation/authentication.py
index 7f8ad665..bfd46084 100644
--- a/api/funkwhale_api/federation/authentication.py
+++ b/api/funkwhale_api/federation/authentication.py
@@ -25,28 +25,19 @@ class SignatureAuthentication(authentication.BaseAuthentication):
raise exceptions.AuthenticationFailed(str(e))
try:
- actor_data = actors.get_actor_data(key_id)
+ actor = actors.get_actor(key_id.split('#')[0])
except Exception as e:
raise exceptions.AuthenticationFailed(str(e))
- try:
- public_key = actor_data['publicKey']['publicKeyPem']
- except KeyError:
+ if not actor.public_key:
raise exceptions.AuthenticationFailed('No public key found')
- serializer = serializers.ActorSerializer(data=actor_data)
- if not serializer.is_valid():
- raise exceptions.AuthenticationFailed('Invalid actor payload: {}'.format(serializer.errors))
-
try:
- signing.verify_django(request, public_key.encode('utf-8'))
+ signing.verify_django(request, actor.public_key.encode('utf-8'))
except cryptography.exceptions.InvalidSignature:
raise exceptions.AuthenticationFailed('Invalid signature')
- try:
- return models.Actor.objects.get(url=actor_data['id'])
- except models.Actor.DoesNotExist:
- return serializer.save()
+ return actor
def authenticate(self, request):
setattr(request, 'actor', None)
diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py
index 38efdd3b..426aabd7 100644
--- a/api/funkwhale_api/federation/serializers.py
+++ b/api/funkwhale_api/federation/serializers.py
@@ -103,9 +103,10 @@ class ActorSerializer(serializers.Serializer):
def save(self, **kwargs):
d = self.prepare_missing_fields()
d.update(kwargs)
- return models.Actor.objects.create(
- **d
- )
+ return models.Actor.objects.update_or_create(
+ url=d['url'],
+ defaults=d,
+ )[0]
def validate_summary(self, value):
if value:
diff --git a/api/tests/federation/test_actors.py b/api/tests/federation/test_actors.py
index 7281147a..f566e7a7 100644
--- a/api/tests/federation/test_actors.py
+++ b/api/tests/federation/test_actors.py
@@ -29,6 +29,42 @@ def test_actor_fetching(r_mock):
assert r == payload
+def test_get_actor(factories, r_mock):
+ actor = factories['federation.Actor'].build()
+ payload = serializers.ActorSerializer(actor).data
+ r_mock.get(actor.url, json=payload)
+ new_actor = actors.get_actor(actor.url)
+
+ assert new_actor.pk is not None
+ assert serializers.ActorSerializer(new_actor).data == payload
+
+
+def test_get_actor_use_existing(factories, settings, mocker):
+ settings.FEDERATION_ACTOR_FETCH_DELAY = 60
+ actor = factories['federation.Actor']()
+ get_data = mocker.patch('funkwhale_api.federation.actors.get_actor_data')
+ new_actor = actors.get_actor(actor.url)
+
+ assert new_actor == actor
+ get_data.assert_not_called()
+
+
+def test_get_actor_refresh(factories, settings, mocker):
+ settings.FEDERATION_ACTOR_FETCH_DELAY = 0
+ actor = factories['federation.Actor']()
+ payload = serializers.ActorSerializer(actor).data
+ # actor changed their username in the meantime
+ payload['preferredUsername'] = 'New me'
+ get_data = mocker.patch(
+ 'funkwhale_api.federation.actors.get_actor_data',
+ return_value=payload)
+ new_actor = actors.get_actor(actor.url)
+
+ assert new_actor == actor
+ assert new_actor.last_fetch_date > actor.last_fetch_date
+ assert new_actor.preferred_username == 'New me'
+
+
def test_get_library(db, settings, mocker):
get_key_pair = mocker.patch(
'funkwhale_api.federation.keys.get_key_pair',
diff --git a/changes/changelog.d/actor-fetch.enhancement b/changes/changelog.d/actor-fetch.enhancement
new file mode 100644
index 00000000..17f3a88d
--- /dev/null
+++ b/changes/changelog.d/actor-fetch.enhancement
@@ -0,0 +1 @@
+Avoid fetching Actor object on every request authentication
diff --git a/front/src/components/federation/LibraryCard.vue b/front/src/components/federation/LibraryCard.vue
index a16c80f7..e7ef7a51 100644
--- a/front/src/components/federation/LibraryCard.vue
+++ b/front/src/components/federation/LibraryCard.vue
@@ -3,10 +3,12 @@
<div class="content">
<div class="header ellipsis">
<router-link
+ v-if="library"
:title="displayName"
:to="{name: 'federation.libraries.detail', params: {id: library.uuid }}">
{{ displayName }}
</router-link>
+ <span :title="displayName" v-else>{{ displayName }}</span>
</div>
</div>
<div class="content">
diff --git a/front/src/views/federation/LibraryDetail.vue b/front/src/views/federation/LibraryDetail.vue
index 64f2cc7c..7a842b67 100644
--- a/front/src/views/federation/LibraryDetail.vue
+++ b/front/src/views/federation/LibraryDetail.vue
@@ -150,6 +150,7 @@ export default {
methods: {
fetchData () {
var self = this
+ this.scanTrigerred = false
this.isLoading = true
let url = 'federation/libraries/' + this.id + '/'
logger.default.debug('Fetching library "' + this.id + '"')
--
GitLab