From fb881dd5defb75298fc09fd83e2ac6d151bf8821 Mon Sep 17 00:00:00 2001 From: Andrew Godwin Date: Sun, 8 Jan 2023 23:06:09 -0700 Subject: [PATCH] Fix tests and most of pagination --- api/pagination.py | 84 ++++++++++++++------------- api/views/accounts.py | 5 +- api/views/notifications.py | 2 +- api/views/statuses.py | 9 +-- api/views/timelines.py | 9 ++- tests/users/views/test_activitypub.py | 14 +++-- users/views/activitypub.py | 2 +- 7 files changed, 65 insertions(+), 60 deletions(-) diff --git a/api/pagination.py b/api/pagination.py index d54cdb3..7584e1d 100644 --- a/api/pagination.py +++ b/api/pagination.py @@ -117,36 +117,17 @@ class PaginationResult: class MastodonPaginator: """ Paginates in the Mastodon style (max_id, min_id, etc). + Note that this basically _requires_ us to always do it on IDs, so we do. """ def __init__( self, - anchor_model: type[models.Model], - sort_attribute: str = "created", default_limit: int = 20, max_limit: int = 40, ): - self.anchor_model = anchor_model - self.sort_attribute = sort_attribute self.default_limit = default_limit self.max_limit = max_limit - def get_anchor(self, anchor_id: str): - """ - Gets an anchor object by ID. - It's possible that the anchor object might be an interaction, in which - case we recurse down to its post. - """ - if anchor_id.startswith("interaction-"): - try: - return PostInteraction.objects.get(pk=anchor_id[12:]) - except PostInteraction.DoesNotExist: - return None - try: - return self.anchor_model.objects.get(pk=anchor_id) - except self.anchor_model.DoesNotExist: - return None - def paginate( self, queryset, @@ -156,32 +137,57 @@ class MastodonPaginator: limit: int | None, ) -> PaginationResult: if max_id: - anchor = self.get_anchor(max_id) - if anchor is None: - return PaginationResult.empty() - queryset = queryset.filter( - **{self.sort_attribute + "__lt": getattr(anchor, self.sort_attribute)} - ) + queryset = queryset.filter(id__lt=max_id) if since_id: - anchor = self.get_anchor(since_id) - if anchor is None: - return PaginationResult.empty() - queryset = queryset.filter( - **{self.sort_attribute + "__gt": getattr(anchor, self.sort_attribute)} - ) + queryset = queryset.filter(id__gt=since_id) if min_id: # Min ID requires items _immediately_ newer than specified, so we # invert the ordering to accommodate - anchor = self.get_anchor(min_id) - if anchor is None: - return PaginationResult.empty() - queryset = queryset.filter( - **{self.sort_attribute + "__gt": getattr(anchor, self.sort_attribute)} - ).order_by(self.sort_attribute) + queryset = queryset.filter(id__gt=min_id).order_by("id") else: - queryset = queryset.order_by("-" + self.sort_attribute) + queryset = queryset.order_by("-id") + + limit = min(limit or self.default_limit, self.max_limit) + return PaginationResult( + results=list(queryset[:limit]), + limit=limit, + ) + + def paginate_home( + self, + queryset, + min_id: str | None, + max_id: str | None, + since_id: str | None, + limit: int | None, + ) -> PaginationResult: + """ + The home timeline requires special handling where we mix Posts and + PostInteractions together. + """ + if max_id: + queryset = queryset.filter( + models.Q(subject_post_id__lt=max_id) + | models.Q(subject_post_interaction_id__lt=max_id) + ) + + if since_id: + queryset = queryset.filter( + models.Q(subject_post_id__gt=max_id) + | models.Q(subject_post_interaction_id__gt=max_id) + ) + + if min_id: + # Min ID requires items _immediately_ newer than specified, so we + # invert the ordering to accommodate + queryset = queryset.filter( + models.Q(subject_post_id__gt=max_id) + | models.Q(subject_post_interaction_id__gt=max_id) + ).order_by("id") + else: + queryset = queryset.order_by("-id") limit = min(limit or self.default_limit, self.max_limit) return PaginationResult( diff --git a/api/views/accounts.py b/api/views/accounts.py index 4309461..b77ce09 100644 --- a/api/views/accounts.py +++ b/api/views/accounts.py @@ -3,7 +3,6 @@ from django.http import HttpRequest, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404 from ninja import Field -from activities.models import Post from activities.services import SearchService from api import schemas from api.decorators import identity_required @@ -151,7 +150,7 @@ def account_statuses( if tagged: queryset = queryset.tagged_with(tagged) # Get user posts with pagination - paginator = MastodonPaginator(Post, sort_attribute="published") + paginator = MastodonPaginator() pager = paginator.paginate( queryset, min_id=min_id, @@ -219,7 +218,7 @@ def account_following( service = IdentityService(identity) - paginator = MastodonPaginator(Identity, max_limit=80, sort_attribute="username") + paginator = MastodonPaginator(max_limit=80) pager = paginator.paginate( service.following(), min_id=min_id, diff --git a/api/views/notifications.py b/api/views/notifications.py index f8e0635..9a6a06f 100644 --- a/api/views/notifications.py +++ b/api/views/notifications.py @@ -35,7 +35,7 @@ def notifications( queryset = TimelineService(request.identity).notifications( [base_types[r] for r in requested_types if r in base_types] ) - paginator = MastodonPaginator(TimelineEvent) + paginator = MastodonPaginator() pager = paginator.paginate( queryset, min_id=min_id, diff --git a/api/views/statuses.py b/api/views/statuses.py index 9a41eb4..566cbea 100644 --- a/api/views/statuses.py +++ b/api/views/statuses.py @@ -16,7 +16,6 @@ from activities.services import PostService from api import schemas from api.views.base import api_router from core.models import Config -from users.models import Identity from ..decorators import identity_required from ..pagination import MastodonPaginator @@ -142,20 +141,18 @@ def favourited_by( # a concept of "private status" yet. post = get_object_or_404(Post, pk=id) - paginator = MastodonPaginator(Identity, sort_attribute="published") + paginator = MastodonPaginator() pager = paginator.paginate( post.interactions.filter( type=PostInteraction.Types.like, state__in=PostInteractionStates.group_active(), - ) - .select_related("identity") - .order_by("published"), + ).select_related("identity"), min_id=min_id, max_id=max_id, since_id=since_id, limit=limit, ) - pager.jsonify_identities() + pager.jsonify_results(lambda r: r.identity.to_mastodon_json(include_counts=False)) if pager.results: response.headers["Link"] = pager.link_header( diff --git a/api/views/timelines.py b/api/views/timelines.py index 4367f0d..01767d9 100644 --- a/api/views/timelines.py +++ b/api/views/timelines.py @@ -1,6 +1,5 @@ from django.http import HttpRequest, HttpResponse, JsonResponse -from activities.models import Post from activities.services import TimelineService from api import schemas from api.decorators import identity_required @@ -20,9 +19,9 @@ def home( limit: int = 20, ): # Grab a paginated result set of instances - paginator = MastodonPaginator(Post, sort_attribute="published") + paginator = MastodonPaginator() queryset = TimelineService(request.identity).home() - pager = paginator.paginate( + pager = paginator.paginate_home( queryset, min_id=min_id, max_id=max_id, @@ -61,7 +60,7 @@ def public( if only_media: queryset = queryset.filter(attachments__id__isnull=True) # Grab a paginated result set of instances - paginator = MastodonPaginator(Post, sort_attribute="published") + paginator = MastodonPaginator() pager = paginator.paginate( queryset, min_id=min_id, @@ -101,7 +100,7 @@ def hashtag( if only_media: queryset = queryset.filter(attachments__id__isnull=True) # Grab a paginated result set of instances - paginator = MastodonPaginator(Post, sort_attribute="published") + paginator = MastodonPaginator() pager = paginator.paginate( queryset, min_id=min_id, diff --git a/tests/users/views/test_activitypub.py b/tests/users/views/test_activitypub.py index 9bde51f..c811e52 100644 --- a/tests/users/views/test_activitypub.py +++ b/tests/users/views/test_activitypub.py @@ -38,15 +38,18 @@ def test_webfinger_system_actor(client): @pytest.mark.django_db -def test_delete_actor(client, identity): +def test_delete_unknown_actor(client, identity): + """ + Tests that unknown actor delete messages are dropped + """ data = { "@context": "https://www.w3.org/ns/activitystreams", - "actor": "https://mastodon.social/users/fakec8b6984105c8f15070a2", - "id": "https://mastodon.social/users/fakec8b6984105c8f15070a2#delete", - "object": "https://mastodon.social/users/fakec8b6984105c8f15070a2", + "actor": "https://mastodon.test/users/fakec8b6984105c8f15070a2", + "id": "https://mastodon.test/users/fakec8b6984105c8f15070a2#delete", + "object": "https://mastodon.test/users/fakec8b6984105c8f15070a2", "signature": { "created": "2022-12-06T03:54:28Z", - "creator": "https://mastodon.social/users/fakec8b6984105c8f15070a2#main-key", + "creator": "https://mastodon.test/users/fakec8b6984105c8f15070a2#main-key", "signatureValue": "This value doesn't matter", "type": "RsaSignature2017", }, @@ -56,4 +59,5 @@ def test_delete_actor(client, identity): resp = client.post( identity.inbox_uri, data=data, content_type="application/activity+json" ) + print(resp.content) assert resp.status_code == 202 diff --git a/users/views/activitypub.py b/users/views/activitypub.py index 1842f67..5f56a93 100644 --- a/users/views/activitypub.py +++ b/users/views/activitypub.py @@ -132,7 +132,7 @@ class Inbox(View): if ( document["type"] == "Delete" and document["actor"] == document["object"] - and not identity.pk + and identity._state.adding ): # We don't have an Identity record for the user. No-op exceptions.capture_message(