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 <dessalines@users.noreply.github.com>
Co-authored-by: Felix Ableitner <me@nutomic.com>

* Rewrite activity lists to fix delete federation (fixes #3625)

* Revert "typo and actually have Gamma comment check use gamma, not alpha"

This reverts commit 7dfb6ee0f4.

* 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 7bd3b20ae0.

* prettier TypeScript

* revised comments, as ResolveObject isn't using routine replication

* fmt

* fix api tests

* remove comment

---------

Co-authored-by: cetra3 <cetra3@hotmail.com>
Co-authored-by: Dessalines <dessalines@users.noreply.github.com>
Co-authored-by: Felix Ableitner <me@nutomic.com>

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",

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 () => {

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(())
   }

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<WithContext<PersonInboxActivities>>),
-  GroupInboxActivities(Box<WithContext<GroupInboxActivities>>),
+  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<PersonInboxActivities>),
-  AnnouncableActivities(Box<AnnouncableActivities>),
+  /// 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::<PersonInboxActivities>("assets/lemmy/activities/following/accept.json")
       .unwrap();
-    test_parse_lemmy_item::<PersonInboxActivitiesWithAnnouncable>(
+    test_parse_lemmy_item::<PersonInboxActivities>(
       "assets/lemmy/activities/create_or_update/create_note.json",
     )
     .unwrap();
-    test_parse_lemmy_item::<PersonInboxActivitiesWithAnnouncable>(
+    test_parse_lemmy_item::<PersonInboxActivities>(
       "assets/lemmy/activities/create_or_update/create_private_message.json",
     )
     .unwrap();
-    test_json::<PersonInboxActivitiesWithAnnouncable>("assets/mastodon/activities/follow.json")
-      .unwrap();
+    test_json::<PersonInboxActivities>("assets/mastodon/activities/follow.json").unwrap();
   }
 
   #[test]

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<LemmyContext>,
 ) -> Result<HttpResponse, LemmyError> {
-  receive_activity::<WithContext<PersonInboxActivitiesWithAnnouncable>, UserOrCommunity, LemmyContext>(
+  receive_activity::<WithContext<PersonInboxActivities>, UserOrCommunity, LemmyContext>(
     request, body, &data,
   )
   .await