Handle out-of-order Delete activity
parent
23832574bc
commit
064921fdd1
|
@ -261,7 +261,7 @@ async def get_object(activity: RawObject) -> RawObject:
|
||||||
|
|
||||||
|
|
||||||
def wrap_object(activity: RawObject) -> RawObject:
|
def wrap_object(activity: RawObject) -> RawObject:
|
||||||
# TODO(ts): improve Create VS Update
|
# TODO(tsileo): improve Create VS Update with a `update=True` flag
|
||||||
if "updated" in activity:
|
if "updated" in activity:
|
||||||
return {
|
return {
|
||||||
"@context": AS_EXTENDED_CTX,
|
"@context": AS_EXTENDED_CTX,
|
||||||
|
|
77
app/boxes.py
77
app/boxes.py
|
@ -654,6 +654,25 @@ async def get_inbox_object_by_ap_id(
|
||||||
).scalar_one_or_none() # type: ignore
|
).scalar_one_or_none() # type: ignore
|
||||||
|
|
||||||
|
|
||||||
|
async def get_inbox_delete_for_activity_object_ap_id(
|
||||||
|
db_session: AsyncSession, activity_object_ap_id: str
|
||||||
|
) -> models.InboxObject | None:
|
||||||
|
return (
|
||||||
|
await db_session.execute(
|
||||||
|
select(models.InboxObject)
|
||||||
|
.where(
|
||||||
|
models.InboxObject.ap_type == "Delete",
|
||||||
|
models.InboxObject.activity_object_ap_id == activity_object_ap_id,
|
||||||
|
)
|
||||||
|
.options(
|
||||||
|
joinedload(models.InboxObject.actor),
|
||||||
|
joinedload(models.InboxObject.relates_to_inbox_object),
|
||||||
|
joinedload(models.InboxObject.relates_to_outbox_object),
|
||||||
|
)
|
||||||
|
)
|
||||||
|
).scalar_one_or_none() # type: ignore
|
||||||
|
|
||||||
|
|
||||||
async def get_outbox_object_by_ap_id(
|
async def get_outbox_object_by_ap_id(
|
||||||
db_session: AsyncSession, ap_id: str
|
db_session: AsyncSession, ap_id: str
|
||||||
) -> models.OutboxObject | None:
|
) -> models.OutboxObject | None:
|
||||||
|
@ -695,8 +714,16 @@ async def _handle_delete_activity(
|
||||||
db_session: AsyncSession,
|
db_session: AsyncSession,
|
||||||
from_actor: models.Actor,
|
from_actor: models.Actor,
|
||||||
delete_activity: models.InboxObject,
|
delete_activity: models.InboxObject,
|
||||||
ap_object_to_delete: models.InboxObject,
|
ap_object_to_delete: models.InboxObject | None,
|
||||||
) -> None:
|
) -> None:
|
||||||
|
if ap_object_to_delete is None:
|
||||||
|
logger.info(
|
||||||
|
"Received Delete for an unknown object "
|
||||||
|
f"{delete_activity.activity_object_ap_id}"
|
||||||
|
)
|
||||||
|
# TODO(tsileo): support deleting actor
|
||||||
|
return
|
||||||
|
|
||||||
if from_actor.ap_id != ap_object_to_delete.actor.ap_id:
|
if from_actor.ap_id != ap_object_to_delete.actor.ap_id:
|
||||||
logger.warning(
|
logger.warning(
|
||||||
"Actor mismatch between the activity and the object: "
|
"Actor mismatch between the activity and the object: "
|
||||||
|
@ -724,11 +751,18 @@ async def _handle_delete_activity(
|
||||||
logger.info(f"Deleting {ap_object_to_delete.ap_type}/{ap_object_to_delete.ap_id}")
|
logger.info(f"Deleting {ap_object_to_delete.ap_type}/{ap_object_to_delete.ap_id}")
|
||||||
ap_object_to_delete.is_deleted = True
|
ap_object_to_delete.is_deleted = True
|
||||||
|
|
||||||
|
await _revert_side_effect_for_deleted_object(db_session, ap_object_to_delete)
|
||||||
|
|
||||||
|
|
||||||
|
async def _revert_side_effect_for_deleted_object(
|
||||||
|
db_session: AsyncSession,
|
||||||
|
deleted_ap_object: models.InboxObject,
|
||||||
|
) -> None:
|
||||||
# Decrement the replies counter if needed
|
# Decrement the replies counter if needed
|
||||||
if ap_object_to_delete.in_reply_to:
|
if deleted_ap_object.in_reply_to:
|
||||||
replied_object = await get_anybox_object_by_ap_id(
|
replied_object = await get_anybox_object_by_ap_id(
|
||||||
db_session,
|
db_session,
|
||||||
ap_object_to_delete.in_reply_to,
|
deleted_ap_object.in_reply_to,
|
||||||
)
|
)
|
||||||
if replied_object:
|
if replied_object:
|
||||||
if replied_object.is_from_outbox:
|
if replied_object.is_from_outbox:
|
||||||
|
@ -928,6 +962,24 @@ async def _handle_create_activity(
|
||||||
raise ValueError("Object actor does not match activity")
|
raise ValueError("Object actor does not match activity")
|
||||||
|
|
||||||
ro = RemoteObject(wrapped_object, actor=from_actor)
|
ro = RemoteObject(wrapped_object, actor=from_actor)
|
||||||
|
|
||||||
|
# Check if we already received a delete for this object (happens often
|
||||||
|
# with forwarded replies)
|
||||||
|
delete_object = await get_inbox_delete_for_activity_object_ap_id(
|
||||||
|
db_session,
|
||||||
|
ro.ap_id,
|
||||||
|
)
|
||||||
|
if delete_object:
|
||||||
|
if delete_object.actor.ap_id != from_actor.ap_id:
|
||||||
|
logger.warning(
|
||||||
|
f"Got a Delete for {ro.ap_id} from {delete_object.actor.ap_id}??"
|
||||||
|
)
|
||||||
|
else:
|
||||||
|
logger.info("Got a Delete for this object, deleting activity")
|
||||||
|
create_activity.is_deleted = True
|
||||||
|
await db_session.flush()
|
||||||
|
return None
|
||||||
|
|
||||||
await _process_note_object(db_session, create_activity, from_actor, ro)
|
await _process_note_object(db_session, create_activity, from_actor, ro)
|
||||||
|
|
||||||
|
|
||||||
|
@ -1251,19 +1303,12 @@ async def save_to_inbox(
|
||||||
elif activity_ro.ap_type == "Update":
|
elif activity_ro.ap_type == "Update":
|
||||||
await _handle_update_activity(db_session, actor, inbox_object)
|
await _handle_update_activity(db_session, actor, inbox_object)
|
||||||
elif activity_ro.ap_type == "Delete":
|
elif activity_ro.ap_type == "Delete":
|
||||||
if relates_to_inbox_object:
|
await _handle_delete_activity(
|
||||||
await _handle_delete_activity(
|
db_session,
|
||||||
db_session,
|
actor,
|
||||||
actor,
|
inbox_object,
|
||||||
inbox_object,
|
relates_to_inbox_object,
|
||||||
relates_to_inbox_object,
|
)
|
||||||
)
|
|
||||||
else:
|
|
||||||
# TODO(ts): handle delete actor
|
|
||||||
logger.info(
|
|
||||||
"Received a Delete for an unknown object: "
|
|
||||||
f"{activity_ro.activity_object_ap_id}"
|
|
||||||
)
|
|
||||||
elif activity_ro.ap_type == "Follow":
|
elif activity_ro.ap_type == "Follow":
|
||||||
await _handle_follow_follow_activity(db_session, actor, inbox_object)
|
await _handle_follow_follow_activity(db_session, actor, inbox_object)
|
||||||
elif activity_ro.ap_type == "Undo":
|
elif activity_ro.ap_type == "Undo":
|
||||||
|
|
|
@ -36,6 +36,24 @@ def build_follow_activity(
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def build_delete_activity(
|
||||||
|
from_remote_actor: actor.RemoteActor | models.Actor,
|
||||||
|
deleted_object_ap_id: str,
|
||||||
|
outbox_public_id: str | None = None,
|
||||||
|
) -> ap.RawObject:
|
||||||
|
return {
|
||||||
|
"@context": ap.AS_CTX,
|
||||||
|
"type": "Delete",
|
||||||
|
"id": (
|
||||||
|
from_remote_actor.ap_id # type: ignore
|
||||||
|
+ "/follow/"
|
||||||
|
+ (outbox_public_id or uuid4().hex)
|
||||||
|
),
|
||||||
|
"actor": from_remote_actor.ap_id,
|
||||||
|
"object": deleted_object_ap_id,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
def build_accept_activity(
|
def build_accept_activity(
|
||||||
from_remote_actor: actor.RemoteActor,
|
from_remote_actor: actor.RemoteActor,
|
||||||
for_remote_object: RemoteObject,
|
for_remote_object: RemoteObject,
|
||||||
|
@ -80,6 +98,19 @@ def build_note_object(
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
def build_create_activity(obj: ap.RawObject) -> ap.RawObject:
|
||||||
|
return {
|
||||||
|
"@context": ap.AS_EXTENDED_CTX,
|
||||||
|
"actor": obj["attributedTo"],
|
||||||
|
"to": obj.get("to", []),
|
||||||
|
"cc": obj.get("cc", []),
|
||||||
|
"id": obj["id"] + "/activity",
|
||||||
|
"object": ap.remove_context(obj),
|
||||||
|
"published": obj["published"],
|
||||||
|
"type": "Create",
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
class BaseModelMeta:
|
class BaseModelMeta:
|
||||||
sqlalchemy_session = _Session
|
sqlalchemy_session = _Session
|
||||||
sqlalchemy_session_persistence = "commit"
|
sqlalchemy_session_persistence = "commit"
|
||||||
|
|
|
@ -3,6 +3,7 @@ from uuid import uuid4
|
||||||
import httpx
|
import httpx
|
||||||
import respx
|
import respx
|
||||||
from fastapi.testclient import TestClient
|
from fastapi.testclient import TestClient
|
||||||
|
from sqlalchemy import select
|
||||||
from sqlalchemy.orm import Session
|
from sqlalchemy.orm import Session
|
||||||
|
|
||||||
from app import activitypub as ap
|
from app import activitypub as ap
|
||||||
|
@ -13,7 +14,9 @@ from app.incoming_activities import process_next_incoming_activity
|
||||||
from tests import factories
|
from tests import factories
|
||||||
from tests.utils import mock_httpsig_checker
|
from tests.utils import mock_httpsig_checker
|
||||||
from tests.utils import run_async
|
from tests.utils import run_async
|
||||||
|
from tests.utils import setup_inbox_delete
|
||||||
from tests.utils import setup_remote_actor
|
from tests.utils import setup_remote_actor
|
||||||
|
from tests.utils import setup_remote_actor_as_follower
|
||||||
|
|
||||||
|
|
||||||
def test_inbox_requires_httpsig(
|
def test_inbox_requires_httpsig(
|
||||||
|
@ -41,7 +44,7 @@ def test_inbox_follow_request(
|
||||||
)
|
)
|
||||||
respx_mock.get(ra.ap_id).mock(return_value=httpx.Response(200, json=ra.ap_actor))
|
respx_mock.get(ra.ap_id).mock(return_value=httpx.Response(200, json=ra.ap_actor))
|
||||||
|
|
||||||
# When sending a Follow activity
|
# When receiving a Follow activity
|
||||||
follow_activity = RemoteObject(
|
follow_activity = RemoteObject(
|
||||||
factories.build_follow_activity(
|
factories.build_follow_activity(
|
||||||
from_remote_actor=ra,
|
from_remote_actor=ra,
|
||||||
|
@ -108,7 +111,7 @@ def test_inbox_accept_follow_request(
|
||||||
follow_id, follow_from_outbox
|
follow_id, follow_from_outbox
|
||||||
)
|
)
|
||||||
|
|
||||||
# When sending a Accept activity
|
# When receiving a Accept activity
|
||||||
accept_activity = RemoteObject(
|
accept_activity = RemoteObject(
|
||||||
factories.build_accept_activity(
|
factories.build_accept_activity(
|
||||||
from_remote_actor=ra,
|
from_remote_actor=ra,
|
||||||
|
@ -137,3 +140,111 @@ def test_inbox_accept_follow_request(
|
||||||
# And a following entry was created internally
|
# And a following entry was created internally
|
||||||
following = db.query(models.Following).one()
|
following = db.query(models.Following).one()
|
||||||
assert following.ap_actor_id == actor_in_db.ap_id
|
assert following.ap_actor_id == actor_in_db.ap_id
|
||||||
|
|
||||||
|
|
||||||
|
def test_inbox__create_from_follower(
|
||||||
|
db: Session,
|
||||||
|
client: TestClient,
|
||||||
|
respx_mock: respx.MockRouter,
|
||||||
|
) -> None:
|
||||||
|
# Given a remote actor
|
||||||
|
ra = setup_remote_actor(respx_mock)
|
||||||
|
|
||||||
|
# Who is also a follower
|
||||||
|
setup_remote_actor_as_follower(ra)
|
||||||
|
|
||||||
|
create_activity = factories.build_create_activity(
|
||||||
|
factories.build_note_object(
|
||||||
|
from_remote_actor=ra,
|
||||||
|
outbox_public_id=str(uuid4()),
|
||||||
|
content="Hello",
|
||||||
|
to=[LOCAL_ACTOR.ap_id],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
# When receiving a Create activity
|
||||||
|
ro = RemoteObject(create_activity, ra)
|
||||||
|
|
||||||
|
with mock_httpsig_checker(ra):
|
||||||
|
response = client.post(
|
||||||
|
"/inbox",
|
||||||
|
headers={"Content-Type": ap.AS_CTX},
|
||||||
|
json=ro.ap_object,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Then the server returns a 204
|
||||||
|
assert response.status_code == 202
|
||||||
|
|
||||||
|
# And when processing the incoming activity
|
||||||
|
run_async(process_next_incoming_activity)
|
||||||
|
|
||||||
|
# Then the Create activity was saved
|
||||||
|
create_activity_from_inbox: models.InboxObject | None = db.execute(
|
||||||
|
select(models.InboxObject).where(models.InboxObject.ap_type == "Create")
|
||||||
|
).scalar_one_or_none()
|
||||||
|
assert create_activity_from_inbox
|
||||||
|
assert create_activity_from_inbox.ap_id == ro.ap_id
|
||||||
|
|
||||||
|
# And the Note object was created
|
||||||
|
note_activity_from_inbox: models.InboxObject | None = db.execute(
|
||||||
|
select(models.InboxObject).where(models.InboxObject.ap_type == "Note")
|
||||||
|
).scalar_one_or_none()
|
||||||
|
assert note_activity_from_inbox
|
||||||
|
assert note_activity_from_inbox.ap_id == ro.activity_object_ap_id
|
||||||
|
|
||||||
|
|
||||||
|
def test_inbox__create_already_deleted_object(
|
||||||
|
db: Session,
|
||||||
|
client: TestClient,
|
||||||
|
respx_mock: respx.MockRouter,
|
||||||
|
) -> None:
|
||||||
|
# Given a remote actor
|
||||||
|
ra = setup_remote_actor(respx_mock)
|
||||||
|
|
||||||
|
# Who is also a follower
|
||||||
|
follower = setup_remote_actor_as_follower(ra)
|
||||||
|
|
||||||
|
# And a Create activity for a Note object
|
||||||
|
create_activity = factories.build_create_activity(
|
||||||
|
factories.build_note_object(
|
||||||
|
from_remote_actor=ra,
|
||||||
|
outbox_public_id=str(uuid4()),
|
||||||
|
content="Hello",
|
||||||
|
to=[LOCAL_ACTOR.ap_id],
|
||||||
|
)
|
||||||
|
)
|
||||||
|
ro = RemoteObject(create_activity, ra)
|
||||||
|
|
||||||
|
# And a Delete activity received for the create object
|
||||||
|
setup_inbox_delete(follower.actor, ro.activity_object_ap_id) # type: ignore
|
||||||
|
|
||||||
|
# When receiving a Create activity
|
||||||
|
with mock_httpsig_checker(ra):
|
||||||
|
response = client.post(
|
||||||
|
"/inbox",
|
||||||
|
headers={"Content-Type": ap.AS_CTX},
|
||||||
|
json=ro.ap_object,
|
||||||
|
)
|
||||||
|
|
||||||
|
# Then the server returns a 204
|
||||||
|
assert response.status_code == 202
|
||||||
|
|
||||||
|
# And when processing the incoming activity
|
||||||
|
run_async(process_next_incoming_activity)
|
||||||
|
|
||||||
|
# Then the Create activity was saved
|
||||||
|
create_activity_from_inbox: models.InboxObject | None = db.execute(
|
||||||
|
select(models.InboxObject).where(models.InboxObject.ap_type == "Create")
|
||||||
|
).scalar_one_or_none()
|
||||||
|
assert create_activity_from_inbox
|
||||||
|
assert create_activity_from_inbox.ap_id == ro.ap_id
|
||||||
|
# But it has the deleted flag
|
||||||
|
assert create_activity_from_inbox.is_deleted is True
|
||||||
|
|
||||||
|
# And the Note wasn't created
|
||||||
|
assert (
|
||||||
|
db.execute(
|
||||||
|
select(models.InboxObject).where(models.InboxObject.ap_type == "Note")
|
||||||
|
).scalar_one_or_none()
|
||||||
|
is None
|
||||||
|
)
|
||||||
|
|
|
@ -73,6 +73,22 @@ def setup_remote_actor_as_follower(ra: actor.RemoteActor) -> models.Follower:
|
||||||
return follower
|
return follower
|
||||||
|
|
||||||
|
|
||||||
|
def setup_inbox_delete(
|
||||||
|
actor: models.Actor, deleted_object_ap_id: str
|
||||||
|
) -> models.InboxObject:
|
||||||
|
follow_from_inbox = RemoteObject(
|
||||||
|
factories.build_delete_activity(
|
||||||
|
from_remote_actor=actor,
|
||||||
|
deleted_object_ap_id=deleted_object_ap_id,
|
||||||
|
),
|
||||||
|
actor,
|
||||||
|
)
|
||||||
|
inbox_object = factories.InboxObjectFactory.from_remote_object(
|
||||||
|
follow_from_inbox, actor
|
||||||
|
)
|
||||||
|
return inbox_object
|
||||||
|
|
||||||
|
|
||||||
def run_async(func, *args, **kwargs):
|
def run_async(func, *args, **kwargs):
|
||||||
async def _func():
|
async def _func():
|
||||||
async with async_session() as db:
|
async with async_session() as db:
|
||||||
|
|
Loading…
Reference in New Issue