Revert "Use trigger to generate apub URL in insert instead of update, and fix query planner options not being set when TLS is disabled (#4797)"

This reverts commit 78702b59fd.

crates/api_crud/src/comment/create.rs

@@ -8,18 +8,20 @@ use lemmy_api_common::{
   utils::{
     check_community_user_action,
     check_post_deleted_or_removed,
+    generate_local_apub_endpoint,
     get_url_blocklist,
     is_mod_or_admin,
     local_site_to_slur_regex,
     process_markdown,
     update_read_comments,
+    EndpointType,
   },
 };
 use lemmy_db_schema::{
   impls::actor_language::default_post_language,
   source::{
     actor_language::CommunityLanguage,
-    comment::{Comment, CommentInsertForm, CommentLike, CommentLikeForm},
+    comment::{Comment, CommentInsertForm, CommentLike, CommentLikeForm, CommentUpdateForm},
     comment_reply::{CommentReply, CommentReplyUpdateForm},
     local_site::LocalSite,
     person_mention::{PersonMention, PersonMentionUpdateForm},
@@ -123,7 +125,25 @@ pub async fn create_comment(
     .await
     .with_lemmy_type(LemmyErrorType::CouldntCreateComment)?;
 
+  // Necessary to update the ap_id
   let inserted_comment_id = inserted_comment.id;
+  let protocol_and_hostname = context.settings().get_protocol_and_hostname();
+
+  let apub_id = generate_local_apub_endpoint(
+    EndpointType::Comment,
+    &inserted_comment_id.to_string(),
+    &protocol_and_hostname,
+  )?;
+  let updated_comment = Comment::update(
+    &mut context.pool(),
+    inserted_comment_id,
+    &CommentUpdateForm {
+      ap_id: Some(apub_id),
+      ..Default::default()
+    },
+  )
+  .await
+  .with_lemmy_type(LemmyErrorType::CouldntCreateComment)?;
 
   // Scan the comment for user mentions, add those rows
   let mentions = scrape_text_for_mentions(&content);
@@ -150,7 +170,7 @@ pub async fn create_comment(
     .with_lemmy_type(LemmyErrorType::CouldntLikeComment)?;
 
   ActivityChannel::submit_activity(
-    SendActivityData::CreateComment(inserted_comment.clone()),
+    SendActivityData::CreateComment(updated_comment.clone()),
     &context,
   )
   .await?;

crates/api_crud/src/post/create.rs

@@ -8,11 +8,13 @@ use lemmy_api_common::{
   send_activity::SendActivityData,
   utils::{
     check_community_user_action,
+    generate_local_apub_endpoint,
     get_url_blocklist,
     honeypot_check,
     local_site_to_slur_regex,
     mark_post_as_read,
     process_markdown_opt,
+    EndpointType,
   },
 };
 use lemmy_db_schema::{
@@ -21,7 +23,7 @@ use lemmy_db_schema::{
     actor_language::CommunityLanguage,
     community::Community,
     local_site::LocalSite,
-    post::{Post, PostInsertForm, PostLike, PostLikeForm},
+    post::{Post, PostInsertForm, PostLike, PostLikeForm, PostUpdateForm},
   },
   traits::{Crud, Likeable},
   utils::diesel_url_create,
@@ -145,8 +147,26 @@ pub async fn create_post(
     .await
     .with_lemmy_type(LemmyErrorType::CouldntCreatePost)?;
 
+  let inserted_post_id = inserted_post.id;
+  let protocol_and_hostname = context.settings().get_protocol_and_hostname();
+  let apub_id = generate_local_apub_endpoint(
+    EndpointType::Post,
+    &inserted_post_id.to_string(),
+    &protocol_and_hostname,
+  )?;
+  let updated_post = Post::update(
+    &mut context.pool(),
+    inserted_post_id,
+    &PostUpdateForm {
+      ap_id: Some(apub_id),
+      ..Default::default()
+    },
+  )
+  .await
+  .with_lemmy_type(LemmyErrorType::CouldntCreatePost)?;
+
   generate_post_link_metadata(
-    inserted_post.clone(),
+    updated_post.clone(),
     custom_thumbnail.map(Into::into),
     |post| Some(SendActivityData::CreatePost(post)),
     Some(local_site),
@@ -169,11 +189,11 @@ pub async fn create_post(
 
   mark_post_as_read(person_id, post_id, &mut context.pool()).await?;
 
-  if let Some(url) = inserted_post.url.clone() {
+  if let Some(url) = updated_post.url.clone() {
     if community.visibility == CommunityVisibility::Public {
       spawn_try_task(async move {
         let mut webmention =
-          Webmention::new::<Url>(inserted_post.ap_id.clone().into(), url.clone().into())?;
+          Webmention::new::<Url>(updated_post.ap_id.clone().into(), url.clone().into())?;
         webmention.set_checked(true);
         match webmention
           .send()

crates/api_crud/src/private_message/create.rs

@@ -6,17 +6,19 @@ use lemmy_api_common::{
   send_activity::{ActivityChannel, SendActivityData},
   utils::{
     check_person_block,
+    generate_local_apub_endpoint,
     get_interface_language,
     get_url_blocklist,
     local_site_to_slur_regex,
     process_markdown,
     send_email_to_user,
+    EndpointType,
   },
 };
 use lemmy_db_schema::{
   source::{
     local_site::LocalSite,
-    private_message::{PrivateMessage, PrivateMessageInsertForm},
+    private_message::{PrivateMessage, PrivateMessageInsertForm, PrivateMessageUpdateForm},
   },
   traits::Crud,
 };
@@ -56,6 +58,24 @@ pub async fn create_private_message(
     .await
     .with_lemmy_type(LemmyErrorType::CouldntCreatePrivateMessage)?;
 
+  let inserted_private_message_id = inserted_private_message.id;
+  let protocol_and_hostname = context.settings().get_protocol_and_hostname();
+  let apub_id = generate_local_apub_endpoint(
+    EndpointType::PrivateMessage,
+    &inserted_private_message_id.to_string(),
+    &protocol_and_hostname,
+  )?;
+  PrivateMessage::update(
+    &mut context.pool(),
+    inserted_private_message.id,
+    &PrivateMessageUpdateForm {
+      ap_id: Some(apub_id),
+      ..Default::default()
+    },
+  )
+  .await
+  .with_lemmy_type(LemmyErrorType::CouldntCreatePrivateMessage)?;
+
   let view = PrivateMessageView::read(&mut context.pool(), inserted_private_message.id)
     .await?
     .ok_or(LemmyErrorType::CouldntFindPrivateMessage)?;

crates/db_schema/replaceable_schema/triggers.sql

@@ -564,10 +564,6 @@ BEGIN
     IF NOT (NEW.path ~ ('*.' || id)::lquery) THEN
         NEW.path = NEW.path || id;
     END IF;
-    -- Set local ap_id
-    IF NEW.local THEN
-        NEW.ap_id = coalesce(NEW.ap_id, r.local_url ('/comment/' || id));
-    END IF;
     RETURN NEW;
 END
 $$;
@@ -577,39 +573,3 @@ CREATE TRIGGER change_values
     FOR EACH ROW
     EXECUTE FUNCTION r.comment_change_values ();
 
-CREATE FUNCTION r.post_change_values ()
-    RETURNS TRIGGER
-    LANGUAGE plpgsql
-    AS $$
-BEGIN
-    -- Set local ap_id
-    IF NEW.local THEN
-        NEW.ap_id = coalesce(NEW.ap_id, r.local_url ('/post/' || NEW.id::text));
-    END IF;
-    RETURN NEW;
-END
-$$;
-
-CREATE TRIGGER change_values
-    BEFORE INSERT ON post
-    FOR EACH ROW
-    EXECUTE FUNCTION r.post_change_values ();
-
-CREATE FUNCTION r.private_message_change_values ()
-    RETURNS TRIGGER
-    LANGUAGE plpgsql
-    AS $$
-BEGIN
-    -- Set local ap_id
-    IF NEW.local THEN
-        NEW.ap_id = coalesce(NEW.ap_id, r.local_url ('/private_message/' || NEW.id::text));
-    END IF;
-    RETURN NEW;
-END
-$$;
-
-CREATE TRIGGER change_values
-    BEFORE INSERT ON private_message
-    FOR EACH ROW
-    EXECUTE FUNCTION r.private_message_change_values ();
-

crates/db_schema/replaceable_schema/utils.sql

@@ -57,13 +57,6 @@ BEGIN
 END;
 $$;
 
-CREATE FUNCTION r.local_url (url_path text)
-    RETURNS text
-    LANGUAGE sql
-    STABLE PARALLEL SAFE RETURN (
-current_setting('lemmy.protocol_and_hostname') || url_path
-);
-
 -- This function creates statement-level triggers for all operation types. It's designed this way
 -- because of these limitations:
 --   * A trigger that uses transition tables can only handle 1 operation type.

crates/db_schema/src/impls/comment.rs

@@ -223,7 +223,6 @@ mod tests {
   use diesel_ltree::Ltree;
   use pretty_assertions::assert_eq;
   use serial_test::serial;
-  use url::Url;
 
   #[tokio::test]
   #[serial]
@@ -274,12 +273,7 @@ mod tests {
       path: Ltree(format!("0.{}", inserted_comment.id)),
       published: inserted_comment.published,
       updated: None,
-      ap_id: Url::parse(&format!(
+      ap_id: inserted_comment.ap_id.clone(),
-        "https://lemmy-alpha/comment/{}",
-        inserted_comment.id
-      ))
-      .unwrap()
-      .into(),
       distinguished: false,
       local: true,
       language_id: LanguageId::default(),

crates/db_schema/src/impls/post.rs

@@ -390,7 +390,6 @@ mod tests {
   use pretty_assertions::assert_eq;
   use serial_test::serial;
   use std::collections::HashSet;
-  use url::Url;
 
   #[tokio::test]
   #[serial]
@@ -448,9 +447,7 @@ mod tests {
       embed_description: None,
       embed_video_url: None,
       thumbnail_url: None,
-      ap_id: Url::parse(&format!("https://lemmy-alpha/post/{}", inserted_post.id))
+      ap_id: inserted_post.ap_id.clone(),
-        .unwrap()
-        .into(),
       local: true,
       language_id: Default::default(),
       featured_community: false,

crates/db_schema/src/impls/private_message.rs

@@ -100,7 +100,6 @@ mod tests {
   };
   use pretty_assertions::assert_eq;
   use serial_test::serial;
-  use url::Url;
 
   #[tokio::test]
   #[serial]
@@ -139,12 +138,7 @@ mod tests {
       read: false,
       updated: None,
       published: inserted_private_message.published,
-      ap_id: Url::parse(&format!(
+      ap_id: inserted_private_message.ap_id.clone(),
-        "https://lemmy-alpha/private_message/{}",
-        inserted_private_message.id
-      ))
-      .unwrap()
-      .into(),
       local: true,
     };
 

crates/db_schema/src/utils.rs

@@ -22,8 +22,7 @@ use diesel_async::{
     AsyncDieselConnectionManager,
     ManagerConfig,
   },
-  AsyncConnection,
+  SimpleAsyncConnection,
-  RunQueryDsl,
 };
 use futures_util::{future::BoxFuture, Future, FutureExt};
 use i_love_jesus::CursorKey;
@@ -324,46 +323,30 @@ pub fn diesel_url_create(opt: Option<&str>) -> LemmyResult<Option<DbUrl>> {
 
 fn establish_connection(config: &str) -> BoxFuture<ConnectionResult<AsyncPgConnection>> {
   let fut = async {
-    // We only support TLS with sslmode=require currently
+    let rustls_config = DangerousClientConfigBuilder {
-    let mut conn = if config.contains("sslmode=require") {
+      cfg: ClientConfig::builder(),
-      let rustls_config = DangerousClientConfigBuilder {
+    }
-        cfg: ClientConfig::builder(),
+    .with_custom_certificate_verifier(Arc::new(NoCertVerifier {}))
+    .with_no_client_auth();
+
+    let tls = tokio_postgres_rustls::MakeRustlsConnect::new(rustls_config);
+    let (client, conn) = tokio_postgres::connect(config, tls)
+      .await
+      .map_err(|e| ConnectionError::BadConnection(e.to_string()))?;
+    tokio::spawn(async move {
+      if let Err(e) = conn.await {
+        error!("Database connection failed: {e}");
       }
-      .with_custom_certificate_verifier(Arc::new(NoCertVerifier {}))
+    });
-      .with_no_client_auth();
+    let mut conn = AsyncPgConnection::try_from(client).await?;
-
+    // * Change geqo_threshold back to default value if it was changed, so it's higher than the
-      let tls = tokio_postgres_rustls::MakeRustlsConnect::new(rustls_config);
+    //   collapse limits
-      let (client, conn) = tokio_postgres::connect(config, tls)
+    // * Change collapse limits from 8 to 11 so the query planner can find a better table join order
-        .await
+    //   for more complicated queries
-        .map_err(|e| ConnectionError::BadConnection(e.to_string()))?;
+    conn
-      tokio::spawn(async move {
+      .batch_execute("SET geqo_threshold=12;SET from_collapse_limit=11;SET join_collapse_limit=11;")
-        if let Err(e) = conn.await {
+      .await
-          error!("Database connection failed: {e}");
+      .map_err(ConnectionError::CouldntSetupConfiguration)?;
-        }
-      });
-      AsyncPgConnection::try_from(client).await?
-    } else {
-      AsyncPgConnection::establish(config).await?
-    };
-
-    diesel::select((
-      // Change geqo_threshold back to default value if it was changed, so it's higher than the
-      // collapse limits
-      functions::set_config("geqo_threshold", "12", false),
-      // Change collapse limits from 8 to 11 so the query planner can find a better table join
-      // order for more complicated queries
-      functions::set_config("from_collapse_limit", "11", false),
-      functions::set_config("join_collapse_limit", "11", false),
-      // Set `lemmy.protocol_and_hostname` so triggers can use it
-      functions::set_config(
-        "lemmy.protocol_and_hostname",
-        SETTINGS.get_protocol_and_hostname(),
-        false,
-      ),
-    ))
-    .execute(&mut conn)
-    .await
-    .map_err(ConnectionError::CouldntSetupConfiguration)?;
     Ok(conn)
   };
   fut.boxed()
@@ -422,11 +405,17 @@ impl ServerCertVerifier for NoCertVerifier {
 
 pub async fn build_db_pool() -> LemmyResult<ActualDbPool> {
   let db_url = SETTINGS.get_database_url();
-  // diesel-async does not support any TLS connections out of the box, so we need to manually
+  // We only support TLS with sslmode=require currently
-  // provide a setup function which handles creating the connection
+  let tls_enabled = db_url.contains("sslmode=require");
-  let mut config = ManagerConfig::default();
+  let manager = if tls_enabled {
-  config.custom_setup = Box::new(establish_connection);
+    // diesel-async does not support any TLS connections out of the box, so we need to manually
-  let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new_with_config(&db_url, config);
+    // provide a setup function which handles creating the connection
+    let mut config = ManagerConfig::default();
+    config.custom_setup = Box::new(establish_connection);
+    AsyncDieselConnectionManager::<AsyncPgConnection>::new_with_config(&db_url, config)
+  } else {
+    AsyncDieselConnectionManager::<AsyncPgConnection>::new(&db_url)
+  };
   let pool = Pool::builder(manager)
     .max_size(SETTINGS.database.pool_size)
     .runtime(Runtime::Tokio1)
@@ -483,7 +472,7 @@ static EMAIL_REGEX: LazyLock<Regex> = LazyLock::new(|| {
 });
 
 pub mod functions {
-  use diesel::sql_types::{BigInt, Bool, Text, Timestamptz};
+  use diesel::sql_types::{BigInt, Text, Timestamptz};
 
   sql_function! {
     #[sql_name = "r.hot_rank"]
@@ -506,8 +495,6 @@ pub mod functions {
 
   // really this function is variadic, this just adds the two-argument version
   sql_function!(fn coalesce<T: diesel::sql_types::SqlType + diesel::sql_types::SingleValue>(x: diesel::sql_types::Nullable<T>, y: T) -> T);
-
-  sql_function!(fn set_config(setting_name: Text, new_value: Text, is_local: Bool) -> Text);
 }
 
 pub const DELETED_REPLACEMENT_TEXT: &str = "*Permanently Deleted*";

migrations/2024-06-24-000000_ap_id_triggers/down.sql

@@ -1,9 +0,0 @@
-ALTER TABLE comment
-    ALTER COLUMN ap_id SET DEFAULT generate_unique_changeme ();
-
-ALTER TABLE post
-    ALTER COLUMN ap_id SET DEFAULT generate_unique_changeme ();
-
-ALTER TABLE private_message
-    ALTER COLUMN ap_id SET DEFAULT generate_unique_changeme ();
-

migrations/2024-06-24-000000_ap_id_triggers/up.sql

@@ -1,9 +0,0 @@
-ALTER TABLE comment
-    ALTER COLUMN ap_id DROP DEFAULT;
-
-ALTER TABLE post
-    ALTER COLUMN ap_id DROP DEFAULT;
-
-ALTER TABLE private_message
-    ALTER COLUMN ap_id DROP DEFAULT;
-