Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix follow IDs #455

Merged
merged 4 commits into from
Mar 4, 2019
Merged

Fix follow IDs #455

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
34effe6
Generate valid IDs for Follow
elegaanz Feb 27, 2019
9b1d34d
Use the new post-insert hook for all the models
elegaanz Feb 27, 2019
832d3c6
Fix plume-cli build
elegaanz Mar 4, 2019
e4065f3
Merge branch 'master' into fix-follow-id
elegaanz Mar 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions plume-cli/src/users.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,7 @@ fn new<'a>(args: &ArgMatches<'a>, conn: &Connection) {
&bio,
email,
User::hash_pass(&password).expect("Couldn't hash password"),
).expect("Couldn't save new user")
.update_boxes(conn).expect("Couldn't update ActivityPub informations for new user");
).expect("Couldn't save new user");
}

fn reset_password<'a>(args: &ArgMatches<'a>, conn: &Connection) {
Expand Down
75 changes: 31 additions & 44 deletions plume-models/src/blogs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use activitypub::{actor::Group, collection::OrderedCollection, Actor, CustomObject, Object};
use chrono::NaiveDateTime;
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use openssl::{
hash::MessageDigest,
pkey::{PKey, Private},
Expand Down Expand Up @@ -30,7 +30,7 @@ use {Connection, BASE_URL, USE_HTTPS, Error, Result};

pub type CustomGroup = CustomObject<ApSignature, Group>;

#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone)]
#[derive(Queryable, Identifiable, Serialize, Deserialize, Clone, AsChangeset)]
pub struct Blog {
pub id: i32,
pub actor_id: String,
Expand All @@ -45,7 +45,7 @@ pub struct Blog {
pub public_key: String,
}

#[derive(Insertable)]
#[derive(Default, Insertable)]
#[table_name = "blogs"]
pub struct NewBlog {
pub actor_id: String,
Expand All @@ -62,7 +62,33 @@ pub struct NewBlog {
const BLOG_PREFIX: &str = "~";

impl Blog {
insert!(blogs, NewBlog);
insert!(blogs, NewBlog, |inserted, conn| {
let instance = inserted.get_instance(conn)?;
if inserted.outbox_url.is_empty() {
inserted.outbox_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"outbox",
);
}

if inserted.inbox_url.is_empty() {
inserted.inbox_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"inbox",
);
}

if inserted.ap_url.is_empty() {
inserted.ap_url = instance.compute_box(
BLOG_PREFIX,
&inserted.actor_id,
"",
);
}
inserted.save_changes(conn).map_err(Error::from)
});
get!(blogs);
find_by!(blogs, find_by_ap_url, ap_url as &str);
find_by!(blogs, find_by_name, actor_id as &str, instance_id as i32);
Expand Down Expand Up @@ -243,36 +269,6 @@ impl Blog {
Ok(CustomGroup::new(blog, ap_signature))
}

pub fn update_boxes(&self, conn: &Connection) -> Result<()> {
let instance = self.get_instance(conn)?;
if self.outbox_url.is_empty() {
diesel::update(self)
.set(blogs::outbox_url.eq(instance.compute_box(
BLOG_PREFIX,
&self.actor_id,
"outbox",
)))
.execute(conn)?;
}

if self.inbox_url.is_empty() {
diesel::update(self)
.set(blogs::inbox_url.eq(instance.compute_box(
BLOG_PREFIX,
&self.actor_id,
"inbox",
)))
.execute(conn)?;
}

if self.ap_url.is_empty() {
diesel::update(self)
.set(blogs::ap_url.eq(instance.compute_box(BLOG_PREFIX, &self.actor_id, "")))
.execute(conn)?;
}
Ok(())
}

pub fn outbox(&self, conn: &Connection) -> Result<ActivityStream<OrderedCollection>> {
let mut coll = OrderedCollection::default();
coll.collection_props.items = serde_json::to_value(self.get_activities(conn)?)?;
Expand Down Expand Up @@ -431,12 +427,10 @@ impl NewBlog {
actor_id,
title,
summary,
outbox_url: String::from(""),
inbox_url: String::from(""),
instance_id,
ap_url: String::from(""),
public_key: String::from_utf8(pub_key).or(Err(Error::Signature))?,
private_key: Some(String::from_utf8(priv_key).or(Err(Error::Signature))?),
..NewBlog::default()
igalic marked this conversation as resolved.
Show resolved Hide resolved
})
}
}
Expand All @@ -461,21 +455,18 @@ pub(crate) mod tests {
"This is a small blog".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog1.update_boxes(conn).unwrap();
let blog2 = Blog::insert(conn, NewBlog::new_local(
"MyBlog".to_owned(),
"My blog".to_owned(),
"Welcome to my blog".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog2.update_boxes(conn).unwrap();
let blog3 = Blog::insert(conn, NewBlog::new_local(
"WhyILikePlume".to_owned(),
"Why I like Plume".to_owned(),
"In this blog I will explay you why I like Plume so much".to_owned(),
Instance::get_local(conn).unwrap().id
).unwrap()).unwrap();
blog3.update_boxes(conn).unwrap();

BlogAuthor::insert(
conn,
Expand Down Expand Up @@ -553,7 +544,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b1.update_boxes(conn).unwrap();
let b2 = Blog::insert(
conn,
NewBlog::new_local(
Expand All @@ -563,7 +553,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id
).unwrap(),
).unwrap();
b2.update_boxes(conn).unwrap();
let blog = vec![ b1, b2 ];

BlogAuthor::insert(
Expand Down Expand Up @@ -719,7 +708,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b1.update_boxes(conn).unwrap();
let b2 = Blog::insert(
conn,
NewBlog::new_local(
Expand All @@ -729,7 +717,6 @@ pub(crate) mod tests {
Instance::get_local(conn).unwrap().id,
).unwrap(),
).unwrap();
b2.update_boxes(conn).unwrap();
let blog = vec![ b1, b2 ];

BlogAuthor::insert(
Expand Down
29 changes: 10 additions & 19 deletions plume-models/src/comments.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use activitypub::{activity::{Create, Delete}, link, object::{Note, Tombstone}};
use chrono::{self, NaiveDateTime};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};
use serde_json;

use std::collections::HashSet;
Expand All @@ -20,7 +20,7 @@ use schema::comments;
use users::User;
use {Connection, Error, Result};

#[derive(Queryable, Identifiable, Serialize, Clone)]
#[derive(Queryable, Identifiable, Serialize, Clone, AsChangeset)]
pub struct Comment {
pub id: i32,
pub content: SafeString,
Expand Down Expand Up @@ -48,7 +48,13 @@ pub struct NewComment {
}

impl Comment {
insert!(comments, NewComment);
insert!(comments, NewComment, |inserted, conn| {
if inserted.ap_url.is_none() {
inserted.ap_url = Some(format!("{}comment/{}", inserted.get_post(conn)?.ap_url, inserted.id));
let _: Comment = inserted.save_changes(conn)?;
}
Ok(inserted)
});
get!(comments);
list_by!(comments, list_by_post, post_id as i32);
find_by!(comments, find_by_ap_url, ap_url as &str);
Expand Down Expand Up @@ -79,21 +85,6 @@ impl Comment {
.map_err(Error::from)
}

pub fn update_ap_url(&self, conn: &Connection) -> Result<Comment> {
if self.ap_url.is_none() {
diesel::update(self)
.set(comments::ap_url.eq(self.compute_id(conn)?))
.execute(conn)?;
Comment::get(conn, self.id)
} else {
Ok(self.clone())
}
}

pub fn compute_id(&self, conn: &Connection) -> Result<String> {
Ok(format!("{}comment/{}", self.get_post(conn)?.ap_url, self.id))
}

pub fn can_see(&self, conn: &Connection, user: Option<&User>) -> bool {
self.public_visibility ||
user.as_ref().map(|u| CommentSeers::can_see(conn, self, u).unwrap_or(false))
Expand All @@ -117,7 +108,7 @@ impl Comment {
note.object_props
.set_in_reply_to_link(Id::new(self.in_response_to_id.map_or_else(
|| Ok(Post::get(conn, self.post_id)?.ap_url),
|id| Ok(Comment::get(conn, id)?.compute_id(conn)?) as Result<String>,
|id| Ok(Comment::get(conn, id)?.ap_url.unwrap_or_default()) as Result<String>,
)?))?;
note.object_props
.set_published_string(chrono::Utc::now().to_rfc3339())?;
Expand Down
43 changes: 40 additions & 3 deletions plume-models/src/follows.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use activitypub::{
actor::Person,
Actor,
};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl};
use diesel::{self, ExpressionMethods, QueryDsl, RunQueryDsl, SaveChangesDsl};

use blogs::Blog;
use notifications::*;
Expand All @@ -17,7 +17,7 @@ use schema::follows;
use users::User;
use {ap_url, Connection, BASE_URL, Error, Result};

#[derive(Clone, Queryable, Identifiable, Associations)]
#[derive(Clone, Queryable, Identifiable, Associations, AsChangeset)]
#[belongs_to(User, foreign_key = "following_id")]
pub struct Follow {
pub id: i32,
Expand All @@ -35,7 +35,14 @@ pub struct NewFollow {
}

impl Follow {
insert!(follows, NewFollow);
insert!(follows, NewFollow, |inserted, conn| {
if inserted.ap_url.is_empty() {
inserted.ap_url = ap_url(&format!("{}/follows/{}", *BASE_URL, inserted.id));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this route should really get implemented one day

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think we already have an issue for it…

inserted.save_changes(conn).map_err(Error::from)
} else {
Ok(inserted)
}
});
get!(follows);
find_by!(follows, find_by_ap_url, ap_url as &str);

Expand Down Expand Up @@ -200,3 +207,33 @@ impl IntoId for Follow {
Id::new(self.ap_url)
}
}

#[cfg(test)]
mod tests {
use diesel::Connection;
use super::*;
use tests::db;
use users::tests as user_tests;

#[test]
fn test_id() {
let conn = db();
conn.test_transaction::<_, (), _>(|| {
let users = user_tests::fill_database(&conn);
let follow = Follow::insert(&conn, NewFollow {
follower_id: users[0].id,
following_id: users[1].id,
ap_url: String::new(),
}).expect("Couldn't insert new follow");
assert_eq!(follow.ap_url, format!("https://{}/follows/{}", *BASE_URL, follow.id));

let follow = Follow::insert(&conn, NewFollow {
follower_id: users[1].id,
following_id: users[0].id,
ap_url: String::from("https://some.url/"),
}).expect("Couldn't insert new follow");
assert_eq!(follow.ap_url, String::from("https://some.url/"));
Ok(())
});
}
}
8 changes: 7 additions & 1 deletion plume-models/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,13 +235,19 @@ macro_rules! get {
/// ```
macro_rules! insert {
($table:ident, $from:ident) => {
insert!($table, $from, |x, _conn| { Ok(x) });
};
($table:ident, $from:ident, |$val:ident, $conn:ident | $after:block) => {
last!($table);

pub fn insert(conn: &crate::Connection, new: $from) -> Result<Self> {
diesel::insert_into($table::table)
.values(new)
.execute(conn)?;
Self::last(conn)
#[allow(unused_mut)]
let mut $val = Self::last(conn)?;
let $conn = conn;
$after
}
};
}
Expand Down
Loading