address review comments

6 files changed, 170 insertions, 103 deletions

diff --git a/server/src/apub/activities.rs b/server/src/apub/activities.rs
index 3816093c..a747ed21 100644
--- a/server/src/apub/activities.rs
+++ b/server/src/apub/activities.rs
@@ -1,5 +1,9 @@
-use crate::apub::{extensions::signatures::sign, is_apub_id_valid, ActorType};
-use activitystreams::{context, object::properties::ObjectProperties, public};
+use crate::{
+  apub::{extensions::signatures::sign, is_apub_id_valid, ActorType},
+  db::{activity::insert_activity, community::Community, user::User_},
+};
+use activitystreams::{context, object::properties::ObjectProperties, public, Activity, Base};
+use diesel::PgConnection;
 use failure::{Error, _core::fmt::Debug};
 use isahc::prelude::*;
 use log::debug;
@@ -22,6 +26,27 @@ pub fn populate_object_props(
   Ok(())
 }
 
+pub fn send_activity_to_community<A>(
+  creator: &User_,
+  conn: &PgConnection,
+  community: &Community,
+  to: Vec<String>,
+  activity: A,
+) -> Result<(), Error>
+where
+  A: Activity + Base + Serialize + Debug,
+{
+  insert_activity(&conn, creator.id, &activity, true)?;
+
+  // if this is a local community, we need to do an announce from the community instead
+  if community.local {
+    Community::do_announce(activity, &community, creator, conn)?;
+  } else {
+    send_activity(&activity, creator, to)?;
+  }
+  Ok(())
+}
+
 /// Send an activity to a list of recipients, using the correct headers etc.
 pub fn send_activity<A>(activity: &A, actor: &dyn ActorType, to: Vec<String>) -> Result<(), Error>
 where
diff --git a/server/src/apub/comment.rs b/server/src/apub/comment.rs
index d199eddb..7c77a1c7 100644
--- a/server/src/apub/comment.rs
+++ b/server/src/apub/comment.rs
@@ -1,6 +1,6 @@
 use crate::{
   apub::{
-    activities::{populate_object_props, send_activity},
+    activities::{populate_object_props, send_activity_to_community},
     create_apub_response,
     create_apub_tombstone_response,
     create_tombstone,
@@ -14,7 +14,6 @@ use crate::{
   },
   convert_datetime,
   db::{
-    activity::insert_activity,
     comment::{Comment, CommentForm},
     community::Community,
     post::Post,
@@ -30,15 +29,13 @@ use activitystreams::{
   context,
   link::Mention,
   object::{kind::NoteType, properties::ObjectProperties, Note, Tombstone},
-  Activity,
-  Base,
 };
 use actix_web::{body::Body, web::Path, HttpResponse, Result};
 use diesel::PgConnection;
-use failure::{Error, _core::fmt::Debug};
+use failure::Error;
 use itertools::Itertools;
 use log::debug;
-use serde::{Deserialize, Serialize};
+use serde::Deserialize;
 
 #[derive(Deserialize)]
 pub struct CommentQuery {
@@ -178,7 +175,7 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, maa.inboxes,create)?;
+    send_activity_to_community(&creator, &conn, &community, maa.inboxes, create)?;
     Ok(())
   }
 
@@ -203,7 +200,7 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, maa.inboxes, update)?;
+    send_activity_to_community(&creator, &conn, &community, maa.inboxes, update)?;
     Ok(())
   }
 
@@ -225,7 +222,13 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community,vec!(community.get_shared_inbox_url()), delete)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      delete,
+    )?;
     Ok(())
   }
 
@@ -265,7 +268,13 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(delete)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, vec!(community.get_shared_inbox_url()),undo)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 
@@ -287,7 +296,13 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(mod_.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&mod_, &conn, &community, vec!(community.get_shared_inbox_url()),remove)?;
+    send_activity_to_community(
+      &mod_,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      remove,
+    )?;
     Ok(())
   }
 
@@ -326,7 +341,13 @@ impl ApubObjectType for Comment {
       .set_actor_xsd_any_uri(mod_.actor_id.to_owned())?
       .set_object_base_box(remove)?;
 
-    Comment::send_comment_activity(&mod_, &conn, &community, vec!(community.get_shared_inbox_url()),undo)?;
+    send_activity_to_community(
+      &mod_,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 }
@@ -349,7 +370,13 @@ impl ApubLikeableType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, vec!(community.get_shared_inbox_url()),like)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      like,
+    )?;
     Ok(())
   }
 
