From 7649dd048bf18212c3b528973a0d92319a4bdabb Mon Sep 17 00:00:00 2001 From: Felix Ableitner Date: Wed, 2 Dec 2020 15:08:43 +0100 Subject: [PATCH] Create shared, generic implementation for `FromApub`, prefer local data --- lemmy_apub/src/objects/comment.rs | 38 +++++++------------- lemmy_apub/src/objects/community.rs | 25 ++----------- lemmy_apub/src/objects/mod.rs | 44 ++++++++++++++++++++--- lemmy_apub/src/objects/post.rs | 21 ++--------- lemmy_apub/src/objects/private_message.rs | 25 +++---------- lemmy_apub/src/objects/user.rs | 4 +-- lemmy_db/src/private_message.rs | 21 ++++++----- 7 files changed, 77 insertions(+), 101 deletions(-) diff --git a/lemmy_apub/src/objects/comment.rs b/lemmy_apub/src/objects/comment.rs index 52e9320ef..6d2933949 100644 --- a/lemmy_apub/src/objects/comment.rs +++ b/lemmy_apub/src/objects/comment.rs @@ -8,6 +8,7 @@ use crate::{ objects::{ check_object_domain, create_tombstone, + get_object_from_apub, get_source_markdown_value, set_content_and_source, FromApub, @@ -26,14 +27,12 @@ use lemmy_db::{ community::Community, post::Post, user::User_, - ApubObject, Crud, DbPool, }; use lemmy_structs::blocking; use lemmy_utils::{ location_info, - settings::Settings, utils::{convert_datetime, remove_slurs}, LemmyError, }; @@ -102,38 +101,27 @@ impl FromApub for Comment { expected_domain: Option, request_counter: &mut i32, ) -> Result { - // TODO: we should move read_from_apub_id() and upsert() into traits so we can make a generic - // function to handle all this (shared with User_::from_apub etc) - let comment_id = note.id_unchecked().context(location_info!())?.to_owned(); - let domain = comment_id.domain().context(location_info!())?; - if domain == Settings::get().hostname { - let comment = blocking(context.pool(), move |conn| { - Comment::read_from_apub_id(conn, comment_id.as_str()) - }) - .await??; - Ok(comment) - } else { - let comment_form = - CommentForm::from_apub(note, context, expected_domain, request_counter).await?; - let post_id = comment_form.post_id; - let post = blocking(context.pool(), move |conn| Post::read(conn, post_id)).await??; - if post.locked { - return Err(anyhow!("Post is locked").into()); - } + let comment: Comment = + get_object_from_apub(note, context, expected_domain, request_counter).await?; - let comment = blocking(context.pool(), move |conn| { - Comment::upsert(conn, &comment_form) + let post_id = comment.post_id; + let post = blocking(context.pool(), move |conn| Post::read(conn, post_id)).await??; + if post.locked { + // This is not very efficient because a comment gets inserted just to be deleted right + // afterwards, but it seems to be the easiest way to implement it. + blocking(context.pool(), move |conn| { + Comment::delete(conn, comment.id) }) .await??; + return Err(anyhow!("Post is locked").into()); + } else { Ok(comment) } } } #[async_trait::async_trait(?Send)] -impl FromApubToForm for CommentForm { - type ApubType = NoteExt; - +impl FromApubToForm for CommentForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, diff --git a/lemmy_apub/src/objects/community.rs b/lemmy_apub/src/objects/community.rs index eababf515..fe05de9e4 100644 --- a/lemmy_apub/src/objects/community.rs +++ b/lemmy_apub/src/objects/community.rs @@ -4,6 +4,7 @@ use crate::{ objects::{ check_object_domain, create_tombstone, + get_object_from_apub, get_source_markdown_value, set_content_and_source, FromApub, @@ -25,13 +26,11 @@ use lemmy_db::{ community::{Community, CommunityForm}, community_view::CommunityModeratorView, naive_now, - ApubObject, DbPool, }; use lemmy_structs::blocking; use lemmy_utils::{ location_info, - settings::Settings, utils::{check_slurs, check_slurs_opt, convert_datetime}, LemmyError, }; @@ -121,30 +120,12 @@ impl FromApub for Community { expected_domain: Option, request_counter: &mut i32, ) -> Result { - let community_id = group.id_unchecked().context(location_info!())?.to_owned(); - let domain = community_id.domain().context(location_info!())?; - if domain == Settings::get().hostname { - let community = blocking(context.pool(), move |conn| { - Community::read_from_apub_id(conn, community_id.as_str()) - }) - .await??; - Ok(community) - } else { - let community_form = - CommunityForm::from_apub(group, context, expected_domain, request_counter).await?; - let community = blocking(context.pool(), move |conn| { - Community::upsert(conn, &community_form) - }) - .await??; - Ok(community) - } + get_object_from_apub(group, context, expected_domain, request_counter).await } } #[async_trait::async_trait(?Send)] -impl FromApubToForm for CommunityForm { - type ApubType = GroupExt; - +impl FromApubToForm for CommunityForm { async fn from_apub( group: &GroupExt, context: &LemmyContext, diff --git a/lemmy_apub/src/objects/mod.rs b/lemmy_apub/src/objects/mod.rs index 8258e7469..6a0175051 100644 --- a/lemmy_apub/src/objects/mod.rs +++ b/lemmy_apub/src/objects/mod.rs @@ -7,7 +7,8 @@ use activitystreams::{ }; use anyhow::{anyhow, Context}; use chrono::NaiveDateTime; -use lemmy_db::DbPool; +use lemmy_db::{ApubObject, Crud, DbPool}; +use lemmy_structs::blocking; use lemmy_utils::{location_info, utils::convert_datetime, LemmyError}; use lemmy_websocket::LemmyContext; use url::Url; @@ -46,10 +47,9 @@ pub(crate) trait FromApub { } #[async_trait::async_trait(?Send)] -pub(in crate::objects) trait FromApubToForm { - type ApubType; +pub(in crate::objects) trait FromApubToForm { async fn from_apub( - apub: &Self::ApubType, + apub: &ApubType, context: &LemmyContext, expected_domain: Option, request_counter: &mut i32, @@ -169,3 +169,39 @@ pub(in crate::objects) fn check_is_markdown(mime: Option<&Mime>) -> Result<(), L Ok(()) } } + +/// Converts an ActivityPub object (eg `Note`) to a database object (eg `Comment`). If an object +/// with the same ActivityPub ID already exists in the database, it is returned directly. Otherwise +/// the apub object is parsed, inserted and returned. +pub(in crate::objects) async fn get_object_from_apub( + from: &From, + context: &LemmyContext, + expected_domain: Option, + request_counter: &mut i32, +) -> Result +where + From: BaseExt, + To: ApubObject + Crud + Send + 'static, + ToForm: FromApubToForm + Send + 'static, +{ + let object_id = from.id_unchecked().context(location_info!())?.to_owned(); + let object_in_database = blocking(context.pool(), move |conn| { + To::read_from_apub_id(conn, object_id.as_str()) + }) + .await?; + + // if we already have the object in our database, return that directly + if let Ok(to) = object_in_database { + Ok(to) + } + // if we dont have it, parse from apub and insert into database + // TODO: this is insecure, a `Like/Post` could result in a non-existent post from a different + // instance being inserted into our database. we should request the object over http in that + // case. this might happen exactly in the case where expected_domain = None, but i'm not sure. + else { + let to_form = ToForm::from_apub(&from, context, expected_domain, request_counter).await?; + + let to = blocking(context.pool(), move |conn| To::create(conn, &to_form)).await??; + Ok(to) + } +} diff --git a/lemmy_apub/src/objects/post.rs b/lemmy_apub/src/objects/post.rs index 98a9c2eef..9c68c1c90 100644 --- a/lemmy_apub/src/objects/post.rs +++ b/lemmy_apub/src/objects/post.rs @@ -4,6 +4,7 @@ use crate::{ objects::{ check_object_domain, create_tombstone, + get_object_from_apub, get_source_markdown_value, set_content_and_source, FromApub, @@ -23,7 +24,6 @@ use lemmy_db::{ community::Community, post::{Post, PostForm}, user::User_, - ApubObject, Crud, DbPool, }; @@ -31,7 +31,6 @@ use lemmy_structs::blocking; use lemmy_utils::{ location_info, request::fetch_iframely_and_pictrs_data, - settings::Settings, utils::{check_slurs, convert_datetime, remove_slurs}, LemmyError, }; @@ -113,26 +112,12 @@ impl FromApub for Post { expected_domain: Option, request_counter: &mut i32, ) -> Result { - let post_id = page.id_unchecked().context(location_info!())?.to_owned(); - let domain = post_id.domain().context(location_info!())?; - if domain == Settings::get().hostname { - let post = blocking(context.pool(), move |conn| { - Post::read_from_apub_id(conn, post_id.as_str()) - }) - .await??; - Ok(post) - } else { - let post_form = PostForm::from_apub(page, context, expected_domain, request_counter).await?; - let post = blocking(context.pool(), move |conn| Post::upsert(conn, &post_form)).await??; - Ok(post) - } + get_object_from_apub(page, context, expected_domain, request_counter).await } } #[async_trait::async_trait(?Send)] -impl FromApubToForm for PostForm { - type ApubType = PageExt; - +impl FromApubToForm for PostForm { async fn from_apub( page: &PageExt, context: &LemmyContext, diff --git a/lemmy_apub/src/objects/private_message.rs b/lemmy_apub/src/objects/private_message.rs index 80bc4e664..84b5f9803 100644 --- a/lemmy_apub/src/objects/private_message.rs +++ b/lemmy_apub/src/objects/private_message.rs @@ -5,6 +5,7 @@ use crate::{ objects::{ check_object_domain, create_tombstone, + get_object_from_apub, get_source_markdown_value, set_content_and_source, FromApub, @@ -25,7 +26,7 @@ use lemmy_db::{ DbPool, }; use lemmy_structs::blocking; -use lemmy_utils::{location_info, settings::Settings, utils::convert_datetime, LemmyError}; +use lemmy_utils::{location_info, utils::convert_datetime, LemmyError}; use lemmy_websocket::LemmyContext; use url::Url; @@ -73,30 +74,12 @@ impl FromApub for PrivateMessage { expected_domain: Option, request_counter: &mut i32, ) -> Result { - let private_message_id = note.id_unchecked().context(location_info!())?.to_owned(); - let domain = private_message_id.domain().context(location_info!())?; - if domain == Settings::get().hostname { - let private_message = blocking(context.pool(), move |conn| { - PrivateMessage::read_from_apub_id(conn, private_message_id.as_str()) - }) - .await??; - Ok(private_message) - } else { - let private_message_form = - PrivateMessageForm::from_apub(note, context, expected_domain, request_counter).await?; - let private_message = blocking(context.pool(), move |conn| { - PrivateMessage::upsert(conn, &private_message_form) - }) - .await??; - Ok(private_message) - } + get_object_from_apub(note, context, expected_domain, request_counter).await } } #[async_trait::async_trait(?Send)] -impl FromApubToForm for PrivateMessageForm { - type ApubType = NoteExt; - +impl FromApubToForm for PrivateMessageForm { async fn from_apub( note: &NoteExt, context: &LemmyContext, diff --git a/lemmy_apub/src/objects/user.rs b/lemmy_apub/src/objects/user.rs index 4fce30a62..f65df242a 100644 --- a/lemmy_apub/src/objects/user.rs +++ b/lemmy_apub/src/objects/user.rs @@ -116,9 +116,7 @@ impl FromApub for User_ { } #[async_trait::async_trait(?Send)] -impl FromApubToForm for UserForm { - type ApubType = PersonExt; - +impl FromApubToForm for UserForm { async fn from_apub( person: &PersonExt, _context: &LemmyContext, diff --git a/lemmy_db/src/private_message.rs b/lemmy_db/src/private_message.rs index 503a26abf..e8f021508 100644 --- a/lemmy_db/src/private_message.rs +++ b/lemmy_db/src/private_message.rs @@ -1,4 +1,4 @@ -use crate::{naive_now, schema::private_message, Crud}; +use crate::{naive_now, schema::private_message, ApubObject, Crud}; use diesel::{dsl::*, result::Error, *}; #[derive(Queryable, Identifiable, PartialEq, Debug)] @@ -55,6 +55,18 @@ impl Crud for PrivateMessage { } } +impl ApubObject for PrivateMessage { + fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result + where + Self: Sized, + { + use crate::schema::private_message::dsl::*; + private_message + .filter(ap_id.eq(object_id)) + .first::(conn) + } +} + impl PrivateMessage { pub fn update_ap_id( conn: &PgConnection, @@ -68,13 +80,6 @@ impl PrivateMessage { .get_result::(conn) } - pub fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result { - use crate::schema::private_message::dsl::*; - private_message - .filter(ap_id.eq(object_id)) - .first::(conn) - } - pub fn update_content( conn: &PgConnection, private_message_id: i32,