Dont upsert Instance row every apub fetch (#2771)

This is not necessary because the domain cant change, so we only
need to insert if no row exists for this domain.

Also fetch instance actor when parsing person, not only community

Co-authored-by: Dessalines <dessalines@users.noreply.github.com>

crates/api/src/lib.rs

@@ -78,7 +78,9 @@ mod tests {
     let secret = Secret::init(pool).await.unwrap();
     let settings = &SETTINGS.to_owned();
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("Gerry9812".into())

crates/apub/src/collections/community_moderators.rs

@@ -156,7 +156,7 @@ mod tests {
     let community = parse_lemmy_community(&context).await;
     let community_id = community.id;
 
-    let inserted_instance = Instance::create(context.pool(), "my_domain.tld")
+    let inserted_instance = Instance::read_or_create(context.pool(), "my_domain.tld".to_string())
       .await
       .unwrap();
 

crates/apub/src/objects/community.rs

@@ -26,7 +26,6 @@ use lemmy_db_schema::{
   source::{
     actor_language::CommunityLanguage,
     community::{Community, CommunityUpdateForm},
-    instance::Instance,
   },
   traits::{ApubActor, Crud},
 };
@@ -136,10 +135,9 @@ impl ApubObject for ApubCommunity {
     context: &LemmyContext,
     request_counter: &mut i32,
   ) -> Result<ApubCommunity, LemmyError> {
-    let apub_id = group.id.inner().clone();
+    let instance_id = fetch_instance_actor_for_object(&group.id, context, request_counter).await?;
-    let instance = Instance::create_from_actor_id(context.pool(), &apub_id).await?;
 
-    let form = Group::into_insert_form(group.clone(), instance.id);
+    let form = Group::into_insert_form(group.clone(), instance_id);
     let languages = LanguageTag::to_language_id_multiple(group.language, context.pool()).await?;
 
     let community = Community::create(context.pool(), &form).await?;
@@ -165,8 +163,6 @@ impl ApubObject for ApubCommunity {
         .ok();
     }
 
-    fetch_instance_actor_for_object(community.actor_id(), context, request_counter).await;
-
     Ok(community)
   }
 }

crates/apub/src/objects/instance.rs

@@ -20,6 +20,7 @@ use activitystreams_kinds::actor::ApplicationType;
 use chrono::NaiveDateTime;
 use lemmy_api_common::{context::LemmyContext, utils::local_site_opt_to_slur_regex};
 use lemmy_db_schema::{
+  newtypes::InstanceId,
   source::{
     actor_language::SiteLanguage,
     instance::Instance as DbInstance,
@@ -134,8 +135,8 @@ impl ApubObject for ApubSite {
     data: &Self::DataType,
     _request_counter: &mut i32,
   ) -> Result<Self, LemmyError> {
-    let apub_id = apub.id.inner().clone();
+    let domain = apub.id.inner().domain().expect("group id has domain");
-    let instance = DbInstance::create_from_actor_id(data.pool(), &apub_id).await?;
+    let instance = DbInstance::read_or_create(data.pool(), domain.to_string()).await?;
 
     let site_form = SiteInsertForm {
       name: apub.name.clone(),
@@ -178,19 +179,29 @@ impl Actor for ApubSite {
   }
 }
 
-/// try to fetch the instance actor (to make things like instance rules available)
+/// Try to fetch the instance actor (to make things like instance rules available).
-pub(in crate::objects) async fn fetch_instance_actor_for_object(
+pub(in crate::objects) async fn fetch_instance_actor_for_object<T: Into<Url> + Clone>(
-  object_id: Url,
+  object_id: &T,
   context: &LemmyContext,
   request_counter: &mut i32,
-) {
+) -> Result<InstanceId, LemmyError> {
-  // try to fetch the instance actor (to make things like instance rules available)
+  let object_id: Url = object_id.clone().into();
   let instance_id = Site::instance_actor_id_from_url(object_id);
   let site = ObjectId::<ApubSite>::new(instance_id.clone())
     .dereference(context, local_instance(context).await, request_counter)
     .await;
-  if let Err(e) = site {
+  match site {
-    debug!("Failed to dereference site for {}: {}", instance_id, e);
+    Ok(s) => Ok(s.instance_id),
+    Err(e) => {
+      // Failed to fetch instance actor, its probably not a lemmy instance
+      debug!("Failed to dereference site for {}: {}", &instance_id, e);
+      let domain = instance_id.domain().expect("has domain");
+      Ok(
+        DbInstance::read_or_create(context.pool(), domain.to_string())
+          .await?
+          .id,
+      )
+    }
   }
 }
 

crates/apub/src/objects/person.rs

@@ -23,10 +23,7 @@ use lemmy_api_common::{
   utils::{generate_outbox_url, local_site_opt_to_slur_regex},
 };
 use lemmy_db_schema::{
-  source::{
+  source::person::{Person as DbPerson, PersonInsertForm, PersonUpdateForm},
-    instance::Instance,
-    person::{Person as DbPerson, PersonInsertForm, PersonUpdateForm},
-  },
   traits::{ApubActor, Crud},
   utils::naive_now,
 };
@@ -149,8 +146,7 @@ impl ApubObject for ApubPerson {
     context: &LemmyContext,
     request_counter: &mut i32,
   ) -> Result<ApubPerson, LemmyError> {
-    let apub_id = person.id.inner().clone();
+    let instance_id = fetch_instance_actor_for_object(&person.id, context, request_counter).await?;
-    let instance = Instance::create_from_actor_id(context.pool(), &apub_id).await?;
 
     let person_form = PersonInsertForm {
       name: person.preferred_username,
@@ -173,13 +169,10 @@ impl ApubObject for ApubPerson {
       inbox_url: Some(person.inbox.into()),
       shared_inbox_url: person.endpoints.map(|e| e.shared_inbox.into()),
       matrix_user_id: person.matrix_user_id,
-      instance_id: instance.id,
+      instance_id,
     };
     let person = DbPerson::create(context.pool(), &person_form).await?;
 
-    let actor_id = person.actor_id.clone().into();
-    fetch_instance_actor_for_object(actor_id, context, request_counter).await;
-
     Ok(person.into())
   }
 }

crates/db_schema/src/aggregates/comment_aggregates.rs

@@ -38,7 +38,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy_comment_agg".into())

crates/db_schema/src/aggregates/community_aggregates.rs

@@ -38,7 +38,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy_community_agg".into())

crates/db_schema/src/aggregates/person_aggregates.rs

@@ -38,7 +38,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy_user_agg".into())

crates/db_schema/src/aggregates/post_aggregates.rs

@@ -38,7 +38,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy_community_agg".into())

crates/db_schema/src/aggregates/site_aggregates.rs

@@ -35,7 +35,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy_site_agg".into())

crates/db_schema/src/impls/activity.rs

@@ -109,7 +109,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let creator_form = PersonInsertForm::builder()
       .name("activity_creator_ pm".into())

crates/db_schema/src/impls/actor_language.rs

@@ -404,7 +404,9 @@ mod tests {
   }
 
   async fn create_test_site(pool: &DbPool) -> (Site, Instance) {
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let site_form = SiteInsertForm::builder()
       .name("test site".to_string())

crates/db_schema/src/impls/comment.rs

@@ -277,7 +277,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("terry".into())

crates/db_schema/src/impls/comment_reply.rs

@@ -94,7 +94,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("terrylake".into())

crates/db_schema/src/impls/community.rs

@@ -423,7 +423,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("bobbee".into())

crates/db_schema/src/impls/federation_allowlist.rs

@@ -21,7 +21,7 @@ impl FederationAllowList {
 
             for domain in list {
               // Upsert all of these as instances
-              let instance = Instance::create_conn(conn, &domain).await?;
+              let instance = Instance::read_or_create_with_conn(conn, domain).await?;
 
               let form = FederationAllowListForm {
                 instance_id: instance.id,

crates/db_schema/src/impls/federation_blocklist.rs

@@ -21,7 +21,7 @@ impl FederationBlockList {
 
             for domain in list {
               // Upsert all of these as instances
-              let instance = Instance::create_conn(conn, &domain).await?;
+              let instance = Instance::read_or_create_with_conn(conn, domain).await?;
 
               let form = FederationBlockListForm {
                 instance_id: instance.id,

crates/db_schema/src/impls/instance.rs

@@ -6,36 +6,45 @@ use crate::{
 };
 use diesel::{dsl::insert_into, result::Error, ExpressionMethods, QueryDsl};
 use diesel_async::{AsyncPgConnection, RunQueryDsl};
-use url::Url;
 
 impl Instance {
-  async fn create_from_form_conn(
+  pub(crate) async fn read_or_create_with_conn(
     conn: &mut AsyncPgConnection,
-    form: &InstanceForm,
+    domain_: String,
   ) -> Result<Self, Error> {
-    // Do upsert on domain name conflict
+    use crate::schema::instance::domain;
+    // First try to read the instance row and return directly if found
+    let instance = instance::table
+      .filter(domain.eq(&domain_))
+      .first::<Self>(conn)
+      .await;
+    match instance {
+      Ok(i) => Ok(i),
+      Err(diesel::NotFound) => {
+        // Instance not in database yet, insert it
+        let form = InstanceForm::builder()
+          .domain(domain_)
+          .updated(Some(naive_now()))
+          .build();
         insert_into(instance::table)
-      .values(form)
+          .values(&form)
+          // Necessary because this method may be called concurrently for the same domain. This
+          // could be handled with a transaction, but nested transactions arent allowed
           .on_conflict(instance::domain)
           .do_update()
-      .set(form)
+          .set(&form)
           .get_result::<Self>(conn)
           .await
       }
-  pub async fn create(pool: &DbPool, domain: &str) -> Result<Self, Error> {
+      e => e,
+    }
+  }
+
+  /// Attempt to read Instance column for the given domain. If it doesnt exist, insert a new one.
+  /// There is no need for update as the domain of an existing instance cant change.
+  pub async fn read_or_create(pool: &DbPool, domain: String) -> Result<Self, Error> {
     let conn = &mut get_conn(pool).await?;
-    Self::create_conn(conn, domain).await
+    Self::read_or_create_with_conn(conn, domain).await
-  }
-  pub async fn create_from_actor_id(pool: &DbPool, actor_id: &Url) -> Result<Self, Error> {
-    let domain = actor_id.host_str().expect("actor id missing a domain");
-    Self::create(pool, domain).await
-  }
-  pub async fn create_conn(conn: &mut AsyncPgConnection, domain: &str) -> Result<Self, Error> {
-    let form = InstanceForm::builder()
-      .domain(domain.to_string())
-      .updated(Some(naive_now()))
-      .build();
-    Self::create_from_form_conn(conn, &form).await
   }
   pub async fn delete(pool: &DbPool, instance_id: InstanceId) -> Result<usize, Error> {
     let conn = &mut get_conn(pool).await?;
@@ -43,6 +52,7 @@ impl Instance {
       .execute(conn)
       .await
   }
+  #[cfg(test)]
   pub async fn delete_all(pool: &DbPool) -> Result<usize, Error> {
     let conn = &mut get_conn(pool).await?;
     diesel::delete(instance::table).execute(conn).await

crates/db_schema/src/impls/moderator.rs

@@ -549,7 +549,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_mod = PersonInsertForm::builder()
       .name("the mod".into())

crates/db_schema/src/impls/password_reset_request.rs

@@ -103,7 +103,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("thommy prw".into())

crates/db_schema/src/impls/person.rs

@@ -285,7 +285,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("holly".into())
@@ -342,7 +344,9 @@ mod tests {
   #[serial]
   async fn follow() {
     let pool = &build_db_pool_for_tests().await;
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let person_form_1 = PersonInsertForm::builder()
       .name("erich".into())

crates/db_schema/src/impls/person_mention.rs

@@ -98,7 +98,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("terrylake".into())

crates/db_schema/src/impls/post.rs

@@ -360,7 +360,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("jim".into())

crates/db_schema/src/impls/private_message.rs

@@ -111,7 +111,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let creator_form = PersonInsertForm::builder()
       .name("creator_pm".into())

crates/db_views/src/comment_report_view.rs

@@ -335,7 +335,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("timmy_crv".into())

crates/db_views/src/comment_view.rs

@@ -450,7 +450,9 @@ mod tests {
   }
 
   async fn init_data(pool: &DbPool) -> Data {
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("timmy".into())

crates/db_views/src/post_report_view.rs

@@ -311,7 +311,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person = PersonInsertForm::builder()
       .name("timmy_prv".into())

crates/db_views/src/post_view.rs

@@ -498,7 +498,9 @@ mod tests {
   }
 
   async fn init_data(pool: &DbPool) -> Data {
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let person_name = "tegan".to_string();
 

crates/db_views/src/private_message_report_view.rs

@@ -169,7 +169,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let new_person_1 = PersonInsertForm::builder()
       .name("timmy_mrv".into())

crates/db_views/src/registration_application_view.rs

@@ -186,7 +186,9 @@ mod tests {
   async fn test_crud() {
     let pool = &build_db_pool_for_tests().await;
 
-    let inserted_instance = Instance::create(pool, "my_domain.tld").await.unwrap();
+    let inserted_instance = Instance::read_or_create(pool, "my_domain.tld".to_string())
+      .await
+      .unwrap();
 
     let timmy_person_form = PersonInsertForm::builder()
       .name("timmy_rav".into())

src/code_migrations.rs

@@ -429,7 +429,7 @@ async fn initialize_local_site_2022_10_10(
     .expect("must have domain");
 
   // Upsert this to the instance table
-  let instance = Instance::create(pool, &domain).await?;
+  let instance = Instance::read_or_create(pool, domain).await?;
 
   if let Some(setup) = &settings.setup {
     let person_keypair = generate_actor_keypair()?;