@@ -370,7 +397,13 @@ impl ApubLikeableType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(note)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, vec!(community.get_shared_inbox_url()),dislike)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      dislike,
+    )?;
     Ok(())
   }
 
@@ -407,7 +440,13 @@ impl ApubLikeableType for Comment {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(like)?;
 
-    Comment::send_comment_activity(&creator, &conn, &community, vec!(community.get_shared_inbox_url()), undo)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 }
@@ -455,7 +494,7 @@ fn collect_non_local_mentions_and_addresses(
     }
   }
 
-  let mut inboxes = vec!(community.get_shared_inbox_url());
+  let mut inboxes = vec![community.get_shared_inbox_url()];
   inboxes.extend(mention_inboxes);
   inboxes = inboxes.into_iter().unique().collect();
 
@@ -465,26 +504,3 @@ fn collect_non_local_mentions_and_addresses(
     tags,
   })
 }
-
-impl Comment {
-  fn send_comment_activity<A>(
-    creator: &User_,
-    conn: &PgConnection,
-    community: &Community,
-    to: Vec<String>,
-    activity: A,
-  ) -> Result<(), Error>
-  where
-    A: Activity + Base + Serialize + Debug,
-  {
-    insert_activity(&conn, creator.id, &activity, true)?;
-
-    // if this is a local community, we need to do an announce from the community instead
-    if community.local {
-      Community::do_announce(activity, &community, &creator.actor_id, conn, true)?;
-    } else {
-      send_activity(&activity, creator, to)?;
-    }
-    Ok(())
-  }
-}
diff --git a/server/src/apub/community.rs b/server/src/apub/community.rs
index 9cd65334..a81e21cf 100644
--- a/server/src/apub/community.rs
+++ b/server/src/apub/community.rs
@@ -385,15 +385,12 @@ impl Community {
   pub fn do_announce<A>(
     activity: A,
     community: &Community,
-    sender: &str,
+    sender: &dyn ActorType,
     conn: &PgConnection,
-    is_local_activity: bool,
   ) -> Result<HttpResponse, Error>
   where
     A: Activity + Base + Serialize + Debug,
   {
-    insert_activity(&conn, -1, &activity, is_local_activity)?;
-
     let mut announce = Announce::default();
     populate_object_props(
       &mut announce.object_props,
@@ -405,14 +402,13 @@ impl Community {
       .set_actor_xsd_any_uri(community.actor_id.to_owned())?
       .set_object_base_box(BaseBox::from_concrete(activity)?)?;
 
-    insert_activity(&conn, -1, &announce, true)?;
+    insert_activity(&conn, community.creator_id, &announce, true)?;
 
     // dont send to the instance where the activity originally came from, because that would result
     // in a database error (same data inserted twice)
     let mut to = community.get_follower_inboxes(&conn)?;
-    let sending_user = get_or_fetch_and_upsert_remote_user(&sender, conn)?;
     // this seems to be the "easiest" stable alternative for remove_item()
-    to.retain(|x| *x != sending_user.get_shared_inbox_url());
+    to.retain(|x| *x != sender.get_shared_inbox_url());
 
     send_activity(&announce, community, to)?;
 
diff --git a/server/src/apub/post.rs b/server/src/apub/post.rs
index 2b8455d5..f53af309 100644
--- a/server/src/apub/post.rs
+++ b/server/src/apub/post.rs
@@ -1,6 +1,6 @@
 use crate::{
   apub::{
-    activities::{populate_object_props, send_activity},
+    activities::{populate_object_props, send_activity_to_community},
     create_apub_response,
     create_apub_tombstone_response,
     create_tombstone,
@@ -16,7 +16,6 @@ use crate::{
   },
   convert_datetime,
   db::{
-    activity::insert_activity,
     community::Community,
     post::{Post, PostForm},
     user::User_,
@@ -29,15 +28,13 @@ use activitystreams::{
   activity::{Create, Delete, Dislike, Like, Remove, Undo, Update},
   context,
   object::{kind::PageType, properties::ObjectProperties, AnyImage, Image, Page, Tombstone},
-  Activity,
-  Base,
   BaseBox,
 };
 use activitystreams_ext::Ext1;
 use actix_web::{body::Body, web::Path, HttpResponse, Result};
 use diesel::PgConnection;
-use failure::{Error, _core::fmt::Debug};
-use serde::{Deserialize, Serialize};
+use failure::Error;
+use serde::Deserialize;
 
 #[derive(Deserialize)]
 pub struct PostQuery {
@@ -241,7 +238,13 @@ impl ApubObjectType for Post {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(BaseBox::from_concrete(page)?)?;
 
-    Post::send_post_activity(creator, conn, &community, create)?;
+    send_activity_to_community(
+      creator,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      create,
+    )?;
     Ok(())
   }
 
@@ -262,7 +265,13 @@ impl ApubObjectType for Post {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(BaseBox::from_concrete(page)?)?;
 
-    Post::send_post_activity(creator, conn, &community, update)?;
+    send_activity_to_community(
+      creator,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      update,
+    )?;
     Ok(())
   }
 
@@ -285,7 +294,13 @@ impl ApubObjectType for Post {
 
     let community = Community::read(conn, self.community_id)?;
 
-    Post::send_post_activity(creator, conn, &community, delete)?;
+    send_activity_to_community(
+      creator,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      delete,
+    )?;
     Ok(())
   }
 
@@ -323,7 +338,13 @@ impl ApubObjectType for Post {
       .set_object_base_box(delete)?;
 
     let community = Community::read(conn, self.community_id)?;
-    Post::send_post_activity(creator, conn, &community, undo)?;
+    send_activity_to_community(
+      creator,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 
@@ -346,7 +367,13 @@ impl ApubObjectType for Post {
 
     let community = Community::read(conn, self.community_id)?;
 
-    Post::send_post_activity(mod_, conn, &community, remove)?;
+    send_activity_to_community(
+      mod_,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      remove,
+    )?;
     Ok(())
   }
   fn send_undo_remove(&self, mod_: &User_, conn: &PgConnection) -> Result<(), Error> {
@@ -382,7 +409,13 @@ impl ApubObjectType for Post {
       .set_object_base_box(remove)?;
 
     let community = Community::read(conn, self.community_id)?;
-    Post::send_post_activity(mod_, conn, &community, undo)?;
+    send_activity_to_community(
+      mod_,
+      conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 }
@@ -404,7 +437,13 @@ impl ApubLikeableType for Post {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(BaseBox::from_concrete(page)?)?;
 
-    Post::send_post_activity(&creator, &conn, &community, like)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      like,
+    )?;
     Ok(())
   }
 
@@ -424,7 +463,13 @@ impl ApubLikeableType for Post {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(BaseBox::from_concrete(page)?)?;
 
-    Post::send_post_activity(&creator, &conn, &community, dislike)?;
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      dislike,
+    )?;
     Ok(())
   }
 
@@ -460,29 +505,13 @@ impl ApubLikeableType for Post {
       .set_actor_xsd_any_uri(creator.actor_id.to_owned())?
       .set_object_base_box(like)?;
 
-    Post::send_post_activity(&creator, &conn, &community, undo)?;
-    Ok(())
-  }
-}
-
-impl Post {
-  fn send_post_activity<A>(
-    creator: &User_,
-    conn: &PgConnection,
-    community: &Community,
-    activity: A,
-  ) -> Result<(), Error>
-  where
-    A: Activity + Base + Serialize + Debug,
-  {
-    insert_activity(&conn, creator.id, &activity, true)?;
-
-    // if this is a local community, we need to do an announce from the community instead
-    if community.local {
-      Community::do_announce(activity, &community, &creator.actor_id, conn, true)?;
-    } else {
-      send_activity(&activity, creator, vec![community.get_shared_inbox_url()])?;
-    }
+    send_activity_to_community(
+      &creator,
+      &conn,
+      &community,
+      vec![community.get_shared_inbox_url()],
+      undo,
+    )?;
     Ok(())
   }
 }
diff --git a/server/src/apub/shared_inbox.rs b/server/src/apub/shared_inbox.rs
index b4a214a0..81dd14c3 100644
--- a/server/src/apub/shared_inbox.rs
+++ b/server/src/apub/shared_inbox.rs
@@ -122,6 +122,7 @@ pub async fn shared_inbox(
   // TODO: this is hacky, we should probably send the community id directly somehow
   let to = cc.replace("/followers", "");
 
+  // TODO: this is ugly
   match get_or_fetch_and_upsert_remote_user(&sender.to_string(), &conn) {
     Ok(u) => verify(&request, &u),
     Err(_) => {
@@ -134,15 +135,15 @@ pub async fn shared_inbox(
     (SharedAcceptedObjects::Create(c), Some("Page")) => {
       receive_create_post(&c, &conn, chat_server)?;
       announce_activity_if_valid::<Create>(*c, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Update(u), Some("Page")) => {
       receive_update_post(&u, &conn, chat_server)?;
       announce_activity_if_valid::<Update>(*u, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Like(l), Some("Page")) => {
       receive_like_post(&l, &conn, chat_server)?;
       announce_activity_if_valid::<Like>(*l, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Dislike(d), Some("Page")) => {
       receive_dislike_post(&d, &conn, chat_server)?;
       announce_activity_if_valid::<Dislike>(*d, &to, sender, conn)
@@ -150,11 +151,11 @@ pub async fn shared_inbox(
     (SharedAcceptedObjects::Delete(d), Some("Page")) => {
       receive_delete_post(&d, &conn, chat_server)?;
       announce_activity_if_valid::<Delete>(*d, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Remove(r), Some("Page")) => {
       receive_remove_post(&r, &conn, chat_server)?;
       announce_activity_if_valid::<Remove>(*r, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Create(c), Some("Note")) => {
       receive_create_comment(&c, &conn, chat_server)?;
       announce_activity_if_valid::<Create>(*c, &to, sender, conn)
@@ -166,7 +167,7 @@ pub async fn shared_inbox(
     (SharedAcceptedObjects::Like(l), Some("Note")) => {
       receive_like_comment(&l, &conn, chat_server)?;
       announce_activity_if_valid::<Like>(*l, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Dislike(d), Some("Note")) => {
       receive_dislike_comment(&d, &conn, chat_server)?;
       announce_activity_if_valid::<Dislike>(*d, &to, sender, conn)
@@ -190,22 +191,21 @@ pub async fn shared_inbox(
     (SharedAcceptedObjects::Undo(u), Some("Delete")) => {
       receive_undo_delete(&u, &conn, chat_server)?;
       announce_activity_if_valid::<Undo>(*u, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Undo(u), Some("Remove")) => {
       receive_undo_remove(&u, &conn, chat_server)?;
       announce_activity_if_valid::<Undo>(*u, &to, sender, conn)
-    },
+    }
     (SharedAcceptedObjects::Undo(u), Some("Like")) => {
       receive_undo_like(&u, &conn, chat_server)?;
       announce_activity_if_valid::<Undo>(*u, &to, sender, conn)
-    },
-    (SharedAcceptedObjects::Announce(a), _) => {
-      receive_announce(a, &conn, chat_server)
-    },
+    }
+    (SharedAcceptedObjects::Announce(a), _) => receive_announce(a, &conn, chat_server),
     (a, _) => receive_unhandled_activity(a),
   }
 }
 
+// TODO: should pass in sender as ActorType, but thats a bit tricky in shared_inbox()
 fn announce_activity_if_valid<A>(
   activity: A,
   community_uri: &str,
@@ -215,12 +215,14 @@ fn announce_activity_if_valid<A>(
 where
   A: Activity + Base + Serialize + Debug,
 {
-  // TODO: first check that it is addressed to a local community
   let community = Community::read_from_actor_id(conn, &community_uri)?;
-  if !community.local {
-    // ignore this object
+  if community.local {
+    let sending_user = get_or_fetch_and_upsert_remote_user(&sender.to_string(), &conn)?;
+    insert_activity(&conn, sending_user.id, &activity, false)?;
+    Community::do_announce(activity, &community, &sending_user, conn)
+  } else {
+    Ok(HttpResponse::NotFound().finish())
   }
-  Community::do_announce(activity, &community, sender, conn, false)
 }
 
 fn receive_announce(
diff --git a/server/src/db/activity.rs b/server/src/db/activity.rs
index 12137887..714c8202 100644
--- a/server/src/db/activity.rs
+++ b/server/src/db/activity.rs
@@ -71,8 +71,7 @@ where
     updated: None,
   };
   debug!("inserting activity for user {}, data {:?}", user_id, data);
-  // TODO: this is broken
-  //Activity::create(&conn, &activity_form)?;
+  Activity::create(&conn, &activity_form)?;
   Ok(())
 }