From 7ac3bb98dab4c2862bc2a1d19af7ddf1df38c59c Mon Sep 17 00:00:00 2001 From: Eliot Berriot Date: Wed, 5 Dec 2018 12:13:37 +0100 Subject: [PATCH] Moved actor domain to a dedicated table --- api/config/settings/common.py | 4 +- api/funkwhale_api/federation/admin.py | 6 ++ api/funkwhale_api/federation/factories.py | 29 ++++++++-- .../migrations/0013_auto_20181226_1935.py | 23 ++++++++ .../migrations/0014_auto_20181205_0958.py | 46 +++++++++++++++ .../migrations/0015_populate_domains.py | 56 +++++++++++++++++++ api/funkwhale_api/federation/models.py | 48 +++++++--------- api/funkwhale_api/federation/serializers.py | 3 +- api/funkwhale_api/users/models.py | 4 +- api/tests/conftest.py | 10 ++++ api/tests/federation/test_migrations.py | 34 +++++++++++ api/tests/federation/test_models.py | 13 +++++ api/tests/federation/test_serializers.py | 10 ++-- api/tests/users/test_models.py | 2 +- 14 files changed, 244 insertions(+), 44 deletions(-) create mode 100644 api/funkwhale_api/federation/migrations/0013_auto_20181226_1935.py create mode 100644 api/funkwhale_api/federation/migrations/0014_auto_20181205_0958.py create mode 100644 api/funkwhale_api/federation/migrations/0015_populate_domains.py create mode 100644 api/tests/federation/test_migrations.py diff --git a/api/config/settings/common.py b/api/config/settings/common.py index 51230bfdc..97a088338 100644 --- a/api/config/settings/common.py +++ b/api/config/settings/common.py @@ -69,6 +69,8 @@ else: FUNKWHALE_HOSTNAME = _parsed.netloc FUNKWHALE_PROTOCOL = _parsed.scheme +FUNKWHALE_PROTOCOL = FUNKWHALE_PROTOCOL.lower() +FUNKWHALE_HOSTNAME = FUNKWHALE_HOSTNAME.lower() FUNKWHALE_URL = "{}://{}".format(FUNKWHALE_PROTOCOL, FUNKWHALE_HOSTNAME) FUNKWHALE_SPA_HTML_ROOT = env( "FUNKWHALE_SPA_HTML_ROOT", default=FUNKWHALE_URL + "/front/" @@ -83,7 +85,7 @@ APP_NAME = "Funkwhale" # XXX: deprecated, see #186 FEDERATION_ENABLED = env.bool("FEDERATION_ENABLED", default=True) -FEDERATION_HOSTNAME = env("FEDERATION_HOSTNAME", default=FUNKWHALE_HOSTNAME) +FEDERATION_HOSTNAME = env("FEDERATION_HOSTNAME", default=FUNKWHALE_HOSTNAME).lower() # XXX: deprecated, see #186 FEDERATION_COLLECTION_PAGE_SIZE = env.int("FEDERATION_COLLECTION_PAGE_SIZE", default=50) # XXX: deprecated, see #186 diff --git a/api/funkwhale_api/federation/admin.py b/api/funkwhale_api/federation/admin.py index 98bc65247..acb2e5b67 100644 --- a/api/funkwhale_api/federation/admin.py +++ b/api/funkwhale_api/federation/admin.py @@ -24,6 +24,12 @@ def redeliver_activities(modeladmin, request, queryset): redeliver_activities.short_description = "Redeliver" +@admin.register(models.Domain) +class DomainAdmin(admin.ModelAdmin): + list_display = ["name", "creation_date"] + search_fields = ["name"] + + @admin.register(models.Activity) class ActivityAdmin(admin.ModelAdmin): list_display = ["type", "fid", "url", "actor", "creation_date"] diff --git a/api/funkwhale_api/federation/factories.py b/api/funkwhale_api/federation/factories.py index a52cf88be..cbe0bee85 100644 --- a/api/funkwhale_api/federation/factories.py +++ b/api/funkwhale_api/federation/factories.py @@ -66,24 +66,39 @@ def create_user(actor): return user_factories.UserFactory(actor=actor) +@registry.register +class Domain(factory.django.DjangoModelFactory): + name = factory.Faker("domain_name") + + class Meta: + model = "federation.Domain" + django_get_or_create = ("name",) + + @registry.register class ActorFactory(factory.DjangoModelFactory): public_key = None private_key = None preferred_username = factory.Faker("user_name") summary = factory.Faker("paragraph") - domain = factory.Faker("domain_name") + domain = factory.SubFactory(Domain) fid = factory.LazyAttribute( - lambda o: "https://{}/users/{}".format(o.domain, o.preferred_username) + lambda o: "https://{}/users/{}".format(o.domain.name, o.preferred_username) ) followers_url = factory.LazyAttribute( - lambda o: "https://{}/users/{}followers".format(o.domain, o.preferred_username) + lambda o: "https://{}/users/{}followers".format( + o.domain.name, o.preferred_username + ) ) inbox_url = factory.LazyAttribute( - lambda o: "https://{}/users/{}/inbox".format(o.domain, o.preferred_username) + lambda o: "https://{}/users/{}/inbox".format( + o.domain.name, o.preferred_username + ) ) outbox_url = factory.LazyAttribute( - lambda o: "https://{}/users/{}/outbox".format(o.domain, o.preferred_username) + lambda o: "https://{}/users/{}/outbox".format( + o.domain.name, o.preferred_username + ) ) class Meta: @@ -95,7 +110,9 @@ class ActorFactory(factory.DjangoModelFactory): return from funkwhale_api.users.factories import UserFactory - self.domain = settings.FEDERATION_HOSTNAME + self.domain = models.Domain.objects.get_or_create( + name=settings.FEDERATION_HOSTNAME + )[0] self.save(update_fields=["domain"]) if not create: if extracted and hasattr(extracted, "pk"): diff --git a/api/funkwhale_api/federation/migrations/0013_auto_20181226_1935.py b/api/funkwhale_api/federation/migrations/0013_auto_20181226_1935.py new file mode 100644 index 000000000..98a481271 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0013_auto_20181226_1935.py @@ -0,0 +1,23 @@ +# Generated by Django 2.0.9 on 2018-12-26 19:35 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [("federation", "0012_auto_20180920_1803")] + + operations = [ + migrations.AlterField( + model_name="actor", + name="private_key", + field=models.TextField(blank=True, max_length=5000, null=True), + ), + migrations.AlterField( + model_name="actor", + name="public_key", + field=models.TextField(blank=True, max_length=5000, null=True), + ), + ] diff --git a/api/funkwhale_api/federation/migrations/0014_auto_20181205_0958.py b/api/funkwhale_api/federation/migrations/0014_auto_20181205_0958.py new file mode 100644 index 000000000..7be361f87 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0014_auto_20181205_0958.py @@ -0,0 +1,46 @@ +# Generated by Django 2.0.9 on 2018-12-05 09:58 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +class Migration(migrations.Migration): + + dependencies = [("federation", "0013_auto_20181226_1935")] + + operations = [ + migrations.CreateModel( + name="Domain", + fields=[ + ( + "name", + models.CharField(max_length=255, primary_key=True, serialize=False), + ), + ( + "creation_date", + models.DateTimeField(default=django.utils.timezone.now), + ), + ], + ), + migrations.AlterField( + model_name="actor", + name="domain", + field=models.CharField(max_length=1000, null=True), + ), + migrations.RenameField("actor", "domain", "old_domain"), + migrations.AddField( + model_name="actor", + name="domain", + field=models.ForeignKey( + null=True, + on_delete=django.db.models.deletion.CASCADE, + related_name="actors", + to="federation.Domain", + ), + ), + migrations.AlterUniqueTogether(name="actor", unique_together=set()), + migrations.AlterUniqueTogether( + name="actor", unique_together={("domain", "preferred_username")} + ), + ] diff --git a/api/funkwhale_api/federation/migrations/0015_populate_domains.py b/api/funkwhale_api/federation/migrations/0015_populate_domains.py new file mode 100644 index 000000000..0f0036c94 --- /dev/null +++ b/api/funkwhale_api/federation/migrations/0015_populate_domains.py @@ -0,0 +1,56 @@ +# Generated by Django 2.0.9 on 2018-11-14 08:55 + +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone + + +def populate_domains(apps, schema_editor): + Domain = apps.get_model("federation", "Domain") + Actor = apps.get_model("federation", "Actor") + + domains = set( + [v.lower() for v in Actor.objects.values_list("old_domain", flat=True)] + ) + for domain in sorted(domains): + print("Populating domain {}...".format(domain)) + first_actor = ( + Actor.objects.order_by("creation_date") + .exclude(creation_date=None) + .filter(old_domain__iexact=domain) + .first() + ) + + if first_actor: + first_seen = first_actor.creation_date + else: + first_seen = django.utils.timezone.now() + + Domain.objects.update_or_create( + name=domain, defaults={"creation_date": first_seen} + ) + + for domain in Domain.objects.all(): + Actor.objects.filter(old_domain__iexact=domain.name).update(domain=domain) + + +def skip(apps, schema_editor): + pass + + +class Migration(migrations.Migration): + + dependencies = [("federation", "0014_auto_20181205_0958")] + + operations = [ + migrations.RunPython(populate_domains, skip), + migrations.AlterField( + model_name="actor", + name="domain", + field=models.ForeignKey( + on_delete=django.db.models.deletion.CASCADE, + related_name="actors", + to="federation.Domain", + ), + ), + ] diff --git a/api/funkwhale_api/federation/models.py b/api/funkwhale_api/federation/models.py index 058bb9c46..7a450e4f5 100644 --- a/api/funkwhale_api/federation/models.py +++ b/api/funkwhale_api/federation/models.py @@ -62,6 +62,23 @@ class ActorQuerySet(models.QuerySet): return qs +class Domain(models.Model): + name = models.CharField(primary_key=True, max_length=255) + creation_date = models.DateTimeField(default=timezone.now) + + def __str__(self): + return self.name + + def save(self, **kwargs): + lowercase_fields = ["name"] + for field in lowercase_fields: + v = getattr(self, field, None) + if v: + setattr(self, field, v.lower()) + + super().save(**kwargs) + + class Actor(models.Model): ap_type = "Actor" @@ -74,7 +91,7 @@ class Actor(models.Model): shared_inbox_url = models.URLField(max_length=500, null=True, blank=True) type = models.CharField(choices=TYPE_CHOICES, default="Person", max_length=25) name = models.CharField(max_length=200, null=True, blank=True) - domain = models.CharField(max_length=1000) + domain = models.ForeignKey(Domain, on_delete=models.CASCADE, related_name="actors") summary = models.CharField(max_length=500, null=True, blank=True) preferred_username = models.CharField(max_length=200, null=True, blank=True) public_key = models.TextField(max_length=5000, null=True, blank=True) @@ -110,36 +127,9 @@ class Actor(models.Model): def __str__(self): return "{}@{}".format(self.preferred_username, self.domain) - def save(self, **kwargs): - lowercase_fields = ["domain"] - for field in lowercase_fields: - v = getattr(self, field, None) - if v: - setattr(self, field, v.lower()) - - super().save(**kwargs) - @property def is_local(self): - return self.domain == settings.FEDERATION_HOSTNAME - - @property - def is_system(self): - from . import actors - - return all( - [ - 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] + return self.domain_id == settings.FEDERATION_HOSTNAME def get_approved_followers(self): follows = self.received_follows.filter(approved=True) diff --git a/api/funkwhale_api/federation/serializers.py b/api/funkwhale_api/federation/serializers.py index 6c4ffeb58..76ab5ba86 100644 --- a/api/funkwhale_api/federation/serializers.py +++ b/api/funkwhale_api/federation/serializers.py @@ -114,7 +114,8 @@ class ActorSerializer(serializers.Serializer): if maf is not None: kwargs["manually_approves_followers"] = maf domain = urllib.parse.urlparse(kwargs["fid"]).netloc - kwargs["domain"] = domain + kwargs["domain"] = models.Domain.objects.get_or_create( + pk=domain)[0] for endpoint, url in self.initial_data.get("endpoints", {}).items(): if endpoint == "sharedInbox": kwargs["shared_inbox_url"] = url diff --git a/api/funkwhale_api/users/models.py b/api/funkwhale_api/users/models.py index 2bc87588e..efd02407b 100644 --- a/api/funkwhale_api/users/models.py +++ b/api/funkwhale_api/users/models.py @@ -252,7 +252,9 @@ def get_actor_data(user): username = federation_utils.slugify_username(user.username) return { "preferred_username": username, - "domain": settings.FEDERATION_HOSTNAME, + "domain": federation_models.Domain.objects.get_or_create( + name=settings.FEDERATION_HOSTNAME + )[0], "type": "Person", "name": user.username, "manually_approves_followers": False, diff --git a/api/tests/conftest.py b/api/tests/conftest.py index 99317303c..22d8f7eba 100644 --- a/api/tests/conftest.py +++ b/api/tests/conftest.py @@ -12,12 +12,16 @@ from faker.providers import internet as internet_provider import factory import pytest +from django.core.management import call_command from django.contrib.auth.models import AnonymousUser from django.core.cache import cache as django_cache, caches from django.core.files import uploadedfile from django.utils import timezone from django.test import client +from django.db import connection +from django.db.migrations.executor import MigrationExecutor from django.db.models import QuerySet + from dynamic_preferences.registries import global_preferences_registry from rest_framework import fields as rest_fields from rest_framework.test import APIClient, APIRequestFactory @@ -400,3 +404,9 @@ def spa_html(r_mock, settings): @pytest.fixture def no_api_auth(preferences): preferences["common__api_authentication_required"] = False + + +@pytest.fixture() +def migrator(transactional_db): + yield MigrationExecutor(connection) + call_command("migrate", interactive=False) diff --git a/api/tests/federation/test_migrations.py b/api/tests/federation/test_migrations.py new file mode 100644 index 000000000..4a9ce4274 --- /dev/null +++ b/api/tests/federation/test_migrations.py @@ -0,0 +1,34 @@ +def test_domain_14_migration(migrator): + a, f, t = ("federation", "0014_auto_20181205_0958", "0015_populate_domains") + + migrator.migrate([(a, f)]) + old_apps = migrator.loader.project_state([(a, f)]).apps + Actor = old_apps.get_model(a, "Actor") + a1 = Actor.objects.create( + fid="http://test1.com", preferred_username="test1", old_domain="dOmaiN1.com" + ) + a2 = Actor.objects.create( + fid="http://test2.com", preferred_username="test2", old_domain="domain1.com" + ) + a3 = Actor.objects.create( + fid="http://test3.com", preferred_username="test2", old_domain="domain2.com" + ) + + migrator.loader.build_graph() + migrator.migrate([(a, t)]) + new_apps = migrator.loader.project_state([(a, t)]).apps + + Actor = new_apps.get_model(a, "Actor") + Domain = new_apps.get_model(a, "Domain") + + a1 = Actor.objects.get(pk=a1.pk) + a2 = Actor.objects.get(pk=a2.pk) + a3 = Actor.objects.get(pk=a3.pk) + + assert Domain.objects.count() == 2 + assert a1.domain == Domain.objects.get(pk="domain1.com") + assert a2.domain == Domain.objects.get(pk="domain1.com") + assert a3.domain == Domain.objects.get(pk="domain2.com") + + assert Domain.objects.get(pk="domain1.com").creation_date == a1.creation_date + assert Domain.objects.get(pk="domain2.com").creation_date == a3.creation_date diff --git a/api/tests/federation/test_models.py b/api/tests/federation/test_models.py index 4a6131934..18966430e 100644 --- a/api/tests/federation/test_models.py +++ b/api/tests/federation/test_models.py @@ -54,3 +54,16 @@ def test_actor_get_quota(factories): expected = {"total": 10, "pending": 1, "skipped": 2, "errored": 3, "finished": 4} assert library.actor.get_current_usage() == expected + + +@pytest.mark.parametrize( + "value, expected", + [ + ("Domain.com", "domain.com"), + ("hello-WORLD.com", "hello-world.com"), + ("posés.com", "posés.com"), + ], +) +def test_domain_name_saved_properly(value, expected, factories): + domain = factories["federation.Domain"](name=value) + assert domain.name == expected diff --git a/api/tests/federation/test_serializers.py b/api/tests/federation/test_serializers.py index fe0485b52..fb151b2d7 100644 --- a/api/tests/federation/test_serializers.py +++ b/api/tests/federation/test_serializers.py @@ -43,7 +43,7 @@ def test_actor_serializer_from_ap(db): assert actor.public_key == payload["publicKey"]["publicKeyPem"] assert actor.preferred_username == payload["preferredUsername"] assert actor.name == payload["name"] - assert actor.domain == "test.federation" + assert actor.domain.pk == "test.federation" assert actor.summary == payload["summary"] assert actor.type == "Person" assert actor.manually_approves_followers == payload["manuallyApprovesFollowers"] @@ -71,7 +71,7 @@ def test_actor_serializer_only_mandatory_field_from_ap(db): assert actor.followers_url == payload["followers"] assert actor.following_url == payload["following"] assert actor.preferred_username == payload["preferredUsername"] - assert actor.domain == "test.federation" + assert actor.domain.pk == "test.federation" assert actor.type == "Person" assert actor.manually_approves_followers is None @@ -110,7 +110,7 @@ def test_actor_serializer_to_ap(): public_key=expected["publicKey"]["publicKeyPem"], preferred_username=expected["preferredUsername"], name=expected["name"], - domain="test.federation", + domain=models.Domain(pk="test.federation"), summary=expected["summary"], type="Person", manually_approves_followers=False, @@ -135,7 +135,7 @@ def test_webfinger_serializer(): actor = models.Actor( fid=expected["links"][0]["href"], preferred_username="service", - domain="test.federation", + domain=models.Domain(pk="test.federation"), ) serializer = serializers.ActorWebfingerSerializer(actor) @@ -898,7 +898,7 @@ def test_local_actor_serializer_to_ap(factories): public_key=expected["publicKey"]["publicKeyPem"], preferred_username=expected["preferredUsername"], name=expected["name"], - domain="test.federation", + domain=models.Domain.objects.create(pk="test.federation"), summary=expected["summary"], type="Person", manually_approves_followers=False, diff --git a/api/tests/users/test_models.py b/api/tests/users/test_models.py index 69d338828..8e4ebea97 100644 --- a/api/tests/users/test_models.py +++ b/api/tests/users/test_models.py @@ -137,7 +137,7 @@ def test_creating_actor_from_user(factories, settings): actor = models.create_actor(user) assert actor.preferred_username == "Hello_M_world" # slugified - assert actor.domain == settings.FEDERATION_HOSTNAME + assert actor.domain.pk == settings.FEDERATION_HOSTNAME assert actor.type == "Person" assert actor.name == user.username assert actor.manually_approves_followers is False