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

* Update create.rs

* Update utils.rs

* Update utils.sql

* Update triggers.sql

* Update utils.sql

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Update create.rs

* Create up.sql

* Update up.sql

* Update triggers.sql

* Update utils.rs

* stuff

* stuff

* revert some changed files

* Revert "revert some changed files"

This reverts commit 028eabb4bd.

* revert the correct files

* partial reverts

* migration, tests, fix establish_connection

* lint

* pg_format
This commit is contained in:
dullbananas 2024-07-02 08:23:21 -07:00 committed by GitHub
parent 117a8b42c8
commit 78702b59fd
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
11 changed files with 142 additions and 109 deletions

View file

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

View file

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

View file

@ -6,19 +6,17 @@ use lemmy_api_common::{
send_activity::{ActivityChannel, SendActivityData}, send_activity::{ActivityChannel, SendActivityData},
utils::{ utils::{
check_person_block, check_person_block,
generate_local_apub_endpoint,
get_interface_language, get_interface_language,
get_url_blocklist, get_url_blocklist,
local_site_to_slur_regex, local_site_to_slur_regex,
process_markdown, process_markdown,
send_email_to_user, send_email_to_user,
EndpointType,
}, },
}; };
use lemmy_db_schema::{ use lemmy_db_schema::{
source::{ source::{
local_site::LocalSite, local_site::LocalSite,
private_message::{PrivateMessage, PrivateMessageInsertForm, PrivateMessageUpdateForm}, private_message::{PrivateMessage, PrivateMessageInsertForm},
}, },
traits::Crud, traits::Crud,
}; };
@ -58,24 +56,6 @@ pub async fn create_private_message(
.await .await
.with_lemmy_type(LemmyErrorType::CouldntCreatePrivateMessage)?; .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) let view = PrivateMessageView::read(&mut context.pool(), inserted_private_message.id)
.await? .await?
.ok_or(LemmyErrorType::CouldntFindPrivateMessage)?; .ok_or(LemmyErrorType::CouldntFindPrivateMessage)?;

View file

@ -564,6 +564,10 @@ BEGIN
IF NOT (NEW.path ~ ('*.' || id)::lquery) THEN IF NOT (NEW.path ~ ('*.' || id)::lquery) THEN
NEW.path = NEW.path || id; NEW.path = NEW.path || id;
END IF; 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; RETURN NEW;
END END
$$; $$;
@ -573,3 +577,39 @@ CREATE TRIGGER change_values
FOR EACH ROW FOR EACH ROW
EXECUTE FUNCTION r.comment_change_values (); 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 ();

View file

@ -57,6 +57,13 @@ BEGIN
END; 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 -- This function creates statement-level triggers for all operation types. It's designed this way
-- because of these limitations: -- because of these limitations:
-- * A trigger that uses transition tables can only handle 1 operation type. -- * A trigger that uses transition tables can only handle 1 operation type.

View file

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

View file

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

View file

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

View file

@ -30,7 +30,8 @@ use diesel_async::{
AsyncDieselConnectionManager, AsyncDieselConnectionManager,
ManagerConfig, ManagerConfig,
}, },
SimpleAsyncConnection, AsyncConnection,
RunQueryDsl,
}; };
use futures_util::{future::BoxFuture, Future, FutureExt}; use futures_util::{future::BoxFuture, Future, FutureExt};
use i_love_jesus::CursorKey; use i_love_jesus::CursorKey;
@ -332,34 +333,50 @@ pub fn diesel_url_create(opt: Option<&str>) -> LemmyResult<Option<DbUrl>> {
fn establish_connection(config: &str) -> BoxFuture<ConnectionResult<AsyncPgConnection>> { fn establish_connection(config: &str) -> BoxFuture<ConnectionResult<AsyncPgConnection>> {
let fut = async { let fut = async {
rustls::crypto::ring::default_provider() // We only support TLS with sslmode=require currently
.install_default() let mut conn = if config.contains("sslmode=require") {
.expect("Failed to install rustls crypto provider"); rustls::crypto::ring::default_provider()
.install_default()
.expect("Failed to install rustls crypto provider");
let rustls_config = DangerousClientConfigBuilder { let rustls_config = DangerousClientConfigBuilder {
cfg: ClientConfig::builder(), 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 {}))
let mut conn = AsyncPgConnection::try_from(client).await?; .with_no_client_auth();
// * Change geqo_threshold back to default value if it was changed, so it's higher than the
// collapse limits let tls = tokio_postgres_rustls::MakeRustlsConnect::new(rustls_config);
// * Change collapse limits from 8 to 11 so the query planner can find a better table join order let (client, conn) = tokio_postgres::connect(config, tls)
// for more complicated queries .await
conn .map_err(|e| ConnectionError::BadConnection(e.to_string()))?;
.batch_execute("SET geqo_threshold=12;SET from_collapse_limit=11;SET join_collapse_limit=11;") tokio::spawn(async move {
.await if let Err(e) = conn.await {
.map_err(ConnectionError::CouldntSetupConfiguration)?; error!("Database connection failed: {e}");
}
});
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) Ok(conn)
}; };
fut.boxed() fut.boxed()
@ -418,17 +435,11 @@ impl ServerCertVerifier for NoCertVerifier {
pub async fn build_db_pool() -> LemmyResult<ActualDbPool> { pub async fn build_db_pool() -> LemmyResult<ActualDbPool> {
let db_url = SETTINGS.get_database_url(); let db_url = SETTINGS.get_database_url();
// We only support TLS with sslmode=require currently // diesel-async does not support any TLS connections out of the box, so we need to manually
let tls_enabled = db_url.contains("sslmode=require"); // provide a setup function which handles creating the connection
let manager = if tls_enabled { let mut config = ManagerConfig::default();
// diesel-async does not support any TLS connections out of the box, so we need to manually config.custom_setup = Box::new(establish_connection);
// provide a setup function which handles creating the connection let manager = AsyncDieselConnectionManager::<AsyncPgConnection>::new_with_config(&db_url, config);
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) let pool = Pool::builder(manager)
.max_size(SETTINGS.database.pool_size) .max_size(SETTINGS.database.pool_size)
.runtime(Runtime::Tokio1) .runtime(Runtime::Tokio1)
@ -485,7 +496,7 @@ static EMAIL_REGEX: Lazy<Regex> = Lazy::new(|| {
}); });
pub mod functions { pub mod functions {
use diesel::sql_types::{BigInt, Text, Timestamptz}; use diesel::sql_types::{BigInt, Bool, Text, Timestamptz};
sql_function! { sql_function! {
#[sql_name = "r.hot_rank"] #[sql_name = "r.hot_rank"]
@ -508,6 +519,8 @@ pub mod functions {
// really this function is variadic, this just adds the two-argument version // 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 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*"; pub const DELETED_REPLACEMENT_TEXT: &str = "*Permanently Deleted*";

View file

@ -0,0 +1,9 @@
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 ();

View file

@ -0,0 +1,9 @@
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;