Create shared, generic implementation for FromApub, prefer local data

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<Url>,
     request_counter: &mut i32,
   ) -> Result<Comment, LemmyError> {
-    // TODO: we should move read_from_apub_id() and upsert() into traits so we can make a generic
+    let comment: Comment =
-    //       function to handle all this (shared with User_::from_apub etc)
+      get_object_from_apub(note, context, expected_domain, request_counter).await?;
-    let comment_id = note.id_unchecked().context(location_info!())?.to_owned();
+
-    let domain = comment_id.domain().context(location_info!())?;
+    let post_id = comment.post_id;
-    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());
+      // 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| {
-      let comment = blocking(context.pool(), move |conn| {
+        Comment::delete(conn, comment.id)
-        Comment::upsert(conn, &comment_form)
       })
       .await??;
+      return Err(anyhow!("Post is locked").into());
+    } else {
       Ok(comment)
     }
   }
 }
 
 #[async_trait::async_trait(?Send)]
-impl FromApubToForm for CommentForm {
+impl FromApubToForm<NoteExt> for CommentForm {
-  type ApubType = NoteExt;
-
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,

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<Url>,
     request_counter: &mut i32,
   ) -> Result<Community, LemmyError> {
-    let community_id = group.id_unchecked().context(location_info!())?.to_owned();
+    get_object_from_apub(group, context, expected_domain, request_counter).await
-    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)
-    }
   }
 }
 
 #[async_trait::async_trait(?Send)]
-impl FromApubToForm for CommunityForm {
+impl FromApubToForm<GroupExt> for CommunityForm {
-  type ApubType = GroupExt;
-
   async fn from_apub(
     group: &GroupExt,
     context: &LemmyContext,

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 {
+pub(in crate::objects) trait FromApubToForm<ApubType> {
-  type ApubType;
   async fn from_apub(
-    apub: &Self::ApubType,
+    apub: &ApubType,
     context: &LemmyContext,
     expected_domain: Option<Url>,
     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, Kind, To, ToForm>(
+  from: &From,
+  context: &LemmyContext,
+  expected_domain: Option<Url>,
+  request_counter: &mut i32,
+) -> Result<To, LemmyError>
+where
+  From: BaseExt<Kind>,
+  To: ApubObject + Crud<ToForm> + Send + 'static,
+  ToForm: FromApubToForm<From> + 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)
+  }
+}

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<Url>,
     request_counter: &mut i32,
   ) -> Result<Post, LemmyError> {
-    let post_id = page.id_unchecked().context(location_info!())?.to_owned();
+    get_object_from_apub(page, context, expected_domain, request_counter).await
-    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)
-    }
   }
 }
 
 #[async_trait::async_trait(?Send)]
-impl FromApubToForm for PostForm {
+impl FromApubToForm<PageExt> for PostForm {
-  type ApubType = PageExt;
-
   async fn from_apub(
     page: &PageExt,
     context: &LemmyContext,

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<Url>,
     request_counter: &mut i32,
   ) -> Result<PrivateMessage, LemmyError> {
-    let private_message_id = note.id_unchecked().context(location_info!())?.to_owned();
+    get_object_from_apub(note, context, expected_domain, request_counter).await
-    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)
-    }
   }
 }
 
 #[async_trait::async_trait(?Send)]
-impl FromApubToForm for PrivateMessageForm {
+impl FromApubToForm<NoteExt> for PrivateMessageForm {
-  type ApubType = NoteExt;
-
   async fn from_apub(
     note: &NoteExt,
     context: &LemmyContext,

lemmy_apub/src/objects/user.rs

@@ -116,9 +116,7 @@ impl FromApub for User_ {
 }
 
 #[async_trait::async_trait(?Send)]
-impl FromApubToForm for UserForm {
+impl FromApubToForm<PersonExt> for UserForm {
-  type ApubType = PersonExt;
-
   async fn from_apub(
     person: &PersonExt,
     _context: &LemmyContext,

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<PrivateMessageForm> for PrivateMessage {
   }
 }
 
+impl ApubObject for PrivateMessage {
+  fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result<Self, Error>
+  where
+    Self: Sized,
+  {
+    use crate::schema::private_message::dsl::*;
+    private_message
+      .filter(ap_id.eq(object_id))
+      .first::<Self>(conn)
+  }
+}
+
 impl PrivateMessage {
   pub fn update_ap_id(
     conn: &PgConnection,
@@ -68,13 +80,6 @@ impl PrivateMessage {
       .get_result::<Self>(conn)
   }
 
-  pub fn read_from_apub_id(conn: &PgConnection, object_id: &str) -> Result<Self, Error> {
-    use crate::schema::private_message::dsl::*;
-    private_message
-      .filter(ap_id.eq(object_id))
-      .first::<Self>(conn)
-  }
-
   pub fn update_content(
     conn: &PgConnection,
     private_message_id: i32,