From 6d933863d2dd13d21e963ff323af473442ba8f84 Mon Sep 17 00:00:00 2001 From: Thomas Sileo Date: Tue, 30 Aug 2022 20:05:10 +0200 Subject: [PATCH] Fix outbox delete side effects --- app/boxes.py | 13 ++++++++++ app/models.py | 6 ++--- tests/factories.py | 5 ++-- tests/test_outbox.py | 61 ++++++++++++++++++++++++++++++++++++++++++++ tests/utils.py | 47 ++++++++++++++++++++++++++++++++++ 5 files changed, 127 insertions(+), 5 deletions(-) diff --git a/app/boxes.py b/app/boxes.py index 9addc7d..977e324 100644 --- a/app/boxes.py +++ b/app/boxes.py @@ -122,6 +122,19 @@ async def send_delete(db_session: AsyncSession, ap_object_id: str) -> None: for rcp in recipients: await new_outgoing_activity(db_session, rcp, outbox_object.id) + # Revert side effects + if outbox_object_to_delete.in_reply_to: + replied_object = await get_anybox_object_by_ap_id( + db_session, outbox_object_to_delete.in_reply_to + ) + if replied_object: + replied_object.replies_count = replied_object.replies_count - 1 + if replied_object.replies_count < 0: + logger.warning("negative replies count for {replied_object.ap_id}") + replied_object.replies_count = 0 + else: + logger.info(f"{outbox_object_to_delete.in_reply_to} not found") + await db_session.commit() diff --git a/app/models.py b/app/models.py index fc551ba..036ef85 100644 --- a/app/models.py +++ b/app/models.py @@ -45,7 +45,7 @@ class Actor(Base, BaseActor): created_at = Column(DateTime(timezone=True), nullable=False, default=now) updated_at = Column(DateTime(timezone=True), nullable=False, default=now) - ap_id = Column(String, unique=True, nullable=False, index=True) + ap_id: Mapped[str] = Column(String, unique=True, nullable=False, index=True) ap_actor: Mapped[ap.RawObject] = Column(JSON, nullable=False) ap_type = Column(String, nullable=False) @@ -126,7 +126,7 @@ class InboxObject(Base, BaseObject): is_deleted = Column(Boolean, nullable=False, default=False) is_transient = Column(Boolean, nullable=False, default=False, server_default="0") - replies_count = Column(Integer, nullable=False, default=0) + replies_count: Mapped[int] = Column(Integer, nullable=False, default=0) og_meta: Mapped[list[dict[str, Any]] | None] = Column(JSON, nullable=True) @@ -176,7 +176,7 @@ class OutboxObject(Base, BaseObject): likes_count = Column(Integer, nullable=False, default=0) announces_count = Column(Integer, nullable=False, default=0) - replies_count = Column(Integer, nullable=False, default=0) + replies_count: Mapped[int] = Column(Integer, nullable=False, default=0) webmentions_count: Mapped[int] = Column( Integer, nullable=False, default=0, server_default="0" ) diff --git a/tests/factories.py b/tests/factories.py index a311472..d2d4e3f 100644 --- a/tests/factories.py +++ b/tests/factories.py @@ -84,12 +84,13 @@ def build_move_activity( def build_note_object( - from_remote_actor: actor.RemoteActor, + from_remote_actor: actor.RemoteActor | models.Actor, outbox_public_id: str | None = None, content: str = "Hello", to: list[str] = None, cc: list[str] = None, tags: list[ap.RawObject] = None, + in_reply_to: str | None = None, ) -> ap.RawObject: published = now().replace(microsecond=0).isoformat().replace("+00:00", "Z") context = from_remote_actor.ap_id + "/ctx/" + uuid4().hex @@ -108,8 +109,8 @@ def build_note_object( "url": from_remote_actor.ap_id + "/note/" + note_id, "tag": tags or [], "summary": None, - "inReplyTo": None, "sensitive": False, + "inReplyTo": in_reply_to, } diff --git a/tests/test_outbox.py b/tests/test_outbox.py index e3b3e94..9a3bef7 100644 --- a/tests/test_outbox.py +++ b/tests/test_outbox.py @@ -2,13 +2,17 @@ from unittest import mock import respx from fastapi.testclient import TestClient +from sqlalchemy import select from sqlalchemy.orm import Session from app import activitypub as ap from app import models from app import webfinger +from app.actor import LOCAL_ACTOR from app.config import generate_csrf_token from tests.utils import generate_admin_session_cookies +from tests.utils import setup_inbox_note +from tests.utils import setup_outbox_note from tests.utils import setup_remote_actor from tests.utils import setup_remote_actor_as_follower @@ -59,6 +63,63 @@ def test_send_follow_request( assert outgoing_activity.recipient == ra.inbox_url +def test_send_delete__reverts_side_effects( + db: Session, + client: TestClient, + respx_mock: respx.MockRouter, +) -> None: + # given a remote actor + ra = setup_remote_actor(respx_mock) + + # who is a follower + follower = setup_remote_actor_as_follower(ra) + actor = follower.actor + + # with a note that has existing replies + inbox_note = setup_inbox_note(actor) + inbox_note.replies_count = 1 + db.commit() + + # and a local reply + outbox_note = setup_outbox_note( + to=[ap.AS_PUBLIC], + cc=[LOCAL_ACTOR.followers_collection_id], # type: ignore + in_reply_to=inbox_note.ap_id, + ) + inbox_note.replies_count = inbox_note.replies_count + 1 + db.commit() + + response = client.post( + "/admin/actions/delete", + data={ + "redirect_url": "http://testserver/", + "ap_object_id": outbox_note.ap_id, + "csrf_token": generate_csrf_token(), + }, + cookies=generate_admin_session_cookies(), + ) + + # Then the server returns a 302 + assert response.status_code == 302 + assert response.headers.get("Location") == "http://testserver/" + + # And the Delete activity was created in the outbox + outbox_object = db.execute( + select(models.OutboxObject).where(models.OutboxObject.ap_type == "Delete") + ).scalar_one() + assert outbox_object.ap_type == "Delete" + assert outbox_object.activity_object_ap_id == outbox_note.ap_id + + # And an outgoing activity was queued + outgoing_activity = db.query(models.OutgoingActivity).one() + assert outgoing_activity.outbox_object_id == outbox_object.id + assert outgoing_activity.recipient == ra.inbox_url + + # And the replies count of the replied object was decremented + db.refresh(inbox_note) + assert inbox_note.replies_count == 1 + + def test_send_create_activity__no_followers_and_with_mention( db: Session, client: TestClient, diff --git a/tests/utils.py b/tests/utils.py index 69787e0..4e9cb05 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -169,6 +169,53 @@ def setup_remote_actor_as_following_and_follower( return following, follower +def setup_outbox_note( + content: str = "Hello", + to: list[str] = None, + cc: list[str] = None, + tags: list[ap.RawObject] = None, + in_reply_to: str | None = None, +) -> models.OutboxObject: + note_id = uuid4().hex + note_from_outbox = RemoteObject( + factories.build_note_object( + from_remote_actor=LOCAL_ACTOR, + outbox_public_id=note_id, + content=content, + to=to, + cc=cc, + tags=tags, + in_reply_to=in_reply_to, + ), + LOCAL_ACTOR, + ) + return factories.OutboxObjectFactory.from_remote_object(note_id, note_from_outbox) + + +def setup_inbox_note( + actor: models.Actor, + content: str = "Hello", + to: list[str] = None, + cc: list[str] = None, + tags: list[ap.RawObject] = None, + in_reply_to: str | None = None, +) -> models.OutboxObject: + note_id = uuid4().hex + note_from_outbox = RemoteObject( + factories.build_note_object( + from_remote_actor=actor, + outbox_public_id=note_id, + content=content, + to=to, + cc=cc, + tags=tags, + in_reply_to=in_reply_to, + ), + actor, + ) + return factories.InboxObjectFactory.from_remote_object(note_from_outbox, actor) + + def setup_inbox_delete( actor: models.Actor, deleted_object_ap_id: str ) -> models.InboxObject: