Let community announce posts #36

Manually merged
nutomic merged 1 commits from announce-posts into federation 2020-06-03 21:56:57 +00:00
6 changed files with 170 additions and 103 deletions

View File

@ -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

View File

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

View File

@ -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(
Review

There's a user_id that's doing the activity, so that needs to get passed through. Instead of sender: &str, use ``creator: &User_`, then you'll have both the actor_id, and the user_id.

There's a user_id that's doing the activity, so that needs to get passed through. Instead of `sender: &str`, use ``creator: &User_`, then you'll have both the actor_id, and the user_id.
Review

Also just to add, part of my thinking in making this a necessary column in the activity table, is to ensure that federated users also have local ids.

Also just to add, part of my thinking in making this a necessary column in the activity table, is to ensure that federated users also have local ids.
&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)
Review

Thinking about it, this is correct, it does make sense to insert both the activity, and the announce.

Thinking about it, this is correct, it does make sense to insert both the activity, and the announce.
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)?;

Now you won't have to do this, because you already have the sending user, which already implements ActorType and you can get the shared_inbox that way.

Now you won't have to do this, because you already have the sending user, which already implements `ActorType` and you can get the shared_inbox that way.

Right, done. There is a similar issue in shared_inbox but that is trickier to fix (I left a todo).

Right, done. There is a similar issue in shared_inbox but that is trickier to fix (I left a todo).

View File

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

View File

@ -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 {

Shouldn't this go in if community.local {

Shouldn't this go in `if community.local {`

Right, I think I forgot to finish this.

Right, I think I forgot to finish this.
Ok(HttpResponse::NotFound().finish())
}
Community::do_announce(activity, &community, sender, conn, false)
}
fn receive_announce(

View File

@ -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(())
Review

As I wrote in the chat, this is breaking with errors like Key (user_id)=(-1) is not present in table "user_". We should probably add foreign keys user_id and community_id and require one of them to be correct. Or instead a column like actor_id?

As I wrote in the chat, this is breaking with errors like `Key (user_id)=(-1) is not present in table "user_"`. We should probably add foreign keys user_id and community_id and require one of them to be correct. Or instead a column like `actor_id`?
Review

I'll read through again, the user_id shouldn't ever be -1, but should be the local DB's id for that user.

I'll read through again, the user_id shouldn't ever be -1, but should be the local DB's id for that user.
Review

That doesnt work if the action is taken by the community (eg announce).

That doesnt work if the action is taken by the community (eg announce).
Review

One level inside announce tho, the activity will have the creator / user that did the action.

One level inside announce tho, the activity will have the creator / user that did the action.
Review

Okay I went with community.creator_id, which is the same thing we use for the accept follow activity. Still seems wrong.

Okay I went with community.creator_id, which is the same thing we use for the accept follow activity. Still seems wrong.
}