From 21a87ebaf2e5c038594eb70ef58bd51826259529 Mon Sep 17 00:00:00 2001 From: RocketDerp <113625597+RocketDerp@users.noreply.github.com> Date: Thu, 27 Jul 2023 03:17:40 -0700 Subject: [PATCH] Federation tests replication round1 - demonstrate absent replication of comment deletes (#3657) * more robust test of unlike a comment, confirm replication to instance downstream from community home * more robust 'delete a comment' test, confirm replication * Far more robust "Report a comment" test. Many comments about situation, this is currently failing because gamma does not get the report * typo and actually have Gamma comment check use gamma, not alpha * prepare-drone-federation-test.sh has some more echo output and note about the LEMMY_DATABASE_URL format (#3651) * Add http cache for webfingers (#3317) * Add http cache for webfingers * Remove the outgoing cache middleware & adjust the cache headers directive * Use 1h & 3day cache header * Update routes and adjust the cache headers location * revert apub caching --------- Co-authored-by: Dessalines Co-authored-by: Felix Ableitner * Rewrite activity lists to fix delete federation (fixes #3625) * Revert "typo and actually have Gamma comment check use gamma, not alpha" This reverts commit 7dfb6ee0f4885da3a2d10316422f5b510772806c. * Revert "Far more robust "Report a comment" test. Many comments about situation, this is currently failing because gamma does not get the report" This reverts commit 7bd3b20ae08a64324029491ddb3ce4295ba16787. * prettier TypeScript * revised comments, as ResolveObject isn't using routine replication * fmt * fix api tests * remove comment --------- Co-authored-by: cetra3 Co-authored-by: Dessalines Co-authored-by: Felix Ableitner --- Cargo.lock | 16 +++++- api_tests/src/comment.spec.ts | 49 ++++++++++++++++- .../apub/src/activities/community/announce.rs | 16 +++--- crates/apub/src/activity_lists.rs | 52 ++++++++----------- crates/apub/src/http/person.rs | 4 +- 5 files changed, 97 insertions(+), 40 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6785fe50b..6ac601c0e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -392,7 +392,7 @@ version = "3.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "64e6d1c7838db705c9b756557ee27c384ce695a1c51a6fe528784cb1c6840170" dependencies = [ - "html5ever 0.26.0", + "html5ever", "maplit", "once_cell", "tendril", @@ -439,6 +439,19 @@ dependencies = [ "serde_json", ] +[[package]] +name = "async-compression" +version = "0.4.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "62b74f44609f0f91493e3082d3734d98497e094777144380ea4db9f9905dd5b6" +dependencies = [ + "flate2", + "futures-core", + "memchr", + "pin-project-lite", + "tokio", +] + [[package]] name = "async-io" version = "1.13.0" @@ -4176,6 +4189,7 @@ version = "0.11.18" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "cde824a14b7c14f85caff81225f411faacc04a2013f41670f41443742b1c1c55" dependencies = [ + "async-compression", "base64 0.21.2", "bytes", "encoding_rs", diff --git a/api_tests/src/comment.spec.ts b/api_tests/src/comment.spec.ts index 19c2797b2..d7d533119 100644 --- a/api_tests/src/comment.spec.ts +++ b/api_tests/src/comment.spec.ts @@ -112,8 +112,27 @@ test("Update a comment", async () => { }); test("Delete a comment", async () => { + // creating a comment on alpha (remote from home of community) let commentRes = await createComment(alpha, postRes.post_view.post.id); + // Find the comment on beta (home of community) + let betaComment = ( + await resolveComment(beta, commentRes.comment_view.comment) + ).comment; + + if (!betaComment) { + throw "Missing beta comment before delete"; + } + + // Find the comment on remote instance gamma + let gammaComment = ( + await resolveComment(gamma, commentRes.comment_view.comment) + ).comment; + + if (!gammaComment) { + throw "Missing gamma comment (remote-home-remote replication) before delete"; + } + let deleteCommentRes = await deleteComment( alpha, true, @@ -126,6 +145,12 @@ test("Delete a comment", async () => { resolveComment(beta, commentRes.comment_view.comment), ).rejects.toBe("couldnt_find_object"); + // Make sure that comment is undefined on gamma after delete + await expect( + resolveComment(gamma, commentRes.comment_view.comment), + ).rejects.toBe("couldnt_find_object"); + + // Test undeleting the comment let undeleteCommentRes = await deleteComment( alpha, false, @@ -225,10 +250,22 @@ test("Remove a comment from admin and community on different instance", async () test("Unlike a comment", async () => { let commentRes = await createComment(alpha, postRes.post_view.post.id); + + // Lemmy automatically creates 1 like (vote) by author of comment. + // Make sure that comment is liked (voted up) on gamma, downstream peer + // This is testing replication from remote-home-remote (alpha-beta-gamma) + let gammaComment1 = ( + await resolveComment(gamma, commentRes.comment_view.comment) + ).comment; + expect(gammaComment1).toBeDefined(); + expect(gammaComment1?.community.local).toBe(false); + expect(gammaComment1?.creator.local).toBe(false); + expect(gammaComment1?.counts.score).toBe(1); + let unlike = await likeComment(alpha, 0, commentRes.comment_view.comment); expect(unlike.comment_view.counts.score).toBe(0); - // Make sure that post is unliked on beta + // Make sure that comment is unliked on beta let betaComment = ( await resolveComment(beta, commentRes.comment_view.comment) ).comment; @@ -236,6 +273,16 @@ test("Unlike a comment", async () => { expect(betaComment?.community.local).toBe(true); expect(betaComment?.creator.local).toBe(false); expect(betaComment?.counts.score).toBe(0); + + // Make sure that comment is unliked on gamma, downstream peer + // This is testing replication from remote-home-remote (alpha-beta-gamma) + let gammaComment = ( + await resolveComment(gamma, commentRes.comment_view.comment) + ).comment; + expect(gammaComment).toBeDefined(); + expect(gammaComment?.community.local).toBe(false); + expect(gammaComment?.creator.local).toBe(false); + expect(gammaComment?.counts.score).toBe(0); }); test("Federated comment like", async () => { diff --git a/crates/apub/src/activities/community/announce.rs b/crates/apub/src/activities/community/announce.rs index ed489158e..6eb23f8da 100644 --- a/crates/apub/src/activities/community/announce.rs +++ b/crates/apub/src/activities/community/announce.rs @@ -50,17 +50,19 @@ impl ActivityHandler for RawAnnouncableActivities { if let AnnouncableActivities::Page(_) = activity { return Err(LemmyErrorType::CannotReceivePage)?; } - let community = activity.community(data).await?; - let actor_id = activity.actor().clone().into(); // verify and receive activity activity.verify(data).await?; - activity.receive(data).await?; + activity.clone().receive(data).await?; - // send to community followers - if community.local { - verify_person_in_community(&actor_id, &community, data).await?; - AnnounceActivity::send(self, &community, data).await?; + // if activity is in a community, send to followers + let community = activity.community(data).await; + if let Ok(community) = community { + if community.local { + let actor_id = activity.actor().clone().into(); + verify_person_in_community(&actor_id, &community, data).await?; + AnnounceActivity::send(self, &community, data).await?; + } } Ok(()) } diff --git a/crates/apub/src/activity_lists.rs b/crates/apub/src/activity_lists.rs index 4cce3372f..d4ca20c33 100644 --- a/crates/apub/src/activity_lists.rs +++ b/crates/apub/src/activity_lists.rs @@ -24,24 +24,32 @@ use crate::{ InCommunity, }, }; -use activitypub_federation::{ - config::Data, - protocol::context::WithContext, - traits::ActivityHandler, -}; +use activitypub_federation::{config::Data, traits::ActivityHandler}; use lemmy_api_common::context::LemmyContext; use lemmy_utils::error::LemmyError; use serde::{Deserialize, Serialize}; use url::Url; +/// List of activities which the shared inbox can handle. +/// +/// This could theoretically be defined as an enum with variants `GroupInboxActivities` and +/// `PersonInboxActivities`. In practice we need to write it out manually so that priorities +/// are handled correctly. #[derive(Debug, Deserialize, Serialize)] #[serde(untagged)] #[enum_delegate::implement(ActivityHandler)] pub enum SharedInboxActivities { - PersonInboxActivities(Box>), - GroupInboxActivities(Box>), + Follow(Follow), + AcceptFollow(AcceptFollow), + UndoFollow(UndoFollow), + CreateOrUpdatePrivateMessage(CreateOrUpdateChatMessage), + Report(Report), + AnnounceActivity(AnnounceActivity), + /// This is a catch-all and needs to be last + RawAnnouncableActivities(RawAnnouncableActivities), } +/// List of activities which the group inbox can handle. #[derive(Debug, Deserialize, Serialize)] #[serde(untagged)] #[enum_delegate::implement(ActivityHandler)] @@ -49,10 +57,11 @@ pub enum GroupInboxActivities { Follow(Follow), UndoFollow(UndoFollow), Report(Report), - // This is a catch-all and needs to be last + /// This is a catch-all and needs to be last AnnouncableActivities(RawAnnouncableActivities), } +/// List of activities which the person inbox can handle. #[derive(Clone, Debug, Deserialize, Serialize)] #[serde(untagged)] #[enum_delegate::implement(ActivityHandler)] @@ -64,17 +73,8 @@ pub enum PersonInboxActivities { Delete(Delete), UndoDelete(UndoDelete), AnnounceActivity(AnnounceActivity), -} - -/// This is necessary for user inbox, which can also receive some "announcable" activities, -/// eg a comment mention. This needs to be a separate enum so that announcables received in shared -/// inbox can fall through to be parsed as GroupInboxActivities::AnnouncableActivities. -#[derive(Clone, Debug, Deserialize, Serialize)] -#[serde(untagged)] -#[enum_delegate::implement(ActivityHandler)] -pub enum PersonInboxActivitiesWithAnnouncable { - PersonInboxActivities(Box), - AnnouncableActivities(Box), + /// User can also receive some "announcable" activities, eg a comment mention. + AnnouncableActivities(AnnouncableActivities), } #[derive(Clone, Debug, Deserialize, Serialize)] @@ -138,12 +138,7 @@ mod tests { #![allow(clippy::indexing_slicing)] use crate::{ - activity_lists::{ - GroupInboxActivities, - PersonInboxActivities, - PersonInboxActivitiesWithAnnouncable, - SiteInboxActivities, - }, + activity_lists::{GroupInboxActivities, PersonInboxActivities, SiteInboxActivities}, protocol::tests::{test_json, test_parse_lemmy_item}, }; @@ -161,16 +156,15 @@ mod tests { fn test_person_inbox() { test_parse_lemmy_item::("assets/lemmy/activities/following/accept.json") .unwrap(); - test_parse_lemmy_item::( + test_parse_lemmy_item::( "assets/lemmy/activities/create_or_update/create_note.json", ) .unwrap(); - test_parse_lemmy_item::( + test_parse_lemmy_item::( "assets/lemmy/activities/create_or_update/create_private_message.json", ) .unwrap(); - test_json::("assets/mastodon/activities/follow.json") - .unwrap(); + test_json::("assets/mastodon/activities/follow.json").unwrap(); } #[test] diff --git a/crates/apub/src/http/person.rs b/crates/apub/src/http/person.rs index 16956ec47..254313634 100644 --- a/crates/apub/src/http/person.rs +++ b/crates/apub/src/http/person.rs @@ -1,5 +1,5 @@ use crate::{ - activity_lists::PersonInboxActivitiesWithAnnouncable, + activity_lists::PersonInboxActivities, fetcher::user_or_community::UserOrCommunity, http::{create_apub_response, create_apub_tombstone_response}, objects::person::ApubPerson, @@ -49,7 +49,7 @@ pub async fn person_inbox( body: Bytes, data: Data, ) -> Result { - receive_activity::, UserOrCommunity, LemmyContext>( + receive_activity::, UserOrCommunity, LemmyContext>( request, body, &data, ) .await